[Design] Split up MvcRouteHandler

This change splits up the conventional routing path from the attribute
routing path *inside* routing, instead of inside `MvcRouteHandler`. Each
attribute route group now gets its own instance of
`MvcAttributeRouteHandler` which just knows about the actions it can
reach.

This removes the concept of a route-group-token and removes the lookup
table entirely for attribute routing. This also means that the
`DefaultHandler` on `IRouteBuilder` will not be used for attribute routes,
which we are OK with for 1.0.0.

The action selector's functionality is now split into two methods. We
think this is OK for 1.0.0 because any customization of `IActionSelector`
up to now had to implement virtually the same policy as ours in order to
work with attribute routing. It should now be possible to customize the
selector in a meaningful way without interfering with attribute routing.

src/Microsoft.AspNetCore.Mvc.Abstractions/Abstractions/ActionDescriptor.cs

@@ -16,7 +16,6 @@ public ActionDescriptor()
             Id = Guid.NewGuid().ToString();
             Properties = new Dictionary<object, object>();
             RouteValues = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
-            RouteValueDefaults = new Dictionary<string, object>(StringComparer.OrdinalIgnoreCase);
         }
 
         /// <summary>
@@ -32,8 +31,6 @@ public ActionDescriptor()
 
         public AttributeRouteInfo AttributeRouteInfo { get; set; }
 
-        public IDictionary<string, object> RouteValueDefaults { get; set; }
-
         /// <summary>
         /// The set of constraints for this action. Must all be satisfied for the action to be selected.
         /// </summary>

src/Microsoft.AspNetCore.Mvc.Abstractions/ActionConstraints/ActionSelectorCandidate.cs

@@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Mvc.ActionConstraints
     /// <summary>
     /// A candidate action for action selection.
     /// </summary>
-    public class ActionSelectorCandidate
+    public struct ActionSelectorCandidate
     {
         /// <summary>
         /// Creates a new <see cref="ActionSelectorCandidate"/>.
@@ -33,11 +33,11 @@ public ActionSelectorCandidate(ActionDescriptor action, IReadOnlyList<IActionCon
         /// <summary>
         /// The <see cref="ActionDescriptor"/> representing a candiate for selection.
         /// </summary>
-        public ActionDescriptor Action { get; private set; }
+        public ActionDescriptor Action { get; }
 
         /// <summary>
         /// The list of <see cref="IActionConstraint"/> instances associated with <see name="Action"/>.
         /// </summary>
-        public IReadOnlyList<IActionConstraint> Constraints { get; private set; }
+        public IReadOnlyList<IActionConstraint> Constraints { get; }
     }
 }

src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/AttributeRouteModel.cs

@@ -198,7 +198,7 @@ private static string CleanTemplate(string result)
             return result.Substring(startIndex, subStringLength);
         }
 
-        public static string ReplaceTokens(string template, IDictionary<string, object> values)
+        public static string ReplaceTokens(string template, IDictionary<string, string> values)
         {
             var builder = new StringBuilder();
             var state = TemplateParserState.Plaintext;
@@ -340,7 +340,7 @@ public static string ReplaceTokens(string template, IDictionary<string, object>
                                 .Replace("[[", "[")
                                 .Replace("]]", "]");
 
-                            object value;
+                            string value;
                             if (!values.TryGetValue(token, out value))
                             {
                                 // Value not found

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

@@ -3,9 +3,7 @@
 
 using System;
 using Microsoft.AspNetCore.Mvc.Core;
-using Microsoft.AspNetCore.Mvc.Infrastructure;
 using Microsoft.AspNetCore.Mvc.Internal;
-using Microsoft.AspNetCore.Mvc.Routing;
 using Microsoft.AspNetCore.Routing;
 using Microsoft.Extensions.DependencyInjection;
 
@@ -94,11 +92,7 @@ public static IApplicationBuilder UseMvc(
 
             configureRoutes(routes);
 
-            // Adding the attribute route comes after running the user-code because
-            // we want to respect any changes to the DefaultHandler.
-            routes.Routes.Insert(0, AttributeRouting.CreateAttributeMegaRoute(
-                routes.DefaultHandler,
-                app.ApplicationServices));
+            routes.Routes.Insert(0, AttributeRouting.CreateAttributeMegaRoute(app.ApplicationServices));
 
             return app.UseRouter(routes.Build());
         }

src/Microsoft.AspNetCore.Mvc.Core/ConsumesAttribute.cs

@@ -142,7 +142,7 @@ public bool Accept(ActionConstraintContext context)
             }
 
             var firstCandidate = context.Candidates[0];
-            if (firstCandidate != context.CurrentCandidate)
+            if (firstCandidate.Action != context.CurrentCandidate.Action)
             {
                 // If the current candidate is not same as the first candidate,
                 // we need not probe other candidates to see if they apply.
@@ -157,7 +157,7 @@ public bool Accept(ActionConstraintContext context)
             // 3). If we have no matches, then we choose the first constraint to return true.It will later return a 415
             foreach (var candidate in context.Candidates)
             {
-                if (candidate == firstCandidate)
+                if (candidate.Action == firstCandidate.Action)
                 {
                     continue;
                 }

src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs

@@ -217,9 +217,10 @@ internal static void AddMvcCoreServices(IServiceCollection services)
             services.TryAddSingleton<RedirectToRouteResultExecutor>();
 
             //
-            // Setup default handler
+            // Route Handlers
             //
-            services.TryAddSingleton<MvcRouteHandler>();
+            services.TryAddSingleton<MvcRouteHandler>(); // Only one per app
+            services.TryAddTransient<MvcAttributeRouteHandler>(); // Many per app
         }
 
         private static void ConfigureDefaultServices(IServiceCollection services)

src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IActionSelector.cs

@@ -1,6 +1,7 @@
 // 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 System.Collections.Generic;
 using Microsoft.AspNetCore.Mvc.Abstractions;
 using Microsoft.AspNetCore.Routing;
 
@@ -12,13 +13,44 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure
     public interface IActionSelector
     {
         /// <summary>
-        /// Selects an <see cref="ActionDescriptor"/> for the request associated with <paramref name="context"/>.
+        /// Selects a set of <see cref="ActionDescriptor"/> candidates for the current request associated with
+        /// <paramref name="context"/>.
         /// </summary>
-        /// <param name="context">The <see cref="RouteContext"/> for the current request.</param>
-        /// <returns>An <see cref="ActionDescriptor"/> or <c>null</c> if no action can be selected.</returns>
+        /// <param name="context">The <see cref="RouteContext"/> associated with the current request.</param>
+        /// <returns>A set of <see cref="ActionDescriptor"/> candidates or <c>null</c>.</returns>
+        /// <remarks>
+        /// <para>
+        /// Used by conventional routing to select the set of actions that match the route values for the
+        /// current request. Action constraints associated with the candidates are not invoked by this method
+        /// </para>
+        /// <para>
+        /// Attribute routing does not call this method.
+        /// </para>
+        /// </remarks>
+        IReadOnlyList<ActionDescriptor> SelectCandidates(RouteContext context);
+
+        /// <summary>
+        /// Selects the best <see cref="ActionDescriptor"/> candidate from <paramref name="candidates"/> for the 
+        /// current request associated with <paramref name="context"/>.
+        /// </summary>
+        /// <param name="context">The <see cref="RouteContext"/> associated with the current request.</param>
+        /// <param name="candidates">The set of <see cref="ActionDescriptor"/> candidates.</param>
+        /// <returns>The best <see cref="ActionDescriptor"/> candidate for the current request or <c>null</c>.</returns>
         /// <exception cref="Internal.AmbiguousActionException">
         /// Thrown when action selection results in an ambiguity.
         /// </exception>
-        ActionDescriptor Select(RouteContext context);
+        /// <remarks>
+        /// <para>
+        /// Invokes action constraints associated with the candidates.
+        /// </para>
+        /// <para>
+        /// Used by conventional routing after calling <see cref="SelectCandidates"/> to apply action constraints and
+        /// disambiguate between multiple candidates.
+        /// </para>
+        /// <para>
+        /// Used by attribute routing to apply action constraints and disambiguate between multiple candidates.
+        /// </para>
+        /// </remarks>
+        ActionDescriptor SelectBestCandidate(RouteContext context, IReadOnlyList<ActionDescriptor> candidates);
     }
 }

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

@@ -6,9 +6,9 @@
 #if NET451
 using System.ComponentModel;
 #endif
+using System.Linq;
 using Microsoft.AspNetCore.Mvc.Abstractions;
 using Microsoft.AspNetCore.Mvc.Infrastructure;
-using Microsoft.AspNetCore.Mvc.Routing;
 using Microsoft.AspNetCore.Routing;
 using Microsoft.AspNetCore.Routing.DecisionTree;
 
@@ -27,8 +27,9 @@ public ActionSelectionDecisionTree(ActionDescriptorCollection actions)
         {
             Version = actions.Version;
 
+            var conventionalRoutedActions = actions.Items.Where(a => a.AttributeRouteInfo?.Template == null).ToArray();
             _root = DecisionTreeBuilder<ActionDescriptor>.GenerateTree(
-                actions.Items,
+                conventionalRoutedActions,
                 new ActionDescriptorClassifier());
         }
 

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

@@ -39,44 +39,32 @@ public ActionSelector(
             _actionConstraintCache = actionConstraintCache;
         }
 
-        /// <inheritdoc />
-        public ActionDescriptor Select(RouteContext context)
+        public IReadOnlyList<ActionDescriptor> SelectCandidates(RouteContext context)
         {
             if (context == null)
             {
                 throw new ArgumentNullException(nameof(context));
             }
 
             var tree = _decisionTreeProvider.DecisionTree;
-            var matchingRouteValues = tree.Select(context.RouteData.Values);
-
-            var candidates = new List<ActionSelectorCandidate>();
+            return tree.Select(context.RouteData.Values);
+        }
 
-            // Perf: Avoid allocations
-            for (var i = 0; i < matchingRouteValues.Count; i++)
+        public ActionDescriptor SelectBestCandidate(RouteContext context, IReadOnlyList<ActionDescriptor> candidates)
+        {
+            if (context == null)
             {
-                var action = matchingRouteValues[i];
-                var constraints = _actionConstraintCache.GetActionConstraints(context.HttpContext, action);
-                candidates.Add(new ActionSelectorCandidate(action, constraints));
+                throw new ArgumentNullException(nameof(context));
             }
 
-            var matchingActionConstraints =
-                EvaluateActionConstraints(context, candidates, startingOrder: null);
-
-            List<ActionDescriptor> matchingActions = null;
-            if (matchingActionConstraints != null)
+            if (candidates == null)
             {
-                matchingActions = new List<ActionDescriptor>(matchingActionConstraints.Count);
-                // Perf: Avoid allocations
-                for (var i = 0; i < matchingActionConstraints.Count; i++)
-                {
-                    var candidate = matchingActionConstraints[i];
-                    matchingActions.Add(candidate.Action);
-                }
+                throw new ArgumentNullException(nameof(candidates));
             }
 
-            var finalMatches = SelectBestActions(matchingActions);
+            var matches = EvaluateActionConstraints(context, candidates);
 
+            var finalMatches = SelectBestActions(matches);
             if (finalMatches == null || finalMatches.Count == 0)
             {
                 return null;
@@ -113,7 +101,38 @@ protected virtual IReadOnlyList<ActionDescriptor> SelectBestActions(IReadOnlyLis
             return actions;
         }
 
-        private IReadOnlyList<ActionSelectorCandidate> EvaluateActionConstraints(
+        private IReadOnlyList<ActionDescriptor> EvaluateActionConstraints(
+            RouteContext context,
+            IReadOnlyList<ActionDescriptor> actions)
+        {
+            var candidates = new List<ActionSelectorCandidate>();
+
+            // Perf: Avoid allocations
+            for (var i = 0; i < actions.Count; i++)
+            {
+                var action = actions[i];
+                var constraints = _actionConstraintCache.GetActionConstraints(context.HttpContext, action);
+                candidates.Add(new ActionSelectorCandidate(action, constraints));
+            }
+
+            var matches = EvaluateActionConstraintsCore(context, candidates, startingOrder: null);
+
+            List<ActionDescriptor> results = null;
+            if (matches != null)
+            {
+                results = new List<ActionDescriptor>(matches.Count);
+                // Perf: Avoid allocations
+                for (var i = 0; i < matches.Count; i++)
+                {
+                    var candidate = matches[i];
+                    results.Add(candidate.Action);
+                }
+            }
+
+            return results;
+        }
+
+        private IReadOnlyList<ActionSelectorCandidate> EvaluateActionConstraintsCore(
             RouteContext context,
             IReadOnlyList<ActionSelectorCandidate> candidates,
             int? startingOrder)
@@ -198,7 +217,7 @@ private IReadOnlyList<ActionSelectorCandidate> EvaluateActionConstraints(
             // If we have matches with constraints, those are 'better' so try to keep processing those
             if (actionsWithConstraint.Count > 0)
             {
-                var matches = EvaluateActionConstraints(context, actionsWithConstraint, order);
+                var matches = EvaluateActionConstraintsCore(context, actionsWithConstraint, order);
                 if (matches?.Count > 0)
                 {
                     return matches;
@@ -212,7 +231,7 @@ private IReadOnlyList<ActionSelectorCandidate> EvaluateActionConstraints(
             }
             else
             {
-                return EvaluateActionConstraints(context, actionsWithoutConstraint, order);
+                return EvaluateActionConstraintsCore(context, actionsWithoutConstraint, order);
             }
         }
     }