Make RouteDataActionConstraint be POCO only

src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs

@@ -233,13 +233,10 @@ namespace Microsoft.AspNet.Mvc
                 return false;
             }
 
-            var actions =
+            var tree = _decisionTreeProvider.DecisionTree;
-                GetActions().Where(
+            var matchingRouteConstraints = tree.Select(context.ProvidedValues);
-                    action =>
-                        action.RouteConstraints == null ||
-                        action.RouteConstraints.All(constraint => constraint.Accept(context.ProvidedValues)));
 
-            return actions.Any();
+            return matchingRouteConstraints.Count > 0;
         }
 
         private IReadOnlyList<ActionDescriptor> GetActions()

src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs

@@ -349,7 +349,7 @@ namespace Microsoft.AspNet.Mvc
                     {
                         actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint(
                             AttributeRouting.RouteGroupKey,
-                            RouteKeyHandling.DenyKey));
+                            string.Empty));
                     }
 
                     // Add a route constraint with DenyKey for each constraint in the set to all the
@@ -636,7 +636,7 @@ namespace Microsoft.AspNet.Mvc
             {
                 actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint(
                     "action",
-                    RouteKeyHandling.DenyKey));
+                    string.Empty));
             }
         }
 
@@ -660,11 +660,11 @@ namespace Microsoft.AspNet.Mvc
                 // Skip duplicates
                 if (!HasConstraint(actionDescriptor.RouteConstraints, constraintAttribute.RouteKey))
                 {
-                    if (constraintAttribute.RouteValue == null)
+                    if (constraintAttribute.RouteKeyHandling == RouteKeyHandling.CatchAll)
                     {
-                        actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint(
+                        actionDescriptor.RouteConstraints.Add(
-                            constraintAttribute.RouteKey,
+                            RouteDataActionConstraint.CreateCatchAll(
-                            constraintAttribute.RouteKeyHandling));
+                            constraintAttribute.RouteKey));
                     }
                     else
                     {
@@ -741,7 +741,7 @@ namespace Microsoft.AspNet.Mvc
                 {
                     actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint(
                         key,
-                        RouteKeyHandling.DenyKey));
+                        string.Empty));
                 }
             }
         }

src/Microsoft.AspNet.Mvc.Core/RouteDataActionConstraint.cs

@@ -2,19 +2,14 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Collections;
-using System.Collections.Generic;
-using System.ComponentModel;
-using System.Diagnostics;
-using Microsoft.AspNet.Mvc.Core;
-using Microsoft.AspNet.Routing;
 
 namespace Microsoft.AspNet.Mvc
 {
+    /// <summary>
+    /// Constraints an action to a route key and value.
+    /// </summary>
     public class RouteDataActionConstraint
     {
-        private IEqualityComparer _comparer;
-
         private RouteDataActionConstraint(string routeKey)
         {
             if (routeKey == null)
@@ -23,109 +18,60 @@ namespace Microsoft.AspNet.Mvc
             }
 
             RouteKey = routeKey;
-            Comparer = StringComparer.OrdinalIgnoreCase; // Is this the right comparer for route values?
         }
 
+        /// <summary>
+        /// Initializes a <see cref="RouteDataActionConstraint"/> with a key and value, that are
+        /// required to make the action match.
+        /// </summary>
+        /// <param name="routeKey">The route key.</param>
+        /// <param name="routeValue">The route value.</param>
+        /// <remarks>
+        /// Passing a <see cref="string.Empty"/> or <see langword="null" /> to <paramref name="routeValue"/>
+        /// is a way to express that routing cannot produce a value for this key.
+        /// </remarks>
         public RouteDataActionConstraint(string routeKey, string routeValue)
             : this(routeKey)
         {
+            RouteValue = routeValue ?? string.Empty;
+
             if (string.IsNullOrEmpty(routeValue))
             {
-                throw new ArgumentNullException("routeValue");
+                KeyHandling = RouteKeyHandling.DenyKey;
             }
-
+            else
-            RouteValue = routeValue;
-            KeyHandling = RouteKeyHandling.RequireKey;
-        }
-
-        public RouteDataActionConstraint(string routeKey, RouteKeyHandling keyHandling)
-            : this(routeKey)
-        {
-            switch (keyHandling)
             {
-                case RouteKeyHandling.CatchAll:
+                KeyHandling = RouteKeyHandling.RequireKey;
-                case RouteKeyHandling.DenyKey:
-                case RouteKeyHandling.RequireKey:
-                    KeyHandling = keyHandling;
-                    break;
-                default:
-#if ASPNET50
-                    throw new InvalidEnumArgumentException("keyHandling", (int)keyHandling, typeof (RouteKeyHandling));
-#else
-                    throw new ArgumentOutOfRangeException("keyHandling");
-#endif
             }
         }
 
+        /// <summary>
+        /// Create a catch all constraint for the given key.
+        /// </summary>
+        /// <param name="routeKey">Route key.</param>
+        /// <returns>a <see cref="RouteDataActionConstraint"/> that represents a catch all constraint.</returns>
+        public static RouteDataActionConstraint CreateCatchAll(string routeKey)
+        {
+            var c = new RouteDataActionConstraint(routeKey);
+            c.KeyHandling = RouteKeyHandling.CatchAll;
+            c.RouteValue = string.Empty;
+
+            return c;
+        }
+
+        /// <summary>
+        /// The route key this constraint matches against.
+        /// </summary>
         public string RouteKey { get; private set; }
+
+        /// <summary>
+        /// The route value this constraint matches against.
+        /// </summary>
         public string RouteValue { get; private set; }
+
+        /// <summary>
+        /// The key handling definition for this constraint.
+        /// </summary>
         public RouteKeyHandling KeyHandling { get; private set; }
-
-        public IEqualityComparer Comparer
-        {
-            get { return _comparer; }
-            set
-            {
-                if (value == null)
-                {
-                    throw new ArgumentNullException("value");
-                }
-
-                _comparer = value;
-            }
-        }
-
-        public bool Accept([NotNull] IDictionary<string, object> routeValues)
-        {
-            object value;
-            switch (KeyHandling)
-            {
-                case RouteKeyHandling.CatchAll:
-                    return routeValues.ContainsKey(RouteKey);
-
-                case RouteKeyHandling.DenyKey:
-                    // Routing considers a null or empty string to also be the lack of a value
-                    if (!routeValues.TryGetValue(RouteKey, out value) || value == null)
-                    {
-                        return true;
-                    }
-
-                    var stringValue = value as string;
-                    if (stringValue != null && stringValue.Length == 0)
-                    {
-                        return true;
-                    }
-
-                    return false;
-
-                case RouteKeyHandling.RequireKey:
-                    if (routeValues.TryGetValue(RouteKey, out value))
-                    {
-                        return Comparer.Equals(value, RouteValue);
-                    }
-                    else
-                    {
-                        return false;
-                    }
-
-                default:
-                    Debug.Fail("Unexpected routeValue");
-                    return false;
-            }
-        }
-
-        public bool Accept([NotNull] RouteContext context)
-        {
-            var routeValues = context.RouteData.Values;
-            if (routeValues == null)
-            {
-                throw new ArgumentException(Resources.FormatPropertyOfTypeCannotBeNull(
-                        "Values",
-                        typeof(RouteData)),
-                    "context");
-            }
-
-            return Accept(routeValues);
-        }
     }
 }

src/Microsoft.AspNet.Mvc.Core/RouteKeyHandling.cs

@@ -3,8 +3,6 @@
 
 namespace Microsoft.AspNet.Mvc
 {
-    // This needs more thought, the intent is that we would be able to cache over this constraint
-    // without running the accept method.
     public enum RouteKeyHandling
     {
         /// <summary>
@@ -19,6 +17,8 @@ namespace Microsoft.AspNet.Mvc
 
         /// <summary>
         /// Requires that the key will be in the route values, but ignore the content.
+        /// Constraints with this value are considered less important than ones with 
+        /// <see cref="RequireKey"/>
         /// </summary>
         CatchAll,
     }

test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionSelectorTests.cs

@@ -501,7 +501,7 @@ namespace Microsoft.AspNet.Mvc
 
             actions[0].RouteConstraints.Add(new RouteDataActionConstraint("country", "CA"));
             actions[1].RouteConstraints.Add(new RouteDataActionConstraint("country", "US"));
-            actions[2].RouteConstraints.Add(new RouteDataActionConstraint("country", RouteKeyHandling.CatchAll));
+            actions[2].RouteConstraints.Add(RouteDataActionConstraint.CreateCatchAll("country"));
 
             var selector = CreateSelector(actions);
             var context = CreateRouteContext("GET");
@@ -530,7 +530,7 @@ namespace Microsoft.AspNet.Mvc
 
             actions[0].RouteConstraints.Add(new RouteDataActionConstraint("country", "CA"));
             actions[1].RouteConstraints.Add(new RouteDataActionConstraint("country", "US"));
-            actions[2].RouteConstraints.Add(new RouteDataActionConstraint("country", RouteKeyHandling.CatchAll));
+            actions[2].RouteConstraints.Add(RouteDataActionConstraint.CreateCatchAll("country"));
 
             var selector = CreateSelector(actions);
             var context = CreateRouteContext("GET");
@@ -559,7 +559,7 @@ namespace Microsoft.AspNet.Mvc
 
             actions[0].RouteConstraints.Add(new RouteDataActionConstraint("country", "CA"));
             actions[1].RouteConstraints.Add(new RouteDataActionConstraint("country", "US"));
-            actions[2].RouteConstraints.Add(new RouteDataActionConstraint("country", RouteKeyHandling.CatchAll));
+            actions[2].RouteConstraints.Add(RouteDataActionConstraint.CreateCatchAll("country"));
 
             var selector = CreateSelector(actions);
             var context = CreateRouteContext("GET");
@@ -635,11 +635,13 @@ namespace Microsoft.AspNet.Mvc
             string controller,
             string action)
         {
+            var comparer = new RouteValueEqualityComparer();
+
             return
                 actions
-                .Where(a => a.RouteConstraints.Any(c => c.RouteKey == "area" && c.Comparer.Equals(c.RouteValue, area)))
+                .Where(a => a.RouteConstraints.Any(c => c.RouteKey == "area" && comparer.Equals(c.RouteValue, area)))
-                .Where(a => a.RouteConstraints.Any(c => c.RouteKey == "controller" && c.Comparer.Equals(c.RouteValue, controller)))
+                .Where(a => a.RouteConstraints.Any(c => c.RouteKey == "controller" && comparer.Equals(c.RouteValue, controller)))
-                .Where(a => a.RouteConstraints.Any(c => c.RouteKey == "action" && c.Comparer.Equals(c.RouteValue, action)));
+                .Where(a => a.RouteConstraints.Any(c => c.RouteKey == "action" && comparer.Equals(c.RouteValue, action)));
         }
 
         private static DefaultActionSelector CreateSelector(IReadOnlyList<ActionDescriptor> actions, ILoggerFactory loggerFactory = null)
@@ -714,17 +716,17 @@ namespace Microsoft.AspNet.Mvc
 
             actionDescriptor.RouteConstraints.Add(
                 area == null ?
-                new RouteDataActionConstraint("area", RouteKeyHandling.DenyKey) :
+                new RouteDataActionConstraint("area", null) :
                 new RouteDataActionConstraint("area", area));
 
             actionDescriptor.RouteConstraints.Add(
                 controller == null ?
-                new RouteDataActionConstraint("controller", RouteKeyHandling.DenyKey) :
+                new RouteDataActionConstraint("controller", null) :
                 new RouteDataActionConstraint("controller", controller));
 
             actionDescriptor.RouteConstraints.Add(
                 action == null ?
-                new RouteDataActionConstraint("action", RouteKeyHandling.DenyKey) :
+                new RouteDataActionConstraint("action", null) :
                 new RouteDataActionConstraint("action", action));
 
             return actionDescriptor;

test/Microsoft.AspNet.Mvc.Core.Test/KnownRouteValueConstraintTests.cs

@@ -178,17 +178,17 @@ namespace Microsoft.AspNet.Routing.Tests
 
             actionDescriptor.RouteConstraints.Add(
                 area == null ?
-                new RouteDataActionConstraint("area", RouteKeyHandling.DenyKey) :
+                new RouteDataActionConstraint("area", null) :
                 new RouteDataActionConstraint("area", area));
 
             actionDescriptor.RouteConstraints.Add(
                 controller == null ?
-                new RouteDataActionConstraint("controller", RouteKeyHandling.DenyKey) :
+                new RouteDataActionConstraint("controller", null) :
                 new RouteDataActionConstraint("controller", controller));
 
             actionDescriptor.RouteConstraints.Add(
                 action == null ?
-                new RouteDataActionConstraint("action", RouteKeyHandling.DenyKey) :
+                new RouteDataActionConstraint("action", null) :
                 new RouteDataActionConstraint("action", action));
 
             return actionDescriptor;

test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs

@@ -389,7 +389,7 @@ namespace Microsoft.AspNet.Mvc.Test
                 descriptorWithoutConstraint.RouteConstraints,
                 c =>
                     c.RouteKey == "key" &&
-                    c.RouteValue == null &&
+                    c.RouteValue == string.Empty &&
                     c.KeyHandling == RouteKeyHandling.DenyKey);
         }
 

test/Microsoft.AspNet.Mvc.Core.Test/RouteDataActionConstraintTest.cs

@@ -0,0 +1,47 @@
+using Xunit;
+
+namespace Microsoft.AspNet.Mvc.Core
+{
+    public class RouteDataActionConstraintTest
+    {
+        [Fact]
+        public void RouteDataActionConstraint_DenyKeyByPassingEmptyString()
+        {
+            var routeDataConstraint = new RouteDataActionConstraint("key", string.Empty);
+
+            Assert.Equal(routeDataConstraint.RouteKey, "key");
+            Assert.Equal(routeDataConstraint.KeyHandling, RouteKeyHandling.DenyKey);
+            Assert.Equal(routeDataConstraint.RouteValue, string.Empty);
+        }
+
+        [Fact]
+        public void RouteDataActionConstraint_DenyKeyByPassingNull()
+        {
+            var routeDataConstraint = new RouteDataActionConstraint("key", null);
+
+            Assert.Equal(routeDataConstraint.RouteKey, "key");
+            Assert.Equal(routeDataConstraint.KeyHandling, RouteKeyHandling.DenyKey);
+            Assert.Equal(routeDataConstraint.RouteValue, string.Empty);
+        }
+
+        [Fact]
+        public void RouteDataActionConstraint_RequireKeyByPassingNonEmpty()
+        {
+            var routeDataConstraint = new RouteDataActionConstraint("key", "value");
+
+            Assert.Equal(routeDataConstraint.RouteKey, "key");
+            Assert.Equal(routeDataConstraint.KeyHandling, RouteKeyHandling.RequireKey);
+            Assert.Equal(routeDataConstraint.RouteValue, "value");
+        }
+
+        [Fact]
+        public void RouteDataActionConstraint_CatchAll()
+        {
+            var routeDataConstraint = RouteDataActionConstraint.CreateCatchAll("key");
+
+            Assert.Equal(routeDataConstraint.RouteKey, "key");
+            Assert.Equal(routeDataConstraint.KeyHandling, RouteKeyHandling.CatchAll);
+            Assert.Equal(routeDataConstraint.RouteValue, string.Empty);
+        }
+    }
+}