Required properties of message types get optional in OpenAPI

[OnkelTem]

🐛 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

[johanbrandhorst]

Thanks for your issue. This is only when referencing messages that are imported, is that correct?

[OnkelTem]

Yep, scalars and "inlined" arrays (not in other schemas) seem to get processed correctly.

[johanbrandhorst]

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).

[OnkelTem]

@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.

[tmccnnll]

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

[OnkelTem]

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
}

[Rajarshi1998]

I would like to contribute to fix this issue. Thank you!!

[johanbrandhorst]

Feel free to start work on this. The first step is usually adding a failing test or example.

[gostajonasson]

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.)