protoc-gen-openapiv2 v2.2.0 only generates 'required' for scalar fields when using field_behavior REQUIRED

[bconway]

🐛 Bug Report

I just updated from v2.1.0 to v2.2.0 and regenerated (no other changes), and it appears that when multiple fields in a message have (google.api.field_behavior) = REQUIRED set, only the first field is being included in the OpenAPI definitions section as required. Diffing against my previous results, I see a bunch of updates to messages like the following:

...
       "description": "User represents a user...",
       "required": [
-        "email",
-        "role",
-        "status"
+        "email"
       ]
...

...
       "description": "Device represents a device...",
       "required": [
-        "uniqID",
-        "status"
+        "uniqID"
       ]
...

To Reproduce

(Write your steps here:)

1. Create a message with multiple fields with (google.api.field_behavior) = REQUIRED set.
2. Generate OpenAPI output. Current options are: --openapiv2_out=../openapi 0-openapiv2_opt=allow_merge=true,merge_file_name=merged

Expected behavior

All fields marked REQUIRED should be in the required array.

Actual Behavior

Only the first field is included (I think?)

Your Environment

Docker, latest stable Alpine, Go 1.15, latest stable packages, v2.2.0 grpc-gateway.

[johanbrandhorst]

I'm guessing this might be due to #1944 which fixed #1937 (CC @gganley). Does this affect your use case?

[gganley]

Hmm smells similar to what I did for sure, @bconway would you mind providing a minimum working proto file to demonstrate? Like I did in #1937?

[bconway]

Sure, below is an example. After further investigation, it looks like scalar fields are included correctly in the required array, but not messages as fields. I will update the title accordingly.

syntax = "proto3";
package api;

import "google/api/annotations.proto";
import "google/api/field_behavior.proto";

service UserService {
  rpc CreateUser(CreateUserRequest) returns (User) {
    option (google.api.http) = {
      post: "/v1/users"
      body: "user"
    };
  }
}

message User {
  string email = 1 [(google.api.field_behavior) = REQUIRED];
  Role role = 2 [(google.api.field_behavior) = REQUIRED];
  Status status = 3 [(google.api.field_behavior) = REQUIRED];
  string email2 = 4 [(google.api.field_behavior) = REQUIRED];
}

message CreateUserRequest {
  User user = 1 [(google.api.field_behavior) = REQUIRED];
}

enum Role {
  ROLE_UNSPECIFIED = 0;
}

enum Status {
  STATUS_UNSPECIFIED = 0;
}

Relevant portion of the OpenAPI output under v2.2.0:

    "apiUser": {
      "type": "object",
      "properties": {
        "email": {
          "type": "string",
          "required": [
            "email"
          ]
        },
        "role": {
          "$ref": "#/definitions/apiRole"
        },
        "status": {
          "$ref": "#/definitions/apiStatus"
        },
        "email2": {
          "type": "string",
          "required": [
            "email2"
          ]
        }
      },
      "required": [
        "email",
        "email2"
      ]
    },

What it looks like under v2.1.0:

    "apiUser": {
      "type": "object",
      "properties": {
        "email": {
          "type": "string",
          "required": [
            "email"
          ]
        },
        "role": {
          "$ref": "#/definitions/apiRole",
          "required": [
            "role"
          ]
        },
        "status": {
          "$ref": "#/definitions/apiStatus",
          "required": [
            "status"
          ]
        },
        "email2": {
          "type": "string",
          "required": [
            "email2"
          ]
        }
      },
      "required": [
        "email",
        "role",
        "status",
        "email2"
      ]

I believe the correct behavior here would be to remove the required key from the role and status maps under properties, but to leave them as items in the required array that follows?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.