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

Generate Swagger description for service methods using proto comments. #134

Merged
merged 11 commits into from
May 17, 2016

Conversation

ivucica
Copy link
Collaborator

@ivucica ivucica commented Apr 17, 2016

This PR adds support for extracting Swagger descriptions from protobuf comments.

This fixes #128.


ORIGINAL OUTDATED TEXT FOLLOWS

While this is a first step in resolving #128, this
needs to be cleaned up, and the same approach needs to be used for
messages, message fields, et al.

echo_service.proto has been annotated with extra comments in order to
demo the new descriptions.

Only the Swagger example has been regenerated, as my local generator
does not output all the expected fields in proto struct tags.


Aside from some uncleanliness and overzealous panic()ing, I'm also (obviously) unhappy with line 444. Other generators seem to be producing and comparing paths by joining them into comma-separated strings. They're also putting all the SourceCodeInfo_Locations into a map indexed by paths.

(However, one thing that line 444 is doing good is dynamically looking up path IDs for 'message' type, etc. Even though it's using reflection for it, other generators written in Go seem to like to copy these constants into their own source code, which feels even worse.)

(For interested readers: More info on how to assemble a SourceCodeInfo_Location is in google/protobuf's descriptor.proto.)


Similar to #133, I was unable to regenerate protobufs usefully. I fully expect Travis build to fail at this point.

While this is a first step in resolving grpc-ecosystem#128, this
needs to be cleaned up, and the same approach needs to be used for
messages, message fields, et al.

echo_service.proto has been annotated with extra comments in order to
demo the new descriptions.

Only the Swagger example has been regenerated, as my local generator
does not output all the expected fields in proto struct tags.
Also, fixed a few typos and renamed protoPath into protoPathIndex
(as we are not returning the entire path in that method).
This field should generally be non-nil while running the generator.
This commit also adds a trivial check to make panic more informative
in case a future test breaks in a similar fashion.
@ivucica
Copy link
Collaborator Author

ivucica commented May 2, 2016

Please let me know if something else is needed for this PR.

This patch will extract summary separately from description in
Swagger objects that happen to support both summary and
description. It will do so by splitting the relevant proto
comment based on paragraphs, and using the first paragraph
as the summary.

This patch will also allow updating the Swagger schema's
otherwise hard-to-map fields by allowing custom JSON to be
placed within the proto comment. The syntax leaves something to
be desired, but it works. Use of JSON is, on the other hand,
rather universal.

This patch will also allow describing the whole API by attaching
the comments to the 'package' stanza inside the proto. In this
case, summary will be applied to info.title, and description
will be applied to info.description. Other properties have to be
set through JSON.

Schema has been slightly updated to allow for fields such as
termsOfService, contact, license, externalDocs, etc.

If a method is not documented, stub summary for Swagger
Operation is no longer generated.

Documentation for enum values is now multiline with dashes
before values.
While the spec does not seem to dictate this, most of the
examples seem to have HTTP status codes as the keys. This
patch replaces the text 'default' with value of '200'.
@tmc
Copy link
Collaborator

tmc commented May 7, 2016

@ivucica IMO you should separate the comment handling from this new <!-- swagger extras start concept as they're distinct and the latter deserves more consideration. Do you mind pulling that concept out into another PR? Thanks!

@tmc
Copy link
Collaborator

tmc commented May 7, 2016

What do you think about simply taking the last portion of the comment and if it's a json document integrate it.

@ivucica
Copy link
Collaborator Author

ivucica commented May 7, 2016

@tmc That sounds like a great idea. I'll work on that, including separating it into a separate PR.

ivucica added a commit to ivucica/grpc-gateway that referenced this pull request May 7, 2016
ivucica added a commit to ivucica/grpc-gateway that referenced this pull request May 7, 2016
@ivucica
Copy link
Collaborator Author

ivucica commented May 7, 2016

@tmc See #145 .

ivucica added a commit to ivucica/grpc-gateway that referenced this pull request May 7, 2016
@yugui
Copy link
Member

yugui commented May 9, 2016

LGTM.

I can merge this PR as soon as I or you resolve merge conflicts.

@tmc
Copy link
Collaborator

tmc commented May 16, 2016

@yugui see #156

@yugui yugui merged commit 562956f into grpc-ecosystem:master May 17, 2016
ivucica added a commit to ivucica/grpc-gateway that referenced this pull request Oct 9, 2016
ivucica added a commit to ivucica/grpc-gateway that referenced this pull request Oct 9, 2016
ivucica added a commit to ivucica/grpc-gateway that referenced this pull request Feb 10, 2017
ivucica added a commit to ivucica/grpc-gateway that referenced this pull request Feb 10, 2017
ivucica added a commit to ivucica/grpc-gateway that referenced this pull request Mar 7, 2017
ivucica added a commit to ivucica/grpc-gateway that referenced this pull request Mar 7, 2017
ivucica added a commit to ivucica/grpc-gateway that referenced this pull request Oct 27, 2017
ivucica added a commit to ivucica/grpc-gateway that referenced this pull request Oct 27, 2017
adasari pushed a commit to adasari/grpc-gateway that referenced this pull request Apr 9, 2020
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

Successfully merging this pull request may close these issues.

protoc-gen-swagger comment parsing for documentation gen
4 participants