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: honor example field of message option #799

Merged

Conversation

birdayz
Copy link
Contributor

@birdayz birdayz commented Nov 7, 2018

I don't see a possibility to set example values through options. These would be shown by e.g. swagger-ui as example bodies for a POST.

message CreatePet {
   option (grpc.gateway.protoc_gen_swagger.options.openapiv2_schema) = {
    example: {
      value: '{SOME JSON}';
    };
  };

  string name = 1;
}

But it also allows setting the example field for nested messages, and the code generator will honor this and just set the nested field to the value.

One thing would be missing: setting examples for fields, i.e. for the string "name" in the example above. sadly this would need deeper changes, as the field option uses a different data type: JSONSchema and not Schema. Schema contains the example field..
I wonder why this is the case. As far as i have seen, it's possible in Swagger to set the example field even for "primitives".

This patch makes it possible to set examples for messages. While not perfect, it can be a way out of this.
If there's a better possibility to provide examples - without editing the .json manually, please tell me ;)

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! I've wanted this for a while. Can we just make sure to add some tests for this as well? And add an example example to the "a little bit of everything".proto and regenerate everything please.

@birdayz
Copy link
Contributor Author

birdayz commented Nov 7, 2018

Yeah sure, will do that in the next days. Would be great if you could give some input regarding examples for fields. I'd really like to have that, but it would require changes in the .proto files.. and it may not look very nice to have the "example" option in JSONSchema (See https://github.com/grpc-ecosystem/grpc-gateway/blob/master/protoc-gen-swagger/options/openapiv2.proto) as well. I guess there was a good reason to use JSONSchema instead of Schema for fields.

@codecov-io
Copy link

codecov-io commented Nov 7, 2018

Codecov Report

Merging #799 into master will decrease coverage by 0.06%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #799      +/-   ##
==========================================
- Coverage   53.27%   53.21%   -0.07%     
==========================================
  Files          30       30              
  Lines        3369     3373       +4     
==========================================
  Hits         1795     1795              
- Misses       1399     1403       +4     
  Partials      175      175
Impacted Files Coverage Δ
protoc-gen-swagger/genswagger/types.go 19.04% <ø> (ø) ⬆️
protoc-gen-swagger/genswagger/template.go 38.22% <0%> (-0.17%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7d429ea...86f635e. Read the comment docs.

@johanbrandhorst
Copy link
Collaborator

I don't know about the Schema vs JSONSchema for fields (@ivucica might know) but as for adding an example to the example files, that should be pretty straight forward I hope? It's the easiest way to show the users how to specify these things in their own files, so it should be comprehensive (as you say, include some nested type examples)

@birdayz
Copy link
Contributor Author

birdayz commented Nov 7, 2018

Hah yeah it's clear how to add usage examples. Confusing with the "examples" word all over the place.

I was just curious about the "example option field", and if we maybe can work on a possibility to expand this to proto fields as well (and not just for messages, which this patch adds)

Edit: Code sample for clarity:

  string id = 1 [(grpc.gateway.protoc_gen_swagger.options.openapiv2_field) = {example: "DEFAULT"}];

(which is not syntactically correct, because it's actually the sub-field "value" inside of example, but whatever)

anyway, the above is not possible, because openapiv2_field = JSONSchema and not Schema, which does not contain the example option field.

with json.RawMessage, users can supply complete json for swagger objects (i.e.
for a proto Message). If it's correct json, it will be added to the output.json
and rendered correctly by consuming applications (i.e. code generators or
swagger-ui)

Example, in proto:
option (grpc.gateway.protoc_gen_swagger.options.openapiv2_schema) = {
  example: {
    value:
    '{"someKey" : "someValue", "anotherKey" : 5}';
  };
};

Rendered JSON:
"example": {
  "someKey": "someValue",
  "anotherKey": 5
}

If we used string instead of json.RawMessage, the serializer would escape the
quotes and it would be serialized as a string. This is not desirable, because
the output should be in fact JSON key/value pairs.
@srenatus
Copy link
Contributor

srenatus commented Jan 9, 2019

🏓 this would really be a nice feature. LMK if there's anything I could help with... 🤔

@johanbrandhorst
Copy link
Collaborator

If you want to fork this and take over please feel free @srenatus

@johanbrandhorst
Copy link
Collaborator

As discussed, we're happy to merge this as is and add tests separately. Thanks for your contribution @birdayz!

@johanbrandhorst johanbrandhorst merged commit e5ffc3b into grpc-ecosystem:master Jan 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants