diff --git a/src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs b/src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs index 24b9b3df6c885dfd72fc1314e0072abf8e57aa77..c99ae45d886c5526f1b054873d799193709f317e 100644 --- a/src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/RoutingEndpointConventionBuilderExtensions.cs @@ -130,7 +130,7 @@ namespace Microsoft.AspNetCore.Builder /// <returns>The <see cref="IEndpointConventionBuilder"/>.</returns> public static TBuilder WithName<TBuilder>(this TBuilder builder, string endpointName) where TBuilder : IEndpointConventionBuilder { - builder.WithMetadata(new EndpointNameAttribute(endpointName), new RouteNameMetadata(endpointName)); + builder.WithMetadata(new EndpointNameMetadata(endpointName), new RouteNameMetadata(endpointName)); return builder; } diff --git a/src/Http/Routing/src/DefaultEndpointConventionBuilder.cs b/src/Http/Routing/src/DefaultEndpointConventionBuilder.cs index f052163592666d7dc34884653405adb0e023283c..992e23a551d3597d711a59ac2698504e1521a185 100644 --- a/src/Http/Routing/src/DefaultEndpointConventionBuilder.cs +++ b/src/Http/Routing/src/DefaultEndpointConventionBuilder.cs @@ -12,24 +12,37 @@ namespace Microsoft.AspNetCore.Routing { internal EndpointBuilder EndpointBuilder { get; } - private readonly List<Action<EndpointBuilder>> _conventions; + private List<Action<EndpointBuilder>>? _conventions; public DefaultEndpointConventionBuilder(EndpointBuilder endpointBuilder) { EndpointBuilder = endpointBuilder; - _conventions = new List<Action<EndpointBuilder>>(); + _conventions = new(); } public void Add(Action<EndpointBuilder> convention) { - _conventions.Add(convention); + var conventions = _conventions; + + if (conventions is null) + { + throw new InvalidOperationException("Conventions cannot be added after building the endpoint"); + } + + conventions.Add(convention); } public Endpoint Build() { - foreach (var convention in _conventions) + // Only apply the conventions once + var conventions = Interlocked.Exchange(ref _conventions, null); + + if (conventions is not null) { - convention(EndpointBuilder); + foreach (var convention in conventions) + { + convention(EndpointBuilder); + } } return EndpointBuilder.Build(); diff --git a/src/Http/Routing/test/UnitTests/Builder/DelegateEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/DelegateEndpointRouteBuilderExtensionsTest.cs index 49e92ad1f0e0bf5cc3dba41fc0a6cfd1733903d8..8b0cb77500fc74ff7f40fc133719853b6fba1b25 100644 --- a/src/Http/Routing/test/UnitTests/Builder/DelegateEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/DelegateEndpointRouteBuilderExtensionsTest.cs @@ -24,28 +24,32 @@ namespace Microsoft.AspNetCore.Builder return Assert.IsType<RouteEndpointBuilder>(Assert.Single(GetBuilderEndpointDataSource(endpointRouteBuilder).EndpointBuilders)); } - public static object[][] MapMethods + public static object?[]?[] MapMethods { get { - void MapGet(IEndpointRouteBuilder routes, string template, Delegate action) => + IEndpointConventionBuilder MapGet(IEndpointRouteBuilder routes, string template, Delegate action) => routes.MapGet(template, action); - void MapPost(IEndpointRouteBuilder routes, string template, Delegate action) => + IEndpointConventionBuilder MapPost(IEndpointRouteBuilder routes, string template, Delegate action) => routes.MapPost(template, action); - void MapPut(IEndpointRouteBuilder routes, string template, Delegate action) => + IEndpointConventionBuilder MapPut(IEndpointRouteBuilder routes, string template, Delegate action) => routes.MapPut(template, action); - void MapDelete(IEndpointRouteBuilder routes, string template, Delegate action) => + IEndpointConventionBuilder MapDelete(IEndpointRouteBuilder routes, string template, Delegate action) => routes.MapDelete(template, action); - return new object[][] + IEndpointConventionBuilder Map(IEndpointRouteBuilder routes, string template, Delegate action) => + routes.Map(template, action); + + return new object?[]?[] { - new object[] { (Action<IEndpointRouteBuilder, string, Delegate>)MapGet, "GET" }, - new object[] { (Action<IEndpointRouteBuilder, string, Delegate>)MapPost, "POST" }, - new object[] { (Action<IEndpointRouteBuilder, string, Delegate>)MapPut, "PUT" }, - new object[] { (Action<IEndpointRouteBuilder, string, Delegate>)MapDelete, "DELETE" }, + new object?[] { (Func<IEndpointRouteBuilder, string, Delegate, IEndpointConventionBuilder>)MapGet, "GET" }, + new object?[] { (Func<IEndpointRouteBuilder, string, Delegate, IEndpointConventionBuilder>)MapPost, "POST" }, + new object?[] { (Func<IEndpointRouteBuilder, string, Delegate, IEndpointConventionBuilder>)MapPut, "PUT" }, + new object?[] { (Func<IEndpointRouteBuilder, string, Delegate, IEndpointConventionBuilder>)MapDelete, "DELETE" }, + new object?[] { (Func<IEndpointRouteBuilder, string, Delegate, IEndpointConventionBuilder>)Map, null }, }; } } @@ -178,7 +182,61 @@ namespace Microsoft.AspNetCore.Builder [Theory] [MemberData(nameof(MapMethods))] - public async Task MapVerbWithExplicitRouteParameterIsCaseInsensitive(Action<IEndpointRouteBuilder, string, Delegate> map, string expectedMethod) + public void MapVerbDoesNotDuplicateMetadata(Func<IEndpointRouteBuilder, string, Delegate, IEndpointConventionBuilder> map, string expectedMethod) + { + var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvider())); + + map(builder, "/{ID}", () => { }).WithName("Foo"); + + var dataSource = GetBuilderEndpointDataSource(builder); + + // Access endpoints a couple of times to make sure it gets built + _ = dataSource.Endpoints; + _ = dataSource.Endpoints; + _ = dataSource.Endpoints; + + var endpoint = Assert.Single(dataSource.Endpoints); + + var endpointNameMetadata = Assert.Single(endpoint.Metadata.GetOrderedMetadata<IEndpointNameMetadata>()); + var routeNameMetadata = Assert.Single(endpoint.Metadata.GetOrderedMetadata<IRouteNameMetadata>()); + Assert.Equal("Foo", endpointNameMetadata.EndpointName); + Assert.Equal("Foo", routeNameMetadata.RouteName); + + if (expectedMethod is not null) + { + var methodMetadata = endpoint.Metadata.GetMetadata<IHttpMethodMetadata>(); + Assert.NotNull(methodMetadata); + var method = Assert.Single(methodMetadata!.HttpMethods); + Assert.Equal(expectedMethod, method); + } + } + + [Theory] + [MemberData(nameof(MapMethods))] + public void AddingMetadataAfterBuildingEndpointThrows(Func<IEndpointRouteBuilder, string, Delegate, IEndpointConventionBuilder> map, string expectedMethod) + { + var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvider())); + + var endpointBuilder = map(builder, "/{ID}", () => { }); + + var dataSource = GetBuilderEndpointDataSource(builder); + + var endpoint = Assert.Single(dataSource.Endpoints); + + if (expectedMethod is not null) + { + var methodMetadata = endpoint.Metadata.GetMetadata<IHttpMethodMetadata>(); + Assert.NotNull(methodMetadata); + var method = Assert.Single(methodMetadata!.HttpMethods); + Assert.Equal(expectedMethod, method); + } + + Assert.Throws<InvalidOperationException>(() => endpointBuilder.WithMetadata(new RouteNameMetadata("Foo"))); + } + + [Theory] + [MemberData(nameof(MapMethods))] + public async Task MapVerbWithExplicitRouteParameterIsCaseInsensitive(Func<IEndpointRouteBuilder, string, Delegate, IEndpointConventionBuilder> map, string expectedMethod) { var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvider())); @@ -194,13 +252,19 @@ namespace Microsoft.AspNetCore.Builder // Trigger Endpoint build by calling getter. var endpoint = Assert.Single(dataSource.Endpoints); - var methodMetadata = endpoint.Metadata.GetMetadata<IHttpMethodMetadata>(); - Assert.NotNull(methodMetadata); - var method = Assert.Single(methodMetadata!.HttpMethods); - Assert.Equal(expectedMethod, method); + if (expectedMethod is not null) + { + var methodMetadata = endpoint.Metadata.GetMetadata<IHttpMethodMetadata>(); + Assert.NotNull(methodMetadata); + var method = Assert.Single(methodMetadata!.HttpMethods); + Assert.Equal(expectedMethod, method); + } var routeEndpointBuilder = GetRouteEndpointBuilder(builder); - Assert.Equal($"HTTP: {expectedMethod} /{{ID}}", routeEndpointBuilder.DisplayName); + if (expectedMethod is not null) + { + Assert.Equal($"HTTP: {expectedMethod} /{{ID}}", routeEndpointBuilder.DisplayName); + } Assert.Equal($"/{{ID}}", routeEndpointBuilder.RoutePattern.RawText); var httpContext = new DefaultHttpContext(); @@ -214,7 +278,7 @@ namespace Microsoft.AspNetCore.Builder [Theory] [MemberData(nameof(MapMethods))] - public async Task MapVerbWithRouteParameterDoesNotFallbackToQuery(Action<IEndpointRouteBuilder, string, Delegate> map, string expectedMethod) + public async Task MapVerbWithRouteParameterDoesNotFallbackToQuery(Func<IEndpointRouteBuilder, string, Delegate, IEndpointConventionBuilder> map, string expectedMethod) { var builder = new DefaultEndpointRouteBuilder(new ApplicationBuilder(new EmptyServiceProvider())); @@ -229,14 +293,19 @@ namespace Microsoft.AspNetCore.Builder var dataSource = GetBuilderEndpointDataSource(builder); // Trigger Endpoint build by calling getter. var endpoint = Assert.Single(dataSource.Endpoints); - - var methodMetadata = endpoint.Metadata.GetMetadata<IHttpMethodMetadata>(); - Assert.NotNull(methodMetadata); - var method = Assert.Single(methodMetadata!.HttpMethods); - Assert.Equal(expectedMethod, method); + if (expectedMethod is not null) + { + var methodMetadata = endpoint.Metadata.GetMetadata<IHttpMethodMetadata>(); + Assert.NotNull(methodMetadata); + var method = Assert.Single(methodMetadata!.HttpMethods); + Assert.Equal(expectedMethod, method); + } var routeEndpointBuilder = GetRouteEndpointBuilder(builder); - Assert.Equal($"HTTP: {expectedMethod} /{{ID}}", routeEndpointBuilder.DisplayName); + if (expectedMethod is not null) + { + Assert.Equal($"HTTP: {expectedMethod} /{{ID}}", routeEndpointBuilder.DisplayName); + } Assert.Equal($"/{{ID}}", routeEndpointBuilder.RoutePattern.RawText); // Assert that we don't fallback to the query string diff --git a/src/Http/Routing/test/UnitTests/Builder/MapEndpointEndpointDataSourceBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/MapEndpointEndpointDataSourceBuilderExtensionsTest.cs index 68a29c2b066eeaecfb46da9cecb224982d07bd34..8d713234a316a1139e217fbde65dfadbac071791 100644 --- a/src/Http/Routing/test/UnitTests/Builder/MapEndpointEndpointDataSourceBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/MapEndpointEndpointDataSourceBuilderExtensionsTest.cs @@ -26,6 +26,36 @@ namespace Microsoft.AspNetCore.Builder return Assert.IsType<RouteEndpointBuilder>(Assert.Single(GetBuilderEndpointDataSource(endpointRouteBuilder).EndpointBuilders)); } + public static object[][] MapMethods + { + get + { + IEndpointConventionBuilder MapGet(IEndpointRouteBuilder routes, string template, RequestDelegate action) => + routes.MapGet(template, action); + + IEndpointConventionBuilder MapPost(IEndpointRouteBuilder routes, string template, RequestDelegate action) => + routes.MapPost(template, action); + + IEndpointConventionBuilder MapPut(IEndpointRouteBuilder routes, string template, RequestDelegate action) => + routes.MapPut(template, action); + + IEndpointConventionBuilder MapDelete(IEndpointRouteBuilder routes, string template, RequestDelegate action) => + routes.MapDelete(template, action); + + IEndpointConventionBuilder Map(IEndpointRouteBuilder routes, string template, RequestDelegate action) => + routes.Map(template, action); + + return new object[][] + { + new object[] { (Func<IEndpointRouteBuilder, string, RequestDelegate, IEndpointConventionBuilder>)MapGet }, + new object[] { (Func<IEndpointRouteBuilder, string, RequestDelegate, IEndpointConventionBuilder>)MapPost }, + new object[] { (Func<IEndpointRouteBuilder, string, RequestDelegate, IEndpointConventionBuilder>)MapPut }, + new object[] { (Func<IEndpointRouteBuilder, string, RequestDelegate, IEndpointConventionBuilder>)MapDelete }, + new object[] { (Func<IEndpointRouteBuilder, string, RequestDelegate, IEndpointConventionBuilder>)Map }, + }; + } + } + [Fact] public void MapEndpoint_StringPattern_BuildsEndpoint() { @@ -69,7 +99,7 @@ namespace Microsoft.AspNetCore.Builder var builder = new DefaultEndpointRouteBuilder(Mock.Of<IApplicationBuilder>()); // Act - var endpointBuilder = builder.Map(RoutePatternFactory.Parse("/"), Handle); + var endpointBuilder = builder.Map(RoutePatternFactory.Parse("/"), Handle); // Assert var endpointBuilder1 = GetRouteEndpointBuilder(builder); @@ -123,6 +153,49 @@ namespace Microsoft.AspNetCore.Builder } } + [Theory] + [MemberData(nameof(MapMethods))] + public void Map_EndpointMetadataNotDuplicated(Func<IEndpointRouteBuilder, string, RequestDelegate, IEndpointConventionBuilder> map) + { + // Arrange + var builder = new DefaultEndpointRouteBuilder(Mock.Of<IApplicationBuilder>()); + + // Act + var endpointBuilder = map(builder, "/", context => Task.CompletedTask).WithMetadata(new EndpointNameMetadata("MapMe")); + + // Assert + var ds = GetBuilderEndpointDataSource(builder); + + _ = ds.Endpoints; + _ = ds.Endpoints; + _ = ds.Endpoints; + + Assert.Single(ds.Endpoints); + var endpoint = ds.Endpoints.Single(); + + Assert.Single(endpoint.Metadata.GetOrderedMetadata<IEndpointNameMetadata>()); + } + + [Theory] + [MemberData(nameof(MapMethods))] + public void AddingMetadataAfterBuildingEndpointThrows(Func<IEndpointRouteBuilder, string, RequestDelegate, IEndpointConventionBuilder> map) + { + // Arrange + var builder = new DefaultEndpointRouteBuilder(Mock.Of<IApplicationBuilder>()); + + // Act + var endpointBuilder = map(builder, "/", context => Task.CompletedTask).WithMetadata(new EndpointNameMetadata("MapMe")); + + // Assert + var ds = GetBuilderEndpointDataSource(builder); + + var endpoint = Assert.Single(ds.Endpoints); + + Assert.Single(endpoint.Metadata.GetOrderedMetadata<IEndpointNameMetadata>()); + + Assert.Throws<InvalidOperationException>(() => endpointBuilder.WithMetadata(new RouteNameMetadata("Foo"))); + } + [Attribute1] [Attribute2] private static Task Handle(HttpContext context) => Task.CompletedTask;