protoc-gen-swagger: patch method request attributes do not map to query parameter

[winglq]

This is a follow up issue to #1012.

1. use the following proto file to genrate a swagger file.

syntax="proto3";

package book;
import "google/api/annotations.proto";
import "google/protobuf/field_mask.proto";

service BookService {
  // Updates a book.
  rpc UpdateBook(UpdateBookRequest) returns (Book) {
    // Update maps to HTTP PATCH. Resource name is mapped to a URL path.
    // Resource is contained in the HTTP request body.
    option (google.api.http) = {
      // Note the URL template variable which captures the resource name of the
      // book to update.
      patch: "/v1/{book.name=shelves/*/books/*}"
      body: "book"
    };
  }
}

message UpdateBookRequest {
  // The book resource which replaces the resource on the server.
  Book book = 1;

  string title = 2;
}

message Book {
  string name = 1;
  string author = 2;
}

2. title shoud be mapped to a url query parameter, but it is not in the swagger file.
   The following is the parameter section of swagger output.

        "parameters": [
          {
            "name": "book.name",
            "in": "path",
            "required": true,
            "type": "string"
          },
          {
            "name": "body",
            "description": "The book resource which replaces the resource on the server.",
            "in": "body",
            "required": true,
            "schema": {
              "$ref": "#/definitions/bookBook"
            }
          }
        ],

3. title is not part of the go generate file output either. This is the url pattern in the gateway output.

 pattern_BookService_UpdateBook_0 = runtime.MustPattern(runtime.NewPattern(1, []int{2, 0, 2, 1, 1, 0, 2, 2, 1, 0, 4, 4, 5, 3}, []string{"v1", "shelves", "books", "book.name"}, "", runtime.AssumeColonVerbOpt(true)))
)

[johanbrandhorst]

I would have expected title to be parsed from the query parameters, regardless of what the generated swagger document says. Have you tried sending ?title=whatever in the URL and tried seeing if it was set? It could just be a protoc-gen-swagger bug.

[winglq]

currrently, I am working on designing the APIs, I will verify the gateway after I working on the server.

[elikatsis]

Hello! Any update on this one?

[winglq]

Sorry, I do not have time working on my side project recently.

[winglq]

I have confirmed query parameter is set correctly. So it seems only the swagger output is not working as expected.

[johanbrandhorst]

Hi @winglq, thanks for doing that investigation. What can I do to help you get a fix for this merged?

[winglq]

Hi @johanbrandhorst sorry for the late response. I think fix the protoc-gen-swagger bug would be helpful as it will give us a correct swagger api defination. But it's ok to low it's priority because I'm using it in a side project.
Thanks.

[vtolstov]

for my this is major bug, as i'm generate typescript stuff from swagger definition.

[johanbrandhorst]

We'd love to have a fix for this, could you help out Vasiliy?

[vtolstov]

i think about it, so for put/patch we need to pass all that not in body via url variables

[johanbrandhorst]

It's just a matter of fixing the swagger generator behaviour to match the behaviour of the gateway; yes, for these requests we're missing some fields.

[renyijiu]

I have a similar problem, if i defined the path get: "/v1/{book.name=shelves/*/books/*}" in proto file according to google api design, then generate the swagger file and i will get the same path v1/{book.name=shelves/*/books/*} defined in swagger file.The problem is that the path not correct (it should looks like: /v1/shelves/*/books/* )and we can not use the swagger ui to test the api directly. And i found the gateway works fine, it looks only swagger file wrong

[oyvindwe]

I think this has the same root cause as issue #1670 that I just fixed. With 2.8.0 I get the following in book.swagger.json:

"parameters": [
  {
    "name": "book.name",
    "in": "path",
    "required": true,
    "type": "string",
    "pattern": "shelves/[^/]+/books/[^/]+"
  },
  {
    "name": "book",
    "description": "The book resource which replaces the resource on the serv
    "in": "body",
    "required": true,
    "schema": {
      "type": "object",
      "properties": {
        "author": {
          "type": "string"
        }
      },
      "title": "The book resource which replaces the resource on the server."
    }
  },
  {
    "name": "title",
    "in": "query",
    "required": false,
    "type": "string"
  }
],

I suggest closing this as fixed.

[oyvindwe]

I have a similar problem, if i defined the path get: "/v1/{book.name=shelves/*/books/*}" in proto file according to google api design, then generate the swagger file and i will get the same path v1/{book.name=shelves/*/books/*} defined in swagger file.The problem is that the path not correct (it should looks like: /v1/shelves/*/books/* )and we can not use the swagger ui to test the api directly. And i found the gateway works fine, it looks only swagger file wrong

With version 2.7.2 or newer, you should get v1/{book.name}; this was reported in issue #407, fixed in PR #2461.

[oyvindwe]

@johanbrandhorst I should have tagged you above - this seems to be fixed.

Note - I might have taken undue credit - I didn't fix the query parameters, only the body parameter.

[johanbrandhorst]

Great, very happy to close this!