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

Elaborate Swagger-specific annotations and move to separate package #694

Closed
domaindrivendev opened this issue Apr 16, 2018 · 8 comments
Closed
Milestone

Comments

@domaindrivendev
Copy link
Owner

domaindrivendev commented Apr 16, 2018

Background

The fundamental goal of Swashbuckle is to generate living, breathing documentation that's driven from and remains in sync with the implementation. So, the implementation is the truth and the documentation is a direct projection of it. This forces API authors, sometimes reluctantly to be very explicit in their implementation so that the API behaves exactly as they'd like it to be described.

So, supporting Swagger-specific annotations (e.g. SwaggerResponseAttribute) that are not linked to "actual" API behavior, has always felt orthogonal to the project's original goals. As a result, support is limited and only a handful of metadata can be set in this way. However, it's clear that they provide value and complement the core functionality, particularly when there's Swagger metadata that simply can't be inferred from the implementation code.

Proposal

  1. Move the functionality into a separate package - Swashbuckle.AspNetCore.Annotations:
    It's currently implemented as a set of Operation and Schema filters and so can be easily decoupled. This will allow the functionality to develop independently, without adding additional complexity to the core functionality.
  2. Extend the current functionality to support more common use cases. Here's a current wish list:
    • support descriptions and summaries (if applicable) on existing attributes.
    • support consumes and produces arrays on SwaggerOperationAttribute
    • introduce SwaggerParameterAttribute and SwaggerTagAttribute
@esbenbach
Copy link
Contributor

I figured I might actually make an attempt at contributing here.
Would you prefer to create the project structure on your own or should I attempt to do it as best I can and then you can fill in the blanks (such as the signing stuff...).

Other than that, I presume the idea is to basically move the stuff from SwaggerGen\Annotations to the new package and then have the new Annotations project refer the SwaggerGen so the filters can also exist in the annotations package, or do you want the filters somewhere else?

@domaindrivendev
Copy link
Owner Author

domaindrivendev commented Apr 19, 2018

@esbenbach - would be great to get your help! You're correct in your presumed approach. Actually, I had already created an elaborate-annotations branch and did the work to extract the annotations out. The Xml comments stuff is a little more tightly coupled into the core project so going to leave that where it is for now and just focus on the SwaggerXXX annotations.

Next steps (and this is where you could take over if you're willing) is to start enhancing the functionality ...
- support descriptions and summaries (if applicable) on existing attributes.
- support consumes and produces arrays on SwaggerOperationAttribute
- introduce SwaggerParameterAttribute (for setting required etc.) and SwaggerTagAttribute (on controllers)
- update readme with more info on this feature

NOTE: we can collaborate on this work via the elaborate-annotations branch. So, please submit any PR's against that instead of master. Thanks

@esbenbach
Copy link
Contributor

Thanks for the detailing, I will have a look and see what I can contribute :)

@esbenbach
Copy link
Contributor

Taking another look at the wishlist, would you clarify the requirements a bit?

  • Support descriptions and summaries (if applicable) on existing attributes?
  • Not sure i understand the purpose of Consumes and Produces in swaggeroperationattribute, isnt this covered by the existing Produces/Consumes attributes of the framework?
  • Do you want a new readme section related to annotations and what they do? Or would you prefer to merge the description in where it might fit with other topics? (the later is probably harder)

@domaindrivendev
Copy link
Owner Author

@esbenbach I'll work on the readme as I have some re-org that I'd like to do as part of this.

Regarding your question around the rationale of adding Consumes and Produces properties to the attribute - it's mainly to keep the two modes of Swashbuckle usage separate. That is, if you're happy to drive documentation purely from your API implementation, then you would use the built-in ConsumesAttribute and `ProducesAttribute to restrict input/output formatters that can actually be used for a given operation. Alternatively, if you want more control over the documentation regardless of the implementation, you can use a Swagger annotation.

Although judging by the description you put in your latest PR, I think you may have already reached this conclusion:

You will typically set these when you want to convey information but don't want to actually disable the Mvc output formatters.

@esbenbach
Copy link
Contributor

Yearh got it.

If you are working on the readme, I won't start messing it up.

How about the last SwaggerParameterAttribute what type of properties is it supposed to have?

Looking at the the https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#parameterObject I would assume "Required"and "In" might be interesting to have, but what about description and name? Overriding the name will break the implementation and the description can just as easily be taken from the xml comment.

And to be honest, defining schema when in is of type body just seems plain weird.

Im basically doing this to get the tag description support :) So i will have a crack at it if you think its useful, but I need a bit more guidance/direction on what to actually implement.

@domaindrivendev
Copy link
Owner Author

For SwaggerParameterAttribute I would just support Description and Required for now

@domaindrivendev domaindrivendev added this to the v3.0.0 milestone Jun 6, 2018
@domaindrivendev
Copy link
Owner Author

Merged into master and will be released with 3.0.0, which is coming really soon (targeting next week). In the meantime you can pull down the latest preview from nuget.org to try it out.

Special thanks to @esbenbach for doing most of the heavy lifting on this.

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