[Issue #730] Attribute Routing: Flesh out attributes - Part 1

1. Added a new AttributeRouteInfo class to store all the information for
   actions that are attribute routed.

2. Added a new ReflectedAttributeRouteModel class to store all the information
   related to attribute routes in the ReflectedApplicationModel.

3. Refactored ReflectedControllerModel and ReflectedActionModel to use ReflectedAttributeRouteModel
   instead of just the attribute route template.

4. Refactored ReflectedActionDescriptorProvider to use AttributeRouteInfo and ReflectedAttributeRouteModel
   instead of just the route template.

5. Added a CombineReflectedAttributeRouteModel method in ReflectedAttributeRouteModel that handles
   combining two ReflectedAttributeRouteModel instances.

6. Removed the AttributeRouteTemplate class and moved the methods for combining attribute routes to the
   ReflectedAttributeRouteModel class.

7. Added unit tests for ReflectedActionModel and  ReflectedControllerModel that
   cover the usage of AttributeRouteInfo.

8. Added unit tests for CombineReflectedAttributeRouteModel.
This commit is contained in:
Javier Calvarro Nelson 2014-07-19 23:24:40 -07:00
parent 26f98b481a
commit 21b1174d76
12 changed files with 237 additions and 76 deletions

View File

@ -3,6 +3,7 @@
using System;
using System.Collections.Generic;
using Microsoft.AspNet.Mvc.Routing;
namespace Microsoft.AspNet.Mvc
{
@ -17,15 +18,9 @@ namespace Microsoft.AspNet.Mvc
public List<RouteDataActionConstraint> RouteConstraints { get; set; }
/// <summary>
/// The set of route values that are added when this action is selected.
/// </summary>
public Dictionary<string, object> RouteValueDefaults { get; private set; }
public AttributeRouteInfo AttributeRouteInfo { get; set; }
/// <summary>
/// The attribute route template. May be null if the action has no attribute routes.
/// </summary>
public string AttributeRouteTemplate { get; set; }
public Dictionary<string, object> RouteValueDefaults { get; private set; }
public List<HttpMethodConstraint> MethodConstraints { get; set; }

View File

@ -146,6 +146,15 @@ namespace Microsoft.AspNet.Mvc
});
}
var combinedRoute = ReflectedAttributeRouteModel.CombineReflectedAttributeRouteModel(
controller.AttributeRouteModel,
action.AttributeRouteModel);
var attributeRouteInfo = combinedRoute == null ? null : new AttributeRouteInfo()
{
Template = combinedRoute.Template
};
var actionDescriptor = new ReflectedActionDescriptor()
{
Name = action.ActionName,
@ -153,6 +162,7 @@ namespace Microsoft.AspNet.Mvc
MethodInfo = action.ActionMethod,
Parameters = parameterDescriptors,
RouteConstraints = new List<RouteDataActionConstraint>(),
AttributeRouteInfo = attributeRouteInfo
};
actionDescriptor.DisplayName = string.Format(
@ -202,11 +212,8 @@ namespace Microsoft.AspNet.Mvc
}
}
var templateText = AttributeRouteTemplate.Combine(
controller.RouteTemplate,
action.RouteTemplate);
if (templateText != null)
if (actionDescriptor.AttributeRouteInfo != null &&
actionDescriptor.AttributeRouteInfo.Template != null)
{
// An attribute routed action will ignore conventional routed constraints. We still
// want to provide these values as ambient values.
@ -217,9 +224,10 @@ namespace Microsoft.AspNet.Mvc
// Replaces tokens like [controller]/[action] in the route template with the actual values
// for this action.
var templateText = actionDescriptor.AttributeRouteInfo.Template;
try
{
templateText = AttributeRouteTemplate.ReplaceTokens(
templateText = ReflectedAttributeRouteModel.ReplaceTokens(
templateText,
actionDescriptor.RouteValueDefaults);
}
@ -233,7 +241,7 @@ namespace Microsoft.AspNet.Mvc
routeTemplateErrors.Add(message);
}
actionDescriptor.AttributeRouteTemplate = templateText;
actionDescriptor.AttributeRouteInfo.Template = templateText;
// An attribute routed action is matched by its 'route group' which identifies all equivalent
// actions.
@ -261,40 +269,41 @@ namespace Microsoft.AspNet.Mvc
actions.Add(actionDescriptor);
}
}
foreach (var actionDescriptor in actions)
{
foreach (var key in removalConstraints)
foreach (var actionDescriptor in actions)
{
if (actionDescriptor.AttributeRouteTemplate == null)
foreach (var key in removalConstraints)
{
// 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 (routeGroupsByTemplate.Any())
if (actionDescriptor.AttributeRouteInfo == null ||
actionDescriptor.AttributeRouteInfo.Template == null)
{
actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint(
AttributeRouting.RouteGroupKey,
RouteKeyHandling.DenyKey));
}
// 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 (routeGroupsByTemplate.Any())
{
actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint(
AttributeRouting.RouteGroupKey,
RouteKeyHandling.DenyKey));
}
if (!HasConstraint(actionDescriptor.RouteConstraints, key))
{
actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint(
key,
RouteKeyHandling.DenyKey));
if (!HasConstraint(actionDescriptor.RouteConstraints, key))
{
actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint(
key,
RouteKeyHandling.DenyKey));
}
}
}
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.
if (!actionDescriptor.RouteValueDefaults.ContainsKey(key))
else
{
actionDescriptor.RouteValueDefaults.Add(key, null);
// 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);
}
}
}
}

View File

@ -23,7 +23,7 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder
var routeTemplateAttribute = Attributes.OfType<IRouteTemplateProvider>().FirstOrDefault();
if (routeTemplateAttribute != null)
{
RouteTemplate = routeTemplateAttribute.Template;
AttributeRouteModel = new ReflectedAttributeRouteModel(routeTemplateAttribute);
}
HttpMethods = new List<string>();
@ -44,6 +44,6 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder
public List<ReflectedParameterModel> Parameters { get; private set; }
public string RouteTemplate { get; set; }
public ReflectedAttributeRouteModel AttributeRouteModel { get; set; }
}
}

View File

@ -5,21 +5,62 @@ using System;
using System.Collections.Generic;
using System.Text;
using Microsoft.AspNet.Mvc.Core;
using Microsoft.AspNet.Mvc.Routing;
namespace Microsoft.AspNet.Mvc.Routing
namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder
{
/// <summary>
/// Functionality supporting route templates for attribute routes.
/// </summary>
public static class AttributeRouteTemplate
public class ReflectedAttributeRouteModel
{
private static readonly ReflectedAttributeRouteModel _default = new ReflectedAttributeRouteModel();
public ReflectedAttributeRouteModel()
{
}
public ReflectedAttributeRouteModel([NotNull] IRouteTemplateProvider templateProvider)
{
Template = templateProvider.Template;
}
public string Template { get; set; }
/// <summary>
/// Combines two <see cref="ReflectedAttributeRouteModel"/> instances and returns
/// a new <see cref="ReflectedAttributeRouteModel"/> instance with the result.
/// </summary>
/// <param name="left">The left <see cref="ReflectedAttributeRouteModel"/>.</param>
/// <param name="right">The right <see cref="ReflectedAttributeRouteModel"/>.</param>
/// <returns>A new instance of <see cref="ReflectedAttributeRouteModel"/> that represents the
/// combination of the two <see cref="ReflectedAttributeRouteModel"/> instances or <c>null</c> if both
/// parameters are <c>null</c>.</returns>
public static ReflectedAttributeRouteModel CombineReflectedAttributeRouteModel(
ReflectedAttributeRouteModel left,
ReflectedAttributeRouteModel right)
{
left = left ?? _default;
right = right ?? _default;
var template = CombineTemplates(left.Template, right.Template);
// The action is not attribute routed.
if (template == null)
{
return null;
}
return new ReflectedAttributeRouteModel()
{
Template = template
};
}
/// <summary>
/// Combines attribute routing templates.
/// </summary>
/// <param name="left">The left template.</param>
/// <param name="right">The right template.</param>
/// <returns>A combined template.</returns>
public static string Combine(string left, string right)
public static string CombineTemplates(string left, string right)
{
var result = CombineCore(left, right);
return CleanTemplate(result);

View File

@ -27,7 +27,7 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder
var routeTemplateAttribute = Attributes.OfType<IRouteTemplateProvider>().FirstOrDefault();
if (routeTemplateAttribute != null)
{
RouteTemplate = routeTemplateAttribute.Template;
AttributeRouteModel = new ReflectedAttributeRouteModel(routeTemplateAttribute);
}
ControllerName = controllerType.Name.EndsWith("Controller", StringComparison.Ordinal)
@ -47,6 +47,6 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder
public List<RouteConstraintAttribute> RouteConstraints { get; private set; }
public string RouteTemplate { get; set; }
public ReflectedAttributeRouteModel AttributeRouteModel { get; set; }
}
}

View File

@ -0,0 +1,16 @@
// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
namespace Microsoft.AspNet.Mvc.Routing
{
/// <summary>
/// Represents the routing information for an action that is attribute routed.
/// </summary>
public class AttributeRouteInfo
{
/// <summary>
/// The route template. May be null if the action has no attribute routes.
/// </summary>
public string Template { get; set; }
}
}

View File

@ -115,7 +115,9 @@ namespace Microsoft.AspNet.Mvc.Routing
// of memory, so sharing is worthwhile.
var templateCache = new Dictionary<string, RouteTemplate>(StringComparer.OrdinalIgnoreCase);
foreach (var action in actions.Where(a => a.AttributeRouteTemplate != null))
var attributeRoutedActions = actions.Where(a => a.AttributeRouteInfo != null &&
a.AttributeRouteInfo.Template != null);
foreach (var action in attributeRoutedActions)
{
var routeInfo = GetRouteInfo(constraintResolver, templateCache, action);
if (routeInfo.ErrorMessage == null)
@ -169,17 +171,17 @@ namespace Microsoft.AspNet.Mvc.Routing
{
ActionDescriptor = action,
RouteGroup = constraint.RouteValue,
RouteTemplate = action.AttributeRouteTemplate,
RouteTemplate = action.AttributeRouteInfo.Template,
};
try
{
RouteTemplate parsedTemplate;
if (!templateCache.TryGetValue(action.AttributeRouteTemplate, out parsedTemplate))
if (!templateCache.TryGetValue(action.AttributeRouteInfo.Template, out parsedTemplate))
{
// Parsing with throw if the template is invalid.
parsedTemplate = TemplateParser.Parse(action.AttributeRouteTemplate, constraintResolver);
templateCache.Add(action.AttributeRouteTemplate, parsedTemplate);
parsedTemplate = TemplateParser.Parse(action.AttributeRouteInfo.Template, constraintResolver);
templateCache.Add(action.AttributeRouteInfo.Template, parsedTemplate);
}
routeInfo.ParsedTemplate = parsedTemplate;

View File

@ -220,10 +220,10 @@ namespace Microsoft.AspNet.Mvc.Test
// Assert
var controller = Assert.Single(model.Controllers);
Assert.Equal("api/Token/[key]/[controller]", controller.RouteTemplate);
Assert.Equal("api/Token/[key]/[controller]", controller.AttributeRouteModel.Template);
var action = Assert.Single(controller.Actions);
Assert.Equal("stub/[action]", action.RouteTemplate);
Assert.Equal("stub/[action]", action.AttributeRouteModel.Template);
}
[Fact]
@ -237,7 +237,7 @@ namespace Microsoft.AspNet.Mvc.Test
// Assert
var action = Assert.Single(actions);
Assert.Equal("api/Token/value/TokenReplacement/stub/ThisIsAnAction", action.AttributeRouteTemplate);
Assert.Equal("api/Token/value/TokenReplacement/stub/ThisIsAnAction", action.AttributeRouteInfo.Template);
}
[Fact]
@ -276,7 +276,7 @@ namespace Microsoft.AspNet.Mvc.Test
// Assert
var action = Assert.Single(actions);
Assert.Equal("stub/ThisIsAnAction", action.AttributeRouteTemplate);
Assert.Equal("stub/ThisIsAnAction", action.AttributeRouteInfo.Template);
}
// Token replacement happens before we 'group' routes. So two route templates
@ -314,7 +314,7 @@ namespace Microsoft.AspNet.Mvc.Test
// Assert
var action = Assert.Single(actions);
Assert.Equal("stub/{controller}/{action}", action.AttributeRouteTemplate);
Assert.Equal("stub/{controller}/{action}", action.AttributeRouteInfo.Template);
}
private ReflectedActionDescriptorProvider GetProvider(

View File

@ -18,9 +18,10 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder.Test
var model = new ReflectedActionModel(actionMethod);
// Assert
Assert.Equal(2, model.Attributes.Count);
Assert.Equal(3, model.Attributes.Count);
Assert.Single(model.Attributes, a => a is MyFilterAttribute);
Assert.Single(model.Attributes, a => a is MyOtherAttribute);
Assert.Single(model.Attributes, a => a is HttpGetAttribute);
}
[Fact]
@ -37,10 +38,25 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder.Test
Assert.IsType<MyFilterAttribute>(model.Filters[0]);
}
[Fact]
public void ReflectedActionModel_PopulatesAttributeRouteInfo()
{
// Arrange
var actionMethod = typeof(BlogController).GetMethod("Edit");
// Act
var model = new ReflectedActionModel(actionMethod);
// Assert
Assert.NotNull(model.AttributeRouteModel);
Assert.Equal("Edit", model.AttributeRouteModel.Template);
}
private class BlogController
{
[MyOther]
[MyFilter]
[HttpGet("Edit")]
public void Edit()
{
}

View File

@ -6,9 +6,9 @@ using System;
using System.Collections.Generic;
using Xunit;
namespace Microsoft.AspNet.Mvc.Routing
namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder
{
public class AttributeRouteTemplateTests
public class ReflectedAttributeRouteModelTests
{
[Theory]
[InlineData(null, null, null)]
@ -30,7 +30,7 @@ namespace Microsoft.AspNet.Mvc.Routing
public void Combine_EmptyTemplates(string left, string right, string expected)
{
// Arrange & Act
var combined = AttributeRouteTemplate.Combine(left, right);
var combined = ReflectedAttributeRouteModel.CombineTemplates(left, right);
// Assert
Assert.Equal(expected, combined);
@ -55,7 +55,7 @@ namespace Microsoft.AspNet.Mvc.Routing
public void Combine_OneTemplateHasValue(string left, string right, string expected)
{
// Arrange & Act
var combined = AttributeRouteTemplate.Combine(left, right);
var combined = ReflectedAttributeRouteModel.CombineTemplates(left, right);
// Assert
Assert.Equal(expected, combined);
@ -71,7 +71,7 @@ namespace Microsoft.AspNet.Mvc.Routing
public void Combine_BothTemplatesHasValue(string left, string right, string expected)
{
// Arrange & Act
var combined = AttributeRouteTemplate.Combine(left, right);
var combined = ReflectedAttributeRouteModel.CombineTemplates(left, right);
// Assert
Assert.Equal(expected, combined);
@ -95,7 +95,7 @@ namespace Microsoft.AspNet.Mvc.Routing
public void Combine_InvalidTemplates(string left, string right, string expected)
{
// Arrange & Act
var combined = AttributeRouteTemplate.Combine(left, right);
var combined = ReflectedAttributeRouteModel.CombineTemplates(left, right);
// Assert
Assert.Equal(expected, combined);
@ -194,7 +194,7 @@ namespace Microsoft.AspNet.Mvc.Routing
}
// Act
var result = AttributeRouteTemplate.ReplaceTokens(template, valuesDictionary);
var result = ReflectedAttributeRouteModel.ReplaceTokens(template, valuesDictionary);
// Assert
Assert.Equal(expected, result);
@ -283,7 +283,7 @@ namespace Microsoft.AspNet.Mvc.Routing
// Act
var ex = Assert.Throws<InvalidOperationException>(
() => { AttributeRouteTemplate.ReplaceTokens(template, valuesDictionary); });
() => { ReflectedAttributeRouteModel.ReplaceTokens(template, valuesDictionary); });
// Assert
Assert.Equal(expected, ex.Message);
@ -308,10 +308,74 @@ namespace Microsoft.AspNet.Mvc.Routing
// Act
var ex = Assert.Throws<InvalidOperationException>(
() => { AttributeRouteTemplate.ReplaceTokens(template, values); });
() => { ReflectedAttributeRouteModel.ReplaceTokens(template, values); });
// Assert
Assert.Equal(expected, ex.Message);
}
[Theory]
[MemberData("ValidReflectedAttributeRouteModelsTestData")]
public void Combine_ValidReflectedAttributeRouteModels(
ReflectedAttributeRouteModel left,
ReflectedAttributeRouteModel right,
ReflectedAttributeRouteModel expectedResult)
{
// Arrange & Act
var combined = ReflectedAttributeRouteModel.CombineReflectedAttributeRouteModel(left, right);
// Assert
Assert.NotNull(combined);
Assert.Equal(expectedResult.Template, combined.Template);
}
[Theory]
[MemberData("NullOrNullTemplateReflectedAttributeRouteModelTestData")]
public void Combine_NullOrNullTemplateReflectedAttributeRouteModels(
ReflectedAttributeRouteModel left,
ReflectedAttributeRouteModel right)
{
// Arrange & Act
var combined = ReflectedAttributeRouteModel.CombineReflectedAttributeRouteModel(left, right);
// Assert
Assert.Null(combined);
}
public static IEnumerable<object[]> NullOrNullTemplateReflectedAttributeRouteModelTestData
{
get
{
var data = new TheoryData<ReflectedAttributeRouteModel, ReflectedAttributeRouteModel>();
data.Add(null, null);
data.Add(null, Create(null));
data.Add(Create(null), null);
data.Add(Create(null), Create(null));
return data;
}
}
public static IEnumerable<object []> ValidReflectedAttributeRouteModelsTestData
{
get
{
var data = new TheoryData<ReflectedAttributeRouteModel, ReflectedAttributeRouteModel, ReflectedAttributeRouteModel>();
data.Add(null, Create("Index"), Create("Index"));
data.Add(Create("Home"), null, Create("Home"));
data.Add(Create("Home"), Create("Index"), Create("Home/Index"));
data.Add(Create("Blog"), Create("/Index"), Create("Index"));
return data;
}
}
private static ReflectedAttributeRouteModel Create(string template)
{
return new ReflectedAttributeRouteModel
{
Template = template
};
}
}
}

View File

@ -19,11 +19,12 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder.Test
var model = new ReflectedControllerModel(controllerType.GetTypeInfo());
// Assert
Assert.Equal(3, model.Attributes.Count);
Assert.Equal(4, model.Attributes.Count);
Assert.Single(model.Attributes, a => a is MyOtherAttribute);
Assert.Single(model.Attributes, a => a is MyFilterAttribute);
Assert.Single(model.Attributes, a => a is MyRouteConstraintAttribute);
Assert.Single(model.Attributes, a => a is RouteAttribute);
}
[Fact]
@ -80,9 +81,24 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder.Test
Assert.Equal("Store", model.ControllerName);
}
[Fact]
public void ReflectedControllerModel_PopulatesAttributeRouteInfo()
{
// Arrange
var controllerType = typeof(BlogController);
// Act
var model = new ReflectedControllerModel(controllerType.GetTypeInfo());
// Assert
Assert.NotNull(model.AttributeRouteModel);
Assert.Equal("Blog", model.AttributeRouteModel.Template);
}
[MyOther]
[MyFilter]
[MyRouteConstraint]
[Route("Blog")]
private class BlogController
{
}

View File

@ -48,7 +48,7 @@ namespace Microsoft.AspNet.Mvc.Routing
"The following errors occurred with attribute routing information:" + Environment.NewLine +
Environment.NewLine +
"For action: 'DisallowedParameter'" + Environment.NewLine +
"Error: The attribute route '{foo}/{action}' cannot contain a parameter named '{foo}'. " +
"Error: The attribute route '{foo}/{action}' cannot contain a parameter named '{foo}'. " +
"Use '[foo]' in the route template to insert the value 'bleh'.";
var router = CreateRouter();
@ -106,7 +106,9 @@ namespace Microsoft.AspNet.Mvc.Routing
{
new RouteDataActionConstraint(AttributeRouting.RouteGroupKey, "group"),
};
action.AttributeRouteTemplate = "{controller}/{action}";
action.AttributeRouteInfo = new AttributeRouteInfo();
action.AttributeRouteInfo.Template = "{controller}/{action}";
action.RouteValueDefaults.Add("controller", "Home");
action.RouteValueDefaults.Add("action", "Index");
@ -136,7 +138,7 @@ namespace Microsoft.AspNet.Mvc.Routing
{
new RouteDataActionConstraint(AttributeRouting.RouteGroupKey, "whatever"),
},
AttributeRouteTemplate = template,
AttributeRouteInfo = new AttributeRouteInfo { Template = template },
};
}