diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelBinderProviderContext.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelBinderProviderContext.cs index aa7f4a91a7..2d01d6acb9 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelBinderProviderContext.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelBinderProviderContext.cs @@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public abstract IModelBinder CreateBinder(ModelMetadata metadata); /// - /// Gets the . May be null. + /// Gets the . /// public abstract BindingInfo BindingInfo { get; } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerArgumentBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerArgumentBinder.cs index 196d19209d..0b24891120 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerArgumentBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerArgumentBinder.cs @@ -156,6 +156,13 @@ namespace Microsoft.AspNetCore.Mvc.Internal } var metadata = _modelMetadataProvider.GetMetadataForType(parameter.ParameterType); + var binder = _modelBinderFactory.CreateBinder(new ModelBinderFactoryContext() + { + BindingInfo = parameter.BindingInfo, + Metadata = metadata, + CacheToken = parameter, + }); + var modelBindingContext = DefaultModelBindingContext.CreateBindingContext( controllerContext, valueProvider, @@ -163,10 +170,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal parameter.BindingInfo, parameter.Name); - if (parameter.BindingInfo?.BinderModelName != null) + var parameterModelName = parameter.BindingInfo?.BinderModelName ?? metadata.BinderModelName; + if (parameterModelName != null) { // The name was set explicitly, always use that as the prefix. - modelBindingContext.ModelName = parameter.BindingInfo.BinderModelName; + modelBindingContext.ModelName = parameterModelName; } else if (modelBindingContext.ValueProvider.ContainsPrefix(parameter.Name)) { @@ -179,13 +187,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal modelBindingContext.ModelName = string.Empty; } - var binder = _modelBinderFactory.CreateBinder(new ModelBinderFactoryContext() - { - BindingInfo = parameter.BindingInfo, - Metadata = metadata, - CacheToken = parameter, - }); - await binder.BindModelAsync(modelBindingContext); var modelBindingResult = modelBindingContext.Result; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BinderTypeModelBinderProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BinderTypeModelBinderProvider.cs index 22a0c1bac7..aa56c0edcb 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BinderTypeModelBinderProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BinderTypeModelBinderProvider.cs @@ -19,7 +19,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders throw new ArgumentNullException(nameof(context)); } - if (context.BindingInfo?.BinderType != null) + if (context.BindingInfo.BinderType != null) { return new BinderTypeModelBinder(context.BindingInfo.BinderType); } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BodyModelBinderProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BodyModelBinderProvider.cs index ac2818c33f..0658cc71d3 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BodyModelBinderProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BodyModelBinderProvider.cs @@ -45,7 +45,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders throw new ArgumentNullException(nameof(context)); } - if (context.BindingInfo?.BindingSource != null && + if (context.BindingInfo.BindingSource != null && context.BindingInfo.BindingSource.CanAcceptDataFrom(BindingSource.Body)) { return new BodyModelBinder(_formatters, _readerFactory); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/HeaderModelBinderProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/HeaderModelBinderProvider.cs index 8da75e8da2..ce10e5ebb8 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/HeaderModelBinderProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/HeaderModelBinderProvider.cs @@ -18,7 +18,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders throw new ArgumentNullException(nameof(context)); } - if (context.BindingInfo?.BindingSource != null && + if (context.BindingInfo.BindingSource != null && context.BindingInfo.BindingSource.CanAcceptDataFrom(BindingSource.Header)) { // We only support strings and collections of strings. Some cases can fail diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/ServicesModelBinderProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/ServicesModelBinderProvider.cs index c543a043d2..2dea775a0a 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/ServicesModelBinderProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/ServicesModelBinderProvider.cs @@ -18,7 +18,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders throw new ArgumentNullException(nameof(context)); } - if (context.BindingInfo?.BindingSource != null && + if (context.BindingInfo.BindingSource != null && context.BindingInfo.BindingSource.CanAcceptDataFrom(BindingSource.Services)) { return new ServicesModelBinder(); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Internal/ModelBindingHelper.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Internal/ModelBindingHelper.cs index 1322f257f2..bb91869e18 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Internal/ModelBindingHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Internal/ModelBindingHelper.cs @@ -20,7 +20,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Internal { /// /// Updates the specified instance using the specified - /// and the specified and executes + /// and the specified and executes /// validation using the specified . /// /// The type of the model object. @@ -476,8 +476,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Internal if (expression.NodeType != ExpressionType.MemberAccess) { - throw new InvalidOperationException(Resources.FormatInvalid_IncludePropertyExpression( - expression.NodeType)); + throw new InvalidOperationException( + Resources.FormatInvalid_IncludePropertyExpression(expression.NodeType)); } var memberExpression = (MemberExpression)expression; @@ -488,7 +488,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Internal { // Chained expressions and non parameter based expressions are not supported. throw new InvalidOperationException( - Resources.FormatInvalid_IncludePropertyExpression(expression.NodeType)); + Resources.FormatInvalid_IncludePropertyExpression(expression.NodeType)); } return memberInfo.Name; @@ -496,8 +496,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Internal else { // Fields are also not supported. - throw new InvalidOperationException(Resources.FormatInvalid_IncludePropertyExpression( - expression.NodeType)); + throw new InvalidOperationException( + Resources.FormatInvalid_IncludePropertyExpression(expression.NodeType)); } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBinderFactory.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBinderFactory.cs index e17538b719..9e82eeddd6 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBinderFactory.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBinderFactory.cs @@ -7,7 +7,6 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; using System.Runtime.CompilerServices; -using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding.Internal; @@ -152,7 +151,14 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { _factory = factory; Metadata = factoryContext.Metadata; - BindingInfo = factoryContext.BindingInfo; + BindingInfo = new BindingInfo + { + BinderModelName = factoryContext.BindingInfo?.BinderModelName ?? Metadata.BinderModelName, + BinderType = factoryContext.BindingInfo?.BinderType ?? Metadata.BinderType, + BindingSource = factoryContext.BindingInfo?.BindingSource ?? Metadata.BindingSource, + PropertyFilterProvider = + factoryContext.BindingInfo?.PropertyFilterProvider ?? Metadata.PropertyFilterProvider, + }; MetadataProvider = _factory._metadataProvider; Stack = new List>(); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerArgumentBinderTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerArgumentBinderTests.cs index d0b1b5a1ba..4031263ec7 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerArgumentBinderTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerArgumentBinderTests.cs @@ -9,6 +9,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.AspNetCore.Routing; using Moq; @@ -573,6 +574,135 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal("Hello", controller.StringProperty); } + public static TheoryData BindModelAsyncData + { + get + { + var emptyBindingInfo = new BindingInfo(); + var bindingInfoWithName = new BindingInfo + { + BinderModelName = "bindingInfoName", + BinderType = typeof(Person), + }; + + // parameterBindingInfo, metadataBinderModelName, parameterName, expectedBinderModelName + return new TheoryData + { + // If the parameter name is not a prefix match, it is ignored. But name is required to create a + // ModelBindingContext. + { null, null, "parameterName", string.Empty }, + { emptyBindingInfo, null, "parameterName", string.Empty }, + { bindingInfoWithName, null, "parameterName", "bindingInfoName" }, + { null, "modelBinderName", "parameterName", "modelBinderName" }, + { null, null, "parameterName", string.Empty }, + // Parameter's BindingInfo has highest precedence + { bindingInfoWithName, "modelBinderName", "parameterName", "bindingInfoName" }, + }; + } + } + + [Theory] + [MemberData(nameof(BindModelAsyncData))] + public async Task BindModelAsync_PassesExpectedBindingInfoAndMetadata_IfPrefixDoesNotMatch( + BindingInfo parameterBindingInfo, + string metadataBinderModelName, + string parameterName, + string expectedModelName) + { + // Arrange + var metadataProvider = new TestModelMetadataProvider(); + metadataProvider.ForType().BindingDetails(binding => + { + binding.BinderModelName = metadataBinderModelName; + }); + + var metadata = metadataProvider.GetMetadataForType(typeof(Person)); + var modelBinder = new Mock(); + modelBinder + .Setup(b => b.BindModelAsync(It.IsAny())) + .Callback((ModelBindingContext context) => + { + Assert.Equal(expectedModelName, context.ModelName, StringComparer.Ordinal); + }) + .Returns(TaskCache.CompletedTask); + + var parameterDescriptor = new ParameterDescriptor + { + BindingInfo = parameterBindingInfo, + Name = parameterName, + ParameterType = typeof(Person), + }; + + var factory = new Mock(MockBehavior.Strict); + factory + .Setup(f => f.CreateBinder(It.IsAny())) + .Callback((ModelBinderFactoryContext context) => + { + // Confirm expected data is passed through to ModelBindingFactory. + Assert.Same(parameterDescriptor.BindingInfo, context.BindingInfo); + Assert.Same(parameterDescriptor, context.CacheToken); + Assert.Equal(metadata, context.Metadata); + }) + .Returns(modelBinder.Object); + + var argumentBinder = new ControllerArgumentBinder(metadataProvider, factory.Object, CreateMockValidator()); + var controllerContext = GetControllerContext(); + controllerContext.ActionDescriptor.Parameters.Add(parameterDescriptor); + + // Act & Assert + await argumentBinder.BindModelAsync(parameterDescriptor, controllerContext); + } + + [Fact] + public async Task BindModelAsync_PassesExpectedBindingInfoAndMetadata_IfPrefixMatches() + { + // Arrange + var expectedModelName = "expectedName"; + + var metadataProvider = new TestModelMetadataProvider(); + var metadata = metadataProvider.GetMetadataForType(typeof(Person)); + var modelBinder = new Mock(); + modelBinder + .Setup(b => b.BindModelAsync(It.IsAny())) + .Callback((ModelBindingContext context) => + { + Assert.Equal(expectedModelName, context.ModelName, StringComparer.Ordinal); + }) + .Returns(TaskCache.CompletedTask); + + var parameterDescriptor = new ParameterDescriptor + { + Name = expectedModelName, + ParameterType = typeof(Person), + }; + + var factory = new Mock(MockBehavior.Strict); + factory + .Setup(f => f.CreateBinder(It.IsAny())) + .Callback((ModelBinderFactoryContext context) => + { + // Confirm expected data is passed through to ModelBindingFactory. + Assert.Null(context.BindingInfo); + Assert.Same(parameterDescriptor, context.CacheToken); + Assert.Equal(metadata, context.Metadata); + }) + .Returns(modelBinder.Object); + + var argumentBinder = new ControllerArgumentBinder(metadataProvider, factory.Object, CreateMockValidator()); + var valueProvider = new SimpleValueProvider + { + { expectedModelName, new object() }, + }; + var valueProviderFactory = new SimpleValueProviderFactory(valueProvider); + + var controllerContext = GetControllerContext(); + controllerContext.ActionDescriptor.Parameters.Add(parameterDescriptor); + controllerContext.ValueProviderFactories.Insert(0, valueProviderFactory); + + // Act & Assert + await argumentBinder.BindModelAsync(parameterDescriptor, controllerContext); + } + private static ControllerContext GetControllerContext(ControllerActionDescriptor descriptor = null) { var context = new ControllerContext() diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ModelBinderFactoryTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ModelBinderFactoryTest.cs index 5a951f1ac2..c8d17b5b4c 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ModelBinderFactoryTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ModelBinderFactoryTest.cs @@ -5,6 +5,7 @@ using System; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding.Binders; using Microsoft.AspNetCore.Mvc.ModelBinding.Internal; +using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Moq; using Xunit; @@ -235,6 +236,112 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Assert.Same(result1, result2); } + public static TheoryData BindingInfoData + { + get + { + var propertyFilterProvider = Mock.Of(); + + var emptyBindingInfo = new BindingInfo(); + var halfBindingInfo = new BindingInfo + { + BinderModelName = "expected name", + BinderType = typeof(Widget), + }; + var fullBindingInfo = new BindingInfo + { + BinderModelName = "expected name", + BinderType = typeof(Widget), + BindingSource = BindingSource.Services, + PropertyFilterProvider = propertyFilterProvider, + }; + + var emptyBindingMetadata = new BindingMetadata(); + var differentBindingMetadata = new BindingMetadata + { + BinderModelName = "not the expected name", + BinderType = typeof(WidgetId), + BindingSource = BindingSource.ModelBinding, + PropertyFilterProvider = Mock.Of(), + }; + var secondHalfBindingMetadata = new BindingMetadata + { + BindingSource = BindingSource.Services, + PropertyFilterProvider = propertyFilterProvider, + }; + var fullBindingMetadata = new BindingMetadata + { + BinderModelName = "expected name", + BinderType = typeof(Widget), + BindingSource = BindingSource.Services, + PropertyFilterProvider = propertyFilterProvider, + }; + + // parameterBindingInfo, bindingMetadata, expectedInfo + return new TheoryData + { + { emptyBindingInfo, emptyBindingMetadata, emptyBindingInfo }, + { fullBindingInfo, emptyBindingMetadata, fullBindingInfo }, + { emptyBindingInfo, fullBindingMetadata, fullBindingInfo }, + // Resulting BindingInfo combines two inputs + { halfBindingInfo, secondHalfBindingMetadata, fullBindingInfo }, + // Parameter information has precedence over type metadata + { fullBindingInfo, differentBindingMetadata, fullBindingInfo }, + }; + } + } + + [Theory] + [MemberData(nameof(BindingInfoData))] + public void CreateBinder_PassesExpectedBindingInfo( + BindingInfo parameterBindingInfo, + BindingMetadata bindingMetadata, + BindingInfo expectedInfo) + { + // Arrange + var metadataProvider = new TestModelMetadataProvider(); + metadataProvider.ForType().BindingDetails(binding => + { + binding.BinderModelName = bindingMetadata.BinderModelName; + binding.BinderType = bindingMetadata.BinderType; + binding.BindingSource = bindingMetadata.BindingSource; + if (bindingMetadata.PropertyFilterProvider != null) + { + binding.PropertyFilterProvider = bindingMetadata.PropertyFilterProvider; + } + }); + + var modelBinder = Mock.Of(); + var modelBinderProvider = new TestModelBinderProvider(context => + { + Assert.Equal(typeof(Employee), context.Metadata.ModelType); + + Assert.NotNull(context.BindingInfo); + Assert.Equal(expectedInfo.BinderModelName, context.BindingInfo.BinderModelName, StringComparer.Ordinal); + Assert.Equal(expectedInfo.BinderType, context.BindingInfo.BinderType); + Assert.Equal(expectedInfo.BindingSource, context.BindingInfo.BindingSource); + Assert.Same(expectedInfo.PropertyFilterProvider, context.BindingInfo.PropertyFilterProvider); + + return modelBinder; + }); + + var options = new TestOptionsManager(); + options.Value.ModelBinderProviders.Insert(0, modelBinderProvider); + + var factory = new ModelBinderFactory(metadataProvider, options); + var factoryContext = new ModelBinderFactoryContext + { + BindingInfo = parameterBindingInfo, + Metadata = metadataProvider.GetMetadataForType(typeof(Employee)), + }; + + // Act & Assert + var result = factory.CreateBinder(factoryContext); + + // Confirm our IModelBinderProvider was called. + Assert.Same(modelBinder, result); + } + private class Widget { public WidgetId Id { get; set; } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/TestModelBinderProviderContext.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/TestModelBinderProviderContext.cs index 1478ebb3fc..d8b963e798 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/TestModelBinderProviderContext.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/TestModelBinderProviderContext.cs @@ -11,7 +11,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding // Has to be internal because TestModelMetadataProvider is 'shared' code. internal static readonly TestModelMetadataProvider CachedMetadataProvider = new TestModelMetadataProvider(); - private readonly List> _binderCreators = + private readonly List> _binderCreators = new List>(); public TestModelBinderProviderContext(Type modelType) @@ -31,7 +31,13 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public TestModelBinderProviderContext(ModelMetadata metadata, BindingInfo bindingInfo) { Metadata = metadata; - BindingInfo = bindingInfo; + BindingInfo = bindingInfo ?? new BindingInfo + { + BinderModelName = metadata.BinderModelName, + BinderType = metadata.BinderType, + BindingSource = metadata.BindingSource, + PropertyFilterProvider = metadata.PropertyFilterProvider, + }; MetadataProvider = CachedMetadataProvider; } diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BinderTypeBasedModelBinderIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BinderTypeBasedModelBinderIntegrationTest.cs index 9d14d55d7c..e766715cdd 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BinderTypeBasedModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BinderTypeBasedModelBinderIntegrationTest.cs @@ -15,7 +15,6 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests public class BinderTypeBasedModelBinderIntegrationTest { [Fact] - [InlineData(typeof(NullModelNotSetModelBinder), false)] public async Task BindParameter_WithModelBinderType_NullData_ReturnsNull() { // Arrange @@ -134,6 +133,112 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests public string Street { get; set; } } + public static TheoryData NullAndEmptyBindingInfo + { + get + { + return new TheoryData + { + null, + new BindingInfo(), + }; + } + } + + // Make sure the metadata is honored when a [ModelBinder] attribute is associated with an action parameter's + // type. This should behave identically to such an attribute on an action parameter. (Tests such as + // BindParameter_WithData_WithPrefix_GetsBound cover associating [ModelBinder] with an action parameter.) + // + // This is a regression test for aspnet/Mvc#4652 + [Theory] + [MemberData(nameof(NullAndEmptyBindingInfo))] + public async Task BinderTypeOnParameterType_WithData_EmptyPrefix_GetsBound(BindingInfo bindingInfo) + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor + { + Name = "Parameter1", + BindingInfo = bindingInfo, + ParameterType = typeof(Address), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(); + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + // ModelBindingResult + Assert.True(modelBindingResult.IsModelSet); + + // Model + var address = Assert.IsType
(modelBindingResult.Model); + Assert.Equal("SomeStreet", address.Street); + + // ModelState + Assert.True(modelState.IsValid); + var kvp = Assert.Single(modelState); + Assert.Equal("Street", kvp.Key); + var entry = kvp.Value; + Assert.NotNull(entry); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + Assert.NotNull(entry.RawValue); // Value is set by test model binder, no need to validate it. + } + + private class Person3 + { + [ModelBinder(BinderType = typeof(Address3ModelBinder))] + public Address3 Address { get; set; } + } + + private class Address3 + { + public string Street { get; set; } + } + + // Make sure the metadata is honored when a [ModelBinder] attribute is associated with a property in the type + // hierarchy of an action parameter. (Tests such as BindProperty_WithData_EmptyPrefix_GetsBound cover + // associating [ModelBinder] with a class somewhere in the type hierarchy of an action parameter.) + [Theory] + [MemberData(nameof(NullAndEmptyBindingInfo))] + public async Task BinderTypeOnProperty_WithData_EmptyPrefix_GetsBound(BindingInfo bindingInfo) + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor + { + Name = "Parameter1", + BindingInfo = bindingInfo, + ParameterType = typeof(Person3), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(); + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + // ModelBindingResult + Assert.True(modelBindingResult.IsModelSet); + + // Model + var person = Assert.IsType(modelBindingResult.Model); + Assert.NotNull(person.Address); + Assert.Equal("SomeStreet", person.Address.Street); + + // ModelState + Assert.True(modelState.IsValid); + var kvp = Assert.Single(modelState); + Assert.Equal("Address.Street", kvp.Key); + var entry = kvp.Value; + Assert.NotNull(entry); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + Assert.NotNull(entry.RawValue); // Value is set by test model binder, no need to validate it. + } + [Fact] public async Task BindProperty_WithData_EmptyPrefix_GetsBound() { @@ -237,6 +342,34 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests } } + private class Address3ModelBinder : IModelBinder + { + public Task BindModelAsync(ModelBindingContext bindingContext) + { + if (bindingContext == null) + { + throw new ArgumentNullException(nameof(bindingContext)); + } + + Debug.Assert(bindingContext.Result == ModelBindingResult.Failed()); + + if (bindingContext.ModelType != typeof(Address3)) + { + return TaskCache.CompletedTask; + } + + var address = new Address3 { Street = "SomeStreet" }; + + bindingContext.ModelState.SetModelValue( + ModelNames.CreatePropertyModelName(bindingContext.ModelName, "Street"), + new string[] { address.Street }, + address.Street); + + bindingContext.Result = ModelBindingResult.Success(address); + return TaskCache.CompletedTask; + } + } + private class SuccessModelBinder : IModelBinder { public Task BindModelAsync(ModelBindingContext bindingContext) diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs index fda4489d54..236ad49635 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs @@ -1,6 +1,7 @@ // 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.ComponentModel.DataAnnotations; using System.IO; @@ -15,18 +16,6 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests { public class BodyValidationIntegrationTests { - private class Person - { - [FromBody] - [Required] - public Address Address { get; set; } - } - - private class Address - { - public string Street { get; set; } - } - [Fact] public async Task ModelMetadataTypeAttribute_ValidBaseClass_NoModelStateErrors() { @@ -354,6 +343,18 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.Equal("Product must be made in the USA if it is not named.", modelStateErrors[""]); } + private class Person + { + [FromBody] + [Required] + public Address Address { get; set; } + } + + private class Address + { + public string Street { get; set; } + } + [Fact] public async Task FromBodyAndRequiredOnProperty_EmptyBody_AddsModelStateError() { @@ -690,6 +691,107 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.Empty(modelState); } + private class Person6 + { + public Address6 Address { get; set; } + } + + private class Address6 + { + public string Street { get; set; } + } + + // [FromBody] cannot be associated with a type. But a [FromBody] or [ModelBinder] subclass or custom + // IBindingSourceMetadata implementation might not have the same restriction. Make sure the metadata is honored + // when such an attribute is associated with a class somewhere in the type hierarchy of an action parameter. + [Theory] + [MemberData( + nameof(BinderTypeBasedModelBinderIntegrationTest.NullAndEmptyBindingInfo), + MemberType = typeof(BinderTypeBasedModelBinderIntegrationTest))] + public async Task FromBodyOnPropertyType_WithData_Succeeds(BindingInfo bindingInfo) + { + // Arrange + var inputText = "{ \"Street\" : \"someStreet\" }"; + var metadataProvider = new TestModelMetadataProvider(); + metadataProvider + .ForProperty(nameof(Person6.Address)) + .BindingDetails(binding => binding.BindingSource = BindingSource.Body); + + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(metadataProvider); + var parameter = new ParameterDescriptor + { + Name = "parameter-name", + BindingInfo = bindingInfo, + ParameterType = typeof(Person6), + }; + + var testContext = ModelBindingTestHelper.GetTestContext( + request => + { + request.Body = new MemoryStream(Encoding.UTF8.GetBytes(inputText)); + request.ContentType = "application/json"; + }); + testContext.MetadataProvider = metadataProvider; + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + var person = Assert.IsType(modelBindingResult.Model); + Assert.NotNull(person.Address); + Assert.Equal("someStreet", person.Address.Street, StringComparer.Ordinal); + + Assert.True(modelState.IsValid); + Assert.Empty(modelState); + } + + // [FromBody] cannot be associated with a type. But a [FromBody] or [ModelBinder] subclass or custom + // IBindingSourceMetadata implementation might not have the same restriction. Make sure the metadata is honored + // when such an attribute is associated with an action parameter's type. + [Theory] + [MemberData( + nameof(BinderTypeBasedModelBinderIntegrationTest.NullAndEmptyBindingInfo), + MemberType = typeof(BinderTypeBasedModelBinderIntegrationTest))] + public async Task FromBodyOnParameterType_WithData_Succeeds(BindingInfo bindingInfo) + { + // Arrange + var inputText = "{ \"Street\" : \"someStreet\" }"; + var metadataProvider = new TestModelMetadataProvider(); + metadataProvider + .ForType() + .BindingDetails(binding => binding.BindingSource = BindingSource.Body); + + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(metadataProvider); + var parameter = new ParameterDescriptor + { + Name = "parameter-name", + BindingInfo = bindingInfo, + ParameterType = typeof(Address6), + }; + + var testContext = ModelBindingTestHelper.GetTestContext( + request => + { + request.Body = new MemoryStream(Encoding.UTF8.GetBytes(inputText)); + request.ContentType = "application/json"; + }); + testContext.MetadataProvider = metadataProvider; + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + var address = Assert.IsType(modelBindingResult.Model); + Assert.Equal("someStreet", address.Street, StringComparer.Ordinal); + + Assert.True(modelState.IsValid); + Assert.Empty(modelState); + } + private Dictionary CreateValidationDictionary(ModelStateDictionary modelState) { var result = new Dictionary(); diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs index f1c6fa3fcd..cd7adce382 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs @@ -154,7 +154,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests { public IModelBinder GetBinder(ModelBinderProviderContext context) { - var allowedBindingSource = context.BindingInfo?.BindingSource; + var allowedBindingSource = context.BindingInfo.BindingSource; if (allowedBindingSource?.CanAcceptDataFrom(BindAddressAttribute.Source) == true) { // Binding Sources are opt-in. This model either didn't specify one or specified something diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs index 32099b28c4..6828216779 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs @@ -1932,6 +1932,200 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.False(modelState.IsValid); } + private class Person12 + { + public Address12 Address { get; set; } + } + + [ModelBinder(Name = "HomeAddress")] + private class Address12 + { + public string Street { get; set; } + } + + // Make sure the metadata is honored when a [ModelBinder] attribute is associated with a class somewhere in the + // type hierarchy of an action parameter. This should behave identically to such an attribute on a property in + // the type hierarchy. + [Theory] + [MemberData( + nameof(BinderTypeBasedModelBinderIntegrationTest.NullAndEmptyBindingInfo), + MemberType = typeof(BinderTypeBasedModelBinderIntegrationTest))] + public async Task ModelNameOnPropertyType_WithData_Succeeds(BindingInfo bindingInfo) + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor + { + Name = "parameter-name", + BindingInfo = bindingInfo, + ParameterType = typeof(Person12), + }; + + var testContext = ModelBindingTestHelper.GetTestContext( + request => request.QueryString = new QueryString("?HomeAddress.Street=someStreet")); + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + var person = Assert.IsType(modelBindingResult.Model); + Assert.NotNull(person.Address); + Assert.Equal("someStreet", person.Address.Street, StringComparer.Ordinal); + + Assert.True(modelState.IsValid); + var kvp = Assert.Single(modelState); + Assert.Equal("HomeAddress.Street", kvp.Key); + var entry = kvp.Value; + Assert.NotNull(entry); + Assert.Empty(entry.Errors); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + } + + // Make sure the metadata is honored when a [ModelBinder] attribute is associated with an action parameter's + // type. This should behave identically to such an attribute on an action parameter. + [Theory] + [MemberData( + nameof(BinderTypeBasedModelBinderIntegrationTest.NullAndEmptyBindingInfo), + MemberType = typeof(BinderTypeBasedModelBinderIntegrationTest))] + public async Task ModelNameOnParameterType_WithData_Succeeds(BindingInfo bindingInfo) + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor + { + Name = "parameter-name", + BindingInfo = bindingInfo, + ParameterType = typeof(Address12), + }; + + var testContext = ModelBindingTestHelper.GetTestContext( + request => request.QueryString = new QueryString("?HomeAddress.Street=someStreet")); + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + var address = Assert.IsType(modelBindingResult.Model); + Assert.Equal("someStreet", address.Street, StringComparer.Ordinal); + + Assert.True(modelState.IsValid); + var kvp = Assert.Single(modelState); + Assert.Equal("HomeAddress.Street", kvp.Key); + var entry = kvp.Value; + Assert.NotNull(entry); + Assert.Empty(entry.Errors); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + } + + private class Person13 + { + public Address13 Address { get; set; } + } + + [Bind("Street")] + private class Address13 + { + public int Number { get; set; } + + public string Street { get; set; } + + public string City { get; set; } + + public string State { get; set; } + } + + // Make sure the metadata is honored when a [Bind] attribute is associated with a class somewhere in the type + // hierarchy of an action parameter. This should behave identically to such an attribute on a property in the + // type hierarchy. (Test is similar to ModelNameOnPropertyType_WithData_Succeeds() but covers implementing + // IPropertyFilterProvider, not IModelNameProvider.) + [Theory] + [MemberData( + nameof(BinderTypeBasedModelBinderIntegrationTest.NullAndEmptyBindingInfo), + MemberType = typeof(BinderTypeBasedModelBinderIntegrationTest))] + public async Task BindAttributeOnPropertyType_WithData_Succeeds(BindingInfo bindingInfo) + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor + { + Name = "parameter-name", + BindingInfo = bindingInfo, + ParameterType = typeof(Person13), + }; + + var testContext = ModelBindingTestHelper.GetTestContext( + request => request.QueryString = new QueryString( + "?Address.Number=23&Address.Street=someStreet&Address.City=Redmond&Address.State=WA")); + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + var person = Assert.IsType(modelBindingResult.Model); + Assert.NotNull(person.Address); + Assert.Null(person.Address.City); + Assert.Equal(0, person.Address.Number); + Assert.Null(person.Address.State); + Assert.Equal("someStreet", person.Address.Street, StringComparer.Ordinal); + + Assert.True(modelState.IsValid); + var kvp = Assert.Single(modelState); + Assert.Equal("Address.Street", kvp.Key); + var entry = kvp.Value; + Assert.NotNull(entry); + Assert.Empty(entry.Errors); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + } + + // Make sure the metadata is honored when a [Bind] attribute is associated with an action parameter's type. + // This should behave identically to such an attribute on an action parameter. (Test is similar + // to ModelNameOnParameterType_WithData_Succeeds() but covers implementing IPropertyFilterProvider, not + // IModelNameProvider.) + [Theory] + [MemberData( + nameof(BinderTypeBasedModelBinderIntegrationTest.NullAndEmptyBindingInfo), + MemberType = typeof(BinderTypeBasedModelBinderIntegrationTest))] + public async Task BindAttributeOnParameterType_WithData_Succeeds(BindingInfo bindingInfo) + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor + { + Name = "parameter-name", + BindingInfo = bindingInfo, + ParameterType = typeof(Address13), + }; + + var testContext = ModelBindingTestHelper.GetTestContext( + request => request.QueryString = new QueryString("?Number=23&Street=someStreet&City=Redmond&State=WA")); + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + var address = Assert.IsType(modelBindingResult.Model); + Assert.Null(address.City); + Assert.Equal(0, address.Number); + Assert.Null(address.State); + Assert.Equal("someStreet", address.Street, StringComparer.Ordinal); + + Assert.True(modelState.IsValid); + var kvp = Assert.Single(modelState); + Assert.Equal("Street", kvp.Key); + var entry = kvp.Value; + Assert.NotNull(entry); + Assert.Empty(entry.Errors); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + } + private static void SetJsonBodyContent(HttpRequest request, string content) { var stream = new MemoryStream(new UTF8Encoding(encoderShouldEmitUTF8Identifier: false).GetBytes(content)); diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs index 2f467e96ea..50567879e2 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs @@ -183,5 +183,89 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests () => argumentBinder.BindModelAsync(parameter, testContext)); Assert.Contains(typeof(IActionResult).FullName, exception.Message); } + + private class Person + { + public JsonOutputFormatter Service { get; set; } + } + + // [FromServices] cannot be associated with a type. But a [FromServices] or [ModelBinder] subclass or custom + // IBindingSourceMetadata implementation might not have the same restriction. Make sure the metadata is honored + // when such an attribute is associated with a type somewhere in the type hierarchy of an action parameter. + [Theory] + [MemberData( + nameof(BinderTypeBasedModelBinderIntegrationTest.NullAndEmptyBindingInfo), + MemberType = typeof(BinderTypeBasedModelBinderIntegrationTest))] + public async Task FromServicesOnPropertyType_WithData_Succeeds(BindingInfo bindingInfo) + { + // Arrange + // Similar to a custom IBindingSourceMetadata implementation or [ModelBinder] subclass on a custom service. + var metadataProvider = new TestModelMetadataProvider(); + metadataProvider + .ForProperty(nameof(Person.Service)) + .BindingDetails(binding => binding.BindingSource = BindingSource.Services); + + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(metadataProvider); + var parameter = new ParameterDescriptor + { + Name = "parameter-name", + BindingInfo = bindingInfo, + ParameterType = typeof(Person), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(); + testContext.MetadataProvider = metadataProvider; + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + var person = Assert.IsType(modelBindingResult.Model); + Assert.NotNull(person.Service); + + Assert.True(modelState.IsValid); + Assert.Empty(modelState); + } + + // [FromServices] cannot be associated with a type. But a [FromServices] or [ModelBinder] subclass or custom + // IBindingSourceMetadata implementation might not have the same restriction. Make sure the metadata is honored + // when such an attribute is associated with an action parameter's type. + [Theory] + [MemberData( + nameof(BinderTypeBasedModelBinderIntegrationTest.NullAndEmptyBindingInfo), + MemberType = typeof(BinderTypeBasedModelBinderIntegrationTest))] + public async Task FromServicesOnParameterType_WithData_Succeeds(BindingInfo bindingInfo) + { + // Arrange + // Similar to a custom IBindingSourceMetadata implementation or [ModelBinder] subclass on a custom service. + var metadataProvider = new TestModelMetadataProvider(); + metadataProvider + .ForType() + .BindingDetails(binding => binding.BindingSource = BindingSource.Services); + + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(metadataProvider); + var parameter = new ParameterDescriptor + { + Name = "parameter-name", + BindingInfo = bindingInfo, + ParameterType = typeof(JsonOutputFormatter), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(); + testContext.MetadataProvider = metadataProvider; + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + Assert.IsType(modelBindingResult.Model); + + Assert.True(modelState.IsValid); + Assert.Empty(modelState); + } } } \ No newline at end of file