From 5207cad89855eba812a8a33232b3ba3ae7b6c211 Mon Sep 17 00:00:00 2001 From: Stephen Halter Date: Mon, 1 Mar 2021 23:49:08 -0800 Subject: [PATCH] Add new "MapAction" overloads (#30556) * Add new "MapAction" overloads * Create RouteEndpointBuilder directly * fix typo: my -> by * is { } -> is not null --- .../MapActionEndpointConventionBuilder.cs | 5 + ...MapActionEndpointRouteBuilderExtensions.cs | 205 ++++++++++++++++++ src/Http/Routing/src/PublicAPI.Unshipped.txt | 7 + .../test/FunctionalTests/MapActionTest.cs | 41 ++++ ...ctionEndpointRouteBuilderExtensionsTest.cs | 154 ++++++++++++- .../MapActionExpressionTreeBuilderTest.cs | 4 +- .../CustomRouteMetadataAttribute.cs | 4 +- 7 files changed, 414 insertions(+), 6 deletions(-) diff --git a/src/Http/Routing/src/Builder/MapActionEndpointConventionBuilder.cs b/src/Http/Routing/src/Builder/MapActionEndpointConventionBuilder.cs index 4491b0305739..00697b4d87da 100644 --- a/src/Http/Routing/src/Builder/MapActionEndpointConventionBuilder.cs +++ b/src/Http/Routing/src/Builder/MapActionEndpointConventionBuilder.cs @@ -13,6 +13,11 @@ public sealed class MapActionEndpointConventionBuilder : IEndpointConventionBuil { private readonly List _endpointConventionBuilders; + internal MapActionEndpointConventionBuilder(IEndpointConventionBuilder endpointConventionBuilder) + { + _endpointConventionBuilders = new List() { endpointConventionBuilder }; + } + internal MapActionEndpointConventionBuilder(List endpointConventionBuilders) { _endpointConventionBuilders = endpointConventionBuilders; diff --git a/src/Http/Routing/src/Builder/MapActionEndpointRouteBuilderExtensions.cs b/src/Http/Routing/src/Builder/MapActionEndpointRouteBuilderExtensions.cs index e132e673ea32..f8dbad11be27 100644 --- a/src/Http/Routing/src/Builder/MapActionEndpointRouteBuilderExtensions.cs +++ b/src/Http/Routing/src/Builder/MapActionEndpointRouteBuilderExtensions.cs @@ -7,6 +7,7 @@ using System.Reflection; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Internal; +using Microsoft.AspNetCore.Routing.Patterns; namespace Microsoft.AspNetCore.Builder { @@ -15,6 +16,12 @@ namespace Microsoft.AspNetCore.Builder /// public static class MapActionEndpointRouteBuilderExtensions { + // Avoid creating a new array every call + private static readonly string[] GetVerb = new[] { "GET" }; + private static readonly string[] PostVerb = new[] { "POST" }; + private static readonly string[] PutVerb = new[] { "PUT" }; + private static readonly string[] DeleteVerb = new[] { "DELETE" }; + /// /// Adds a to the that matches the pattern specified via attributes. /// @@ -76,5 +83,203 @@ public static MapActionEndpointConventionBuilder MapAction( return new MapActionEndpointConventionBuilder(conventionBuilders); } + + /// + /// Adds a to the that matches HTTP GET requests + /// for the specified pattern. + /// + /// The to add the route to. + /// The route pattern. + /// The delegate executed when the endpoint is matched. + /// A that can be used to further customize the endpoint. + public static MapActionEndpointConventionBuilder MapGet( + this IEndpointRouteBuilder endpoints, + string pattern, + Delegate action) + { + return MapMethods(endpoints, pattern, GetVerb, action); + } + + /// + /// Adds a to the that matches HTTP POST requests + /// for the specified pattern. + /// + /// The to add the route to. + /// The route pattern. + /// The delegate executed when the endpoint is matched. + /// A that can be used to further customize the endpoint. + public static MapActionEndpointConventionBuilder MapPost( + this IEndpointRouteBuilder endpoints, + string pattern, + Delegate action) + { + return MapMethods(endpoints, pattern, PostVerb, action); + } + + /// + /// Adds a to the that matches HTTP PUT requests + /// for the specified pattern. + /// + /// The to add the route to. + /// The route pattern. + /// The delegate executed when the endpoint is matched. + /// A that canaction be used to further customize the endpoint. + public static MapActionEndpointConventionBuilder MapPut( + this IEndpointRouteBuilder endpoints, + string pattern, + Delegate action) + { + return MapMethods(endpoints, pattern, PutVerb, action); + } + + /// + /// Adds a to the that matches HTTP DELETE requests + /// for the specified pattern. + /// + /// The to add the route to. + /// The route pattern. + /// The delegate executed when the endpoint is matched. + /// A that can be used to further customize the endpoint. + public static MapActionEndpointConventionBuilder MapDelete( + this IEndpointRouteBuilder endpoints, + string pattern, + Delegate action) + { + return MapMethods(endpoints, pattern, DeleteVerb, action); + } + + /// + /// Adds a to the that matches HTTP requests + /// for the specified HTTP methods and pattern. + /// + /// The to add the route to. + /// The route pattern. + /// The delegate executed when the endpoint is matched. + /// HTTP methods that the endpoint will match. + /// A that can be used to further customize the endpoint. + public static MapActionEndpointConventionBuilder MapMethods( + this IEndpointRouteBuilder endpoints, + string pattern, + IEnumerable httpMethods, + Delegate action) + { + if (httpMethods is null) + { + throw new ArgumentNullException(nameof(httpMethods)); + } + + var displayName = $"{pattern} HTTP: {string.Join(", ", httpMethods)}"; + var builder = endpoints.Map(RoutePatternFactory.Parse(pattern), action, displayName); + builder.WithMetadata(new HttpMethodMetadata(httpMethods)); + return builder; + } + + /// + /// Adds a to the that matches HTTP requests + /// for the specified pattern. + /// + /// The to add the route to. + /// The route pattern. + /// The delegate executed when the endpoint is matched. + /// A that can be used to further customize the endpoint. + public static MapActionEndpointConventionBuilder Map( + this IEndpointRouteBuilder endpoints, + string pattern, + Delegate action) + { + return Map(endpoints, RoutePatternFactory.Parse(pattern), action); + } + + /// + /// Adds a to the that matches HTTP requests + /// for the specified pattern. + /// + /// The to add the route to. + /// The route pattern. + /// The delegate executed when the endpoint is matched. + /// A that can be used to further customize the endpoint. + public static MapActionEndpointConventionBuilder Map( + this IEndpointRouteBuilder endpoints, + RoutePattern pattern, + Delegate action) + { + return Map(endpoints, pattern, action, displayName: null); + } + + private static MapActionEndpointConventionBuilder Map( + this IEndpointRouteBuilder endpoints, + RoutePattern pattern, + Delegate action, + string? displayName) + { + if (endpoints is null) + { + throw new ArgumentNullException(nameof(endpoints)); + } + + if (pattern is null) + { + throw new ArgumentNullException(nameof(pattern)); + } + + if (action is null) + { + throw new ArgumentNullException(nameof(action)); + } + + const int defaultOrder = 0; + + var builder = new RouteEndpointBuilder( + MapActionExpressionTreeBuilder.BuildRequestDelegate(action), + pattern, + defaultOrder) + { + DisplayName = pattern.RawText ?? pattern.DebuggerToString(), + }; + + // Add delegate attributes as metadata + var attributes = action.Method.GetCustomAttributes(); + string? routeName = null; + int? routeOrder = null; + + // This can be null if the delegate is a dynamic method or compiled from an expression tree + if (attributes is not null) + { + foreach (var attribute in attributes) + { + if (attribute is IRoutePatternMetadata patternMetadata && patternMetadata.RoutePattern is not null) + { + throw new InvalidOperationException($"'{attribute.GetType()}' implements {nameof(IRoutePatternMetadata)} which is not supported by this method."); + } + if (attribute is IHttpMethodMetadata methodMetadata && methodMetadata.HttpMethods.Any()) + { + throw new InvalidOperationException($"'{attribute.GetType()}' implements {nameof(IHttpMethodMetadata)} which is not supported by this method."); + } + + if (attribute is IRouteNameMetadata nameMetadata && nameMetadata.RouteName is string name) + { + routeName = name; + } + if (attribute is IRouteOrderMetadata orderMetadata && orderMetadata.RouteOrder is int order) + { + routeOrder = order; + } + + builder.Metadata.Add(attribute); + } + } + + builder.DisplayName = routeName ?? displayName ?? builder.DisplayName; + builder.Order = routeOrder ?? defaultOrder; + + var dataSource = endpoints.DataSources.OfType().FirstOrDefault(); + if (dataSource is null) + { + dataSource = new ModelEndpointDataSource(); + endpoints.DataSources.Add(dataSource); + } + + return new MapActionEndpointConventionBuilder(dataSource.AddEndpointBuilder(builder)); + } } } diff --git a/src/Http/Routing/src/PublicAPI.Unshipped.txt b/src/Http/Routing/src/PublicAPI.Unshipped.txt index adacdfd54759..a4c37efae127 100644 --- a/src/Http/Routing/src/PublicAPI.Unshipped.txt +++ b/src/Http/Routing/src/PublicAPI.Unshipped.txt @@ -18,4 +18,11 @@ Microsoft.AspNetCore.Routing.IRoutePatternMetadata Microsoft.AspNetCore.Routing.IRoutePatternMetadata.RoutePattern.get -> string? Microsoft.AspNetCore.Routing.RouteNameMetadata.RouteName.get -> string? Microsoft.AspNetCore.Routing.RouteNameMetadata.RouteNameMetadata(string? routeName) -> void +static Microsoft.AspNetCore.Builder.MapActionEndpointRouteBuilderExtensions.Map(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, Microsoft.AspNetCore.Routing.Patterns.RoutePattern! pattern, System.Delegate! action) -> Microsoft.AspNetCore.Builder.MapActionEndpointConventionBuilder! +static Microsoft.AspNetCore.Builder.MapActionEndpointRouteBuilderExtensions.Map(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, string! pattern, System.Delegate! action) -> Microsoft.AspNetCore.Builder.MapActionEndpointConventionBuilder! static Microsoft.AspNetCore.Builder.MapActionEndpointRouteBuilderExtensions.MapAction(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, System.Delegate! action) -> Microsoft.AspNetCore.Builder.MapActionEndpointConventionBuilder! +static Microsoft.AspNetCore.Builder.MapActionEndpointRouteBuilderExtensions.MapDelete(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, string! pattern, System.Delegate! action) -> Microsoft.AspNetCore.Builder.MapActionEndpointConventionBuilder! +static Microsoft.AspNetCore.Builder.MapActionEndpointRouteBuilderExtensions.MapGet(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, string! pattern, System.Delegate! action) -> Microsoft.AspNetCore.Builder.MapActionEndpointConventionBuilder! +static Microsoft.AspNetCore.Builder.MapActionEndpointRouteBuilderExtensions.MapMethods(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, string! pattern, System.Collections.Generic.IEnumerable! httpMethods, System.Delegate! action) -> Microsoft.AspNetCore.Builder.MapActionEndpointConventionBuilder! +static Microsoft.AspNetCore.Builder.MapActionEndpointRouteBuilderExtensions.MapPost(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, string! pattern, System.Delegate! action) -> Microsoft.AspNetCore.Builder.MapActionEndpointConventionBuilder! +static Microsoft.AspNetCore.Builder.MapActionEndpointRouteBuilderExtensions.MapPut(this Microsoft.AspNetCore.Routing.IEndpointRouteBuilder! endpoints, string! pattern, System.Delegate! action) -> Microsoft.AspNetCore.Builder.MapActionEndpointConventionBuilder! diff --git a/src/Http/Routing/test/FunctionalTests/MapActionTest.cs b/src/Http/Routing/test/FunctionalTests/MapActionTest.cs index f48d45ee1b3d..3723b3ca5b02 100644 --- a/src/Http/Routing/test/FunctionalTests/MapActionTest.cs +++ b/src/Http/Routing/test/FunctionalTests/MapActionTest.cs @@ -61,6 +61,47 @@ public async Task MapAction_FromBodyWorksWithJsonPayload() Assert.Equal(42, echoedTodo?.Id); } + [Fact] + public async Task MapPost_FromBodyWorksWithJsonPayload() + { + Todo EchoTodo([FromRoute] int id, [FromBody] Todo todo) => todo with { Id = id }; + + using var host = new HostBuilder() + .ConfigureWebHost(webHostBuilder => + { + webHostBuilder + .Configure(app => + { + app.UseRouting(); + app.UseEndpoints(b => b.MapPost("/EchoTodo/{id}", (Func)EchoTodo)); + }) + .UseTestServer(); + }) + .ConfigureServices(services => + { + services.AddRouting(); + }) + .Build(); + + using var server = host.GetTestServer(); + await host.StartAsync(); + var client = server.CreateClient(); + + var todo = new Todo + { + Name = "Write tests!" + }; + + var response = await client.PostAsJsonAsync("/EchoTodo/42", todo); + response.EnsureSuccessStatusCode(); + + var echoedTodo = await response.Content.ReadFromJsonAsync(); + + Assert.NotNull(echoedTodo); + Assert.Equal(todo.Name, echoedTodo?.Name); + Assert.Equal(42, echoedTodo?.Id); + } + private record Todo { public int Id { get; set; } diff --git a/src/Http/Routing/test/UnitTests/Builder/MapActionEndpointRouteBuilderExtensionsTest.cs b/src/Http/Routing/test/UnitTests/Builder/MapActionEndpointRouteBuilderExtensionsTest.cs index 7ead738d18d0..ea68f2d96e31 100644 --- a/src/Http/Routing/test/UnitTests/Builder/MapActionEndpointRouteBuilderExtensionsTest.cs +++ b/src/Http/Routing/test/UnitTests/Builder/MapActionEndpointRouteBuilderExtensionsTest.cs @@ -4,8 +4,10 @@ #nullable enable using System; +using System.Collections.Generic; using System.Linq; using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.AspNetCore.Routing.TestObjects; using Moq; using Xunit; @@ -53,7 +55,8 @@ public void MapAction_BuildsEndpointWithRouteNameAndOrder() const string customName = "Custom Name"; const int customOrder = 1337; - [CustomRouteMetadata(Name = customName, Order = customOrder)] + // This is tested separately because MapAction requires a Pattern and the other overloads forbit it. + [CustomRouteMetadata(Pattern = "/", Name = customName, Order = customOrder)] void TestAction() { }; var builder = new DefaultEndpointRouteBuilder(Mock.Of()); @@ -67,5 +70,154 @@ public void MapAction_BuildsEndpointWithRouteNameAndOrder() Assert.Equal(customName, routeEndpointBuilder.DisplayName); Assert.Equal(customOrder, routeEndpointBuilder.Order); } + + [Theory] + [MemberData(nameof(MapActionMethods))] + public void MapOverloads_BuildsEndpointWithRouteNameAndOrder(Action mapOverload) + { + const string customName = "Custom Name"; + const int customOrder = 1337; + + [CustomRouteMetadata(Name = customName, Order = customOrder)] + void TestAction() { }; + + var builder = new DefaultEndpointRouteBuilder(Mock.Of()); + mapOverload(builder, (Action)TestAction); + + var dataSource = GetBuilderEndpointDataSource(builder); + // Trigger Endpoint build by calling getter. + var endpoint = Assert.Single(dataSource.Endpoints); + + var routeEndpointBuilder = GetRouteEndpointBuilder(builder); + Assert.Equal(customName, routeEndpointBuilder.DisplayName); + Assert.Equal(customOrder, routeEndpointBuilder.Order); + } + + [Fact] + public void MapGet_BuildsEndpointWithRouteNameAndOrder() + { + var builder = new DefaultEndpointRouteBuilder(Mock.Of()); + _ = builder.MapGet("/", (Action)(() => { })); + + var dataSource = GetBuilderEndpointDataSource(builder); + // Trigger Endpoint build by calling getter. + var endpoint = Assert.Single(dataSource.Endpoints); + + var methodMetadata = endpoint.Metadata.GetMetadata(); + Assert.NotNull(methodMetadata); + var method = Assert.Single(methodMetadata!.HttpMethods); + Assert.Equal("GET", method); + } + + [Fact] + public void MapPost_BuildsEndpointWithRouteNameAndOrder() + { + var builder = new DefaultEndpointRouteBuilder(Mock.Of()); + _ = builder.MapPost("/", (Action)(() => { })); + + var dataSource = GetBuilderEndpointDataSource(builder); + // Trigger Endpoint build by calling getter. + var endpoint = Assert.Single(dataSource.Endpoints); + + var methodMetadata = endpoint.Metadata.GetMetadata(); + Assert.NotNull(methodMetadata); + var method = Assert.Single(methodMetadata!.HttpMethods); + Assert.Equal("POST", method); + } + + [Fact] + public void MapPut_BuildsEndpointWithRouteNameAndOrder() + { + var builder = new DefaultEndpointRouteBuilder(Mock.Of()); + _ = builder.MapPut("/", (Action)(() => { })); + + var dataSource = GetBuilderEndpointDataSource(builder); + // Trigger Endpoint build by calling getter. + var endpoint = Assert.Single(dataSource.Endpoints); + + var methodMetadata = endpoint.Metadata.GetMetadata(); + Assert.NotNull(methodMetadata); + var method = Assert.Single(methodMetadata!.HttpMethods); + Assert.Equal("PUT", method); + } + + [Fact] + public void MapDelete_BuildsEndpointWithRouteNameAndOrder() + { + var builder = new DefaultEndpointRouteBuilder(Mock.Of()); + _ = builder.MapDelete("/", (Action)(() => { })); + + var dataSource = GetBuilderEndpointDataSource(builder); + // Trigger Endpoint build by calling getter. + var endpoint = Assert.Single(dataSource.Endpoints); + + var methodMetadata = endpoint.Metadata.GetMetadata(); + Assert.NotNull(methodMetadata); + var method = Assert.Single(methodMetadata!.HttpMethods); + Assert.Equal("DELETE", method); + } + + [Theory] + [MemberData(nameof(MapActionMethods))] + public void MapOverloads_RejectActionsWithPatternMetadata(Action mapOverload) + { + [CustomRouteMetadata(Pattern = "/")] + void TestAction() { }; + + var builder = new DefaultEndpointRouteBuilder(Mock.Of()); + var ex = Assert.Throws(() => mapOverload(builder, (Action)TestAction)); + Assert.Contains(nameof(IRoutePatternMetadata), ex.Message); + } + + [Theory] + [MemberData(nameof(MapActionMethods))] + public void MapOverloads_RejectActionsWithMethodMetadata(Action mapOverload) + { + [CustomRouteMetadata(Methods = new[] { "GET" })] + void TestAction() { }; + + var builder = new DefaultEndpointRouteBuilder(Mock.Of()); + var ex = Assert.Throws(() => mapOverload(builder, (Action)TestAction)); + Assert.Contains(nameof(IHttpMethodMetadata), ex.Message); + } + + public static IEnumerable MapActionMethods => new object[][] + { + new object[] + { + (Action)( + (builder, action) => builder.MapGet("/", action)) + }, + new object[] + { + (Action)( + (builder, action) => builder.MapPost("/", action)) + }, + new object[] + { + (Action)( + (builder, action) => builder.MapPut("/", action)) + }, + new object[] + { + (Action)( + (builder, action) => builder.MapDelete("/", action)) + }, + new object[] + { + (Action)( + (builder, action) => builder.MapMethods("/", Array.Empty(), action)) + }, + new object[] + { + (Action)( + (builder, action) => builder.Map("/", action)) + }, + new object[] + { + (Action)( + (builder, action) => builder.Map(RoutePatternFactory.Parse("/"), action)) + }, + }; } } diff --git a/src/Http/Routing/test/UnitTests/Internal/MapActionExpressionTreeBuilderTest.cs b/src/Http/Routing/test/UnitTests/Internal/MapActionExpressionTreeBuilderTest.cs index 543e8a17d808..39372600077f 100644 --- a/src/Http/Routing/test/UnitTests/Internal/MapActionExpressionTreeBuilderTest.cs +++ b/src/Http/Routing/test/UnitTests/Internal/MapActionExpressionTreeBuilderTest.cs @@ -24,7 +24,7 @@ namespace Microsoft.AspNetCore.Routing.Internal { public class MapActionExpressionTreeBuilderTest { - public static IEnumerable NoResult + public static IEnumerable NoResult { get { @@ -113,8 +113,6 @@ ValueTask ValueTaskTestAction(HttpContext httpContext, [FromRoute] int value) return ValueTask.CompletedTask; } - - return new List { new object[] { (Action)TestAction }, diff --git a/src/Http/Routing/test/UnitTests/TestObjects/CustomRouteMetadataAttribute.cs b/src/Http/Routing/test/UnitTests/TestObjects/CustomRouteMetadataAttribute.cs index 4a2b73ec748f..bf14d2ce7a0b 100644 --- a/src/Http/Routing/test/UnitTests/TestObjects/CustomRouteMetadataAttribute.cs +++ b/src/Http/Routing/test/UnitTests/TestObjects/CustomRouteMetadataAttribute.cs @@ -10,13 +10,13 @@ namespace Microsoft.AspNetCore.Routing.TestObjects { internal class CustomRouteMetadataAttribute : Attribute, IRoutePatternMetadata, IHttpMethodMetadata, IRouteNameMetadata, IRouteOrderMetadata { - public string Pattern { get; set; } = "/"; + public string? Pattern { get; set; } public string? Name { get; set; } public int Order { get; set; } = 0; - public string[] Methods { get; set; } = new[] { "GET" }; + public string[] Methods { get; set; } = Array.Empty(); string? IRoutePatternMetadata.RoutePattern => Pattern;