Swagger get query param documentation shows repeated fields incorrectly

[veqryn]

Bug reports:

The swagger documentation generated for query parameters shows repeated fields incorrectly.

Steps you follow to reproduce the error:

Proto file example:

service Foo {
	rpc Bar (Params) returns (Series) {
		option (google.api.http) = {
			get: "/v2/foo/bar"
		};
	}
}
message Params {
	repeated string tags = 1;
}

This generates swagger documentation that looks correct at first glance, but if you hit "Try It Now" and then fill in multiple "tags", then select execute, it gives you a curl command like this.
(This was done with me filling in the following tags: foo=bar and blah=blarg.)
curl -X GET "http://editor.swagger.io/v2/foo/bar?tags=foo%3Dbar,blah%3Dblarg" -H "accept: application/json"

That is, it asks the user to comma separate the individual tags:
tags=foo%3Dbar,blah%3Dblarg

When my function runs, it receives a single element in the array equal to: ["foo=bar,blah=blarg"]

What did you expect to happen instead:

The documentation should actually generate an example like this:
curl -X GET "http://editor.swagger.io/v2/foo/bar?tags=foo%3Dbar&tags=blah%3Dblarg" -H "accept: application/json"

This correctly gives my function two elements: ["foo=bar", "blah=blarg"]

[johanbrandhorst]

Thanks for raising this issue! I think this is actually just an incompatibility between Swagger and the Gateways expectations. The gateway generates a correct swagger file and there's no way that I know to tell swagger to put these parameters in separate query parameters.

If you know of a different solution to this, please let me know, but I think this might just be a matter of documenting the problem. I think the gateway is doing the sensible thing here and Swagger is generating a nonsensical query.

[johanbrandhorst]

Having looked into it, I think we might be able to fix this by emitting a collectionFormat value set to multi: https://swagger.io/docs/specification/2-0/describing-parameters/. Would you like to contribute this?

[veqryn]

I guess I could try at least.
Is this where my change would go?

grpc-gateway/protoc-gen-swagger/genswagger/template.go

Line 92 in ab0345b

func queryParams(message *descriptor.Message, field *descriptor.Field, prefix string, reg *descriptor.Registry, pathParams []descriptor.Parameter) (params []swaggerParameterObject, err error) {

[johanbrandhorst]

I think we'd add a new field to the schema type, collectionFormat, and that would only be set if we were working with an array of query parameters. So yes, in that function, but more specifically in schemaOfField I think.

[litichevskiydv]

Hello, guys! We are using gRPC with Node.js and generating swagger documentation by protoc-gen-swagger. Our REST gateway is hosted by express and we met the same problem, as was described. @johanbrandhorst, do you have plans to implement configurable behavior for option collectionFormat?

[johanbrandhorst]

@litichevskiydv I don't personally have the time to implement this, but I think mine and @veqryn's previous discussion is good enough for anyone with the determination to get a pull request started, with or without Go experience! What can I do to help you bring this contribution in?

[johanbrandhorst]

Reopening due to #906.

[johanbrandhorst]

@bmperrea sorry we didn't catch the bug (#906) the first time around, would you be interested in trying again in a new PR? The previous PR should be a good starting point, but we'll need to ensure we handle the case where there are repeated fields in the body too.

[bmperrea]

@johanbrandhorst I would like to take a look. My first guess is that we don't want to be setting a collectionFormat in schemaOfField but only in queryParams. I'll try to reproduce and then see if that fixes it.