Skip to content

Add support for generating OpenAPI responses #55020

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

Merged
merged 4 commits into from
Apr 11, 2024

Conversation

captainsafia
Copy link
Member

Support generating OpenAPI response for a given operation.

Current results for GenerationBenchmark:

Method EndpointCount Mean Error StdDev Op/s Gen0 Gen1 Allocated
GenerateDocument 10 30.79 us 0.263 us 0.246 us 32,479.2 0.9155 0.1831 115.39 KB
GenerateDocument 100 255.65 us 1.359 us 1.271 us 3,911.7 8.3008 2.9297 1024.07 KB
GenerateDocument 1000 2,762.12 us 18.172 us 16.109 us 362.0 78.1250 31.2500 10106.99 KB

@captainsafia captainsafia requested a review from a team as a code owner April 8, 2024 21:57
@ghost ghost added the needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically label Apr 8, 2024
@captainsafia captainsafia added area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-openapi area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed needs-area-label Used by the dotnet-issue-labeler to label those issues which couldn't be triaged automatically labels Apr 8, 2024
response.Content[contentType] = new OpenApiMediaType();
}

// MVC's `ProducesAttribute` doesn't implement the produces metadata that the ApiExplorer
Copy link
Member

Choose a reason for hiding this comment

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

Can we change that? The less MVC we reference, the better.

Copy link
Member Author

Choose a reason for hiding this comment

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

I believe we discussed this in the past when we originally introduced the ProducesResponseTypeMetadata type. I think the conclusion at the time was to create new metadata/attributes for these annotations that was shared between MVC/minimal instead of recycling the existing ones.

Because this implementation needs to work for both controller-based and minimal APIs, we're not going to be able to get away with having less MVC.

Copy link
Member

@amcasey amcasey left a comment

Choose a reason for hiding this comment

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

I have questions, but I'm pretty sure they're about OpenApi and not your code.

@@ -18,4 +18,8 @@
<Reference Include="Microsoft.AspNetCore.Mvc" />
</ItemGroup>

<ItemGroup>
<ProjectReference Include="../test/Microsoft.AspNetCore.OpenApi.Tests.csproj" />
Copy link
Member

Choose a reason for hiding this comment

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

Why would a sample reference the tests? We don't want customers to do that, do we?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, it's for the "shared" TODO types. I think I'd probably redeclare them to keep the sample self-contained.

Copy link
Member Author

@captainsafia captainsafia Apr 10, 2024

Choose a reason for hiding this comment

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

I just realized that I can use a Compile-include to do this. 🤦🏽‍♀️ 😅

Edit: I realize this doesn't make the sample very self-contained but the samples in the repo don't lend themselves nicely to being pulled out, especially because they rely on the special reference resolution setup that we have in the repo.

Copy link
Member

@amcasey amcasey left a comment

Choose a reason for hiding this comment

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

Much clearer. Thanks!

@captainsafia captainsafia merged commit 4fe1a1f into feature/openapi Apr 11, 2024
21 checks passed
@captainsafia captainsafia deleted the safia/get-response branch April 11, 2024 00:11
captainsafia added a commit that referenced this pull request Apr 18, 2024
Co-authored-by: Martin Costello <[email protected]>
Co-authored-by: Rick Anderson <[email protected]>

This PR adds support for OpenAPI document generation, sans schema generation to Microsoft.AspNetCore.OpenApi. Relevant changes are available in individual PRs:

- Add entry-point APIs for OpenAPI support (#54789)
- Support resolving OpenApiPaths entries from document (#54847) 
- Support generating OpenAPI operation and associated fields (#54903) 
- Add APIs for OpenAPI document transformers (#54935) 
- Add support for generating OpenAPI parameters (#55041)
- Add support for generating OpenAPI responses (#55020) 
- Add support for generating OpenAPI request bodies (#55040)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-openapi
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants