From 775a780f49a85becca7264a4aff828c3ab548fb0 Mon Sep 17 00:00:00 2001 From: jacalvar Date: Tue, 9 Sep 2014 15:14:25 -0700 Subject: [PATCH] [Fixes #739] Attribute Routing: Multiple routes per-action 1. Support multiple [Http*] attributes on an action. 2. Support multiple [Route] attributes on a controller and on an action. 3. Support creating multiple attribute routes using [AcceptVerbs("...", Route = "...")] 4. Detect attribute routed actions during action discovery and return one action per [Http*], [Route] or [AcceptVerbs] attribute found on the method when there is at least one valid attribute route. 5. Merge all the HTTP methods of [Http*] and [AcceptVerbs] attributes in a method during action discovery when there are no valid attribute routes defined on the action. 6. Build one action descriptor per controller [Route] + action [Http*], [AcceptVerbs] or [Route] combination in an action. 7. Disallow the use of attributes that do not implement IActionHttpMethodProvider and IRouteTemplateProvider simultaneously in methods that define attribute routed actions and throw an exception during startup. 8. Disallow mixing attribute routed and non attribute routed actions on the same method and throw an exception during startup. --- .../AcceptVerbsAttribute.cs | 41 +- .../ActionConvention.cs | 2 + .../DefaultActionDiscoveryConventions.cs | 121 +++++- .../HttpMethodAttribute.cs | 2 +- .../Properties/Resources.Designer.cs | 80 ++++ .../ReflectedActionDescriptorProvider.cs | 402 +++++++++++++++--- .../ReflectedActionModel.cs | 7 - .../ReflectedAttributeRouteModel.cs | 9 + .../ReflectedControllerModel.cs | 10 +- src/Microsoft.AspNet.Mvc.Core/Resources.resx | 18 + .../RouteAttribute.cs | 2 +- .../DefaultActionDiscoveryConventionsTests.cs | 356 ++++++++++++++++ .../ReflectedActionDescriptorProviderTests.cs | 354 ++++++++++++++- .../ReflectedActionModelTests.cs | 14 - .../ReflectedControllerModelTests.cs | 16 +- .../RoutingTests.cs | 208 +++++++++ .../Controllers/BanksController.cs | 35 ++ .../Controllers/EmployeeController.cs | 6 + .../Controllers/MapsController.cs | 51 +++ 19 files changed, 1629 insertions(+), 105 deletions(-) create mode 100644 test/WebSites/RoutingWebSite/Controllers/BanksController.cs create mode 100644 test/WebSites/RoutingWebSite/Controllers/MapsController.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/AcceptVerbsAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/AcceptVerbsAttribute.cs index 3c8e7db900..f4ddcfa148 100644 --- a/src/Microsoft.AspNet.Mvc.Core/AcceptVerbsAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/AcceptVerbsAttribute.cs @@ -4,16 +4,18 @@ using System; using System.Collections.Generic; using System.Linq; +using Microsoft.AspNet.Mvc.Routing; namespace Microsoft.AspNet.Mvc { /// /// Specifies what HTTP methods an action supports. /// - [AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)] - public sealed class AcceptVerbsAttribute : Attribute, IActionHttpMethodProvider + [AttributeUsage(AttributeTargets.Method, AllowMultiple = true, Inherited = true)] + public sealed class AcceptVerbsAttribute : Attribute, IActionHttpMethodProvider, IRouteTemplateProvider { private readonly IEnumerable _httpMethods; + private int? _order; /// /// Initializes a new instance of the class. @@ -45,5 +47,40 @@ namespace Microsoft.AspNet.Mvc return _httpMethods; } } + + /// + /// The route template. May be null. + /// + public string Route { get; set; } + + /// + string IRouteTemplateProvider.Template + { + get { return Route; } + } + + /// + /// 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; + } + } + + /// + public string Name { get; set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/ActionConvention.cs b/src/Microsoft.AspNet.Mvc.Core/ActionConvention.cs index ad733671e5..d8a57336bd 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ActionConvention.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ActionConvention.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.Collections.Generic; +using Microsoft.AspNet.Mvc.Routing; namespace Microsoft.AspNet.Mvc { @@ -9,6 +10,7 @@ namespace Microsoft.AspNet.Mvc { public string ActionName { get; set; } public string[] HttpMethods { get; set; } + public IRouteTemplateProvider AttributeRoute { get; set; } public bool RequireActionNameMatch { get; set; } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultActionDiscoveryConventions.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultActionDiscoveryConventions.cs index c12e93bc0f..0713d78cf8 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultActionDiscoveryConventions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultActionDiscoveryConventions.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Linq; using System.Net; using System.Reflection; +using Microsoft.AspNet.Mvc.Routing; namespace Microsoft.AspNet.Mvc { @@ -55,7 +56,7 @@ namespace Microsoft.AspNet.Mvc // If the convention is All methods starting with Get do not have an action name, // for a input GetXYZ methodInfo, the return value will be - // { { HttpMethods = "GET", ActionName = "GetXYZ", RequireActionNameMatch = false }} + // { { HttpMethods = "GET", ActionName = "GetXYZ", RequireActionNameMatch = false, AttributeRoute = null }} public virtual IEnumerable GetActions( [NotNull] MethodInfo methodInfo, [NotNull] TypeInfo controllerTypeInfo) @@ -65,7 +66,7 @@ namespace Microsoft.AspNet.Mvc return null; } - var actionInfos = GetActionsForMethodsWithCustomAttributes(methodInfo); + var actionInfos = GetActionsForMethodsWithCustomAttributes(methodInfo, controllerTypeInfo); if (actionInfos.Any()) { return actionInfos; @@ -129,26 +130,77 @@ namespace Microsoft.AspNet.Mvc var attributes = methodInfo.GetCustomAttributes(); var actionNameAttribute = attributes.OfType().FirstOrDefault(); var httpMethodConstraints = attributes.OfType(); + var routeTemplates = attributes.OfType(); + return new ActionAttributes() { + ActionNameAttribute = actionNameAttribute, HttpMethodProviderAttributes = httpMethodConstraints, - ActionNameAttribute = actionNameAttribute + RouteTemplateProviderAttributes = routeTemplates, }; } - private IEnumerable GetActionsForMethodsWithCustomAttributes(MethodInfo methodInfo) + private IEnumerable GetActionsForMethodsWithCustomAttributes( + MethodInfo methodInfo, + TypeInfo controller) { + var hasControllerAttributeRoutes = HasValidControllerRouteTemplates(controller); var actionAttributes = GetActionCustomAttributes(methodInfo); - if (!actionAttributes.Any()) + + // We need to check controllerRouteTemplates to take into account the + // case where the controller has [Route] on it and the action does not have any + // attributes applied to it. + if (actionAttributes.Any() || hasControllerAttributeRoutes) + { + var actionNameAttribute = actionAttributes.ActionNameAttribute; + var actionName = actionNameAttribute != null ? actionNameAttribute.Name : methodInfo.Name; + + // The moment we see a non null attribute route template in the method or + // in the controller we consider the whole group to be attribute routed actions. + // If a combination ends up producing a non attribute routed action we consider + // that an error and throw at a later point in the pipeline. + if (hasControllerAttributeRoutes || ActionHasAttributeRoutes(actionAttributes)) + { + return GetAttributeRoutedActions(actionAttributes, actionName); + } + else + { + return GetHttpConstrainedActions(actionAttributes, actionName); + } + } + else { // If the action is not decorated with any of the attributes, // it would be handled by convention. - yield break; + return Enumerable.Empty(); } + } - var actionNameAttribute = actionAttributes.ActionNameAttribute; - var actionName = actionNameAttribute != null ? actionNameAttribute.Name : methodInfo.Name; + private static bool ActionHasAttributeRoutes(ActionAttributes actionAttributes) + { + // We neet to check for null as some attributes implement IActionHttpMethodProvider + // and IRouteTemplateProvider and allow the user to provide a null template. An example + // of this is HttpGetAttribute. If the user provides a template, the attribute marks the + // action as attribute routed, but in other case, the attribute only adds a constraint + // that allows the action to be called with the GET HTTP method. + return actionAttributes.RouteTemplateProviderAttributes + .Any(rtp => rtp.Template != null); + } + private static bool HasValidControllerRouteTemplates(TypeInfo controller) + { + // A method inside a controller is considered to create attribute routed actions if the controller + // has one or more attributes that implement IRouteTemplateProvider with a non null template applied + // to it. + return controller.GetCustomAttributes() + .OfType() + .Any(cr => cr.Template != null); + } + + private static IEnumerable GetHttpConstrainedActions( + ActionAttributes actionAttributes, + string actionName) + { var httpMethodProviders = actionAttributes.HttpMethodProviderAttributes; var httpMethods = httpMethodProviders.SelectMany(x => x.HttpMethods).Distinct().ToArray(); @@ -156,10 +208,54 @@ namespace Microsoft.AspNet.Mvc { HttpMethods = httpMethods, ActionName = actionName, - RequireActionNameMatch = true + RequireActionNameMatch = true, }; } + private static IEnumerable GetAttributeRoutedActions( + ActionAttributes actionAttributes, + string actionName) + { + var actions = new List(); + + // This is the case where the controller has [Route] applied to it and + // the action doesn't have any [Route] or [Http*] attribute applied. + if (!actionAttributes.RouteTemplateProviderAttributes.Any()) + { + actions.Add(new ActionInfo + { + ActionName = actionName, + HttpMethods = null, + RequireActionNameMatch = true, + AttributeRoute = null + }); + } + + foreach (var routeTemplateProvider in actionAttributes.RouteTemplateProviderAttributes) + { + actions.Add(new ActionInfo() + { + ActionName = actionName, + HttpMethods = GetRouteTemplateHttpMethods(routeTemplateProvider), + RequireActionNameMatch = true, + AttributeRoute = routeTemplateProvider + }); + } + + return actions; + } + + private static string[] GetRouteTemplateHttpMethods(IRouteTemplateProvider routeTemplateProvider) + { + var provider = routeTemplateProvider as IActionHttpMethodProvider; + if (provider != null && provider.HttpMethods != null) + { + return provider.HttpMethods.ToArray(); + } + + return null; + } + private IEnumerable GetActionsForMethodsWithoutCustomAttributes( MethodInfo methodInfo, TypeInfo controllerTypeInfo) @@ -226,12 +322,15 @@ namespace Microsoft.AspNet.Mvc private class ActionAttributes { - public IEnumerable HttpMethodProviderAttributes { get; set; } public ActionNameAttribute ActionNameAttribute { get; set; } + public IEnumerable HttpMethodProviderAttributes { get; set; } + public IEnumerable RouteTemplateProviderAttributes { get; set; } public bool Any() { - return ActionNameAttribute != null || HttpMethodProviderAttributes.Any(); + return ActionNameAttribute != null || + HttpMethodProviderAttributes.Any() || + RouteTemplateProviderAttributes.Any(); } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/HttpMethodAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/HttpMethodAttribute.cs index 9fbdbc05fd..64f5913186 100644 --- a/src/Microsoft.AspNet.Mvc.Core/HttpMethodAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/HttpMethodAttribute.cs @@ -10,7 +10,7 @@ namespace Microsoft.AspNet.Mvc /// /// Identifies an action that only supports a given set of HTTP methods. /// - [AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = true)] + [AttributeUsage(AttributeTargets.Method, AllowMultiple = true, Inherited = true)] public abstract class HttpMethodAttribute : Attribute, IActionHttpMethodProvider, IRouteTemplateProvider { private int? _order; diff --git a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs index e444499ce1..3664b0dcd7 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs @@ -1354,6 +1354,86 @@ namespace Microsoft.AspNet.Mvc.Core return string.Format(CultureInfo.CurrentCulture, GetString("AttributeRoute_AggregateErrorMessage_ErrorNumber"), p0, p1, p2); } + /// + /// A method '{0}' that defines attribute routed actions must not have attributes that implement '{1}' and do not implement '{2}':{3}{4} + /// + internal static string AttributeRoute_InvalidHttpConstraints + { + get { return GetString("AttributeRoute_InvalidHttpConstraints"); } + } + + /// + /// A method '{0}' that defines attribute routed actions must not have attributes that implement '{1}' and do not implement '{2}':{3}{4} + /// + internal static string FormatAttributeRoute_InvalidHttpConstraints(object p0, object p1, object p2, object p3, object p4) + { + return string.Format(CultureInfo.CurrentCulture, GetString("AttributeRoute_InvalidHttpConstraints"), p0, p1, p2, p3, p4); + } + + /// + /// Action '{0}' with route template '{1}' has '{2}' invalid '{3}' attributes. + /// + internal static string AttributeRoute_InvalidHttpConstraints_Item + { + get { return GetString("AttributeRoute_InvalidHttpConstraints_Item"); } + } + + /// + /// Action '{0}' with route template '{1}' has '{2}' invalid '{3}' attributes. + /// + internal static string FormatAttributeRoute_InvalidHttpConstraints_Item(object p0, object p1, object p2, object p3) + { + return string.Format(CultureInfo.CurrentCulture, GetString("AttributeRoute_InvalidHttpConstraints_Item"), p0, p1, p2, p3); + } + + /// + /// A method '{0}' must not define attribute routed actions and non attribute routed actions at the same time:{1}{2} + /// + internal static string AttributeRoute_MixedAttributeAndConventionallyRoutedActions_ForMethod + { + get { return GetString("AttributeRoute_MixedAttributeAndConventionallyRoutedActions_ForMethod"); } + } + + /// + /// A method '{0}' must not define attribute routed actions and non attribute routed actions at the same time:{1}{2} + /// + internal static string FormatAttributeRoute_MixedAttributeAndConventionallyRoutedActions_ForMethod(object p0, object p1, object p2) + { + return string.Format(CultureInfo.CurrentCulture, GetString("AttributeRoute_MixedAttributeAndConventionallyRoutedActions_ForMethod"), p0, p1, p2); + } + + /// + /// Action: '{0}' - Template: '{1}' + /// + internal static string AttributeRoute_MixedAttributeAndConventionallyRoutedActions_ForMethod_Item + { + get { return GetString("AttributeRoute_MixedAttributeAndConventionallyRoutedActions_ForMethod_Item"); } + } + + /// + /// Action: '{0}' - Template: '{1}' + /// + internal static string FormatAttributeRoute_MixedAttributeAndConventionallyRoutedActions_ForMethod_Item(object p0, object p1) + { + return string.Format(CultureInfo.CurrentCulture, GetString("AttributeRoute_MixedAttributeAndConventionallyRoutedActions_ForMethod_Item"), p0, p1); + } + + /// + /// (none) + /// + internal static string AttributeRoute_NullTemplateRepresentation + { + get { return GetString("AttributeRoute_NullTemplateRepresentation"); } + } + + /// + /// (none) + /// + internal static string FormatAttributeRoute_NullTemplateRepresentation() + { + return GetString("AttributeRoute_NullTemplateRepresentation"); + } + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs index 704e0a8c17..0b08315ec9 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs @@ -4,9 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; -#if ASPNETCORE50 using System.Reflection; -#endif using Microsoft.AspNet.Mvc.Core; using Microsoft.AspNet.Mvc.ReflectedModelBuilder; using Microsoft.AspNet.Mvc.Routing; @@ -99,6 +97,12 @@ namespace Microsoft.AspNet.Mvc actionModel.IsActionNameMatchRequired = actionInfo.RequireActionNameMatch; actionModel.HttpMethods.AddRange(actionInfo.HttpMethods ?? Enumerable.Empty()); + if (actionInfo.AttributeRoute != null) + { + actionModel.AttributeRouteModel = new ReflectedAttributeRouteModel( + actionInfo.AttributeRoute); + } + foreach (var parameter in methodInfo.GetParameters()) { actionModel.Parameters.Add(new ReflectedParameterModel(parameter)); @@ -119,48 +123,74 @@ namespace Microsoft.AspNet.Mvc var hasAttributeRoutes = false; var removalConstraints = new HashSet(StringComparer.OrdinalIgnoreCase); + var methodInfoMap = new MethodToActionMap(); + var routeTemplateErrors = new List(); + var attributeRoutingConfigurationErrors = new Dictionary(); foreach (var controller in model.Controllers) { var controllerDescriptor = new ControllerDescriptor(controller.ControllerType); foreach (var action in controller.Actions) { - var actionDescriptor = CreateActionDescriptor( - action, - controller, - controllerDescriptor, - model.Filters); + // Controllers with multiple [Route] attributes (or user defined implementation of + // IRouteTemplateProvider) will generate one action descriptor per IRouteTemplateProvider + // instance. + // Actions with multiple [Http*] attributes or other (IRouteTemplateProvider implementations + // have already been identified as different actions during action discovery. + var actionDescriptors = CreateActionDescriptors(action, controller, controllerDescriptor); - AddActionConstraints(actionDescriptor, action, controller); - AddControllerRouteConstraints(actionDescriptor, controller.RouteConstraints, removalConstraints); - - if (IsAttributeRoutedAction(actionDescriptor)) + foreach (var actionDescriptor in actionDescriptors) { - hasAttributeRoutes = true; + AddActionFilters(actionDescriptor, action.Filters, controller.Filters, model.Filters); + AddActionConstraints(actionDescriptor, action, controller); + AddControllerRouteConstraints( + actionDescriptor, + controller.RouteConstraints, + removalConstraints); - // An attribute routed action will ignore conventional routed constraints. We still - // want to provide these values as ambient values for link generation. - AddConstraintsAsDefaultRouteValues(actionDescriptor); + if (IsAttributeRoutedAction(actionDescriptor)) + { + hasAttributeRoutes = true; - // Replaces tokens like [controller]/[action] in the route template with the actual values - // for this action. - ReplaceAttributeRouteTokens(actionDescriptor, routeTemplateErrors); + // An attribute routed action will ignore conventional routed constraints. We still + // want to provide these values as ambient values for link generation. + AddConstraintsAsDefaultRouteValues(actionDescriptor); - // Attribute routed actions will ignore conventional routed constraints. Instead they have - // a single route constraint "RouteGroup" associated with it. - ReplaceRouteConstraints(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 constraints. Instead they have + // a single route constraint "RouteGroup" associated with it. + ReplaceRouteConstraints(actionDescriptor); + } } - actions.Add(actionDescriptor); + methodInfoMap.AddToMethodInfo(action, actionDescriptors); + actions.AddRange(actionDescriptors); } } var actionsByRouteName = new Dictionary>( StringComparer.OrdinalIgnoreCase); + // Keeps track of all the methods that we've validated to avoid visiting each action group + // more than once. + var validatedMethods = new HashSet(); + foreach (var actionDescriptor in actions) { + if (!validatedMethods.Contains(actionDescriptor.MethodInfo)) + { + ValidateActionGroupConfiguration( + methodInfoMap, + actionDescriptor, + attributeRoutingConfigurationErrors); + + validatedMethods.Add(actionDescriptor.MethodInfo); + } + if (!IsAttributeRoutedAction(actionDescriptor)) { // Any attribute routes are in use, then non-attribute-routed action descriptors can't be @@ -203,34 +233,71 @@ namespace Microsoft.AspNet.Mvc } } + if (attributeRoutingConfigurationErrors.Any()) + { + var message = CreateAttributeRoutingAggregateErrorMessage( + attributeRoutingConfigurationErrors.Values); + + throw new InvalidOperationException(message); + } + var namedRoutedErrors = ValidateNamedAttributeRoutedActions(actionsByRouteName); if (namedRoutedErrors.Any()) { - namedRoutedErrors = AddErrorNumbers(namedRoutedErrors); - - var message = Resources.FormatAttributeRoute_AggregateErrorMessage( - Environment.NewLine, - string.Join(Environment.NewLine + Environment.NewLine, namedRoutedErrors)); - + var message = CreateAttributeRoutingAggregateErrorMessage(namedRoutedErrors); throw new InvalidOperationException(message); } if (routeTemplateErrors.Any()) { - var message = Resources.FormatAttributeRoute_AggregateErrorMessage( - Environment.NewLine, - string.Join(Environment.NewLine + Environment.NewLine, routeTemplateErrors)); - + var message = CreateAttributeRoutingAggregateErrorMessage(routeTemplateErrors); throw new InvalidOperationException(message); } return actions; } - private static ReflectedActionDescriptor CreateActionDescriptor(ReflectedActionModel action, + private static IList CreateActionDescriptors( + ReflectedActionModel action, ReflectedControllerModel controller, - ControllerDescriptor controllerDescriptor, - IEnumerable globalFilters) + ControllerDescriptor controllerDescriptor) + { + var actionDescriptors = new List(); + + // We check the action to see if the template allows combination behavior + // (It doesn't start with / or ~/) so that in the case where we have multiple + // [Route] attributes on the controller we don't end up creating multiple + // attribute identical attribute routes. + if (controller.AttributeRoutes != null && + controller.AttributeRoutes.Count > 0 && + (action.AttributeRouteModel == null || + !action.AttributeRouteModel.IsAbsoluteTemplate)) + { + foreach (var controllerAttributeRoute in controller.AttributeRoutes) + { + var actionDescriptor = CreateActionDescriptor( + action, + controllerAttributeRoute, + controllerDescriptor); + + actionDescriptors.Add(actionDescriptor); + } + } + else + { + actionDescriptors.Add(CreateActionDescriptor( + action, + controllerAttributeRoute: null, + controllerDescriptor: controllerDescriptor)); + } + + return actionDescriptors; + } + + private static ReflectedActionDescriptor CreateActionDescriptor( + ReflectedActionModel action, + ReflectedAttributeRouteModel controllerAttributeRoute, + ControllerDescriptor controllerDescriptor) { var parameterDescriptors = new List(); foreach (var parameter in action.Parameters) @@ -257,7 +324,9 @@ namespace Microsoft.AspNet.Mvc parameterDescriptors.Add(paramDescriptor); } - var attributeRouteInfo = CreateAttributeRouteInfo(action, controller); + var attributeRouteInfo = CreateAttributeRouteInfo( + action.AttributeRouteModel, + controllerAttributeRoute); var actionDescriptor = new ReflectedActionDescriptor() { @@ -274,23 +343,30 @@ namespace Microsoft.AspNet.Mvc action.ActionMethod.DeclaringType.FullName, action.ActionMethod.Name); - actionDescriptor.FilterDescriptors = - action.Filters.Select(f => new FilterDescriptor(f, FilterScope.Action)) - .Concat(controller.Filters.Select(f => new FilterDescriptor(f, FilterScope.Controller))) - .Concat(globalFilters.Select(f => new FilterDescriptor(f, FilterScope.Global))) - .OrderBy(d => d, FilterDescriptorOrderComparer.Comparer) - .ToList(); - return actionDescriptor; } + private static void AddActionFilters( + ReflectedActionDescriptor actionDescriptor, + IEnumerable actionFilters, + IEnumerable controllerFilters, + IEnumerable globalFilters) + { + actionDescriptor.FilterDescriptors = actionFilters + .Select(f => new FilterDescriptor(f, FilterScope.Action)) + .Concat(controllerFilters.Select(f => new FilterDescriptor(f, FilterScope.Controller))) + .Concat(globalFilters.Select(f => new FilterDescriptor(f, FilterScope.Global))) + .OrderBy(d => d, FilterDescriptorOrderComparer.Comparer) + .ToList(); + } + private static AttributeRouteInfo CreateAttributeRouteInfo( - ReflectedActionModel action, - ReflectedControllerModel controller) + ReflectedAttributeRouteModel action, + ReflectedAttributeRouteModel controller) { var combinedRoute = ReflectedAttributeRouteModel.CombineReflectedAttributeRouteModel( - controller.AttributeRouteModel, - action.AttributeRouteModel); + controller, + action); if (combinedRoute == null) { @@ -471,7 +547,7 @@ namespace Microsoft.AspNet.Mvc } private static IList AddErrorNumbers( - IList namedRoutedErrors) + IEnumerable namedRoutedErrors) { return namedRoutedErrors .Select((nre, i) => @@ -527,10 +603,240 @@ namespace Microsoft.AspNet.Mvc return namedRouteErrors; } + private void ValidateActionGroupConfiguration( + IDictionary>> methodMap, + ReflectedActionDescriptor actionDescriptor, + IDictionary routingConfigurationErrors) + { + string combinedErrorMessage = null; + + var hasAttributeRoutedActions = false; + var hasConventionallyRoutedActions = false; + + var invalidHttpMethodActions = new Dictionary>(); + + var actionsForMethod = methodMap[actionDescriptor.MethodInfo]; + foreach (var reflectedAction in actionsForMethod) + { + foreach (var action in reflectedAction.Value) + { + if (IsAttributeRoutedAction(action)) + { + hasAttributeRoutedActions = true; + } + else + { + hasConventionallyRoutedActions = true; + } + } + + // Keep a list of actions with possible invalid IHttpActionMethodProvider attributes + // to generate an error in case the method generates attribute routed actions. + ValidateActionHttpMethodProviders(reflectedAction.Key, invalidHttpMethodActions); + } + + // Validate that no method result in attribute and non attribute actions at the same time. + // By design, mixing attribute and conventionally actions in the same method is not allowed. + // This is for example the case when someone uses[HttpGet("Products")] and[HttpPost] + // on the same method. + if (hasAttributeRoutedActions && hasConventionallyRoutedActions) + { + combinedErrorMessage = CreateMixedRoutedActionDescriptorsErrorMessage( + actionDescriptor, + actionsForMethod); + } + + // Validate that no method that creates attribute routed actions and + // also uses attributes that only constrain the set of HTTP methods. For example, + // if an attribute that implements IActionHttpMethodProvider but does not implement + // IRouteTemplateProvider is used with an attribute that implements IRouteTemplateProvider on + // the same action, the HTTP methods provided by the attribute that only implements + // IActionHttpMethodProvider would be silently ignored, so we choose to throw to + // inform the user of the invalid configuration. + if (hasAttributeRoutedActions && invalidHttpMethodActions.Any()) + { + var errorMessage = CreateInvalidActionHttpMethodProviderErrorMessage( + actionDescriptor, + invalidHttpMethodActions, + actionsForMethod); + + combinedErrorMessage = CombineErrorMessage(combinedErrorMessage, errorMessage); + } + + if (combinedErrorMessage != null) + { + routingConfigurationErrors.Add(actionDescriptor.MethodInfo, combinedErrorMessage); + } + } + + private static void ValidateActionHttpMethodProviders( + ReflectedActionModel reflectedAction, + IDictionary> invalidHttpMethodActions) + { + var invalidHttpMethodProviderAttributes = reflectedAction.Attributes + .Where(attr => attr is IActionHttpMethodProvider && + !(attr is IRouteTemplateProvider)) + .Select(attr => attr.GetType().FullName); + + if (invalidHttpMethodProviderAttributes.Any()) + { + invalidHttpMethodActions.Add( + reflectedAction, + invalidHttpMethodProviderAttributes); + } + } + + private static string CombineErrorMessage(string combinedErrorMessage, string errorMessage) + { + if (combinedErrorMessage == null) + { + combinedErrorMessage = errorMessage; + } + else + { + combinedErrorMessage = string.Join( + Environment.NewLine, + combinedErrorMessage, + errorMessage); + } + + return combinedErrorMessage; + } + + private static string CreateInvalidActionHttpMethodProviderErrorMessage( + ReflectedActionDescriptor actionDescriptor, + IDictionary> invalidHttpMethodActions, + IDictionary> actionsForMethod) + { + var messagesForMethodInfo = new List(); + foreach (var invalidAction in invalidHttpMethodActions) + { + var invalidAttributesList = string.Join(", ", invalidAction.Value); + + foreach (var descriptor in actionsForMethod[invalidAction.Key]) + { + // We only report errors in attribute routed actions. For example, an action + // that contains [HttpGet("Products")], [HttpPost] and [HttpHead], where [HttpHead] + // only implements IHttpActionMethodProvider and restricts the action to only allow + // the head method, will report that the action contains invalid IActionHttpMethodProvider + // attributes only for the action generated by [HttpGet("Products")]. + // [HttpPost] will be treated as an action that produces a conventionally routed action + // and the fact that the method generates attribute and non attributed actions will be + // reported as a different error. + if (IsAttributeRoutedAction(descriptor)) + { + var messageItem = Resources.FormatAttributeRoute_InvalidHttpConstraints_Item( + descriptor.DisplayName, + descriptor.AttributeRouteInfo.Template, + invalidAttributesList, + typeof(IActionHttpMethodProvider).FullName); + + messagesForMethodInfo.Add(messageItem); + } + } + } + + var methodFullName = string.Format("{0}.{1}", + actionDescriptor.MethodInfo.DeclaringType.FullName, + actionDescriptor.MethodInfo.Name); + + // Sample message: + // A method 'MyApplication.CustomerController.Index' that defines attribute routed actions must + // not have attributes that implement 'Microsoft.AspNet.Mvc.IActionHttpMethodProvider' + // and do not implement 'Microsoft.AspNet.Mvc.Routing.IRouteTemplateProvider': + // Action 'MyApplication.CustomerController.Index' has 'Namespace.CustomHttpMethodAttribute' + // invalid 'Microsoft.AspNet.Mvc.IActionHttpMethodProvider' attributes. + return + Resources.FormatAttributeRoute_InvalidHttpConstraints( + methodFullName, + typeof(IActionHttpMethodProvider).FullName, + typeof(IRouteTemplateProvider).FullName, + Environment.NewLine, + string.Join(Environment.NewLine, messagesForMethodInfo)); + } + + private static string CreateMixedRoutedActionDescriptorsErrorMessage( + ReflectedActionDescriptor actionDescriptor, + IDictionary> actionsForMethod) + { + // Text to show as the attribute route template for conventionally routed actions. + var nullTemplate = Resources.AttributeRoute_NullTemplateRepresentation; + + var actionDescriptions = actionsForMethod + .SelectMany(a => a.Value) + .Select(ad => + Resources.FormatAttributeRoute_MixedAttributeAndConventionallyRoutedActions_ForMethod_Item( + ad.DisplayName, + ad.AttributeRouteInfo != null ? ad.AttributeRouteInfo.Template : nullTemplate)); + + var methodFullName = string.Format("{0}.{1}", + actionDescriptor.MethodInfo.DeclaringType.FullName, + actionDescriptor.MethodInfo.Name); + + // Sample error message: + // A method 'MyApplication.CustomerController.Index' must not define attributed actions and + // non attributed actions at the same time: + // Action: 'MyApplication.CustomerController.Index' - Template: 'Products' + // Action: 'MyApplication.CustomerController.Index' - Template: '(none)' + return + Resources.FormatAttributeRoute_MixedAttributeAndConventionallyRoutedActions_ForMethod( + methodFullName, + Environment.NewLine, + string.Join(Environment.NewLine, actionDescriptions)); + } + + private static string CreateAttributeRoutingAggregateErrorMessage( + IEnumerable individualErrors) + { + var errorMessages = AddErrorNumbers(individualErrors); + + var message = Resources.FormatAttributeRoute_AggregateErrorMessage( + Environment.NewLine, + string.Join(Environment.NewLine + Environment.NewLine, errorMessages)); + return message; + } + private static string GetRouteGroupValue(int order, string template) { var group = string.Format("{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 + // routed actions has no attributes that implement IActionHttpMethodProvider and do not + // implement IRouteTemplateProvider. For example: + // + // public class ProductsController + // { + // [HttpGet("Products")] + // [HttpPost] + // public ActionResult Items(){ ... } + // + // [HttpGet("Products")] + // [CustomHttpMethods("POST, PUT")] + // public ActionResult List(){ ... } + // } + private class MethodToActionMap : + Dictionary>> + { + public void AddToMethodInfo(ReflectedActionModel action, + IList actionDescriptors) + { + IDictionary> actionsForMethod = null; + if (TryGetValue(action.ActionMethod, out actionsForMethod)) + { + actionsForMethod.Add(action, actionDescriptors); + } + else + { + var reflectedActionMap = + new Dictionary>(); + reflectedActionMap.Add(action, actionDescriptors); + Add(action.ActionMethod, reflectedActionMap); + } + } + } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedActionModel.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedActionModel.cs index 41b186be57..944511ada4 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedActionModel.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedActionModel.cs @@ -19,13 +19,6 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder Attributes = actionMethod.GetCustomAttributes(inherit: true).OfType().ToList(); Filters = Attributes.OfType().ToList(); - - var routeTemplateAttribute = Attributes.OfType().FirstOrDefault(); - if (routeTemplateAttribute != null) - { - AttributeRouteModel = new ReflectedAttributeRouteModel(routeTemplateAttribute); - } - HttpMethods = new List(); Parameters = new List(); } diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedAttributeRouteModel.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedAttributeRouteModel.cs index a0dc996058..8eb240ebac 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedAttributeRouteModel.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedAttributeRouteModel.cs @@ -30,6 +30,15 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder public string Name { get; set; } + public bool IsAbsoluteTemplate + { + get + { + return Template != null && + IsOverridePattern(Template); + } + } + /// /// Combines two instances and returns /// a new instance with the result. diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedControllerModel.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedControllerModel.cs index 816d36abc8..97cf0e593d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedControllerModel.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedModelBuilder/ReflectedControllerModel.cs @@ -24,11 +24,9 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder Filters = Attributes.OfType().ToList(); RouteConstraints = Attributes.OfType().ToList(); - var routeTemplateAttribute = Attributes.OfType().FirstOrDefault(); - if (routeTemplateAttribute != null) - { - AttributeRouteModel = new ReflectedAttributeRouteModel(routeTemplateAttribute); - } + AttributeRoutes = Attributes.OfType() + .Select(rtp => new ReflectedAttributeRouteModel(rtp)) + .ToList(); ControllerName = controllerType.Name.EndsWith("Controller", StringComparison.Ordinal) ? controllerType.Name.Substring(0, controllerType.Name.Length - "Controller".Length) @@ -47,6 +45,6 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder public List RouteConstraints { get; private set; } - public ReflectedAttributeRouteModel AttributeRouteModel { get; set; } + public List AttributeRoutes { get; private set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Resources.resx b/src/Microsoft.AspNet.Mvc.Core/Resources.resx index d08914297f..ce9383a19a 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.Core/Resources.resx @@ -375,4 +375,22 @@ Error {0}:{1}{2} {0} is the error number, {1} is Environment.NewLine {2} is the error message + + A method '{0}' that defines attribute routed actions must not have attributes that implement '{1}' and do not implement '{2}':{3}{4} + {0} is the MethodInfo.FullName, {1} is typeof(IActionHttpMethodProvider).FullName, {2} is typeof(IRouteTemplateProvider).FullName, {3} is Environment.NewLine, {4} is the list of actions and their respective invalid IActionHttpMethodProvider attributes formatted using AttributeRoute_InvalidHttpMethodConstraints_Item + + + Action '{0}' with route template '{1}' has '{2}' invalid '{3}' attributes. + {0} The display name of the action, {1} the route template, {2} the formatted list of invalid attributes using string.Join(", ", attributes), {3} is typeof(IActionHttpMethodProvider).FullName. + + + A method '{0}' must not define attribute routed actions and non attribute routed actions at the same time:{1}{2} + {0} is the MethodInfo.FullName, {1} is Environment.NewLine, {2} is the formatted list of actions defined by that method info. + + + Action: '{0}' - Template: '{1}' + + + (none) + \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/RouteAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/RouteAttribute.cs index b2ffed2e18..5d3230f5c0 100644 --- a/src/Microsoft.AspNet.Mvc.Core/RouteAttribute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/RouteAttribute.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNet.Mvc /// /// Specifies an attribute route on a controller. /// - [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = false, Inherited = true)] + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, AllowMultiple = true, Inherited = true)] public class RouteAttribute : Attribute, IRouteTemplateProvider { private int? _order; diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionDiscoveryConventionsTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionDiscoveryConventionsTests.cs index f1e2ae5f48..f80f7fbd39 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionDiscoveryConventionsTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionDiscoveryConventionsTests.cs @@ -4,6 +4,9 @@ using System.Reflection; using Microsoft.AspNet.Mvc.DefaultActionDiscoveryConventionsControllers; using Xunit; +using System.Linq; +using System; +using System.Collections.Generic; namespace Microsoft.AspNet.Mvc { @@ -146,6 +149,279 @@ namespace Microsoft.AspNet.Mvc Assert.False(isValid); } + [Fact] + public void GetActions_ConventionallyRoutedAction_WithoutHttpConstraints() + { + // Arrange + var conventions = new DefaultActionDiscoveryConventions(); + var typeInfo = typeof(ConventionallyRoutedController).GetTypeInfo(); + var actionName = nameof(ConventionallyRoutedController.Edit); + + // Act + var actionInfos = conventions.GetActions(typeInfo.GetMethod(actionName), typeInfo); + + // Assert + var action = Assert.Single(actionInfos); + Assert.Equal("Edit", action.ActionName); + Assert.True(action.RequireActionNameMatch); + Assert.Null(action.HttpMethods); + Assert.Null(action.AttributeRoute); + } + + [Fact] + public void GetActions_ConventionallyRoutedAction_WithHttpConstraints() + { + // Arrange + var conventions = new DefaultActionDiscoveryConventions(); + var typeInfo = typeof(ConventionallyRoutedController).GetTypeInfo(); + var actionName = nameof(ConventionallyRoutedController.Update); + + // Act + var actionInfos = conventions.GetActions(typeInfo.GetMethod(actionName), typeInfo); + + // Assert + var action = Assert.Single(actionInfos); + Assert.Equal("Update", action.ActionName); + Assert.True(action.RequireActionNameMatch); + Assert.Equal(new[] { "PUT", "PATCH" }, action.HttpMethods); + Assert.Null(action.AttributeRoute); + } + + [Fact] + public void GetActions_ConventionallyRoutedActionWithHttpConstraints_AndInvalidRouteTemplateProvider() + { + // Arrange + var conventions = new DefaultActionDiscoveryConventions(); + var typeInfo = typeof(ConventionallyRoutedController).GetTypeInfo(); + var actionName = nameof(ConventionallyRoutedController.Delete); + + // Act + var actionInfos = conventions.GetActions(typeInfo.GetMethod(actionName), typeInfo); + + // Assert + var action = Assert.Single(actionInfos); + Assert.Equal("Delete", action.ActionName); + Assert.True(action.RequireActionNameMatch); + + var httpMethod = Assert.Single(action.HttpMethods); + Assert.Equal("DELETE", httpMethod); + Assert.Null(action.AttributeRoute); + } + + [Fact] + public void GetActions_ConventionallyRoutedAction_WithMultipleHttpConstraints() + { + // Arrange + var conventions = new DefaultActionDiscoveryConventions(); + var typeInfo = typeof(ConventionallyRoutedController).GetTypeInfo(); + var actionName = nameof(ConventionallyRoutedController.Details); + + // Act + var actionInfos = conventions.GetActions(typeInfo.GetMethod(actionName), typeInfo); + + // Assert + var action = Assert.Single(actionInfos); + Assert.Equal("Details", action.ActionName); + Assert.True(action.RequireActionNameMatch); + Assert.Equal(new[] { "POST", "GET" }, action.HttpMethods); + Assert.Null(action.AttributeRoute); + } + + [Fact] + public void GetActions_ConventionallyRoutedAction_WithMultipleOverlappingHttpConstraints() + { + // Arrange + var conventions = new DefaultActionDiscoveryConventions(); + var typeInfo = typeof(ConventionallyRoutedController).GetTypeInfo(); + var actionName = nameof(ConventionallyRoutedController.List); + + // Act + var actionInfos = conventions.GetActions(typeInfo.GetMethod(actionName), typeInfo); + + // Assert + var action = Assert.Single(actionInfos); + Assert.Equal("List", action.ActionName); + Assert.True(action.RequireActionNameMatch); + Assert.Equal(new[] { "GET", "PUT", "POST" }, action.HttpMethods); + Assert.Null(action.AttributeRoute); + } + + [Fact] + public void GetActions_AttributeRouteOnAction() + { + // Arrange + var conventions = new DefaultActionDiscoveryConventions(); + var typeInfo = typeof(NoRouteAttributeOnControllerController).GetTypeInfo(); + var actionName = nameof(NoRouteAttributeOnControllerController.Edit); + + // Act + var actionInfos = conventions.GetActions(typeInfo.GetMethod(actionName), typeInfo); + + // Assert + var action = Assert.Single(actionInfos); + + Assert.Equal("Edit", action.ActionName); + Assert.True(action.RequireActionNameMatch); + + var httpMethod = Assert.Single(action.HttpMethods); + Assert.Equal("POST", httpMethod); + + Assert.NotNull(action.AttributeRoute); + Assert.Equal("Change", action.AttributeRoute.Template); + } + + [Fact] + public void GetActions_AttributeRouteOnAction_RouteAttribute() + { + // Arrange + var conventions = new DefaultActionDiscoveryConventions(); + var typeInfo = typeof(NoRouteAttributeOnControllerController).GetTypeInfo(); + var actionName = nameof(NoRouteAttributeOnControllerController.Update); + + // Act + var actionInfos = conventions.GetActions(typeInfo.GetMethod(actionName), typeInfo); + + // Assert + var action = Assert.Single(actionInfos); + + Assert.Equal("Update", action.ActionName); + Assert.True(action.RequireActionNameMatch); + + Assert.Null(action.HttpMethods); + + Assert.NotNull(action.AttributeRoute); + Assert.Equal("Update", action.AttributeRoute.Template); + } + + [Fact] + public void GetActions_AttributeRouteOnAction_AcceptVerbsAttributeWithTemplate() + { + // Arrange + var conventions = new DefaultActionDiscoveryConventions(); + var typeInfo = typeof(NoRouteAttributeOnControllerController).GetTypeInfo(); + var actionName = nameof(NoRouteAttributeOnControllerController.List); + + // Act + var actionInfos = conventions.GetActions(typeInfo.GetMethod(actionName), typeInfo); + + // Assert + var action = Assert.Single(actionInfos); + + Assert.Equal("List", action.ActionName); + Assert.True(action.RequireActionNameMatch); + + Assert.Equal(new[] { "GET", "HEAD" }, action.HttpMethods); + + Assert.NotNull(action.AttributeRoute); + Assert.Equal("ListAll", action.AttributeRoute.Template); + } + + [Fact] + public void GetActions_AttributeRouteOnAction_CreatesOneActionInforPerRouteTemplate() + { + // Arrange + var conventions = new DefaultActionDiscoveryConventions(); + var typeInfo = typeof(NoRouteAttributeOnControllerController).GetTypeInfo(); + var actionName = nameof(NoRouteAttributeOnControllerController.Index); + + // Act + var actionInfos = conventions.GetActions(typeInfo.GetMethod(actionName), typeInfo); + + // Assert + Assert.Equal(2, actionInfos.Count()); + + foreach (var action in actionInfos) + { + Assert.Equal("Index", action.ActionName); + Assert.True(action.RequireActionNameMatch); + + Assert.NotNull(action.AttributeRoute); + } + + var list = Assert.Single(actionInfos, ai => ai.AttributeRoute.Template.Equals("List")); + var listMethod = Assert.Single(list.HttpMethods); + Assert.Equal("POST", listMethod); + + var all = Assert.Single(actionInfos, ai => ai.AttributeRoute.Template.Equals("All")); + var allMethod = Assert.Single(all.HttpMethods); + Assert.Equal("GET", allMethod); + } + + [Fact] + public void GetActions_NoRouteOnController_AllowsConventionallyRoutedActions_OnTheSameController() + { + // Arrange + var conventions = new DefaultActionDiscoveryConventions(); + var typeInfo = typeof(NoRouteAttributeOnControllerController).GetTypeInfo(); + var actionName = nameof(NoRouteAttributeOnControllerController.Remove); + + // Act + var actionInfos = conventions.GetActions(typeInfo.GetMethod(actionName), typeInfo); + + // Assert + var action = Assert.Single(actionInfos); + + Assert.Equal("Remove", action.ActionName); + Assert.True(action.RequireActionNameMatch); + + Assert.Null(action.HttpMethods); + + Assert.Null(action.AttributeRoute); + } + + [Theory] + [InlineData(typeof(SingleRouteAttributeController))] + [InlineData(typeof(MultipleRouteAttributeController))] + public void GetActions_RouteAttributeOnController_CreatesAttributeRoute_ForNonAttributedActions(Type controller) + { + // Arrange + var conventions = new DefaultActionDiscoveryConventions(); + var typeInfo = controller.GetTypeInfo(); + + // Act + var actionInfos = conventions.GetActions(typeInfo.GetMethod("Delete"), typeInfo); + + // Assert + var action = Assert.Single(actionInfos); + + Assert.Equal("Delete", action.ActionName); + Assert.True(action.RequireActionNameMatch); + + Assert.Null(action.HttpMethods); + + Assert.Null(action.AttributeRoute); + } + + [Theory] + [InlineData(typeof(SingleRouteAttributeController))] + [InlineData(typeof(MultipleRouteAttributeController))] + public void GetActions_RouteOnController_CreatesOneActionInforPerRouteTemplateOnAction(Type controller) + { + // Arrange + var conventions = new DefaultActionDiscoveryConventions(); + var typeInfo = controller.GetTypeInfo(); + + // Act + var actionInfos = conventions.GetActions(typeInfo.GetMethod("Index"), typeInfo); + + // Assert + Assert.Equal(2, actionInfos.Count()); + + foreach (var action in actionInfos) + { + Assert.Equal("Index", action.ActionName); + Assert.True(action.RequireActionNameMatch); + + var httpMethod = Assert.Single(action.HttpMethods); + Assert.Equal("GET", httpMethod); + + Assert.NotNull(action.AttributeRoute); + } + + Assert.Single(actionInfos, ai => ai.AttributeRoute.Template.Equals("List")); + Assert.Single(actionInfos, ai => ai.AttributeRoute.Template.Equals("All")); + } + [Fact] public void IsController_UserDefinedClass() { @@ -408,4 +684,84 @@ namespace Microsoft.AspNet.Mvc.DefaultActionDiscoveryConventionsControllers return new OperatorOverloadingController(); } } + + public class NoRouteAttributeOnControllerController : Controller + { + [HttpGet("All")] + [HttpPost("List")] + public void Index() { } + + [HttpPost("Change")] + public void Edit() { } + + public void Remove() { } + + [Route("Update")] + public void Update() { } + + [AcceptVerbs("GET", "HEAD", Route = "ListAll")] + public void List() { } + } + + [Route("Products")] + public class SingleRouteAttributeController : Controller + { + [HttpGet("All")] + [HttpGet("List")] + public void Index() { } + + public void Delete() { } + } + + [Route("Products")] + [Route("Items")] + public class MultipleRouteAttributeController : Controller + { + [HttpGet("All")] + [HttpGet("List")] + public void Index() { } + + public void Delete() { } + } + + // Here the constraints on the methods are acting as an IActionHttpMethodProvider and + // not as an IRouteTemplateProvider given that there is no RouteAttribute + // on the controller and the template for all the constraints on a method is null. + public class ConventionallyRoutedController : Controller + { + public void Edit() { } + + [CustomHttpMethods("PUT", "PATCH")] + public void Update() { } + + [HttpDelete] + public void Delete() { } + + [HttpPost] + [HttpGet] + public void Details() { } + + [HttpGet] + [HttpPut] + [AcceptVerbs("GET", "POST")] + public void List() { } + + // Keep it private and nested to avoid polluting the namespace. + private class CustomHttpMethodsAttribute : Attribute, IActionHttpMethodProvider + { + private readonly string[] _methods; + + public CustomHttpMethodsAttribute(params string [] methods) + { + _methods = methods; + } + public IEnumerable HttpMethods + { + get + { + return _methods; + } + } + } + } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs index fb03777e6f..069689b43b 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedActionDescriptorProviderTests.cs @@ -58,23 +58,55 @@ namespace Microsoft.AspNet.Mvc.Test Assert.Equal(FilterScope.Action, filter3.Scope); } - [Theory] - [InlineData(typeof(HttpMethodController), nameof(HttpMethodController.OnlyPost), "POST")] - [InlineData(typeof(AttributeRoutedHttpMethodController), nameof(AttributeRoutedHttpMethodController.PutOrPatch), "PUT,PATCH")] - public void GetDescriptors_AddsHttpMethodConstraints(Type controllerType, string actionName, string expectedMethods) + [Fact] + public void GetDescriptors_AddsHttpMethodConstraints_ForConventionallyRoutedActions() { // Arrange - var provider = GetProvider(controllerType.GetTypeInfo()); + var provider = GetProvider(typeof(HttpMethodController).GetTypeInfo()); // Act var descriptors = provider.GetDescriptors(); var descriptor = Assert.Single(descriptors); // Assert - Assert.Equal(actionName, descriptor.Name); + Assert.Equal("OnlyPost", descriptor.Name); Assert.Single(descriptor.MethodConstraints); - Assert.Equal(expectedMethods.Split(','), descriptor.MethodConstraints[0].HttpMethods); + Assert.Equal(new string[] { "POST" }, descriptor.MethodConstraints[0].HttpMethods); + } + + [Fact] + public void GetDescriptors_ThrowsIfHttpMethodConstraints_OnAttributeRoutedActions() + { + // Arrange + var expectedExceptionMessage = + "The following errors occurred with attribute routing information:" + Environment.NewLine + + Environment.NewLine + + "Error 1:" + Environment.NewLine + + "A method 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "AttributeRoutedHttpMethodController.PutOrPatch'" + + " that defines attribute routed actions must not have attributes that implement " + + "'Microsoft.AspNet.Mvc.IActionHttpMethodProvider' and do not implement " + + "'Microsoft.AspNet.Mvc.Routing.IRouteTemplateProvider':" + Environment.NewLine + + "Action 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "AttributeRoutedHttpMethodController.PutOrPatch' with route template 'Products' has " + + "'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+CustomHttpMethodConstraintAttribute'" + + " invalid 'Microsoft.AspNet.Mvc.IActionHttpMethodProvider' attributes." + Environment.NewLine + + "Action 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "AttributeRoutedHttpMethodController.PutOrPatch' with route template 'Items' has " + + "'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+CustomHttpMethodConstraintAttribute'" + + " invalid 'Microsoft.AspNet.Mvc.IActionHttpMethodProvider' attributes."; + + var provider = GetProvider( + typeof(AttributeRoutedHttpMethodController) + .GetTypeInfo()); + + // Act + var ex = Assert.Throws( + () => provider.GetDescriptors()); + + // Act + Assert.Equal(expectedExceptionMessage, ex.Message); } [Fact] @@ -450,13 +482,13 @@ namespace Microsoft.AspNet.Mvc.Test var conventional = Assert.Single(model.Controllers, c => c.ControllerName == "ConventionallyRouted"); - Assert.Null(conventional.AttributeRouteModel); + Assert.Empty(conventional.AttributeRoutes); Assert.Single(conventional.Actions); var attributeRouted = Assert.Single(model.Controllers, c => c.ControllerName == "AttributeRouted"); Assert.Single(attributeRouted.Actions); - Assert.NotNull(attributeRouted.AttributeRouteModel); + Assert.Single(attributeRouted.AttributeRoutes); var empty = Assert.Single(model.Controllers, c => c.ControllerName == "Empty"); @@ -521,7 +553,9 @@ namespace Microsoft.AspNet.Mvc.Test // Assert var controller = Assert.Single(model.Controllers); - Assert.Equal("api/Token/[key]/[controller]", controller.AttributeRouteModel.Template); + + var attributeRouteModel = Assert.Single(controller.AttributeRoutes); + Assert.Equal("api/Token/[key]/[controller]", attributeRouteModel.Template); var action = Assert.Single(controller.Actions); Assert.Equal("stub/[action]", action.AttributeRouteModel.Template); @@ -550,11 +584,13 @@ namespace Microsoft.AspNet.Mvc.Test var expectedMessage = "The following errors occurred with attribute routing information:" + Environment.NewLine + Environment.NewLine + + "Error 1:" + Environment.NewLine + "For action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + "MultipleErrorsController.Unknown'" + Environment.NewLine + "Error: While processing template 'stub/[action]/[unknown]', a replacement value for the token 'unknown' " + "could not be found. Available tokens: 'controller, action'." + Environment.NewLine + Environment.NewLine + + "Error 2:" + Environment.NewLine + "For action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + "MultipleErrorsController.Invalid'" + Environment.NewLine + "Error: The route template '[invalid/syntax' has invalid syntax. A replacement token is not closed."; @@ -566,6 +602,211 @@ namespace Microsoft.AspNet.Mvc.Test Assert.Equal(expectedMessage, ex.Message); } + [Fact] + public void AttributeRouting_CreatesOneActionDescriptor_PerControllerAndActionRouteCombination() + { + // Arrange + var provider = GetProvider(typeof(MultiRouteAttributesController).GetTypeInfo()); + + // Act + var descriptors = provider.GetDescriptors(); + + // Assert + var actions = descriptors.Where(d => d.Name == "MultipleHttpGet"); + Assert.Equal(4, actions.Count()); + + foreach (var action in actions) + { + Assert.Equal("MultipleHttpGet", action.Name); + Assert.Equal("MultiRouteAttributes", action.ControllerName); + } + + Assert.Single(actions, a => a.AttributeRouteInfo.Template.Equals("v1/List")); + Assert.Single(actions, a => a.AttributeRouteInfo.Template.Equals("v1/All")); + Assert.Single(actions, a => a.AttributeRouteInfo.Template.Equals("v2/List")); + Assert.Single(actions, a => a.AttributeRouteInfo.Template.Equals("v2/All")); + } + + [Fact] + public void AttributeRouting_AcceptVerbsOnAction_CreatesActionPerControllerAttributeRouteCombination() + { + // Arrange + var provider = GetProvider(typeof(MultiRouteAttributesController).GetTypeInfo()); + + // Act + var descriptors = provider.GetDescriptors(); + + // Assert + var actions = descriptors.Where(d => d.Name == "AcceptVerbs"); + Assert.Equal(2, actions.Count()); + + foreach (var action in actions) + { + Assert.Equal("MultiRouteAttributes", action.ControllerName); + + Assert.NotNull(action.MethodConstraints); + var methodConstraint = Assert.Single(action.MethodConstraints); + + Assert.NotNull(methodConstraint.HttpMethods); + Assert.Equal(new[] { "POST" }, methodConstraint.HttpMethods); + } + + Assert.Single(actions, a => a.AttributeRouteInfo.Template.Equals("v1/List")); + Assert.Single(actions, a => a.AttributeRouteInfo.Template.Equals("v2/List")); + } + + [Fact] + public void AttributeRouting_AcceptVerbsOnActionWithOverrideTemplate_CreatesSingleAttributeRoutedAction() + { + // Arrange + var provider = GetProvider(typeof(MultiRouteAttributesController).GetTypeInfo()); + + // Act + var descriptors = provider.GetDescriptors(); + + // Assert + var action = Assert.Single(descriptors, d => d.Name == "AcceptVerbsOverride"); + + Assert.Equal("MultiRouteAttributes", action.ControllerName); + + Assert.NotNull(action.MethodConstraints); + var methodConstraint = Assert.Single(action.MethodConstraints); + + Assert.NotNull(methodConstraint.HttpMethods); + Assert.Equal(new[] { "PUT" }, methodConstraint.HttpMethods); + + Assert.NotNull(action.AttributeRouteInfo); + Assert.Equal("Override", action.AttributeRouteInfo.Template); + } + + [Fact] + public void AttributeRouting_AcceptVerbsOnAction_DoesNotApplyHttpMethods_ToOtherAttributeRoutes() + { + // Arrange + var provider = GetProvider(typeof(MultiRouteAttributesController).GetTypeInfo()); + + // Act + var descriptors = provider.GetDescriptors(); + + // Assert + var actions = descriptors.Where(d => d.Name == "AcceptVerbsRouteAttributeAndHttpPut"); + Assert.Equal(6, actions.Count()); + + foreach (var action in actions) + { + Assert.Equal("MultiRouteAttributes", action.ControllerName); + + Assert.NotNull(action.AttributeRouteInfo); + Assert.NotNull(action.AttributeRouteInfo.Template); + } + + var constrainedActions = actions.Where(a => a.MethodConstraints != null); + Assert.Equal(4, constrainedActions.Count()); + + // Actions generated by AcceptVerbs + var postActions = constrainedActions.Where(a => a.MethodConstraints.Single().HttpMethods.Single() == "POST"); + Assert.Equal(2, postActions.Count()); + Assert.Single(postActions, a => a.AttributeRouteInfo.Template.Equals("v1")); + Assert.Single(postActions, a => a.AttributeRouteInfo.Template.Equals("v2")); + + // Actions generated by PutAttribute + var putActions = constrainedActions.Where(a => a.MethodConstraints.Single().HttpMethods.Single() == "PUT"); + Assert.Equal(2, putActions.Count()); + Assert.Single(putActions, a => a.AttributeRouteInfo.Template.Equals("v1/All")); + Assert.Single(putActions, a => a.AttributeRouteInfo.Template.Equals("v2/All")); + + // Actions generated by RouteAttribute + var unconstrainedActions = actions.Where(a => a.MethodConstraints == null); + Assert.Equal(2, unconstrainedActions.Count()); + Assert.Single(unconstrainedActions, a => a.AttributeRouteInfo.Template.Equals("v1/List")); + Assert.Single(unconstrainedActions, a => a.AttributeRouteInfo.Template.Equals("v2/List")); + } + + [Fact] + public void AttributeRouting_AllowsDuplicateAttributeRoutedActions_WithTheSameTemplateAndSameHttpMethodsOnDifferentActions() + { + // Arrange + var provider = GetProvider(typeof(NonDuplicatedAttributeRouteController).GetTypeInfo()); + var firstActionName = nameof(NonDuplicatedAttributeRouteController.ControllerAndAction); + var secondActionName = nameof(NonDuplicatedAttributeRouteController.OverrideOnAction); + + // Act + var actions = provider.GetDescriptors(); + + // Assert + var controllerAndAction = Assert.Single(actions, a => a.Name.Equals(firstActionName)); + Assert.NotNull(controllerAndAction.AttributeRouteInfo); + + var controllerActionAndOverride = Assert.Single(actions, a => a.Name.Equals(secondActionName)); + Assert.NotNull(controllerActionAndOverride.AttributeRouteInfo); + + Assert.Equal( + controllerAndAction.AttributeRouteInfo.Template, + controllerActionAndOverride.AttributeRouteInfo.Template, + StringComparer.OrdinalIgnoreCase); + } + + [Fact] + public void AttributeRouting_AllowsDuplicateAttributeRoutedActions_WithTheSameTemplateAndDifferentHttpMethodsOnTheSameAction() + { + // Arrange + var provider = GetProvider(typeof(NonDuplicatedAttributeRouteController).GetTypeInfo()); + var actionName = nameof(NonDuplicatedAttributeRouteController.DifferentHttpMethods); + + // Act + var descriptors = provider.GetDescriptors(); + + // Assert + var actions = descriptors.Where(d => d.Name.Equals(actionName)); + Assert.Equal(5, actions.Count()); + + foreach (var method in new[] { "GET", "POST", "PUT", "PATCH", "DELETE" }) + { + var action = Assert.Single( + actions, + a => a.MethodConstraints.SelectMany(c => c.HttpMethods).Contains(method)); + + Assert.NotNull(action.AttributeRouteInfo); + Assert.Equal("Products/list", action.AttributeRouteInfo.Template); + } + } + + [Fact] + public void AttributeRouting_ThrowsIfAttributeRoutedAndNonAttributedActions_OnTheSameMethod() + { + // Arrange + var expectedMessage = + "The following errors occurred with attribute routing information:" + Environment.NewLine + + Environment.NewLine + + "Error 1:" + Environment.NewLine + + "A method 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "AttributeAndNonAttributeRoutedActionsOnSameMethodController.Method'" + + " must not define attribute routed actions and non attribute routed actions at the same time:" + Environment.NewLine + + "Action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "AttributeAndNonAttributeRoutedActionsOnSameMethodController.Method' - Template: 'AttributeRouted'" + Environment.NewLine + + "Action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "AttributeAndNonAttributeRoutedActionsOnSameMethodController.Method' - Template: '(none)'" + Environment.NewLine + + "Action: 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "AttributeAndNonAttributeRoutedActionsOnSameMethodController.Method' - Template: '(none)'" + Environment.NewLine + + "A method 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "AttributeAndNonAttributeRoutedActionsOnSameMethodController.Method' that defines attribute routed actions must not" + + " have attributes that implement 'Microsoft.AspNet.Mvc.IActionHttpMethodProvider' and do not implement" + + " 'Microsoft.AspNet.Mvc.Routing.IRouteTemplateProvider':" + Environment.NewLine + + "Action 'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+" + + "AttributeAndNonAttributeRoutedActionsOnSameMethodController.Method' with route template 'AttributeRouted' has " + + "'Microsoft.AspNet.Mvc.Test.ReflectedActionDescriptorProviderTests+CustomHttpMethodConstraintAttribute'" + + " invalid 'Microsoft.AspNet.Mvc.IActionHttpMethodProvider' attributes."; + + var provider = GetProvider( + typeof(AttributeAndNonAttributeRoutedActionsOnSameMethodController).GetTypeInfo()); + + // Act + var exception = Assert.Throws(() => provider.GetDescriptors()); + + // Assert + Assert.Equal(expectedMessage, exception.Message); + } + [Fact] public void AttributeRouting_RouteOnControllerAndAction_CreatesActionDescriptorWithoutHttpConstraints() { @@ -843,9 +1084,10 @@ namespace Microsoft.AspNet.Mvc.Test } [Route("Products")] + [Route("Items")] private class AttributeRoutedHttpMethodController { - [AcceptVerbs("PUT", "PATCH")] + [CustomHttpMethodConstraint("PUT", "PATCH")] public void PutOrPatch() { } } @@ -1013,6 +1255,26 @@ namespace Microsoft.AspNet.Mvc.Test public void Delete(int id) { } } + [Route("v1")] + [Route("v2")] + public class MultiRouteAttributesController + { + [HttpGet("List")] + [HttpGet("All")] + public void MultipleHttpGet() { } + + [AcceptVerbs("POST", Route = "List")] + public void AcceptVerbs() { } + + [AcceptVerbs("PUT", Route = "/Override")] + public void AcceptVerbsOverride() { } + + [AcceptVerbs("POST")] + [Route("List")] + [HttpPut("All")] + public void AcceptVerbsRouteAttributeAndHttpPut() { } + } + [Route("Products")] public class OnlyRouteController { @@ -1020,6 +1282,48 @@ namespace Microsoft.AspNet.Mvc.Test public void Action() { } } + public class AttributeAndNonAttributeRoutedActionsOnSameMethodController + { + [HttpGet("AttributeRouted")] + [HttpPost] + [AcceptVerbs("PUT", "PATCH")] + [CustomHttpMethodConstraint("DELETE")] + public void Method() { } + } + + [Route("Product")] + [Route("/Product")] + [Route("/product")] + public class DuplicatedAttributeRouteController : Controller + { + [HttpGet("/List")] + [HttpGet("/List")] + public void Action() { } + + public void Controller() { } + + [HttpPut("list")] + [PutOrPatch("list")] + public void CommonHttpMethod() { } + } + + [Route("Products")] + public class NonDuplicatedAttributeRouteController : Controller + { + [HttpGet("list")] + public void ControllerAndAction() { } + + [HttpGet("/PRODUCTS/LIST")] + public void OverrideOnAction() { } + + [HttpGet("list")] + [HttpPost("list")] + [HttpPut("list")] + [HttpPatch("list")] + [HttpDelete("list")] + public void DifferentHttpMethods() { } + } + [MyRouteConstraint(blockNonAttributedActions: true)] [MySecondRouteConstraint(blockNonAttributedActions: true)] private class ConstrainedController @@ -1066,6 +1370,34 @@ namespace Microsoft.AspNet.Mvc.Test public void Action() { } } + private class CustomHttpMethodConstraintAttribute : Attribute, IActionHttpMethodProvider + { + private readonly string[] _methods; + + public CustomHttpMethodConstraintAttribute(params string[] methods) + { + _methods = methods; + } + + public IEnumerable HttpMethods + { + get + { + return _methods; + } + } + } + + private class PutOrPatchAttribute : HttpMethodAttribute + { + private static readonly string[] _httpMethods = new string[] { "PUT", "PATCH" }; + + public PutOrPatchAttribute(string template) + : base(_httpMethods, template) + { + } + } + private class TestActionParameter { public int Id { get; set; } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedModelBuilder/ReflectedActionModelTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedModelBuilder/ReflectedActionModelTests.cs index 3e14da0b50..5c51d5da5b 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedModelBuilder/ReflectedActionModelTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedModelBuilder/ReflectedActionModelTests.cs @@ -38,20 +38,6 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder.Test Assert.IsType(model.Filters[0]); } - [Fact] - public void ReflectedActionModel_PopulatesAttributeRouteInfo() - { - // Arrange - var actionMethod = typeof(BlogController).GetMethod("Edit"); - - // Act - var model = new ReflectedActionModel(actionMethod); - - // Assert - Assert.NotNull(model.AttributeRouteModel); - Assert.Equal("Edit", model.AttributeRouteModel.Template); - } - private class BlogController { [MyOther] diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedModelBuilder/ReflectedControllerModelTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedModelBuilder/ReflectedControllerModelTests.cs index b3664d283d..128985d8e5 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedModelBuilder/ReflectedControllerModelTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ReflectedModelBuilder/ReflectedControllerModelTests.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.Linq; using System.Reflection; using Xunit; @@ -19,12 +20,16 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder.Test var model = new ReflectedControllerModel(controllerType.GetTypeInfo()); // Assert - Assert.Equal(4, model.Attributes.Count); + Assert.Equal(5, model.Attributes.Count); Assert.Single(model.Attributes, a => a is MyOtherAttribute); Assert.Single(model.Attributes, a => a is MyFilterAttribute); Assert.Single(model.Attributes, a => a is MyRouteConstraintAttribute); - Assert.Single(model.Attributes, a => a is RouteAttribute); + + var routes = model.Attributes.OfType().ToList(); + Assert.Equal(2, routes.Count()); + Assert.Single(routes, r => r.Template.Equals("Blog")); + Assert.Single(routes, r => r.Template.Equals("Microblog")); } [Fact] @@ -91,14 +96,17 @@ namespace Microsoft.AspNet.Mvc.ReflectedModelBuilder.Test var model = new ReflectedControllerModel(controllerType.GetTypeInfo()); // Assert - Assert.NotNull(model.AttributeRouteModel); - Assert.Equal("Blog", model.AttributeRouteModel.Template); + Assert.NotNull(model.AttributeRoutes); + Assert.Equal(2, model.AttributeRoutes.Count); ; + Assert.Single(model.AttributeRoutes, r => r.Template.Equals("Blog")); + Assert.Single(model.AttributeRoutes, r => r.Template.Equals("Microblog")); } [MyOther] [MyFilter] [MyRouteConstraint] [Route("Blog")] + [Route("Microblog")] private class BlogController { } diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs index e545198a52..5282d7fdcc 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/RoutingTests.cs @@ -162,6 +162,163 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests result.RouteValues); } + [Theory] + [InlineData("http://localhost/api/v1/Maps")] + [InlineData("http://localhost/api/v2/Maps")] + public async Task AttributeRoutedAction_MultipleRouteAttributes_WorksWithNameAndOrder(string url) + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.CreateClient(); + + // Act + var response = await client.GetAsync(url); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var body = await response.Content.ReadAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + Assert.Equal("Maps", result.Controller); + Assert.Equal("Get", result.Action); + + Assert.Equal(new string[] + { + "/api/v2/Maps", + "/api/v1/Maps", + "/api/v2/Maps" + }, + result.ExpectedUrls); + } + + [Fact] + public async Task AttributeRoutedAction_MultipleRouteAttributes_WorksWithOverrideRoutes() + { + // Arrange + var url = "http://localhost/api/v2/Maps"; + var server = TestServer.Create(_services, _app); + var client = server.CreateClient(); + + // Act + var response = await client.SendAsync(new HttpRequestMessage(HttpMethod.Post, url)); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var body = await response.Content.ReadAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + Assert.Equal("Maps", result.Controller); + Assert.Equal("Post", result.Action); + + Assert.Equal(new string[] + { + "/api/v2/Maps", + "/api/v2/Maps" + }, + result.ExpectedUrls); + } + + [Fact] + public async Task AttributeRoutedAction_MultipleRouteAttributes_RouteAttributeTemplatesIgnoredForOverrideActions() + { + // Arrange + var url = "http://localhost/api/v1/Maps"; + var server = TestServer.Create(_services, _app); + var client = server.CreateClient(); + + // Act + var response = await client.SendAsync(new HttpRequestMessage(new HttpMethod("POST"), url)); + + // Assert + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + + [Theory] + [InlineData("http://localhost/api/v1/Maps/5", "PUT")] + [InlineData("http://localhost/api/v2/Maps/5", "PUT")] + [InlineData("http://localhost/api/v1/Maps/PartialUpdate/5", "PATCH")] + [InlineData("http://localhost/api/v2/Maps/PartialUpdate/5", "PATCH")] + public async Task AttributeRoutedAction_MultipleRouteAttributes_CombinesWithMultipleHttpAttributes( + string url, + string method) + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.CreateClient(); + + // Act + var response = await client.SendAsync(new HttpRequestMessage(new HttpMethod(method), url)); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var body = await response.Content.ReadAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + Assert.Equal("Maps", result.Controller); + Assert.Equal("Update", result.Action); + + Assert.Equal(new string[] + { + "/api/v2/Maps/PartialUpdate/5", + "/api/v2/Maps/PartialUpdate/5" + }, + result.ExpectedUrls); + } + + [Theory] + [InlineData("http://localhost/Banks/Get/5")] + [InlineData("http://localhost/Bank/Get/5")] + public async Task AttributeRoutedAction_MultipleHttpAttributesAndTokenReplacement(string url) + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.CreateClient(); + var expectedUrl = new Uri(url).AbsolutePath; + + // Act + var response = await client.GetAsync(url); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var body = await response.Content.ReadAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + Assert.Equal("Banks", result.Controller); + Assert.Equal("Get", result.Action); + + Assert.Equal(new string[] + { + "/Bank/Get/5", + "/Bank/Get/5" + }, + result.ExpectedUrls); + } + + [Theory] + [InlineData("http://localhost/api/v1/Maps/5", "PATCH")] + [InlineData("http://localhost/api/v2/Maps/5", "PATCH")] + [InlineData("http://localhost/api/v1/Maps/PartialUpdate/5", "PUT")] + [InlineData("http://localhost/api/v2/Maps/PartialUpdate/5", "PUT")] + public async Task AttributeRoutedAction_MultipleRouteAttributes_WithMultipleHttpAttributes_RespectsConstraints( + string url, + string method) + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.CreateClient(); + var expectedUrl = new Uri(url).AbsolutePath; + + // Act + var response = await client.SendAsync(new HttpRequestMessage(new HttpMethod(method), url)); + + // Assert + Assert.Equal(HttpStatusCode.NotFound, response.StatusCode); + } + // The url would be /Store/ListProducts with conventional routes [Fact] public async Task AttributeRoutedAction_IsNotReachableWithTraditionalRoute() @@ -359,6 +516,57 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests Assert.Equal("UpdateEmployee", result.Action); } + [Theory] + [InlineData("PUT")] + [InlineData("PATCH")] + public async Task AttributeRoutedAction_ControllerLevelRoute_WithAcceptVerbsAndRouteTemplate_IsReachable(string verb) + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.CreateClient(); + + // Act + var message = new HttpRequestMessage(new HttpMethod(verb), "http://localhost/api/Employee/Manager"); + var response = await client.SendAsync(message); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var body = await response.Content.ReadAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + Assert.Contains("/api/Employee/Manager", result.ExpectedUrls); + Assert.Equal("Employee", result.Controller); + Assert.Equal("UpdateManager", result.Action); + } + + [Theory] + [InlineData("PUT", "Bank")] + [InlineData("PATCH", "Bank")] + [InlineData("PUT", "Bank/Update")] + [InlineData("PATCH", "Bank/Update")] + public async Task AttributeRoutedAction_AcceptVerbsAndRouteTemplate_IsReachable(string verb, string path) + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.CreateClient(); + var expectedUrl = "/Bank"; + + // Act + var message = new HttpRequestMessage(new HttpMethod(verb), "http://localhost/" + path); + var response = await client.SendAsync(message); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var body = await response.Content.ReadAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + Assert.Equal(new string[] { expectedUrl, expectedUrl }, result.ExpectedUrls); + Assert.Equal("Banks", result.Controller); + Assert.Equal("UpdateBank", result.Action); + } + [Fact] public async Task AttributeRoutedAction_WithCustomHttpAttributes_IsReachable() { diff --git a/test/WebSites/RoutingWebSite/Controllers/BanksController.cs b/test/WebSites/RoutingWebSite/Controllers/BanksController.cs new file mode 100644 index 0000000000..0458571362 --- /dev/null +++ b/test/WebSites/RoutingWebSite/Controllers/BanksController.cs @@ -0,0 +1,35 @@ +using Microsoft.AspNet.Mvc; +using System; + +namespace RoutingWebSite +{ + public class BanksController : Controller + { + private readonly TestResponseGenerator _generator; + + public BanksController(TestResponseGenerator generator) + { + _generator = generator; + } + + [HttpGet("Banks/[action]/{id}")] + [HttpGet("Bank/[action]/{id}")] + public ActionResult Get(int id) + { + return _generator.Generate( + Url.Action(), + Url.RouteUrl(new { })); + } + + [AcceptVerbs("PUT", Route ="Bank")] + [HttpPatch("Bank")] + [AcceptVerbs("PUT", Route ="Bank/Update")] + [HttpPatch("Bank/Update")] + public ActionResult UpdateBank() + { + return _generator.Generate( + Url.Action(), + Url.RouteUrl(new { })); + } + } +} \ No newline at end of file diff --git a/test/WebSites/RoutingWebSite/Controllers/EmployeeController.cs b/test/WebSites/RoutingWebSite/Controllers/EmployeeController.cs index a712afb8e3..3385c118dd 100644 --- a/test/WebSites/RoutingWebSite/Controllers/EmployeeController.cs +++ b/test/WebSites/RoutingWebSite/Controllers/EmployeeController.cs @@ -26,6 +26,12 @@ namespace RoutingWebSite return _generator.Generate("/api/Employee"); } + [AcceptVerbs("PUT", "PATCH", Route = "Manager")] + public IActionResult UpdateManager() + { + return _generator.Generate("/api/Employee/Manager"); + } + [HttpMerge("{id}")] public IActionResult MergeEmployee(int id) { diff --git a/test/WebSites/RoutingWebSite/Controllers/MapsController.cs b/test/WebSites/RoutingWebSite/Controllers/MapsController.cs new file mode 100644 index 0000000000..0a44ba2374 --- /dev/null +++ b/test/WebSites/RoutingWebSite/Controllers/MapsController.cs @@ -0,0 +1,51 @@ +using Microsoft.AspNet.Mvc; +using System; + +namespace RoutingWebSite +{ + [Route("api/v1/Maps", Name = "v1", Order = 1)] + [Route("api/v2/Maps")] + public class MapsController : Controller + { + private readonly TestResponseGenerator _generator; + + public MapsController(TestResponseGenerator generator) + { + _generator = generator; + } + + [HttpGet] + public ActionResult Get() + { + // Multiple attribute routes with name and order. + // We will always generate v2 routes except when + // we explicitly use "v1" to generate a v1 route. + return _generator.Generate( + Url.Action(), + Url.RouteUrl("v1"), + Url.RouteUrl(new { })); + } + + [HttpPost("/api/v2/Maps")] + public ActionResult Post() + { + return _generator.Generate( + Url.Action(), + Url.RouteUrl(new { })); + } + + [HttpPut("{id}")] + [HttpPatch("PartialUpdate/{id}")] + public ActionResult Update(int id) + { + // We will generate "/api/v2/Maps/PartialUpdate/{id}" + // in both cases, v1 routes will be discarded due to their + // Order and for v2 routes PartialUpdate has higher precedence. + // api/v1/Maps/{id} and api/v2/Maps/{id} will only match on PUT. + // api/v1/Maps/PartialUpdate/{id} and api/v2/Maps/PartialUpdate/{id} will only match on PATCH. + return _generator.Generate( + Url.Action(), + Url.RouteUrl(new { })); + } + } +} \ No newline at end of file