diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/Abstractions/ActionDescriptor.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/Abstractions/ActionDescriptor.cs index 64ddd1b849..e54f05499c 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/Abstractions/ActionDescriptor.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/Abstractions/ActionDescriptor.cs @@ -16,7 +16,6 @@ namespace Microsoft.AspNetCore.Mvc.Abstractions Id = Guid.NewGuid().ToString(); Properties = new Dictionary(); RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase); - RouteValueDefaults = new Dictionary(StringComparer.OrdinalIgnoreCase); } /// @@ -32,8 +31,6 @@ namespace Microsoft.AspNetCore.Mvc.Abstractions public AttributeRouteInfo AttributeRouteInfo { get; set; } - public IDictionary RouteValueDefaults { get; set; } - /// /// The set of constraints for this action. Must all be satisfied for the action to be selected. /// diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ActionConstraints/ActionSelectorCandidate.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ActionConstraints/ActionSelectorCandidate.cs index 9cbe3aeeef..7a72a93c76 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ActionConstraints/ActionSelectorCandidate.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ActionConstraints/ActionSelectorCandidate.cs @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Mvc.ActionConstraints /// /// A candidate action for action selection. /// - public class ActionSelectorCandidate + public struct ActionSelectorCandidate { /// /// Creates a new . @@ -33,11 +33,11 @@ namespace Microsoft.AspNetCore.Mvc.ActionConstraints /// /// The representing a candiate for selection. /// - public ActionDescriptor Action { get; private set; } + public ActionDescriptor Action { get; } /// /// The list of instances associated with . /// - public IReadOnlyList Constraints { get; private set; } + public IReadOnlyList Constraints { get; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/AttributeRouteModel.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/AttributeRouteModel.cs index fd6f3a8f1b..7331898262 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/AttributeRouteModel.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/AttributeRouteModel.cs @@ -198,7 +198,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels return result.Substring(startIndex, subStringLength); } - public static string ReplaceTokens(string template, IDictionary values) + public static string ReplaceTokens(string template, IDictionary values) { var builder = new StringBuilder(); var state = TemplateParserState.Plaintext; @@ -340,7 +340,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels .Replace("[[", "[") .Replace("]]", "]"); - object value; + string value; if (!values.TryGetValue(token, out value)) { // Value not found diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Builder/MvcApplicationBuilderExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/Builder/MvcApplicationBuilderExtensions.cs index 3506ac7e0f..03451d62a1 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Builder/MvcApplicationBuilderExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Builder/MvcApplicationBuilderExtensions.cs @@ -3,9 +3,7 @@ using System; using Microsoft.AspNetCore.Mvc.Core; -using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.Internal; -using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; @@ -94,11 +92,7 @@ namespace Microsoft.AspNetCore.Builder configureRoutes(routes); - // Adding the attribute route comes after running the user-code because - // we want to respect any changes to the DefaultHandler. - routes.Routes.Insert(0, AttributeRouting.CreateAttributeMegaRoute( - routes.DefaultHandler, - app.ApplicationServices)); + routes.Routes.Insert(0, AttributeRouting.CreateAttributeMegaRoute(app.ApplicationServices)); return app.UseRouter(routes.Build()); } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ConsumesAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/ConsumesAttribute.cs index eaf3403a36..54b4d1bda0 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ConsumesAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ConsumesAttribute.cs @@ -142,7 +142,7 @@ namespace Microsoft.AspNetCore.Mvc } var firstCandidate = context.Candidates[0]; - if (firstCandidate != context.CurrentCandidate) + if (firstCandidate.Action != context.CurrentCandidate.Action) { // If the current candidate is not same as the first candidate, // we need not probe other candidates to see if they apply. @@ -157,7 +157,7 @@ namespace Microsoft.AspNetCore.Mvc // 3). If we have no matches, then we choose the first constraint to return true.It will later return a 415 foreach (var candidate in context.Candidates) { - if (candidate == firstCandidate) + if (candidate.Action == firstCandidate.Action) { continue; } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index aa6b6665b0..b67ce85911 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -217,9 +217,10 @@ namespace Microsoft.Extensions.DependencyInjection services.TryAddSingleton(); // - // Setup default handler + // Route Handlers // - services.TryAddSingleton(); + services.TryAddSingleton(); // Only one per app + services.TryAddTransient(); // Many per app } private static void ConfigureDefaultServices(IServiceCollection services) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IActionSelector.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IActionSelector.cs index d29a8bfac3..30c9902e7d 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IActionSelector.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/IActionSelector.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Collections.Generic; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Routing; @@ -12,13 +13,44 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure public interface IActionSelector { /// - /// Selects an for the request associated with . + /// Selects a set of candidates for the current request associated with + /// . /// - /// The for the current request. - /// An or null if no action can be selected. + /// The associated with the current request. + /// A set of candidates or null. + /// + /// + /// Used by conventional routing to select the set of actions that match the route values for the + /// current request. Action constraints associated with the candidates are not invoked by this method + /// + /// + /// Attribute routing does not call this method. + /// + /// + IReadOnlyList SelectCandidates(RouteContext context); + + /// + /// Selects the best candidate from for the + /// current request associated with . + /// + /// The associated with the current request. + /// The set of candidates. + /// The best candidate for the current request or null. /// /// Thrown when action selection results in an ambiguity. /// - ActionDescriptor Select(RouteContext context); + /// + /// + /// Invokes action constraints associated with the candidates. + /// + /// + /// Used by conventional routing after calling to apply action constraints and + /// disambiguate between multiple candidates. + /// + /// + /// Used by attribute routing to apply action constraints and disambiguate between multiple candidates. + /// + /// + ActionDescriptor SelectBestCandidate(RouteContext context, IReadOnlyList candidates); } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionSelectionDecisionTree.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionSelectionDecisionTree.cs index 24d0e065b8..f76a3662a6 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionSelectionDecisionTree.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionSelectionDecisionTree.cs @@ -6,9 +6,9 @@ using System.Collections.Generic; #if NET451 using System.ComponentModel; #endif +using System.Linq; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Infrastructure; -using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.DecisionTree; @@ -27,8 +27,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal { Version = actions.Version; + var conventionalRoutedActions = actions.Items.Where(a => a.AttributeRouteInfo?.Template == null).ToArray(); _root = DecisionTreeBuilder.GenerateTree( - actions.Items, + conventionalRoutedActions, new ActionDescriptorClassifier()); } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionSelector.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionSelector.cs index 1f54301bd5..ef4b6a3cfd 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionSelector.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionSelector.cs @@ -39,8 +39,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal _actionConstraintCache = actionConstraintCache; } - /// - public ActionDescriptor Select(RouteContext context) + public IReadOnlyList SelectCandidates(RouteContext context) { if (context == null) { @@ -48,35 +47,24 @@ namespace Microsoft.AspNetCore.Mvc.Internal } var tree = _decisionTreeProvider.DecisionTree; - var matchingRouteValues = tree.Select(context.RouteData.Values); + return tree.Select(context.RouteData.Values); + } - var candidates = new List(); - - // Perf: Avoid allocations - for (var i = 0; i < matchingRouteValues.Count; i++) + public ActionDescriptor SelectBestCandidate(RouteContext context, IReadOnlyList candidates) + { + if (context == null) { - var action = matchingRouteValues[i]; - var constraints = _actionConstraintCache.GetActionConstraints(context.HttpContext, action); - candidates.Add(new ActionSelectorCandidate(action, constraints)); + throw new ArgumentNullException(nameof(context)); } - var matchingActionConstraints = - EvaluateActionConstraints(context, candidates, startingOrder: null); - - List matchingActions = null; - if (matchingActionConstraints != null) + if (candidates == null) { - matchingActions = new List(matchingActionConstraints.Count); - // Perf: Avoid allocations - for (var i = 0; i < matchingActionConstraints.Count; i++) - { - var candidate = matchingActionConstraints[i]; - matchingActions.Add(candidate.Action); - } + throw new ArgumentNullException(nameof(candidates)); } - var finalMatches = SelectBestActions(matchingActions); + var matches = EvaluateActionConstraints(context, candidates); + var finalMatches = SelectBestActions(matches); if (finalMatches == null || finalMatches.Count == 0) { return null; @@ -113,7 +101,38 @@ namespace Microsoft.AspNetCore.Mvc.Internal return actions; } - private IReadOnlyList EvaluateActionConstraints( + private IReadOnlyList EvaluateActionConstraints( + RouteContext context, + IReadOnlyList actions) + { + var candidates = new List(); + + // Perf: Avoid allocations + for (var i = 0; i < actions.Count; i++) + { + var action = actions[i]; + var constraints = _actionConstraintCache.GetActionConstraints(context.HttpContext, action); + candidates.Add(new ActionSelectorCandidate(action, constraints)); + } + + var matches = EvaluateActionConstraintsCore(context, candidates, startingOrder: null); + + List results = null; + if (matches != null) + { + results = new List(matches.Count); + // Perf: Avoid allocations + for (var i = 0; i < matches.Count; i++) + { + var candidate = matches[i]; + results.Add(candidate.Action); + } + } + + return results; + } + + private IReadOnlyList EvaluateActionConstraintsCore( RouteContext context, IReadOnlyList candidates, int? startingOrder) @@ -198,7 +217,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // If we have matches with constraints, those are 'better' so try to keep processing those if (actionsWithConstraint.Count > 0) { - var matches = EvaluateActionConstraints(context, actionsWithConstraint, order); + var matches = EvaluateActionConstraintsCore(context, actionsWithConstraint, order); if (matches?.Count > 0) { return matches; @@ -212,7 +231,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } else { - return EvaluateActionConstraints(context, actionsWithoutConstraint, order); + return EvaluateActionConstraintsCore(context, actionsWithoutConstraint, order); } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/AttributeRoute.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/AttributeRoute.cs index ba01c23259..2caa02cbb2 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/AttributeRoute.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/AttributeRoute.cs @@ -8,32 +8,27 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.Infrastructure; -using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Template; using Microsoft.AspNetCore.Routing.Tree; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Internal; namespace Microsoft.AspNetCore.Mvc.Internal { public class AttributeRoute : IRouter { - private readonly IRouter _handler; private readonly IActionDescriptorCollectionProvider _actionDescriptorCollectionProvider; private readonly IServiceProvider _services; + private readonly Func _handlerFactory; private TreeRouter _router; public AttributeRoute( - IRouter handler, IActionDescriptorCollectionProvider actionDescriptorCollectionProvider, - IServiceProvider services) + IServiceProvider services, + Func handlerFactory) { - if (handler == null) - { - throw new ArgumentNullException(nameof(handler)); - } - if (actionDescriptorCollectionProvider == null) { throw new ArgumentNullException(nameof(actionDescriptorCollectionProvider)); @@ -44,9 +39,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal throw new ArgumentNullException(nameof(services)); } - _handler = handler; + if (handlerFactory == null) + { + _handlerFactory = handlerFactory; + } + _actionDescriptorCollectionProvider = actionDescriptorCollectionProvider; _services = services; + _handlerFactory = handlerFactory; } /// @@ -88,10 +88,18 @@ namespace Microsoft.AspNetCore.Mvc.Internal // action by expected route values, and then use the TemplateBinder to generate the link. foreach (var routeInfo in routeInfos) { + var defaults = new RouteValueDictionary(); + foreach (var kvp in routeInfo.ActionDescriptor.RouteValues) + { + defaults.Add(kvp.Key, kvp.Value); + } + + // We use the `NullRouter` as the route handler because we don't need to do anything for link + // generations. The TreeRouter does it all for us. builder.MapOutbound( - _handler, + NullRouter.Instance, routeInfo.RouteTemplate, - new RouteValueDictionary(routeInfo.ActionDescriptor.RouteValueDefaults), + defaults, routeInfo.RouteName, routeInfo.Order); } @@ -99,37 +107,27 @@ namespace Microsoft.AspNetCore.Mvc.Internal // We're creating one AttributeRouteMatchingEntry per group, so we need to identify the distinct set of // groups. It's guaranteed that all members of the group have the same template and precedence, // so we only need to hang on to a single instance of the RouteInfo for each group. - var distinctRouteInfosByGroup = GroupRouteInfosByGroupId(routeInfos); - foreach (var routeInfo in distinctRouteInfosByGroup) + var groups = GroupRouteInfos(routeInfos); + foreach (var group in groups) { + var handler = _handlerFactory(group.ToArray()); + // Note that because we only support 'inline' defaults, each routeInfo group also has the same // set of defaults. // // We then inject the route group as a default for the matcher so it gets passed back to MVC // for use in action selection. - var entry = builder.MapInbound( - _handler, - routeInfo.RouteTemplate, - routeInfo.RouteName, - routeInfo.Order); - - entry.Defaults[TreeRouter.RouteGroupKey] = routeInfo.RouteGroup; + builder.MapInbound( + handler, + group.Key.RouteTemplate, + group.Key.RouteName, + group.Key.Order); } } - private static IEnumerable GroupRouteInfosByGroupId(List routeInfos) + private static IEnumerable> GroupRouteInfos(List routeInfos) { - var routeInfosByGroupId = new Dictionary(StringComparer.OrdinalIgnoreCase); - - foreach (var routeInfo in routeInfos) - { - if (!routeInfosByGroupId.ContainsKey(routeInfo.RouteGroup)) - { - routeInfosByGroupId.Add(routeInfo.RouteGroup, routeInfo); - } - } - - return routeInfosByGroupId.Values; + return routeInfos.GroupBy(r => r, r => r.ActionDescriptor, RouteInfoEqualityComparer.Instance); } private static List GetRouteInfos(IReadOnlyList actions) @@ -179,23 +177,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal Dictionary templateCache, ActionDescriptor action) { - string value; - action.RouteValues.TryGetValue(TreeRouter.RouteGroupKey, out value); - - if (string.IsNullOrEmpty(value)) - { - // This can happen if an ActionDescriptor has a route template, but doesn't have one of our - // special route group constraints. This is a good indication that the user is using a 3rd party - // routing system, or has customized their ADs in a way that we can no longer understand them. - // - // We just treat this case as an 'opt-out' of our attribute routing system. - return null; - } - var routeInfo = new RouteInfo() { ActionDescriptor = action, - RouteGroup = value, }; try @@ -216,7 +200,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal return routeInfo; } - foreach (var kvp in action.RouteValueDefaults) + foreach (var kvp in action.RouteValues) { foreach (var parameter in routeInfo.RouteTemplate.Parameters) { @@ -246,11 +230,66 @@ namespace Microsoft.AspNetCore.Mvc.Internal public int Order { get; set; } - public string RouteGroup { get; set; } - public string RouteName { get; set; } public RouteTemplate RouteTemplate { get; set; } } + + private class RouteInfoEqualityComparer : IEqualityComparer + { + public static readonly RouteInfoEqualityComparer Instance = new RouteInfoEqualityComparer(); + + public bool Equals(RouteInfo x, RouteInfo y) + { + if (x == null && y == null) + { + return true; + } + else if (x == null ^ y == null) + { + return false; + } + else if (x.Order != y.Order) + { + return false; + } + else + { + return string.Equals( + x.RouteTemplate.TemplateText, + y.RouteTemplate.TemplateText, + StringComparison.OrdinalIgnoreCase); + } + } + + public int GetHashCode(RouteInfo obj) + { + if (obj == null) + { + return 0; + } + + var hash = new HashCodeCombiner(); + hash.Add(obj.Order); + hash.Add(obj.RouteTemplate.TemplateText, StringComparer.OrdinalIgnoreCase); + return hash; + } + } + + // Used only to hook up link generation, and it doesn't need to do anything. + private class NullRouter : IRouter + { + public static readonly NullRouter Instance = new NullRouter(); + + public VirtualPathData GetVirtualPath(VirtualPathContext context) + { + return null; + } + + public Task RouteAsync(RouteContext context) + { + throw new NotImplementedException(); + } + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/AttributeRouting.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/AttributeRouting.cs index 9f21e66105..e5bf2d5ec4 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/AttributeRouting.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/AttributeRouting.cs @@ -13,25 +13,24 @@ namespace Microsoft.AspNetCore.Mvc.Internal /// /// Creates an attribute route using the provided services and provided target router. /// - /// The router to invoke when a route entry matches. /// The application services. /// An attribute route. - public static IRouter CreateAttributeMegaRoute(IRouter target, IServiceProvider services) + public static IRouter CreateAttributeMegaRoute(IServiceProvider services) { - if (target == null) - { - throw new ArgumentNullException(nameof(target)); - } - if (services == null) { throw new ArgumentNullException(nameof(services)); } return new AttributeRoute( - target, services.GetRequiredService(), - services); + services, + actions => + { + var handler = services.GetRequiredService(); + handler.Actions = actions; + return handler; + }); } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs index 0d3c8a790d..dba88b0c20 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs @@ -36,7 +36,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal { var actions = new List(); - var hasAttributeRoutes = false; var routeValueKeys = new HashSet(StringComparer.OrdinalIgnoreCase); var methodInfoMap = new MethodToActionMap(); @@ -71,21 +70,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal AddProperties(actionDescriptor, action, controller, application); actionDescriptor.BoundProperties = controllerPropertyDescriptors; + if (IsAttributeRoutedAction(actionDescriptor)) { - hasAttributeRoutes = true; - - // An attribute routed action will ignore conventional routed constraints. We still - // want to provide these values as ambient values for link generation. - AddRouteValuesAsDefaultRouteValues(actionDescriptor); - // Replaces tokens like [controller]/[action] in the route template with the actual values // for this action. ReplaceAttributeRouteTokens(actionDescriptor, routeTemplateErrors); - - // Attribute routed actions will ignore conventional routed values. Instead they have - // a single route value "RouteGroup" associated with it. - ReplaceRouteValues(actionDescriptor); } } @@ -113,44 +103,19 @@ namespace Microsoft.AspNetCore.Mvc.Internal validatedMethods.Add(actionDescriptor.MethodInfo); } - if (!IsAttributeRoutedAction(actionDescriptor)) + var attributeRouteInfo = actionDescriptor.AttributeRouteInfo; + if (attributeRouteInfo?.Name != null) { - // Any attribute routes are in use, then non-attribute-routed action descriptors can't be - // selected when a route group returned by the route. - if (hasAttributeRoutes) - { - actionDescriptor.RouteValues.Add(TreeRouter.RouteGroupKey, string.Empty); - } - - // Add a route value with 'null' for each user-defined route value in the set to all the - // actions that don't have that value. For example, if a controller defines - // an area, all actions that don't belong to an area must have a route - // value that prevents them from matching an incoming request when area is specified. - AddGlobalRouteValues(actionDescriptor, routeValueKeys); + // Build a map of attribute route name to action descriptors to ensure that all + // attribute routes with a given name have the same template. + AddActionToNamedGroup(actionsByRouteName, attributeRouteInfo.Name, actionDescriptor); } - else - { - var attributeRouteInfo = actionDescriptor.AttributeRouteInfo; - if (attributeRouteInfo.Name != null) - { - // Build a map of attribute route name to action descriptors to ensure that all - // attribute routes with a given name have the same template. - AddActionToNamedGroup(actionsByRouteName, attributeRouteInfo.Name, actionDescriptor); - } - // 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 routeValueKeys) - { - if (!actionDescriptor.RouteValueDefaults.ContainsKey(key)) - { - actionDescriptor.RouteValueDefaults.Add(key, value: null); - } - } - } + // Add a route value with 'null' for each user-defined route value in the set to all the + // actions that don't have that value. For example, if a controller defines + // an area, all actions that don't belong to an area must have a route + // value that prevents them from matching an incoming request when area is specified. + AddGlobalRouteValues(actionDescriptor, routeValueKeys); } if (attributeRoutingConfigurationErrors.Any()) @@ -459,7 +424,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal foreach (var kvp in action.RouteValues) { keys.Add(kvp.Key); - + // Skip duplicates if (!actionDescriptor.RouteValues.ContainsKey(kvp.Key)) { @@ -490,16 +455,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal } } - private static void ReplaceRouteValues(ControllerActionDescriptor actionDescriptor) - { - var routeGroupValue = GetRouteGroupValue( - actionDescriptor.AttributeRouteInfo.Order, - actionDescriptor.AttributeRouteInfo.Template); - - actionDescriptor.RouteValues.Clear(); - actionDescriptor.RouteValues.Add(TreeRouter.RouteGroupKey, routeGroupValue); - } - private static void ReplaceAttributeRouteTokens( ControllerActionDescriptor actionDescriptor, IList routeTemplateErrors) @@ -508,13 +463,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal { actionDescriptor.AttributeRouteInfo.Template = AttributeRouteModel.ReplaceTokens( actionDescriptor.AttributeRouteInfo.Template, - actionDescriptor.RouteValueDefaults); + actionDescriptor.RouteValues); if (actionDescriptor.AttributeRouteInfo.Name != null) { actionDescriptor.AttributeRouteInfo.Name = AttributeRouteModel.ReplaceTokens( actionDescriptor.AttributeRouteInfo.Name, - actionDescriptor.RouteValueDefaults); + actionDescriptor.RouteValues); } } catch (InvalidOperationException ex) @@ -530,14 +485,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal } } - private static void AddRouteValuesAsDefaultRouteValues(ControllerActionDescriptor actionDescriptor) - { - foreach (var kvp in actionDescriptor.RouteValues) - { - actionDescriptor.RouteValueDefaults.Add(kvp.Key, kvp.Value); - } - } - private static void AddGlobalRouteValues( ControllerActionDescriptor actionDescriptor, ISet removalConstraints) @@ -731,12 +678,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal return message; } - private static string GetRouteGroupValue(int order, string template) - { - var group = string.Format(CultureInfo.InvariantCulture, "{0}-{1}", order, template); - return ("__route__" + group).ToUpperInvariant(); - } - // We need to build a map of methods to reflected actions and reflected actions to // action descriptors so that we can validate later that no method produced attribute // and non attributed actions at the same time, and that no method that produced attribute diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcAttributeRouteHandler.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcAttributeRouteHandler.cs new file mode 100644 index 0000000000..4324adb3a4 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcAttributeRouteHandler.cs @@ -0,0 +1,117 @@ +// Copyright (c) .NET Foundation. 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.Diagnostics; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Core; +using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.Logging; + +namespace Microsoft.AspNetCore.Mvc.Internal +{ + public class MvcAttributeRouteHandler : IRouter + { + private IActionContextAccessor _actionContextAccessor; + private IActionInvokerFactory _actionInvokerFactory; + private IActionSelector _actionSelector; + private ILogger _logger; + private DiagnosticSource _diagnosticSource; + + public MvcAttributeRouteHandler( + IActionInvokerFactory actionInvokerFactory, + IActionSelector actionSelector, + DiagnosticSource diagnosticSource, + ILoggerFactory loggerFactory) + : this(actionInvokerFactory, actionSelector, diagnosticSource, loggerFactory, actionContextAccessor: null) + { + } + + public MvcAttributeRouteHandler( + IActionInvokerFactory actionInvokerFactory, + IActionSelector actionSelector, + DiagnosticSource diagnosticSource, + ILoggerFactory loggerFactory, + IActionContextAccessor actionContextAccessor) + { + // The IActionContextAccessor is optional. We want to avoid the overhead of using CallContext + // if possible. + _actionContextAccessor = actionContextAccessor; + + _actionInvokerFactory = actionInvokerFactory; + _actionSelector = actionSelector; + _diagnosticSource = diagnosticSource; + _logger = loggerFactory.CreateLogger(); + } + + public ActionDescriptor[] Actions { get; set; } + + public VirtualPathData GetVirtualPath(VirtualPathContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + // We return null here because we're not responsible for generating the url, the route is. + return null; + } + + public Task RouteAsync(RouteContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (Actions == null) + { + var message = Resources.FormatPropertyOfTypeCannotBeNull( + nameof(Actions), + nameof(MvcAttributeRouteHandler)); + throw new InvalidOperationException(message); + } + + var actionDescriptor = _actionSelector.SelectBestCandidate(context, Actions); + if (actionDescriptor == null) + { + _logger.NoActionsMatched(); + return TaskCache.CompletedTask; + } + + foreach (var kvp in actionDescriptor.RouteValues) + { + if (!string.IsNullOrEmpty(kvp.Value)) + { + context.RouteData.Values[kvp.Key] = kvp.Value; + } + } + + context.Handler = (c) => + { + var routeData = c.GetRouteData(); + + var actionContext = new ActionContext(context.HttpContext, routeData, actionDescriptor); + if (_actionContextAccessor != null) + { + _actionContextAccessor.ActionContext = actionContext; + } + + var invoker = _actionInvokerFactory.CreateInvoker(actionContext); + if (invoker == null) + { + throw new InvalidOperationException( + Resources.FormatActionInvokerFactory_CouldNotCreateInvoker( + actionDescriptor.DisplayName)); + } + + return invoker.InvokeAsync(); + }; + + return TaskCache.CompletedTask; + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcRouteHandler.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcRouteHandler.cs index 51ee379b83..9f5e84a505 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcRouteHandler.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcRouteHandler.cs @@ -7,7 +7,6 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Routing; -using Microsoft.AspNetCore.Routing.Tree; using Microsoft.Extensions.Logging; namespace Microsoft.AspNetCore.Mvc.Internal @@ -64,28 +63,21 @@ namespace Microsoft.AspNetCore.Mvc.Internal throw new ArgumentNullException(nameof(context)); } - var actionDescriptor = _actionSelector.Select(context); + var candidates = _actionSelector.SelectCandidates(context); + if (candidates == null || candidates.Count == 0) + { + _logger.NoActionsMatched(); + return TaskCache.CompletedTask; + } + + var actionDescriptor = _actionSelector.SelectBestCandidate(context, candidates); if (actionDescriptor == null) { _logger.NoActionsMatched(); return TaskCache.CompletedTask; } - if (actionDescriptor.RouteValueDefaults != null) - { - foreach (var kvp in actionDescriptor.RouteValueDefaults) - { - if (!context.RouteData.Values.ContainsKey(kvp.Key)) - { - context.RouteData.Values.Add(kvp.Key, kvp.Value); - } - } - - // Removing RouteGroup from RouteValues to simulate the result of conventional routing - context.RouteData.Values.Remove(TreeRouter.RouteGroupKey); - } - - context.Handler = async (c) => + context.Handler = (c) => { var routeData = c.GetRouteData(); @@ -103,10 +95,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal actionDescriptor.DisplayName)); } - await invoker.InvokeAsync(); + return invoker.InvokeAsync(); }; return TaskCache.CompletedTask; } } -} +} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs b/src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs index 3bce3cf339..b44647f7c6 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs @@ -91,8 +91,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor /// /// The casing of a route value in is determined by the client. /// This making constructing paths for view locations in a case sensitive file system unreliable. Using the - /// for attribute routes and - /// for traditional routes to get route values + /// to get route values /// produces consistently cased results. /// public static string GetNormalizedRouteValue(ActionContext context, string key) @@ -115,22 +114,12 @@ namespace Microsoft.AspNetCore.Mvc.Razor var actionDescriptor = context.ActionDescriptor; string normalizedValue = null; - if (actionDescriptor.AttributeRouteInfo != null) + + string value; + if (actionDescriptor.RouteValues.TryGetValue(key, out value) && + !string.IsNullOrEmpty(value)) { - object match; - if (actionDescriptor.RouteValueDefaults.TryGetValue(key, out match)) - { - normalizedValue = match?.ToString(); - } - } - else - { - string value; - if (actionDescriptor.RouteValues.TryGetValue(key, out value) && - !string.IsNullOrEmpty(value)) - { - normalizedValue = value; - } + normalizedValue = value; } var stringRouteValue = routeValue?.ToString(); diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/PartialViewResultExecutor.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/PartialViewResultExecutor.cs index 67cb50784d..cd8b476835 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/PartialViewResultExecutor.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/PartialViewResultExecutor.cs @@ -179,22 +179,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal var actionDescriptor = context.ActionDescriptor; string normalizedValue = null; - if (actionDescriptor.AttributeRouteInfo != null) + string value; + if (actionDescriptor.RouteValues.TryGetValue(ActionNameKey, out value) && + !string.IsNullOrEmpty(value)) { - object match; - if (actionDescriptor.RouteValueDefaults.TryGetValue(ActionNameKey, out match)) - { - normalizedValue = match?.ToString(); - } - } - else - { - string value; - if (actionDescriptor.RouteValues.TryGetValue(ActionNameKey, out value) && - !string.IsNullOrEmpty(value)) - { - normalizedValue = value; - } + normalizedValue = value; } var stringRouteValue = routeValue?.ToString(); diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewResultExecutor.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewResultExecutor.cs index 88b5071b91..6f94efc4b8 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewResultExecutor.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ViewResultExecutor.cs @@ -193,22 +193,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal var actionDescriptor = context.ActionDescriptor; string normalizedValue = null; - if (actionDescriptor.AttributeRouteInfo != null) + string value; + if (actionDescriptor.RouteValues.TryGetValue(ActionNameKey, out value) && + !string.IsNullOrEmpty(value)) { - object match; - if (actionDescriptor.RouteValueDefaults.TryGetValue(ActionNameKey, out match)) - { - normalizedValue = match?.ToString(); - } - } - else - { - string value; - if (actionDescriptor.RouteValues.TryGetValue(ActionNameKey, out value) && - !string.IsNullOrEmpty(value)) - { - normalizedValue = value; - } + normalizedValue = value; } var stringRouteValue = routeValue?.ToString(); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModel/AttributeRouteModelTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModel/AttributeRouteModelTests.cs index 1131fc9b8d..ab1a76ee0b 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModel/AttributeRouteModelTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModel/AttributeRouteModelTests.cs @@ -146,17 +146,11 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels [Theory] [MemberData(nameof(ReplaceTokens_ValueValuesData))] - public void ReplaceTokens_ValidValues(string template, object values, string expected) + public void ReplaceTokens_ValidValues(string template, Dictionary values, string expected) { // Arrange - var valuesDictionary = values as IDictionary; - if (valuesDictionary == null) - { - valuesDictionary = new RouteValueDictionary(values); - } - // Act - var result = AttributeRouteModel.ReplaceTokens(template, valuesDictionary); + var result = AttributeRouteModel.ReplaceTokens(template, values); // Assert Assert.Equal(expected, result); @@ -164,15 +158,9 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels [Theory] [MemberData(nameof(ReplaceTokens_InvalidFormatValuesData))] - public void ReplaceTokens_InvalidFormat(string template, object values, string reason) + public void ReplaceTokens_InvalidFormat(string template, Dictionary 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, @@ -180,7 +168,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels // Act var ex = Assert.Throws( - () => { AttributeRouteModel.ReplaceTokens(template, valuesDictionary); }); + () => { AttributeRouteModel.ReplaceTokens(template, values); }); // Assert Assert.Equal(expected, ex.Message); @@ -191,7 +179,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels { // Arrange var template = "[area]/[controller]/[action2]"; - var values = new RouteValueDictionary() + var values = new Dictionary(StringComparer.OrdinalIgnoreCase) { { "area", "Help" }, { "controller", "Admin" }, @@ -428,49 +416,73 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels yield return new object[] { "[controller]/[action]", - new { controller = "Home", action = "Index" }, + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "controller", "Home" }, + { "action", "Index" } + }, "Home/Index" }; yield return new object[] { "[controller]", - new { controller = "Home", action = "Index" }, + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "controller", "Home" }, + { "action", "Index" } + }, "Home" }; yield return new object[] { "[controller][[", - new { controller = "Home", action = "Index" }, + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "controller", "Home" }, + { "action", "Index" } + }, "Home[" }; yield return new object[] { "[coNTroller]", - new { contrOLler = "Home", action = "Index" }, + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "controller", "Home" }, + { "action", "Index" } + }, "Home" }; yield return new object[] { "thisisSomeText[action]", - new { controller = "Home", action = "Index" }, + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "controller", "Home" }, + { "action", "Index" } + }, "thisisSomeTextIndex" }; yield return new object[] { "[[-]][[/[[controller]]", - new { controller = "Home", action = "Index" }, + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "controller", "Home" }, + { "action", "Index" } + }, "[-][/[controller]" }; yield return new object[] { "[contr[[oller]/[act]]ion]", - new Dictionary(StringComparer.OrdinalIgnoreCase) + new Dictionary(StringComparer.OrdinalIgnoreCase) { { "contr[oller", "Home" }, { "act]ion", "Index" } @@ -481,7 +493,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels yield return new object[] { "[controller][action]", - new Dictionary(StringComparer.OrdinalIgnoreCase) + new Dictionary(StringComparer.OrdinalIgnoreCase) { { "controller", "Home" }, { "action", "Index" } @@ -492,7 +504,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels yield return new object[] { "[contr}oller]/[act{ion]/{id}", - new Dictionary(StringComparer.OrdinalIgnoreCase) + new Dictionary(StringComparer.OrdinalIgnoreCase) { { "contr}oller", "Home" }, { "act{ion", "Index" } @@ -509,35 +521,35 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels yield return new object[] { "[", - new { }, + new Dictionary(StringComparer.OrdinalIgnoreCase), "A replacement token is not closed." }; yield return new object[] { "text]", - new { }, + new Dictionary(StringComparer.OrdinalIgnoreCase), "Token delimiters ('[', ']') are imbalanced.", }; yield return new object[] { "text]morecooltext", - new { }, + new Dictionary(StringComparer.OrdinalIgnoreCase), "Token delimiters ('[', ']') are imbalanced.", }; yield return new object[] { "[action", - new { }, + new Dictionary(StringComparer.OrdinalIgnoreCase), "A replacement token is not closed.", }; yield return new object[] { "[action]]][", - new RouteValueDictionary() + new Dictionary(StringComparer.OrdinalIgnoreCase) { { "action]", "Index" } }, @@ -547,21 +559,21 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels yield return new object[] { "[action]]", - new { }, + new Dictionary(StringComparer.OrdinalIgnoreCase), "A replacement token is not closed." }; yield return new object[] { "[ac[tion]", - new { }, + new Dictionary(StringComparer.OrdinalIgnoreCase), "An unescaped '[' token is not allowed inside of a replacement token. Use '[[' to escape." }; yield return new object[] { "[]", - new { }, + new Dictionary(StringComparer.OrdinalIgnoreCase), "An empty replacement token ('[]') is not allowed.", }; } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/DefaultActionSelectorTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/DefaultActionSelectorTests.cs index 288c4a76c3..71ffc0f7f6 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/DefaultActionSelectorTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/DefaultActionSelectorTests.cs @@ -8,7 +8,6 @@ using System.Reflection; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ActionConstraints; -using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.ApplicationParts; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Internal; @@ -22,10 +21,159 @@ using Xunit; namespace Microsoft.AspNetCore.Mvc.Infrastructure { + // Most of the in-depth testing for SelectCandidates is part of the descision tree tests. + // This is just basic coverage of the API in common scenarios. public class DefaultActionSelectorTests { [Fact] - public void Select_AmbiguousActions_LogIsCorrect() + public void SelectCandidates_SingleMatch() + { + var actions = new ActionDescriptor[] + { + new ActionDescriptor() + { + DisplayName = "A1", + RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "controller", "Home" }, + { "action", "Index" } + }, + }, + new ActionDescriptor() + { + DisplayName = "A2", + RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "controller", "Home" }, + { "action", "About" } + }, + }, + }; + + var selector = CreateSelector(actions); + + var routeContext = CreateRouteContext("GET"); + routeContext.RouteData.Values.Add("controller", "Home"); + routeContext.RouteData.Values.Add("action", "Index"); + + // Act + var candidates = selector.SelectCandidates(routeContext); + + // Assert + Assert.Collection(candidates, (a) => Assert.Same(actions[0], a)); + } + + [Fact] + public void SelectCandidates_MultipleMatches() + { + var actions = new ActionDescriptor[] + { + new ActionDescriptor() + { + DisplayName = "A1", + RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "controller", "Home" }, + { "action", "Index" } + }, + }, + new ActionDescriptor() + { + DisplayName = "A2", + RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "controller", "Home" }, + { "action", "Index" } + }, + }, + }; + + var selector = CreateSelector(actions); + + var routeContext = CreateRouteContext("GET"); + routeContext.RouteData.Values.Add("controller", "Home"); + routeContext.RouteData.Values.Add("action", "Index"); + + // Act + var candidates = selector.SelectCandidates(routeContext); + + // Assert + Assert.Equal(actions.ToArray(), candidates.ToArray()); + } + + [Fact] + public void SelectCandidates_NoMatch() + { + var actions = new ActionDescriptor[] + { + new ActionDescriptor() + { + DisplayName = "A1", + RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "controller", "Home" }, + { "action", "Index" } + }, + }, + new ActionDescriptor() + { + DisplayName = "A2", + RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "controller", "Home" }, + { "action", "About" } + }, + }, + }; + + var selector = CreateSelector(actions); + + var routeContext = CreateRouteContext("GET"); + routeContext.RouteData.Values.Add("controller", "Foo"); + routeContext.RouteData.Values.Add("action", "Index"); + + // Act + var candidates = selector.SelectCandidates(routeContext); + + // Assert + Assert.Empty(candidates); + } + + [Fact] + public void SelectCandidates_NoMatch_ExcludesAttributeRoutedActions() + { + var actions = new ActionDescriptor[] + { + new ActionDescriptor() + { + DisplayName = "A1", + RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "controller", "Home" }, + { "action", "Index" } + }, + AttributeRouteInfo = new AttributeRouteInfo() + { + Template = "/Home", + } + }, + }; + + var selector = CreateSelector(actions); + + var routeContext = CreateRouteContext("GET"); + routeContext.RouteData.Values.Add("controller", "Home"); + routeContext.RouteData.Values.Add("action", "Index"); + + // Act + var candidates = selector.SelectCandidates(routeContext); + + // Assert + Assert.Empty(candidates); + } + + [Fact] + public void SelectBestCandidate_AmbiguousActions_LogIsCorrect() { // Arrange var sink = new TestSink(); @@ -44,7 +192,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure $"ambiguity. Matching actions: {actionNames}"; // Act - Assert.Throws(() => { selector.Select(routeContext); }); + Assert.Throws(() => { selector.SelectBestCandidate(routeContext, actions); }); // Assert Assert.Empty(sink.Scopes); @@ -53,7 +201,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure } [Fact] - public void Select_PrefersActionWithConstraints() + public void SelectBestCandidate_PrefersActionWithConstraints() { // Arrange var actionWithConstraints = new ActionDescriptor() @@ -76,14 +224,14 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure var context = CreateRouteContext("POST"); // Act - var action = selector.Select(context); + var action = selector.SelectBestCandidate(context, actions); // Assert Assert.Same(action, actionWithConstraints); } [Fact] - public void Select_ConstraintsRejectAll() + public void SelectBestCandidate_ConstraintsRejectAll() { // Arrange var action1 = new ActionDescriptor() @@ -108,14 +256,14 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure var context = CreateRouteContext("POST"); // Act - var action = selector.Select(context); + var action = selector.SelectBestCandidate(context, actions); // Assert Assert.Null(action); } [Fact] - public void Select_ConstraintsRejectAll_DifferentStages() + public void SelectBestCandidate_ConstraintsRejectAll_DifferentStages() { // Arrange var action1 = new ActionDescriptor() @@ -142,14 +290,14 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure var context = CreateRouteContext("POST"); // Act - var action = selector.Select(context); + var action = selector.SelectBestCandidate(context, actions); // Assert Assert.Null(action); } [Fact] - public void Select_ActionConstraintFactory() + public void SelectBestCandidate_ActionConstraintFactory() { // Arrange var actionWithConstraints = new ActionDescriptor() @@ -174,14 +322,14 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure var context = CreateRouteContext("POST"); // Act - var action = selector.Select(context); + var action = selector.SelectBestCandidate(context, actions); // Assert Assert.Same(action, actionWithConstraints); } [Fact] - public void Select_ActionConstraintFactory_ReturnsNull() + public void SelectBestCandidate_ActionConstraintFactory_ReturnsNull() { // Arrange var nullConstraint = new ActionDescriptor() @@ -200,7 +348,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure var context = CreateRouteContext("POST"); // Act - var action = selector.Select(context); + var action = selector.SelectBestCandidate(context, actions); // Assert Assert.Same(action, nullConstraint); @@ -208,7 +356,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure // There's a custom constraint provider registered that only understands BooleanConstraintMarker [Fact] - public void Select_CustomProvider() + public void SelectBestCandidate_CustomProvider() { // Arrange var actionWithConstraints = new ActionDescriptor() @@ -230,7 +378,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure var context = CreateRouteContext("POST"); // Act - var action = selector.Select(context); + var action = selector.SelectBestCandidate(context, actions); // Assert Assert.Same(action, actionWithConstraints); @@ -238,7 +386,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure // Due to ordering of stages, the first action will be better. [Fact] - public void Select_ConstraintsInOrder() + public void SelectBestCandidate_ConstraintsInOrder() { // Arrange var best = new ActionDescriptor() @@ -263,7 +411,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure var context = CreateRouteContext("POST"); // Act - var action = selector.Select(context); + var action = selector.SelectBestCandidate(context, actions); // Assert Assert.Same(action, best); @@ -271,7 +419,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure // Due to ordering of stages, the first action will be better. [Fact] - public void Select_ConstraintsInOrder_MultipleStages() + public void SelectBestCandidate_ConstraintsInOrder_MultipleStages() { // Arrange var best = new ActionDescriptor() @@ -300,14 +448,14 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure var context = CreateRouteContext("POST"); // Act - var action = selector.Select(context); + var action = selector.SelectBestCandidate(context, actions); // Assert Assert.Same(action, best); } [Fact] - public void Select_Fallback_ToActionWithoutConstraints() + public void SelectBestCandidate_Fallback_ToActionWithoutConstraints() { // Arrange var nomatch1 = new ActionDescriptor() @@ -338,14 +486,14 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure var context = CreateRouteContext("POST"); // Act - var action = selector.Select(context); + var action = selector.SelectBestCandidate(context, actions); // Assert Assert.Same(action, best); } [Fact] - public void Select_Ambiguous() + public void SelectBestCandidate_Ambiguous() { // Arrange var expectedMessage = @@ -359,7 +507,6 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure { CreateAction(area: null, controller: "Store", action: "Buy"), CreateAction(area: null, controller: "Store", action: "Buy"), - CreateAction(area: null, controller: "Store", action: "Cart"), }; actions[0].DisplayName = "Ambiguous1"; @@ -374,7 +521,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure // Act var ex = Assert.Throws(() => { - selector.Select(context); + selector.SelectBestCandidate(context, actions); }); // Assert @@ -528,12 +675,13 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure new DefaultActionConstraintProvider(), }; - var defaultActionSelector = new ActionSelector( + var actionSelector = new ActionSelector( decisionTreeProvider, GetActionConstraintCache(actionConstraintProviders), NullLoggerFactory.Instance); - return (ControllerActionDescriptor)defaultActionSelector.Select(context); + var candidates = actionSelector.SelectCandidates(context); + return (ControllerActionDescriptor)actionSelector.SelectBestCandidate(context, candidates); } private ControllerActionDescriptorProvider GetActionDescriptorProvider() diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/MvcRouteHandlerTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/MvcRouteHandlerTests.cs index c3f18b8ec1..e6c2e23e5a 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/MvcRouteHandlerTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/MvcRouteHandlerTests.cs @@ -2,6 +2,7 @@ // 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 System.Diagnostics; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; @@ -28,8 +29,8 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure var mockActionSelector = new Mock(); mockActionSelector - .Setup(a => a.Select(It.IsAny())) - .Returns(null); + .Setup(a => a.SelectCandidates(It.IsAny())) + .Returns(new ActionDescriptor[0]); var context = CreateRouteContext(); @@ -48,39 +49,6 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure Assert.Equal(expectedMessage, sink.Writes[0].State?.ToString()); } - [Fact] - public async Task RouteHandler_RemovesRouteGroupFromRouteValues() - { - // Arrange - var invoker = new Mock(); - invoker - .Setup(i => i.InvokeAsync()) - .Returns(Task.FromResult(true)); - - var invokerFactory = new Mock(); - invokerFactory - .Setup(f => f.CreateInvoker(It.IsAny())) - .Returns((c) => - { - return invoker.Object; - }); - - var context = CreateRouteContext(); - var handler = CreateMvcRouteHandler(invokerFactory: invokerFactory.Object); - - var originalRouteData = context.RouteData; - originalRouteData.Values.Add(TreeRouter.RouteGroupKey, "/Home/Test"); - - // Act - await handler.RouteAsync(context); - - // Assert - Assert.Same(originalRouteData, context.RouteData); - - - Assert.False(context.RouteData.Values.ContainsKey(TreeRouter.RouteGroupKey)); - } - private MvcRouteHandler CreateMvcRouteHandler( ActionDescriptor actionDescriptor = null, IActionSelector actionSelector = null, @@ -99,7 +67,12 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure if (actionSelector == null) { var mockActionSelector = new Mock(); - mockActionSelector.Setup(a => a.Select(It.IsAny())) + mockActionSelector + .Setup(a => a.SelectCandidates(It.IsAny())) + .Returns(new ActionDescriptor[] { actionDescriptor }); + + mockActionSelector + .Setup(a => a.SelectBestCandidate(It.IsAny(), It.IsAny>())) .Returns(actionDescriptor); actionSelector = mockActionSelector.Object; } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AttributeRouteTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AttributeRouteTest.cs index 6b21dea251..2e99324d32 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AttributeRouteTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AttributeRouteTest.cs @@ -32,7 +32,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal public async Task AttributeRoute_UsesUpdatedActionDescriptors() { // Arrange - var handler = CreateHandler(); + ActionDescriptor selected = null; var actions = new List() { @@ -40,28 +40,45 @@ namespace Microsoft.AspNetCore.Mvc.Internal { AttributeRouteInfo = new AttributeRouteInfo() { - Template = "api/Blog/{id}" - }, - RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { TreeRouter.RouteGroupKey, "1" } + Template = "api/Blog/{key1}" }, }, new ActionDescriptor() { AttributeRouteInfo = new AttributeRouteInfo() { - Template = "api/Store/Buy/{id}" - }, - RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { TreeRouter.RouteGroupKey, "2" } + Template = "api/Store/Buy/{key2}" }, }, }; + + Func handlerFactory = (_) => + { + var handler = new Mock(); + handler + .Setup(r => r.RouteAsync(It.IsAny())) + .Returns(routeContext => + { + if (routeContext.RouteData.Values.ContainsKey("key1")) + { + selected = actions[0]; + } + else if (routeContext.RouteData.Values.ContainsKey("key2")) + { + selected = actions[1]; + } + + routeContext.Handler = (c) => TaskCache.CompletedTask; + + return TaskCache.CompletedTask; + + }); + return handler.Object; + }; + var actionDescriptorProvider = CreateActionDescriptorProvider(actions); - var route = CreateRoute(handler.Object, actionDescriptorProvider.Object); + var route = CreateRoute(handlerFactory, actionDescriptorProvider.Object); var requestServices = new Mock(MockBehavior.Strict); requestServices @@ -79,12 +96,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert 1 Assert.NotNull(context.Handler); - Assert.Equal("5", context.RouteData.Values["id"]); - Assert.Equal("2", context.RouteData.Values[TreeRouter.RouteGroupKey]); - - handler.Verify(h => h.RouteAsync(It.IsAny()), Times.Once()); - + Assert.Equal("5", context.RouteData.Values["key2"]); + Assert.Same(actions[1], selected); + // Arrange 2 - remove the action and update the collection + selected = null; actions.RemoveAt(1); actionDescriptorProvider .SetupGet(ad => ad.ActionDescriptors) @@ -98,8 +114,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert 2 Assert.Null(context.Handler); Assert.Empty(context.RouteData.Values); - - handler.Verify(h => h.RouteAsync(It.IsAny()), Times.Once()); + Assert.Null(selected); } [Fact] @@ -117,10 +132,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal Order = 17, }, RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { TreeRouter.RouteGroupKey, "1" } - }, - RouteValueDefaults = new Dictionary() { { "controller", "Blog" }, { "action", "Index" }, @@ -145,7 +156,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal(RoutePrecedence.ComputeOutbound(e.RouteTemplate), e.Precedence); Assert.Equal("BLOG_INDEX", e.RouteName); Assert.Equal(17, e.Order); - Assert.Equal(actions[0].RouteValueDefaults.ToArray(), e.RequiredLinkValues.ToArray()); + Assert.Equal(ToRouteValueDictionary(actions[0].RouteValues), e.RequiredLinkValues); Assert.Equal("api/Blog/{id}", e.RouteTemplate.TemplateText); }); } @@ -165,10 +176,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal Order = 17, }, RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { TreeRouter.RouteGroupKey, "1" } - }, - RouteValueDefaults = new Dictionary() { { "controller", "Blog" }, { "action", "Index" }, @@ -193,7 +200,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal(RoutePrecedence.ComputeOutbound(e.RouteTemplate), e.Precedence); Assert.Equal("BLOG_INDEX", e.RouteName); Assert.Equal(17, e.Order); - Assert.Equal(actions[0].RouteValueDefaults.ToArray(), e.RequiredLinkValues.ToArray()); + Assert.Equal(ToRouteValueDictionary(actions[0].RouteValues), e.RequiredLinkValues); Assert.Equal("api/Blog/{id:int}", e.RouteTemplate.TemplateText); }); } @@ -213,10 +220,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal Order = 17, }, RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { TreeRouter.RouteGroupKey, "1" } - }, - RouteValueDefaults = new Dictionary() { { "controller", "Blog" }, { "action", "Index" }, @@ -241,7 +244,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal(RoutePrecedence.ComputeOutbound(e.RouteTemplate), e.Precedence); Assert.Equal("BLOG_INDEX", e.RouteName); Assert.Equal(17, e.Order); - Assert.Equal(actions[0].RouteValueDefaults.ToArray(), e.RequiredLinkValues.ToArray()); + Assert.Equal(ToRouteValueDictionary(actions[0].RouteValues), e.RequiredLinkValues); Assert.Equal("api/Blog/{*slug=hello}", e.RouteTemplate.TemplateText); }); } @@ -264,10 +267,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal Order = 17, }, RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { TreeRouter.RouteGroupKey, "1" } - }, - RouteValueDefaults = new Dictionary() { { "controller", "Blog" }, { "action", "Index" }, @@ -282,10 +281,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal Order = 17, }, RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { TreeRouter.RouteGroupKey, "1" } - }, - RouteValueDefaults = new Dictionary() { { "controller", "Blog" }, { "action", "Index2" }, @@ -310,7 +305,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal(RoutePrecedence.ComputeOutbound(e.RouteTemplate), e.Precedence); Assert.Equal("BLOG_INDEX", e.RouteName); Assert.Equal(17, e.Order); - Assert.Equal(actions[0].RouteValueDefaults.ToArray(), e.RequiredLinkValues.ToArray()); + Assert.Equal(ToRouteValueDictionary(actions[0].RouteValues), e.RequiredLinkValues); Assert.Equal("api/Blog/{id}", e.RouteTemplate.TemplateText); }, e => @@ -320,7 +315,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal(RoutePrecedence.ComputeOutbound(e.RouteTemplate), e.Precedence); Assert.Equal("BLOG_INDEX", e.RouteName); Assert.Equal(17, e.Order); - Assert.Equal(actions[1].RouteValueDefaults.ToArray(), e.RequiredLinkValues.ToArray()); + Assert.Equal(ToRouteValueDictionary(actions[1].RouteValues), e.RequiredLinkValues); Assert.Equal("api/Blog/{id}", e.RouteTemplate.TemplateText); }); } @@ -340,10 +335,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal Order = 17, }, RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { TreeRouter.RouteGroupKey, "1" } - }, - RouteValueDefaults = new Dictionary() { { "controller", "Blog" }, { "action", "Index" }, @@ -368,9 +359,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal(RoutePrecedence.ComputeInbound(e.RouteTemplate), e.Precedence); Assert.Equal("BLOG_INDEX", e.RouteName); Assert.Equal("api/Blog/{id}", e.RouteTemplate.TemplateText); - Assert.Collection( - e.Defaults.OrderBy(kvp => kvp.Key), - kvp => Assert.Equal(new KeyValuePair(TreeRouter.RouteGroupKey, "1"), kvp)); + Assert.Empty(e.Defaults); }); } @@ -389,10 +378,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal Order = 17, }, RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { TreeRouter.RouteGroupKey, "1" } - }, - RouteValueDefaults = new Dictionary() { { "controller", "Blog" }, { "action", "Index" }, @@ -417,9 +402,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal(RoutePrecedence.ComputeInbound(e.RouteTemplate), e.Precedence); Assert.Equal("BLOG_INDEX", e.RouteName); Assert.Equal("api/Blog/{id:int}", e.RouteTemplate.TemplateText); - Assert.Collection( - e.Defaults.OrderBy(kvp => kvp.Key), - kvp => Assert.Equal(new KeyValuePair(TreeRouter.RouteGroupKey, "1"), kvp)); + Assert.Empty(e.Defaults); }); } @@ -438,10 +421,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal Order = 17, }, RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { TreeRouter.RouteGroupKey, "1" } - }, - RouteValueDefaults = new Dictionary() { { "controller", "Blog" }, { "action", "Index" }, @@ -468,7 +447,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal("api/Blog/{*slug=hello}", e.RouteTemplate.TemplateText); Assert.Collection( e.Defaults.OrderBy(kvp => kvp.Key), - kvp => Assert.Equal(new KeyValuePair(TreeRouter.RouteGroupKey, "1"), kvp), kvp => Assert.Equal(new KeyValuePair("slug", "hello"), kvp)); }); } @@ -491,10 +469,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal Order = 17, }, RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { TreeRouter.RouteGroupKey, "1" } - }, - RouteValueDefaults = new Dictionary() { { "controller", "Blog" }, { "action", "Index" }, @@ -509,10 +483,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal Order = 17, }, RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { TreeRouter.RouteGroupKey, "1" } - }, - RouteValueDefaults = new Dictionary() { { "controller", "Blog" }, { "action", "Index2" }, @@ -537,9 +507,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal(RoutePrecedence.ComputeInbound(e.RouteTemplate), e.Precedence); Assert.Equal("BLOG_INDEX", e.RouteName); Assert.Equal("api/Blog/{id}", e.RouteTemplate.TemplateText); - Assert.Collection( - e.Defaults.OrderBy(kvp => kvp.Key), - kvp => Assert.Equal(new KeyValuePair(TreeRouter.RouteGroupKey, "1"), kvp)); + Assert.Empty(e.Defaults); }); } @@ -580,6 +548,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal private static AttributeRoute CreateRoute( IRouter handler, IActionDescriptorCollectionProvider actionDescriptorProvider) + { + return CreateRoute((_) => handler, actionDescriptorProvider); + } + + private static AttributeRoute CreateRoute( + Func handlerFactory, + IActionDescriptorCollectionProvider actionDescriptorProvider) { var services = new ServiceCollection() .AddSingleton() @@ -588,7 +563,20 @@ namespace Microsoft.AspNetCore.Mvc.Internal .AddRouting() .AddOptions() .BuildServiceProvider(); - return new AttributeRoute(handler, actionDescriptorProvider, services); + return new AttributeRoute(actionDescriptorProvider, services, handlerFactory); + } + + // Needed because new RouteValueDictionary(values) would give us all the properties of + // the Dictionary class. + private static RouteValueDictionary ToRouteValueDictionary(IDictionary values) + { + var result = new RouteValueDictionary(); + foreach (var kvp in values) + { + result.Add(kvp.Key, kvp.Value); + } + + return result; } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AttributeRoutingTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AttributeRoutingTest.cs index e597961ef1..2dc8531835 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AttributeRoutingTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/AttributeRoutingTest.cs @@ -14,7 +14,6 @@ using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.AspNetCore.Routing; -using Microsoft.AspNetCore.Routing.Tree; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; @@ -43,11 +42,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal "and can occur only at the end of the parameter. The '*' character marks a parameter as catch-all, " + "and can occur only at the start of the parameter." + Environment.NewLine + "Parameter name: routeTemplate"; - - var handler = CreateRouter(); + var services = CreateServices(action); - var route = AttributeRouting.CreateAttributeMegaRoute(handler, services); + var route = AttributeRouting.CreateAttributeMegaRoute(services); // Act & Assert var ex = await Assert.ThrowsAsync(async () => @@ -63,7 +61,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal { // Arrange var action = CreateAction("DisallowedParameter", "{foo}/{action}"); - action.RouteValueDefaults.Add("foo", "bleh"); + action.RouteValues.Add("foo", "bleh"); var expectedMessage = "The following errors occurred with attribute routing information:" + Environment.NewLine + @@ -71,11 +69,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal "For action: 'DisallowedParameter'" + Environment.NewLine + "Error: The attribute route '{foo}/{action}' cannot contain a parameter named '{foo}'. " + "Use '[foo]' in the route template to insert the value 'bleh'."; - - var handler = CreateRouter(); + var services = CreateServices(action); - var route = AttributeRouting.CreateAttributeMegaRoute(handler, services); + var route = AttributeRouting.CreateAttributeMegaRoute(services); // Act & Assert var ex = await Assert.ThrowsAsync(async () => @@ -91,10 +88,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal { // Arrange var action1 = CreateAction("DisallowedParameter1", "{foo}/{action}"); - action1.RouteValueDefaults.Add("foo", "bleh"); + action1.RouteValues.Add("foo", "bleh"); var action2 = CreateAction("DisallowedParameter2", "cool/{action}"); - action2.RouteValueDefaults.Add("action", "hey"); + action2.RouteValues.Add("action", "hey"); var expectedMessage = "The following errors occurred with attribute routing information:" + Environment.NewLine + @@ -106,11 +103,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal "For action: 'DisallowedParameter2'" + Environment.NewLine + "Error: The attribute route 'cool/{action}' cannot contain a parameter named '{action}'. " + "Use '[action]' in the route template to insert the value 'hey'."; - - var handler = CreateRouter(); + var services = CreateServices(action1, action2); - var route = AttributeRouting.CreateAttributeMegaRoute(handler, services); + var route = AttributeRouting.CreateAttributeMegaRoute(services); // Act & Assert var ex = await Assert.ThrowsAsync(async () => @@ -133,14 +129,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal action.MethodInfo = actionMethod; action.RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) { - { TreeRouter.RouteGroupKey, "group" } + { "controller", "Home" }, + { "action", "Index" }, }; action.AttributeRouteInfo = new AttributeRouteInfo(); action.AttributeRouteInfo.Template = "{controller}/{action}"; - action.RouteValueDefaults.Add("controller", "Home"); - action.RouteValueDefaults.Add("action", "Index"); - var expectedMessage = "The following errors occurred with attribute routing information:" + Environment.NewLine + Environment.NewLine + @@ -148,10 +142,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal "Error: The attribute route '{controller}/{action}' cannot contain a parameter named '{controller}'. " + "Use '[controller]' in the route template to insert the value 'Home'."; - var handler = CreateRouter(); var services = CreateServices(action); - var route = AttributeRouting.CreateAttributeMegaRoute(handler, services); + var route = AttributeRouting.CreateAttributeMegaRoute(services); // Act & Assert var ex = await Assert.ThrowsAsync(async () => @@ -167,19 +160,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal return new DisplayNameActionDescriptor() { DisplayName = displayName, - RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { TreeRouter.RouteGroupKey, "whatever" } - }, + RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase), AttributeRouteInfo = new AttributeRouteInfo { Template = template }, }; } - private static IRouter CreateRouter() - { - return Mock.Of(); - } - private static IServiceProvider CreateServices(params ActionDescriptor[] actions) { var collection = new ActionDescriptorCollection(actions, version: 0); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionDescriptorProviderTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionDescriptorProviderTests.cs index c63a98b9b0..98d1c9ef0e 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionDescriptorProviderTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionDescriptorProviderTests.cs @@ -261,12 +261,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert var action = Assert.Single(descriptors); - Assert.Equal(TreeRouter.RouteGroupKey, Assert.Single(action.RouteValues).Key); - - var controller = Assert.Single(action.RouteValueDefaults, kvp => kvp.Key.Equals("controller")); + var controller = Assert.Single(action.RouteValues, kvp => kvp.Key.Equals("controller")); Assert.Equal("AttributeRouted", controller.Value); - var actionConstraint = Assert.Single(action.RouteValueDefaults, kvp => kvp.Key.Equals("action")); + var actionConstraint = Assert.Single(action.RouteValues, kvp => kvp.Key.Equals("action")); Assert.Equal(nameof(AttributeRoutedController.AttributeRoutedAction), actionConstraint.Value); } @@ -916,30 +914,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal(expectedMessage, ex.Message); } - [Fact] - public void AttributeRouting_RouteGroupConstraint_IsAddedOnceForNonAttributeRoutes() - { - // Arrange - var provider = GetProvider( - typeof(ConventionalAndAttributeRoutedActionsWithAreaController).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.RouteValues.Count); - Assert.Single( - actionDescriptor.RouteValues, - kvp => kvp.Key.Equals(TreeRouter.RouteGroupKey) && string.IsNullOrEmpty(kvp.Value)); - } - } - [Fact] public void AttributeRouting_AddsDefaultRouteValues_ForAttributeRoutedActions() { @@ -957,27 +931,22 @@ namespace Microsoft.AspNetCore.Mvc.Internal var indexAction = Assert.Single(actionDescriptors, ad => ad.ActionName.Equals("Index")); - Assert.Equal(1, indexAction.RouteValues.Count); + Assert.Equal(5, indexAction.RouteValues.Count); - var routeGroup = Assert.Single(indexAction.RouteValues, kvp => kvp.Key.Equals(TreeRouter.RouteGroupKey)); - Assert.NotNull(routeGroup.Value); - - Assert.Equal(5, indexAction.RouteValueDefaults.Count); - - var controllerDefault = Assert.Single(indexAction.RouteValueDefaults, rd => rd.Key.Equals("controller", StringComparison.OrdinalIgnoreCase)); + var controllerDefault = Assert.Single(indexAction.RouteValues, rd => rd.Key.Equals("controller", StringComparison.OrdinalIgnoreCase)); Assert.Equal("ConventionalAndAttributeRoutedActionsWithArea", controllerDefault.Value); - var actionDefault = Assert.Single(indexAction.RouteValueDefaults, rd => rd.Key.Equals("action", StringComparison.OrdinalIgnoreCase)); + var actionDefault = Assert.Single(indexAction.RouteValues, 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)); + var areaDefault = Assert.Single(indexAction.RouteValues, rd => rd.Key.Equals("area", StringComparison.OrdinalIgnoreCase)); Assert.Equal("Home", areaDefault.Value); - var mvRouteValueDefault = Assert.Single(indexAction.RouteValueDefaults, rd => rd.Key.Equals("key", StringComparison.OrdinalIgnoreCase)); - Assert.Null(mvRouteValueDefault.Value); + var mvRouteValueDefault = Assert.Single(indexAction.RouteValues, rd => rd.Key.Equals("key", StringComparison.OrdinalIgnoreCase)); + Assert.Equal(string.Empty, mvRouteValueDefault.Value); - var anotherRouteValue = Assert.Single(indexAction.RouteValueDefaults, rd => rd.Key.Equals("second", StringComparison.OrdinalIgnoreCase)); - Assert.Null(anotherRouteValue.Value); + var anotherRouteValue = Assert.Single(indexAction.RouteValues, rd => rd.Key.Equals("second", StringComparison.OrdinalIgnoreCase)); + Assert.Equal(string.Empty, anotherRouteValue.Value); } [Fact] @@ -994,29 +963,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal("stub/ThisIsAnAction", action.AttributeRouteInfo.Template); } - // Token replacement happens before we 'group' routes. So two route templates - // that are equivalent after token replacement go to the same 'group'. - [Fact] - public void AttributeRouting_TokenReplacement_BeforeGroupId() - { - // Arrange - var provider = GetProvider(typeof(SameGroupIdController).GetTypeInfo()); - - // Act - var actions = provider.GetDescriptors().ToArray(); - - var groupIds = actions.Select( - a => a.RouteValues - .Where(kvp => kvp.Key == TreeRouter.RouteGroupKey) - .Select(kvp => kvp.Value) - .Single()) - .ToArray(); - - // Assert - Assert.Equal(2, groupIds.Length); - Assert.Equal(groupIds[0], groupIds[1]); - } - // Parameters are validated later. This action uses the forbidden {action} and {controller} [Fact] public void AttributeRouting_DoesNotValidateParameters() diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RouteDataTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RouteDataTest.cs index 57c520c5c6..57330d5e7b 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RouteDataTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RouteDataTest.cs @@ -59,7 +59,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests { typeof(RouteCollection).FullName, typeof(AttributeRoute).FullName, - typeof(MvcRouteHandler).FullName, + typeof(MvcAttributeRouteHandler).FullName, }, result.Routers); } diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs b/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs index 5be204df1c..b2da3a02f3 100644 --- a/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs @@ -1148,12 +1148,8 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test Assert.Equal(expected, result.SearchedLocations); } - [Theory] - // Looks in RouteValueDefaults - [InlineData(true)] - // Looks in RouteValues - [InlineData(false)] - public void FindPage_SelectsActionCaseInsensitively(bool isAttributeRouted) + [Fact] + public void FindPage_SelectsActionCaseInsensitively() { // The ActionDescriptor contains "Foo" and the RouteData contains "foo" // which matches the case of the constructor thus searching in the appropriate location. @@ -1177,8 +1173,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test var context = GetActionContextWithActionDescriptor( routeValues, - routesInActionDescriptor, - isAttributeRouted); + routesInActionDescriptor); // Act var result = viewEngine.FindPage(context, "details"); @@ -1190,12 +1185,8 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test pageFactory.Verify(); } - [Theory] - // Looks in RouteValueDefaults - [InlineData(true)] - // Looks in RouteValues - [InlineData(false)] - public void FindPage_LooksForPages_UsingActionDescriptor_Controller(bool isAttributeRouted) + [Fact] + public void FindPage_LooksForPages_UsingActionDescriptor_Controller() { // Arrange var expected = new[] @@ -1216,8 +1207,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test var viewEngine = CreateViewEngine(); var context = GetActionContextWithActionDescriptor( routeValues, - routesInActionDescriptor, - isAttributeRouted); + routesInActionDescriptor); // Act var result = viewEngine.FindPage(context, "foo"); @@ -1228,12 +1218,8 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test Assert.Equal(expected, result.SearchedLocations); } - [Theory] - // Looks in RouteValueDefaults - [InlineData(true)] - // Looks in RouteValues - [InlineData(false)] - public void FindPage_LooksForPages_UsingActionDescriptor_Areas(bool isAttributeRouted) + [Fact] + public void FindPage_LooksForPages_UsingActionDescriptor_Areas() { // Arrange var expected = new[] @@ -1257,8 +1243,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test var viewEngine = CreateViewEngine(); var context = GetActionContextWithActionDescriptor( routeValues, - routesInActionDescriptor, - isAttributeRouted); + routesInActionDescriptor); // Act var result = viewEngine.FindPage(context, "foo"); @@ -1269,10 +1254,8 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test Assert.Equal(expected, result.SearchedLocations); } - [Theory] - [InlineData(true)] - [InlineData(false)] - public void FindPage_LooksForPages_UsesRouteValuesAsFallback(bool isAttributeRouted) + [Fact] + public void FindPage_LooksForPages_UsesRouteValuesAsFallback() { // Arrange var expected = new[] @@ -1289,8 +1272,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test var viewEngine = CreateViewEngine(); var context = GetActionContextWithActionDescriptor( routeValues, - new Dictionary(), - isAttributeRouted); + new Dictionary()); // Act var result = viewEngine.FindPage(context, "bar"); @@ -1462,7 +1444,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test } [Fact] - public void GetNormalizedRouteValue_ReturnsValueFromRouteValues_IfKeyHandlingIsRequired() + public void GetNormalizedRouteValue_ReturnsValueFromRouteValues() { // Arrange var key = "some-key"; @@ -1530,112 +1512,6 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test Assert.Equal("route-value", result); } - [Fact] - public void GetNormalizedRouteValue_UsesRouteValueDefaults_IfAttributeRouted() - { - // Arrange - var key = "some-key"; - var actionDescriptor = new ActionDescriptor - { - AttributeRouteInfo = new AttributeRouteInfo(), - }; - actionDescriptor.RouteValueDefaults[key] = "Route-Value"; - - var actionContext = new ActionContext - { - ActionDescriptor = actionDescriptor, - RouteData = new RouteData() - }; - - actionContext.RouteData.Values[key] = "route-value"; - - // Act - var result = RazorViewEngine.GetNormalizedRouteValue(actionContext, key); - - // Assert - Assert.Equal("Route-Value", result); - } - - [Fact] - public void GetNormalizedRouteValue_UsesRouteValue_IfRouteValueDefaultsDoesNotMatchRouteValue() - { - // Arrange - var key = "some-key"; - var actionDescriptor = new ActionDescriptor - { - AttributeRouteInfo = new AttributeRouteInfo(), - }; - actionDescriptor.RouteValueDefaults[key] = "different-value"; - - var actionContext = new ActionContext - { - ActionDescriptor = actionDescriptor, - RouteData = new RouteData() - }; - - actionContext.RouteData.Values[key] = "route-value"; - - // Act - var result = RazorViewEngine.GetNormalizedRouteValue(actionContext, key); - - // Assert - Assert.Equal("route-value", result); - } - - [Fact] - public void GetNormalizedRouteValue_ConvertsRouteDefaultToStringValue_IfAttributeRouted() - { - using (new CultureReplacer()) - { - // Arrange - var key = "some-key"; - var actionDescriptor = new ActionDescriptor - { - AttributeRouteInfo = new AttributeRouteInfo(), - }; - actionDescriptor.RouteValueDefaults[key] = 32; - - var actionContext = new ActionContext - { - ActionDescriptor = actionDescriptor, - RouteData = new RouteData() - }; - - actionContext.RouteData.Values[key] = 32; - - // Act - var result = RazorViewEngine.GetNormalizedRouteValue(actionContext, key); - - // Assert - Assert.Equal("32", result); - } - } - - [Fact] - public void GetNormalizedRouteValue_UsesRouteDataValue_IfKeyDoesNotExistInRouteDefaultValues() - { - // Arrange - var key = "some-key"; - var actionDescriptor = new ActionDescriptor - { - AttributeRouteInfo = new AttributeRouteInfo(), - }; - - var actionContext = new ActionContext - { - ActionDescriptor = actionDescriptor, - RouteData = new RouteData() - }; - - actionContext.RouteData.Values[key] = "route-value"; - - // Act - var result = RazorViewEngine.GetNormalizedRouteValue(actionContext, key); - - // Assert - Assert.Equal("route-value", result); - } - [Fact] public void GetNormalizedRouteValue_ConvertsRouteValueToString() { @@ -1743,8 +1619,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test private static ActionContext GetActionContextWithActionDescriptor( IDictionary routeValues, - IDictionary actionRouteValues, - bool isAttributeRouted) + IDictionary actionRouteValues) { var httpContext = new DefaultHttpContext(); var routeData = new RouteData(); @@ -1754,20 +1629,10 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test } var actionDescriptor = new ActionDescriptor(); - if (isAttributeRouted) + + foreach (var kvp in actionRouteValues) { - actionDescriptor.AttributeRouteInfo = new AttributeRouteInfo(); - foreach (var kvp in actionRouteValues) - { - actionDescriptor.RouteValueDefaults.Add(kvp.Key, kvp.Value); - } - } - else - { - foreach (var kvp in actionRouteValues) - { - actionDescriptor.RouteValues.Add(kvp.Key, kvp.Value); - } + actionDescriptor.RouteValues.Add(kvp.Key, kvp.Value); } return new ActionContext(httpContext, routeData, actionDescriptor); diff --git a/test/WebSites/BasicWebSite/Areas/Area1/Views/RemoteAttribute_Home/Create.cshtml b/test/WebSites/BasicWebSite/Areas/Area1/Views/RemoteAttribute_Home/Create.cshtml index 3fa298b308..8cd88c2ecb 100644 --- a/test/WebSites/BasicWebSite/Areas/Area1/Views/RemoteAttribute_Home/Create.cshtml +++ b/test/WebSites/BasicWebSite/Areas/Area1/Views/RemoteAttribute_Home/Create.cshtml @@ -1,9 +1,9 @@ @model BasicWebSite.Models.RemoteAttributeUser @{ Layout = "_Layout.cshtml"; - object areaObject; - ViewContext.ActionDescriptor.RouteValueDefaults.TryGetValue("area", out areaObject); - var areaName = (areaObject as string) ?? "root"; + object areaName; + ViewContext.RouteData.Values.TryGetValue("area", out areaName); + areaName = areaName ?? "root"; ViewBag.Title = "Create in " + areaName + " area."; } diff --git a/test/WebSites/BasicWebSite/Areas/Area1/Views/RemoteAttribute_Home/Details.cshtml b/test/WebSites/BasicWebSite/Areas/Area1/Views/RemoteAttribute_Home/Details.cshtml index 5212c90af3..18c95164b1 100644 --- a/test/WebSites/BasicWebSite/Areas/Area1/Views/RemoteAttribute_Home/Details.cshtml +++ b/test/WebSites/BasicWebSite/Areas/Area1/Views/RemoteAttribute_Home/Details.cshtml @@ -1,9 +1,9 @@ @model BasicWebSite.Models.RemoteAttributeUser @{ Layout = "_Layout.cshtml"; - object areaObject; - ViewContext.ActionDescriptor.RouteValueDefaults.TryGetValue("area", out areaObject); - var areaName = (areaObject as string) ?? "root"; + object areaName; + ViewContext.RouteData.Values.TryGetValue("area", out areaName); + areaName = areaName ?? "root"; ViewBag.Title = "Details in " + areaName + " area."; } diff --git a/test/WebSites/BasicWebSite/Views/RemoteAttribute_Home/Create.cshtml b/test/WebSites/BasicWebSite/Views/RemoteAttribute_Home/Create.cshtml index 3fa298b308..8cd88c2ecb 100644 --- a/test/WebSites/BasicWebSite/Views/RemoteAttribute_Home/Create.cshtml +++ b/test/WebSites/BasicWebSite/Views/RemoteAttribute_Home/Create.cshtml @@ -1,9 +1,9 @@ @model BasicWebSite.Models.RemoteAttributeUser @{ Layout = "_Layout.cshtml"; - object areaObject; - ViewContext.ActionDescriptor.RouteValueDefaults.TryGetValue("area", out areaObject); - var areaName = (areaObject as string) ?? "root"; + object areaName; + ViewContext.RouteData.Values.TryGetValue("area", out areaName); + areaName = areaName ?? "root"; ViewBag.Title = "Create in " + areaName + " area."; } diff --git a/test/WebSites/BasicWebSite/Views/RemoteAttribute_Home/Details.cshtml b/test/WebSites/BasicWebSite/Views/RemoteAttribute_Home/Details.cshtml index 5212c90af3..6aa8ce67a5 100644 --- a/test/WebSites/BasicWebSite/Views/RemoteAttribute_Home/Details.cshtml +++ b/test/WebSites/BasicWebSite/Views/RemoteAttribute_Home/Details.cshtml @@ -1,9 +1,9 @@ @model BasicWebSite.Models.RemoteAttributeUser @{ Layout = "_Layout.cshtml"; - object areaObject; - ViewContext.ActionDescriptor.RouteValueDefaults.TryGetValue("area", out areaObject); - var areaName = (areaObject as string) ?? "root"; + string areaName; + ViewContext.RouteData.Values.TryGetValue("area", out areaName); + areaName = areaName ?? "root"; ViewBag.Title = "Details in " + areaName + " area."; }