protoc-gen-swagger does not parse query params of a DELETE HTTP rpc definition into swagger.json file

[zwtj]

Hi,

I defined a proto file called abc.proto as follows:

syntax="proto3";

import "google/api/annotations.proto";
import "google/protobuf/empty.proto";

service ABC {
    // delete all ABC elements
    //
    // delete all ABC elements 
    rpc DelConfigs(DelConfigsRequest) returns (google.protobuf.Empty) {
        option (google.api.http) = {
            delete: "/api/v1/abc"
        };
    }
}

message DelConfigsRequest {
    int32 whatever = 1;
}

When I used protoc -I . -I/usr/include -I $GOPATH/src/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis --go_out=plugins=grpc:. --grpc-gateway_out=logtostderr=true:. --swagger_out=logtostderr=true:. ./abc.proto to get abc.swagger.json, I expected that the DelConfigsRequest was parsed by protoc-gen-swagger to generate query parameters for the delete method. In fact it's not parsed at all. The abc.swagger.json I got is the following:

{
  "swagger": "2.0",
  "info": {
    "title": "abc.proto",
    "version": "version not set"
  },
  "schemes": [
    "http",
    "https"
  ],
  "consumes": [
    "application/json"
  ],
  "produces": [
    "application/json"
  ],
  "paths": {
    "/api/v1/abc": {
      "delete": {
        "summary": "delete all ABC elements",
        "description": "delete all ABC elements",
        "operationId": "DelConfigs",
        "responses": {
          "200": {
            "description": "",
            "schema": {
              "$ref": "#/definitions/protobufEmpty"
            }
          }
        },
        "tags": [
          "ABC"
        ]
      }
    }
  },
  "definitions": {
    "protobufEmpty": {
      "type": "object",
      "description": "service Foo {\n      rpc Bar(google.protobuf.Empty) returns (google.protobuf.Empty);\n    }\n\nThe JSON representation for `Empty` is empty JSON object `{}`.",
      "title": "A generic empty message that you can re-use to avoid defining duplicated\nempty messages in your APIs. A typical example is to use it as the request\nor the response type of an API method. For instance:"
    }
  }
}

Is this an expected behavior to ignore the arguments of the delete method?

I'm using a Mac with macOS Sierra, go v1.9.3 darwin/amd64, and protoc 3.5.0.

[novabyte]

@zwtj I've seen this behaviour with POST method which has query params. I think it is a bug in the code generator for the JSON Swagger spec which is output.

[IronPan]

I've also seen the issue with POST method. The query parameters are not generated in the swagger definition. For example, if I generate swagger definition for this example, I got the following (In theory I should see a and b).

"/rpc/body/query/rpc": {
  "post": {
    "operationId": "RpcBodyRpc5",
    "responses": {
      "200": {
        "description": "",
        "schema": {
          "$ref": "#/definitions/apiEmptyProto"
        }
      },
      "default": {
        "description": "",
        "schema": {
          "$ref": "#/definitions/apiStatus"
        }
      }
    },
    "parameters": [
      {
        "name": "body",
        "in": "body",
        "required": true,
        "schema": {
          "type": "string"
        }
      }
    ],
  }
},

[johanbrandhorst]

Indeed, seems like a and b are missing in the parameters array. Is this a regression?

[novabyte]

@johanbrandhorst I don't think it's a regression. I'm not sure it ever worked. I'd have to gitbisect to understand better though.

[wotzhs]

i've run into similar problem when transcoding a POST method where the query params are not parsed, should i create a separate issue?

also, i would like to help, but need some pointer as to where might the problem be, i have simply tried to apply the same trick used for DELETE method for POST, but it didn't work

[johanbrandhorst]

@wotzhs I think the easiest thing would be to start with adding an example that reproduces the issue, e.g. add a method to a_little_bit_of_everything.proto which will trigger this bug, regenerate all the files (https://github.com/grpc-ecosystem/grpc-gateway/blob/master/CONTRIBUTING.md#i-want-to-regenerate-the-files-after-making-changes) and lets see what we can work out. I think from that point on it will be a matter of editing https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-swagger/genswagger/generator.go to fix any bugs.

[wotzhs]

@johanbrandhorst just checking with you if this is going into the right direction, i have tried to update the function in grpc-gateway/protoc-gen-swagger/genswagger/template.go so that the non path and body parameters will be added to query string for http POST method. i.e.:

message BodyWithQueryString {
    string someid = 1;
    string somequery = 2;
    string somevalue = 3;
    Body data = 4;
}

message Body {
    string body = 1 ;
}

service SomeService {
    rpc SomeRPC(BodyWithQueryString) returns (google.protobuf.Empty) {
        post: "/somepath/someprc/{somevalue}",
        body: "data"
    };
}

and the generated swagger.json would have:

{
    ...
    parameters: [
        {"name": "value", "in": "path", "required": true, "type": string},
        {"name": "body", "in": "body", "required": true, "schema": "#/definitions/exampleBody"},
        {"name": "someid", "in": "query", "required": false, "type": string},
        {"name": "somequery", "in": "query", "required": false, "type": string},
        {"name": "somevalue", "in": "query", "required": false, "type": string},
    ]
}

this change has however caused a lot of side effects, and failing multiple tests, while i can update the test cases so that they would pass, but i wonder if this is something the package authors would have wanted as well? -> #234

disclaimer: this is my very first endeavor to contributing to an opensource project, feedback are very much appreciated

[johanbrandhorst]

Great work so far @wotzhs, could you submit your changes in a pull request so we can review them? It sounds to me like you're doing the right thing :).

[novabyte]

@wotzhs Would love to see this land. Let me know how I can help 👍

[wotzhs]

@novabyte thank you for your offer to help 👍, this is taking a lot longer than i thought as i am quite stuck on the complexity of the non-primitive types such as the oneof being included as part of the query string.

I have not yet taken the time to observe and understand how the above works currently in either the GET or DELETE request, but i did come across #321 on how it blocks the oneof field from being provided with multiple values.

I have checked the google.api.http doc, there was no mention of whether the oneof field should be allowed or how it should be transcoded.

I suspect that i will have to look into the code generator to prevent all the fields of the oneof from being included into the query string, or the oneof field shouldn't be allowed in the query string at all.

[simonzg]

Hi, just want to check if anyone has this sorted out yet? I'm still encountering the exact same problem.

[johanbrandhorst]

Doesn't seem like it's been fixed yet. What can I do to help you bring in a fix for this?

[ghevge]

I'm using protoc-gen-swagger-v1.16.0-linux-x86_64, I can see that the issue was fixed for post methods, but it still behaving on put methods. Any idea when everything will be fixed ?