From 144a4b5921a277714c238cb293c90bf59c3c6723 Mon Sep 17 00:00:00 2001 From: YishaiGalatzer Date: Mon, 6 Oct 2014 16:13:26 -0700 Subject: [PATCH] Make RouteDataActionConstraint be POCO only --- .../DefaultActionSelector.cs | 9 +- .../ReflectedActionDescriptorProvider.cs | 14 +- .../RouteDataActionConstraint.cs | 140 ++++++------------ .../RouteKeyHandling.cs | 4 +- .../DefaultActionSelectorTests.cs | 20 +-- .../KnownRouteValueConstraintTests.cs | 6 +- .../ReflectedActionDescriptorProviderTests.cs | 2 +- .../RouteDataActionConstraintTest.cs | 47 ++++++ 8 files changed, 117 insertions(+), 125 deletions(-) create mode 100644 test/Microsoft.AspNet.Mvc.Core.Test/RouteDataActionConstraintTest.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs index 9146685172..d240508079 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs @@ -233,13 +233,10 @@ namespace Microsoft.AspNet.Mvc return false; } - var actions = - GetActions().Where( - action => - action.RouteConstraints == null || - action.RouteConstraints.All(constraint => constraint.Accept(context.ProvidedValues))); + var tree = _decisionTreeProvider.DecisionTree; + var matchingRouteConstraints = tree.Select(context.ProvidedValues); - return actions.Any(); + return matchingRouteConstraints.Count > 0; } private IReadOnlyList GetActions() diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs index 05a7cb7ba6..c95fc0d2e4 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs +++ b/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( - constraintAttribute.RouteKey, - constraintAttribute.RouteKeyHandling)); + actionDescriptor.RouteConstraints.Add( + RouteDataActionConstraint.CreateCatchAll( + constraintAttribute.RouteKey)); } else { @@ -741,7 +741,7 @@ namespace Microsoft.AspNet.Mvc { actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint( key, - RouteKeyHandling.DenyKey)); + string.Empty)); } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/RouteDataActionConstraint.cs b/src/Microsoft.AspNet.Mvc.Core/RouteDataActionConstraint.cs index 61212c2a34..4019a3fd1b 100644 --- a/src/Microsoft.AspNet.Mvc.Core/RouteDataActionConstraint.cs +++ b/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 { + /// + /// Constraints an action to a route key and value. + /// 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? } + /// + /// Initializes a with a key and value, that are + /// required to make the action match. + /// + /// The route key. + /// The route value. + /// + /// Passing a or to + /// is a way to express that routing cannot produce a value for this key. + /// public RouteDataActionConstraint(string routeKey, string routeValue) : this(routeKey) { + RouteValue = routeValue ?? string.Empty; + if (string.IsNullOrEmpty(routeValue)) { - throw new ArgumentNullException("routeValue"); + KeyHandling = RouteKeyHandling.DenyKey; } - - RouteValue = routeValue; - KeyHandling = RouteKeyHandling.RequireKey; - } - - public RouteDataActionConstraint(string routeKey, RouteKeyHandling keyHandling) - : this(routeKey) - { - switch (keyHandling) + else { - case RouteKeyHandling.CatchAll: - 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 + KeyHandling = RouteKeyHandling.RequireKey; } } + /// + /// Create a catch all constraint for the given key. + /// + /// Route key. + /// a that represents a catch all constraint. + public static RouteDataActionConstraint CreateCatchAll(string routeKey) + { + var c = new RouteDataActionConstraint(routeKey); + c.KeyHandling = RouteKeyHandling.CatchAll; + c.RouteValue = string.Empty; + + return c; + } + + /// + /// The route key this constraint matches against. + /// public string RouteKey { get; private set; } + + /// + /// The route value this constraint matches against. + /// public string RouteValue { get; private set; } + + /// + /// The key handling definition for this constraint. + /// 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 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); - } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/RouteKeyHandling.cs b/src/Microsoft.AspNet.Mvc.Core/RouteKeyHandling.cs index b92229a754..6eb57cb81a 100644 --- a/src/Microsoft.AspNet.Mvc.Core/RouteKeyHandling.cs +++ b/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 { /// @@ -19,6 +17,8 @@ namespace Microsoft.AspNet.Mvc /// /// 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 + /// /// CatchAll, } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionSelectorTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionSelectorTests.cs index 8a1c1755b4..ba9c98d066 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionSelectorTests.cs +++ b/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 == "controller" && c.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 == "area" && comparer.Equals(c.RouteValue, area))) + .Where(a => a.RouteConstraints.Any(c => c.RouteKey == "controller" && comparer.Equals(c.RouteValue, controller))) + .Where(a => a.RouteConstraints.Any(c => c.RouteKey == "action" && comparer.Equals(c.RouteValue, action))); } private static DefaultActionSelector CreateSelector(IReadOnlyList 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; diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/KnownRouteValueConstraintTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/KnownRouteValueConstraintTests.cs index b21a79a8b9..91ba456c89 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/KnownRouteValueConstraintTests.cs +++ b/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; diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs index 4f1ccc3dea..1e64f1bc9f 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs +++ b/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); } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/RouteDataActionConstraintTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/RouteDataActionConstraintTest.cs new file mode 100644 index 0000000000..d8bd5b2dd0 --- /dev/null +++ b/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); + } + } +} \ No newline at end of file