From 1521f9298bc44e70d0fc5f9bc0814e101bbcc479 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Sat, 17 Nov 2018 16:27:11 -0800 Subject: [PATCH] Relayer action discovery infrastructure This change introduces a service facade for creating the application model, running conventions, validating the result, and flattening the model. This is used in the ControllerActionDescriptorProvider and provides the existing functionality for now. The ControllerActionDescriptorProvider will process the results and turn each 'flattened' model into a single action descriptor. The next change will introduce another consumer of this service, that turns the 'flattened' model into an EndpointModel so that it can be exposed via Endpoint Routing's convention system. --- The main considerations here: The flattening semantics of application model are pretty complicated :( The validation that CADP does is actually pretty in depth and might be really low value... Errors with writing route templates do happen, and those will be caught by the routing system eventually.... Errors with duplicate route names are similar... Errors with 'mixed' attribute and conventional routing are not at all common. I don't think I've ever seen an issue get filed about this. I did the work to port all of this stuff forward but I'm not totally sure it's valuable - however, I don't really want to make an argument for its removal. These are just some random thoughts to keep in mind if you're reviewing this :+1: --- .../ActionAttributeRouteModel.cs | 130 +++++ .../ApplicationModelFactory.cs | 364 +++++++++++++ .../ControllerActionDescriptorBuilder.cs | 489 ++---------------- .../ControllerActionDescriptorProvider.cs | 42 +- .../InferParameterBindingInfoConvention.cs | 6 +- .../MvcCoreServiceCollectionExtensions.cs | 2 +- ...ControllerActionDescriptorProviderTests.cs | 168 +----- .../DefaultApplicationModelProviderTest.cs | 110 +++- .../Infrastructure/ActionSelectorTest.cs | 3 +- 9 files changed, 682 insertions(+), 632 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ApplicationModelFactory.cs diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ActionAttributeRouteModel.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ActionAttributeRouteModel.cs index dc81fd4a3c..b7a60f3362 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ActionAttributeRouteModel.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ActionAttributeRouteModel.cs @@ -3,11 +3,141 @@ using System.Collections.Generic; using System.Linq; +using Microsoft.AspNetCore.Mvc.ActionConstraints; +using Microsoft.AspNetCore.Mvc.Routing; namespace Microsoft.AspNetCore.Mvc.ApplicationModels { internal static class ActionAttributeRouteModel { + public static IEnumerable FlattenSelectors(ActionModel actionModel) + { + // Loop through all attribute routes defined on the controller. + // These perform a cross-product with all of the action-level attribute routes. + var controllerSelectors = actionModel.Controller.Selectors + .Where(sm => sm.AttributeRouteModel != null) + .ToList(); + + // We also include metadata and action constraints from the controller + // even when there are no routes, or when an action overrides the route template. + SelectorModel additionalSelector = null; + if (actionModel.Controller.Selectors.Count > 0) + { + // This logic seems arbitrary but there's a good reason for it. + // + // When we build the controller level selectors, any metadata or action constraints + // that aren't IRouteTemplateProvider will be included in all selectors. So we + // pick any selector and then grab all of the stuff that isn't IRouteTemplateProvider + // then we've found all of the items that aren't routes. + // + // This is fragile wrt application model customizing the data - but no one has + // run into an issue with this and its pretty esoteric. + additionalSelector = new SelectorModel(actionModel.Controller.Selectors.First()); + additionalSelector.AttributeRouteModel = null; + + for (var i = additionalSelector.ActionConstraints.Count - 1; i >= 0; i--) + { + if (additionalSelector.ActionConstraints[i] is IRouteTemplateProvider) + { + additionalSelector.ActionConstraints.RemoveAt(i); + } + } + + for (var i = additionalSelector.EndpointMetadata.Count - 1; i >= 0; i--) + { + if (additionalSelector.EndpointMetadata[i] is IRouteTemplateProvider) + { + additionalSelector.EndpointMetadata.RemoveAt(i); + } + } + } + + var actionConstraints = new List(); + + foreach (var actionSelector in actionModel.Selectors) + { + var actionRouteModel = actionSelector.AttributeRouteModel; + + // 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 + if (actionRouteModel != null && actionRouteModel.IsAbsoluteTemplate) + { + // We're overriding the routes from the controller, but any *unbound* constraints + // still apply. + var selector = new SelectorModel(actionSelector); + + selector.AttributeRouteModel = AttributeRouteModel.CombineAttributeRouteModel( + left: null, + right: actionRouteModel); + + AddActionConstraints(selector, additionalSelector?.ActionConstraints); + AddEndpointMetadata(selector, additionalSelector?.EndpointMetadata); + + yield return selector; + } + else if (controllerSelectors.Count > 0) + { + for (var i = 0; i < controllerSelectors.Count; i++) + { + var controllerSelector = controllerSelectors[i]; + + // We're using the attribute routes from the controller + var selector = new SelectorModel(actionSelector); + + selector.AttributeRouteModel = AttributeRouteModel.CombineAttributeRouteModel( + controllerSelector.AttributeRouteModel, + actionRouteModel); + + AddActionConstraints(selector, controllerSelector.ActionConstraints); + AddEndpointMetadata(selector, controllerSelector.EndpointMetadata); + + // No need to include the additional selector here because it would duplicate + // data in controllerSelector. + + yield return selector; + } + } + else + { + // There are no routes on the controller, but any *unbound* constraints + // still apply. + var selector = new SelectorModel(actionSelector); + + selector.AttributeRouteModel = AttributeRouteModel.CombineAttributeRouteModel( + left: null, + right: actionRouteModel); + + AddActionConstraints(selector, additionalSelector?.ActionConstraints); + AddEndpointMetadata(selector, additionalSelector?.EndpointMetadata); + + yield return selector; + } + } + } + + private static void AddActionConstraints(SelectorModel selector, IList actionConstraints) + { + if (actionConstraints != null) + { + for (var i = 0; i < actionConstraints.Count;i++) + { + selector.ActionConstraints.Add(actionConstraints[i]); + } + } + } + + private static void AddEndpointMetadata(SelectorModel selector, IList metadata) + { + if (metadata != null) + { + for (var i = 0; i < metadata.Count; i++) + { + selector.EndpointMetadata.Add(metadata[i]); + } + } + } + public static IEnumerable<(AttributeRouteModel route, SelectorModel actionSelector, SelectorModel controllerSelector)> GetAttributeRoutes(ActionModel actionModel) { var controllerAttributeRoutes = actionModel.Controller.Selectors diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ApplicationModelFactory.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ApplicationModelFactory.cs new file mode 100644 index 0000000000..f93fcb5eff --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ApplicationModelFactory.cs @@ -0,0 +1,364 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Reflection; +using Microsoft.AspNetCore.Mvc.ActionConstraints; +using Microsoft.AspNetCore.Mvc.Core; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.Internal; +using Microsoft.Extensions.Options; + +namespace Microsoft.AspNetCore.Mvc.ApplicationModels +{ + /// + /// A facade service for creating application models. + /// + internal class ApplicationModelFactory + { + private readonly IApplicationModelProvider[] _applicationModelProviders; + private readonly IList _conventions; + + public ApplicationModelFactory( + IEnumerable applicationModelProviders, + IOptions options) + { + if (applicationModelProviders == null) + { + throw new ArgumentNullException(nameof(applicationModelProviders)); + } + + if (options == null) + { + throw new ArgumentNullException(nameof(options)); + } + + _applicationModelProviders = applicationModelProviders.OrderBy(p => p.Order).ToArray(); + _conventions = options.Value.Conventions; + } + + public ApplicationModel CreateApplicationModel(IEnumerable controllerTypes) + { + if (controllerTypes == null) + { + throw new ArgumentNullException(nameof(controllerTypes)); + } + + var context = new ApplicationModelProviderContext(controllerTypes); + + for (var i = 0; i < _applicationModelProviders.Length; i++) + { + _applicationModelProviders[i].OnProvidersExecuting(context); + } + + for (var i = _applicationModelProviders.Length - 1; i >= 0; i--) + { + _applicationModelProviders[i].OnProvidersExecuted(context); + } + + ApplicationModelConventions.ApplyConventions(context.Result, _conventions); + + return context.Result; + } + + public static List Flatten( + ApplicationModel application, + Func flattener) + { + var results = new List(); + var errors = new Dictionary>(); + + var actionsByMethod = new Dictionary>(); + var actionsByRouteName = new Dictionary>(StringComparer.OrdinalIgnoreCase); + + var routeTemplateErrors = new List(); + + foreach (var controller in application.Controllers) + { + foreach (var action in controller.Actions) + { + foreach (var selector in ActionAttributeRouteModel.FlattenSelectors(action)) + { + // PostProcess attribute routes so we can observe any errors. + ReplaceAttributeRouteTokens(controller, action, selector, routeTemplateErrors); + + // Add to the data structures we use to find errors. + AddActionToMethodInfoMap(actionsByMethod, action, selector); + AddActionToRouteNameMap(actionsByRouteName, action, selector); + + var result = flattener(application, controller, action, selector); + Debug.Assert(result != null); + + results.Add(result); + } + } + } + + var attributeRoutingConfigurationErrors = new Dictionary(); + foreach (var (method, actions) in actionsByMethod) + { + ValidateActionGroupConfiguration( + method, + actions, + attributeRoutingConfigurationErrors); + } + + if (attributeRoutingConfigurationErrors.Any()) + { + var message = CreateAttributeRoutingAggregateErrorMessage(attributeRoutingConfigurationErrors.Values); + + throw new InvalidOperationException(message); + } + + var namedRoutedErrors = ValidateNamedAttributeRoutedActions(actionsByRouteName); + if (namedRoutedErrors.Any()) + { + var message = CreateAttributeRoutingAggregateErrorMessage(namedRoutedErrors); + throw new InvalidOperationException(message); + } + + if (routeTemplateErrors.Any()) + { + var message = CreateAttributeRoutingAggregateErrorMessage(routeTemplateErrors); + throw new InvalidOperationException(message); + } + + + return results; + } + + private static void ReplaceAttributeRouteTokens( + ControllerModel controller, + ActionModel action, + SelectorModel selector, + List errors) + { + if (selector.AttributeRouteModel == null) + { + return; + } + + try + { + var routeValues = new Dictionary(StringComparer.OrdinalIgnoreCase) + { + { "action", action.ActionName }, + { "controller", controller.ControllerName }, + }; + + foreach (var kvp in action.RouteValues) + { + routeValues.TryAdd(kvp.Key, kvp.Value); + } + + foreach (var kvp in controller.RouteValues) + { + routeValues.TryAdd(kvp.Key, kvp.Value); + } + + action.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out var obj); + var transformer = obj as IOutboundParameterTransformer; + + selector.AttributeRouteModel.Template = AttributeRouteModel.ReplaceTokens( + selector.AttributeRouteModel.Template, + routeValues, + transformer); + + if (selector.AttributeRouteModel.Name != null) + { + selector.AttributeRouteModel.Name = AttributeRouteModel.ReplaceTokens( + selector.AttributeRouteModel.Name, + routeValues, + transformer); + } + } + catch (InvalidOperationException ex) + { + // Routing will throw an InvalidOperationException here if we can't parse/replace tokens + // in the template. + var message = Resources.FormatAttributeRoute_IndividualErrorMessage( + action.DisplayName, + Environment.NewLine, + ex.Message); + + errors.Add(message); + } + } + + private static void AddActionToMethodInfoMap( + Dictionary> actionsByMethod, + ActionModel action, + SelectorModel selector) + { + if (!actionsByMethod.TryGetValue(action.ActionMethod, out var actions)) + { + actions = new List<(ActionModel, SelectorModel)>(); + actionsByMethod.Add(action.ActionMethod, actions); + } + + actions.Add((action, selector)); + } + + private static void AddActionToRouteNameMap( + Dictionary> actionsByRouteName, + ActionModel action, + SelectorModel selector) + { + var routeName = selector.AttributeRouteModel?.Name; + if (selector.AttributeRouteModel?.Name == null) + { + return; + } + + if (!actionsByRouteName.TryGetValue(routeName, out var actions)) + { + actions = new List<(ActionModel, SelectorModel)>(); + actionsByRouteName.Add(routeName, actions); + } + + actions.Add((action, selector)); + } + + private static List AddErrorNumbers(IEnumerable namedRoutedErrors) + { + return namedRoutedErrors + .Select((error, i) => + Resources.FormatAttributeRoute_AggregateErrorMessage_ErrorNumber( + i + 1, + Environment.NewLine, + error)) + .ToList(); + } + + private static List ValidateNamedAttributeRoutedActions( + Dictionary> actionsByRouteName) + { + var namedRouteErrors = new List(); + + foreach (var (routeName, actions) in actionsByRouteName) + { + // We are looking for attribute routed actions that have the same name but + // different route templates. We pick the first template of the group and + // we compare it against the rest of the templates that have that same name + // associated. + // The moment we find one that is different we report the whole group to the + // user in the error message so that he can see the different actions and the + // different templates for a given named attribute route. + var template = actions[0].selector.AttributeRouteModel.Template; + + for (var i = 1; i < actions.Count; i++) + { + var other = actions[i].selector.AttributeRouteModel.Template; + + if (!template.Equals(other, StringComparison.OrdinalIgnoreCase)) + { + var descriptions = actions.Select(a => + { + return Resources.FormatAttributeRoute_DuplicateNames_Item(a.action.DisplayName, a.selector.AttributeRouteModel.Template); + }); + + var message = Resources.FormatAttributeRoute_DuplicateNames(routeName, Environment.NewLine, string.Join(Environment.NewLine, descriptions)); + namedRouteErrors.Add(message); + break; + } + } + } + + return namedRouteErrors; + } + + private static void ValidateActionGroupConfiguration( + MethodInfo method, + List<(ActionModel action, SelectorModel selector)> actions, + IDictionary routingConfigurationErrors) + { + var hasAttributeRoutedActions = false; + var hasConventionallyRoutedActions = false; + + for (var i = 0; i < actions.Count; i++) + { + if (actions[i].selector.AttributeRouteModel == null) + { + hasConventionallyRoutedActions = true; + } + else + { + hasAttributeRoutedActions = true; + } + } + + // 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. + // + // Assuming the controller doesn't specify a route template, this example would not be allowed: + // + // [HttpGet] + // [HttpPost("Foo")] + // public void Foo() { } + if (hasAttributeRoutedActions && hasConventionallyRoutedActions) + { + var message = CreateMixedRoutedActionDescriptorsErrorMessage(method, actions); + routingConfigurationErrors.Add(method, message); + } + } + + private static string CreateMixedRoutedActionDescriptorsErrorMessage( + MethodInfo method, + List<(ActionModel action, SelectorModel selector)> actions) + { + // Text to show as the attribute route template for conventionally routed actions. + var nullTemplate = Resources.AttributeRoute_NullTemplateRepresentation; + + var actionDescriptions = new List(); + for (var i = 0; i < actions.Count; i++) + { + var (action, selector) = actions[i]; + var routeTemplate = selector.AttributeRouteModel?.Template ?? nullTemplate; + + var verbs = selector.ActionConstraints?.OfType().FirstOrDefault()?.HttpMethods; + + var formattedVerbs = string.Empty; + if (verbs != null) + { + formattedVerbs = string.Join(", ", verbs.OrderBy(v => v, StringComparer.OrdinalIgnoreCase)); + } + + var description = Resources.FormatAttributeRoute_MixedAttributeAndConventionallyRoutedActions_ForMethod_Item( + action.DisplayName, + routeTemplate, + formattedVerbs); + + actionDescriptions.Add(description); + } + + // 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' - Route Template: 'Products' - HTTP Verbs: 'PUT' + // Action: 'MyApplication.CustomerController.Index' - Route Template: '(none)' - HTTP Verbs: 'POST' + // + // Use 'AcceptVerbsAttribute' to create a single route that allows multiple HTTP verbs and defines a route, + // or set a route template in all attributes that constrain HTTP verbs. + + var formattedMethodInfo = $"{TypeNameHelper.GetTypeDisplayName(method.ReflectedType)}.{method.Name} ({method.ReflectedType.Assembly.GetName().Name})"; + return Resources.FormatAttributeRoute_MixedAttributeAndConventionallyRoutedActions_ForMethod( + formattedMethodInfo, + 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; + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ControllerActionDescriptorBuilder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ControllerActionDescriptorBuilder.cs index 47cc495378..c53234af3e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ControllerActionDescriptorBuilder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ControllerActionDescriptorBuilder.cs @@ -5,178 +5,63 @@ using System; using System.Collections.Generic; using System.Diagnostics; using System.Linq; -using System.Reflection; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ActionConstraints; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.Routing; -using Microsoft.AspNetCore.Routing; using Resources = Microsoft.AspNetCore.Mvc.Core.Resources; namespace Microsoft.AspNetCore.Mvc.ApplicationModels { /// - /// Creates instances of from . + /// Creates instances of from application model + /// types. /// internal static class ControllerActionDescriptorBuilder { - // This is the default order for attribute routes whose order calculated from - // the controller model is null. - private const int DefaultAttributeRouteOrder = 0; - - /// - /// Creates instances of from . - /// - /// The . - /// The list of . public static IList Build(ApplicationModel application) { - var actions = new List(); - - var methodInfoMap = new MethodToActionMap(); - - var routeTemplateErrors = new List(); - var attributeRoutingConfigurationErrors = new Dictionary(); - - foreach (var controller in application.Controllers) - { - // Only add properties which are explicitly marked to bind. - // The attribute check is required for ModelBinder attribute. - var controllerPropertyDescriptors = controller.ControllerProperties - .Where(p => p.BindingInfo != null) - .Select(CreateParameterDescriptor) - .ToList(); - foreach (var action in controller.Actions) - { - // 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(application, controller, action); - - foreach (var actionDescriptor in actionDescriptors) - { - actionDescriptor.ControllerName = controller.ControllerName; - actionDescriptor.ControllerTypeInfo = controller.ControllerType; - - AddApiExplorerInfo(actionDescriptor, application, controller, action); - AddRouteValues(actionDescriptor, controller, action); - AddProperties(actionDescriptor, action, controller, application); - - actionDescriptor.BoundProperties = controllerPropertyDescriptors; - - if (IsAttributeRoutedAction(actionDescriptor)) - { - // Replaces tokens like [controller]/[action] in the route template with the actual values - // for this action. - ReplaceAttributeRouteTokens(actionDescriptor, routeTemplateErrors); - } - } - - 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); - } - - var attributeRouteInfo = actionDescriptor.AttributeRouteInfo; - if (attributeRouteInfo?.Name != null) - { - // Build a map of attribute route name to action descriptors to ensure that all - // attribute routes with a given name have the same template. - AddActionToNamedGroup(actionsByRouteName, attributeRouteInfo.Name, actionDescriptor); - } - } - - if (attributeRoutingConfigurationErrors.Any()) - { - var message = CreateAttributeRoutingAggregateErrorMessage( - attributeRoutingConfigurationErrors.Values); - - throw new InvalidOperationException(message); - } - - var namedRoutedErrors = ValidateNamedAttributeRoutedActions(actionsByRouteName); - if (namedRoutedErrors.Any()) - { - var message = CreateAttributeRoutingAggregateErrorMessage(namedRoutedErrors); - throw new InvalidOperationException(message); - } - - if (routeTemplateErrors.Any()) - { - var message = CreateAttributeRoutingAggregateErrorMessage(routeTemplateErrors); - throw new InvalidOperationException(message); - } - - return actions; + return ApplicationModelFactory.Flatten(application, CreateActionDescriptor); } - private static IList CreateActionDescriptors( + private static ControllerActionDescriptor CreateActionDescriptor( ApplicationModel application, ControllerModel controller, - ActionModel action) + ActionModel action, + SelectorModel selector) { - var defaultControllerConstraints = Enumerable.Empty(); - var defaultControllerEndpointMetadata = Enumerable.Empty(); - if (controller.Selectors.Count > 0) + var actionDescriptor = new ControllerActionDescriptor { - defaultControllerConstraints = controller.Selectors[0].ActionConstraints - .Where(constraint => !(constraint is IRouteTemplateProvider)); - defaultControllerEndpointMetadata = controller.Selectors[0].EndpointMetadata; - } + ActionName = action.ActionName, + MethodInfo = action.ActionMethod, + }; - var actionDescriptors = new List(); - foreach (var result in ActionAttributeRouteModel.GetAttributeRoutes(action)) - { - var actionSelector = result.actionSelector; - var controllerSelector = result.controllerSelector; + actionDescriptor.ControllerName = controller.ControllerName; + actionDescriptor.ControllerTypeInfo = controller.ControllerType; + AddControllerPropertyDescriptors(actionDescriptor, controller); - var actionDescriptor = CreateActionDescriptor(action, result.route); - actionDescriptors.Add(actionDescriptor); - AddActionFilters(actionDescriptor, action.Filters, controller.Filters, application.Filters); + AddActionConstraints(actionDescriptor, selector); + AddEndpointMetadata(actionDescriptor, selector); + AddAttributeRoute(actionDescriptor, selector); + AddParameterDescriptors(actionDescriptor, action); + AddActionFilters(actionDescriptor, action.Filters, controller.Filters, application.Filters); + AddApiExplorerInfo(actionDescriptor, application, controller, action); + AddRouteValues(actionDescriptor, controller, action); + AddProperties(actionDescriptor, action, controller, application); - var controllerConstraints = defaultControllerConstraints; - - if (controllerSelector?.AttributeRouteModel?.Attribute is IActionConstraintMetadata actionConstraint) - { - // Use the attribute route as a constraint if the controller selector participated in creating this route. - controllerConstraints = controllerConstraints.Concat(new[] { actionConstraint }); - } - - AddActionConstraints(actionDescriptor, actionSelector, controllerConstraints); - - // Metadata for the action is more significant so order it before the controller metadata - var actionDescriptorMetadata = actionSelector.EndpointMetadata.ToList(); - actionDescriptorMetadata.AddRange(defaultControllerEndpointMetadata); - - actionDescriptor.EndpointMetadata = actionDescriptorMetadata; - } - - return actionDescriptors; + return actionDescriptor; } - private static ControllerActionDescriptor CreateActionDescriptor(ActionModel action, AttributeRouteModel routeModel) + private static void AddControllerPropertyDescriptors(ActionDescriptor actionDescriptor, ControllerModel controller) + { + actionDescriptor.BoundProperties = controller.ControllerProperties + .Where(p => p.BindingInfo != null) + .Select(CreateParameterDescriptor) + .ToList(); + } + + private static void AddParameterDescriptors(ActionDescriptor actionDescriptor, ActionModel action) { var parameterDescriptors = new List(); foreach (var parameter in action.Parameters) @@ -185,17 +70,9 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels parameterDescriptors.Add(parameterDescriptor); } - var actionDescriptor = new ControllerActionDescriptor - { - ActionName = action.ActionName, - MethodInfo = action.ActionMethod, - Parameters = parameterDescriptors, - AttributeRouteInfo = CreateAttributeRouteInfo(routeModel), - }; - - return actionDescriptor; + actionDescriptor.Parameters = parameterDescriptors; } - + private static ParameterDescriptor CreateParameterDescriptor(ParameterModel parameterModel) { var parameterDescriptor = new ControllerParameterDescriptor() @@ -244,19 +121,19 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels // ApiExplorer for conventional-routed controllers when this happens. var isVisibleSetOnApplication = application.ApiExplorer?.IsVisible ?? false; - if (isVisibleSetOnActionOrController && !IsAttributeRoutedAction(actionDescriptor)) + if (isVisibleSetOnActionOrController && !IsAttributeRouted(actionDescriptor)) { // ApiExplorer is only supported on attribute routed actions. throw new InvalidOperationException(Resources.FormatApiExplorer_UnsupportedAction( actionDescriptor.DisplayName)); } - else if (isVisibleSetOnApplication && !IsAttributeRoutedAction(actionDescriptor)) + else if (isVisibleSetOnApplication && !IsAttributeRouted(actionDescriptor)) { // This is the case where we're going to be lenient, just ignore it. } else if (isVisible) { - Debug.Assert(IsAttributeRoutedAction(actionDescriptor)); + Debug.Assert(IsAttributeRouted(actionDescriptor)); var apiExplorerActionData = new ApiDescriptionActionData() { @@ -303,43 +180,34 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels .ToList(); } - private static AttributeRouteInfo CreateAttributeRouteInfo(AttributeRouteModel routeModel) + private static void AddActionConstraints(ControllerActionDescriptor actionDescriptor, SelectorModel selectorModel) { - if (routeModel == null) + if (selectorModel.ActionConstraints?.Count > 0) { - return null; + actionDescriptor.ActionConstraints = new List(selectorModel.ActionConstraints); } - - return new AttributeRouteInfo - { - Template = routeModel.Template, - Order = routeModel.Order ?? DefaultAttributeRouteOrder, - Name = routeModel.Name, - SuppressLinkGeneration = routeModel.SuppressLinkGeneration, - SuppressPathMatching = routeModel.SuppressPathMatching, - }; } - private static void AddActionConstraints( - ControllerActionDescriptor actionDescriptor, - SelectorModel selectorModel, - IEnumerable controllerConstraints) + private static void AddEndpointMetadata(ControllerActionDescriptor actionDescriptor, SelectorModel selectorModel) { - var constraints = new List(); - - if (selectorModel.ActionConstraints != null) + if (selectorModel.EndpointMetadata?.Count > 0) { - constraints.AddRange(selectorModel.ActionConstraints); + actionDescriptor.EndpointMetadata = new List(selectorModel.EndpointMetadata); } + } - if (controllerConstraints != null) + private static void AddAttributeRoute(ControllerActionDescriptor actionDescriptor, SelectorModel selectorModel) + { + if (selectorModel.AttributeRouteModel != null) { - constraints.AddRange(controllerConstraints); - } - - if (constraints.Count > 0) - { - actionDescriptor.ActionConstraints = constraints; + actionDescriptor.AttributeRouteInfo = new AttributeRouteInfo + { + Template = selectorModel.AttributeRouteModel.Template, + Order = selectorModel.AttributeRouteModel.Order ?? 0, + Name = selectorModel.AttributeRouteModel.Name, + SuppressLinkGeneration = selectorModel.AttributeRouteModel.SuppressLinkGeneration, + SuppressPathMatching = selectorModel.AttributeRouteModel.SuppressPathMatching, + }; } } @@ -383,254 +251,9 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels } } - private static void ReplaceAttributeRouteTokens( - ControllerActionDescriptor actionDescriptor, - IList routeTemplateErrors) + private static bool IsAttributeRouted(ActionDescriptor actionDescriptor) { - try - { - actionDescriptor.Properties.TryGetValue(typeof(IOutboundParameterTransformer), out var transformer); - var routeTokenTransformer = transformer as IOutboundParameterTransformer; - - actionDescriptor.AttributeRouteInfo.Template = AttributeRouteModel.ReplaceTokens( - actionDescriptor.AttributeRouteInfo.Template, - actionDescriptor.RouteValues, - routeTokenTransformer); - - if (actionDescriptor.AttributeRouteInfo.Name != null) - { - actionDescriptor.AttributeRouteInfo.Name = AttributeRouteModel.ReplaceTokens( - actionDescriptor.AttributeRouteInfo.Name, - actionDescriptor.RouteValues, - routeTokenTransformer); - } - } - catch (InvalidOperationException ex) - { - // Routing will throw an InvalidOperationException here if we can't parse/replace tokens - // in the template. - var message = Resources.FormatAttributeRoute_IndividualErrorMessage( - actionDescriptor.DisplayName, - Environment.NewLine, - ex.Message); - - routeTemplateErrors.Add(message); - } - } - - private static void AddActionToNamedGroup( - IDictionary> actionsByRouteName, - string routeName, - ControllerActionDescriptor actionDescriptor) - { - if (actionsByRouteName.TryGetValue(routeName, out var namedActionGroup)) - { - namedActionGroup.Add(actionDescriptor); - } - else - { - namedActionGroup = new List(); - namedActionGroup.Add(actionDescriptor); - actionsByRouteName.Add(routeName, namedActionGroup); - } - } - - private static bool IsAttributeRoutedAction(ControllerActionDescriptor actionDescriptor) - { - return actionDescriptor.AttributeRouteInfo?.Template != null; - } - - private static IList AddErrorNumbers( - IEnumerable namedRoutedErrors) - { - return namedRoutedErrors - .Select((error, i) => - Resources.FormatAttributeRoute_AggregateErrorMessage_ErrorNumber( - i + 1, - Environment.NewLine, - error)) - .ToList(); - } - - private static IList ValidateNamedAttributeRoutedActions( - IDictionary> actionsGroupedByRouteName) - { - var namedRouteErrors = new List(); - - foreach (var kvp in actionsGroupedByRouteName) - { - // We are looking for attribute routed actions that have the same name but - // different route templates. We pick the first template of the group and - // we compare it against the rest of the templates that have that same name - // associated. - // The moment we find one that is different we report the whole group to the - // user in the error message so that he can see the different actions and the - // different templates for a given named attribute route. - var firstActionDescriptor = kvp.Value[0]; - var firstTemplate = firstActionDescriptor.AttributeRouteInfo.Template; - - for (var i = 1; i < kvp.Value.Count; i++) - { - var otherActionDescriptor = kvp.Value[i]; - var otherActionTemplate = otherActionDescriptor.AttributeRouteInfo.Template; - - if (!firstTemplate.Equals(otherActionTemplate, StringComparison.OrdinalIgnoreCase)) - { - var descriptions = kvp.Value.Select(ad => - Resources.FormatAttributeRoute_DuplicateNames_Item( - ad.DisplayName, - ad.AttributeRouteInfo.Template)); - - var errorDescription = string.Join(Environment.NewLine, descriptions); - var message = Resources.FormatAttributeRoute_DuplicateNames( - kvp.Key, - Environment.NewLine, - errorDescription); - - namedRouteErrors.Add(message); - break; - } - } - } - - return namedRouteErrors; - } - - private static void ValidateActionGroupConfiguration( - IDictionary>> methodMap, - ControllerActionDescriptor actionDescriptor, - IDictionary routingConfigurationErrors) - { - var hasAttributeRoutedActions = false; - var hasConventionallyRoutedActions = false; - - var actionsForMethod = methodMap[actionDescriptor.MethodInfo]; - foreach (var reflectedAction in actionsForMethod) - { - foreach (var action in reflectedAction.Value) - { - if (IsAttributeRoutedAction(action)) - { - hasAttributeRoutedActions = true; - } - else - { - hasConventionallyRoutedActions = true; - } - } - } - - // 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 for example: - // - // [HttpGet] - // [HttpPost("Foo")] - // public void Foo() { } - if (hasAttributeRoutedActions && hasConventionallyRoutedActions) - { - var message = CreateMixedRoutedActionDescriptorsErrorMessage( - actionDescriptor, - actionsForMethod); - - routingConfigurationErrors.Add(actionDescriptor.MethodInfo, message); - } - } - - private static string CreateMixedRoutedActionDescriptorsErrorMessage( - ControllerActionDescriptor actionDescriptor, - IDictionary> actionsForMethod) - { - // Text to show as the attribute route template for conventionally routed actions. - var nullTemplate = Resources.AttributeRoute_NullTemplateRepresentation; - - var actionDescriptions = new List(); - foreach (var action in actionsForMethod.SelectMany(kvp => kvp.Value)) - { - var routeTemplate = action.AttributeRouteInfo?.Template ?? nullTemplate; - - var verbs = action.ActionConstraints?.OfType() - .FirstOrDefault()?.HttpMethods; - - var formattedVerbs = string.Empty; - if (verbs != null) - { - formattedVerbs = string.Join(", ", verbs.OrderBy(v => v, StringComparer.OrdinalIgnoreCase)); - } - - var description = - Resources.FormatAttributeRoute_MixedAttributeAndConventionallyRoutedActions_ForMethod_Item( - action.DisplayName, - routeTemplate, - formattedVerbs); - - actionDescriptions.Add(description); - } - - // 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' - Route Template: 'Products' - HTTP Verbs: 'PUT' - // Action: 'MyApplication.CustomerController.Index' - Route Template: '(none)' - HTTP Verbs: 'POST' - // - // Use 'AcceptVerbsAttribute' to create a single route that allows multiple HTTP verbs and defines a route, - // or set a route template in all attributes that constrain HTTP verbs. - return - Resources.FormatAttributeRoute_MixedAttributeAndConventionallyRoutedActions_ForMethod( - actionDescriptor.DisplayName, - 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; - } - - // 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( - ActionModel action, - IList actionDescriptors) - { - if (TryGetValue(action.ActionMethod, out var actionsForMethod)) - { - actionsForMethod.Add(action, actionDescriptors); - } - else - { - var reflectedActionMap = - new Dictionary>(); - reflectedActionMap.Add(action, actionDescriptors); - Add(action.ActionMethod, reflectedActionMap); - } - } + return actionDescriptor.AttributeRouteInfo != null; } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ControllerActionDescriptorProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ControllerActionDescriptorProvider.cs index 882139a62d..e82b715697 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ControllerActionDescriptorProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ControllerActionDescriptorProvider.cs @@ -3,44 +3,34 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Reflection; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ApplicationParts; using Microsoft.AspNetCore.Mvc.Controllers; -using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Mvc.ApplicationModels { internal class ControllerActionDescriptorProvider : IActionDescriptorProvider { private readonly ApplicationPartManager _partManager; - private readonly IApplicationModelProvider[] _applicationModelProviders; - private readonly IEnumerable _conventions; + private readonly ApplicationModelFactory _applicationModelFactory; public ControllerActionDescriptorProvider( ApplicationPartManager partManager, - IEnumerable applicationModelProviders, - IOptions optionsAccessor) + ApplicationModelFactory applicationModelFactory) { if (partManager == null) { throw new ArgumentNullException(nameof(partManager)); } - if (applicationModelProviders == null) + if (applicationModelFactory == null) { - throw new ArgumentNullException(nameof(applicationModelProviders)); - } - - if (optionsAccessor == null) - { - throw new ArgumentNullException(nameof(optionsAccessor)); + throw new ArgumentNullException(nameof(applicationModelFactory)); } _partManager = partManager; - _applicationModelProviders = applicationModelProviders.OrderBy(p => p.Order).ToArray(); - _conventions = optionsAccessor.Value.Conventions; + _applicationModelFactory = applicationModelFactory; } public int Order => -1000; @@ -95,28 +85,10 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels } internal IEnumerable GetDescriptors() - { - var applicationModel = BuildModel(); - ApplicationModelConventions.ApplyConventions(applicationModel, _conventions); - return ControllerActionDescriptorBuilder.Build(applicationModel); - } - - internal ApplicationModel BuildModel() { var controllerTypes = GetControllerTypes(); - var context = new ApplicationModelProviderContext(controllerTypes); - - for (var i = 0; i < _applicationModelProviders.Length; i++) - { - _applicationModelProviders[i].OnProvidersExecuting(context); - } - - for (var i = _applicationModelProviders.Length - 1; i >= 0; i--) - { - _applicationModelProviders[i].OnProvidersExecuted(context); - } - - return context.Result; + var application = _applicationModelFactory.CreateApplicationModel(controllerTypes); + return ControllerActionDescriptorBuilder.Build(application); } private IEnumerable GetControllerTypes() diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/InferParameterBindingInfoConvention.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/InferParameterBindingInfoConvention.cs index edfd9a5e0b..0148be60b9 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/InferParameterBindingInfoConvention.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/InferParameterBindingInfoConvention.cs @@ -98,14 +98,14 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels private bool ParameterExistsInAnyRoute(ActionModel action, string parameterName) { - foreach (var (route, _, _) in ActionAttributeRouteModel.GetAttributeRoutes(action)) + foreach (var selector in ActionAttributeRouteModel.FlattenSelectors(action)) { - if (route == null) + if (selector.AttributeRouteModel == null) { continue; } - var parsedTemplate = TemplateParser.Parse(route.Template); + var parsedTemplate = TemplateParser.Parse(selector.AttributeRouteModel.Template); if (parsedTemplate.GetParameter(parameterName) != null) { return true; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index 9e36a681de..8ea71b7178 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -158,7 +158,7 @@ namespace Microsoft.Extensions.DependencyInjection // Action Discovery // // These are consumed only when creating action descriptors, then they can be deallocated - + services.TryAddSingleton(); services.TryAddEnumerable( ServiceDescriptor.Transient()); services.TryAddEnumerable( diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/ControllerActionDescriptorProviderTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/ControllerActionDescriptorProviderTests.cs index 2942431599..7540ed8a8b 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/ControllerActionDescriptorProviderTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/ControllerActionDescriptorProviderTests.cs @@ -401,102 +401,6 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels c.Value == "OnlyPost"); } - [Fact] - public void BuildModel_IncludesGlobalFilters() - { - // Arrange - var filter = new MyFilterAttribute(1); - var provider = GetProvider(typeof(PersonController).GetTypeInfo(), new IFilterMetadata[] - { - filter, - }); - - // Act - var model = provider.BuildModel(); - - // Assert - var filters = model.Filters; - Assert.Same(filter, Assert.Single(filters)); - } - - [Fact] - public void BuildModel_CreatesControllerModels_ForAllControllers() - { - // Arrange - var provider = GetProvider( - typeof(ConventionallyRoutedController).GetTypeInfo(), - typeof(AttributeRoutedController).GetTypeInfo(), - typeof(EmptyController).GetTypeInfo(), - typeof(NonActionAttributeController).GetTypeInfo()); - - // Act - var model = provider.BuildModel(); - - // Assert - Assert.NotNull(model); - Assert.Equal(4, model.Controllers.Count); - - var conventional = Assert.Single(model.Controllers, - c => c.ControllerName == "ConventionallyRouted"); - Assert.Empty(conventional.Selectors.Where(sm => sm.AttributeRouteModel != null)); - Assert.Single(conventional.Actions); - - var attributeRouted = Assert.Single(model.Controllers, - c => c.ControllerName == "AttributeRouted"); - Assert.Single(attributeRouted.Actions); - Assert.Single(attributeRouted.Selectors.Where(sm => sm.AttributeRouteModel != null)); - - var empty = Assert.Single(model.Controllers, - c => c.ControllerName == "Empty"); - Assert.Empty(empty.Actions); - - var nonAction = Assert.Single(model.Controllers, - c => c.ControllerName == "NonActionAttribute"); - Assert.Empty(nonAction.Actions); - } - - [Fact] - public void BuildModel_CreatesControllerActionDescriptors_ForValidActions() - { - // Arrange - var provider = GetProvider( - typeof(PersonController).GetTypeInfo()); - - // Act - var model = provider.BuildModel(); - - // Assert - var controller = Assert.Single(model.Controllers); - - Assert.Equal(2, controller.Actions.Count); - - var getPerson = Assert.Single(controller.Actions, a => a.ActionName == "GetPerson"); - Assert.Empty(getPerson.Selectors[0].ActionConstraints.OfType()); - - var showPeople = Assert.Single(controller.Actions, a => a.ActionName == "ShowPeople"); - Assert.Empty(showPeople.Selectors[0].ActionConstraints.OfType()); - } - - [Fact] - public void AttributeRouting_TokenReplacement_IsAfterReflectedModel() - { - // Arrange - var provider = GetProvider(typeof(TokenReplacementController).GetTypeInfo()); - - // Act - var model = provider.BuildModel(); - - // Assert - var controller = Assert.Single(model.Controllers); - - var selectorModel = Assert.Single(controller.Selectors.Where(sm => sm.AttributeRouteModel != null)); - Assert.Equal("api/Token/[key]/[controller]", selectorModel.AttributeRouteModel.Template); - - var action = Assert.Single(controller.Actions); - var actionSelectorModel = Assert.Single(action.Selectors.Where(sm => sm.AttributeRouteModel != null)); - Assert.Equal("stub/[action]", actionSelectorModel.AttributeRouteModel.Template); - } - [Fact] public void AttributeRouting_TokenReplacement_InActionDescriptor() { @@ -791,13 +695,14 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels // Arrange var controllerTypeInfo = typeof(UserController).GetTypeInfo(); var manager = GetApplicationManager(new[] { controllerTypeInfo }); + var options = Options.Create(new MvcOptions()); - options.Value.Conventions.Add(new TestRoutingConvention()); + var modelProvider = new DefaultApplicationModelProvider(options, new EmptyModelMetadataProvider()); var provider = new ControllerActionDescriptorProvider( manager, - new[] { modelProvider }, - options); + new ApplicationModelFactory(new[] { modelProvider }, options)); + var assemblyName = controllerTypeInfo.Assembly.GetName().Name; var expectedMessage = "The following errors occurred with attribute routing information:" @@ -813,7 +718,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels "HTTP Verbs: ''" + Environment.NewLine + $"Action: '{controllerTypeInfo.FullName}.GetUser ({assemblyName})' " + - "- Route Template: 'Microsoft/AspNetCore/Mvc/ApplicationModels/User/GetUser/{id?}' - " + + "- Route Template: '!!!' - " + "HTTP Verbs: ''" + Environment.NewLine + Environment.NewLine + "Use 'AcceptVerbsAttribute' to create a single route that allows multiple HTTP verbs and defines a " + @@ -1288,38 +1193,6 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels Assert.Equal(4, sequence); } - [Fact] - public void BuildModel_SplitsConstraintsBasedOnRoute() - { - // Arrange - var provider = GetProvider(typeof(MultipleRouteProviderOnActionController).GetTypeInfo()); - - // Act - var model = provider.BuildModel(); - - // Assert - var controllerModel = Assert.Single(model.Controllers); - var actionModel = Assert.Single(controllerModel.Actions); - Assert.Equal(3, actionModel.Attributes.Count); - Assert.Equal(2, actionModel.Attributes.OfType().Count()); - Assert.Single(actionModel.Attributes.OfType()); - Assert.Equal(2, actionModel.Selectors.Count); - - var selectorModel = Assert.Single( - actionModel.Selectors.Where(sm => sm.AttributeRouteModel?.Template == "R1")); - - Assert.Equal(2, selectorModel.ActionConstraints.Count); - Assert.Single(selectorModel.ActionConstraints.OfType()); - Assert.Single(selectorModel.ActionConstraints.OfType()); - - selectorModel = Assert.Single( - actionModel.Selectors.Where(sm => sm.AttributeRouteModel?.Template == "R2")); - - Assert.Equal(2, selectorModel.ActionConstraints.Count); - Assert.Single(selectorModel.ActionConstraints.OfType()); - Assert.Single(selectorModel.ActionConstraints.OfType()); - } - [Fact] public void GetDescriptors_SplitsConstraintsBasedOnRoute() { @@ -1485,8 +1358,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels var provider = new ControllerActionDescriptorProvider( manager, - new[] { modelProvider }, - options); + new ApplicationModelFactory(new[] { modelProvider }, options)); return provider; } @@ -1501,8 +1373,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels var provider = new ControllerActionDescriptorProvider( manager, - new[] { modelProvider }, - options); + new ApplicationModelFactory(new[] { modelProvider }, options)); return provider; } @@ -1520,8 +1391,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels var provider = new ControllerActionDescriptorProvider( manager, - new[] { modelProvider }, - options); + new ApplicationModelFactory(new[] { modelProvider }, options)); return provider; } @@ -2095,31 +1965,23 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels } } - private class TestRoutingConvention : IApplicationModelConvention + private class MixedRoutingConventionAttribute : Attribute, IActionModelConvention { - public void Apply(ApplicationModel application) + public void Apply(ActionModel action) { - foreach (var controller in application.Controllers) + action.Selectors.Add(new SelectorModel() { - var hasAttributeRouteModels = controller.Selectors - .Any(selector => selector.AttributeRouteModel != null); - if (!hasAttributeRouteModels) + AttributeRouteModel = new AttributeRouteModel() { - var template = controller.ControllerType.Namespace.Replace('.', '/') - + "/[controller]/[action]/{id?}"; - var attributeRouteModel = new AttributeRouteModel() - { - Template = template - }; - - controller.Selectors.Add(new SelectorModel { AttributeRouteModel = attributeRouteModel }); + Template = "/!!!", } - } + }); } } private class UserController : ControllerBase { + [MixedRoutingConvention] public string GetUser(int id) { return string.Format("User {0} retrieved successfully", id); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/DefaultApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/DefaultApplicationModelProviderTest.cs index 8d89b668bd..4e303d91b1 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/DefaultApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/DefaultApplicationModelProviderTest.cs @@ -22,18 +22,43 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels private readonly TestApplicationModelProvider Provider = new TestApplicationModelProvider(); [Fact] - public void CreateControllerModel_DerivedFromControllerClass_HasFilter() + public void OnProvidersExecuting_AddsGlobalFilters() + { + // Arrange + var options = new MvcOptions() + { + Filters = + { + new MyFilterAttribute(), + }, + }; + + var builder = new TestApplicationModelProvider(options, TestModelMetadataProvider.CreateDefaultProvider()); + var context = new ApplicationModelProviderContext(Array.Empty()); + + // Act + builder.OnProvidersExecuting(context); + + // Assert + Assert.Equal(options.Filters.ToArray(), context.Result.Filters); + } + + [Fact] + public void OnProvidersExecuting_IncludesAllControllers() { // Arrange var builder = new TestApplicationModelProvider(); - var typeInfo = typeof(StoreController).GetTypeInfo(); + + var context = new ApplicationModelProviderContext(new[] { typeof(ModelBinderController).GetTypeInfo(), typeof(ConventionallyRoutedController).GetTypeInfo() }); // Act - var model = builder.CreateControllerModel(typeInfo); + builder.OnProvidersExecuting(context); // Assert - var filter = Assert.Single(model.Filters); - Assert.IsType(filter); + Assert.Collection( + context.Result.Controllers.OrderBy(c => c.ControllerType.Name), + c => Assert.Equal(typeof(ConventionallyRoutedController).GetTypeInfo(), c.ControllerType), + c => Assert.Equal(typeof(ModelBinderController).GetTypeInfo(), c.ControllerType)); } [Fact] @@ -313,6 +338,21 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels }); } + [Fact] + public void CreateControllerModel_DerivedFromControllerClass_HasFilter() + { + // Arrange + var builder = new TestApplicationModelProvider(); + var typeInfo = typeof(StoreController).GetTypeInfo(); + + // Act + var model = builder.CreateControllerModel(typeInfo); + + // Assert + var filter = Assert.Single(model.Filters); + Assert.IsType(filter); + } + // This class has a filter attribute, but doesn't implement any filter interfaces, // so ControllerFilter is not present. [Fact] @@ -1098,6 +1138,38 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels Assert.Equal(new string[] { "GET" }, methodConstraint.HttpMethods); } + [Fact] + public void CreateActionModel_SplitsConstraintsBasedOnRoute() + { + // Arrange + var builder = new TestApplicationModelProvider(); + var typeInfo = typeof(MultipleRouteProviderOnActionController).GetTypeInfo(); + var methodInfo = typeInfo.GetMethod(nameof(MultipleRouteProviderOnActionController.Edit)); + + // Act + var actionModel = builder.CreateActionModel(typeInfo, methodInfo); + + // Assert + Assert.Equal(3, actionModel.Attributes.Count); + Assert.Equal(2, actionModel.Attributes.OfType().Count()); + Assert.Single(actionModel.Attributes.OfType()); + Assert.Equal(2, actionModel.Selectors.Count); + + var selectorModel = Assert.Single( + actionModel.Selectors.Where(sm => sm.AttributeRouteModel?.Template == "R1")); + + Assert.Equal(2, selectorModel.ActionConstraints.Count); + Assert.Single(selectorModel.ActionConstraints.OfType()); + Assert.Single(selectorModel.ActionConstraints.OfType()); + + selectorModel = Assert.Single( + actionModel.Selectors.Where(sm => sm.AttributeRouteModel?.Template == "R2")); + + Assert.Equal(2, selectorModel.ActionConstraints.Count); + Assert.Single(selectorModel.ActionConstraints.OfType()); + Assert.Single(selectorModel.ActionConstraints.OfType()); + } + [Fact] public void CreateActionModel_InheritedAttributeRoutes() { @@ -1719,6 +1791,34 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels } } + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, Inherited = true, AllowMultiple = true)] + private class RouteAndConstraintAttribute : Attribute, IActionConstraintMetadata, IRouteTemplateProvider + { + public RouteAndConstraintAttribute(string template) + { + Template = template; + } + + public string Name { get; set; } + + public int? Order { get; set; } + + public string Template { get; private set; } + } + + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Method, Inherited = true, AllowMultiple = true)] + private class ConstraintAttribute : Attribute, IActionConstraintMetadata + { + } + + private class MultipleRouteProviderOnActionController + { + [Constraint] + [RouteAndConstraint("R1")] + [RouteAndConstraint("R2")] + public void Edit() { } + } + private class TestApplicationModelProvider : DefaultApplicationModelProvider { public TestApplicationModelProvider() diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ActionSelectorTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ActionSelectorTest.cs index 47da34e7ef..8c508cb3a6 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ActionSelectorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Infrastructure/ActionSelectorTest.cs @@ -1009,8 +1009,7 @@ namespace Microsoft.AspNetCore.Mvc.Infrastructure var provider = new ControllerActionDescriptorProvider( manager, - new[] { modelProvider }, - options); + new ApplicationModelFactory(new[] { modelProvider }, options)); return provider; }