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