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.
This commit is contained in:
Ryan Nowak 2014-04-29 17:03:59 -07:00
parent 20c8dece7b
commit 8ae1865740
2 changed files with 75 additions and 6 deletions

View File

@ -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) if (isApplicable)
{ {
applicableCandiates.Add(candidate); applicableCandiates.Add(candidate);
} }
} }
var mostParametersSatisfied = applicableCandiates.GroupBy(c => c.FoundParameters).OrderByDescending(g => g.Key).First(); if (applicableCandiates.Count == 0)
if (mostParametersSatisfied == null)
{ {
return null; 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) if (fewestOptionalParameters.Length > 1)
{ {
throw new InvalidOperationException("The actions are ambiguious."); throw new InvalidOperationException("The actions are ambiguious.");
@ -322,6 +344,9 @@ namespace Microsoft.AspNet.Mvc
{ {
public ActionDescriptor Action { get; set; } 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 FoundParameters { get; set; }
public int FoundOptionalParameters { get; set; } public int FoundOptionalParameters { get; set; }

View File

@ -1,6 +1,7 @@
 
using System.Collections.Generic; using System.Collections.Generic;
using System.Linq; using System.Linq;
using System.Threading.Tasks;
using Microsoft.AspNet.Abstractions; using Microsoft.AspNet.Abstractions;
using Microsoft.AspNet.DependencyInjection; using Microsoft.AspNet.DependencyInjection;
using Microsoft.AspNet.Routing; using Microsoft.AspNet.Routing;
@ -193,6 +194,36 @@ namespace Microsoft.AspNet.Mvc.Core.Test
Assert.False(isValid); 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() private static ActionDescriptor[] GetActions()
{ {
return new ActionDescriptor[] return new ActionDescriptor[]
@ -232,6 +263,9 @@ namespace Microsoft.AspNet.Mvc.Core.Test
.Callback<ActionDescriptorProviderContext>(c => c.Results.AddRange(actions)); .Callback<ActionDescriptorProviderContext>(c => c.Results.AddRange(actions));
var bindingProvider = new Mock<IActionBindingContextProvider>(MockBehavior.Strict); 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); 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) private static VirtualPathContext CreateContext(object routeValues, object ambientValues)
{ {
var httpContext = new Mock<HttpContext>(MockBehavior.Strict);
return new VirtualPathContext( return new VirtualPathContext(
httpContext.Object, new Mock<HttpContext>(MockBehavior.Strict).Object,
new RouteValueDictionary(ambientValues), new RouteValueDictionary(ambientValues),
new RouteValueDictionary(routeValues)); 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) private static ActionDescriptor CreateAction(string area, string controller, string action)
{ {
var actionDescriptor = new ActionDescriptor() var actionDescriptor = new ActionDescriptor()