From 2cb5b1379a2aefd93fc62d0fdea2f03adceede6b Mon Sep 17 00:00:00 2001 From: Brennan Date: Thu, 12 Aug 2021 10:53:58 -0700 Subject: [PATCH 01/11] WIP --- src/DefaultBuilder/src/WebApplication.cs | 3 + .../src/WebApplicationBuilder.cs | 14 ++--- .../WebApplicationTests.cs | 63 ++++++++++++++++++- ...ointRoutingApplicationBuilderExtensions.cs | 24 ++++--- 4 files changed, 89 insertions(+), 15 deletions(-) diff --git a/src/DefaultBuilder/src/WebApplication.cs b/src/DefaultBuilder/src/WebApplication.cs index 4d2de4df64fe..e64aad5d2ab0 100644 --- a/src/DefaultBuilder/src/WebApplication.cs +++ b/src/DefaultBuilder/src/WebApplication.cs @@ -28,6 +28,9 @@ internal WebApplication(IHost host) _host = host; ApplicationBuilder = new ApplicationBuilder(host.Services); Logger = host.Services.GetRequiredService().CreateLogger(Environment.ApplicationName); + + //Properties.Add("__EndpointRouteBuilder", this); + //Properties.Add("__GlobalEndpointRouteBuilder", this); } /// diff --git a/src/DefaultBuilder/src/WebApplicationBuilder.cs b/src/DefaultBuilder/src/WebApplicationBuilder.cs index 09d59c6d43f3..fb7c04bcf637 100644 --- a/src/DefaultBuilder/src/WebApplicationBuilder.cs +++ b/src/DefaultBuilder/src/WebApplicationBuilder.cs @@ -197,17 +197,17 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui // The endpoints were already added on the outside if (_builtApplication.DataSources.Count > 0) { - // The user did not register the routing middleware so wrap the entire - // destination pipeline in UseRouting() and UseEndpoints(), essentially: - // destination.UseRouting() - // destination.Run(source) - // destination.UseEndpoints() - // Copy endpoints to the IEndpointRouteBuilder created by an explicit call to UseRouting() if possible. var targetRouteBuilder = GetEndpointRouteBuilder(_builtApplication); if (targetRouteBuilder is null) { + // The user did not register the routing middleware so wrap the entire + // destination pipeline in UseRouting() and UseEndpoints(), essentially: + // destination.UseRouting() + // destination.Run(source) + // destination.UseEndpoints() + // The app defined endpoints without calling UseRouting() explicitly, so call UseRouting() implicitly. app.UseRouting(); @@ -226,7 +226,7 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui // so it must be called after we copy the endpoints. if (!implicitRouting) { - // UseRouting() was called explicitely, but we may still need to call UseEndpoints() implicitely at + // UseRouting() was called explicitly, but we may still need to call UseEndpoints() implicitly at // the end of the pipeline. _builtApplication.UseEndpoints(_ => { }); } diff --git a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs index c2e637882894..46e17e4fa0ae 100644 --- a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs +++ b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs @@ -10,6 +10,7 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Hosting.Server; using Microsoft.AspNetCore.Hosting.Server.Features; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.TestHost; @@ -732,7 +733,7 @@ public void WebApplication_CanResolveDefaultServicesFromServiceCollectionInCorre } [Fact] - public async Task WebApplication_CanCallUseRoutingWithouUseEndpoints() + public async Task WebApplication_CanCallUseRoutingWithoutUseEndpoints() { var builder = WebApplication.CreateBuilder(); builder.WebHost.UseTestServer(); @@ -922,6 +923,66 @@ public void WebApplicationBuilder_ThrowsFromExtensionMethodsNotSupportedByHostAn Assert.Throws(() => builder.Host.ConfigureWebHostDefaults(webHostBuilder => { })); } + [Fact] + public async Task EndpointDataSourceOnlyAddsOnce() + { + var builder = WebApplication.CreateBuilder(); + await using var app = builder.Build(); + + app.UseRouting(); + + app.MapGet("/", () => "Hello World!").WithDisplayName("One"); + + app.UseEndpoints(routes => + { + routes.MapGet("/hi", () => "Hi World").WithDisplayName("Two"); + routes.MapGet("/heyo", () => "Heyo World").WithDisplayName("Three"); + }); + + app.Start(); + + var ds = app.Services.GetRequiredService(); + Assert.Equal(3, ds.Endpoints.Count); + Assert.Equal("Two", ds.Endpoints[0].DisplayName); + Assert.Equal("Three", ds.Endpoints[1].DisplayName); + Assert.Equal("One", ds.Endpoints[2].DisplayName); + } + + [Fact] + public async Task RoutesAddedToCorrectMatcher() + { + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); + + app.UseRouting(); + + var chosenRoute = string.Empty; + + app.Use((context, next) => + { + chosenRoute = context.GetEndpoint()?.DisplayName; + return next(context); + }); + + app.MapGet("/", () => "Hello World").WithDisplayName("One"); + + app.UseEndpoints(endpoints => + { + endpoints.MapGet("/hi", () => "Hello Endpoints").WithDisplayName("Two"); + }); + + app.UseRouting(); + app.UseEndpoints(_ => { }); + + await app.StartAsync(); + + var client = app.GetTestClient(); + + _ = await client.GetAsync("http://localhost/"); + Assert.Equal("One", chosenRoute); + } + private class Service : IService { } private interface IService { } diff --git a/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs b/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs index 07466e2d16a2..bcab1a0cee7c 100644 --- a/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs @@ -44,8 +44,16 @@ public static IApplicationBuilder UseRouting(this IApplicationBuilder builder) VerifyRoutingServicesAreRegistered(builder); - var endpointRouteBuilder = new DefaultEndpointRouteBuilder(builder); - builder.Properties[EndpointRouteBuilder] = endpointRouteBuilder; + IEndpointRouteBuilder endpointRouteBuilder; + if (builder.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out var obj)) + { + endpointRouteBuilder = (IEndpointRouteBuilder)obj!; + } + else + { + endpointRouteBuilder = new DefaultEndpointRouteBuilder(builder); + builder.Properties[EndpointRouteBuilder] = endpointRouteBuilder; + } return builder.UseMiddleware(endpointRouteBuilder); } @@ -102,6 +110,9 @@ public static IApplicationBuilder UseEndpoints(this IApplicationBuilder builder, routeOptions.Value.EndpointDataSources.Add(dataSource); } + builder.Properties.Remove(EndpointRouteBuilder); + //builder.Properties.Remove("__GlobalEndpointRouteBuilder"); + return builder.UseMiddleware(); } @@ -118,9 +129,9 @@ private static void VerifyRoutingServicesAreRegistered(IApplicationBuilder app) } } - private static void VerifyEndpointRoutingMiddlewareIsRegistered(IApplicationBuilder app, out DefaultEndpointRouteBuilder endpointRouteBuilder) + private static void VerifyEndpointRoutingMiddlewareIsRegistered(IApplicationBuilder app, out IEndpointRouteBuilder endpointRouteBuilder) { - if (!app.Properties.TryGetValue(EndpointRouteBuilder, out var obj)) + if (!app.Properties.TryGetValue(EndpointRouteBuilder, out var obj) && !app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out obj)) { var message = $"{nameof(EndpointRoutingMiddleware)} matches endpoints setup by {nameof(EndpointMiddleware)} and so must be added to the request " + @@ -130,12 +141,11 @@ private static void VerifyEndpointRoutingMiddlewareIsRegistered(IApplicationBuil throw new InvalidOperationException(message); } - // If someone messes with this, just let it crash. - endpointRouteBuilder = (DefaultEndpointRouteBuilder)obj!; + endpointRouteBuilder = (IEndpointRouteBuilder)obj!; // This check handles the case where Map or something else that forks the pipeline is called between the two // routing middleware. - if (!object.ReferenceEquals(app, endpointRouteBuilder.ApplicationBuilder)) + if (endpointRouteBuilder is DefaultEndpointRouteBuilder defaultRouteBuilder && !object.ReferenceEquals(app, defaultRouteBuilder.ApplicationBuilder)) { var message = $"The {nameof(EndpointRoutingMiddleware)} and {nameof(EndpointMiddleware)} must be added to the same {nameof(IApplicationBuilder)} instance. " + From 532be58396bc069bbacab7a27893f5b41d4d2f40 Mon Sep 17 00:00:00 2001 From: Brennan Date: Thu, 12 Aug 2021 11:41:33 -0700 Subject: [PATCH 02/11] more WIP --- src/DefaultBuilder/src/WebApplication.cs | 4 +- .../src/WebApplicationBuilder.cs | 66 +++++-------------- .../WebApplicationTests.cs | 6 +- ...ointRoutingApplicationBuilderExtensions.cs | 2 +- 4 files changed, 23 insertions(+), 55 deletions(-) diff --git a/src/DefaultBuilder/src/WebApplication.cs b/src/DefaultBuilder/src/WebApplication.cs index e64aad5d2ab0..f1d054f0c927 100644 --- a/src/DefaultBuilder/src/WebApplication.cs +++ b/src/DefaultBuilder/src/WebApplication.cs @@ -23,6 +23,8 @@ public sealed class WebApplication : IHost, IApplicationBuilder, IEndpointRouteB private readonly IHost _host; private readonly List _dataSources = new(); + internal static string GlobalEndpointRouteBuilderKey = "__GlobalEndpointRouteBuilder"; + internal WebApplication(IHost host) { _host = host; @@ -30,7 +32,7 @@ internal WebApplication(IHost host) Logger = host.Services.GetRequiredService().CreateLogger(Environment.ApplicationName); //Properties.Add("__EndpointRouteBuilder", this); - //Properties.Add("__GlobalEndpointRouteBuilder", this); + Properties.Add(GlobalEndpointRouteBuilderKey, this); } /// diff --git a/src/DefaultBuilder/src/WebApplicationBuilder.cs b/src/DefaultBuilder/src/WebApplicationBuilder.cs index fb7c04bcf637..9e3208770bad 100644 --- a/src/DefaultBuilder/src/WebApplicationBuilder.cs +++ b/src/DefaultBuilder/src/WebApplicationBuilder.cs @@ -17,8 +17,6 @@ namespace Microsoft.AspNetCore.Builder /// public sealed class WebApplicationBuilder { - private const string EndpointRouteBuilderKey = "__EndpointRouteBuilder"; - private readonly HostBuilder _hostBuilder = new(); private readonly BootstrapHostBuilder _bootstrapHostBuilder; private readonly WebApplicationServiceCollection _services = new(); @@ -192,45 +190,14 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui app.UseDeveloperExceptionPage(); } - var implicitRouting = false; - - // The endpoints were already added on the outside - if (_builtApplication.DataSources.Count > 0) - { - // Copy endpoints to the IEndpointRouteBuilder created by an explicit call to UseRouting() if possible. - var targetRouteBuilder = GetEndpointRouteBuilder(_builtApplication); + // Wrap the entire destination pipeline in UseRouting() and UseEndpoints(), essentially: + // destination.UseRouting() + // destination.Run(source) + // destination.UseEndpoints() - if (targetRouteBuilder is null) - { - // The user did not register the routing middleware so wrap the entire - // destination pipeline in UseRouting() and UseEndpoints(), essentially: - // destination.UseRouting() - // destination.Run(source) - // destination.UseEndpoints() - - // The app defined endpoints without calling UseRouting() explicitly, so call UseRouting() implicitly. - app.UseRouting(); - - // An implicitly created IEndpointRouteBuilder was addeded to app.Properties by the UseRouting() call above. - targetRouteBuilder = GetEndpointRouteBuilder(app)!; - implicitRouting = true; - } - - // Copy the endpoints to the explicitly or implicitly created IEndopintRouteBuilder. - foreach (var ds in _builtApplication.DataSources) - { - targetRouteBuilder.DataSources.Add(ds); - } - - // UseEndpoints consumes the DataSources immediately to populate CompositeEndpointDataSource via RouteOptions, - // so it must be called after we copy the endpoints. - if (!implicitRouting) - { - // UseRouting() was called explicitly, but we may still need to call UseEndpoints() implicitly at - // the end of the pipeline. - _builtApplication.UseEndpoints(_ => { }); - } - } + // Set the route builder so we preserve the routing information that may have been set already + app.Properties.Add(WebApplication.GlobalEndpointRouteBuilderKey, _builtApplication); + app.UseRouting(); // Wire the source pipeline to run in the destination pipeline app.Use(next => @@ -239,13 +206,18 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui return _builtApplication.BuildRequestDelegate(); }); - // Implicitly call UseEndpoints() at the end of the pipeline if UseRouting() was called implicitly. - // We could add this to the end of _buildApplication instead if UseEndpoints() was not so picky about - // being called with the same IApplicationBluilder instance as UseRouting(). - if (implicitRouting) + // UseEndpoints() removes the GlobalEndpointRouteBuilderKey, so we can check if UseEndpoints() + // was called by the user, if not we should call it to populate the RouteOptions + if (_builtApplication.Properties.TryGetValue(WebApplication.GlobalEndpointRouteBuilderKey, out _)) { app.UseEndpoints(_ => { }); } + else + { + // Remove the route builder to clean up the properties, we're done adding routes to the pipeline + // REVIEW: This might be unneeded + app.Properties.Remove(WebApplication.GlobalEndpointRouteBuilderKey); + } // Copy the properties to the destination app builder foreach (var item in _builtApplication.Properties) @@ -254,12 +226,6 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui } } - private static IEndpointRouteBuilder? GetEndpointRouteBuilder(IApplicationBuilder app) - { - app.Properties.TryGetValue(EndpointRouteBuilderKey, out var value); - return (IEndpointRouteBuilder?)value; - } - private class LoggingBuilder : ILoggingBuilder { public LoggingBuilder(IServiceCollection services) diff --git a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs index 46e17e4fa0ae..ac2dce10fb41 100644 --- a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs +++ b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs @@ -943,9 +943,9 @@ public async Task EndpointDataSourceOnlyAddsOnce() var ds = app.Services.GetRequiredService(); Assert.Equal(3, ds.Endpoints.Count); - Assert.Equal("Two", ds.Endpoints[0].DisplayName); - Assert.Equal("Three", ds.Endpoints[1].DisplayName); - Assert.Equal("One", ds.Endpoints[2].DisplayName); + Assert.Equal("One", ds.Endpoints[0].DisplayName); + Assert.Equal("Two", ds.Endpoints[1].DisplayName); + Assert.Equal("Three", ds.Endpoints[2].DisplayName); } [Fact] diff --git a/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs b/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs index bcab1a0cee7c..bb31e3889cb1 100644 --- a/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs @@ -111,7 +111,7 @@ public static IApplicationBuilder UseEndpoints(this IApplicationBuilder builder, } builder.Properties.Remove(EndpointRouteBuilder); - //builder.Properties.Remove("__GlobalEndpointRouteBuilder"); + builder.Properties.Remove("__GlobalEndpointRouteBuilder"); return builder.UseMiddleware(); } From 4a60dbb0ee976000808bc5030dfb9296d1eb07b6 Mon Sep 17 00:00:00 2001 From: Brennan Date: Fri, 13 Aug 2021 09:23:21 -0700 Subject: [PATCH 03/11] more --- src/DefaultBuilder/src/WebApplication.cs | 1 - .../src/WebApplicationBuilder.cs | 24 ++-- .../WebApplicationTests.cs | 91 ++++++++++++++- ...ointRoutingApplicationBuilderExtensions.cs | 19 +++- ...RoutingApplicationBuilderExtensionsTest.cs | 104 ++++++++++++++++++ 5 files changed, 220 insertions(+), 19 deletions(-) diff --git a/src/DefaultBuilder/src/WebApplication.cs b/src/DefaultBuilder/src/WebApplication.cs index f1d054f0c927..59d37bed3e98 100644 --- a/src/DefaultBuilder/src/WebApplication.cs +++ b/src/DefaultBuilder/src/WebApplication.cs @@ -31,7 +31,6 @@ internal WebApplication(IHost host) ApplicationBuilder = new ApplicationBuilder(host.Services); Logger = host.Services.GetRequiredService().CreateLogger(Environment.ApplicationName); - //Properties.Add("__EndpointRouteBuilder", this); Properties.Add(GlobalEndpointRouteBuilderKey, this); } diff --git a/src/DefaultBuilder/src/WebApplicationBuilder.cs b/src/DefaultBuilder/src/WebApplicationBuilder.cs index 9e3208770bad..fd95d46039d2 100644 --- a/src/DefaultBuilder/src/WebApplicationBuilder.cs +++ b/src/DefaultBuilder/src/WebApplicationBuilder.cs @@ -187,6 +187,7 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui if (context.HostingEnvironment.IsDevelopment()) { + // TODO: add test for this app.UseDeveloperExceptionPage(); } @@ -206,24 +207,23 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui return _builtApplication.BuildRequestDelegate(); }); - // UseEndpoints() removes the GlobalEndpointRouteBuilderKey, so we can check if UseEndpoints() - // was called by the user, if not we should call it to populate the RouteOptions - if (_builtApplication.Properties.TryGetValue(WebApplication.GlobalEndpointRouteBuilderKey, out _)) - { - app.UseEndpoints(_ => { }); - } - else - { - // Remove the route builder to clean up the properties, we're done adding routes to the pipeline - // REVIEW: This might be unneeded - app.Properties.Remove(WebApplication.GlobalEndpointRouteBuilderKey); - } + // We don't know if user code called UseEndpoints(), so we will call it just in case, and we will make sure we are the only ones setting + // the EndpointDataSource in RouteOptions with this property + app.Properties.Add("__GlobalEndpointBuilderShouldCopyRoutes", null); + app.UseEndpoints(_ => { }); + app.Properties.Remove("__GlobalEndpointBuilderShouldCopyRoutes"); // Copy the properties to the destination app builder foreach (var item in _builtApplication.Properties) { app.Properties[item.Key] = item.Value; } + + // Remove the route builder to clean up the properties, we're done adding routes to the pipeline + // REVIEW: this makes startup filter with userouting/useendpoints fail unless we don't remove the EndpointRouteBuilder in UseEndpoints if a global one exists + // or if we stored the property in this method at the beginning and replaced it at the end. + app.Properties.Remove(WebApplication.GlobalEndpointRouteBuilderKey); + _builtApplication.Properties.Remove(WebApplication.GlobalEndpointRouteBuilderKey); } private class LoggingBuilder : ILoggingBuilder diff --git a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs index ac2dce10fb41..606b62afa8e9 100644 --- a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs +++ b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs @@ -236,7 +236,7 @@ public void WebApplicationBuilderWebHostSettingsThatAffectTheHostCannotBeModifie } [Fact] - public void WebApplicationBuildeSettingInvalidApplicationWillFailAssemblyLoadForUserSecrets() + public void WebApplicationBuilderSettingInvalidApplicationWillFailAssemblyLoadForUserSecrets() { var options = new WebApplicationOptions { @@ -770,6 +770,34 @@ public async Task WebApplication_CanCallUseRoutingWithoutUseEndpoints() Assert.Equal("new", await oldResult.Content.ReadAsStringAsync()); } + // REVIEW: David doesn't like this behavior :) + [Fact] + public async Task WebApplication_CanCallUseEndpointsWithoutUseRouting() + { + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); + + app.MapGet("/1", () => "1"); + + app.UseEndpoints(endpoints => { }); + + await app.StartAsync(); + + var endpointDataSource = app.Services.GetRequiredService(); + + var newEndpoint = Assert.Single(endpointDataSource.Endpoints); + var newRouteEndpoint = Assert.IsType(newEndpoint); + Assert.Equal("/1", newRouteEndpoint.RoutePattern.RawText); + + var client = app.GetTestClient(); + + var oldResult = await client.GetAsync("http://localhost/1"); + oldResult.EnsureSuccessStatusCode(); + + Assert.Equal("1", await oldResult.Content.ReadAsStringAsync()); + } + [Fact] public void WebApplicationCreate_RegistersEventSourceLogger() { @@ -861,6 +889,51 @@ public async Task WebApplicationBuilder_StartupFilterCanAddTerminalMiddleware() Assert.Equal(418, (int)terminalResult.StatusCode); } + [Fact] + public async Task StartupFilter_WithUseRoutingWorks() + { + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + builder.Services.AddSingleton(); + await using var app = builder.Build(); + + var chosenEndpoint = string.Empty; + app.MapGet("/", async c => { + chosenEndpoint = c.GetEndpoint().DisplayName; + await c.Response.WriteAsync("Hello World"); + }).WithDisplayName("One"); + + await app.StartAsync(); + + var client = app.GetTestClient(); + + _ = await client.GetAsync("http://localhost/"); + Assert.Equal("One", chosenEndpoint); + + var response = await client.GetAsync("http://localhost/1"); + Assert.Equal(203, ((int)response.StatusCode)); + } + + class MyFilter : IStartupFilter + { + public Action Configure(Action next) + { + return app => + { + app.UseRouting(); + next(app); + app.UseEndpoints(endpoints => + { + endpoints.MapGet("/1", async c => + { + c.Response.StatusCode = 203; + await c.Response.WriteAsync("Hello Filter"); + }).WithDisplayName("Two"); + }); + }; + } + } + [Fact] public async Task WebApplicationBuilder_OnlyAddsDefaultServicesOnce() { @@ -983,6 +1056,22 @@ public async Task RoutesAddedToCorrectMatcher() Assert.Equal("One", chosenRoute); } + [Fact] + public async Task AppPropertiesDoNotContainARouteBuilder() + { + var builder = WebApplication.CreateBuilder(); + await using var app = builder.Build(); + + app.UseRouting(); + + app.UseEndpoints(endpoints => { }); + + app.Start(); + + Assert.False(app.Properties.TryGetValue("__EndpointRouteBuilder", out _)); + Assert.False(app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out _)); + } + private class Service : IService { } private interface IService { } diff --git a/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs b/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs index bb31e3889cb1..fe9cd5c0b2de 100644 --- a/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs @@ -104,14 +104,23 @@ public static IApplicationBuilder UseEndpoints(this IApplicationBuilder builder, // // Each middleware gets its own collection of data sources, and all of those data sources also // get added to a global collection. - var routeOptions = builder.ApplicationServices.GetRequiredService>(); - foreach (var dataSource in endpointRouteBuilder.DataSources) + // In the global endpoint route case we only want to copy data sources once, so we wait for a specific key before copying data sources + // like in the case of minimal hosting + if (builder.Properties.TryGetValue("__GlobalEndpointBuilderShouldCopyRoutes", out _) || + !builder.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out _)) { - routeOptions.Value.EndpointDataSources.Add(dataSource); + var routeOptions = builder.ApplicationServices.GetRequiredService>(); + foreach (var dataSource in endpointRouteBuilder.DataSources) + { + routeOptions.Value.EndpointDataSources.Add(dataSource); + } } - builder.Properties.Remove(EndpointRouteBuilder); - builder.Properties.Remove("__GlobalEndpointRouteBuilder"); + // REVIEW: this 'if' could be removed, see comment in WebApplicationBuilder + if (!builder.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out _)) + { + builder.Properties.Remove(EndpointRouteBuilder); + } return builder.UseMiddleware(); } diff --git a/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs index 4b9d875d0a1c..8f2e7bdb90f6 100644 --- a/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs @@ -12,6 +12,7 @@ using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.AspNetCore.Routing.TestObjects; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using Moq; using Xunit; @@ -295,6 +296,109 @@ public void UseEndpoints_CallWithBuilder_SetsEndpointDataSource_WithMap() e => Assert.Equal("Test endpoint 4", e.DisplayName)); } + [Fact] + public void UseEndpoints_WithGlobalEndpointRouteBuilder_CopiesRoutesWhenPropertySet() + { + // Arrange + var services = CreateServices(); + + var app = new ApplicationBuilder(services); + + var mockRouteBuilder = new Mock(); + mockRouteBuilder.Setup(m => m.DataSources).Returns(new List()); + + var routeBuilder = mockRouteBuilder.Object; + app.Properties.Add("__GlobalEndpointRouteBuilder", routeBuilder); + app.UseRouting(); + + Assert.False(app.Properties.TryGetValue("__EndpointRouteBuilder", out _)); + + app.UseEndpoints(endpoints => + { + endpoints.Map("/1", d => Task.CompletedTask).WithDisplayName("Test endpoint 1"); + }); + var routeOptions = app.ApplicationServices.GetRequiredService>(); + Assert.Empty(routeOptions.Value.EndpointDataSources); + + app.Properties.Add("__GlobalEndpointBuilderShouldCopyRoutes", null); + + app.UseEndpoints(endpoints => { }); + + var requestDelegate = app.Build(); + + var endpointDataSource = Assert.Single(mockRouteBuilder.Object.DataSources); + Assert.Collection(endpointDataSource.Endpoints, + e => Assert.Equal("Test endpoint 1", e.DisplayName)); + + routeOptions = app.ApplicationServices.GetRequiredService>(); + Assert.Equal(mockRouteBuilder.Object.DataSources, routeOptions.Value.EndpointDataSources); + } + + [Fact] + public void UseEndpoint_WithGlobalEndpointRouteBuilderNoCopyProperty_HasEndpointsNoRoutes() + { + // Arrange + var services = CreateServices(); + + var app = new ApplicationBuilder(services); + + var mockRouteBuilder = new Mock(); + mockRouteBuilder.Setup(m => m.DataSources).Returns(new List()); + + var routeBuilder = mockRouteBuilder.Object; + app.Properties.Add("__GlobalEndpointRouteBuilder", routeBuilder); + app.UseRouting(); + + Assert.False(app.Properties.TryGetValue("__EndpointRouteBuilder", out _)); + + app.UseEndpoints(endpoints => + { + endpoints.Map("/1", d => Task.CompletedTask).WithDisplayName("Test endpoint 1"); + }); + + var requestDelegate = app.Build(); + + var endpointDataSource = Assert.Single(mockRouteBuilder.Object.DataSources); + Assert.Collection(endpointDataSource.Endpoints, + e => Assert.Equal("Test endpoint 1", e.DisplayName)); + + var routeOptions = app.ApplicationServices.GetRequiredService>(); + Assert.Empty(routeOptions.Value.EndpointDataSources); + } + + [Fact] + public void UseRouting_DoesNotSetEndpointRouteBuilder_IfGlobalOneExists() + { + // Arrange + var services = CreateServices(); + + var app = new ApplicationBuilder(services); + + var routeBuilder = new Mock().Object; + app.Properties.Add("__GlobalEndpointRouteBuilder", routeBuilder); + app.UseRouting(); + + Assert.False(app.Properties.TryGetValue("__EndpointRouteBuilder", out _)); + Assert.True(app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out _)); + } + + [Fact] + public void UseEndpoints_RemovesEndpointRouteBuilderProperty() + { + // Arrange + var services = CreateServices(); + + var app = new ApplicationBuilder(services); + + app.UseRouting(); + + Assert.True(app.Properties.TryGetValue("__EndpointRouteBuilder", out _)); + + app.UseEndpoints(endpoints => { }); + + Assert.False(app.Properties.TryGetValue("__EndpointRouteBuilder", out _)); + } + private IServiceProvider CreateServices() { return CreateServices(matcherFactory: null); From 5dffb3d930ca3e8c369eab62ff497c9f4c2922a3 Mon Sep 17 00:00:00 2001 From: Brennan Date: Fri, 13 Aug 2021 11:21:14 -0700 Subject: [PATCH 04/11] lets do this --- .../WebApplicationTests.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs index 606b62afa8e9..ab63ff479ee4 100644 --- a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs +++ b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs @@ -1072,6 +1072,32 @@ public async Task AppPropertiesDoNotContainARouteBuilder() Assert.False(app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out _)); } + [Fact] + public async Task WebApplication_CallsUseRoutingAndUseEndpoints() + { + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); + + var chosenRoute = string.Empty; + app.MapGet("/", async c => + { + chosenRoute = c.GetEndpoint()?.DisplayName; + await c.Response.WriteAsync("Hello World"); + }).WithDisplayName("One"); + + await app.StartAsync(); + + var ds = app.Services.GetRequiredService(); + Assert.Equal(1, ds.Endpoints.Count); + Assert.Equal("One", ds.Endpoints[0].DisplayName); + + var client = app.GetTestClient(); + + _ = await client.GetAsync("http://localhost/"); + Assert.Equal("One", chosenRoute); + } + private class Service : IService { } private interface IService { } From 856d8a8c64318028d5c25a433181280b0622b9ca Mon Sep 17 00:00:00 2001 From: Brennan Date: Sat, 14 Aug 2021 10:36:52 -0700 Subject: [PATCH 05/11] fb --- src/DefaultBuilder/src/WebApplication.cs | 9 +- .../src/WebApplicationBuilder.cs | 6 +- .../WebApplicationTests.cs | 106 ++++++++++++++---- ...ointRoutingApplicationBuilderExtensions.cs | 12 +- 4 files changed, 102 insertions(+), 31 deletions(-) diff --git a/src/DefaultBuilder/src/WebApplication.cs b/src/DefaultBuilder/src/WebApplication.cs index 59d37bed3e98..3a8184ba08af 100644 --- a/src/DefaultBuilder/src/WebApplication.cs +++ b/src/DefaultBuilder/src/WebApplication.cs @@ -31,7 +31,7 @@ internal WebApplication(IHost host) ApplicationBuilder = new ApplicationBuilder(host.Services); Logger = host.Services.GetRequiredService().CreateLogger(Environment.ApplicationName); - Properties.Add(GlobalEndpointRouteBuilderKey, this); + Properties[GlobalEndpointRouteBuilderKey] = this; } /// @@ -174,7 +174,12 @@ public void Run(string? url = null) RequestDelegate IApplicationBuilder.Build() => BuildRequestDelegate(); // REVIEW: Should this be wrapping another type? - IApplicationBuilder IApplicationBuilder.New() => ApplicationBuilder.New(); + IApplicationBuilder IApplicationBuilder.New() + { + var newBuilder = ApplicationBuilder.New(); + newBuilder.Properties.Remove(GlobalEndpointRouteBuilderKey); + return newBuilder; + } IApplicationBuilder IApplicationBuilder.Use(Func middleware) { diff --git a/src/DefaultBuilder/src/WebApplicationBuilder.cs b/src/DefaultBuilder/src/WebApplicationBuilder.cs index fd95d46039d2..a222da266458 100644 --- a/src/DefaultBuilder/src/WebApplicationBuilder.cs +++ b/src/DefaultBuilder/src/WebApplicationBuilder.cs @@ -4,7 +4,6 @@ using System.Diagnostics; using System.Reflection; using Microsoft.AspNetCore.Hosting; -using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; @@ -20,6 +19,7 @@ public sealed class WebApplicationBuilder private readonly HostBuilder _hostBuilder = new(); private readonly BootstrapHostBuilder _bootstrapHostBuilder; private readonly WebApplicationServiceCollection _services = new(); + private const string GlobalEndpointBuilderCopyRoutesKey = "__GlobalEndpointBuilderShouldCopyRoutes"; private WebApplication? _builtApplication; @@ -209,9 +209,9 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui // We don't know if user code called UseEndpoints(), so we will call it just in case, and we will make sure we are the only ones setting // the EndpointDataSource in RouteOptions with this property - app.Properties.Add("__GlobalEndpointBuilderShouldCopyRoutes", null); + app.Properties[GlobalEndpointBuilderCopyRoutesKey] = null; app.UseEndpoints(_ => { }); - app.Properties.Remove("__GlobalEndpointBuilderShouldCopyRoutes"); + app.Properties.Remove(GlobalEndpointBuilderCopyRoutesKey); // Copy the properties to the destination app builder foreach (var item in _builtApplication.Properties) diff --git a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs index ab63ff479ee4..750c60fa78ca 100644 --- a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs +++ b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs @@ -894,7 +894,7 @@ public async Task StartupFilter_WithUseRoutingWorks() { var builder = WebApplication.CreateBuilder(); builder.WebHost.UseTestServer(); - builder.Services.AddSingleton(); + builder.Services.AddSingleton(); await using var app = builder.Build(); var chosenEndpoint = string.Empty; @@ -914,26 +914,6 @@ public async Task StartupFilter_WithUseRoutingWorks() Assert.Equal(203, ((int)response.StatusCode)); } - class MyFilter : IStartupFilter - { - public Action Configure(Action next) - { - return app => - { - app.UseRouting(); - next(app); - app.UseEndpoints(endpoints => - { - endpoints.MapGet("/1", async c => - { - c.Response.StatusCode = 203; - await c.Response.WriteAsync("Hello Filter"); - }).WithDisplayName("Two"); - }); - }; - } - } - [Fact] public async Task WebApplicationBuilder_OnlyAddsDefaultServicesOnce() { @@ -1098,6 +1078,70 @@ public async Task WebApplication_CallsUseRoutingAndUseEndpoints() Assert.Equal("One", chosenRoute); } + [Fact] + public async Task BranchingPipelineHasOwnRoutes() + { + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); + + app.UseRouting(); + + var chosenRoute = string.Empty; + app.MapGet("/", () => "Hello World!").WithDisplayName("One"); + + app.UseEndpoints(routes => + { + routes.MapGet("/hi", async c => + { + chosenRoute = c.GetEndpoint()?.DisplayName; + await c.Response.WriteAsync("Hello World"); + }).WithDisplayName("Two"); + routes.MapGet("/heyo", () => "Heyo World").WithDisplayName("Three"); + }); + + // REVIEW: this cast is strange + var newBuilder = ((IApplicationBuilder)app).New(); + Assert.False(newBuilder.Properties.TryGetValue(WebApplication.GlobalEndpointRouteBuilderKey, out _)); + + newBuilder.UseRouting(); + newBuilder.UseEndpoints(endpoints => + { + endpoints.MapGet("/h3", async c => + { + chosenRoute = c.GetEndpoint()?.DisplayName; + await c.Response.WriteAsync("Hello World"); + }).WithDisplayName("Four"); + endpoints.MapGet("hi", async c => + { + chosenRoute = c.GetEndpoint()?.DisplayName; + await c.Response.WriteAsync("Hi New"); + }).WithDisplayName("Five"); + }); + var branch = newBuilder.Build(); + app.Run(c => branch(c)); + + app.Start(); + + var ds = app.Services.GetRequiredService(); + Assert.Equal(5, ds.Endpoints.Count); + Assert.Equal("Four", ds.Endpoints[0].DisplayName); + Assert.Equal("Five", ds.Endpoints[1].DisplayName); + Assert.Equal("One", ds.Endpoints[2].DisplayName); + Assert.Equal("Two", ds.Endpoints[3].DisplayName); + Assert.Equal("Three", ds.Endpoints[4].DisplayName); + + var client = app.GetTestClient(); + + // '/hi' routes don't conflict and the non-branched one is chosen + _ = await client.GetAsync("http://localhost/hi"); + Assert.Equal("Two", chosenRoute); + + // Can access branched routes + _ = await client.GetAsync("http://localhost/h3"); + Assert.Equal("Four", chosenRoute); + } + private class Service : IService { } private interface IService { } @@ -1307,5 +1351,25 @@ public Action Configure(Action next) }; } } + + class UseRoutingStartupFilter : IStartupFilter + { + public Action Configure(Action next) + { + return app => + { + app.UseRouting(); + next(app); + app.UseEndpoints(endpoints => + { + endpoints.MapGet("/1", async c => + { + c.Response.StatusCode = 203; + await c.Response.WriteAsync("Hello Filter"); + }).WithDisplayName("Two"); + }); + }; + } + } } } diff --git a/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs b/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs index fe9cd5c0b2de..f309136a1646 100644 --- a/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs @@ -15,6 +15,8 @@ namespace Microsoft.AspNetCore.Builder public static class EndpointRoutingApplicationBuilderExtensions { private const string EndpointRouteBuilder = "__EndpointRouteBuilder"; + private const string GlobalEndpointRouteBuilderKey = "__GlobalEndpointRouteBuilder"; + private const string GlobalEndpointBuilderCopyRoutesKey = "__GlobalEndpointBuilderShouldCopyRoutes"; /// /// Adds a middleware to the specified . @@ -45,7 +47,7 @@ public static IApplicationBuilder UseRouting(this IApplicationBuilder builder) VerifyRoutingServicesAreRegistered(builder); IEndpointRouteBuilder endpointRouteBuilder; - if (builder.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out var obj)) + if (builder.Properties.TryGetValue(GlobalEndpointRouteBuilderKey, out var obj)) { endpointRouteBuilder = (IEndpointRouteBuilder)obj!; } @@ -106,8 +108,8 @@ public static IApplicationBuilder UseEndpoints(this IApplicationBuilder builder, // get added to a global collection. // In the global endpoint route case we only want to copy data sources once, so we wait for a specific key before copying data sources // like in the case of minimal hosting - if (builder.Properties.TryGetValue("__GlobalEndpointBuilderShouldCopyRoutes", out _) || - !builder.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out _)) + if (builder.Properties.TryGetValue(GlobalEndpointBuilderCopyRoutesKey, out _) || + !builder.Properties.TryGetValue(GlobalEndpointRouteBuilderKey, out _)) { var routeOptions = builder.ApplicationServices.GetRequiredService>(); foreach (var dataSource in endpointRouteBuilder.DataSources) @@ -117,7 +119,7 @@ public static IApplicationBuilder UseEndpoints(this IApplicationBuilder builder, } // REVIEW: this 'if' could be removed, see comment in WebApplicationBuilder - if (!builder.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out _)) + if (!builder.Properties.TryGetValue(GlobalEndpointRouteBuilderKey, out _)) { builder.Properties.Remove(EndpointRouteBuilder); } @@ -140,7 +142,7 @@ private static void VerifyRoutingServicesAreRegistered(IApplicationBuilder app) private static void VerifyEndpointRoutingMiddlewareIsRegistered(IApplicationBuilder app, out IEndpointRouteBuilder endpointRouteBuilder) { - if (!app.Properties.TryGetValue(EndpointRouteBuilder, out var obj) && !app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out obj)) + if (!app.Properties.TryGetValue(EndpointRouteBuilder, out var obj) && !app.Properties.TryGetValue(GlobalEndpointRouteBuilderKey, out obj)) { var message = $"{nameof(EndpointRoutingMiddleware)} matches endpoints setup by {nameof(EndpointMiddleware)} and so must be added to the request " + From 0cc8fd52f9631b66892ba0591a8eb614f54aa15d Mon Sep 17 00:00:00 2001 From: Brennan Date: Mon, 16 Aug 2021 14:32:43 -0700 Subject: [PATCH 06/11] some stuff --- src/DefaultBuilder/src/WebApplication.cs | 3 +- .../src/WebApplicationBuilder.cs | 41 ++++++++++++++----- .../WebApplicationTests.cs | 37 ++++++++++++++++- ...ointRoutingApplicationBuilderExtensions.cs | 8 +--- ...RoutingApplicationBuilderExtensionsTest.cs | 17 -------- 5 files changed, 71 insertions(+), 35 deletions(-) diff --git a/src/DefaultBuilder/src/WebApplication.cs b/src/DefaultBuilder/src/WebApplication.cs index 3a8184ba08af..d6c8ca5c609a 100644 --- a/src/DefaultBuilder/src/WebApplication.cs +++ b/src/DefaultBuilder/src/WebApplication.cs @@ -177,6 +177,7 @@ public void Run(string? url = null) IApplicationBuilder IApplicationBuilder.New() { var newBuilder = ApplicationBuilder.New(); + // Remove the route builder so branched pipelines have their own routing world newBuilder.Properties.Remove(GlobalEndpointRouteBuilderKey); return newBuilder; } @@ -187,7 +188,7 @@ IApplicationBuilder IApplicationBuilder.Use(Func ApplicationBuilder.New(); + IApplicationBuilder IEndpointRouteBuilder.CreateApplicationBuilder() => ((IApplicationBuilder)this).New(); private void Listen(string? url) { diff --git a/src/DefaultBuilder/src/WebApplicationBuilder.cs b/src/DefaultBuilder/src/WebApplicationBuilder.cs index a222da266458..a95b6bf4da38 100644 --- a/src/DefaultBuilder/src/WebApplicationBuilder.cs +++ b/src/DefaultBuilder/src/WebApplicationBuilder.cs @@ -185,6 +185,13 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui { Debug.Assert(_builtApplication is not null); + // UseRouting called before WebApplication such as in a StartupFilter + // lets remove the property and reset it at the end so we don't mess with the routes in the filter + if (app.Properties.TryGetValue("__EndpointRouteBuilder", out var priorRouteBuilder)) + { + app.Properties.Remove("__EndpointRouteBuilder"); + } + if (context.HostingEnvironment.IsDevelopment()) { // TODO: add test for this @@ -196,9 +203,13 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui // destination.Run(source) // destination.UseEndpoints() - // Set the route builder so we preserve the routing information that may have been set already - app.Properties.Add(WebApplication.GlobalEndpointRouteBuilderKey, _builtApplication); - app.UseRouting(); + // Only call UseRouting() if there are endpoints configured and UseRouting() wasn't called on the global route builder already + if (_builtApplication.DataSources.Count > 0 && !_builtApplication.Properties.TryGetValue("__UseRoutingWithGlobalSet", out _)) + { + // Set the route builder so that UseRouting will use the WebApplication as the IEndpointRouteBuilder for route matching + app.Properties.Add(WebApplication.GlobalEndpointRouteBuilderKey, _builtApplication); + app.UseRouting(); + } // Wire the source pipeline to run in the destination pipeline app.Use(next => @@ -207,11 +218,15 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui return _builtApplication.BuildRequestDelegate(); }); - // We don't know if user code called UseEndpoints(), so we will call it just in case, and we will make sure we are the only ones setting - // the EndpointDataSource in RouteOptions with this property - app.Properties[GlobalEndpointBuilderCopyRoutesKey] = null; - app.UseEndpoints(_ => { }); - app.Properties.Remove(GlobalEndpointBuilderCopyRoutesKey); + if (_builtApplication.DataSources.Count > 0) + { + app.Properties.TryAdd(WebApplication.GlobalEndpointRouteBuilderKey, _builtApplication); + // We don't know if user code called UseEndpoints(), so we will call it just in case, and we will make sure we are the only ones setting + // the EndpointDataSource in RouteOptions with this property + app.Properties[GlobalEndpointBuilderCopyRoutesKey] = null; + app.UseEndpoints(_ => { }); + app.Properties.Remove(GlobalEndpointBuilderCopyRoutesKey); + } // Copy the properties to the destination app builder foreach (var item in _builtApplication.Properties) @@ -220,10 +235,16 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui } // Remove the route builder to clean up the properties, we're done adding routes to the pipeline - // REVIEW: this makes startup filter with userouting/useendpoints fail unless we don't remove the EndpointRouteBuilder in UseEndpoints if a global one exists - // or if we stored the property in this method at the beginning and replaced it at the end. + // REVIEW: There aren't any tests that fail if these two lines are removed (except the one that verfies the properties are removed) + // Not sure if it's a missing scenario or not needed app.Properties.Remove(WebApplication.GlobalEndpointRouteBuilderKey); _builtApplication.Properties.Remove(WebApplication.GlobalEndpointRouteBuilderKey); + + // reset route builder if it existed, this is needed for StartupFilters + if (priorRouteBuilder is { }) + { + app.Properties["__EndpointRouteBuilder"] = priorRouteBuilder; + } } private class LoggingBuilder : ILoggingBuilder diff --git a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs index 750c60fa78ca..f7acdbe95939 100644 --- a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs +++ b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs @@ -914,6 +914,42 @@ public async Task StartupFilter_WithUseRoutingWorks() Assert.Equal(203, ((int)response.StatusCode)); } + [Fact] + public async Task CanAddMiddlewareBeforeUseRouting() + { + var builder = WebApplication.CreateBuilder(); + builder.WebHost.UseTestServer(); + await using var app = builder.Build(); + + var chosenEndpoint = string.Empty; + + app.Use((c, n) => + { + chosenEndpoint = c.GetEndpoint()?.DisplayName; + Assert.Null(c.GetEndpoint()); + return n(c); + }); + + app.UseRouting(); + + app.MapGet("/1", async c => { + chosenEndpoint = c.GetEndpoint().DisplayName; + await c.Response.WriteAsync("Hello World"); + }).WithDisplayName("One"); + + app.UseEndpoints(e => { }); + + await app.StartAsync(); + + var client = app.GetTestClient(); + + _ = await client.GetAsync("http://localhost/"); + Assert.Null(chosenEndpoint); + + _ = await client.GetAsync("http://localhost/1"); + Assert.Equal("One", chosenEndpoint); + } + [Fact] public async Task WebApplicationBuilder_OnlyAddsDefaultServicesOnce() { @@ -1100,7 +1136,6 @@ public async Task BranchingPipelineHasOwnRoutes() routes.MapGet("/heyo", () => "Heyo World").WithDisplayName("Three"); }); - // REVIEW: this cast is strange var newBuilder = ((IApplicationBuilder)app).New(); Assert.False(newBuilder.Properties.TryGetValue(WebApplication.GlobalEndpointRouteBuilderKey, out _)); diff --git a/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs b/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs index f309136a1646..ebc221abc306 100644 --- a/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs @@ -50,6 +50,8 @@ public static IApplicationBuilder UseRouting(this IApplicationBuilder builder) if (builder.Properties.TryGetValue(GlobalEndpointRouteBuilderKey, out var obj)) { endpointRouteBuilder = (IEndpointRouteBuilder)obj!; + // Let interested parties know if UseRouting() was called while a global route builder was set + builder.Properties["__UseRoutingWithGlobalSet"] = true; } else { @@ -118,12 +120,6 @@ public static IApplicationBuilder UseEndpoints(this IApplicationBuilder builder, } } - // REVIEW: this 'if' could be removed, see comment in WebApplicationBuilder - if (!builder.Properties.TryGetValue(GlobalEndpointRouteBuilderKey, out _)) - { - builder.Properties.Remove(EndpointRouteBuilder); - } - return builder.UseMiddleware(); } diff --git a/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs index 8f2e7bdb90f6..930ec8bbee46 100644 --- a/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs @@ -382,23 +382,6 @@ public void UseRouting_DoesNotSetEndpointRouteBuilder_IfGlobalOneExists() Assert.True(app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out _)); } - [Fact] - public void UseEndpoints_RemovesEndpointRouteBuilderProperty() - { - // Arrange - var services = CreateServices(); - - var app = new ApplicationBuilder(services); - - app.UseRouting(); - - Assert.True(app.Properties.TryGetValue("__EndpointRouteBuilder", out _)); - - app.UseEndpoints(endpoints => { }); - - Assert.False(app.Properties.TryGetValue("__EndpointRouteBuilder", out _)); - } - private IServiceProvider CreateServices() { return CreateServices(matcherFactory: null); From 752f3030a8a2f7bff44a4484db8c0dc06a71feaf Mon Sep 17 00:00:00 2001 From: Brennan Date: Mon, 16 Aug 2021 21:47:01 -0700 Subject: [PATCH 07/11] progress --- .../src/WebApplicationBuilder.cs | 11 ++--- .../WebApplicationTests.cs | 11 +++-- ...ointRoutingApplicationBuilderExtensions.cs | 10 ++--- ...RoutingApplicationBuilderExtensionsTest.cs | 42 +------------------ 4 files changed, 14 insertions(+), 60 deletions(-) diff --git a/src/DefaultBuilder/src/WebApplicationBuilder.cs b/src/DefaultBuilder/src/WebApplicationBuilder.cs index a95b6bf4da38..f8e0b1109a7e 100644 --- a/src/DefaultBuilder/src/WebApplicationBuilder.cs +++ b/src/DefaultBuilder/src/WebApplicationBuilder.cs @@ -203,11 +203,12 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui // destination.Run(source) // destination.UseEndpoints() + // Set the route builder so that UseRouting will use the WebApplication as the IEndpointRouteBuilder for route matching + app.Properties.Add(WebApplication.GlobalEndpointRouteBuilderKey, _builtApplication); + // Only call UseRouting() if there are endpoints configured and UseRouting() wasn't called on the global route builder already if (_builtApplication.DataSources.Count > 0 && !_builtApplication.Properties.TryGetValue("__UseRoutingWithGlobalSet", out _)) { - // Set the route builder so that UseRouting will use the WebApplication as the IEndpointRouteBuilder for route matching - app.Properties.Add(WebApplication.GlobalEndpointRouteBuilderKey, _builtApplication); app.UseRouting(); } @@ -220,7 +221,6 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui if (_builtApplication.DataSources.Count > 0) { - app.Properties.TryAdd(WebApplication.GlobalEndpointRouteBuilderKey, _builtApplication); // We don't know if user code called UseEndpoints(), so we will call it just in case, and we will make sure we are the only ones setting // the EndpointDataSource in RouteOptions with this property app.Properties[GlobalEndpointBuilderCopyRoutesKey] = null; @@ -235,13 +235,10 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui } // Remove the route builder to clean up the properties, we're done adding routes to the pipeline - // REVIEW: There aren't any tests that fail if these two lines are removed (except the one that verfies the properties are removed) - // Not sure if it's a missing scenario or not needed app.Properties.Remove(WebApplication.GlobalEndpointRouteBuilderKey); - _builtApplication.Properties.Remove(WebApplication.GlobalEndpointRouteBuilderKey); // reset route builder if it existed, this is needed for StartupFilters - if (priorRouteBuilder is { }) + if (priorRouteBuilder is not null) { app.Properties["__EndpointRouteBuilder"] = priorRouteBuilder; } diff --git a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs index f7acdbe95939..ad44706c876b 100644 --- a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs +++ b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs @@ -1085,7 +1085,6 @@ public async Task AppPropertiesDoNotContainARouteBuilder() app.Start(); Assert.False(app.Properties.TryGetValue("__EndpointRouteBuilder", out _)); - Assert.False(app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out _)); } [Fact] @@ -1160,11 +1159,11 @@ public async Task BranchingPipelineHasOwnRoutes() var ds = app.Services.GetRequiredService(); Assert.Equal(5, ds.Endpoints.Count); - Assert.Equal("Four", ds.Endpoints[0].DisplayName); - Assert.Equal("Five", ds.Endpoints[1].DisplayName); - Assert.Equal("One", ds.Endpoints[2].DisplayName); - Assert.Equal("Two", ds.Endpoints[3].DisplayName); - Assert.Equal("Three", ds.Endpoints[4].DisplayName); + Assert.Equal("One", ds.Endpoints[0].DisplayName); + Assert.Equal("Two", ds.Endpoints[1].DisplayName); + Assert.Equal("Three", ds.Endpoints[2].DisplayName); + Assert.Equal("Four", ds.Endpoints[3].DisplayName); + Assert.Equal("Five", ds.Endpoints[4].DisplayName); var client = app.GetTestClient(); diff --git a/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs b/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs index ebc221abc306..a56e000226d8 100644 --- a/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs @@ -16,7 +16,6 @@ public static class EndpointRoutingApplicationBuilderExtensions { private const string EndpointRouteBuilder = "__EndpointRouteBuilder"; private const string GlobalEndpointRouteBuilderKey = "__GlobalEndpointRouteBuilder"; - private const string GlobalEndpointBuilderCopyRoutesKey = "__GlobalEndpointBuilderShouldCopyRoutes"; /// /// Adds a middleware to the specified . @@ -108,13 +107,10 @@ public static IApplicationBuilder UseEndpoints(this IApplicationBuilder builder, // // Each middleware gets its own collection of data sources, and all of those data sources also // get added to a global collection. - // In the global endpoint route case we only want to copy data sources once, so we wait for a specific key before copying data sources - // like in the case of minimal hosting - if (builder.Properties.TryGetValue(GlobalEndpointBuilderCopyRoutesKey, out _) || - !builder.Properties.TryGetValue(GlobalEndpointRouteBuilderKey, out _)) + var routeOptions = builder.ApplicationServices.GetRequiredService>(); + foreach (var dataSource in endpointRouteBuilder.DataSources) { - var routeOptions = builder.ApplicationServices.GetRequiredService>(); - foreach (var dataSource in endpointRouteBuilder.DataSources) + if (!routeOptions.Value.EndpointDataSources.Contains(dataSource)) { routeOptions.Value.EndpointDataSources.Add(dataSource); } diff --git a/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs index 930ec8bbee46..ebcd0d5a198f 100644 --- a/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs @@ -297,45 +297,7 @@ public void UseEndpoints_CallWithBuilder_SetsEndpointDataSource_WithMap() } [Fact] - public void UseEndpoints_WithGlobalEndpointRouteBuilder_CopiesRoutesWhenPropertySet() - { - // Arrange - var services = CreateServices(); - - var app = new ApplicationBuilder(services); - - var mockRouteBuilder = new Mock(); - mockRouteBuilder.Setup(m => m.DataSources).Returns(new List()); - - var routeBuilder = mockRouteBuilder.Object; - app.Properties.Add("__GlobalEndpointRouteBuilder", routeBuilder); - app.UseRouting(); - - Assert.False(app.Properties.TryGetValue("__EndpointRouteBuilder", out _)); - - app.UseEndpoints(endpoints => - { - endpoints.Map("/1", d => Task.CompletedTask).WithDisplayName("Test endpoint 1"); - }); - var routeOptions = app.ApplicationServices.GetRequiredService>(); - Assert.Empty(routeOptions.Value.EndpointDataSources); - - app.Properties.Add("__GlobalEndpointBuilderShouldCopyRoutes", null); - - app.UseEndpoints(endpoints => { }); - - var requestDelegate = app.Build(); - - var endpointDataSource = Assert.Single(mockRouteBuilder.Object.DataSources); - Assert.Collection(endpointDataSource.Endpoints, - e => Assert.Equal("Test endpoint 1", e.DisplayName)); - - routeOptions = app.ApplicationServices.GetRequiredService>(); - Assert.Equal(mockRouteBuilder.Object.DataSources, routeOptions.Value.EndpointDataSources); - } - - [Fact] - public void UseEndpoint_WithGlobalEndpointRouteBuilderNoCopyProperty_HasEndpointsNoRoutes() + public void UseEndpoints_WithGlobalEndpointRouteBuilderHasRoutes() { // Arrange var services = CreateServices(); @@ -363,7 +325,7 @@ public void UseEndpoint_WithGlobalEndpointRouteBuilderNoCopyProperty_HasEndpoint e => Assert.Equal("Test endpoint 1", e.DisplayName)); var routeOptions = app.ApplicationServices.GetRequiredService>(); - Assert.Empty(routeOptions.Value.EndpointDataSources); + Assert.Equal(mockRouteBuilder.Object.DataSources, routeOptions.Value.EndpointDataSources); } [Fact] From 9edd5eb8e76c0d7511391b10b1c78843cb7d94a6 Mon Sep 17 00:00:00 2001 From: Brennan Date: Mon, 16 Aug 2021 22:08:02 -0700 Subject: [PATCH 08/11] remove key --- src/DefaultBuilder/src/WebApplicationBuilder.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/DefaultBuilder/src/WebApplicationBuilder.cs b/src/DefaultBuilder/src/WebApplicationBuilder.cs index f8e0b1109a7e..602f063ab5c0 100644 --- a/src/DefaultBuilder/src/WebApplicationBuilder.cs +++ b/src/DefaultBuilder/src/WebApplicationBuilder.cs @@ -19,7 +19,6 @@ public sealed class WebApplicationBuilder private readonly HostBuilder _hostBuilder = new(); private readonly BootstrapHostBuilder _bootstrapHostBuilder; private readonly WebApplicationServiceCollection _services = new(); - private const string GlobalEndpointBuilderCopyRoutesKey = "__GlobalEndpointBuilderShouldCopyRoutes"; private WebApplication? _builtApplication; @@ -221,11 +220,8 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui if (_builtApplication.DataSources.Count > 0) { - // We don't know if user code called UseEndpoints(), so we will call it just in case, and we will make sure we are the only ones setting - // the EndpointDataSource in RouteOptions with this property - app.Properties[GlobalEndpointBuilderCopyRoutesKey] = null; + // We don't know if user code called UseEndpoints(), so we will call it just in case, UseEndpoints() will ignore duplicate DataSources app.UseEndpoints(_ => { }); - app.Properties.Remove(GlobalEndpointBuilderCopyRoutesKey); } // Copy the properties to the destination app builder From 03af810d6ca3cecdeddc99e70e98ede455459a3c Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 17 Aug 2021 10:24:18 -0700 Subject: [PATCH 09/11] remove prop --- .../src/WebApplicationBuilder.cs | 20 ++++++-- .../WebApplicationTests.cs | 48 ++++++++++++------- ...ointRoutingApplicationBuilderExtensions.cs | 4 +- ...RoutingApplicationBuilderExtensionsTest.cs | 9 ++-- 4 files changed, 52 insertions(+), 29 deletions(-) diff --git a/src/DefaultBuilder/src/WebApplicationBuilder.cs b/src/DefaultBuilder/src/WebApplicationBuilder.cs index 602f063ab5c0..f3fc939621be 100644 --- a/src/DefaultBuilder/src/WebApplicationBuilder.cs +++ b/src/DefaultBuilder/src/WebApplicationBuilder.cs @@ -19,6 +19,7 @@ public sealed class WebApplicationBuilder private readonly HostBuilder _hostBuilder = new(); private readonly BootstrapHostBuilder _bootstrapHostBuilder; private readonly WebApplicationServiceCollection _services = new(); + private static string EndpointRouteBuilderKey = "__EndpointRouteBuilder"; private WebApplication? _builtApplication; @@ -186,7 +187,7 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui // UseRouting called before WebApplication such as in a StartupFilter // lets remove the property and reset it at the end so we don't mess with the routes in the filter - if (app.Properties.TryGetValue("__EndpointRouteBuilder", out var priorRouteBuilder)) + if (app.Properties.TryGetValue(EndpointRouteBuilderKey, out var priorRouteBuilder)) { app.Properties.Remove("__EndpointRouteBuilder"); } @@ -206,9 +207,18 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui app.Properties.Add(WebApplication.GlobalEndpointRouteBuilderKey, _builtApplication); // Only call UseRouting() if there are endpoints configured and UseRouting() wasn't called on the global route builder already - if (_builtApplication.DataSources.Count > 0 && !_builtApplication.Properties.TryGetValue("__UseRoutingWithGlobalSet", out _)) + if (_builtApplication.DataSources.Count > 0) { - app.UseRouting(); + // If this is set, someone called UseRouting() when a global route builder was already set + if (!_builtApplication.Properties.TryGetValue(EndpointRouteBuilderKey, out var localRouteBuilder)) + { + app.UseRouting(); + } + else + { + // UseEndpoints will be looking for the RouteBuilder so make sure it's set + app.Properties[EndpointRouteBuilderKey] = localRouteBuilder; + } } // Wire the source pipeline to run in the destination pipeline @@ -232,11 +242,13 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui // Remove the route builder to clean up the properties, we're done adding routes to the pipeline app.Properties.Remove(WebApplication.GlobalEndpointRouteBuilderKey); + app.Properties.Remove(EndpointRouteBuilderKey); + _builtApplication.Properties.Remove(EndpointRouteBuilderKey); // reset route builder if it existed, this is needed for StartupFilters if (priorRouteBuilder is not null) { - app.Properties["__EndpointRouteBuilder"] = priorRouteBuilder; + app.Properties[EndpointRouteBuilderKey] = priorRouteBuilder; } } diff --git a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs index ad44706c876b..aa416d94c3ad 100644 --- a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs +++ b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs @@ -770,9 +770,8 @@ public async Task WebApplication_CanCallUseRoutingWithoutUseEndpoints() Assert.Equal("new", await oldResult.Content.ReadAsStringAsync()); } - // REVIEW: David doesn't like this behavior :) [Fact] - public async Task WebApplication_CanCallUseEndpointsWithoutUseRouting() + public async Task WebApplication_CanCallUseEndpointsWithoutUseRoutingFails() { var builder = WebApplication.CreateBuilder(); builder.WebHost.UseTestServer(); @@ -780,22 +779,8 @@ public async Task WebApplication_CanCallUseEndpointsWithoutUseRouting() app.MapGet("/1", () => "1"); - app.UseEndpoints(endpoints => { }); - - await app.StartAsync(); - - var endpointDataSource = app.Services.GetRequiredService(); - - var newEndpoint = Assert.Single(endpointDataSource.Endpoints); - var newRouteEndpoint = Assert.IsType(newEndpoint); - Assert.Equal("/1", newRouteEndpoint.RoutePattern.RawText); - - var client = app.GetTestClient(); - - var oldResult = await client.GetAsync("http://localhost/1"); - oldResult.EnsureSuccessStatusCode(); - - Assert.Equal("1", await oldResult.Content.ReadAsStringAsync()); + var ex = Assert.Throws(() => app.UseEndpoints(endpoints => { })); + Assert.Contains("UseRouting", ex.Message); } [Fact] @@ -1176,6 +1161,33 @@ public async Task BranchingPipelineHasOwnRoutes() Assert.Equal("Four", chosenRoute); } + [Fact] + public async Task PropertiesPreservedFromInnerApplication() + { + var builder = WebApplication.CreateBuilder(); + builder.Services.AddSingleton(); + await using var app = builder.Build(); + + ((IApplicationBuilder)app).Properties["didsomething"] = true; + + app.Start(); + } + + class PropertyFilter : IStartupFilter + { + public Action Configure(Action next) + { + return app => + { + next(app); + + // This should be true + var val = app.Properties["didsomething"]; + Assert.True((bool)val); + }; + } + } + private class Service : IService { } private interface IService { } diff --git a/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs b/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs index a56e000226d8..8bde42284614 100644 --- a/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/EndpointRoutingApplicationBuilderExtensions.cs @@ -50,7 +50,7 @@ public static IApplicationBuilder UseRouting(this IApplicationBuilder builder) { endpointRouteBuilder = (IEndpointRouteBuilder)obj!; // Let interested parties know if UseRouting() was called while a global route builder was set - builder.Properties["__UseRoutingWithGlobalSet"] = true; + builder.Properties[EndpointRouteBuilder] = endpointRouteBuilder; } else { @@ -134,7 +134,7 @@ private static void VerifyRoutingServicesAreRegistered(IApplicationBuilder app) private static void VerifyEndpointRoutingMiddlewareIsRegistered(IApplicationBuilder app, out IEndpointRouteBuilder endpointRouteBuilder) { - if (!app.Properties.TryGetValue(EndpointRouteBuilder, out var obj) && !app.Properties.TryGetValue(GlobalEndpointRouteBuilderKey, out obj)) + if (!app.Properties.TryGetValue(EndpointRouteBuilder, out var obj)) { var message = $"{nameof(EndpointRoutingMiddleware)} matches endpoints setup by {nameof(EndpointMiddleware)} and so must be added to the request " + diff --git a/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs index ebcd0d5a198f..ec9c88e9e107 100644 --- a/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/EndpointRoutingApplicationBuilderExtensionsTest.cs @@ -311,8 +311,6 @@ public void UseEndpoints_WithGlobalEndpointRouteBuilderHasRoutes() app.Properties.Add("__GlobalEndpointRouteBuilder", routeBuilder); app.UseRouting(); - Assert.False(app.Properties.TryGetValue("__EndpointRouteBuilder", out _)); - app.UseEndpoints(endpoints => { endpoints.Map("/1", d => Task.CompletedTask).WithDisplayName("Test endpoint 1"); @@ -329,7 +327,7 @@ public void UseEndpoints_WithGlobalEndpointRouteBuilderHasRoutes() } [Fact] - public void UseRouting_DoesNotSetEndpointRouteBuilder_IfGlobalOneExists() + public void UseRouting_SetsEndpointRouteBuilder_IfGlobalOneExists() { // Arrange var services = CreateServices(); @@ -340,8 +338,9 @@ public void UseRouting_DoesNotSetEndpointRouteBuilder_IfGlobalOneExists() app.Properties.Add("__GlobalEndpointRouteBuilder", routeBuilder); app.UseRouting(); - Assert.False(app.Properties.TryGetValue("__EndpointRouteBuilder", out _)); - Assert.True(app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out _)); + Assert.True(app.Properties.TryGetValue("__EndpointRouteBuilder", out var local)); + Assert.True(app.Properties.TryGetValue("__GlobalEndpointRouteBuilder", out var global)); + Assert.Same(local, global); } private IServiceProvider CreateServices() From 925682c479355670acb4bbaccfc39ffc169d4305 Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 17 Aug 2021 10:44:42 -0700 Subject: [PATCH 10/11] fix --- src/DefaultBuilder/src/WebApplicationBuilder.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DefaultBuilder/src/WebApplicationBuilder.cs b/src/DefaultBuilder/src/WebApplicationBuilder.cs index f3fc939621be..793d12e33343 100644 --- a/src/DefaultBuilder/src/WebApplicationBuilder.cs +++ b/src/DefaultBuilder/src/WebApplicationBuilder.cs @@ -19,7 +19,7 @@ public sealed class WebApplicationBuilder private readonly HostBuilder _hostBuilder = new(); private readonly BootstrapHostBuilder _bootstrapHostBuilder; private readonly WebApplicationServiceCollection _services = new(); - private static string EndpointRouteBuilderKey = "__EndpointRouteBuilder"; + private const string EndpointRouteBuilderKey = "__EndpointRouteBuilder"; private WebApplication? _builtApplication; From 67f0ba44cdf5053f3ab036cfd239076a35e7ec11 Mon Sep 17 00:00:00 2001 From: Brennan Date: Tue, 17 Aug 2021 11:25:41 -0700 Subject: [PATCH 11/11] fin --- src/DefaultBuilder/src/WebApplicationBuilder.cs | 4 +--- .../WebApplicationTests.cs | 15 --------------- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/src/DefaultBuilder/src/WebApplicationBuilder.cs b/src/DefaultBuilder/src/WebApplicationBuilder.cs index 793d12e33343..07b17731f2e4 100644 --- a/src/DefaultBuilder/src/WebApplicationBuilder.cs +++ b/src/DefaultBuilder/src/WebApplicationBuilder.cs @@ -189,7 +189,7 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui // lets remove the property and reset it at the end so we don't mess with the routes in the filter if (app.Properties.TryGetValue(EndpointRouteBuilderKey, out var priorRouteBuilder)) { - app.Properties.Remove("__EndpointRouteBuilder"); + app.Properties.Remove(EndpointRouteBuilderKey); } if (context.HostingEnvironment.IsDevelopment()) @@ -242,8 +242,6 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui // Remove the route builder to clean up the properties, we're done adding routes to the pipeline app.Properties.Remove(WebApplication.GlobalEndpointRouteBuilderKey); - app.Properties.Remove(EndpointRouteBuilderKey); - _builtApplication.Properties.Remove(EndpointRouteBuilderKey); // reset route builder if it existed, this is needed for StartupFilters if (priorRouteBuilder is not null) diff --git a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs index aa416d94c3ad..a3fea68fcc1d 100644 --- a/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs +++ b/src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs @@ -1057,21 +1057,6 @@ public async Task RoutesAddedToCorrectMatcher() Assert.Equal("One", chosenRoute); } - [Fact] - public async Task AppPropertiesDoNotContainARouteBuilder() - { - var builder = WebApplication.CreateBuilder(); - await using var app = builder.Build(); - - app.UseRouting(); - - app.UseEndpoints(endpoints => { }); - - app.Start(); - - Assert.False(app.Properties.TryGetValue("__EndpointRouteBuilder", out _)); - } - [Fact] public async Task WebApplication_CallsUseRoutingAndUseEndpoints() {