grpc-gateway
grpc-gateway copied to clipboard
Required properties of message types get optional in OpenAPI
🐛 Bug Report
Message properties marked with [(google.api.field_behavior) = REQUIRED]
get ignored when generating OpenAPI spec, if properties reference other messages (schemas).
To Reproduce
Create a proto with the following definition:
message Vendor {
uint32 id = 1 [(google.api.field_behavior) = REQUIRED];
}
message CreateVendorRequest {
Vendor vendor = 1 [(google.api.field_behavior) = REQUIRED];
}
And generate anOpenApi spec. It will contain:
"CreateVendorRequest": {
"type": "object",
"properties": {
"vendor": {
"$ref": "#/definitions/Vendor"
}
}
},
i.e. the required: [ "vendor" ]
part is missed.
Expected behavior
"CreateVendorRequest": {
"type": "object",
"properties": {
"vendor": {
"$ref": "#/definitions/Vendor"
}
},
"required": [
"vendor"
]
},
Actual Behavior
The required part is missed.
Your Environment
Linux 2.11.1
Thanks for your issue. This is only when referencing messages that are imported, is that correct?
Yep, scalars and "inlined" arrays (not in other schemas) seem to get processed correctly.
Ok, thanks for confirming. Would you be interested in contributing a fix for this? The API fields configuration feature is still a bit new so there will be bugs like this (see also #2829).
@johanbrandhorst Thanks for the quick replies. I'm not qualified enough for that work, unfortunately, my field is JavaScript. But I'll let my teammates know about such an opportunity :)
UPD. One of my colleagues told me that it could sound like banter, but actually no. I really meant what I said.
Yep, scalars and "inlined" arrays (not in other schemas) seem to get processed correctly.
I think scalars are actually incorrectly handled but this is covered by issue #2635
I also noticed that if a property is of an array message type, it gets rendered as required, e.g.:
export interface Pbv1ActionReasonsResponseV1 {
reasons: Pbv1ActionReason[]
}
but if I remove repeated
, then it becomes non-required:
export interface Pbv1ActionReasonsResponseV1 {
reason?: Pbv1ActionReason
}
I would like to contribute to fix this issue. Thank you!!
Feel free to start work on this. The first step is usually adding a failing test or example.
I created a small and naive PR for this but it works as far as I can see. Happy for review by a maintainer. (I'll be filling out the CLA soon.)