protoc-gen-openapiv2 does not respect json_name when generating the required properties

[patrick246]

🐛 Bug Report

Starting with protoc-gen-openapiv2 v2.8.0, the generated OpenAPIv2 schema is generated incorrectly when using the grpc.gateway.protoc_gen_openapiv2.options.openapiv2_schema option, together with json_name. The properties section uses the name from the json_name annotation, but the required section uses the autogenerated name instead.

To Reproduce

I've tested this with multiple versions, the behavior between latest (v2.11.3 currently) and v2.8.0 does not change, this is the first version with the current behavior.

Protobuf file:

syntax = "proto3";

option go_package = "pkg/api";

import "protoc-gen-openapiv2/options/annotations.proto";
import "google/api/annotations.proto";

service Test {
  rpc Test(Required) returns (Required) {
      option (google.api.http) = {
        get: "/test"
      };
  };
}

message Required {
  option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_schema) = {
    json_schema: {
      required: [ "test_id"]
    }
  };
  string test_id = 1 [json_name="test_id"];
}

Project setup for v2.8.0:

go install github.com/grpc-ecosystem/grpc-gateway/v2/[email protected] github.com/grpc-ecosystem/grpc-gateway/v2/[email protected]
mkdir -p ./protoc-gen-openapiv2/options/
curl -o ./protoc-gen-openapiv2/options/annotations.proto https://raw.githubusercontent.com/grpc-ecosystem/grpc-gateway/v2.8.0/protoc-gen-openapiv2/options/annotations.proto
curl -o ./protoc-gen-openapiv2/options/openapiv2.proto https://raw.githubusercontent.com/grpc-ecosystem/grpc-gateway/v2.8.0/protoc-gen-openapiv2/options/openapiv2.proto
mkdir -p ./google/api/
curl -o ./google/api/http.proto https://raw.githubusercontent.com/googleapis/googleapis/master/google/api/http.proto
curl -o ./google/api/annotations.proto https://raw.githubusercontent.com/googleapis/googleapis/master/google/api/annotations.proto
protoc -I . --openapiv2_out ./ required.proto

Project setup for v2.7.3:

go install github.com/grpc-ecosystem/grpc-gateway/v2/[email protected] github.com/grpc-ecosystem/grpc-gateway/v2/[email protected]
mkdir -p ./protoc-gen-openapiv2/options/
curl -o ./protoc-gen-openapiv2/options/annotations.proto https://raw.githubusercontent.com/grpc-ecosystem/grpc-gateway/v2.7.3/protoc-gen-openapiv2/options/annotations.proto
curl -o ./protoc-gen-openapiv2/options/openapiv2.proto https://raw.githubusercontent.com/grpc-ecosystem/grpc-gateway/v2.7.3/protoc-gen-openapiv2/options/openapiv2.proto
mkdir -p ./google/api/
curl -o ./google/api/http.proto https://raw.githubusercontent.com/googleapis/googleapis/master/google/api/http.proto
curl -o ./google/api/annotations.proto https://raw.githubusercontent.com/googleapis/googleapis/master/google/api/annotations.proto
protoc -I . --openapiv2_out ./ required.proto

Expected behavior

The generated OpenAPIv2 spec is generated like in v2.7.3:

{
  "swagger": "2.0",
  "info": {
    "title": "required.proto",
    "version": "version not set"
  },
  "tags": [
    {
      "name": "Test"
    }
  ],
  "consumes": [
    "application/json"
  ],
  "produces": [
    "application/json"
  ],
  "paths": {
    "/test": {
      "get": {
        "operationId": "Test_Test",
        "responses": {
          "200": {
            "description": "A successful response.",
            "schema": {
              "$ref": "#/definitions/Required"
            }
          },
          "default": {
            "description": "An unexpected error response.",
            "schema": {
              "$ref": "#/definitions/rpcStatus"
            }
          }
        },
        "parameters": [
          {
            "name": "test_id",
            "in": "query",
            "required": false,
            "type": "string"
          }
        ],
        "tags": [
          "Test"
        ]
      }
    }
  },
  "definitions": {
    "Required": {
      "type": "object",
      "properties": {
        "test_id": {
          "type": "string"
        }
      },
      "required": [
        "test_id"
      ]
    },
    "protobufAny": {
      "type": "object",
      "properties": {
        "@type": {
          "type": "string"
        }
      },
      "additionalProperties": {}
    },
    "rpcStatus": {
      "type": "object",
      "properties": {
        "code": {
          "type": "integer",
          "format": "int32"
        },
        "message": {
          "type": "string"
        },
        "details": {
          "type": "array",
          "items": {
            "$ref": "#/definitions/protobufAny"
          }
        }
      }
    }
  }
}

Actual Behavior

Instead the required section of the generated schema references a property with the wrong (autogenerated) name:

{
  "swagger": "2.0",
  "info": {
    "title": "required.proto",
    "version": "version not set"
  },
  "tags": [
    {
      "name": "Test"
    }
  ],
  "consumes": [
    "application/json"
  ],
  "produces": [
    "application/json"
  ],
  "paths": {
    "/test": {
      "get": {
        "operationId": "Test_Test",
        "responses": {
          "200": {
            "description": "A successful response.",
            "schema": {
              "$ref": "#/definitions/Required"
            }
          },
          "default": {
            "description": "An unexpected error response.",
            "schema": {
              "$ref": "#/definitions/rpcStatus"
            }
          }
        },
        "parameters": [
          {
            "name": "test_id",
            "in": "query",
            "required": false,
            "type": "string"
          }
        ],
        "tags": [
          "Test"
        ]
      }
    }
  },
  "definitions": {
    "Required": {
      "type": "object",
      "properties": {
        "test_id": {
          "type": "string"
        }
      },
      "required": [
        "testId"
      ]
    },
    "protobufAny": {
      "type": "object",
      "properties": {
        "@type": {
          "type": "string"
        }
      },
      "additionalProperties": {}
    },
    "rpcStatus": {
      "type": "object",
      "properties": {
        "code": {
          "type": "integer",
          "format": "int32"
        },
        "message": {
          "type": "string"
        },
        "details": {
          "type": "array",
          "items": {
            "$ref": "#/definitions/protobufAny"
          }
        }
      }
    }
  }
}

Diff between expected and actual:

--- expected.json       2022-09-06 13:02:03.000000000 +0200
+++ actual.json 2022-09-06 12:56:06.000000000 +0200
@@ -56,7 +56,7 @@
         }
       },
       "required": [
-        "test_id"
+        "testId"
       ]
     },
     "protobufAny": {

Your Environment

MacOS Big Sur (11.6.8), go1.19 darwin/amd64, grpc-gateway versions v2.11.3, v2.8.0 and v2.7.3

[patrick246]

Found a TODO comment for exactly this issue:

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

Line 2599 in 6efdab1

// TODO(oyvindwe): Look up field and use field.GetJsonName()?

It also seems that it only accidentally worked for us in 2.7.3 and before, as we always had protobuf name = json name.

[johanbrandhorst]

Thanks for the bug report, and sorry about breaking your use case 😞. It sounds like you're already making good progress on a fix, is there anything I can do to help?

CC @oyvindwe

[oyvindwe]

I investigated this when working on PR #2553. This was a partial fix for some cases where required would contain snake_case fields when it shouldn't: #2553 (comment)

[patrick246]

Thanks for the bug report, and sorry about breaking your use case 😞.

No worries, it looks like it was already broken before, just in the other direction.

It sounds like you're already making good progress on a fix, is there anything I can do to help?

My current plan would be to move the conversion of the required fields part out of updateswaggerObjectFromJSONSchema and into renderMessageAsDefinition, as we have access to msg.Fields there. Does this sound like a good approach?

[johanbrandhorst]

@patrick246 I wish I had enough context on this code to say, but throw up a PR with some tests and lets see!