grpc-gateway
grpc-gateway copied to clipboard
protoc-gen-openapiv2: `google.api.field_behavior = REQUIRED` annotates the wrong schema object
🐛 Bug Report
protoc-gen-openapiv2 offers to mark properties as required based on the google.api.field_behavior
field option, but it adds the "required": ["<property>"],
annotation to the property itself in addition to adding it to the object containing the property.
https://grpc-ecosystem.github.io/grpc-gateway/docs/mapping/customizing_openapi_output/#using-googleapifield_behavior
To Reproduce
import "protoc-gen-openapiv2/options/annotations.proto";
import "google/api/field_behavior.proto";
message TestMessage {
option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_schema) = {
json_schema: { required: [ "fieldOne", "fieldTwo" ] }
};
string field_one = 1;
string field_two = 2 [(google.api.field_behavior) = REQUIRED];
string field_three = 3 [(google.api.field_behavior) = REQUIRED];
}
Run protoc-gen-openapiv2
.
Expected Output
"testingTestMessage": {
"type": "object",
"properties": {
"fieldOne": {
"type": "string"
},
"fieldTwo": {
"type": "string"
},
"fieldThree": {
"type": "string"
}
},
"required": [
"fieldOne",
"fieldTwo",
"fieldThree"
]
}
Actual Output
"testingTestMessage": {
"type": "object",
"properties": {
"fieldOne": {
"type": "string"
},
"fieldTwo": {
"type": "string",
"required": [ <-- wrong
"fieldTwo"
]
},
"fieldThree": {
"type": "string",
"required": [ <-- wrong
"fieldThree"
]
}
},
"required": [
"fieldOne",
"fieldTwo",
"fieldTwo", <-- duplicate
"fieldThree"
]
}
Your Environment
github.com/grpc-ecosystem/grpc-gateway/v2 v2.10.0
Hi, thanks for the issue! That definitely looks wrong. If you're interested in helping to contribute a fix, the first step is usually to create a failing test. It looks like with what you've got here that shouldn't be too hard at all. Would you be interested in contributing a fix?
CC @ewhauser
I am looking to contribute to this issue please assign it to me and I kind of know where to change. where to write the unit tests for change? I am planning to make the updates in the below function https://github.com/grpc-ecosystem/grpc-gateway/blob/5d41954a8eb8075aef0c72842296da38a151411d/protoc-gen-openapiv2/internal/genopenapi/template.go#L420
CC @johanbrandhorst
Thank you @lakshkeswani, I think we usually add tests to template_test
in the same directory, but it is a bit of a mess so feel free to think about other ways to test it if you like. We should also see if we have examples of this in the current code already, and if not, introduce an example that reproduces this bug. The code was introduced in https://github.com/grpc-ecosystem/grpc-gateway/pull/1806 and it seems like we added some REQUIRED fields in that PR. Maybe see if they already exhibit the wrong behavior?
This was fixed by #2769. Lets reopen if it persists.