-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Move OpenAPI extensions to routing assembly #36161
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
Conversation
4d652e1 to
91a25cb
Compare
91a25cb to
83a2d31
Compare
|
Thank you for your API proposal. I'm removing the |
83a2d31 to
ef59510
Compare
|
Thank you for your API proposal. I'm removing the |
src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs
Outdated
Show resolved
Hide resolved
src/Mvc/Mvc.Core/src/ApiExplorer/IApiResponseMetadataProvider.cs
Outdated
Show resolved
Hide resolved
ba9e4fa to
d6cc08c
Compare
src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs
Outdated
Show resolved
Hide resolved
src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs
Outdated
Show resolved
Hide resolved
src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs
Outdated
Show resolved
Hide resolved
src/Http/Http.Abstractions/src/Metadata/IProducesResponseTypeMetadata.cs
Outdated
Show resolved
Hide resolved
src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs
Outdated
Show resolved
Hide resolved
| /// <summary> | ||
| /// Gets the content types supported by the metadata. | ||
| /// </summary> | ||
| IReadOnlyCollection<string>? ContentTypes { get; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be nullable or empty? Does it mean anything different in this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think null more clearly communicates "no content types set by user" which is technically possible for void-returning endpoints where the user does something like app.MapPost("/process", () => { }).Produces(200)
src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs
Outdated
Show resolved
Hide resolved
src/Http/Routing/src/Builder/OpenApiDelegateEndpointConventionBuilderExtensions.cs
Show resolved
Hide resolved
src/Http/Routing/src/Builder/OpenApiDelegateEndpointConventionBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
src/Http/Routing/src/Builder/OpenApiDelegateEndpointConventionBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
| Type = type ?? throw new ArgumentNullException(nameof(type)); | ||
| StatusCode = statusCode; | ||
| IsResponseTypeSetByDefault = false; | ||
| _contentTypes = new List<string>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| _contentTypes = new List<string>(); | |
| _contentTypes = Enumerable.Empty<string>(); |
src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs
Outdated
Show resolved
Hide resolved
| // System.Console.WriteLine($"Waiting for debugger to attach on {System.Environment.ProcessId}"); | ||
| // while (!System.Diagnostics.Debugger.IsAttached) | ||
| // { | ||
| // Thread.Sleep(1000); | ||
| // } | ||
| // System.Console.WriteLine("Debugger attached"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooof -- I need a git hook to check for these. #justvscodeproblems
src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs
Outdated
Show resolved
Hide resolved
src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs
Outdated
Show resolved
Hide resolved
src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs
Outdated
Show resolved
Hide resolved
src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs
Outdated
Show resolved
Hide resolved
9ba3013 to
71aa544
Compare
71aa544 to
4ea0cb6
Compare
|
/backport to release/6.0 |
|
Started backporting to release/6.0: https://github.com/dotnet/aspnetcore/actions/runs/1215216291 |
Background and Motivation
When the OpenAPI extension methods were added in RC1, they were added in the
Mvc.Coreassembly under theMicrosoft.AspNetCore.Httpnamespace like so:For RC2, we want to move these extension methods to the
Routingassembly since they are designed to be used with theDelegateEndpointConventionBuilder. The types will stay in the same namespace. Functionally, the user won't have to change anything in their code but this change will set us up for success moving forward with regard to making trimming of minimal apps a lot easier.Proposed API
To support this change, some new types had to be added to support the relocation of the API.
First, the
MediaTypeCollectiontype is being moved to theMicrosoft.AspNetCore.Httpassembly. The type is still in the same namespace.This PR introduces a new
IProducesTypeMetadatato theMicrosoft.AspNetCore.Http.MetadatanamespaceUsage Examples
The
IProducesResponseTypeMetadatais designed to be used as the implementing interface for theProducesResponseTypeAttributeslike so:Fixes #35905