diff --git a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs index 08fa044699..d5be158b25 100644 --- a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs +++ b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs @@ -33,6 +33,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing public void AddEndpoints( List endpoints, + HashSet routeNames, ActionDescriptor action, IReadOnlyList routes, IReadOnlyList> conventions) @@ -42,6 +43,11 @@ namespace Microsoft.AspNetCore.Mvc.Routing throw new ArgumentNullException(nameof(endpoints)); } + if (routeNames == null) + { + throw new ArgumentNullException(nameof(routeNames)); + } + if (action == null) { throw new ArgumentNullException(nameof(action)); @@ -76,6 +82,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing // We suppress link generation for each conventionally routed endpoint. We generate a single endpoint per-route // to handle link generation. var builder = CreateEndpoint( + routeNames, action, updatedRoutePattern, route.RouteName, @@ -103,6 +110,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing } var endpoint = CreateEndpoint( + routeNames, action, updatedRoutePattern, action.AttributeRouteInfo.Name, @@ -118,6 +126,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing public void AddConventionalLinkGenerationRoute( List endpoints, + HashSet routeNames, HashSet keys, ConventionalRouteEntry route, IReadOnlyList> conventions) @@ -178,6 +187,16 @@ namespace Microsoft.AspNetCore.Mvc.Routing builder.Metadata.Add(new RouteNameMetadata(route.RouteName)); } + // See comments on the other usage of EndpointNameMetadata in this class. + // + // The set of cases for a conventional route are much simpler. We don't need to check + // for Endpoint Name already exising here because there's no way to add an attribute to + // a conventional route. + if (route.RouteName != null && routeNames.Add(route.RouteName)) + { + builder.Metadata.Add(new EndpointNameMetadata(route.RouteName)); + } + for (var i = 0; i < conventions.Count; i++) { conventions[i](builder); @@ -244,6 +263,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing } private RouteEndpoint CreateEndpoint( + HashSet routeNames, ActionDescriptor action, RoutePattern routePattern, string routeName, @@ -294,6 +314,27 @@ namespace Microsoft.AspNetCore.Mvc.Routing builder.Metadata.Add(action); + // MVC guarantees that when two of it's endpoints have the same route name they are equivalent. + // + // The case for this looks like: + // + // [HttpGet] + // [HttpPost] + // [Route("/Foo", Name = "Foo")] + // public void DoStuff() { } + // + // However, Endpoint Routing requires Endpoint Names to be unique. + // + // We can use the route name as the endpoint name if it's not set. Note that there's no + // attribute for this today so it's unlikley. Using endpoint name on a + if (routeName != null && + !suppressLinkGeneration && + routeNames.Add(routeName) && + builder.Metadata.OfType().LastOrDefault()?.EndpointName == null) + { + builder.Metadata.Add(new EndpointNameMetadata(routeName)); + } + if (dataTokens != null) { builder.Metadata.Add(new DataTokensMetadata(dataTokens)); diff --git a/src/Mvc/Mvc.Core/src/Routing/ControllerActionEndpointDataSource.cs b/src/Mvc/Mvc.Core/src/Routing/ControllerActionEndpointDataSource.cs index 1dbae56c5c..6f9e4d492f 100644 --- a/src/Mvc/Mvc.Core/src/Routing/ControllerActionEndpointDataSource.cs +++ b/src/Mvc/Mvc.Core/src/Routing/ControllerActionEndpointDataSource.cs @@ -3,14 +3,12 @@ using System; using System.Collections.Generic; -using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Routing; -using Microsoft.AspNetCore.Routing.Patterns; namespace Microsoft.AspNetCore.Mvc.Routing { @@ -67,6 +65,11 @@ namespace Microsoft.AspNetCore.Mvc.Routing var endpoints = new List(); var keys = new HashSet(StringComparer.OrdinalIgnoreCase); + // MVC guarantees that when two of it's endpoints have the same route name they are equivalent. + // + // However, Endpoint Routing requires Endpoint Names to be unique. + var routeNames = new HashSet(StringComparer.OrdinalIgnoreCase); + // For each controller action - add the relevant endpoints. // // 1. If the action is attribute routed, we use that information verbatim @@ -77,7 +80,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing { if (actions[i] is ControllerActionDescriptor action) { - _endpointFactory.AddEndpoints(endpoints, action, _routes, conventions); + _endpointFactory.AddEndpoints(endpoints, routeNames, action, _routes, conventions); if (_routes.Count > 0) { @@ -96,7 +99,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing for (var i = 0; i < _routes.Count; i++) { var route = _routes[i]; - _endpointFactory.AddConventionalLinkGenerationRoute(endpoints, keys, route, conventions); + _endpointFactory.AddConventionalLinkGenerationRoute(endpoints, routeNames, keys, route, conventions); } return endpoints; diff --git a/src/Mvc/Mvc.Core/test/Routing/ActionEndpointFactoryTest.cs b/src/Mvc/Mvc.Core/test/Routing/ActionEndpointFactoryTest.cs index bfa7743f40..a209f710c3 100644 --- a/src/Mvc/Mvc.Core/test/Routing/ActionEndpointFactoryTest.cs +++ b/src/Mvc/Mvc.Core/test/Routing/ActionEndpointFactoryTest.cs @@ -225,6 +225,26 @@ namespace Microsoft.AspNetCore.Mvc.Routing Assert.False(endpoint.RoutePattern.RequiredValues.ContainsKey("page")); } + [Fact] + public void AddEndpoints_AttributeRouted_WithRouteName_EndpointCreated() + { + // Arrange + var values = new { controller = "TestController", action = "TestAction", page = (string)null }; + var action = CreateActionDescriptor(values, "{controller}/{action}/{page}"); + action.AttributeRouteInfo.Name = "Test"; + + // Act + var endpoint = CreateAttributeRoutedEndpoint(action); + + // Assert + Assert.Equal("{controller}/{action}/{page}", endpoint.RoutePattern.RawText); + Assert.Equal("TestController", endpoint.RoutePattern.RequiredValues["controller"]); + Assert.Equal("TestAction", endpoint.RoutePattern.RequiredValues["action"]); + Assert.False(endpoint.RoutePattern.RequiredValues.ContainsKey("page")); + Assert.Equal("Test", endpoint.Metadata.GetMetadata().RouteName); + Assert.Equal("Test", endpoint.Metadata.GetMetadata().EndpointName); + } + [Fact] public void AddEndpoints_ConventionalRouted_WithMatchingConstraint_CreatesEndpoint() { @@ -300,7 +320,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing private RouteEndpoint CreateAttributeRoutedEndpoint(ActionDescriptor action) { var endpoints = new List(); - Factory.AddEndpoints(endpoints, action, Array.Empty(), Array.Empty>()); + Factory.AddEndpoints(endpoints, new HashSet(StringComparer.OrdinalIgnoreCase), action, Array.Empty(), Array.Empty>()); return Assert.IsType(Assert.Single(endpoints)); } @@ -314,7 +334,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing Assert.NotNull(action.RouteValues); var endpoints = new List(); - Factory.AddEndpoints(endpoints, action, new[] { route, }, Array.Empty>()); + Factory.AddEndpoints(endpoints, new HashSet(StringComparer.OrdinalIgnoreCase), action, new[] { route, }, Array.Empty>()); var endpoint = Assert.IsType(Assert.Single(endpoints)); // This should be true for all conventional-routed actions. @@ -331,7 +351,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing private IReadOnlyList CreateConventionalRoutedEndpoints(ActionDescriptor action, IReadOnlyList routes) { var endpoints = new List(); - Factory.AddEndpoints(endpoints, action, routes, Array.Empty>()); + Factory.AddEndpoints(endpoints, new HashSet(StringComparer.OrdinalIgnoreCase), action, routes, Array.Empty>()); return endpoints.Cast().ToList(); } diff --git a/src/Mvc/Mvc.Core/test/Routing/ControllerActionEndpointDataSourceTest.cs b/src/Mvc/Mvc.Core/test/Routing/ControllerActionEndpointDataSourceTest.cs index efb9e2bd91..deb97532ce 100644 --- a/src/Mvc/Mvc.Core/test/Routing/ControllerActionEndpointDataSourceTest.cs +++ b/src/Mvc/Mvc.Core/test/Routing/ControllerActionEndpointDataSourceTest.cs @@ -58,6 +58,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing AttributeRouteInfo = new AttributeRouteInfo() { Template = "/test", + Name = "Test", }, RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) { @@ -107,16 +108,22 @@ namespace Microsoft.AspNetCore.Mvc.Routing { Assert.Equal("/1/{controller}/{action}/{id?}", e.RoutePattern.RawText); Assert.Null(e.Metadata.GetMetadata()); + Assert.Equal("1", e.Metadata.GetMetadata().RouteName); + Assert.Equal("1", e.Metadata.GetMetadata().EndpointName); }, e => { Assert.Equal("/2/{controller}/{action}/{id?}", e.RoutePattern.RawText); Assert.Null(e.Metadata.GetMetadata()); + Assert.Equal("2", e.Metadata.GetMetadata().RouteName); + Assert.Equal("2", e.Metadata.GetMetadata().EndpointName); }, e => { Assert.Equal("/test", e.RoutePattern.RawText); Assert.Same(actions[0], e.Metadata.GetMetadata()); + Assert.Equal("Test", e.Metadata.GetMetadata().RouteName); + Assert.Equal("Test", e.Metadata.GetMetadata().EndpointName); }); } @@ -201,6 +208,95 @@ namespace Microsoft.AspNetCore.Mvc.Routing }); } + [Fact] + public void Endpoints_AppliesConventions_CanOverideEndpointName() + { + // Arrange + var actions = new List + { + new ControllerActionDescriptor + { + AttributeRouteInfo = new AttributeRouteInfo() + { + Template = "/test", + Name = "Test", + }, + RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "action", "Test" }, + { "controller", "Test" }, + }, + }, + new ControllerActionDescriptor + { + RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "action", "Index" }, + { "controller", "Home" }, + }, + } + }; + + var mockDescriptorProvider = new Mock(); + mockDescriptorProvider + .Setup(m => m.ActionDescriptors) + .Returns(new ActionDescriptorCollection(actions, 0)); + + var dataSource = (ControllerActionEndpointDataSource)CreateDataSource(mockDescriptorProvider.Object); + dataSource.AddRoute("1", "/1/{controller}/{action}/{id?}", null, null, null); + dataSource.AddRoute("2", "/2/{controller}/{action}/{id?}", null, null, null); + + + dataSource.DefaultBuilder.Add(b => + { + if (b.Metadata.OfType().FirstOrDefault()?.AttributeRouteInfo != null) + { + b.Metadata.Add(new EndpointNameMetadata("NewName")); + } + }); + + // Act + var endpoints = dataSource.Endpoints; + + // Assert + Assert.Collection( + endpoints.OfType().Where(e => !SupportsLinkGeneration(e)).OrderBy(e => e.RoutePattern.RawText), + e => + { + Assert.Equal("/1/{controller}/{action}/{id?}", e.RoutePattern.RawText); + Assert.Same(actions[1], e.Metadata.GetMetadata()); + }, + e => + { + Assert.Equal("/2/{controller}/{action}/{id?}", e.RoutePattern.RawText); + Assert.Same(actions[1], e.Metadata.GetMetadata()); + }); + + Assert.Collection( + endpoints.OfType().Where(e => SupportsLinkGeneration(e)).OrderBy(e => e.RoutePattern.RawText), + e => + { + Assert.Equal("/1/{controller}/{action}/{id?}", e.RoutePattern.RawText); + Assert.Null(e.Metadata.GetMetadata()); + Assert.Equal("1", e.Metadata.GetMetadata().RouteName); + Assert.Equal("1", e.Metadata.GetMetadata().EndpointName); + }, + e => + { + Assert.Equal("/2/{controller}/{action}/{id?}", e.RoutePattern.RawText); + Assert.Null(e.Metadata.GetMetadata()); + Assert.Equal("2", e.Metadata.GetMetadata().RouteName); + Assert.Equal("2", e.Metadata.GetMetadata().EndpointName); + }, + e => + { + Assert.Equal("/test", e.RoutePattern.RawText); + Assert.Same(actions[0], e.Metadata.GetMetadata()); + Assert.Equal("Test", e.Metadata.GetMetadata().RouteName); + Assert.Equal("NewName", e.Metadata.GetMetadata().EndpointName); + }); + } + [Fact] public void Endpoints_AppliesConventions_RouteSpecificMetadata() { diff --git a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageLoader.cs b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageLoader.cs index 6942a49531..f9042f1d64 100644 --- a/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageLoader.cs +++ b/src/Mvc/Mvc.RazorPages/src/Infrastructure/DefaultPageLoader.cs @@ -105,7 +105,12 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure // compiling/loading the page. Then once we have a match we load the page and we can create an endpoint // with all of the information we get from the compiled action descriptor. var endpoints = new List(); - _endpointFactory.AddEndpoints(endpoints, compiled, Array.Empty(), Array.Empty>()); + _endpointFactory.AddEndpoints( + endpoints, + routeNames: new HashSet(StringComparer.OrdinalIgnoreCase), + action: compiled, + routes: Array.Empty(), + conventions: Array.Empty>()); // In some test scenarios there's no route so the endpoint isn't created. This is fine because // it won't happen for real. diff --git a/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionEndpointDataSource.cs b/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionEndpointDataSource.cs index d5282f4c00..ce7cd62e94 100644 --- a/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionEndpointDataSource.cs +++ b/src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionEndpointDataSource.cs @@ -32,11 +32,12 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure protected override List CreateEndpoints(IReadOnlyList actions, IReadOnlyList> conventions) { var endpoints = new List(); + var routeNames = new HashSet(StringComparer.OrdinalIgnoreCase); for (var i = 0; i < actions.Count; i++) { if (actions[i] is PageActionDescriptor action) { - _endpointFactory.AddEndpoints(endpoints, action, Array.Empty(), conventions); + _endpointFactory.AddEndpoints(endpoints, routeNames, action, Array.Empty(), conventions); } } diff --git a/src/Mvc/test/Mvc.FunctionalTests/RoutingEndpointRoutingTest.cs b/src/Mvc/test/Mvc.FunctionalTests/RoutingEndpointRoutingTest.cs index 07512b9b01..553af52750 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/RoutingEndpointRoutingTest.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/RoutingEndpointRoutingTest.cs @@ -377,6 +377,67 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.Equal("/ConventionalTransformerRoute/conventional-transformer", result.Link); } + [Fact] + public async Task LinkGenerator_EndpointName_LinkToConventionalRoutedAction() + { + // Arrange + + // Act + var response = await Client.GetAsync("/EndpointName/LinkToConventionalRouted"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadAsStringAsync(); + + Assert.Equal("/EndpointName/LinkToConventionalRouted", body); + } + + [Fact] + public async Task LinkGenerator_EndpointName_LinkToConventionalRoutedAction_WithAmbientValueIgnored() + { + // Arrange + + // Act + var response = await Client.GetAsync("/EndpointName/LinkToConventionalRouted/test"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadAsStringAsync(); + + Assert.Equal("/EndpointName/LinkToConventionalRouted", body); + } + + + [Fact] + public async Task LinkGenerator_EndpointName_LinkToAttributeRoutedAction() + { + // Arrange + + // Act + var response = await Client.GetAsync("/EndpointName/LinkToAttributeRouted"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadAsStringAsync(); + + Assert.Equal("/EndpointName/LinkToAttributeRouted", body); + } + + [Fact] + public async Task LinkGenerator_EndpointName_LinkToAttributeRoutedAction_WithAmbientValueIgnored() + { + // Arrange + + // Act + var response = await Client.GetAsync("/EndpointName/LinkToAttributeRouted/test"); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadAsStringAsync(); + + Assert.Equal("/EndpointName/LinkToAttributeRouted", body); + } + // Endpoint routing exposes HTTP 405s for HTTP method mismatches. protected override void AssertCorsRejectionStatusCode(HttpResponseMessage response) { diff --git a/src/Mvc/test/WebSites/RoutingWebSite/Controllers/EndpointNameController.cs b/src/Mvc/test/WebSites/RoutingWebSite/Controllers/EndpointNameController.cs new file mode 100644 index 0000000000..cca88e3962 --- /dev/null +++ b/src/Mvc/test/WebSites/RoutingWebSite/Controllers/EndpointNameController.cs @@ -0,0 +1,33 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Routing; + +namespace RoutingWebSite.Controllers +{ + public class EndpointNameController : ControllerBase + { + private readonly LinkGenerator _generator; + + public EndpointNameController(LinkGenerator generator) + { + _generator = generator; + } + + // This is a special case that leads to multiple endpoints with the same route name. IRouter-based routing + // supports this. + [HttpGet] + [HttpPost] + [Route("/[controller]/[action]/{path?}", Name = "EndpointNameController_LinkToAttributeRouted")] + public string LinkToAttributeRouted() + { + return _generator.GetPathByName(HttpContext, "EndpointNameController_LinkToAttributeRouted", values: null); + } + + public string LinkToConventionalRouted() + { + return _generator.GetPathByName(HttpContext, "RouteWithOptionalSegment", new { controller = "EndpointName", action = nameof(LinkToConventionalRouted), }); + } + } +}