allow protobuf well known types in params

[tmc]

example: allow a google.protobuf.Duration in a get param.

[AlekSi]

Right now, support for google.protobuf.Duration is very confusing.

1. https://developers.google.com/protocol-buffers/docs/proto3#json says:

Generated output always contains 0, 3, 6, or 9 fractional digits, depending on required precision, followed by the suffix "s". Accepted are any fractional digits (also none) as long as they fit into nano-seconds precision and the suffix "s" is required.

2. https://github.com/go-openapi/strfmt/blob/v0.19.2/duration.go contains support for various suffixes.

3. But in practice, grpc-gateway always returns duration as seconds with s suffix and uses time.ParseDuration for parsing, so suffixes like d are not supported.

[johanbrandhorst]

Is this not implemented already? Also not sure I understand your comment @AlekSi, are you saying it should support the full range required by the openapi definition of a duration? It might be possible with some pre-json unmarshalling parsing.

[AlekSi]

All I'm saying is that current handling is inconsistent.

I see several options:

1. Do nothing in code, just document that behavior. :)
2. Extend parsing (only) with strfmt mapping, so suffixes like d are accepted.
3. Use protobuf JSON mapping consistently: accept and return only seconds with s suffix.
4. Use strfmt mapping consistently: accept and return other suffixes like d.

Options 3 and 4 are breaking changes, so probably some generator flag / protobuf option / runtime parameter would be required to use a new behavior.

Personally, I would prefer option 4 with a generator flag option 2. Option 4 will make parsing on the receiver side harder.

[johanbrandhorst]

What are we doing right now? I thought it was #3.

[AlekSi]

That's how it works now:

"59s": "59s",
"60s": "60s",
"61s": "61s",
"61" : "", // no suffix => error
"2m" : "120s",
"1h" : "3600s",
"1d" : "", // d suffix => error

Suffixes m and h are accepted, but never returned.

[johanbrandhorst]

I see. So #2 but without d support?

[AlekSi]

Yes, right now it is #2 without d and w.

[johanbrandhorst]

@AlekSi would you mind opening a new issue with this background as a feature request? I will close this one since we do support it.