Add support for global conventions on WebApplication #59983
Conversation
dotnet-issue-labeler
bot
added
the
needs-area-label
| } | ||
|
|
||
| [Fact] | ||
| public async Task ThrowsExceptionOnNonRouteEndpointsAtTopLevel() |
There was a problem hiding this comment.
@halter73 I realized while working through test cases for this that route groups are only designed to work with RouteEndpoints due to the requirement to be able to concatenate the route prefix to the route when constructing the endpoint. See the comment you left here below.
aspnetcore/src/Http/Routing/src/EndpointDataSource.cs
Lines 45 to 50 in 31d5a5f
We have two options at the moment:
- Leave things as is. The WebApplication will still work with our primary scenarios (Blazor, MVC, SignalR, etc.) but users using custom endpoints will have to use a branched pipeline to register endpoints.
- Revise the implementation of grouped endpoints to support un-routed endpoints, perhaps by defaulting to an empty prefix + route for all? I haven't evaluated how expensive this would be.
There was a problem hiding this comment.
I don't think we can really make WebApplication throw for any non-RouteEndpoints if they worked before, but maybe we could only throw if you've added a global convention.
I don't think it's even feasible to support global conventions for non-RouteEndpoints, because I imagine most non-RouteEndpoint implementations would be some other derived type which we wouldn't be able to instantiate with the additional metadata from the global conventions.
At best, we could create a plain Endpoint with the additional metadata. We could restrict it to support just RouteEndpoint and plain non-derived Endpoint (but only for global conventions and maybe prefix-less route groups), but I'd want to know exactly what the use case is before we expand support.
This also makes me wonder if just adding a parameterless MapGroup overload is sufficient for this scenario. I know it lacks discoverability though.
captainsafia
added
area-minimal
There was a problem hiding this comment.
Copilot reviewed 5 out of 8 changed files in this pull request and generated no comments.
Files not reviewed (3)
- src/DefaultBuilder/src/PublicAPI.Unshipped.txt: Language not supported
- src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/Microsoft.AspNetCore.Tests.csproj: Language not supported
- src/DefaultBuilder/src/WebApplicationBuilder.cs: Evaluated as low risk
Comments suppressed due to low confidence (1)
src/Http/Routing/test/UnitTests/Builder/GroupTest.cs:406
- Typo in class name: 'TestCustomEndpintDataSource' should be 'TestCustomEndpointDataSource'.
private sealed class TestCustomEndpintDataSource : EndpointDataSource
| ICollection<EndpointDataSource> IEndpointRouteBuilder.DataSources => DataSources; | ||
|
|
||
| /// <summary> | ||
| /// Gets the <see cref="IEndpointConventionBuilder"/> for the application. |
There was a problem hiding this comment.
Remarks: Only applies to Endpoints not middleware like UseStaticFiles, etc.
| @@ -213,6 +225,7 @@ IApplicationBuilder IApplicationBuilder.New() | |||
| var newBuilder = ApplicationBuilder.New(); | |||
| // Remove the route builder so branched pipelines have their own routing world | |||
| newBuilder.Properties.Remove(GlobalEndpointRouteBuilderKey); | |||
| newBuilder.Properties.Remove(GlobalRouteGroupBuilderKey); | |||
There was a problem hiding this comment.
MapWhen calls won't preserve EndpointConventions?
| // by the WebApplication as the IEndpointRouteBuilder instance | ||
| var globalRouteGroupBuilder = _builtApplication.Properties[WebApplication.GlobalEndpointRouteBuilderKey]; | ||
| app.Properties.Add(WebApplication.GlobalEndpointRouteBuilderKey, globalRouteGroupBuilder); | ||
| app.Properties.Add(WebApplication.GlobalRouteGroupBuilderKey, _builtApplication.Properties[WebApplication.GlobalRouteGroupBuilderKey]); |
There was a problem hiding this comment.
extreme nit: two different coding patterns used here, local var for property value to assign to dictionary vs. inline property value.
Implements #59755.
This pull request introduces changes to the
WebApplicationclass and its associated components to support global endpoint conventions.Changes include:
_dataSourceslist withGlobalEndpointRouteBuilderandRouteGroupBuilderto create an implicit global route group for all endpoints (src/DefaultBuilder/src/WebApplication.cs).Conventionsproperty to exposeIEndpointConventionBuilderfor the application (src/DefaultBuilder/src/WebApplication.cs) that maps to underlyingRouteGroupBuilderWebApplicationGlobalConventionTeststo verify the application of global conventions on endpoints (src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationGlobalConventionTests.cs).Wait to review + merge until attached API proposal is reviewed.