diff --git a/samples/MvcSample.Web/HomeController.cs b/samples/MvcSample.Web/HomeController.cs index 2928cde9e4..a7f8251811 100644 --- a/samples/MvcSample.Web/HomeController.cs +++ b/samples/MvcSample.Web/HomeController.cs @@ -126,6 +126,37 @@ namespace MvcSample.Web return user; } + [HttpGet("/AttributeRouting/{other}", Order = 0)] + public string LowerPrecedence(string param) + { + return "Lower"; + } + + // Normally this route would be tried before the one above + // as it is more explicit (doesn't have a parameter), but + // due to the fact that it has a higher order, it will be + // tried after the route above. + [HttpGet("/AttributeRouting/HigherPrecedence", Order = 1)] + public string HigherOrder() + { + return "Higher"; + } + + // Both routes have the same template, which would make + // them ambiguous, but the order we defined in the routes + // disambiguates them. + [HttpGet("/AttributeRouting/SameTemplate", Order = 0)] + public string SameTemplateHigherOrderPrecedence() + { + return "HigherOrderPrecedence"; + } + + [HttpGet("/AttributeRouting/SameTemplate", Order = 1)] + public string SameTemplateLowerOrderPrecedence() + { + return "LowerOrderPrecedence"; + } + /// /// Action that exercises default view names. /// diff --git a/src/Microsoft.AspNet.Mvc.Core/HttpDeleteAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/HttpDeleteAttribute.cs index 6a1fb69183..7595f906d6 100644 --- a/src/Microsoft.AspNet.Mvc.Core/HttpDeleteAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/HttpDeleteAttribute.cs @@ -3,15 +3,13 @@ using System; using System.Collections.Generic; -using Microsoft.AspNet.Mvc.Routing; namespace Microsoft.AspNet.Mvc { /// /// Identifies an action that only supports the HTTP DELETE method. /// - [AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)] - public class HttpDeleteAttribute : Attribute, IActionHttpMethodProvider, IRouteTemplateProvider + public class HttpDeleteAttribute : HttpMethodAttribute { private static readonly IEnumerable _supportedMethods = new string[] { "DELETE" }; @@ -19,6 +17,7 @@ namespace Microsoft.AspNet.Mvc /// Creates a new . /// public HttpDeleteAttribute() + : base(_supportedMethods) { } @@ -27,17 +26,8 @@ namespace Microsoft.AspNet.Mvc /// /// The route template. May not be null. public HttpDeleteAttribute([NotNull] string template) + : base(_supportedMethods, template) { - Template = template; } - - /// - public IEnumerable HttpMethods - { - get { return _supportedMethods; } - } - - /// - public string Template { get; private set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/HttpGetAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/HttpGetAttribute.cs index 72c3145108..8ef6d09939 100644 --- a/src/Microsoft.AspNet.Mvc.Core/HttpGetAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/HttpGetAttribute.cs @@ -3,15 +3,13 @@ using System; using System.Collections.Generic; -using Microsoft.AspNet.Mvc.Routing; namespace Microsoft.AspNet.Mvc { /// /// Identifies an action that only supports the HTTP GET method. /// - [AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)] - public class HttpGetAttribute : Attribute, IActionHttpMethodProvider, IRouteTemplateProvider + public class HttpGetAttribute : HttpMethodAttribute { private static readonly IEnumerable _supportedMethods = new string[] { "GET" }; @@ -19,6 +17,7 @@ namespace Microsoft.AspNet.Mvc /// Creates a new . /// public HttpGetAttribute() + : base(_supportedMethods) { } @@ -27,17 +26,8 @@ namespace Microsoft.AspNet.Mvc /// /// The route template. May not be null. public HttpGetAttribute([NotNull] string template) + : base(_supportedMethods, template) { - Template = template; } - - /// - public IEnumerable HttpMethods - { - get { return _supportedMethods; } - } - - /// - public string Template { get; private set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/HttpMethodAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/HttpMethodAttribute.cs new file mode 100644 index 0000000000..c7d96f1074 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/HttpMethodAttribute.cs @@ -0,0 +1,74 @@ +// 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. + +using System; +using System.Collections.Generic; +using Microsoft.AspNet.Mvc.Routing; + +namespace Microsoft.AspNet.Mvc +{ + /// + /// Identifies an action that only supports a given set of HTTP methods. + /// + [AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)] + public abstract class HttpMethodAttribute : Attribute, IActionHttpMethodProvider, IRouteTemplateProvider + { + private int? _order; + private readonly IEnumerable _httpMethods; + + /// + /// Creates a new with the given + /// set of HTTP methods. + /// The set of supported HTTP methods. + /// + public HttpMethodAttribute([NotNull] IEnumerable httpMethods) + : this(httpMethods, null) + { + } + + /// + /// Creates a new with the given + /// set of HTTP methods an the given route template. + /// + /// The set of supported methods. + /// The route template. May not be null. + public HttpMethodAttribute([NotNull] IEnumerable httpMethods, string template) + { + _httpMethods = httpMethods; + Template = template; + } + + /// + public IEnumerable HttpMethods + { + get + { + return _httpMethods; + } + } + + /// + public string Template { get; private set; } + + /// + /// Gets the route order. The order determines the order of route execution. Routes with a lower + /// order value are tried first. When a route doesn't specify a value, it gets the value of the + /// or a default value of 0 if the + /// doesn't define a value on the controller. + /// + public int Order + { + get { return _order ?? 0; } + set { _order = value; } + } + + /// + int? IRouteTemplateProvider.Order + { + get + { + return _order; + } + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/HttpPatchAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/HttpPatchAttribute.cs index 4710ff29b7..143b361d7e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/HttpPatchAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/HttpPatchAttribute.cs @@ -3,15 +3,13 @@ using System; using System.Collections.Generic; -using Microsoft.AspNet.Mvc.Routing; namespace Microsoft.AspNet.Mvc { /// /// Identifies an action that only supports the HTTP PATCH method. /// - [AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)] - public class HttpPatchAttribute : Attribute, IActionHttpMethodProvider, IRouteTemplateProvider + public class HttpPatchAttribute : HttpMethodAttribute { private static readonly IEnumerable _supportedMethods = new string[] { "PATCH" }; @@ -19,6 +17,7 @@ namespace Microsoft.AspNet.Mvc /// Creates a new . /// public HttpPatchAttribute() + : base(_supportedMethods) { } @@ -27,17 +26,8 @@ namespace Microsoft.AspNet.Mvc /// /// The route template. May not be null. public HttpPatchAttribute([NotNull] string template) + : base(_supportedMethods, template) { - Template = template; } - - /// - public IEnumerable HttpMethods - { - get { return _supportedMethods; } - } - - /// - public string Template { get; private set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/HttpPostAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/HttpPostAttribute.cs index 5a771992d4..3a06c25aa8 100644 --- a/src/Microsoft.AspNet.Mvc.Core/HttpPostAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/HttpPostAttribute.cs @@ -3,23 +3,21 @@ using System; using System.Collections.Generic; -using Microsoft.AspNet.Mvc.Routing; namespace Microsoft.AspNet.Mvc { /// /// Identifies an action that only supports the HTTP POST method. /// - [AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)] - public class HttpPostAttribute : Attribute, IActionHttpMethodProvider, IRouteTemplateProvider + public class HttpPostAttribute : HttpMethodAttribute { private static readonly IEnumerable _supportedMethods = new string[] { "POST" }; /// /// Creates a new . /// - /// The route template. May not be null. public HttpPostAttribute() + : base(_supportedMethods) { } @@ -28,17 +26,8 @@ namespace Microsoft.AspNet.Mvc /// /// The route template. May not be null. public HttpPostAttribute([NotNull] string template) + : base(_supportedMethods, template) { - Template = template; } - - /// - public IEnumerable HttpMethods - { - get { return _supportedMethods; } - } - - /// - public string Template { get; private set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/HttpPutAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/HttpPutAttribute.cs index 9ef79a1bb4..c1beff4256 100644 --- a/src/Microsoft.AspNet.Mvc.Core/HttpPutAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/HttpPutAttribute.cs @@ -3,15 +3,13 @@ using System; using System.Collections.Generic; -using Microsoft.AspNet.Mvc.Routing; namespace Microsoft.AspNet.Mvc { /// /// Identifies an action that only supports the HTTP PUT method. /// - [AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)] - public class HttpPutAttribute : Attribute, IActionHttpMethodProvider, IRouteTemplateProvider + public class HttpPutAttribute : HttpMethodAttribute { private static readonly IEnumerable _supportedMethods = new string[] { "PUT" }; @@ -19,6 +17,7 @@ namespace Microsoft.AspNet.Mvc /// Creates a new . /// public HttpPutAttribute() + : base(_supportedMethods) { } @@ -27,17 +26,8 @@ namespace Microsoft.AspNet.Mvc /// /// The route template. May not be null. public HttpPutAttribute([NotNull] string template) + : base(_supportedMethods, template) { - Template = template; } - - /// - public IEnumerable HttpMethods - { - get { return _supportedMethods; } - } - - /// - public string Template { get; private set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Microsoft.AspNet.Mvc.Core.kproj b/src/Microsoft.AspNet.Mvc.Core/Microsoft.AspNet.Mvc.Core.kproj index 46a286d607..208f9ec964 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Microsoft.AspNet.Mvc.Core.kproj +++ b/src/Microsoft.AspNet.Mvc.Core/Microsoft.AspNet.Mvc.Core.kproj @@ -31,6 +31,7 @@ + @@ -190,6 +191,7 @@ + @@ -247,10 +249,10 @@ + - diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs index 0d9eb23a51..e7d3e3cd62 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs @@ -17,8 +17,16 @@ namespace Microsoft.AspNet.Mvc { public class ReflectedActionDescriptorProvider : IActionDescriptorProvider { + /// + /// Represents the default order associated with this provider for dependency injection + /// purposes. + /// public static readonly int DefaultOrder = 0; + // This is the default order for attribute routes whose order calculated from + // the reflected model is null. + private const int DefaultAttributeRouteOrder = 0; + private readonly IControllerAssemblyProvider _controllerAssemblyProvider; private readonly IActionDiscoveryConventions _conventions; private readonly IEnumerable _globalFilters; @@ -114,7 +122,7 @@ namespace Microsoft.AspNet.Mvc { var actions = new List(); - var routeGroupsByTemplate = new Dictionary(StringComparer.OrdinalIgnoreCase); + var hasAttributeRoutes = false; var removalConstraints = new HashSet(StringComparer.OrdinalIgnoreCase); var routeTemplateErrors = new List(); @@ -152,7 +160,8 @@ namespace Microsoft.AspNet.Mvc var attributeRouteInfo = combinedRoute == null ? null : new AttributeRouteInfo() { - Template = combinedRoute.Template + Template = combinedRoute.Template, + Order = combinedRoute.Order ?? DefaultAttributeRouteOrder }; var actionDescriptor = new ReflectedActionDescriptor() @@ -215,6 +224,7 @@ namespace Microsoft.AspNet.Mvc if (actionDescriptor.AttributeRouteInfo != null && actionDescriptor.AttributeRouteInfo.Template != null) { + hasAttributeRoutes = true; // An attribute routed action will ignore conventional routed constraints. We still // want to provide these values as ambient values. foreach (var constraint in actionDescriptor.RouteConstraints) @@ -243,19 +253,14 @@ namespace Microsoft.AspNet.Mvc actionDescriptor.AttributeRouteInfo.Template = templateText; - // An attribute routed action is matched by its 'route group' which identifies all equivalent - // actions. - string routeGroup; - if (!routeGroupsByTemplate.TryGetValue(templateText, out routeGroup)) - { - routeGroup = GetRouteGroup(templateText); - routeGroupsByTemplate.Add(templateText, routeGroup); - } + var routeGroupValue = GetRouteGroupValue( + actionDescriptor.AttributeRouteInfo.Order, + templateText); var routeConstraints = new List(); routeConstraints.Add(new RouteDataActionConstraint( AttributeRouting.RouteGroupKey, - routeGroup)); + routeGroupValue)); actionDescriptor.RouteConstraints = routeConstraints; } @@ -279,7 +284,7 @@ namespace Microsoft.AspNet.Mvc { // 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 (hasAttributeRoutes) { actionDescriptor.RouteConstraints.Add(new RouteDataActionConstraint( AttributeRouting.RouteGroupKey, @@ -320,10 +325,10 @@ namespace Microsoft.AspNet.Mvc return actions; } - // Returns a unique, stable key per-route-template (OrdinalIgnoreCase) - private static string GetRouteGroup(string template) + private static string GetRouteGroupValue(int order, string template) { - return ("__route__" + template).ToUpperInvariant(); + var group = string.Format("{0}-{1}", order, template); + return ("__route__" + group).ToUpperInvariant(); } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedAttributeRouteModel.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedAttributeRouteModel.cs index 875677a030..a58f46e490 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedAttributeRouteModel.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedAttributeRouteModel.cs @@ -20,14 +20,17 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder public ReflectedAttributeRouteModel([NotNull] IRouteTemplateProvider templateProvider) { Template = templateProvider.Template; + Order = templateProvider.Order; } public string Template { get; set; } - /// + public int? Order { get; set; } + + /// /// Combines two instances and returns /// a new instance with the result. - /// + /// /// The left . /// The right . /// A new instance of that represents the @@ -37,30 +40,31 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder ReflectedAttributeRouteModel left, ReflectedAttributeRouteModel right) { - left = left ?? _default; right = right ?? _default; - var template = CombineTemplates(left.Template, right.Template); + // If the right template is an override template (starts with / or ~/) + // we ignore the values from left. + if (left == null || IsOverridePattern(right.Template)) + { + left = _default; + } + + var combinedTemplate = CombineTemplates(left.Template, right.Template); // The action is not attribute routed. - if (template == null) + if (combinedTemplate == null) { return null; } return new ReflectedAttributeRouteModel() - { - Template = template + { + Template = combinedTemplate, + Order = right.Order ?? left.Order }; } - /// - /// Combines attribute routing templates. - /// - /// The left template. - /// The right template. - /// A combined template. - public static string CombineTemplates(string left, string right) + internal static string CombineTemplates(string left, string right) { var result = CombineCore(left, right); return CleanTemplate(result); @@ -72,19 +76,11 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder { return null; } - else if (left == null) - { - return right; - } else if (right == null) { return left; } - - if (right.StartsWith("~/", StringComparison.OrdinalIgnoreCase) || - right.StartsWith("/", StringComparison.OrdinalIgnoreCase) || - left.Equals("~/", StringComparison.OrdinalIgnoreCase) || - left.Equals("/", StringComparison.OrdinalIgnoreCase)) + else if (IsEmptyLeftSegment(left) || IsOverridePattern(right)) { return right; } @@ -98,6 +94,21 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder return left + '/' + right; } + private static bool IsOverridePattern(string template) + { + return template != null && + (template.StartsWith("~/", StringComparison.OrdinalIgnoreCase) || + template.StartsWith("/", StringComparison.OrdinalIgnoreCase)); + } + + private static bool IsEmptyLeftSegment(string template) + { + return template == null || + template.Equals(string.Empty, StringComparison.OrdinalIgnoreCase) || + template.Equals("~/", StringComparison.OrdinalIgnoreCase) || + template.Equals("/", StringComparison.OrdinalIgnoreCase); + } + private static string CleanTemplate(string result) { if (result == null) diff --git a/src/Microsoft.AspNet.Mvc.Core/RouteAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/RouteAttribute.cs index b27fd64df4..138215762f 100644 --- a/src/Microsoft.AspNet.Mvc.Core/RouteAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/RouteAttribute.cs @@ -12,6 +12,8 @@ namespace Microsoft.AspNet.Mvc [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)] public class RouteAttribute : Attribute, IRouteTemplateProvider { + private int? _order; + /// /// Creates a new with the given route template. /// @@ -23,5 +25,26 @@ namespace Microsoft.AspNet.Mvc /// public string Template { get; private set; } + + /// + /// Gets the route order. The order determines the order of route execution. Routes with a lower order + /// value are tried first. If an action defines a route by providing an + /// with a non null order, that order is used instead of this value. If neither the action nor the + /// controller defines an order, a default value of 0 is used. + /// + public int Order + { + get { return _order ?? 0; } + set { _order = value; } + } + + /// + int? IRouteTemplateProvider.Order + { + get + { + return _order; + } + } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRoute.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRoute.cs index f4ffe7d273..a71d902924 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRoute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRoute.cs @@ -29,20 +29,30 @@ namespace Microsoft.AspNet.Mvc.Routing /// The next router. Invoked when a route entry matches. /// The set of route entries. public AttributeRoute( - [NotNull] IRouter next, + [NotNull] IRouter next, [NotNull] IEnumerable matchingEntries, [NotNull] IEnumerable linkGenerationEntries, [NotNull] ILoggerFactory factory) { _next = next; - // FOR RIGHT NOW - this is just an array of regular template routes. We'll follow up by implementing - // a good data-structure here. See #740 - _matchingRoutes = matchingEntries.OrderBy(e => e.Precedence).Select(e => e.Route).ToArray(); + // Order all the entries by order, then precedence, and then finally by template in order to provide + // a stable routing and link generation order for templates with same order and precedence. + // We use ordinal comparison for the templates because we only care about them being exactly equal and + // we don't want to make any equivalence between templates based on the culture of the machine. - // FOR RIGHT NOW - this is just an array of entries. We'll follow up by implementing - // a good data-structure here. See #741 - _linkGenerationEntries = linkGenerationEntries.OrderBy(e => e.Precedence).ToArray(); + _matchingRoutes = matchingEntries + .OrderBy(o => o.Order) + .ThenBy(e => e.Precedence) + .ThenBy(e => e.Route.RouteTemplate, StringComparer.Ordinal) + .Select(e => e.Route) + .ToArray(); + + _linkGenerationEntries = linkGenerationEntries + .OrderBy(o => o.Order) + .ThenBy(e => e.Precedence) + .ThenBy(e => e.TemplateText, StringComparer.Ordinal) + .ToArray(); _logger = factory.Create(); _constraintLogger = factory.Create(typeof(RouteConstraintMatcher).FullName); @@ -53,12 +63,12 @@ namespace Microsoft.AspNet.Mvc.Routing { using (_logger.BeginScope("AttributeRoute.RouteAsync")) { - foreach (var route in _matchingRoutes) - { - await route.RouteAsync(context); + foreach (var route in _matchingRoutes) + { + await route.RouteAsync(context); - if (context.IsHandled) - { + if (context.IsHandled) + { break; } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteInfo.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteInfo.cs index f8ba92e7d9..463de8c510 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteInfo.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteInfo.cs @@ -12,5 +12,12 @@ namespace Microsoft.AspNet.Mvc.Routing /// The route template. May be null if the action has no attribute routes. /// public string Template { get; set; } + + /// + /// Gets the order of the route associated with this . This property determines + /// the order in which routes get executed. Routes with a lower order value are tried first. In case a route + /// doesn't specify a value, it gets a default order of 0. + /// + public int Order { get; set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteLinkGenerationEntry.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteLinkGenerationEntry.cs index f2fc7204dc..f088991513 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteLinkGenerationEntry.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteLinkGenerationEntry.cs @@ -28,6 +28,11 @@ namespace Microsoft.AspNet.Mvc.Routing /// public IDictionary Defaults { get; set; } + /// + /// The order of the template. + /// + public int Order { get; set; } + /// /// The precedence of the template. /// @@ -47,5 +52,10 @@ namespace Microsoft.AspNet.Mvc.Routing /// The . /// public RouteTemplate Template { get; set; } + + /// + /// The original representing the route template. + /// + public string TemplateText { get; set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteMatchingEntry.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteMatchingEntry.cs index 39790523d6..8816b3882f 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteMatchingEntry.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouteMatchingEntry.cs @@ -12,6 +12,11 @@ namespace Microsoft.AspNet.Mvc.Routing /// public class AttributeRouteMatchingEntry { + /// + /// The order of the template. + /// + public int Order { get; set; } + /// /// The precedence of the template. /// diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouting.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouting.cs index d7cbecdebb..a9922c846f 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouting.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRouting.cs @@ -41,10 +41,12 @@ namespace Microsoft.AspNet.Mvc.Routing Binder = new TemplateBinder(routeInfo.ParsedTemplate, routeInfo.Defaults), Defaults = routeInfo.Defaults, Constraints = routeInfo.Constraints, + Order = routeInfo.Order, Precedence = routeInfo.Precedence, RequiredLinkValues = routeInfo.ActionDescriptor.RouteValueDefaults, RouteGroup = routeInfo.RouteGroup, Template = routeInfo.ParsedTemplate, + TemplateText = routeInfo.RouteTemplate }); } @@ -57,6 +59,7 @@ namespace Microsoft.AspNet.Mvc.Routing { matchingEntries.Add(new AttributeRouteMatchingEntry() { + Order = routeInfo.Order, Precedence = routeInfo.Precedence, Route = new TemplateRoute( target, @@ -72,9 +75,9 @@ namespace Microsoft.AspNet.Mvc.Routing } return new AttributeRoute( - target, - matchingEntries, - generationEntries, + target, + matchingEntries, + generationEntries, services.GetService()); } @@ -133,11 +136,11 @@ namespace Microsoft.AspNet.Mvc.Routing if (errors.Count > 0) { var allErrors = string.Join( - Environment.NewLine + Environment.NewLine, + Environment.NewLine + Environment.NewLine, errors.Select( e => Resources.FormatAttributeRoute_IndividualErrorMessage( - e.ActionDescriptor.DisplayName, - Environment.NewLine, + e.ActionDescriptor.DisplayName, + Environment.NewLine, e.ErrorMessage))); var message = Resources.FormatAttributeRoute_AggregateErrorMessage(Environment.NewLine, allErrors); @@ -208,6 +211,8 @@ namespace Microsoft.AspNet.Mvc.Routing } } + routeInfo.Order = action.AttributeRouteInfo.Order; + routeInfo.Precedence = AttributeRoutePrecedence.Compute(routeInfo.ParsedTemplate); routeInfo.Constraints = routeInfo.ParsedTemplate.Parameters @@ -233,6 +238,8 @@ namespace Microsoft.AspNet.Mvc.Routing public RouteTemplate ParsedTemplate { get; set; } + public int Order { get; set; } + public decimal Precedence { get; set; } public string RouteGroup { get; set; } diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/IRouteTemplateProvider.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/IRouteTemplateProvider.cs index 2dc7969d2d..4cbd7ff3d1 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Routing/IRouteTemplateProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Routing/IRouteTemplateProvider.cs @@ -12,5 +12,13 @@ namespace Microsoft.AspNet.Mvc.Routing /// The route template. May be null. /// string Template { get; } + + /// + /// Gets the route order. The order determines the order of route execution. Routes with a lower + /// order value are tried first. When a route doesn't specify a value, it gets a default value of 0. + /// A null value for the Order property means that the user didn't specify an explicit order for the + /// route. + /// + int? Order { get; } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj b/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj index 3b8eddb6e7..8040731a17 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Microsoft.AspNet.Mvc.Core.Test.kproj @@ -35,6 +35,7 @@ + @@ -88,6 +89,7 @@ + @@ -103,6 +105,7 @@ + diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedModelBuilder/ReflectedAttributeRouteModelTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedModelBuilder/ReflectedAttributeRouteModelTests.cs index 8f5647aa59..304b6a9a90 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedModelBuilder/ReflectedAttributeRouteModelTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedModelBuilder/ReflectedAttributeRouteModelTests.cs @@ -101,6 +101,210 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder Assert.Equal(expected, combined); } + [Theory] + [MemberData("ReplaceTokens_ValueValuesData")] + public void ReplaceTokens_ValidValues(string template, object values, string expected) + { + // Arrange + var valuesDictionary = values as IDictionary; + if (valuesDictionary == null) + { + valuesDictionary = new RouteValueDictionary(values); + } + + // Act + var result = ReflectedAttributeRouteModel.ReplaceTokens(template, valuesDictionary); + + // Assert + Assert.Equal(expected, result); + } + + [Theory] + [MemberData("ReplaceTokens_InvalidFormatValuesData")] + public void ReplaceTokens_InvalidFormat(string template, object values, string reason) + { + // Arrange + var valuesDictionary = values as IDictionary; + if (valuesDictionary == null) + { + valuesDictionary = new RouteValueDictionary(values); + } + + var expected = string.Format( + "The route template '{0}' has invalid syntax. {1}", + template, + reason); + + // Act + var ex = Assert.Throws( + () => { ReflectedAttributeRouteModel.ReplaceTokens(template, valuesDictionary); }); + + // Assert + Assert.Equal(expected, ex.Message); + } + + [Fact] + public void ReplaceTokens_UnknownValue() + { + // Arrange + var template = "[area]/[controller]/[action2]"; + var values = new RouteValueDictionary() + { + { "area", "Help" }, + { "controller", "Admin" }, + { "action", "SeeUsers" }, + }; + + var expected = + "While processing template '[area]/[controller]/[action2]', " + + "a replacement value for the token 'action2' could not be found. " + + "Available tokens: 'area, controller, action'."; + + // Act + var ex = Assert.Throws( + () => { ReflectedAttributeRouteModel.ReplaceTokens(template, values); }); + + // Assert + Assert.Equal(expected, ex.Message); + } + + [Theory] + [MemberData("CombineOrdersTestData")] + public void Combine_Orders( + ReflectedAttributeRouteModel left, + ReflectedAttributeRouteModel right, + int? expected) + { + // Arrange & Act + var combined = ReflectedAttributeRouteModel.CombineReflectedAttributeRouteModel(left, right); + + // Assert + Assert.NotNull(combined); + Assert.Equal(expected, combined.Order); + } + + [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); + } + + [Theory] + [MemberData("RightOverridesReflectedAttributeRouteModelTestData")] + public void Combine_RightOverridesReflectedAttributeRouteModel( + ReflectedAttributeRouteModel left, + ReflectedAttributeRouteModel right) + { + // Arrange + var expectedTemplate = ReflectedAttributeRouteModel.CombineTemplates(null, right.Template); + + // Act + var combined = ReflectedAttributeRouteModel.CombineReflectedAttributeRouteModel(left, right); + + // Assert + Assert.NotNull(combined); + Assert.Equal(expectedTemplate, combined.Template); + Assert.Equal(combined.Order, right.Order); + } + + public static IEnumerable CombineOrdersTestData + { + get + { + var data = new TheoryData(); + + data.Add(Create("", order: 1), Create("", order: 2), 2); + data.Add(Create("", order: 1), Create("", order: null), 1); + data.Add(Create("", order: 1), null, 1); + data.Add(Create("", order: 1), Create("/", order: 2), 2); + data.Add(Create("", order: 1), Create("/", order: null), null); + data.Add(Create("", order: null), Create("", order: 2), 2); + data.Add(Create("", order: null), Create("", order: null), null); + data.Add(Create("", order: null), null, null); + data.Add(Create("", order: null), Create("/", order: 2), 2); + data.Add(Create("", order: null), Create("/", order: null), null); + data.Add(null, Create("", order: 2), 2); + data.Add(null, Create("", order: null), null); + data.Add(null, Create("/", order: 2), 2); + data.Add(null, Create("/", order: null), null); + + // We don't a test case for (left = null, right = null) as it is already tested in another test + // and will produce a null ReflectedAttributeRouteModel, which complicates the test case. + + return data; + } + } + + public static IEnumerable RightOverridesReflectedAttributeRouteModelTestData + { + get + { + var data = new TheoryData(); + var leftModel = Create("Home", order: 3); + + data.Add(leftModel, Create("/")); + data.Add(leftModel, Create("~/")); + data.Add(null, Create("/")); + data.Add(null, Create("~/")); + data.Add(Create(null), Create("/")); + data.Add(Create(null), Create("~/")); + + return data; + } + } + + public static IEnumerable NullOrNullTemplateReflectedAttributeRouteModelTestData + { + get + { + var data = new TheoryData(); + 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 ValidReflectedAttributeRouteModelsTestData + { + get + { + var data = new TheoryData(); + data.Add(null, Create("Index"), Create("Index")); + data.Add(Create(null), Create("Index"), Create("Index"));; + data.Add(Create("Home"), null, Create("Home")); + data.Add(Create("Home"), Create(null), Create("Home")); + data.Add(Create("Home"), Create("Index"), Create("Home/Index")); + data.Add(Create("Blog"), Create("/Index"), Create("Index")); + + return data; + } + } + public static IEnumerable ReplaceTokens_ValueValuesData { get @@ -182,24 +386,6 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder } } - [Theory] - [MemberData("ReplaceTokens_ValueValuesData")] - public void ReplaceTokens_ValidValues(string template, object values, string expected) - { - // Arrange - var valuesDictionary = values as IDictionary; - if (valuesDictionary == null) - { - valuesDictionary = new RouteValueDictionary(values); - } - - // Act - var result = ReflectedAttributeRouteModel.ReplaceTokens(template, valuesDictionary); - - // Assert - Assert.Equal(expected, result); - } - public static IEnumerable ReplaceTokens_InvalidFormatValuesData { get @@ -265,116 +451,12 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder } } - [Theory] - [MemberData("ReplaceTokens_InvalidFormatValuesData")] - public void ReplaceTokens_InvalidFormat(string template, object values, string reason) - { - // Arrange - var valuesDictionary = values as IDictionary; - if (valuesDictionary == null) - { - valuesDictionary = new RouteValueDictionary(values); - } - - var expected = string.Format( - "The route template '{0}' has invalid syntax. {1}", - template, - reason); - - // Act - var ex = Assert.Throws( - () => { ReflectedAttributeRouteModel.ReplaceTokens(template, valuesDictionary); }); - - // Assert - Assert.Equal(expected, ex.Message); - } - - [Fact] - public void ReplaceTokens_UnknownValue() - { - // Arrange - var template = "[area]/[controller]/[action2]"; - var values = new RouteValueDictionary() - { - { "area", "Help" }, - { "controller", "Admin" }, - { "action", "SeeUsers" }, - }; - - var expected = - "While processing template '[area]/[controller]/[action2]', " + - "a replacement value for the token 'action2' could not be found. " + - "Available tokens: 'area, controller, action'."; - - // Act - var ex = Assert.Throws( - () => { 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 NullOrNullTemplateReflectedAttributeRouteModelTestData - { - get - { - var data = new TheoryData(); - 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 ValidReflectedAttributeRouteModelsTestData - { - get - { - var data = new TheoryData(); - 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) + private static ReflectedAttributeRouteModel Create(string template, int? order = null) { return new ReflectedAttributeRouteModel { - Template = template + Template = template, + Order = order }; } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/RouteTemplateProviderAttributesTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/RouteTemplateProviderAttributesTests.cs new file mode 100644 index 0000000000..fdc72b161e --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/RouteTemplateProviderAttributesTests.cs @@ -0,0 +1,32 @@ +using Microsoft.AspNet.Mvc.Routing; +using Xunit; + +namespace Microsoft.AspNet.Mvc +{ + public class RouteTemplateProviderAttributesTest + { + [Theory] + [MemberData("RouteTemplateProvidersTestData")] + public void Order_Defaults_ToNull(IRouteTemplateProvider routeTemplateProvider) + { + // Act & Assert + Assert.Null(routeTemplateProvider.Order); + } + + public static TheoryData RouteTemplateProvidersTestData + { + get + { + var data = new TheoryData(); + data.Add(new HttpGetAttribute()); + data.Add(new HttpPostAttribute()); + data.Add(new HttpPutAttribute()); + data.Add(new HttpPatchAttribute()); + data.Add(new HttpDeleteAttribute()); + data.Add(new RouteAttribute("")); + + return data; + } + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRouteTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRouteTests.cs index 19afc71eea..a23054ff22 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRouteTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Routing/AttributeRouteTests.cs @@ -13,11 +13,372 @@ using Microsoft.Framework.OptionsModel; using Microsoft.Framework.Logging; using Moq; using Xunit; +using Microsoft.AspNet.PipelineCore; namespace Microsoft.AspNet.Mvc.Routing { - public class AttributeRouteTests + public class AttributeRouteTest { + [Theory] + [InlineData("template/5", "template/{parameter:int}")] + [InlineData("template/5", "template/{parameter}")] + [InlineData("template/5", "template/{*parameter:int}")] + [InlineData("template/5", "template/{*parameter}")] + [InlineData("template/{parameter:int}", "template/{parameter}")] + [InlineData("template/{parameter:int}", "template/{*parameter:int}")] + [InlineData("template/{parameter:int}", "template/{*parameter}")] + [InlineData("template/{parameter}", "template/{*parameter:int}")] + [InlineData("template/{parameter}", "template/{*parameter}")] + [InlineData("template/{*parameter:int}", "template/{*parameter}")] + public async Task AttributeRoute_RouteAsync_RespectsPrecedence( + string firstTemplate, + string secondTemplate) + { + // Arrange + var expectedRouteGroup = string.Format("{0}&&{1}", 0, firstTemplate); + + // We need to force the creation of a closure in order to avoid an issue with Moq and Roslyn. + var numberOfCalls = 0; + Action callBack = ctx => { ctx.IsHandled = true; numberOfCalls++; }; + + var next = new Mock(); + next.Setup(r => r.RouteAsync(It.IsAny())) + .Callback(callBack) + .Returns(Task.FromResult(true)) + .Verifiable(); + + var firstRoute = CreateMatchingEntry(next.Object, firstTemplate, order: 0); + var secondRoute = CreateMatchingEntry(next.Object, secondTemplate, order: 0); + + // We setup the route entries in reverse order of precedence to ensure that when we + // try to route the request, the route with a higher precedence gets tried first. + var matchingRoutes = new[] { secondRoute, firstRoute }; + + var linkGenerationEntries = Enumerable.Empty(); + + var route = new AttributeRoute(next.Object, matchingRoutes, linkGenerationEntries, NullLoggerFactory.Instance); + + var context = CreateRouteContext("/template/5"); + + // Act + await route.RouteAsync(context); + + // Assert + Assert.Equal(expectedRouteGroup, context.RouteData.Values["test_route_group"]); + } + + [Theory] + [InlineData("template/5", "template/{parameter:int}")] + [InlineData("template/5", "template/{parameter}")] + [InlineData("template/5", "template/{*parameter:int}")] + [InlineData("template/5", "template/{*parameter}")] + [InlineData("template/{parameter:int}", "template/{parameter}")] + [InlineData("template/{parameter:int}", "template/{*parameter:int}")] + [InlineData("template/{parameter:int}", "template/{*parameter}")] + [InlineData("template/{parameter}", "template/{*parameter:int}")] + [InlineData("template/{parameter}", "template/{*parameter}")] + [InlineData("template/{*parameter:int}", "template/{*parameter}")] + public async Task AttributeRoute_RouteAsync_RespectsOrderOverPrecedence( + string firstTemplate, + string secondTemplate) + { + // Arrange + var expectedRouteGroup = string.Format("{0}&&{1}", 0, secondTemplate); + + // We need to force the creation of a closure in order to avoid an issue with Moq and Roslyn. + var numberOfCalls = 0; + Action callBack = ctx => { ctx.IsHandled = true; numberOfCalls++; }; + + var next = new Mock(); + next.Setup(r => r.RouteAsync(It.IsAny())) + .Callback(callBack) + .Returns(Task.FromResult(true)) + .Verifiable(); + + var firstRoute = CreateMatchingEntry(next.Object, firstTemplate, order: 1); + var secondRoute = CreateMatchingEntry(next.Object, secondTemplate, order: 0); + + // We setup the route entries with a lower relative order and higher relative precedence + // first to ensure that when we try to route the request, the route with the higher + // relative order gets tried first. + var matchingRoutes = new[] { firstRoute, secondRoute }; + + var linkGenerationEntries = Enumerable.Empty(); + + var route = new AttributeRoute(next.Object, matchingRoutes, linkGenerationEntries, NullLoggerFactory.Instance); + + var context = CreateRouteContext("/template/5"); + + // Act + await route.RouteAsync(context); + + // Assert + Assert.Equal(expectedRouteGroup, context.RouteData.Values["test_route_group"]); + } + + [Theory] + [InlineData("template/5")] + [InlineData("template/{parameter:int}")] + [InlineData("template/{parameter}")] + [InlineData("template/{*parameter:int}")] + [InlineData("template/{*parameter}")] + public async Task AttributeRoute_RouteAsync_RespectsOrder(string template) + { + // Arrange + var expectedRouteGroup = string.Format("{0}&&{1}", 0, template); + + // We need to force the creation of a closure in order to avoid an issue with Moq and Roslyn. + var numberOfCalls = 0; + Action callBack = ctx => { ctx.IsHandled = true; numberOfCalls++; }; + + var next = new Mock(); + next.Setup(r => r.RouteAsync(It.IsAny())) + .Callback(callBack) + .Returns(Task.FromResult(true)) + .Verifiable(); + + var firstRoute = CreateMatchingEntry(next.Object, template, order: 1); + var secondRoute = CreateMatchingEntry(next.Object, template, order: 0); + + // We setup the route entries with a lower relative order first to ensure that when + // we try to route the request, the route with the higher relative order gets tried first. + var matchingRoutes = new[] { firstRoute, secondRoute }; + + var linkGenerationEntries = Enumerable.Empty(); + + var route = new AttributeRoute(next.Object, matchingRoutes, linkGenerationEntries, NullLoggerFactory.Instance); + + var context = CreateRouteContext("/template/5"); + + // Act + await route.RouteAsync(context); + + // Assert + Assert.Equal(expectedRouteGroup, context.RouteData.Values["test_route_group"]); + } + + [Theory] + [InlineData("template/{first:int}", "template/{second:int}")] + [InlineData("template/{first}", "template/{second}")] + [InlineData("template/{*first:int}", "template/{*second:int}")] + [InlineData("template/{*first}", "template/{*second}")] + public async Task AttributeRoute_RouteAsync_EnsuresStableOrdering(string first, string second) + { + // Arrange + var expectedRouteGroup = string.Format("{0}&&{1}", 0, first); + + // We need to force the creation of a closure in order to avoid an issue with Moq and Roslyn. + var numberOfCalls = 0; + Action callBack = ctx => { ctx.IsHandled = true; numberOfCalls++; }; + + var next = new Mock(); + next.Setup(r => r.RouteAsync(It.IsAny())) + .Callback(callBack) + .Returns(Task.FromResult(true)) + .Verifiable(); + + var secondRouter = new Mock(MockBehavior.Strict); + + var firstRoute = CreateMatchingEntry(next.Object, first, order: 0); + var secondRoute = CreateMatchingEntry(next.Object, second, order: 0); + + // We setup the route entries with a lower relative template order first to ensure that when + // we try to route the request, the route with the higher template order gets tried first. + var matchingRoutes = new[] { secondRoute, firstRoute }; + + var linkGenerationEntries = Enumerable.Empty(); + + var route = new AttributeRoute(next.Object, matchingRoutes, linkGenerationEntries, NullLoggerFactory.Instance); + + var context = CreateRouteContext("/template/5"); + + // Act + await route.RouteAsync(context); + + // Assert + Assert.Equal(expectedRouteGroup, context.RouteData.Values["test_route_group"]); + } + + [Theory] + [InlineData("template/5", "template/{parameter:int}")] + [InlineData("template/5", "template/{parameter}")] + [InlineData("template/5", "template/{*parameter:int}")] + [InlineData("template/5", "template/{*parameter}")] + [InlineData("template/{parameter:int}", "template/{parameter}")] + [InlineData("template/{parameter:int}", "template/{*parameter:int}")] + [InlineData("template/{parameter:int}", "template/{*parameter}")] + [InlineData("template/{parameter}", "template/{*parameter:int}")] + [InlineData("template/{parameter}", "template/{*parameter}")] + [InlineData("template/{*parameter:int}", "template/{*parameter}")] + public void AttributeRoute_GenerateLink_RespectsPrecedence(string firstTemplate, string secondTemplate) + { + // Arrange + var expectedGroup = CreateRouteGroup(0, firstTemplate); + + string selectedGroup = null; + + var next = new Mock(); + next.Setup(n => n.GetVirtualPath(It.IsAny())).Callback(ctx => + { + selectedGroup = (string)ctx.ProvidedValues[AttributeRouting.RouteGroupKey]; + ctx.IsBound = true; + }) + .Returns((string)null); + + var matchingRoutes = Enumerable.Empty(); + + var firstEntry = CreateGenerationEntry(firstTemplate, requiredValues: null); + var secondEntry = CreateGenerationEntry(secondTemplate, requiredValues: null, order: 0); + + // We setup the route entries in reverse order of precedence to ensure that when we + // try to generate a link, the route with a higher precedence gets tried first. + var linkGenerationEntries = new[] { secondEntry, firstEntry }; + + var route = new AttributeRoute(next.Object, matchingRoutes, linkGenerationEntries, NullLoggerFactory.Instance); + + var context = CreateVirtualPathContext(values: null, ambientValues: new { parameter = 5 }); + + // Act + string result = route.GetVirtualPath(context); + + // Assert + Assert.NotNull(result); + Assert.Equal("template/5", result); + Assert.Equal(expectedGroup, selectedGroup); + } + + [Theory] + [InlineData("template/5", "template/{parameter:int}")] + [InlineData("template/5", "template/{parameter}")] + [InlineData("template/5", "template/{*parameter:int}")] + [InlineData("template/5", "template/{*parameter}")] + [InlineData("template/{parameter:int}", "template/{parameter}")] + [InlineData("template/{parameter:int}", "template/{*parameter:int}")] + [InlineData("template/{parameter:int}", "template/{*parameter}")] + [InlineData("template/{parameter}", "template/{*parameter:int}")] + [InlineData("template/{parameter}", "template/{*parameter}")] + [InlineData("template/{*parameter:int}", "template/{*parameter}")] + public void AttributeRoute_GenerateLink_RespectsOrderOverPrecedence(string firstTemplate, string secondTemplate) + { + // Arrange + var selectedGroup = CreateRouteGroup(0, secondTemplate); + + string firstRouteGroupSelected = null; + var next = new Mock(); + next.Setup(n => n.GetVirtualPath(It.IsAny())).Callback(ctx => + { + firstRouteGroupSelected = (string)ctx.ProvidedValues[AttributeRouting.RouteGroupKey]; + ctx.IsBound = true; + }) + .Returns((string)null); + + var matchingRoutes = Enumerable.Empty(); + + var firstRoute = CreateGenerationEntry(firstTemplate, requiredValues: null, order: 1); + var secondRoute = CreateGenerationEntry(secondTemplate, requiredValues: null, order: 0); + + // We setup the route entries with a lower relative order and higher relative precedence + // first to ensure that when we try to generate a link, the route with the higher + // relative order gets tried first. + var linkGenerationEntries = new[] { firstRoute, secondRoute }; + + var route = new AttributeRoute(next.Object, matchingRoutes, linkGenerationEntries, NullLoggerFactory.Instance); + + var context = CreateVirtualPathContext(null, ambientValues: new { parameter = 5 }); + + // Act + string result = route.GetVirtualPath(context); + + // Assert + Assert.NotNull(result); + Assert.Equal("template/5", result); + Assert.Equal(selectedGroup, firstRouteGroupSelected); + } + + [Theory] + [InlineData("template/5", "template/5")] + [InlineData("template/{first:int}", "template/{second:int}")] + [InlineData("template/{first}", "template/{second}")] + [InlineData("template/{*first:int}", "template/{*second:int}")] + [InlineData("template/{*first}", "template/{*second}")] + public void AttributeRoute_GenerateLink_RespectsOrder(string firstTemplate, string secondTemplate) + { + // Arrange + var expectedGroup = CreateRouteGroup(0, secondTemplate); + + var next = new Mock(); + string selectedGroup = null; + next.Setup(n => n.GetVirtualPath(It.IsAny())).Callback(ctx => + { + selectedGroup = (string)ctx.ProvidedValues[AttributeRouting.RouteGroupKey]; + ctx.IsBound = true; + }) + .Returns((string)null); + + var matchingRoutes = Enumerable.Empty(); + + var firstRoute = CreateGenerationEntry(firstTemplate, requiredValues: null, order: 1); + var secondRoute = CreateGenerationEntry(secondTemplate, requiredValues: null, order: 0); + + // We setup the route entries with a lower relative order first to ensure that when + // we try to generate a link, the route with the higher relative order gets tried first. + var linkGenerationEntries = new[] { firstRoute, secondRoute }; + + var route = new AttributeRoute(next.Object, matchingRoutes, linkGenerationEntries, NullLoggerFactory.Instance); + + var context = CreateVirtualPathContext(values: null, ambientValues: new { first = 5, second = 5 }); + + // Act + string result = route.GetVirtualPath(context); + + // Assert + Assert.NotNull(result); + Assert.Equal("template/5", result); + Assert.Equal(expectedGroup, selectedGroup); + } + + [Theory] + [InlineData("first/5", "second/5")] + [InlineData("first/{first:int}", "second/{second:int}")] + [InlineData("first/{first}", "second/{second}")] + [InlineData("first/{*first:int}", "second/{*second:int}")] + [InlineData("first/{*first}", "second/{*second}")] + public void AttributeRoute_GenerateLink_EnsuresStableOrder(string firstTemplate, string secondTemplate) + { + // Arrange + var expectedGroup = CreateRouteGroup(0, firstTemplate); + + var next = new Mock(); + string selectedGroup = null; + next.Setup(n => n.GetVirtualPath(It.IsAny())).Callback(ctx => + { + selectedGroup = (string)ctx.ProvidedValues[AttributeRouting.RouteGroupKey]; + ctx.IsBound = true; + }) + .Returns((string)null); + + var matchingRoutes = Enumerable.Empty(); + + var firstRoute = CreateGenerationEntry(firstTemplate, requiredValues: null, order: 0); + var secondRoute = CreateGenerationEntry(secondTemplate, requiredValues: null, order: 0); + + // We setup the route entries with a lower relative template order first to ensure that when + // we try to generate a link, the route with the higher template order gets tried first. + var linkGenerationEntries = new[] { secondRoute, firstRoute }; + + var route = new AttributeRoute(next.Object, matchingRoutes, linkGenerationEntries, NullLoggerFactory.Instance); + + var context = CreateVirtualPathContext(values: null, ambientValues: new { first = 5, second = 5 }); + + // Act + string result = route.GetVirtualPath(context); + + // Assert + Assert.NotNull(result); + Assert.Equal("first/5", result); + Assert.Equal(expectedGroup, selectedGroup); + } + [Fact] public async void AttributeRoute_RouteAsyncHandled_LogsCorrectValues() { @@ -55,7 +416,7 @@ namespace Microsoft.AspNet.Mvc.Routing Assert.Equal(true, values.Handled); } - [Fact] + [Fact] public async void AttributeRoute_RouteAsyncNotHandled_LogsCorrectValues() { // Arrange @@ -291,7 +652,7 @@ namespace Microsoft.AspNet.Mvc.Routing var entry = CreateGenerationEntry("api/Store", new { action = "Index", controller = "Store" }); var route = CreateAttributeRoute(entry); - var context = CreateVirtualPathContext(new { action = "Index", id = 5}, new { controller = "Store" }); + var context = CreateVirtualPathContext(new { action = "Index", id = 5 }, new { controller = "Store" }); // Act var path = route.GetVirtualPath(context); @@ -356,7 +717,7 @@ namespace Microsoft.AspNet.Mvc.Routing // Reject entry 1. callCount++; return !c.ProvidedValues.Contains(new KeyValuePair( - AttributeRouting.RouteGroupKey, + AttributeRouting.RouteGroupKey, entry1.RouteGroup)); }; @@ -489,16 +850,34 @@ namespace Microsoft.AspNet.Mvc.Routing .Returns(NullLoggerFactory.Instance); return new VirtualPathContext( - mockHttpContext.Object, - new RouteValueDictionary(ambientValues), + mockHttpContext.Object, + new RouteValueDictionary(ambientValues), new RouteValueDictionary(values)); } - private static AttributeRouteLinkGenerationEntry CreateGenerationEntry(string template, object requiredValues) + private static AttributeRouteMatchingEntry CreateMatchingEntry(IRouter router, string template, int order) + { + var constraintResolver = CreateConstraintResolver(); + + var routeTemplate = TemplateParser.Parse(template, constraintResolver); + + var entry = new AttributeRouteMatchingEntry(); + entry.Route = new TemplateRoute(router, template, constraintResolver); + entry.Precedence = AttributeRoutePrecedence.Compute(routeTemplate); + entry.Order = order; + + string routeGroup = string.Format("{0}&&{1}", order, template); + entry.Route.Defaults.Add("test_route_group", routeGroup); + + return entry; + } + + private static AttributeRouteLinkGenerationEntry CreateGenerationEntry(string template, object requiredValues, int order = 0) { var constraintResolver = CreateConstraintResolver(); var entry = new AttributeRouteLinkGenerationEntry(); + entry.TemplateText = template; entry.Template = TemplateParser.Parse(template, constraintResolver); var defaults = entry.Template.Parameters @@ -512,9 +891,10 @@ namespace Microsoft.AspNet.Mvc.Routing entry.Constraints = constraints; entry.Defaults = defaults; entry.Binder = new TemplateBinder(entry.Template, defaults); + entry.Order = order; entry.Precedence = AttributeRoutePrecedence.Compute(entry.Template); entry.RequiredLinkValues = new RouteValueDictionary(requiredValues); - entry.RouteGroup = template; + entry.RouteGroup = CreateRouteGroup(order, template); return entry; } @@ -543,6 +923,11 @@ namespace Microsoft.AspNet.Mvc.Routing return entry; } + private static string CreateRouteGroup(int order, string template) + { + return string.Format("{0}&{1}", order, template); + } + private static DefaultInlineConstraintResolver CreateConstraintResolver() { var services = Mock.Of(); diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs index e6495a539a..b4b4c44661 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs @@ -76,7 +76,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests { { "controller", "Home" }, { "action", "Index" }, - }, + }, result.RouteValues); } @@ -360,7 +360,83 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests result.RouteValues); } - [Fact] + [Fact] + public async Task AttributeRoutedAction_OverrideActionOverridesOrderOnController() + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.Handler; + + // Act + var response = await client.GetAsync("http://localhost/Team/5"); + + // Assert + Assert.Equal(200, response.StatusCode); + + var body = await response.ReadBodyAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + Assert.Contains("/Team/5", result.ExpectedUrls); + Assert.Equal("Team", result.Controller); + Assert.Equal("GetOrganization", result.Action); + + Assert.Contains( + new KeyValuePair("teamId", "5"), + result.RouteValues); + } + + [Fact] + public async Task AttributeRoutedAction_OrderOnActionOverridesOrderOnController() + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.Handler; + + // Act + var response = await client.GetAsync("http://localhost/Teams"); + + // Assert + Assert.Equal(200, response.StatusCode); + + var body = await response.ReadBodyAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + Assert.Contains("/Teams", result.ExpectedUrls); + Assert.Equal("Team", result.Controller); + Assert.Equal("GetOrganizations", result.Action); + } + + [Fact] + public async Task AttributeRoutedAction_LinkGeneration_OverrideActionOverridesOrderOnController() + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.Handler; + + // Act + var response = await client.GetStringAsync("http://localhost/Organization/5"); + + // Assert + Assert.NotNull(response); + Assert.Equal("/Club/5", response); + } + + [Fact] + public async Task AttributeRoutedAction__LinkGeneration_OrderOnActionOverridesOrderOnController() + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.Handler; + + // Act + var response = await client.GetStringAsync("http://localhost/Teams/AllTeams"); + + // Assert + Assert.NotNull(response); + Assert.Equal("/Teams/AllOrganizations", response); + } + + [Fact] public async Task AttributeRoutedAction_LinkToSelf() { // Arrange diff --git a/test/WebSites/RoutingWebSite/Controllers/TeamController.cs b/test/WebSites/RoutingWebSite/Controllers/TeamController.cs new file mode 100644 index 0000000000..85989830c8 --- /dev/null +++ b/test/WebSites/RoutingWebSite/Controllers/TeamController.cs @@ -0,0 +1,64 @@ +using Microsoft.AspNet.Mvc; +using System; + +namespace RoutingWebSite +{ + [Route("/Teams", Order = 1)] + public class TeamController : Controller + { + private readonly TestResponseGenerator _generator; + + public TeamController(TestResponseGenerator generator) + { + _generator = generator; + } + + [HttpGet("/Team/{teamId}", Order = 2)] + public ActionResult GetTeam(int teamId) + { + return _generator.Generate("/Team/" + teamId); + } + + [HttpGet("/Team/{teamId}")] + public ActionResult GetOrganization(int teamId) + { + return _generator.Generate("/Team/" + teamId); + } + + [HttpGet("")] + public ActionResult GetTeams() + { + return _generator.Generate("/Teams"); + } + + [HttpGet("", Order = 0)] + public ActionResult GetOrganizations() + { + return _generator.Generate("/Teams"); + } + + [HttpGet("/Club/{clubId?}")] + public ActionResult GetClub() + { + return Content(Url.Action(),"text/plain"); + } + + [HttpGet("/Organization/{clubId?}", Order = 1)] + public ActionResult GetClub(int clubId) + { + return Content(Url.Action(), "text/plain"); + } + + [HttpGet("AllTeams")] + public ActionResult GetAllTeams() + { + return Content(Url.Action(), "text/plain"); + } + + [HttpGet("AllOrganizations", Order = 0)] + public ActionResult GetAllTeams(int notRelevant) + { + return Content(Url.Action(), "text/plain"); + } + } +} \ No newline at end of file diff --git a/test/WebSites/RoutingWebSite/HttpMergeAttribute.cs b/test/WebSites/RoutingWebSite/HttpMergeAttribute.cs index 9c10c7118a..d497aea828 100644 --- a/test/WebSites/RoutingWebSite/HttpMergeAttribute.cs +++ b/test/WebSites/RoutingWebSite/HttpMergeAttribute.cs @@ -24,6 +24,10 @@ namespace RoutingWebSite get { return _supportedMethods; } } + /// public string Template { get; private set; } + + /// + public int? Order { get; set; } } } \ No newline at end of file diff --git a/test/WebSites/RoutingWebSite/RoutingWebSite.kproj b/test/WebSites/RoutingWebSite/RoutingWebSite.kproj index 8eef3bf9bd..a7ccec35b7 100644 --- a/test/WebSites/RoutingWebSite/RoutingWebSite.kproj +++ b/test/WebSites/RoutingWebSite/RoutingWebSite.kproj @@ -36,6 +36,7 @@ +