Use type metadata for ModelBinderProviderContext.BindingInfo

- #4652 - previously ignored for top-level models - `ModelBinderProviderContext.BindingInfo` is now never `null` - similarly, use type metadata (as well as parameter info) for `ModelBindingContext.BinderModelName` - previously ignored when overridden in `ControllerArgumentBinder`
aspnet · May 28, 2016 · e5732c5 · e5732c5
1 parent 9963359
commit e5732c5
Show file tree

Hide file tree

Showing 13 changed files with 445 additions and 23 deletions.
diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelBinderProviderContext.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelBinderProviderContext.cs
@@ -16,7 +16,7 @@ public abstract class ModelBinderProviderContext
         public abstract IModelBinder CreateBinder(ModelMetadata metadata);
 
         /// <summary>
-        /// Gets the <see cref="BindingInfo"/>. May be <c>null</c>.
+        /// Gets the <see cref="BindingInfo"/>.
         /// </summary>
         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
@@ -163,10 +163,11 @@ public async Task<ModelBindingResult> BindModelAsync(
                 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))
             {

diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BinderTypeModelBinderProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BinderTypeModelBinderProvider.cs
@@ -19,7 +19,7 @@ public IModelBinder GetBinder(ModelBinderProviderContext context)
                 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
@@ -45,7 +45,7 @@ public IModelBinder GetBinder(ModelBinderProviderContext context)
                 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
@@ -18,7 +18,7 @@ public IModelBinder GetBinder(ModelBinderProviderContext context)
                 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
@@ -18,7 +18,7 @@ public IModelBinder GetBinder(ModelBinderProviderContext context)
                 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/ModelBinderFactory.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBinderFactory.cs
@@ -152,7 +152,14 @@ public DefaultModelBinderProviderContext(
             {
                 _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<KeyValuePair<Key, PlaceholderBinder>>();

diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/TestModelBinderProviderContext.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/TestModelBinderProviderContext.cs
@@ -11,7 +11,7 @@ public class TestModelBinderProviderContext : ModelBinderProviderContext
         // Has to be internal because TestModelMetadataProvider is 'shared' code.
         internal static readonly TestModelMetadataProvider CachedMetadataProvider = new TestModelMetadataProvider();
 
-        private readonly List<Func<ModelMetadata, IModelBinder>> _binderCreators = 
+        private readonly List<Func<ModelMetadata, IModelBinder>> _binderCreators =
             new List<Func<ModelMetadata, IModelBinder>>();
 
         public TestModelBinderProviderContext(Type modelType)
@@ -31,7 +31,13 @@ public TestModelBinderProviderContext(Type modelType)
         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
@@ -134,6 +134,55 @@ private class Address
             public string Street { get; set; }
         }
 
+        public static TheoryData<BindingInfo> NullAndEmptyBindingInfo
+        {
+            get
+            {
+                return new TheoryData<BindingInfo>
+                {
+                    null,
+                    new BindingInfo(),
+                };
+            }
+        }
+
+        [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<Address>(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.
+        }
+
         [Fact]
         public async Task BindProperty_WithData_EmptyPrefix_GetsBound()
         {

diff --git 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 @@ public async Task ModelMetadataTypeAttribute_InvalidClassAttributeOnBaseClassPro
             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,105 @@ public async Task FromBodyOnProperty_Succeeds_IgnoresRequiredOnValueTypeSubPrope
             Assert.Empty(modelState);
         }
 
+        private class Person6
+        {
+            public Address6 Address { get; set; }
+        }
+
+        private class Address6
+        {
+            public string Street { get; set; }
+        }
+
+        [Theory]
+        [MemberData(
+            nameof(BinderTypeBasedModelBinderIntegrationTest.NullAndEmptyBindingInfo),
+            MemberType = typeof(BinderTypeBasedModelBinderIntegrationTest))]
+        public async Task FromBodyOnPropertyType_WithData_Succeeds(BindingInfo bindingInfo)
+        {
+            // Arrange
+            var inputText = "{ \"Street\" : \"someStreet\" }";
+
+            // Similar to a custom IBindingSourceMetadata implementation or [ModelBinder] subclass.
+            var metadataProvider = new TestModelMetadataProvider();
+            metadataProvider
+                .ForProperty<Person6>(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<Person6>(modelBindingResult.Model);
+            Assert.NotNull(person.Address);
+            Assert.Equal("someStreet", person.Address.Street, StringComparer.Ordinal);
+
+            Assert.True(modelState.IsValid);
+            Assert.Empty(modelState);
+        }
+
+        [Theory]
+        [MemberData(
+            nameof(BinderTypeBasedModelBinderIntegrationTest.NullAndEmptyBindingInfo),
+            MemberType = typeof(BinderTypeBasedModelBinderIntegrationTest))]
+        public async Task FromBodyOnParameterType_WithData_Succeeds(BindingInfo bindingInfo)
+        {
+            // Arrange
+            var inputText = "{ \"Street\" : \"someStreet\" }";
+
+            // Similar to a custom IBindingSourceMetadata implementation or [ModelBinder] subclass.
+            var metadataProvider = new TestModelMetadataProvider();
+            metadataProvider
+                .ForType<Address6>()
+                .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<Address6>(modelBindingResult.Model);
+            Assert.Equal("someStreet", address.Street, StringComparer.Ordinal);
+
+            Assert.True(modelState.IsValid);
+            Assert.Empty(modelState);
+        }
+
         private Dictionary<string, string> CreateValidationDictionary(ModelStateDictionary modelState)
         {
             var result = new Dictionary<string, string>();

diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs
@@ -154,7 +154,7 @@ private class AddressBinderProvider : IModelBinderProvider
         {
             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