[Fixes #1035] RouteGroupConstraint should only be added once for non attribute routed actions

1. Changed ReflectedActionDescriptorProvider to add RouteGroupConstraint only once
   for non attribute routed actions.

2. Added tests to cover the scenario.
This commit is contained in:
jacalvar 2014-08-18 18:48:04 -07:00
parent 6f0fa67170
commit 2eec0bbf6d
3 changed files with 152 additions and 41 deletions

View File

@ -289,41 +289,44 @@ namespace Microsoft.AspNet.Mvc
actions.Add(actionDescriptor);
}
}
foreach (var actionDescriptor in actions)
foreach (var actionDescriptor in actions)
{
if (actionDescriptor.AttributeRouteInfo == null ||
actionDescriptor.AttributeRouteInfo.Template == null)
{
// Any any attribute routes are in use, then non-attribute-routed ADs can't be selected
// when a route group returned by the route.
if (hasAttributeRoutes)
{
actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint(
AttributeRouting.RouteGroupKey,
RouteKeyHandling.DenyKey));
}
foreach (var key in removalConstraints)
{
if (actionDescriptor.AttributeRouteInfo == null ||
actionDescriptor.AttributeRouteInfo.Template == null)
if (!HasConstraint(actionDescriptor.RouteConstraints, key))
{
// Any any attribute routes are in use, then non-attribute-routed ADs can't be selected
// when a route group returned by the route.
if (hasAttributeRoutes)
{
actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint(
AttributeRouting.RouteGroupKey,
RouteKeyHandling.DenyKey));
}
if (!HasConstraint(actionDescriptor.RouteConstraints, key))
{
actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint(
key,
RouteKeyHandling.DenyKey));
}
actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint(
key,
RouteKeyHandling.DenyKey));
}
else
}
}
else
{
// We still want to add a 'null' for any constraint with DenyKey so that link generation
// works properly.
//
// Consider an action like { area = "", controller = "Home", action = "Index" }. Even if
// it's attribute routed, it needs to know that area must be null to generate a link.
foreach (var key in removalConstraints)
{
if (!actionDescriptor.RouteValueDefaults.ContainsKey(key))
{
// We still want to add a 'null' for any constraint with DenyKey so that link generation
// works properly.
//
// Consider an action like { area = "", controller = "Home", action = "Index" }. Even if
// it's attribute routed, it needs to know that area must be null to generate a link.
if (!actionDescriptor.RouteValueDefaults.ContainsKey(key))
{
actionDescriptor.RouteValueDefaults.Add(key, null);
}
actionDescriptor.RouteValueDefaults.Add(key, null);
}
}
}

View File

@ -168,19 +168,15 @@ namespace Microsoft.AspNet.Mvc.Routing
// would throw.
#if NET45
throw new InvalidEnumArgumentException(
"item",
(int)constraint.KeyHandling,
"item",
(int)constraint.KeyHandling,
typeof(RouteKeyHandling));
#else
throw new ArgumentOutOfRangeException("item");
#endif
}
// Workaround for Javier's cool bug.
if (!results.ContainsKey(constraint.RouteKey))
{
results.Add(constraint.RouteKey, value);
}
results.Add(constraint.RouteKey, value);
}
}

View File

@ -84,16 +84,16 @@ namespace Microsoft.AspNet.Mvc.Test
typeof(BlockNonAttributedActionsController).GetTypeInfo()).ToArray();
var descriptorWithoutConstraint = Assert.Single(
descriptors,
descriptors,
ad => ad.RouteConstraints.Any(
c => c.RouteKey == "key" && c.KeyHandling == RouteKeyHandling.DenyKey));
var descriptorWithConstraint = Assert.Single(
descriptors,
ad => ad.RouteConstraints.Any(
c =>
c.KeyHandling == RouteKeyHandling.RequireKey &&
c.RouteKey == "key" &&
c =>
c.KeyHandling == RouteKeyHandling.RequireKey &&
c.RouteKey == "key" &&
c.RouteValue == "value"));
// Assert
@ -265,6 +265,71 @@ namespace Microsoft.AspNet.Mvc.Test
Assert.Equal(expectedMessage, ex.Message);
}
[Fact]
public void AttributeRouting_RouteGroupConstraint_IsAddedOnceForNonAttributeRoutes()
{
// Arrange
var provider = GetProvider(
typeof(MixedAttributeRouteController).GetTypeInfo(),
typeof(ConstrainedController).GetTypeInfo());
// Act
var actionDescriptors = provider.GetDescriptors();
// Assert
Assert.NotNull(actionDescriptors);
Assert.Equal(4, actionDescriptors.Count());
foreach (var actionDescriptor in actionDescriptors.Where(ad => ad.AttributeRouteInfo == null))
{
Assert.Equal(6, actionDescriptor.RouteConstraints.Count);
var routeGroupConstraint = Assert.Single(actionDescriptor.RouteConstraints,
rc => rc.RouteKey.Equals(AttributeRouting.RouteGroupKey));
Assert.Equal(RouteKeyHandling.DenyKey, routeGroupConstraint.KeyHandling);
}
}
[Fact]
public void AttributeRouting_AddsDefaultRouteValues_ForAttributeRoutedActions()
{
// Arrange
var provider = GetProvider(
typeof(MixedAttributeRouteController).GetTypeInfo(),
typeof(ConstrainedController).GetTypeInfo());
// Act
var actionDescriptors = provider.GetDescriptors();
// Assert
Assert.NotNull(actionDescriptors);
Assert.Equal(4, actionDescriptors.Count());
var indexAction = Assert.Single(actionDescriptors, ad => ad.Name.Equals("Index"));
Assert.Equal(1, indexAction.RouteConstraints.Count);
var routeGroupConstraint = Assert.Single(indexAction.RouteConstraints, rc => rc.RouteKey.Equals(AttributeRouting.RouteGroupKey));
Assert.Equal(RouteKeyHandling.RequireKey, routeGroupConstraint.KeyHandling);
Assert.NotNull(routeGroupConstraint.RouteValue);
Assert.Equal(5, indexAction.RouteValueDefaults.Count);
var controllerDefault = Assert.Single(indexAction.RouteValueDefaults, rd => rd.Key.Equals("controller", StringComparison.OrdinalIgnoreCase));
Assert.Equal("MixedAttributeRoute", controllerDefault.Value);
var actionDefault = Assert.Single(indexAction.RouteValueDefaults, rd => rd.Key.Equals("action", StringComparison.OrdinalIgnoreCase));
Assert.Equal("Index", actionDefault.Value);
var areaDefault = Assert.Single(indexAction.RouteValueDefaults, rd => rd.Key.Equals("area", StringComparison.OrdinalIgnoreCase));
Assert.Equal("Home", areaDefault.Value);
var myRouteConstraintDefault = Assert.Single(indexAction.RouteValueDefaults, rd => rd.Key.Equals("key", StringComparison.OrdinalIgnoreCase));
Assert.Null(myRouteConstraintDefault.Value);
var anotherRouteConstraint = Assert.Single(indexAction.RouteValueDefaults, rd => rd.Key.Equals("second", StringComparison.OrdinalIgnoreCase));
Assert.Null(anotherRouteConstraint.Value);
}
[Fact]
public void AttributeRouting_TokenReplacement_CaseInsensitive()
{
@ -318,7 +383,7 @@ namespace Microsoft.AspNet.Mvc.Test
}
private ReflectedActionDescriptorProvider GetProvider(
TypeInfo controllerTypeInfo,
TypeInfo controllerTypeInfo,
IEnumerable<IFilter> filters = null)
{
var conventions = new StaticActionDiscoveryConventions(controllerTypeInfo);
@ -338,6 +403,26 @@ namespace Microsoft.AspNet.Mvc.Test
return provider;
}
private ReflectedActionDescriptorProvider GetProvider(
params TypeInfo[] controllerTypeInfo)
{
var conventions = new StaticActionDiscoveryConventions(controllerTypeInfo);
var assemblyProvider = new Mock<IControllerAssemblyProvider>();
assemblyProvider
.SetupGet(ap => ap.CandidateAssemblies)
.Returns(new Assembly[] { controllerTypeInfo.First().Assembly });
var provider = new ReflectedActionDescriptorProvider(
assemblyProvider.Object,
conventions,
Enumerable.Empty<IFilter>(),
new MockMvcOptionsAccessor(),
Mock.Of<IInlineConstraintResolver>());
return provider;
}
private IEnumerable<ActionDescriptor> GetDescriptors(params TypeInfo[] controllerTypeInfos)
{
var conventions = new StaticActionDiscoveryConventions(controllerTypeInfos);
@ -353,7 +438,7 @@ namespace Microsoft.AspNet.Mvc.Test
null,
new MockMvcOptionsAccessor(),
null);
return provider.GetDescriptors();
}
@ -386,6 +471,14 @@ namespace Microsoft.AspNet.Mvc.Test
}
}
public class MySecondRouteConstraintAttribute : RouteConstraintAttribute
{
public MySecondRouteConstraintAttribute(bool blockNonAttributedActions)
: base("second", "value", blockNonAttributedActions)
{
}
}
[MyRouteConstraintAttribute(blockNonAttributedActions: true)]
private class BlockNonAttributedActionsController
{
@ -458,5 +551,24 @@ namespace Microsoft.AspNet.Mvc.Test
[HttpGet("stub/Action1")]
public void Action2() { }
}
[Area("Home")]
private class MixedAttributeRouteController
{
[HttpGet("Index")]
public void Index() { }
[HttpGet("Edit")]
public void Edit() { }
public void AnotherNonAttributedAction() { }
}
[MyRouteConstraint(blockNonAttributedActions: true)]
[MySecondRouteConstraint(blockNonAttributedActions: true)]
private class ConstrainedController
{
public void ConstrainedNonAttributedAction() { }
}
}
}