protoc_gen_openapiv2: Required field does not take effect under HTTP-GET

[wuxingzhong]

🐛 Bug Report

syntax = "proto3";
package test.v1;
import "google/api/annotations.proto";
import "google/protobuf/empty.proto";
import "protoc-gen-openapiv2/options/annotations.proto";

option go_package = "api/test/v1;v1";
service TestsService {

    //  GetTest
    rpc GetTest (GetTestRequest) returns (google.protobuf.Empty){
        option (google.api.http) = {
        get: "/test"
        };
    }
}

// GetTestRequest
message GetTestRequest {
    option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_schema) = {
        json_schema: {
            required: ["id"],
        }
    };
    // id.
    string id =1 ;
    // name.
    string name = 2;
}

To Reproduce

protoc --proto_path=.  --openapiv2_out . --openapiv2_opt logtostderr=true test.proto 

Expected behavior

in test.swagger.json field id is required

        "parameters": [
          {
            "name": "id",
            "description": "id.",
            "in": "query",
            "required": true,
            "type": "string"
          }

Actual Behavior

        "parameters": [
          {
            "name": "id",
            "description": "id.",
            "in": "query",
            "required": false,
            "type": "string"
          }

[stale[bot]]

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.

[johanbrandhorst]

Sorry for the "wontfix" label, that was wrong. I think there is a discussion to be had here though - you want to mark a query parameter as required. What do you want to happen when that parameter is omitted from the request? GET /test is a completely valid request. I don't know that the behavior here is wrong, but what we could do is add a warning to the generator when the user does this. What do you think?

[johanbrandhorst]

Scratch that, apparently the OpenAPIv2 spec explicitly calls out that query parameters can be required: https://swagger.io/docs/specification/describing-parameters/. I guess this is a bug then!

[MakDon]

@johanbrandhorst hello, I will work on it :D