Makes RouteName => endpoint name for MVC (#9721)

* Makes RouteName => endpoint name for MVC

* feedback

src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs

@@ -33,6 +33,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
 
         public void AddEndpoints(
             List<Endpoint> endpoints,
+            HashSet<string> routeNames,
             ActionDescriptor action,
             IReadOnlyList<ConventionalRouteEntry> routes,
             IReadOnlyList<Action<EndpointBuilder>> 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<Endpoint> endpoints,
+            HashSet<string> routeNames,
             HashSet<string> keys,
             ConventionalRouteEntry route,
             IReadOnlyList<Action<EndpointBuilder>> 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<string> 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<IEndpointNameMetadata>().LastOrDefault()?.EndpointName == null)
+            {
+                builder.Metadata.Add(new EndpointNameMetadata(routeName));
+            }
+
             if (dataTokens != null)
             {
                 builder.Metadata.Add(new DataTokensMetadata(dataTokens));

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<Endpoint>();
             var keys = new HashSet<string>(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<string>(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;

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<IRouteNameMetadata>().RouteName);
+            Assert.Equal("Test", endpoint.Metadata.GetMetadata<IEndpointNameMetadata>().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<Endpoint>();
-            Factory.AddEndpoints(endpoints, action, Array.Empty<ConventionalRouteEntry>(), Array.Empty<Action<EndpointBuilder>>());
+            Factory.AddEndpoints(endpoints, new HashSet<string>(StringComparer.OrdinalIgnoreCase), action, Array.Empty<ConventionalRouteEntry>(), Array.Empty<Action<EndpointBuilder>>());
             return Assert.IsType<RouteEndpoint>(Assert.Single(endpoints));
         }
 
@@ -314,7 +334,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
             Assert.NotNull(action.RouteValues);
 
             var endpoints = new List<Endpoint>();
-            Factory.AddEndpoints(endpoints, action, new[] { route, }, Array.Empty<Action<EndpointBuilder>>());
+            Factory.AddEndpoints(endpoints, new HashSet<string>(StringComparer.OrdinalIgnoreCase), action, new[] { route, }, Array.Empty<Action<EndpointBuilder>>());
             var endpoint = Assert.IsType<RouteEndpoint>(Assert.Single(endpoints));
 
             // This should be true for all conventional-routed actions.
@@ -331,7 +351,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing
         private IReadOnlyList<RouteEndpoint> CreateConventionalRoutedEndpoints(ActionDescriptor action, IReadOnlyList<ConventionalRouteEntry> routes)
         {
             var endpoints = new List<Endpoint>();
-            Factory.AddEndpoints(endpoints, action, routes, Array.Empty<Action<EndpointBuilder>>());
+            Factory.AddEndpoints(endpoints, new HashSet<string>(StringComparer.OrdinalIgnoreCase), action, routes, Array.Empty<Action<EndpointBuilder>>());
             return endpoints.Cast<RouteEndpoint>().ToList();
         }
 

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<string, string>(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<ActionDescriptor>());
+                    Assert.Equal("1", e.Metadata.GetMetadata<IRouteNameMetadata>().RouteName);
+                    Assert.Equal("1", e.Metadata.GetMetadata<IEndpointNameMetadata>().EndpointName);
                 },
                 e =>
                 {
                     Assert.Equal("/2/{controller}/{action}/{id?}", e.RoutePattern.RawText);
                     Assert.Null(e.Metadata.GetMetadata<ActionDescriptor>());
+                    Assert.Equal("2", e.Metadata.GetMetadata<IRouteNameMetadata>().RouteName);
+                    Assert.Equal("2", e.Metadata.GetMetadata<IEndpointNameMetadata>().EndpointName);
                 },
                 e =>
                 {
                     Assert.Equal("/test", e.RoutePattern.RawText);
                     Assert.Same(actions[0], e.Metadata.GetMetadata<ActionDescriptor>());
+                    Assert.Equal("Test", e.Metadata.GetMetadata<IRouteNameMetadata>().RouteName);
+                    Assert.Equal("Test", e.Metadata.GetMetadata<IEndpointNameMetadata>().EndpointName);
                 });
         }
 
@@ -201,6 +208,95 @@ namespace Microsoft.AspNetCore.Mvc.Routing
                 });
         }
 
+        [Fact]
+        public void Endpoints_AppliesConventions_CanOverideEndpointName()
+        {
+            // Arrange
+            var actions = new List<ActionDescriptor>
+            {
+                new ControllerActionDescriptor
+                {
+                    AttributeRouteInfo = new AttributeRouteInfo()
+                    {
+                        Template = "/test",
+                        Name = "Test",
+                    },
+                    RouteValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
+                    {
+                        { "action", "Test" },
+                        { "controller", "Test" },
+                    },
+                },
+                new ControllerActionDescriptor
+                {
+                    RouteValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase)
+                    {
+                        { "action", "Index" },
+                        { "controller", "Home" },
+                    },
+                }
+            };
+
+            var mockDescriptorProvider = new Mock<IActionDescriptorCollectionProvider>();
+            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<ActionDescriptor>().FirstOrDefault()?.AttributeRouteInfo != null)
+                {
+                    b.Metadata.Add(new EndpointNameMetadata("NewName"));
+                }
+            });
+
+            // Act
+            var endpoints = dataSource.Endpoints;
+
+            // Assert
+            Assert.Collection(
+                endpoints.OfType<RouteEndpoint>().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<ActionDescriptor>());
+                },
+                e =>
+                {
+                    Assert.Equal("/2/{controller}/{action}/{id?}", e.RoutePattern.RawText);
+                    Assert.Same(actions[1], e.Metadata.GetMetadata<ActionDescriptor>());
+                });
+
+            Assert.Collection(
+                endpoints.OfType<RouteEndpoint>().Where(e => SupportsLinkGeneration(e)).OrderBy(e => e.RoutePattern.RawText),
+                e =>
+                {
+                    Assert.Equal("/1/{controller}/{action}/{id?}", e.RoutePattern.RawText);
+                    Assert.Null(e.Metadata.GetMetadata<ActionDescriptor>());
+                    Assert.Equal("1", e.Metadata.GetMetadata<IRouteNameMetadata>().RouteName);
+                    Assert.Equal("1", e.Metadata.GetMetadata<IEndpointNameMetadata>().EndpointName);
+                },
+                e =>
+                {
+                    Assert.Equal("/2/{controller}/{action}/{id?}", e.RoutePattern.RawText);
+                    Assert.Null(e.Metadata.GetMetadata<ActionDescriptor>());
+                    Assert.Equal("2", e.Metadata.GetMetadata<IRouteNameMetadata>().RouteName);
+                    Assert.Equal("2", e.Metadata.GetMetadata<IEndpointNameMetadata>().EndpointName);
+                },
+                e =>
+                {
+                    Assert.Equal("/test", e.RoutePattern.RawText);
+                    Assert.Same(actions[0], e.Metadata.GetMetadata<ActionDescriptor>());
+                    Assert.Equal("Test", e.Metadata.GetMetadata<IRouteNameMetadata>().RouteName);
+                    Assert.Equal("NewName", e.Metadata.GetMetadata<IEndpointNameMetadata>().EndpointName);
+                });
+        }
+
         [Fact]
         public void Endpoints_AppliesConventions_RouteSpecificMetadata()
         {

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<Endpoint>();
-            _endpointFactory.AddEndpoints(endpoints, compiled, Array.Empty<ConventionalRouteEntry>(), Array.Empty<Action<EndpointBuilder>>());
+            _endpointFactory.AddEndpoints(
+                endpoints,
+                routeNames: new HashSet<string>(StringComparer.OrdinalIgnoreCase),
+                action: compiled,
+                routes: Array.Empty<ConventionalRouteEntry>(),
+                conventions: Array.Empty<Action<EndpointBuilder>>());
 
             // In some test scenarios there's no route so the endpoint isn't created. This is fine because
             // it won't happen for real.

src/Mvc/Mvc.RazorPages/src/Infrastructure/PageActionEndpointDataSource.cs

@@ -32,11 +32,12 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure
         protected override List<Endpoint> CreateEndpoints(IReadOnlyList<ActionDescriptor> actions, IReadOnlyList<Action<EndpointBuilder>> conventions)
         {
             var endpoints = new List<Endpoint>();
+            var routeNames = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
             for (var i = 0; i < actions.Count; i++)
             {
                 if (actions[i] is PageActionDescriptor action)
                 {
-                    _endpointFactory.AddEndpoints(endpoints, action, Array.Empty<ConventionalRouteEntry>(), conventions);
+                    _endpointFactory.AddEndpoints(endpoints, routeNames, action, Array.Empty<ConventionalRouteEntry>(), conventions);
                 }
             }
 

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)
         {

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), });
+        }
+    }
+}