Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

protoc-gen-swagger: Support Response-specific examples #1117

Closed
jgiles opened this issue Jan 19, 2020 · 6 comments
Closed

protoc-gen-swagger: Support Response-specific examples #1117

jgiles opened this issue Jan 19, 2020 · 6 comments

Comments

@jgiles
Copy link
Contributor

jgiles commented Jan 19, 2020

Swagger 2.0 supports specifying a per-mimetype response example for a given operation:

https://github.com/OAI/OpenAPI-Specification/blob/3.0.0/versions/2.0.md#responseObject
https://github.com/OAI/OpenAPI-Specification/blob/3.0.0/versions/2.0.md#exampleObject

This allows displaying a per-operation example for a given response schema, typically overriding any example specified on the response schema type definition in generated documentation. This is useful for clearly documenting workflows where the same schema type is returned with varied or evolving state/status.

This capability seems to have been anticipated but left unimplemented in the protobuf annotation types used by grpc-gateway:

// field 3 is reserved for 'example'.

// `Response` is a representation of OpenAPI v2 specification's Response object.
//
// See: https://github.com/OAI/OpenAPI-Specification/blob/3.0.0/versions/2.0.md#responseObject
//
message Response {
  // `Description` is a short description of the response.
  // GFM syntax can be used for rich text representation.
  string description = 1;
  // `Schema` optionally defines the structure of the response.
  // If `Schema` is not provided, it means there is no content to the response.
  Schema schema = 2;
  // field 3 is reserved for 'headers'.
  reserved 3;
  // field 3 is reserved for 'example'.
  reserved 4;
  map<string, google.protobuf.Value> extensions = 5;
}

This issue is a feature request for the addition of that capability.

I propose that the added field be structured like

map<string, string> examples = 4;

to support the specification's requirement of mapping mime types to examples, and to match the pattern string-based example value specification elsewhere in the grpc-gateway annotations (see https://github.com/OAI/OpenAPI-Specification/blob/3.0.0/versions/2.0.md#example-object-example )

@johanbrandhorst
Copy link
Collaborator

This SGTM, I guess we have to copy the values verbatim into the output swagger definition. Would you be interested in implementing this?

@jgiles
Copy link
Contributor Author

jgiles commented Jan 19, 2020

I would like to, though history suggests I should be skeptical of my availability to do so (see #888, #877 ). I may be able to get this one prioritized though because it's harder than those to temporarily patch over using jq-based post-processing.

I expect this would be a pretty simple case of piping the new input field into the output Swagger?

@jgiles
Copy link
Contributor Author

jgiles commented Jan 19, 2020

The only thing I might like to discuss before launching into implementation is whether we should simplify the input to be application/json specific. While technically the Swagger spec has a map of MIME type to response example string, it seems like the grpc-gateway case would be overwhelmingly application/json, and using a field specific to that like

string json_example = 6;

would simplify applications of the annotation from

examples: {
  key: 'application/json',
  value: '{"foo": "bar"}'
}

to

json_example: '{"foo": "bar"}'

While that seems like a small thing, we're already struggling a bit in practice with the verbosity / trickiness of the openapi mapping annotations, and keeping this bit simpler would be a win.

What do you think @johanbrandhorst ? Note the proposed alternative name json_example would leave open the possibility of later adding a field examples with all the capabilities permitted by Swagger.

@johanbrandhorst
Copy link
Collaborator

The only thing I might like to discuss before launching into implementation is whether we should simplify the input to be application/json specific. While technically the Swagger spec has a map of MIME type to response example string, it seems like the grpc-gateway case would be overwhelmingly application/json, and using a field specific to that like

string json_example = 6;

would simplify applications of the annotation from

examples: {
  key: 'application/json',
  value: '{"foo": "bar"}'
}

to

json_example: '{"foo": "bar"}'

While that seems like a small thing, we're already struggling a bit in practice with the verbosity / trickiness of the openapi mapping annotations, and keeping this bit simpler would be a win.

What do you think @johanbrandhorst ? Note the proposed alternative name json_example would leave open the possibility of later adding a field examples with all the capabilities permitted by Swagger.

I invite @ivucica to this discussion, he has more opinions/knowledge than I in this area specifically, but I think we want to keep the mapping between the swagger proto spec and the swagger 2.0 as 1:1 as possible, and adding a field json_example that doesn't exist in the swagger spec would be contrary to this aim. Furthermore, while the grpc-gateway supports application/json straight out of the box, it is also easily extensible with other mime types via https://pkg.go.dev/github.com/grpc-ecosystem/grpc-gateway/runtime?tab=doc#WithMarshalerOption. I think we should add the correct mapping here.

On the implementation side, it sounds like it shouldn't be too bad, touch wood! Thanks again for your interest in the project.

@jgiles
Copy link
Contributor Author

jgiles commented Jan 20, 2020

Agreed, the clarity value of maintaining direct correspondence with the Swagger standard probably does win out here.

It would be nice if text-format protobuf allowed writing maps more succinctly (for this and other applications), but that is out of scope here and we shouldn't try to work around it in the message definitions.

map<string, string> examples it is.

jgiles added a commit to paxosglobal/grpc-gateway that referenced this issue Jan 22, 2020
Add support for examples in operation annotation responses. Merge
custom response data into the default 200 response.
jgiles added a commit to paxosglobal/grpc-gateway that referenced this issue Jan 22, 2020
@jgiles
Copy link
Contributor Author

jgiles commented Feb 1, 2020

Hey @johanbrandhorst, I put up a PR for this: #1124

adasari pushed a commit to adasari/grpc-gateway that referenced this issue Apr 9, 2020
)

* Fix grpc-ecosystem#1117: Support response examples.

Add support for examples in operation annotation responses. Merge
custom response data into the default 200 response.

* Regenerate files.

* Documentation improvements.
pull bot pushed a commit to BuildingRobotics/grpc-gateway that referenced this issue Apr 29, 2020
)

* Fix grpc-ecosystem#1117: Support response examples.

Add support for examples in operation annotation responses. Merge
custom response data into the default 200 response.

* Regenerate files.

* Documentation improvements.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants