diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs index 8701d4f4a3..05f5f4cdfe 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs @@ -349,8 +349,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// Gets a value indicating whether is a complex type. /// /// - /// A complex type is defined as a which has a - /// that can convert from . + /// A complex type is defined as a without a that can convert + /// from . Most POCO and types are therefore complex. Most, if + /// not all, BCL value types are simple types. /// public bool IsComplexType { get; private set; } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs index b2b2c2cd6f..26a796130c 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApiBehaviorOptions.cs @@ -17,6 +17,7 @@ namespace Microsoft.AspNetCore.Mvc { private readonly CompatibilitySwitch _suppressMapClientErrors; private readonly CompatibilitySwitch _suppressUseValidationProblemDetailsForInvalidModelStateResponses; + private readonly CompatibilitySwitch _allowInferringBindingSourceForCollectionTypesAsFromQuery; private readonly ICompatibilitySwitch[] _switches; private Func _invalidModelStateResponseFactory; @@ -28,10 +29,12 @@ namespace Microsoft.AspNetCore.Mvc { _suppressMapClientErrors = new CompatibilitySwitch(nameof(SuppressMapClientErrors)); _suppressUseValidationProblemDetailsForInvalidModelStateResponses = new CompatibilitySwitch(nameof(SuppressUseValidationProblemDetailsForInvalidModelStateResponses)); + _allowInferringBindingSourceForCollectionTypesAsFromQuery = new CompatibilitySwitch(nameof(AllowInferringBindingSourceForCollectionTypesAsFromQuery)); _switches = new[] { _suppressMapClientErrors, _suppressUseValidationProblemDetailsForInvalidModelStateResponses, + _allowInferringBindingSourceForCollectionTypesAsFromQuery }; } @@ -151,6 +154,43 @@ namespace Microsoft.AspNetCore.Mvc set => _suppressUseValidationProblemDetailsForInvalidModelStateResponses.Value = value; } + /// + /// Gets or sets a value that determines if for collection types + /// (). + /// + /// When , the binding source for collection types is inferred as . + /// Otherwise is inferred. + /// + /// + /// + /// The default value is if the version is + /// or later; otherwise. + /// + /// + /// + /// This property is associated with a compatibility switch and can provide a different behavior depending on + /// the configured compatibility version for the application. See for + /// guidance and examples of setting the application's compatibility version. + /// + /// + /// Configuring the desired value of the compatibility switch by calling this property's setter takes + /// precedence over the value implied by the application's . + /// + /// + /// If the application's compatibility version is set to or + /// lower then this setting will have the value unless explicitly configured. + /// + /// + /// If the application's compatibility version is set to or + /// higher then this setting will have the value unless explicitly configured. + /// + /// + public bool AllowInferringBindingSourceForCollectionTypesAsFromQuery + { + get => _allowInferringBindingSourceForCollectionTypesAsFromQuery.Value; + set => _allowInferringBindingSourceForCollectionTypesAsFromQuery.Value = value; + } + /// /// Gets a map of HTTP status codes to . Configured values /// are used to transform to an diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ApiBehaviorApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ApiBehaviorApplicationModelProvider.cs index 62831919de..35a415297b 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ApiBehaviorApplicationModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/ApiBehaviorApplicationModelProvider.cs @@ -48,14 +48,15 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels var defaultErrorTypeAttribute = new ProducesErrorResponseTypeAttribute(defaultErrorType); ActionModelConventions.Add(new ApiConventionApplicationModelConvention(defaultErrorTypeAttribute)); - var inferParameterBindingInfoConvention = new InferParameterBindingInfoConvention(modelMetadataProvider) + if (!options.SuppressInferBindingSourcesForParameters) { - SuppressInferBindingSourcesForParameters = options.SuppressInferBindingSourcesForParameters - }; - ControllerModelConventions = new List - { - inferParameterBindingInfoConvention, - }; + var convention = new InferParameterBindingInfoConvention(modelMetadataProvider) + { + AllowInferringBindingSourceForCollectionTypesAsFromQuery = options.AllowInferringBindingSourceForCollectionTypesAsFromQuery, + }; + + ActionModelConventions.Add(convention); + } } /// @@ -66,8 +67,6 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels public List ActionModelConventions { get; } - public List ControllerModelConventions { get; } - public void OnProvidersExecuted(ApplicationModelProviderContext context) { } @@ -91,11 +90,6 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels convention.Apply(action); } } - - foreach (var convention in ControllerModelConventions) - { - convention.Apply(controller); - } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/InferParameterBindingInfoConvention.cs b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/InferParameterBindingInfoConvention.cs index 15721bd2fa..9a3839308a 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/InferParameterBindingInfoConvention.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ApplicationModels/InferParameterBindingInfoConvention.cs @@ -2,9 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Collections.Generic; using System.Linq; -using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Routing.Template; @@ -13,13 +11,18 @@ using Resources = Microsoft.AspNetCore.Mvc.Core.Resources; namespace Microsoft.AspNetCore.Mvc.ApplicationModels { /// - /// A that - /// - /// infers binding sources for parameters - /// for bound properties and parameters. - /// + /// An that infers for parameters. /// - public class InferParameterBindingInfoConvention : IControllerModelConvention + /// + /// The goal of this covention is to make intuitive and easy to document inferences. The rules are: + /// + /// A previously specified is never overwritten. + /// A complex type parameter () is assigned . + /// Parameter with a name that appears as a route value in ANY route template is assigned . + /// All other parameters are . + /// + /// + public class InferParameterBindingInfoConvention : IActionModelConvention { private readonly IModelMetadataProvider _modelMetadataProvider; @@ -29,41 +32,27 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels _modelMetadataProvider = modelMetadataProvider ?? throw new ArgumentNullException(nameof(modelMetadataProvider)); } - /// - /// Gets or sets a value that determines if model binding sources are inferred for action parameters on controllers is suppressed. - /// - public bool SuppressInferBindingSourcesForParameters { get; set; } + internal bool AllowInferringBindingSourceForCollectionTypesAsFromQuery { get; set; } - protected virtual bool ShouldApply(ControllerModel controller) => true; + protected virtual bool ShouldApply(ActionModel action) => true; - public void Apply(ControllerModel controller) + public void Apply(ActionModel action) { - if (controller == null) + if (action == null) { - throw new ArgumentNullException(nameof(controller)); + throw new ArgumentNullException(nameof(action)); } - if (!ShouldApply(controller)) + if (!ShouldApply(action)) { return; } - InferBoundPropertyModelPrefixes(controller); - - foreach (var action in controller.Actions) - { - InferParameterBindingSources(action); - InferParameterModelPrefixes(action); - } + InferParameterBindingSources(action); } internal void InferParameterBindingSources(ActionModel action) { - if (SuppressInferBindingSourcesForParameters) - { - return; - } - for (var i = 0; i < action.Parameters.Count; i++) { var parameter = action.Parameters[i]; @@ -95,56 +84,17 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels // Internal for unit testing. internal BindingSource InferBindingSourceForParameter(ParameterModel parameter) { + if (IsComplexTypeParameter(parameter)) + { + return BindingSource.Body; + } + if (ParameterExistsInAnyRoute(parameter.Action, parameter.ParameterName)) { return BindingSource.Path; } - var bindingSource = IsComplexTypeParameter(parameter) ? - BindingSource.Body : - BindingSource.Query; - - return bindingSource; - } - - // For any complex types that are bound from value providers, set the prefix - // to the empty prefix by default. This makes binding much more predictable - // and describable via ApiExplorer - internal void InferBoundPropertyModelPrefixes(ControllerModel controllerModel) - { - foreach (var property in controllerModel.ControllerProperties) - { - if (property.BindingInfo != null && - property.BindingInfo.BinderModelName == null && - property.BindingInfo.BindingSource != null && - !property.BindingInfo.BindingSource.IsGreedy && - !IsFormFile(property.ParameterType)) - { - var metadata = _modelMetadataProvider.GetMetadataForProperty( - controllerModel.ControllerType, - property.PropertyInfo.Name); - if (metadata.IsComplexType && !metadata.IsCollectionType) - { - property.BindingInfo.BinderModelName = string.Empty; - } - } - } - } - - internal void InferParameterModelPrefixes(ActionModel action) - { - foreach (var parameter in action.Parameters) - { - var bindingInfo = parameter.BindingInfo; - if (bindingInfo?.BindingSource != null && - bindingInfo.BinderModelName == null && - !bindingInfo.BindingSource.IsGreedy && - !IsFormFile(parameter.ParameterType) && - IsComplexTypeParameter(parameter)) - { - parameter.BindingInfo.BinderModelName = string.Empty; - } - } + return BindingSource.Query; } private bool ParameterExistsInAnyRoute(ActionModel action, string parameterName) @@ -171,13 +121,13 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels // No need for information from attributes on the parameter. Just use its type. var metadata = _modelMetadataProvider .GetMetadataForType(parameter.ParameterInfo.ParameterType); - return metadata.IsComplexType && !metadata.IsCollectionType; - } - private static bool IsFormFile(Type parameterType) - { - return typeof(IFormFile).IsAssignableFrom(parameterType) || - typeof(IEnumerable).IsAssignableFrom(parameterType); + if (AllowInferringBindingSourceForCollectionTypesAsFromQuery && metadata.IsCollectionType) + { + return false; + } + + return metadata.IsComplexType; } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorOptionsSetup.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorOptionsSetup.cs index bddaaa2478..02ae3bc30d 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorOptionsSetup.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ApiBehaviorOptionsSetup.cs @@ -35,6 +35,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal { dictionary[nameof(ApiBehaviorOptions.SuppressMapClientErrors)] = true; dictionary[nameof(ApiBehaviorOptions.SuppressUseValidationProblemDetailsForInvalidModelStateResponses)] = true; + dictionary[nameof(ApiBehaviorOptions.AllowInferringBindingSourceForCollectionTypesAsFromQuery)] = true; } return dictionary; diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Properties/AssemblyInfo.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Properties/AssemblyInfo.cs index 458502d802..5f7daf43f9 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Properties/AssemblyInfo.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Properties/AssemblyInfo.cs @@ -4,4 +4,5 @@ using System.Runtime.CompilerServices; [assembly: InternalsVisibleTo("Microsoft.AspNetCore.Mvc.Test, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] +[assembly: InternalsVisibleTo("Microsoft.AspNetCore.Mvc.IntegrationTests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] [assembly: InternalsVisibleTo("Microsoft.AspNetCore.Mvc.RazorPages.Test, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs index 1d00e52dc2..12c4984cf5 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/ApiBehaviorApplicationModelProviderTest.cs @@ -98,15 +98,8 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels { var convention = Assert.IsType(c); Assert.Equal(typeof(ProblemDetails), convention.DefaultErrorResponseType.Type); - }); - - Assert.Collection( - provider.ControllerModelConventions, - c => - { - var convention = Assert.IsType(c); - Assert.False(convention.SuppressInferBindingSourcesForParameters); - }); + }, + c => Assert.IsType(c)); } [Fact] @@ -140,14 +133,13 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels } [Fact] - public void Constructor_SetsSuppressInferBindingSourcesForParametersIsSet() + public void Constructor_DoesNotAddInferParameterBindingInfoConvention_IfSuppressInferBindingSourcesForParametersIsSet() { // Arrange var provider = GetProvider(new ApiBehaviorOptions { SuppressInferBindingSourcesForParameters = true }); // Act & Assert - var convention = Assert.Single(provider.ControllerModelConventions.OfType()); - Assert.True(convention.SuppressInferBindingSourcesForParameters); + Assert.Empty(provider.ActionModelConventions.OfType()); } [Fact] diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/InferParameterBindingInfoConventionTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/InferParameterBindingInfoConventionTest.cs index 5f27a66df8..8b82f3d6ad 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/InferParameterBindingInfoConventionTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ApplicationModels/InferParameterBindingInfoConventionTest.cs @@ -28,7 +28,7 @@ namespace Microsoft.AspNetCore.Mvc.ApplicationModels var action = GetActionModel(typeof(ParameterWithBindingInfo), actionName); // Act - convention.Apply(action.Controller); + convention.Apply(action); // Assert var parameterModel = Assert.Single(action.Parameters); @@ -126,38 +126,6 @@ Environment.NewLine + "int b"; }); } - [Fact] - public void InferParameterBindingSources_DoesNotInferSources_IfSuppressInferBindingSourcesForParametersIsSet() - { - // Arrange - var actionName = nameof(ParameterBindingController.ComplexTypeModelWithCancellationToken); - var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); - var convention = GetConvention(modelMetadataProvider); - var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider); - - convention.SuppressInferBindingSourcesForParameters = true; - - // Act - convention.InferParameterBindingSources(action); - - // Assert - Assert.Collection( - action.Parameters, - parameter => - { - Assert.Equal("model", parameter.Name); - Assert.Null(parameter.BindingInfo); - }, - parameter => - { - Assert.Equal("cancellationToken", parameter.Name); - - var bindingInfo = parameter.BindingInfo; - Assert.NotNull(bindingInfo); - Assert.Equal(BindingSource.Special, bindingInfo.BindingSource); - }); - } - [Fact] public void Apply_PreservesBindingInfo_WhenInferringFor_ParameterWithModelBinder_AndExplicitName() { @@ -168,7 +136,7 @@ Environment.NewLine + "int b"; var action = GetActionModel(typeof(ModelBinderOnParameterController), actionName, modelMetadataProvider); // Act - convention.Apply(action.Controller); + convention.Apply(action); // Assert var parameter = Assert.Single(action.Parameters); @@ -189,7 +157,7 @@ Environment.NewLine + "int b"; var action = GetActionModel(typeof(ModelBinderOnParameterController), actionName, modelMetadataProvider); // Act - convention.Apply(action.Controller); + convention.Apply(action); // Assert var parameter = Assert.Single(action.Parameters); @@ -210,7 +178,7 @@ Environment.NewLine + "int b"; var action = GetActionModel(typeof(ModelBinderOnParameterController), actionName, modelMetadataProvider); // Act - convention.Apply(action.Controller); + convention.Apply(action); // Assert var parameter = Assert.Single(action.Parameters); @@ -267,10 +235,10 @@ Environment.NewLine + "int b"; } [Fact] - public void InferBindingSourceForParameter_ReturnsPath_IfParameterNameExistsInAbsoluteRoute() + public void InferBindingSourceForParameter_ReturnsBody_ForComplexTypeParameterThatAppearsInRoute() { // Arrange - var actionName = nameof(ParameterBindingController.AbsoluteRoute); + var actionName = nameof(ParameterBindingController.ComplexTypeInRoute); var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); var convention = GetConvention(); @@ -278,7 +246,7 @@ Environment.NewLine + "int b"; var result = convention.InferBindingSourceForParameter(parameter); // Assert - Assert.Same(BindingSource.Path, result); + Assert.Same(BindingSource.Body, result); } [Fact] @@ -472,7 +440,7 @@ Environment.NewLine + "int b"; } [Fact] - public void InferBindingSourceForParameter_ReturnsBodyForSimpleTypes() + public void InferBindingSourceForParameter_ReturnsQueryForSimpleTypes() { // Arrange var actionName = nameof(ParameterBindingController.SimpleTypeModel); @@ -486,6 +454,68 @@ Environment.NewLine + "int b"; Assert.Same(BindingSource.Query, result); } + [Fact] + public void InferBindingSourceForParameter_ReturnsBodyForCollectionOfSimpleTypes() + { + // Arrange + var actionName = nameof(ParameterBindingController.CollectionOfSimpleTypes); + var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); + var convention = GetConvention(); + + // Act + var result = convention.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Body, result); + } + + [Fact] + public void InferBindingSourceForParameter_ReturnsQueryForCollectionOfSimpleTypes_WhenAllowInferringBindingSourceForCollectionTypesAsFromQueryIsSet() + { + // Arrange + var actionName = nameof(ParameterBindingController.CollectionOfSimpleTypes); + var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); + var convention = GetConvention(); + convention.AllowInferringBindingSourceForCollectionTypesAsFromQuery = true; + + // Act + var result = convention.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Query, result); + } + + [Fact] + public void InferBindingSourceForParameter_ReturnsBodyForCollectionOfComplexTypes() + { + // Arrange + var actionName = nameof(ParameterBindingController.CollectionOfComplexTypes); + var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); + var convention = GetConvention(); + + // Act + var result = convention.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Body, result); + } + + [Fact] + public void InferBindingSourceForParameter_ReturnsQueryForCollectionOfComplexTypes_WhenAllowInferringBindingSourceForCollectionTypesAsFromQueryIsSet() + { + // Arrange + var actionName = nameof(ParameterBindingController.CollectionOfComplexTypes); + var parameter = GetParameterModel(typeof(ParameterBindingController), actionName); + var convention = GetConvention(); + convention.AllowInferringBindingSourceForCollectionTypesAsFromQuery = true; + + // Act + var result = convention.InferBindingSourceForParameter(parameter); + + // Assert + Assert.Same(BindingSource.Query, result); + } + [Fact] public void PreservesBindingSourceInference_ForFromQueryParameter_WithDefaultName() { @@ -496,7 +526,7 @@ Environment.NewLine + "int b"; var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider); // Act - convention.Apply(action.Controller); + convention.Apply(action); // Assert var parameter = Assert.Single(action.Parameters); @@ -517,7 +547,7 @@ Environment.NewLine + "int b"; var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider); // Act - convention.Apply(action.Controller); + convention.Apply(action); // Assert var parameter = Assert.Single(action.Parameters); @@ -538,7 +568,7 @@ Environment.NewLine + "int b"; var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider); // Act - convention.Apply(action.Controller); + convention.Apply(action); // Assert var parameter = Assert.Single(action.Parameters); @@ -558,7 +588,7 @@ Environment.NewLine + "int b"; var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider); // Act - convention.Apply(action.Controller); + convention.Apply(action); // Assert var parameter = Assert.Single(action.Parameters); @@ -579,7 +609,7 @@ Environment.NewLine + "int b"; var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider); // Act - convention.Apply(action.Controller); + convention.Apply(action); // Assert var parameter = Assert.Single(action.Parameters); @@ -600,7 +630,7 @@ Environment.NewLine + "int b"; var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider); // Act - convention.Apply(action.Controller); + convention.Apply(action); // Assert var parameter = Assert.Single(action.Parameters); @@ -621,7 +651,7 @@ Environment.NewLine + "int b"; var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider); // Act - convention.Apply(action.Controller); + convention.Apply(action); // Assert var parameter = Assert.Single(action.Parameters); @@ -642,7 +672,7 @@ Environment.NewLine + "int b"; var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider); // Act - convention.Apply(action.Controller); + convention.Apply(action); // Assert var parameter = Assert.Single(action.Parameters); @@ -663,7 +693,7 @@ Environment.NewLine + "int b"; var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider); // Act - convention.Apply(action.Controller); + convention.Apply(action); // Assert var parameter = Assert.Single(action.Parameters); @@ -684,7 +714,7 @@ Environment.NewLine + "int b"; var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider); // Act - convention.Apply(action.Controller); + convention.Apply(action); // Assert var parameter = Assert.Single(action.Parameters); @@ -704,7 +734,7 @@ Environment.NewLine + "int b"; var action = GetActionModel(typeof(ParameterBindingController), actionName, modelMetadataProvider); // Act - convention.Apply(action.Controller); + convention.Apply(action); // Assert var parameter = Assert.Single(action.Parameters); @@ -727,7 +757,7 @@ Environment.NewLine + "int b"; var convention = GetConvention(); // Act - convention.Apply(action.Controller); + convention.Apply(action); // Assert var parameter = Assert.Single(action.Parameters); @@ -740,132 +770,6 @@ Environment.NewLine + "int b"; Assert.Null(bindingInfo.BinderModelName); } - [Fact] - public void InferBoundPropertyModelPrefixes_SetsModelPrefix_ForComplexTypeFromValueProvider() - { - // Arrange - var controller = GetControllerModel(typeof(ControllerWithBoundProperty)); - var convention = GetConvention(); - - // Act - convention.InferBoundPropertyModelPrefixes(controller); - - // Assert - var property = Assert.Single(controller.ControllerProperties, p => p.Name == nameof(ControllerWithBoundProperty.TestProperty)); - Assert.Equal(string.Empty, property.BindingInfo.BinderModelName); - } - - [Fact] - public void InferBoundPropertyModelPrefixes_SetsModelPrefix_ForCollectionTypeFromValueProvider() - { - // Arrange - var controller = GetControllerModel(typeof(ControllerWithBoundCollectionProperty)); - var convention = GetConvention(); - - // Act - convention.InferBoundPropertyModelPrefixes(controller); - - // Assert - var property = Assert.Single(controller.ControllerProperties); - Assert.Null(property.BindingInfo.BinderModelName); - } - - [Fact] - public void InferParameterModelPrefixes_SetsModelPrefix_ForComplexTypeFromValueProvider() - { - // Arrange - var action = GetActionModel(typeof(ControllerWithBoundProperty), nameof(ControllerWithBoundProperty.SomeAction)); - var convention = GetConvention(); - - // Act - convention.InferParameterModelPrefixes(action); - - // Assert - var parameter = Assert.Single(action.Parameters); - Assert.Equal(string.Empty, parameter.BindingInfo.BinderModelName); - } - - [Fact] - public void InferParameterModelPrefixes_DoesNotSetModelPrefix_ForFormFileParametersAnnotatedWithFromForm() - { - // Arrange - var action = GetActionModel( - typeof(ParameterBindingController), - nameof(ParameterBindingController.FromFormFormFileParameters), - TestModelMetadataProvider.CreateDefaultProvider()); - var convention = GetConvention(); - - // Act - convention.InferParameterModelPrefixes(action); - - // Assert - Assert.Collection( - action.Parameters, - parameter => - { - Assert.Equal("p1", parameter.Name); - Assert.Null(parameter.BindingInfo.BinderModelName); - }, - parameter => - { - Assert.Equal("p2", parameter.Name); - Assert.Null(parameter.BindingInfo.BinderModelName); - }, - parameter => - { - Assert.Equal("p3", parameter.Name); - Assert.Null(parameter.BindingInfo.BinderModelName); - }); - } - - [Fact] - public void InferParameterModelPrefixes_DoesNotSetModelPrefix_ForFormFileParameters() - { - // Arrange - var action = GetActionModel( - typeof(ParameterBindingController), - nameof(ParameterBindingController.FormFileParameters), - TestModelMetadataProvider.CreateDefaultProvider()); - var convention = GetConvention(); - - // Act - convention.InferParameterModelPrefixes(action); - - // Assert - Assert.Collection( - action.Parameters, - parameter => - { - Assert.Equal("p1", parameter.Name); - Assert.Null(parameter.BindingInfo.BinderModelName); - }, - parameter => - { - Assert.Equal("p2", parameter.Name); - Assert.Null(parameter.BindingInfo.BinderModelName); - }, - parameter => - { - Assert.Equal("p3", parameter.Name); - Assert.Null(parameter.BindingInfo.BinderModelName); - }); - } - - [Fact] - public void InferBoundPropertyModelPrefixes_DoesNotSetModelPrefix_ForFormFileCollectionPropertiesAnnotatedWithFromForm() - { - // Arrange - var controller = GetControllerModel(typeof(ControllerWithBoundProperty)); - var convention = GetConvention(); - - // Act - convention.InferBoundPropertyModelPrefixes(controller); - - // Assert - var parameter = Assert.Single(controller.ControllerProperties, p => p.Name == nameof(ControllerWithBoundProperty.Files)); - Assert.Null(parameter.BindingInfo.BinderModelName); - } - private static InferParameterBindingInfoConvention GetConvention( IModelMetadataProvider modelMetadataProvider = null) { @@ -887,14 +791,6 @@ Environment.NewLine + "int b"; return context; } - private static ControllerModel GetControllerModel( - Type controllerType, - IModelMetadataProvider modelMetadataProvider = null) - { - var context = GetContext(controllerType, modelMetadataProvider); - return Assert.Single(context.Result.Controllers); - } - private static ActionModel GetActionModel( Type controllerType, string actionName, @@ -930,10 +826,10 @@ Environment.NewLine + "int b"; public IActionResult OptionalRouteToken(int id) => null; [HttpDelete("delete-by-status/{status:int?}")] - public IActionResult ConstrainedRouteToken(object status) => null; + public IActionResult ConstrainedRouteToken(int status) => null; [HttpPut("/absolute-route/{status:int}")] - public IActionResult AbsoluteRoute(object status) => null; + public IActionResult ComplexTypeInRoute(object status) => null; [HttpPost("multiple/{id}")] [HttpPut("multiple/{id}")] @@ -1011,6 +907,10 @@ Environment.NewLine + "int b"; public IActionResult FromFormFormFileParameters([FromForm] IFormFile p1, [FromForm] IFormFile[] p2, [FromForm] IFormFileCollection p3) => null; public IActionResult FormFileParameters(IFormFile p1, IFormFile[] p2, IFormFileCollection p3) => null; + + public IActionResult CollectionOfSimpleTypes(IList parameter) => null; + + public IActionResult CollectionOfComplexTypes(IList parameter) => null; } [ApiController] @@ -1064,7 +964,6 @@ Environment.NewLine + "int b"; public IActionResult ParameterInMultipleRoutes(int id) => null; } - private class TestModel { } [TypeConverter(typeof(ConvertibleFromStringConverter))] diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs index c30dd30750..521db7b904 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/ApiBehaviorTest.cs @@ -227,9 +227,9 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.Equal(HttpStatusCode.OK, response.StatusCode); var result = await response.Content.ReadAsAsync(); - Assert.Equal(0, result.ContactId); - Assert.Null(result.Name); - Assert.Null(result.Email); + Assert.Equal(id, result.ContactId); + Assert.Equal(name, result.Name); + Assert.Equal(email, result.Email); } [Fact] diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesWithBasePathTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesWithBasePathTest.cs index 3de7a91ebe..4a11189cb2 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesWithBasePathTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesWithBasePathTest.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.Net; using System.Net.Http; -using System.Net.Http.Headers; using System.Threading.Tasks; using Xunit; @@ -420,14 +419,16 @@ Hello from /Pages/Shared/"; var token = AntiforgeryTestHelper.RetrieveAntiforgeryToken(await getPage.Content.ReadAsStringAsync(), ""); var cookie = AntiforgeryTestHelper.RetrieveAntiforgeryCookie(getPage); - var message = new HttpRequestMessage(HttpMethod.Post, "/CustomModelTypeModel"); - message.Content = new FormUrlEncodedContent(new Dictionary + var message = new HttpRequestMessage(HttpMethod.Post, "/CustomModelTypeModel") { - ["__RequestVerificationToken"] = token, - ["ConfirmPassword"] = "", - ["Password"] = "", - ["Email"] = "" - }); + Content = new FormUrlEncodedContent(new Dictionary + { + ["__RequestVerificationToken"] = token, + ["ConfirmPassword"] = "", + ["Password"] = "", + ["Email"] = "" + }) + }; message.Headers.TryAddWithoutValidation("Cookie", $"{cookie.Key}={cookie.Value}"); // Act @@ -443,18 +444,20 @@ Hello from /Pages/Shared/"; public async Task PageConventions_CustomizedModelCanWorkWithModelState() { // Arrange - var getPage = await Client.GetAsync("/CustomModelTypeModel"); + var getPage = await Client.GetAsync("/CustomModelTypeModel?Attempts=0"); var token = AntiforgeryTestHelper.RetrieveAntiforgeryToken(await getPage.Content.ReadAsStringAsync(), ""); var cookie = AntiforgeryTestHelper.RetrieveAntiforgeryCookie(getPage); - var message = new HttpRequestMessage(HttpMethod.Post, "/CustomModelTypeModel"); - message.Content = new FormUrlEncodedContent(new Dictionary + var message = new HttpRequestMessage(HttpMethod.Post, "/CustomModelTypeModel?Attempts=3") { - ["__RequestVerificationToken"] = token, - ["Email"] = "javi@example.com", - ["Password"] = "Password.12$", - ["ConfirmPassword"] = "Password.12$", - }); + Content = new FormUrlEncodedContent(new Dictionary + { + ["__RequestVerificationToken"] = token, + ["Email"] = "javi@example.com", + ["Password"] = "Password.12$", + ["ConfirmPassword"] = "Password.12$", + }) + }; message.Headers.TryAddWithoutValidation("Cookie", $"{cookie.Key}={cookie.Value}"); // Act @@ -465,6 +468,37 @@ Hello from /Pages/Shared/"; Assert.Equal("/", response.Headers.Location.ToString()); } + [Fact] + public async Task PageConventions_CustomizedModelCanWorkWithModelState_EnforcesBindRequired() + { + // Arrange + var getPage = await Client.GetAsync("/CustomModelTypeModel?Attempts=0"); + var token = AntiforgeryTestHelper.RetrieveAntiforgeryToken(await getPage.Content.ReadAsStringAsync(), ""); + var cookie = AntiforgeryTestHelper.RetrieveAntiforgeryCookie(getPage); + + var message = new HttpRequestMessage(HttpMethod.Post, "/CustomModelTypeModel") + { + Content = new FormUrlEncodedContent(new Dictionary + { + ["__RequestVerificationToken"] = token, + ["Email"] = "javi@example.com", + ["Password"] = "Password.12$", + ["ConfirmPassword"] = "Password.12$", + }) + }; + message.Headers.TryAddWithoutValidation("Cookie", $"{cookie.Key}={cookie.Value}"); + + // Act + var response = await Client.SendAsync(message); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var responseText = await response.Content.ReadAsStringAsync(); + Assert.Contains( + "A value for the 'Attempts' parameter or property was not provided.", + responseText); + } + [Fact] public async Task ValidationAttributes_OnTopLevelProperties() { @@ -642,10 +676,12 @@ Hello from /Pages/Shared/"; var cookie = AntiforgeryTestHelper.RetrieveAntiforgeryCookie(response); - var content = new MultipartFormDataContent(); - content.Add(new StringContent("property1-value"), property1); - content.Add(new StringContent("test-value1"), file1, "test1.txt"); - content.Add(new StringContent("test-value2"), file3, "test2.txt"); + var content = new MultipartFormDataContent + { + { new StringContent("property1-value"), property1 }, + { new StringContent("test-value1"), file1, "test1.txt" }, + { new StringContent("test-value2"), file3, "test2.txt" } + }; var request = new HttpRequestMessage(HttpMethod.Post, url) { diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BindPropertyIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BindPropertyIntegrationTest.cs index 1b42c219f0..d054633f72 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BindPropertyIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BindPropertyIntegrationTest.cs @@ -1,15 +1,17 @@ // 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.ComponentModel; using System.ComponentModel.DataAnnotations; using System.Linq; +using System.Reflection; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ModelBinding; -using Microsoft.Extensions.Primitives; +using Microsoft.AspNetCore.Mvc.RazorPages; +using Microsoft.AspNetCore.Mvc.RazorPages.Infrastructure; +using Microsoft.AspNetCore.Mvc.RazorPages.Internal; using Xunit; namespace Microsoft.AspNetCore.Mvc.IntegrationTests @@ -179,6 +181,74 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests } } + [Theory] + [InlineData(null, false)] + [InlineData(123, true)] + public async Task BindModelAsync_WithBindPageProperty_EnforcesBindRequired(int? input, bool isValid) + { + // Arrange + var propertyInfo = typeof(TestPage).GetProperty(nameof(TestPage.BindRequiredProperty)); + var propertyDescriptor = new PageBoundPropertyDescriptor + { + BindingInfo = BindingInfo.GetBindingInfo(new[] + { + new FromQueryAttribute { Name = propertyInfo.Name }, + }), + Name = propertyInfo.Name, + ParameterType = propertyInfo.PropertyType, + Property = propertyInfo, + }; + + var typeInfo = typeof(TestPage).GetTypeInfo(); + var actionDescriptor = new CompiledPageActionDescriptor + { + BoundProperties = new[] { propertyDescriptor }, + HandlerTypeInfo = typeInfo, + ModelTypeInfo = typeInfo, + PageTypeInfo = typeInfo, + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.Method = "POST"; + if (input.HasValue) + { + request.QueryString = new QueryString($"?{propertyDescriptor.Name}={input.Value}"); + } + }); + + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(modelMetadataProvider); + var modelBinderFactory = ModelBindingTestHelper.GetModelBinderFactory(modelMetadataProvider); + var modelMetadata = modelMetadataProvider + .GetMetadataForProperty(typeof(TestPage), propertyDescriptor.Name); + + var pageBinder = PageBinderFactory.CreatePropertyBinder( + parameterBinder, + modelMetadataProvider, + modelBinderFactory, + actionDescriptor); + var pageContext = new PageContext + { + ActionDescriptor = actionDescriptor, + HttpContext = testContext.HttpContext, + RouteData = testContext.RouteData, + ValueProviderFactories = testContext.ValueProviderFactories, + }; + + var page = new TestPage(); + + // Act + await pageBinder(pageContext, page); + + // Assert + Assert.Equal(isValid, pageContext.ModelState.IsValid); + if (isValid) + { + Assert.Equal(input.Value, page.BindRequiredProperty); + } + } + [Theory] [InlineData("RequiredAndStringLengthProp", null, false)] [InlineData("RequiredAndStringLengthProp", "", false)] @@ -231,12 +301,18 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests } } - class TestController + private class TestController { [BindNever] public string BindNeverProp { get; set; } [BindRequired] public int BindRequiredProp { get; set; } [Required, StringLength(3)] public string RequiredAndStringLengthProp { get; set; } [DisplayName("My Display Name"), StringLength(3)] public string DisplayNameStringLengthProp { get; set; } } + + private class TestPage : PageModel + { + [BindRequired] + public int BindRequiredProperty { get; set; } + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs index e4020042f3..d16e2b85d9 100644 --- a/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Test/IntegrationTest/CompatibilitySwitchIntegrationTest.cs @@ -57,6 +57,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest Assert.False(razorPagesOptions.AllowDefaultHandlingForOptionsRequests); Assert.False(xmlOptions.AllowRfc7807CompliantProblemDetailsFormat); Assert.False(mvcOptions.AllowShortCircuitingValidationWhenNoValidatorsArePresent); + Assert.True(apiBehaviorOptions.AllowInferringBindingSourceForCollectionTypesAsFromQuery); } [Fact] @@ -95,6 +96,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest Assert.False(razorPagesOptions.AllowDefaultHandlingForOptionsRequests); Assert.False(xmlOptions.AllowRfc7807CompliantProblemDetailsFormat); Assert.False(mvcOptions.AllowShortCircuitingValidationWhenNoValidatorsArePresent); + Assert.True(apiBehaviorOptions.AllowInferringBindingSourceForCollectionTypesAsFromQuery); } [Fact] @@ -133,6 +135,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest Assert.True(razorPagesOptions.AllowDefaultHandlingForOptionsRequests); Assert.True(xmlOptions.AllowRfc7807CompliantProblemDetailsFormat); Assert.True(mvcOptions.AllowShortCircuitingValidationWhenNoValidatorsArePresent); + Assert.False(apiBehaviorOptions.AllowInferringBindingSourceForCollectionTypesAsFromQuery); } [Fact] @@ -171,6 +174,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTest Assert.True(razorPagesOptions.AllowDefaultHandlingForOptionsRequests); Assert.True(xmlOptions.AllowRfc7807CompliantProblemDetailsFormat); Assert.True(mvcOptions.AllowShortCircuitingValidationWhenNoValidatorsArePresent); + Assert.False(apiBehaviorOptions.AllowInferringBindingSourceForCollectionTypesAsFromQuery); } // This just does the minimum needed to be able to resolve these options. diff --git a/test/WebSites/RazorPagesWebSite/Pages/CustomModelTypeModel.cshtml.cs b/test/WebSites/RazorPagesWebSite/Pages/CustomModelTypeModel.cshtml.cs index e32a2e83c4..e78cce0428 100644 --- a/test/WebSites/RazorPagesWebSite/Pages/CustomModelTypeModel.cshtml.cs +++ b/test/WebSites/RazorPagesWebSite/Pages/CustomModelTypeModel.cshtml.cs @@ -1,6 +1,7 @@ using System; using System.ComponentModel.DataAnnotations; using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.RazorPages; using Microsoft.Extensions.Logging; @@ -13,6 +14,10 @@ namespace RazorPagesWebSite public string ReturnUrl { get; set; } + [BindRequired] + [FromQuery(Name = nameof(Attempts))] + public int Attempts { get; set; } + public class InputModel { [Required] @@ -69,10 +74,13 @@ namespace RazorPagesWebSite { if (!ModelState.IsValid) { + Attempts++; + RouteData.Values.Add(nameof(Attempts), Attempts); + return Page(); } return Redirect("~/"); } } -} \ No newline at end of file +}