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

Add Description to ProducesResponseType (and others) for better OpenAPI document creation #58193

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

sander1095
Copy link

@sander1095 sander1095 commented Oct 1, 2024

Add Description to ProducesResponseType (and other types) for better OpenAPI documents in Controllers

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Description

This pull request introduces the concept of adding OpenAPI's description to responses returned by ASP.NET Core. The changes include adding a Description property to response metadata interfaces, classes, and attributes, as well as updating related tests.

As discussed in #55656 , Minimal API's are not (or minimally) addressed in this PR. This should instead be built in #57963 (which I would love to work on in the future!)

Fixes #55656

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Oct 1, 2024
Copy link
Contributor

Thanks for your PR, @sander1095. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Oct 1, 2024
@sander1095
Copy link
Author

sander1095 commented Oct 1, 2024

Hey @captainsafia, ASP.NET Core team and community!

I'm extremely excited to be able to create this PR! I'm dedicating this to the .NET and OpenAPI community, and I shouldn't forget to mention hacktoberfest as well! I'd love to get this merged or marked as accepted in the month of October, so keep the PR feedback coming!

I've been working on this for a while since talking to Safia about it at the MVP Summit 2024, and I'm very grateful for the opportunity.

Now, let's talk about some details 😁.

The state of the PR

I believe the PR is coming along nicely, but I find it difficult to say if it is done. I would need help from the ASP.NET Core team or community to get the following answered:

  • How can I test these changes? I have added unit tests, but because I do not know how I can test this functionally in an ASP.NET Core project, I do not know what I still might need to do.
    • I couldn't find any docs about this, so perhaps it'd be useful to add some documentation about this in the future?
  • How can Swashbuckle.AspNetCore, NSwag and Microsoft.AspNetCore.OpenAPI benefit from this change? Are there code changes needed on their side, if so, how do we coordinate this? Library authors, let me know if I can help!
  • I've already mentioned this in the PR description, but I believe we also need to get feature parity on the Minimal API side (Allow OpenAPI's response descriptions to be set using Minimal API #57963). However, this still needs some discussion as implementing it could apparently lead to source incompatibility issues. I'd love assistance with knowing how we will tackle this; after that I'd love a crack at implementing it!

And finally:

Please let me know what other changes need to be made to make this work functionally! Again, my goal is to get this to an accepted (or even merged!) state in october, if possible!

@sander1095
Copy link
Author

@dotnet-policy-service agree

@mkArtakMSFT mkArtakMSFT added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-web-frameworks *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Oct 2, 2024
@sander1095
Copy link
Author

Hi @mkArtakMSFT and @adityamandaleeka ! When can I expect a review and response to my comment with questions? I am very excited to get started :)

Copy link
Member

@captainsafia captainsafia left a comment

Choose a reason for hiding this comment

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

Great start! We'll also want to update the OpenAPI implementation to respect this new property. The code for constructing OpenAPI responses is located here. Tests for this area are here.

@sander1095
Copy link
Author

Hey @captainsafia! Thanks so much! I will look into this as soon as possible.

@sander1095
Copy link
Author

Hi @captainsafia ! I've implemented your suggestions, please see the new commit and let me know what else I still need to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions community-contribution Indicates that the PR has been added by a community member feature-openapi
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Description to ProducesResponseType (and others) for better OpenAPI documents
5 participants