Fix endpoint support for area/controller/action in attribute route (#8447)

src/Microsoft.AspNetCore.Mvc.Core/Builder/MvcEndpointInfo.cs

@@ -28,7 +28,7 @@ namespace Microsoft.AspNetCore.Builder
                 // Data we parse from the pattern will be used to fill in the rest of the constraints or
                 // defaults. The parser will throw for invalid routes.
                 ParsedPattern = RoutePatternFactory.Parse(pattern, defaults, constraints);
-                Constraints = BuildConstraints(parameterPolicyFactory);
+                ParameterPolicies = BuildParameterPolicies(ParsedPattern.Parameters, parameterPolicyFactory);
 
                 Defaults = defaults;
                 // Merge defaults outside of RoutePattern because the defaults will already have values from pattern
@@ -50,33 +50,30 @@ namespace Microsoft.AspNetCore.Builder
         // Inline and non-inline defaults merged into one
         public RouteValueDictionary MergedDefaults { get; }
 
-        public IDictionary<string, IList<IRouteConstraint>> Constraints { get; }
+        public IDictionary<string, IList<IParameterPolicy>> ParameterPolicies { get; }
         public RouteValueDictionary DataTokens { get; }
         public RoutePattern ParsedPattern { get; private set; }
 
-        private Dictionary<string, IList<IRouteConstraint>> BuildConstraints(ParameterPolicyFactory parameterPolicyFactory)
+        internal static Dictionary<string, IList<IParameterPolicy>> BuildParameterPolicies(IReadOnlyList<RoutePatternParameterPart> parameters, ParameterPolicyFactory parameterPolicyFactory)
         {
-            var constraints = new Dictionary<string, IList<IRouteConstraint>>(StringComparer.OrdinalIgnoreCase);
+            var policies = new Dictionary<string, IList<IParameterPolicy>>(StringComparer.OrdinalIgnoreCase);
 
-            foreach (var parameter in ParsedPattern.Parameters)
+            foreach (var parameter in parameters)
             {
                 foreach (var parameterPolicy in parameter.ParameterPolicies)
                 {
                     var createdPolicy = parameterPolicyFactory.Create(parameter, parameterPolicy);
-                    if (createdPolicy is IRouteConstraint routeConstraint)
+                    if (!policies.TryGetValue(parameter.Name, out var policyList))
                     {
-                        if (!constraints.TryGetValue(parameter.Name, out var paramConstraints))
+                        policyList = new List<IParameterPolicy>();
-                        {
+                        policies.Add(parameter.Name, policyList);
-                            paramConstraints = new List<IRouteConstraint>();
-                            constraints.Add(parameter.Name, paramConstraints);
-                        }
-
-                        paramConstraints.Add(routeConstraint);
                     }
+
+                    policyList.Add(createdPolicy);
                 }
             }
 
-            return constraints;
+            return policies;
         }
     }
-}
+}

src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcEndpointDataSource.cs

@@ -9,7 +9,6 @@ using System.Text;
 using System.Threading;
 using Microsoft.AspNetCore.Builder;
 using Microsoft.AspNetCore.Http;
-using Microsoft.AspNetCore.Http.Features;
 using Microsoft.AspNetCore.Mvc.Abstractions;
 using Microsoft.AspNetCore.Mvc.Infrastructure;
 using Microsoft.AspNetCore.Mvc.Routing;
@@ -113,7 +112,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
                         //
                         // Start with an order of '1' for conventional routes as attribute routes have a default order of '0'.
                         // This is for scenarios dealing with migrating existing Router based code to Endpoint Routing world.
-                        var conventionalRouteOrder = 0;
+                        var conventionalRouteOrder = 1;
 
                         // Check each of the conventional patterns to see if the action would be reachable
                         // If the action and pattern are compatible then create an endpoint with the
@@ -143,90 +142,38 @@ namespace Microsoft.AspNetCore.Mvc.Internal
                                 continue;
                             }
 
-                            var newPathSegments = endpointInfo.ParsedPattern.PathSegments.ToList();
+                            conventionalRouteOrder = CreateEndpoints(
-
+                                endpoints,
-                            for (var i = 0; i < newPathSegments.Count; i++)
+                                ref patternStringBuilder,
-                            {
-                                // Check if the pattern can be shortened because the remaining parameters are optional
-                                //
-                                // e.g. Matching pattern {controller=Home}/{action=Index}/{id?} against HomeController.Index
-                                // can resolve to the following endpoints:
-                                // - /Home/Index/{id?}
-                                // - /Home
-                                // - /
-                                if (UseDefaultValuePlusRemainingSegmentsOptional(i, action, endpointInfo, newPathSegments))
-                                {
-                                    var subPathSegments = newPathSegments.Take(i);
-
-                                    var subEndpoint = CreateEndpoint(
-                                        action,
-                                        endpointInfo.Name,
-                                        GetPattern(ref patternStringBuilder, subPathSegments),
-                                        subPathSegments,
-                                        endpointInfo.Defaults,
-                                        ++conventionalRouteOrder,
-                                        endpointInfo,
-                                        endpointInfo.DataTokens,
-                                        suppressLinkGeneration: false,
-                                        suppressPathMatching: false);
-                                    endpoints.Add(subEndpoint);
-                                }
-
-                                List<RoutePatternPart> segmentParts = null; // Initialize only as needed
-                                var segment = newPathSegments[i];
-                                for (var j = 0; j < segment.Parts.Count; j++)
-                                {
-                                    var part = segment.Parts[j];
-
-                                    if (part.IsParameter &&
-                                        part is RoutePatternParameterPart parameterPart &&
-                                        action.RouteValues.ContainsKey(parameterPart.Name))
-                                    {
-                                        if (segmentParts == null)
-                                        {
-                                            segmentParts = segment.Parts.ToList();
-                                        }
-
-                                        // Replace parameter with literal value
-                                        segmentParts[j] = RoutePatternFactory.LiteralPart(action.RouteValues[parameterPart.Name]);
-                                    }
-                                }
-
-                                // A parameter part was replaced so replace segment with updated parts
-                                if (segmentParts != null)
-                                {
-                                    newPathSegments[i] = RoutePatternFactory.Segment(segmentParts);
-                                }
-                            }
-
-                            var endpoint = CreateEndpoint(
                                 action,
-                                endpointInfo.Name,
+                                conventionalRouteOrder,
-                                GetPattern(ref patternStringBuilder, newPathSegments),
+                                endpointInfo.ParsedPattern,
-                                newPathSegments,
+                                endpointInfo.MergedDefaults,
                                 endpointInfo.Defaults,
-                                ++conventionalRouteOrder,
+                                endpointInfo.Name,
-                                endpointInfo,
                                 endpointInfo.DataTokens,
+                                endpointInfo.ParameterPolicies,
                                 suppressLinkGeneration: false,
                                 suppressPathMatching: false);
-                            endpoints.Add(endpoint);
                         }
                     }
                     else
                     {
-                        var endpoint = CreateEndpoint(
+                        var attributeRoutePattern = RoutePatternFactory.Parse(action.AttributeRouteInfo.Template);
+
+                        CreateEndpoints(
+                            endpoints,
+                            ref patternStringBuilder,
                             action,
-                            action.AttributeRouteInfo.Name,
-                            action.AttributeRouteInfo.Template,
-                            RoutePatternFactory.Parse(action.AttributeRouteInfo.Template).PathSegments,
-                            nonInlineDefaults: null,
                             action.AttributeRouteInfo.Order,
-                            action.AttributeRouteInfo,
+                            attributeRoutePattern,
+                            attributeRoutePattern.Defaults,
+                            nonInlineDefaults: null,
+                            action.AttributeRouteInfo.Name,
                             dataTokens: null,
-                            suppressLinkGeneration: action.AttributeRouteInfo.SuppressLinkGeneration,
+                            allParameterPolicies: null,
-                            suppressPathMatching: action.AttributeRouteInfo.SuppressPathMatching);
+                            action.AttributeRouteInfo.SuppressLinkGeneration,
-                        endpoints.Add(endpoint);
+                            action.AttributeRouteInfo.SuppressPathMatching);
                     }
                 }
 
@@ -246,6 +193,93 @@ namespace Microsoft.AspNetCore.Mvc.Internal
                 // Step 4 - trigger old token
                 oldCancellationTokenSource?.Cancel();
             }
+        }
+
+        // CreateEndpoints processes the route pattern, replacing area/controller/action parameters with endpoint values
+        // Because of default values it is possible for a route pattern to resolve to multiple endpoints
+        private int CreateEndpoints(
+            List<Endpoint> endpoints,
+            ref StringBuilder patternStringBuilder,
+            ActionDescriptor action,
+            int routeOrder,
+            RoutePattern routePattern,
+            IReadOnlyDictionary<string, object> allDefaults,
+            IReadOnlyDictionary<string, object> nonInlineDefaults,
+            string name,
+            RouteValueDictionary dataTokens,
+            IDictionary<string, IList<IParameterPolicy>> allParameterPolicies,
+            bool suppressLinkGeneration,
+            bool suppressPathMatching)
+        {
+            var newPathSegments = routePattern.PathSegments.ToList();
+
+            for (var i = 0; i < newPathSegments.Count; i++)
+            {
+                // Check if the pattern can be shortened because the remaining parameters are optional
+                //
+                // e.g. Matching pattern {controller=Home}/{action=Index}/{id?} against HomeController.Index
+                // can resolve to the following endpoints:
+                // - /Home/Index/{id?}
+                // - /Home
+                // - /
+                if (UseDefaultValuePlusRemainingSegmentsOptional(i, action, allDefaults, newPathSegments))
+                {
+                    var subPathSegments = newPathSegments.Take(i);
+
+                    var subEndpoint = CreateEndpoint(
+                        action,
+                        name,
+                        GetPattern(ref patternStringBuilder, subPathSegments),
+                        subPathSegments,
+                        nonInlineDefaults,
+                        routeOrder++,
+                        dataTokens,
+                        suppressLinkGeneration,
+                        suppressPathMatching);
+                    endpoints.Add(subEndpoint);
+                }
+
+                List<RoutePatternPart> segmentParts = null; // Initialize only as needed
+                var segment = newPathSegments[i];
+                for (var j = 0; j < segment.Parts.Count; j++)
+                {
+                    var part = segment.Parts[j];
+
+                    if (part.IsParameter &&
+                        part is RoutePatternParameterPart parameterPart &&
+                        action.RouteValues.ContainsKey(parameterPart.Name))
+                    {
+                        if (segmentParts == null)
+                        {
+                            segmentParts = segment.Parts.ToList();
+                        }
+
+                        var parameterRouteValue = action.RouteValues[parameterPart.Name];
+
+                        segmentParts[j] = RoutePatternFactory.LiteralPart(parameterRouteValue);
+                    }
+                }
+
+                // A parameter part was replaced so replace segment with updated parts
+                if (segmentParts != null)
+                {
+                    newPathSegments[i] = RoutePatternFactory.Segment(segmentParts);
+                }
+            }
+
+            var endpoint = CreateEndpoint(
+                action,
+                name,
+                GetPattern(ref patternStringBuilder, newPathSegments),
+                newPathSegments,
+                nonInlineDefaults,
+                routeOrder++,
+                dataTokens,
+                suppressLinkGeneration,
+                suppressPathMatching);
+            endpoints.Add(endpoint);
+
+            return routeOrder;
 
             string GetPattern(ref StringBuilder sb, IEnumerable<RoutePatternPathSegment> segments)
             {
@@ -265,7 +299,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
         private bool UseDefaultValuePlusRemainingSegmentsOptional(
             int segmentIndex,
             ActionDescriptor action,
-            MvcEndpointInfo endpointInfo,
+            IReadOnlyDictionary<string, object> allDefaults,
             List<RoutePatternPathSegment> pathSegments)
         {
             // Check whether the remaining segments are all optional and one or more of them is
@@ -287,7 +321,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal
 
                         if (action.RouteValues.ContainsKey(parameterPart.Name))
                         {
-                            if (endpointInfo.MergedDefaults[parameterPart.Name] is string defaultValue
+                            if (allDefaults.TryGetValue(parameterPart.Name, out var v)
+                                && v is string defaultValue
                                 && action.RouteValues.TryGetValue(parameterPart.Name, out var routeValue)
                                 && string.Equals(defaultValue, routeValue, StringComparison.OrdinalIgnoreCase))
                             {
@@ -346,11 +381,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal
                 {
                     // Check that the value matches against constraints on that parameter
                     // e.g. For {controller:regex((Home|Login))} the controller value must match the regex
-                    if (endpointInfo.Constraints.TryGetValue(routeKey, out var constraints))
+                    if (endpointInfo.ParameterPolicies.TryGetValue(routeKey, out var parameterPolicies))
                     {
-                        foreach (var constraint in constraints)
+                        foreach (var policy in parameterPolicies)
                         {
-                            if (!constraint.Match(httpContext: null, NullRouter.Instance, routeKey, new RouteValueDictionary(action.RouteValues), RouteDirection.IncomingRequest))
+                            if (policy is IRouteConstraint constraint
+                                && !constraint.Match(httpContext: null, NullRouter.Instance, routeKey, new RouteValueDictionary(action.RouteValues), RouteDirection.IncomingRequest))
                             {
                                 // Did not match constraint
                                 return false;
@@ -372,7 +408,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             IEnumerable<RoutePatternPathSegment> segments,
             object nonInlineDefaults,
             int order,
-            object source,
             RouteValueDictionary dataTokens,
             bool suppressLinkGeneration,
             bool suppressPathMatching)
@@ -394,7 +429,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
                 action,
                 routeName,
                 new RouteValueDictionary(action.RouteValues),
-                source,
                 dataTokens,
                 suppressLinkGeneration,
                 suppressPathMatching);
@@ -413,7 +447,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             ActionDescriptor action,
             string routeName,
             RouteValueDictionary requiredValues,
-            object source,
             RouteValueDictionary dataTokens,
             bool suppressLinkGeneration,
             bool suppressPathMatching)

test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MvcEndpointDataSourceTests.cs

@@ -74,7 +74,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
 
             Assert.Equal(displayName, matcherEndpoint.DisplayName);
             Assert.Equal(order, matcherEndpoint.Order);
-            Assert.Equal(template, matcherEndpoint.RoutePattern.RawText);
+            Assert.Equal("Template!", matcherEndpoint.RoutePattern.RawText);
         }
 
         [Fact]
@@ -134,21 +134,41 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             Assert.True(actionInvokerCalled);
         }
 
+        public static TheoryData GetSingleActionData_Conventional
+        {
+            get => GetSingleActionData(true);
+        }
+
+        public static TheoryData GetSingleActionData_Attribute
+        {
+            get => GetSingleActionData(false);
+        }
+
+        private static TheoryData GetSingleActionData(bool isConventionalRouting)
+        {
+            var data = new TheoryData<string, string[]>
+                {
+                    {"{controller}/{action}/{id?}", new[] { "TestController/TestAction/{id?}" }},
+                    {"{controller}/{id?}", isConventionalRouting ? new string[] { } : new[] { "TestController/{id?}" }},
+                    {"{action}/{id?}", isConventionalRouting ? new string[] { } : new[] { "TestAction/{id?}" }},
+                    {"{Controller}/{Action}/{id?}", new[] { "TestController/TestAction/{id?}" }},
+                    {"{CONTROLLER}/{ACTION}/{id?}", new[] { "TestController/TestAction/{id?}" }},
+                    {"{controller}/{action=TestAction}", new[] { "TestController", "TestController/TestAction" }},
+                    {"{controller}/{action=TestAction}/{id?}", new[] { "TestController", "TestController/TestAction/{id?}" }},
+                    {"{controller=TestController}/{action=TestAction}/{id?}", new[] { "", "TestController", "TestController/TestAction/{id?}" }},
+                    {"{controller}/{action}/{*catchAll}", new[] { "TestController/TestAction/{*catchAll}" }},
+                    {"{controller}/{action=TestAction}/{*catchAll}", new[] { "TestController", "TestController/TestAction/{*catchAll}" }},
+                    {"{controller}/{action=TestAction}/{id?}/{*catchAll}", new[] { "TestController", "TestController/TestAction/{id?}/{*catchAll}" }},
+                    {"{controller}/{action}.{ext?}", new[] { "TestController/TestAction.{ext?}" }},
+                    {"{controller}/{action=TestAction}.{ext?}", new[] { "TestController", "TestController/TestAction.{ext?}" }},
+                };
+
+            return data;
+        }
+
         [Theory]
-        [InlineData("{controller}/{action}/{id?}", new[] { "TestController/TestAction/{id?}" })]
+        [MemberData(nameof(GetSingleActionData_Conventional))]
-        [InlineData("{controller}/{id?}", new string[] { })]
+        public void Endpoints_Conventional_SingleAction(string endpointInfoRoute, string[] finalEndpointPatterns)
-        [InlineData("{action}/{id?}", new string[] { })]
-        [InlineData("{Controller}/{Action}/{id?}", new[] { "TestController/TestAction/{id?}" })]
-        [InlineData("{CONTROLLER}/{ACTION}/{id?}", new[] { "TestController/TestAction/{id?}" })]
-        [InlineData("{controller}/{action=TestAction}", new[] { "TestController", "TestController/TestAction" })]
-        [InlineData("{controller}/{action=TestAction}/{id?}", new[] { "TestController", "TestController/TestAction/{id?}" })]
-        [InlineData("{controller=TestController}/{action=TestAction}/{id?}", new[] { "", "TestController", "TestController/TestAction/{id?}" })]
-        [InlineData("{controller}/{action}/{*catchAll}", new[] { "TestController/TestAction/{*catchAll}" })]
-        [InlineData("{controller}/{action=TestAction}/{*catchAll}", new[] { "TestController", "TestController/TestAction/{*catchAll}" })]
-        [InlineData("{controller}/{action=TestAction}/{id?}/{*catchAll}", new[] { "TestController", "TestController/TestAction/{id?}/{*catchAll}" })]
-        [InlineData("{controller}/{action}.{ext?}", new[] { "TestController/TestAction.{ext?}" })]
-        [InlineData("{controller}/{action=TestAction}.{ext?}", new[] { "TestController", "TestController/TestAction.{ext?}" })]
-        public void Endpoints_SingleAction(string endpointInfoRoute, string[] finalEndpointPatterns)
         {
             // Arrange
             var actionDescriptorCollection = GetActionDescriptorCollection(
@@ -168,6 +188,28 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             Assert.Collection(endpoints, inspectors);
         }
 
+        [Theory]
+        [MemberData(nameof(GetSingleActionData_Attribute))]
+        public void Endpoints_AttributeRouting_SingleAction(string endpointInfoRoute, string[] finalEndpointPatterns)
+        {
+            // Arrange
+            var actionDescriptorCollection = GetActionDescriptorCollection(
+                attributeRouteTemplate: endpointInfoRoute,
+                new { controller = "TestController", action = "TestAction" });
+            var dataSource = CreateMvcEndpointDataSource(actionDescriptorCollection);
+
+            // Act
+            var endpoints = dataSource.Endpoints;
+
+            // Assert
+            var inspectors = finalEndpointPatterns
+                .Select(t => new Action<Endpoint>(e => Assert.Equal(t, Assert.IsType<RouteEndpoint>(e).RoutePattern.RawText)))
+                .ToArray();
+
+            // Assert
+            Assert.Collection(endpoints, inspectors);
+        }
+
         [Theory]
         [InlineData("{area}/{controller}/{action}/{id?}", new[] { "TestArea/TestController/TestAction/{id?}" })]
         [InlineData("{controller}/{action}/{id?}", new string[] { })]
@@ -740,11 +782,16 @@ namespace Microsoft.AspNetCore.Mvc.Internal
         }
 
         private IActionDescriptorCollectionProvider GetActionDescriptorCollection(params object[] requiredValues)
+        {
+            return GetActionDescriptorCollection(attributeRouteTemplate: null, requiredValues);
+        }
+
+        private IActionDescriptorCollectionProvider GetActionDescriptorCollection(string attributeRouteTemplate, params object[] requiredValues)
         {
             var actionDescriptors = new List<ActionDescriptor>();
             foreach (var requiredValue in requiredValues)
             {
-                actionDescriptors.Add(CreateActionDescriptor(requiredValue));
+                actionDescriptors.Add(CreateActionDescriptor(requiredValue, attributeRouteTemplate));
             }
 
             var actionDescriptorCollectionProviderMock = new Mock<IActionDescriptorCollectionProvider>();
@@ -756,10 +803,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal
 
         private ActionDescriptor CreateActionDescriptor(string controller, string action, string area = null)
         {
-            return CreateActionDescriptor(new { controller = controller, action = action, area = area });
+            return CreateActionDescriptor(new { controller = controller, action = action, area = area }, attributeRouteTemplate: null);
         }
 
-        private ActionDescriptor CreateActionDescriptor(object requiredValues)
+        private ActionDescriptor CreateActionDescriptor(object requiredValues, string attributeRouteTemplate = null)
         {
             var actionDescriptor = new ActionDescriptor();
             var routeValues = new RouteValueDictionary(requiredValues);
@@ -767,6 +814,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             {
                 actionDescriptor.RouteValues[kvp.Key] = kvp.Value?.ToString();
             }
+            if (!string.IsNullOrEmpty(attributeRouteTemplate))
+            {
+                actionDescriptor.AttributeRouteInfo = new AttributeRouteInfo
+                {
+                    Name = attributeRouteTemplate,
+                    Template = attributeRouteTemplate
+                };
+            }
             return actionDescriptor;
         }