protoc-gen-openapiv2: google.api.field_behavior = REQUIRED annotates the wrong schema object

[cloneable]

🐛 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

[johanbrandhorst]

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

[lakshkeswani]

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

grpc-gateway/protoc-gen-openapiv2/internal/genopenapi/template.go

Line 420 in 5d41954

func renderMessageAsDefinition(msg *descriptor.Message, reg *descriptor.Registry, customRefs refMap, pathParams []descriptor.Parameter) (openapiSchemaObject, error) {

[lakshkeswani]

CC @johanbrandhorst

[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 #1806 and it seems like we added some REQUIRED fields in that PR. Maybe see if they already exhibit the wrong behavior?

[johanbrandhorst]

This was fixed by #2769. Lets reopen if it persists.