diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs index 1cc0535c6b..3d68fd45bb 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs @@ -325,11 +325,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public Type ElementType { get; private set; } /// - /// Gets a value indicating whether is a simple type. + /// Gets a value indicating whether is a complex type. /// /// - /// A simple type is defined as a which has a - /// that can convert from . + /// A complex type is defined as a which has a + /// that can convert from . /// public bool IsComplexType { get; private set; } diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadataProvider.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadataProvider.cs index f32bcf00e2..1b4f01cd6d 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadataProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadataProvider.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.Reflection; -using Microsoft.AspNetCore.Mvc.Abstractions; namespace Microsoft.AspNetCore.Mvc.ModelBinding { @@ -31,7 +30,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// Supplies metadata describing a parameter. /// /// The . - /// A instance describing properties of the . + /// A instance describing the . public abstract ModelMetadata GetMetadataForParameter(ParameterInfo parameter); } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs index 2a827c0141..ec1ec4a678 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs @@ -28,10 +28,21 @@ namespace Microsoft.AspNetCore.Mvc } /// - /// Disables the filter that returns an when - /// is invalid. - /// . + /// Gets or sets a value that determines if the filter that returns an when + /// is invalid is suppressed. . /// - public bool EnableModelStateInvalidFilter { get; set; } = true; + public bool SuppressModelStateInvalidFilter { get; set; } + + /// + /// Gets or sets a value that determines if model binding sources are inferred for action parameters on controllers annotated + /// with is suppressed. + /// + /// When enabled, the following sources are inferred: + /// Parameters that appear as route values, are assumed to be bound from the path (). + /// Parameters that are complex () are assumed to be bound from the body (). + /// All other parameters are assumed to be bound from the query. + /// + /// + public bool SuppressInferBindingSourcesForParameters { get; set; } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionAttributeRouteModel.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionAttributeRouteModel.cs new file mode 100644 index 0000000000..45fe4b2578 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ActionAttributeRouteModel.cs @@ -0,0 +1,60 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using System.Linq; +using Microsoft.AspNetCore.Mvc.ApplicationModels; + +namespace Microsoft.AspNetCore.Mvc.Internal +{ + public static class ActionAttributeRouteModel + { + public static IEnumerable<(AttributeRouteModel route, SelectorModel actionSelector, SelectorModel controllerSelector)> GetAttributeRoutes(ActionModel actionModel) + { + var controllerAttributeRoutes = actionModel.Controller.Selectors + .Where(sm => sm.AttributeRouteModel != null) + .Select(sm => sm.AttributeRouteModel) + .ToList(); + + foreach (var actionSelectorModel in actionModel.Selectors) + { + var actionRouteModel = actionSelectorModel.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) + { + var route = AttributeRouteModel.CombineAttributeRouteModel( + left: null, + right: actionRouteModel); + + yield return (route, actionSelectorModel, null); + } + else if (controllerAttributeRoutes.Count > 0) + { + for (var i = 0; i < actionModel.Controller.Selectors.Count; i++) + { + // We're using the attribute routes from the controller + var controllerSelector = actionModel.Controller.Selectors[i]; + + var route = AttributeRouteModel.CombineAttributeRouteModel( + controllerSelector.AttributeRouteModel, + actionRouteModel); + + yield return (route, actionSelectorModel, controllerSelector); + } + } + + else + { + var route = AttributeRouteModel.CombineAttributeRouteModel( + left: null, + right: actionRouteModel); + + yield return (route, actionSelectorModel, null); + } + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs index 968eeef334..bb44d1165e 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorApplicationModelProvider.cs @@ -7,6 +7,8 @@ using System.Linq; using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Routing.Template; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -15,12 +17,20 @@ namespace Microsoft.AspNetCore.Mvc.Internal public class ApiBehaviorApplicationModelProvider : IApplicationModelProvider { private readonly ApiBehaviorOptions _apiBehaviorOptions; + private readonly IModelMetadataProvider _modelMetadataProvider; private readonly ModelStateInvalidFilter _modelStateInvalidFilter; + private readonly ILogger _logger; - public ApiBehaviorApplicationModelProvider(IOptions apiBehaviorOptions, ILoggerFactory loggerFactory) + public ApiBehaviorApplicationModelProvider( + IOptions apiBehaviorOptions, + IModelMetadataProvider modelMetadataProvider, + ILoggerFactory loggerFactory) { _apiBehaviorOptions = apiBehaviorOptions.Value; - if (_apiBehaviorOptions.EnableModelStateInvalidFilter && _apiBehaviorOptions.InvalidModelStateResponseFactory == null) + _modelMetadataProvider = modelMetadataProvider; + _logger = loggerFactory.CreateLogger(); + + if (!_apiBehaviorOptions.SuppressModelStateInvalidFilter && _apiBehaviorOptions.InvalidModelStateResponseFactory == null) { throw new ArgumentException(Resources.FormatPropertyOfTypeCannotBeNull( typeof(ApiBehaviorOptions), @@ -51,22 +61,140 @@ namespace Microsoft.AspNetCore.Mvc.Internal foreach (var actionModel in controllerModel.Actions) { - if (isApiController || actionModel.Attributes.OfType().Any()) + if (!isApiController && !actionModel.Attributes.OfType().Any()) { - if (!controllerHasSelectorModel && !actionModel.Selectors.Any(s => s.AttributeRouteModel != null)) - { - // Require attribute routing with controllers annotated with ApiControllerAttribute - throw new InvalidOperationException(Resources.FormatApiController_AttributeRouteRequired(nameof(ApiControllerAttribute))); - } - - if (_apiBehaviorOptions.EnableModelStateInvalidFilter) - { - Debug.Assert(_apiBehaviorOptions.InvalidModelStateResponseFactory != null); - actionModel.Filters.Add(_modelStateInvalidFilter); - } + continue; } + + EnsureActionIsAttributeRouted(controllerHasSelectorModel, actionModel); + + AddInvalidModelStateFilter(actionModel); + + InferParameterBindingSources(actionModel); } } } + + private static void EnsureActionIsAttributeRouted(bool controllerHasSelectorModel, ActionModel actionModel) + { + if (!controllerHasSelectorModel && !actionModel.Selectors.Any(s => s.AttributeRouteModel != null)) + { + // Require attribute routing with controllers annotated with ApiControllerAttribute + throw new InvalidOperationException(Resources.FormatApiController_AttributeRouteRequired(nameof(ApiControllerAttribute))); + } + } + + private void AddInvalidModelStateFilter(ActionModel actionModel) + { + if (_apiBehaviorOptions.SuppressModelStateInvalidFilter) + { + return; + } + + Debug.Assert(_apiBehaviorOptions.InvalidModelStateResponseFactory != null); + actionModel.Filters.Add(_modelStateInvalidFilter); + } + + private void InferParameterBindingSources(ActionModel actionModel) + { + if (_modelMetadataProvider == null || _apiBehaviorOptions.SuppressInferBindingSourcesForParameters) + { + return; + } + var inferredBindingSources = new BindingSource[actionModel.Parameters.Count]; + var foundFromBodyParameter = false; + + for (var i = 0; i < inferredBindingSources.Length; i++) + { + var parameter = actionModel.Parameters[i]; + var bindingSource = parameter.BindingInfo?.BindingSource; + if (bindingSource == null) + { + bindingSource = InferBindingSourceForParameter(parameter); + } + + if (bindingSource == BindingSource.Body) + { + if (foundFromBodyParameter) + { + // More than one parameter is inferred as FromBody. Log a warning and skip this action. + _logger.UnableToInferBindingSource(actionModel); + } + else + { + foundFromBodyParameter = true; + } + } + + inferredBindingSources[i] = bindingSource; + } + + for (var i = 0; i < inferredBindingSources.Length; i++) + { + var bindingSource = inferredBindingSources[i]; + if (bindingSource != null) + { + actionModel.Parameters[i].BindingInfo = new BindingInfo + { + BindingSource = bindingSource, + }; + } + } + } + + // Internal for unit testing. + internal BindingSource InferBindingSourceForParameter(ParameterModel parameter) + { + if (ParameterExistsInAllRoutes(parameter.Action, parameter.ParameterName)) + { + return BindingSource.Path; + } + else + { + ModelMetadata parameterMetadata; + if (_modelMetadataProvider is ModelMetadataProvider modelMetadataProvider) + { + parameterMetadata = modelMetadataProvider.GetMetadataForParameter(parameter.ParameterInfo); + } + else + { + parameterMetadata = _modelMetadataProvider.GetMetadataForType(parameter.ParameterInfo.ParameterType); + } + + if (parameterMetadata != null) + { + var bindingSource = parameterMetadata.IsComplexType ? + BindingSource.Body : + BindingSource.Query; + + return bindingSource; + } + } + + return null; + } + + private bool ParameterExistsInAllRoutes(ActionModel actionModel, string parameterName) + { + var parameterExistsInSomeRoute = false; + foreach (var (route, _, _) in ActionAttributeRouteModel.GetAttributeRoutes(actionModel)) + { + if (route == null) + { + continue; + } + + var parsedTemplate = TemplateParser.Parse(route.Template); + if (parsedTemplate.GetParameter(parameterName) == null) + { + return false; + } + + // Ensure at least one route exists. + parameterExistsInSomeRoute = true; + } + + return parameterExistsInSomeRoute; + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs index 572f3aa441..18ae818293 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerActionDescriptorBuilder.cs @@ -137,96 +137,38 @@ namespace Microsoft.AspNetCore.Mvc.Internal ControllerModel controller, ActionModel action) { - var controllerAttributeRoutes = controller.Selectors - .Where(sm => sm.AttributeRouteModel != null) - .Select(sm => sm.AttributeRouteModel) - .ToList(); + var defaultControllerConstraints = Enumerable.Empty(); + if (controller.Selectors.Count > 0) + { + defaultControllerConstraints = controller.Selectors[0].ActionConstraints + .Where(constraint => !(constraint is IRouteTemplateProvider)); + } var actionDescriptors = new List(); - - foreach (var actionSelectorModel in action.Selectors) + foreach (var result in ActionAttributeRouteModel.GetAttributeRoutes(action)) { - var actionAttributeRoute = actionSelectorModel.AttributeRouteModel; + var actionSelector = result.actionSelector; + var controllerSelector = result.controllerSelector; - // 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 (actionAttributeRoute != null && actionAttributeRoute.IsAbsoluteTemplate) + var actionDescriptor = CreateActionDescriptor(action, result.route); + actionDescriptors.Add(actionDescriptor); + AddActionFilters(actionDescriptor, action.Filters, controller.Filters, application.Filters); + + var controllerConstraints = defaultControllerConstraints; + + if (controllerSelector?.AttributeRouteModel?.Attribute is IActionConstraintMetadata actionConstraint) { - // We're overriding the attribute routes on the controller, so filter out any metadata - // from controller level routes. - var actionDescriptor = CreateActionDescriptor( - action, - actionAttributeRoute, - controllerAttributeRoute: null); - - actionDescriptors.Add(actionDescriptor); - - AddActionFilters(actionDescriptor, action.Filters, controller.Filters, application.Filters); - - // If we're using an attribute route on the controller, then filter out any additional - // metadata from the 'other' attribute routes. - IList controllerConstraints = null; - if (controller.Selectors.Count > 0) - { - controllerConstraints = controller.Selectors[0].ActionConstraints - .Where(constraint => !(constraint is IRouteTemplateProvider)).ToList(); - } - - AddActionConstraints(actionDescriptor, actionSelectorModel, controllerConstraints); + // Use the attribute route as a constraint if the controller selector participated in creating this route. + controllerConstraints = controllerConstraints.Concat(new[] { actionConstraint }); } - else if (controllerAttributeRoutes.Count > 0) - { - // We're using the attribute routes from the controller - foreach (var controllerSelectorModel in controller.Selectors) - { - var controllerAttributeRoute = controllerSelectorModel.AttributeRouteModel; - var actionDescriptor = CreateActionDescriptor( - action, - actionAttributeRoute, - controllerAttributeRoute); - - actionDescriptors.Add(actionDescriptor); - - AddActionFilters(actionDescriptor, action.Filters, controller.Filters, application.Filters); - - // If we're using an attribute route on the controller, then filter out any additional - // metadata from the 'other' attribute routes. - var controllerConstraints = controllerSelectorModel.ActionConstraints - .Where(c => c == controllerAttributeRoute?.Attribute || !(c is IRouteTemplateProvider)); - AddActionConstraints(actionDescriptor, actionSelectorModel, controllerConstraints); - } - } - else - { - // No attribute routes on the controller - var actionDescriptor = CreateActionDescriptor( - action, - actionAttributeRoute, - controllerAttributeRoute: null); - actionDescriptors.Add(actionDescriptor); - - IList controllerConstraints = null; - if (controller.Selectors.Count > 0) - { - controllerConstraints = controller.Selectors[0].ActionConstraints; - } - - // If there's no attribute route on the controller, then we use all of the filters/constraints - // on the controller regardless. - AddActionFilters(actionDescriptor, action.Filters, controller.Filters, application.Filters); - AddActionConstraints(actionDescriptor, actionSelectorModel, controllerConstraints); - } + AddActionConstraints(actionDescriptor, actionSelector, controllerConstraints); } return actionDescriptors; } - private static ControllerActionDescriptor CreateActionDescriptor( - ActionModel action, - AttributeRouteModel actionAttributeRoute, - AttributeRouteModel controllerAttributeRoute) + private static ControllerActionDescriptor CreateActionDescriptor(ActionModel action, AttributeRouteModel routeModel) { var parameterDescriptors = new List(); foreach (var parameter in action.Parameters) @@ -235,12 +177,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameterDescriptors.Add(parameterDescriptor); } - var actionDescriptor = new ControllerActionDescriptor() + var actionDescriptor = new ControllerActionDescriptor { ActionName = action.ActionName, MethodInfo = action.ActionMethod, Parameters = parameterDescriptors, - AttributeRouteInfo = CreateAttributeRouteInfo(actionAttributeRoute, controllerAttributeRoute) + AttributeRouteInfo = CreateAttributeRouteInfo(routeModel), }; return actionDescriptor; @@ -353,27 +295,21 @@ namespace Microsoft.AspNetCore.Mvc.Internal .ToList(); } - private static AttributeRouteInfo CreateAttributeRouteInfo( - AttributeRouteModel action, - AttributeRouteModel controller) + private static AttributeRouteInfo CreateAttributeRouteInfo(AttributeRouteModel routeModel) { - var combinedRoute = AttributeRouteModel.CombineAttributeRouteModel(controller, action); - - if (combinedRoute == null) + if (routeModel == null) { return null; } - else + + return new AttributeRouteInfo { - return new AttributeRouteInfo - { - Template = combinedRoute.Template, - Order = combinedRoute.Order ?? DefaultAttributeRouteOrder, - Name = combinedRoute.Name, - SuppressLinkGeneration = combinedRoute.SuppressLinkGeneration, - SuppressPathMatching = combinedRoute.SuppressPathMatching, - }; - } + Template = routeModel.Template, + Order = routeModel.Order ?? DefaultAttributeRouteOrder, + Name = routeModel.Name, + SuppressLinkGeneration = routeModel.SuppressLinkGeneration, + SuppressPathMatching = routeModel.SuppressPathMatching, + }; } private static void AddActionConstraints( @@ -496,10 +432,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal { return namedRoutedErrors .Select((error, i) => - Resources.FormatAttributeRoute_AggregateErrorMessage_ErrorNumber( - i + 1, - Environment.NewLine, - error)) + Resources.FormatAttributeRoute_AggregateErrorMessage_ErrorNumber( + i + 1, + Environment.NewLine, + error)) .ToList(); } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs index c691df1ea9..3f49136162 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreLoggerExtensions.cs @@ -6,9 +6,11 @@ using System.Collections; using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using System.Reflection; using System.Security.Claims; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ActionConstraints; +using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Formatters.Internal; @@ -82,6 +84,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal private static readonly Action _modelStateInvalidFilterExecuting; + private static readonly Action _inferredParameterSource; + private static readonly Action _unableToInferParameterSources; + static MvcCoreLoggerExtensions() { _actionExecuting = LoggerMessage.Define( @@ -289,6 +294,15 @@ namespace Microsoft.AspNetCore.Mvc.Internal 1, "The request has model state errors, returning an error response."); + _inferredParameterSource = LoggerMessage.Define( + LogLevel.Debug, + 1, + "Inferred binding source for '{ParameterName}` on `{ActionName}` as {BindingSource}."); + + _unableToInferParameterSources = LoggerMessage.Define( + LogLevel.Warning, + 2, + "Unable to unambiguously infer binding sources for parameters on '{ActionName}'. More than one parameter may be inferred to bound from body."); } public static IDisposable ActionScope(this ILogger logger, ActionDescriptor action) @@ -601,6 +615,27 @@ namespace Microsoft.AspNetCore.Mvc.Internal public static void ModelStateInvalidFilterExecuting(this ILogger logger) => _modelStateInvalidFilterExecuting(logger, null); + public static void InferredParameterBindingSource( + this ILogger logger, + ParameterModel parameterModel, + BindingSource bindingSource) + { + if (logger.IsEnabled(LogLevel.Debug)) + { + _inferredParameterSource(logger, parameterModel.Action.ActionMethod, parameterModel.ParameterName, bindingSource.DisplayName, null); + } + } + + public static void UnableToInferBindingSource( + this ILogger logger, + ActionModel actionModel) + { + if (logger.IsEnabled(LogLevel.Warning)) + { + _unableToInferParameterSources(logger, actionModel.ActionMethod, null); + } + } + private class ActionLogScope : IReadOnlyList> { private readonly ActionDescriptor _action; diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs index 0f10c182c6..5aa1ff119b 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ApiBehaviorApplicationModelProviderTest.cs @@ -2,11 +2,14 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.ComponentModel; using System.Linq; using System.Reflection; using Microsoft.AspNetCore.Mvc.ApplicationModels; using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; using Xunit; namespace Microsoft.AspNetCore.Mvc.Internal @@ -18,12 +21,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal { // Arrange var context = GetContext(typeof(TestApiController)); - var options = new TestOptionsManager(new ApiBehaviorOptions - { - InvalidModelStateResponseFactory = _ => null, - }); - - var provider = new ApiBehaviorApplicationModelProvider(options, NullLoggerFactory.Instance); + var provider = GetProvider(); // Act provider.OnProvidersExecuting(context); @@ -40,10 +38,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = GetContext(typeof(TestApiController)); var options = new TestOptionsManager(new ApiBehaviorOptions { - EnableModelStateInvalidFilter = false, + SuppressModelStateInvalidFilter = true, }); - var provider = new ApiBehaviorApplicationModelProvider(options, NullLoggerFactory.Instance); + var provider = GetProvider(options); // Act provider.OnProvidersExecuting(context); @@ -58,12 +56,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal { // Arrange var context = GetContext(typeof(SimpleController)); - var options = new TestOptionsManager(new ApiBehaviorOptions - { - InvalidModelStateResponseFactory = _ => null, - }); - - var provider = new ApiBehaviorApplicationModelProvider(options, NullLoggerFactory.Instance); + var provider = GetProvider(); // Act provider.OnProvidersExecuting(context); @@ -88,10 +81,10 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = GetContext(typeof(SimpleController)); var options = new TestOptionsManager(new ApiBehaviorOptions { - EnableModelStateInvalidFilter = false, + SuppressModelStateInvalidFilter = true, }); - var provider = new ApiBehaviorApplicationModelProvider(options, NullLoggerFactory.Instance); + var provider = GetProvider(options); // Act provider.OnProvidersExecuting(context); @@ -114,12 +107,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal { // Arrange var context = GetContext(typeof(ActionsWithoutAttributeRouting)); - var options = new TestOptionsManager(new ApiBehaviorOptions - { - InvalidModelStateResponseFactory = _ => null, - }); - - var provider = new ApiBehaviorApplicationModelProvider(options, NullLoggerFactory.Instance); + var provider = GetProvider(); // Act & Assert var ex = Assert.Throws(() => provider.OnProvidersExecuting(context)); @@ -128,6 +116,260 @@ namespace Microsoft.AspNetCore.Mvc.Internal ex.Message); } + [Fact] + public void InferBindingSourceForParameter_ReturnsPath_IfParameterNameExistsInRouteAsSimpleToken() + { + // Arrange + var actionName = nameof(ParameterBindingController.SimpleRouteToken); + var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); + var provider = GetProvider(); + + // Act + var result = provider.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Path, result); + } + + [Fact] + public void InferBindingSourceForParameter_ReturnsPath_IfParameterNameExistsInRouteAsOptionalToken() + { + // Arrange + var actionName = nameof(ParameterBindingController.OptionalRouteToken); + var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); + var provider = GetProvider(); + + // Act + var result = provider.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Path, result); + } + + [Fact] + public void InferBindingSourceForParameter_ReturnsPath_IfParameterNameExistsInRouteAsConstrainedToken() + { + // Arrange + var actionName = nameof(ParameterBindingController.ConstrainedRouteToken); + var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); + var provider = GetProvider(); + + // Act + var result = provider.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Path, result); + } + + [Fact] + public void InferBindingSourceForParameter_ReturnsPath_IfParameterNameExistsInAbsoluteRoute() + { + // Arrange + var actionName = nameof(ParameterBindingController.AbsoluteRoute); + var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); + var provider = GetProvider(); + + // Act + var result = provider.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Path, result); + } + + [Fact] + public void InferBindingSourceForParameter_ReturnsPath_IfParameterAppearsInAllRoutes() + { + // Arrange + var actionName = nameof(ParameterBindingController.ParameterInMultipleRoutes); + var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); + var provider = GetProvider(); + + // Act + var result = provider.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Path, result); + } + + [Fact] + public void InferBindingSourceForParameter_DoesNotReturnPath_IfParameterDoesNotAppearInAllRoutes() + { + // Arrange + var actionName = nameof(ParameterBindingController.ParameterNotInAllRoutes); + var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); + var provider = GetProvider(); + + // Act + var result = provider.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Query, result); + } + + [Fact] + public void InferBindingSourceForParameter_ReturnsPath_IfParameterAppearsInControllerRoute() + { + // Arrange + var actionName = nameof(ParameterInController.ActionWithoutRoute); + var parameter = GetParameterModel(typeof(ParameterInController), actionName); + var provider = GetProvider(); + + // Act + var result = provider.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Path, result); + } + + [Fact] + public void InferBindingSourceForParameter_ReturnsPath_IfParameterAppearsInControllerRoute_AndActionHasRoute() + { + // Arrange + var actionName = nameof(ParameterInController.ActionWithRoute); + var parameter = GetParameterModel(typeof(ParameterInController), actionName); + var provider = GetProvider(); + + // Act + var result = provider.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Path, result); + } + + [Fact] + public void InferBindingSourceForParameter_ReturnsPath_IfParameterAppearsInAllActionRoutes() + { + // Arrange + var actionName = nameof(ParameterInController.MultipleRoute); + var parameter = GetParameterModel(typeof(ParameterInController), actionName); + var provider = GetProvider(); + + // Act + var result = provider.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Path, result); + } + + [Fact] + public void InferBindingSourceForParameter_DoesNotReturnPath_IfActionRouteOverridesControllerRoute() + { + // Arrange + var actionName = nameof(ParameterInController.AbsoluteRoute); + var parameter = GetParameterModel(typeof(ParameterInController), actionName); + var provider = GetProvider(); + + // Act + var result = provider.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Query, result); + } + + [Fact] + public void InferBindingSourceForParameter_DoesNotReturnPath_IfOneActionRouteOverridesControllerRoute() + { + // Arrange + var actionName = nameof(ParameterInController.MultipleRouteWithOverride); + var parameter = GetParameterModel(typeof(ParameterInController), actionName); + var provider = GetProvider(); + + // Act + var result = provider.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Query, result); + } + + [Fact] + public void InferBindingSourceForParameter_ReturnsPath_IfParameterExistsInRoute_OnControllersWithoutSelectors() + { + // Arrange + var actionName = nameof(ParameterBindingNoRoutesOnController.SimpleRoute); + var parameter = GetParameterModel(typeof(ParameterBindingNoRoutesOnController), actionName); + var provider = GetProvider(); + + // Act + var result = provider.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Path, result); + } + + [Fact] + public void InferBindingSourceForParameter_ReturnsPath_IfParameterExistsInAllRoutes_OnControllersWithoutSelectors() + { + // Arrange + var actionName = nameof(ParameterBindingNoRoutesOnController.ParameterInMultipleRoutes); + var parameter = GetParameterModel(typeof(ParameterBindingNoRoutesOnController), actionName); + var provider = GetProvider(); + + // Act + var result = provider.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Path, result); + } + + [Fact] + public void InferBindingSourceForParameter_DoesNotReturnPath_IfNeitherActionNorControllerHasTemplate() + { + // Arrange + var actionName = nameof(ParameterBindingNoRoutesOnController.NoRouteTemplate); + var parameter = GetParameterModel(typeof(ParameterBindingNoRoutesOnController), actionName); + var provider = GetProvider(); + + // Act + var result = provider.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Query, result); + } + + [Fact] + public void InferBindingSourceForParameter_ReturnsBodyForComplexTypes() + { + // Arrange + var actionName = nameof(ParameterBindingController.ComplexTypeModel); + var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); + var provider = GetProvider(); + + // Act + var result = provider.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Body, result); + } + + [Fact] + public void InferBindingSourceForParameter_ReturnsBodyForSimpleTypes() + { + // Arrange + var actionName = nameof(ParameterBindingController.SimpleTypeModel); + var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); + var provider = GetProvider(); + + // Act + var result = provider.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Query, result); + } + + private static ApiBehaviorApplicationModelProvider GetProvider( + IOptions options = null, + IModelMetadataProvider modelMetadataProvider = null) + { + options = options ?? new TestOptionsManager(new ApiBehaviorOptions + { + InvalidModelStateResponseFactory = _ => null, + }); + modelMetadataProvider = modelMetadataProvider ?? new TestModelMetadataProvider(); + var loggerFactory = NullLoggerFactory.Instance; + + return new ApiBehaviorApplicationModelProvider(options, modelMetadataProvider, loggerFactory); + } + private static ApplicationModelProviderContext GetContext(Type type) { var context = new ApplicationModelProviderContext(new[] { type.GetTypeInfo() }); @@ -135,6 +377,14 @@ namespace Microsoft.AspNetCore.Mvc.Internal return context; } + private static ParameterModel GetParameterModel(Type controllerType, string actionName) + { + var context = GetContext(controllerType); + var controller = Assert.Single(context.Result.Controllers); + var action = Assert.Single(controller.Actions, m => m.ActionName == actionName); + return Assert.Single(action.Parameters); + } + [ApiController] [Route("TestApi")] private class TestApiController : Controller @@ -162,5 +412,88 @@ namespace Microsoft.AspNetCore.Mvc.Internal private class TestApiBehavior : Attribute, IApiBehaviorMetadata { } + + [ApiController] + [Route("[controller]/[action]")] + private class ParameterBindingController + { + [HttpGet("{parameter}")] + public IActionResult ActionWithBoundParameter([FromBody] object parameter) => null; + + [HttpGet("{id}")] + public IActionResult SimpleRouteToken(int id) => null; + + [HttpPost("optional/{id?}")] + public IActionResult OptionalRouteToken(int id) => null; + + [HttpDelete("delete-by-status/{status:int?}")] + public IActionResult ConstrainedRouteToken(object status) => null; + + [HttpPut("/absolute-route/{status:int}")] + public IActionResult AbsoluteRoute(object status) => null; + + [HttpPost("multiple/{id}")] + [HttpPut("multiple/{id}")] + public IActionResult ParameterInMultipleRoutes(int id) => null; + + [HttpPatch("patchroute")] + [HttpPost("multiple/{id}")] + [HttpPut("multiple/{id}")] + public IActionResult ParameterNotInAllRoutes(int id) => null; + + [HttpPut("put-action/{id}")] + public IActionResult ComplexTypeModel(TestModel model) => null; + + [HttpPut("put-action/{id}")] + public IActionResult SimpleTypeModel(ConvertibleFromString model) => null; + } + + [ApiController] + [Route("/route1/[controller]/[action]/{id}")] + [Route("/route2/[controller]/[action]/{id?}")] + private class ParameterInController + { + [HttpGet] + public IActionResult ActionWithoutRoute(int id) => null; + + [HttpGet("stuff/{status}")] + public IActionResult ActionWithRoute(int id) => null; + + [HttpGet("/absolute-route")] + public IActionResult AbsoluteRoute(int id) => null; + + [HttpPut] + [HttpPost("stuff/{status}")] + public IActionResult MultipleRoute(int id) => null; + + [HttpPut] + [HttpPost("~/stuff/{status}")] + public IActionResult MultipleRouteWithOverride(int id) => null; + } + + [ApiController] + private class ParameterBindingNoRoutesOnController + { + [HttpGet("{parameter}")] + public IActionResult SimpleRoute(int parameter) => null; + + [HttpGet] + public IActionResult NoRouteTemplate(int id) => null; + + [HttpPost("multiple/{id}")] + [HttpPut("multiple/{id}")] + public IActionResult ParameterInMultipleRoutes(int id) => null; + } + + private class TestModel { } + + [TypeConverter(typeof(ConvertibleFromStringConverter))] + private class ConvertibleFromString { } + + private class ConvertibleFromStringConverter : TypeConverter + { + public override bool CanConvertFrom(ITypeDescriptorContext context, Type sourceType) + => sourceType == typeof(string); + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiControllerAttributeTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs similarity index 68% rename from test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiControllerAttributeTests.cs rename to test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs index 61183259ce..99703f4cc2 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiControllerAttributeTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs @@ -12,9 +12,9 @@ using Xunit; namespace Microsoft.AspNetCore.Mvc.FunctionalTests { - public class ApiControllerAttributeTests : IClassFixture> + public class ApiBehaviorTest : IClassFixture> { - public ApiControllerAttributeTests(MvcTestFixture fixture) + public ApiBehaviorTest(MvcTestFixture fixture) { Client = fixture.Client; } @@ -97,5 +97,43 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.Equal(expected[i].Message, expected[i].Message); } } + + [Fact] + public async Task ActionsWithApiBehavior_InferFromBodyParameters() + { + // Arrange + var input = new Contact + { + ContactId = 13, + Name = "Test123", + }; + + // Act + var response = await Client.PostAsJsonAsync("/contact/ActionWithInferredFromBodyParameter", input); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var result = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); + Assert.Equal(input.ContactId, result.ContactId); + Assert.Equal(input.Name, result.Name); + } + + [Fact] + public async Task ActionsWithApiBehavior_InferQueryAndRouteParameters() + { + // Arrange + var id = 31; + var name = "test"; + var email = "email@test.com"; + var url = $"/contact/ActionWithInferredRouteAndQueryParameters/{name}/{id}?email={email}"; + var response = await Client.PostAsync(url, new StringContent(string.Empty)); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var result = JsonConvert.DeserializeObject(await response.Content.ReadAsStringAsync()); + Assert.Equal(id, result.ContactId); + Assert.Equal(name, result.Name); + Assert.Equal(email, result.Email); + } } } diff --git a/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs b/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs index 43006e7966..38195a455d 100644 --- a/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs +++ b/test/WebSites/BasicWebSite/Controllers/ContactApiController.cs @@ -47,5 +47,19 @@ namespace BasicWebSite _repository.Add(contact); return CreatedAtAction(nameof(Get), new { id = contact.ContactId }, contact); } + + [HttpPost("ActionWithInferredFromBodyParameter")] + public ActionResult ActionWithInferredFromBodyParameter(Contact contact) => contact; + + [HttpPost("ActionWithInferredRouteAndQueryParameters/{name}/{id}")] + public ActionResult ActionWithInferredRouteAndQueryParameter(int id, string name, string email) + { + return new Contact + { + ContactId = id, + Name = name, + Email = email, + }; + } } } \ No newline at end of file