From 8ae18657408a4b4c28a6af80c657d8687e540d12 Mon Sep 17 00:00:00 2001
From: Ryan Nowak <nowakra@gmail.com>
Date: Tue, 29 Apr 2014 17:03:59 -0700
Subject: [PATCH] Fix for part of #339

Treat actions with a dynamic constraint or httpmethod constraint as
'better' than those with just route constraints.

This is the first criteria used to filter down the 'best' match, so it's
applied before parameter-arity.
---
 .../DefaultActionSelector.cs                  | 31 ++++++++++--
 .../DefaultActionSelectorTest.cs              | 50 +++++++++++++++++--
 2 files changed, 75 insertions(+), 6 deletions(-)

diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs
index e0cc095b1d..967b36c308 100644
--- a/src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs
+++ b/src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs
@@ -93,19 +93,41 @@ namespace Microsoft.AspNet.Mvc
                     }
                 }
 
+                if (action.DynamicConstraints != null && action.DynamicConstraints.Any() ||
+                    action.MethodConstraints != null && action.MethodConstraints.Any())
+                {
+                    candidate.HasNonRouteConstraints = true;
+                }
+
                 if (isApplicable)
                 {
                     applicableCandiates.Add(candidate);
                 }
             }
 
-            var mostParametersSatisfied = applicableCandiates.GroupBy(c => c.FoundParameters).OrderByDescending(g => g.Key).First();
-            if (mostParametersSatisfied == null)
+            if (applicableCandiates.Count == 0)
             {
                 return null;
             }
 
-            var fewestOptionalParameters = mostParametersSatisfied.GroupBy(c => c.FoundOptionalParameters).OrderBy(g => g.Key).First().ToArray();
+            var bestByConstraints = 
+                applicableCandiates
+                .GroupBy(c => c.HasNonRouteConstraints ? 1 : 0)
+                .OrderByDescending(g => g.Key)
+                .First();
+
+            var mostParametersSatisfied = 
+                bestByConstraints
+                .GroupBy(c => c.FoundParameters)
+                .OrderByDescending(g => g.Key)
+                .First();
+
+            var fewestOptionalParameters = 
+                mostParametersSatisfied
+                .GroupBy(c => c.FoundOptionalParameters)
+                .OrderBy(g => g.Key).First()
+                .ToArray();
+
             if (fewestOptionalParameters.Length > 1)
             {
                 throw new InvalidOperationException("The actions are ambiguious.");
@@ -322,6 +344,9 @@ namespace Microsoft.AspNet.Mvc
         {
             public ActionDescriptor Action { get; set; }
 
+            // Actions with HTTP method constraints or dynamic constraints are better than those without.
+            public bool HasNonRouteConstraints { get; set; }
+
             public int FoundParameters { get; set; }
 
             public int FoundOptionalParameters { get; set; }
diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionSelectorTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionSelectorTest.cs
index 06c84cc120..b74338e774 100644
--- a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionSelectorTest.cs
+++ b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionSelectorTest.cs
@@ -1,6 +1,7 @@
 
 using System.Collections.Generic;
 using System.Linq;
+using System.Threading.Tasks;
 using Microsoft.AspNet.Abstractions;
 using Microsoft.AspNet.DependencyInjection;
 using Microsoft.AspNet.Routing;
@@ -193,6 +194,36 @@ namespace Microsoft.AspNet.Mvc.Core.Test
             Assert.False(isValid);
         }
 
+        [Fact]
+        public async Task SelectAsync_PrefersActionWithConstraints()
+        {
+            // Arrange
+            var actionWithConstraints = new ActionDescriptor()
+            {
+                MethodConstraints = new List<HttpMethodConstraint>()
+                {
+                    new HttpMethodConstraint(new string[] { "POST" }),
+                },
+                Parameters = new List<ParameterDescriptor>(),
+            };
+
+            var actionWithoutConstraints = new ActionDescriptor()
+            {
+                Parameters = new List<ParameterDescriptor>(),
+            };
+
+            var actions = new ActionDescriptor[] { actionWithConstraints, actionWithoutConstraints };
+
+            var selector = CreateSelector(actions);
+            var context = new RequestContext(CreateHttpContext("POST"), new Dictionary<string, object>());
+
+            // Act
+            var action = await selector.SelectAsync(context);
+
+            // Assert
+            Assert.Same(action, actionWithConstraints);
+        }
+
         private static ActionDescriptor[] GetActions()
         {
             return new ActionDescriptor[]
@@ -232,6 +263,9 @@ namespace Microsoft.AspNet.Mvc.Core.Test
                 .Callback<ActionDescriptorProviderContext>(c => c.Results.AddRange(actions));
 
             var bindingProvider = new Mock<IActionBindingContextProvider>(MockBehavior.Strict);
+            bindingProvider
+                .Setup(bp => bp.GetActionBindingContextAsync(It.IsAny<ActionContext>()))
+                .Returns(() => Task.FromResult<ActionBindingContext>(null));
 
             return new DefaultActionSelector(actionProvider.Object, bindingProvider.Object);
         }
@@ -243,14 +277,24 @@ namespace Microsoft.AspNet.Mvc.Core.Test
 
         private static VirtualPathContext CreateContext(object routeValues, object ambientValues)
         {
-            var httpContext = new Mock<HttpContext>(MockBehavior.Strict);
-
             return new VirtualPathContext(
-                httpContext.Object,
+                new Mock<HttpContext>(MockBehavior.Strict).Object,
                 new RouteValueDictionary(ambientValues),
                 new RouteValueDictionary(routeValues));
         }
 
+        private static HttpContext CreateHttpContext(string httpMethod)
+        {
+            var context = new Mock<HttpContext>(MockBehavior.Strict);
+
+            var request = new Mock<HttpRequest>(MockBehavior.Strict);
+            context.SetupGet(c => c.Request).Returns(request.Object);
+
+            request.SetupGet(r => r.Method).Returns(httpMethod);
+
+            return context.Object;
+        }
+
         private static ActionDescriptor CreateAction(string area, string controller, string action)
         {
             var actionDescriptor = new ActionDescriptor()