diff --git a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingContext.cs b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingContext.cs index ac06ef741f..1baacfe984 100644 --- a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingContext.cs +++ b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingContext.cs @@ -174,8 +174,31 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// Gets or sets a value that indicates whether the binder should use an empty prefix to look up /// values in when no values are found using the prefix. /// + /// + /// Passed into the model binding system. Should not be true when is + /// false. + /// public bool FallbackToEmptyPrefix { get; set; } + /// + /// Gets or sets an indication that the current binder is handling the top-level object. + /// + /// Passed into the model binding system. + public bool IsTopLevelObject { get; set; } + + /// + /// Gets or sets an indication that the model binding system will make another binding attempt (e.g. fall back + /// to the empty prefix) after this one. + /// + /// + /// Not passed into the model binding system but instead set by the top-level binder. With built-in binders, + /// true only in binders called directly from a + /// Microsoft.AspNet.Mvc.ModelBinding.CompositeModelBinder that was passed a + /// with true. false + /// otherwise. + /// + public bool IsFirstChanceBinding { get; set; } + /// /// Gets or sets the associated with this context. /// diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs index a8b71f9e84..896f10d833 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs @@ -196,6 +196,7 @@ namespace Microsoft.AspNet.Mvc bindingInfo, parameterName); + modelBindingContext.IsTopLevelObject = true; modelBindingContext.ModelState = modelState; modelBindingContext.ValueProvider = operationBindingContext.ValueProvider; modelBindingContext.OperationBindingContext = operationBindingContext; diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs index 3261747428..07ceecff7e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs @@ -30,8 +30,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // For compatibility with MVC 5.0 for top level object we want to consider an empty key instead of // the parameter name/a custom name. In all other cases (like when binding body to a property) we // consider the entire ModelName as a prefix. - var isTopLevelObject = bindingContext.ModelMetadata.ContainerType == null; - var modelBindingKey = isTopLevelObject ? string.Empty : bindingContext.ModelName; + var modelBindingKey = bindingContext.IsTopLevelObject ? string.Empty : bindingContext.ModelName; var httpContext = bindingContext.OperationBindingContext.HttpContext; diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs index fe4ffca3ac..7992c02c5e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs @@ -27,12 +27,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding if (!await bindingContext.ValueProvider.ContainsPrefixAsync(bindingContext.ModelName)) { - // If this is the fallback case, and we failed to find data as a top-level model, then generate a + // If this is the fallback case and we failed to find data for a top-level model, then generate a // default 'empty' model and return it. - var isTopLevelObject = bindingContext.ModelMetadata.ContainerType == null; - var hasExplicitAlias = bindingContext.BinderModelName != null; - - if (isTopLevelObject && (hasExplicitAlias || bindingContext.ModelName == string.Empty)) + if (!bindingContext.IsFirstChanceBinding && bindingContext.IsTopLevelObject) { model = CreateEmptyCollection(); diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs index 8272a1a0dc..f759776be4 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs @@ -34,12 +34,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public virtual async Task BindModelAsync([NotNull] ModelBindingContext bindingContext) { + // Will there be a last chance (fallback) binding attempt? + var isFirstChanceBinding = bindingContext.FallbackToEmptyPrefix && + !string.IsNullOrEmpty(bindingContext.ModelName); + var newBindingContext = CreateNewBindingContext(bindingContext, bindingContext.ModelName); + newBindingContext.IsFirstChanceBinding = isFirstChanceBinding; var modelBindingResult = await TryBind(newBindingContext); - if (modelBindingResult == null && - bindingContext.FallbackToEmptyPrefix && - !string.IsNullOrEmpty(bindingContext.ModelName)) + if (modelBindingResult == null && isFirstChanceBinding) { // Fall back to empty prefix. newBindingContext = CreateNewBindingContext(bindingContext, modelName: string.Empty); @@ -142,6 +145,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding BinderModelName = oldBindingContext.BinderModelName, BindingSource = oldBindingContext.BindingSource, BinderType = oldBindingContext.BinderType, + IsTopLevelObject = oldBindingContext.IsTopLevelObject, }; newBindingContext.OperationBindingContext.BodyBindingState = GetBodyBindingState(oldBindingContext); diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/KeyValuePairModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/KeyValuePairModelBinder.cs index 62ebdf047f..f0b4321a76 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/KeyValuePairModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/KeyValuePairModelBinder.cs @@ -62,12 +62,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } else { - // If this is the fallback case, and we failed to find data as a top-level model, then generate a + // If this is the fallback case and we failed to find data for a top-level model, then generate a // default 'empty' model and return it. - var isTopLevelObject = bindingContext.ModelMetadata.ContainerType == null; - var hasExplicitAlias = bindingContext.BinderModelName != null; - - if (isTopLevelObject && (hasExplicitAlias || bindingContext.ModelName == string.Empty)) + if (!bindingContext.IsFirstChanceBinding && bindingContext.IsTopLevelObject) { var model = new KeyValuePair(); diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs index dcdcbac981..e8eb037768 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs @@ -71,9 +71,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding internal async Task CanCreateModel(MutableObjectBinderContext context) { var bindingContext = context.ModelBindingContext; - - var isTopLevelObject = bindingContext.ModelMetadata.ContainerType == null; - var hasExplicitAlias = bindingContext.BinderModelName != null; + var isTopLevelObject = bindingContext.IsTopLevelObject; // If we get here the model is a complex object which was not directly bound by any previous model binder, // so we want to decide if we want to continue binding. This is important to get right to avoid infinite @@ -83,34 +81,24 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // will always include value provider data. For instance if the model is marked with [FromBody], then we // can just skip it. A greedy source cannot be a value provider. // - // If the model isn't marked with ANY binding source, then we assume it's ok also. + // If the model isn't marked with ANY binding source, then we assume it's OK also. // // We skip this check if it is a top level object because we want to always evaluate // the creation of top level object (this is also required for ModelBinderAttribute to work.) var bindingSource = bindingContext.BindingSource; - if (!isTopLevelObject && - bindingSource != null && - bindingSource.IsGreedy) + if (!isTopLevelObject && bindingSource != null && bindingSource.IsGreedy) { return false; } - // Create the object if : - // 1. It is a top level model with an explicit user supplied prefix. - // In this case since it will never fallback to empty prefix, we need to create the model here. - if (isTopLevelObject && hasExplicitAlias) + // Create the object if: + // 1. It is a top level model and no later fallback (to empty prefix) will occur. + if (isTopLevelObject && !bindingContext.IsFirstChanceBinding) { return true; } - // 2. It is a top level object and there is no model name ( Fallback to empty prefix case ). - // This is necessary as we do not want to depend on a value provider to contain an empty prefix. - if (isTopLevelObject && bindingContext.ModelName == string.Empty) - { - return true; - } - - // 3. Any of the model properties can be bound using a value provider. + // 2. Any of the model properties can be bound using a value provider. if (await CanValueBindAnyModelProperties(context)) { return true; diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBindingHelper.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBindingHelper.cs index 8deb7d4947..804de334f7 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBindingHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBindingHelper.cs @@ -2,10 +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; using System.Collections.Generic; -using System.Diagnostics; -using System.Globalization; using System.Linq; using System.Linq.Expressions; using System.Reflection; @@ -303,8 +300,9 @@ namespace Microsoft.AspNet.Mvc ModelState = modelState, ValueProvider = valueProvider, FallbackToEmptyPrefix = true, + IsTopLevelObject = true, OperationBindingContext = operationBindingContext, - PropertyFilter = predicate + PropertyFilter = predicate, }; var modelBindingResult = await modelBinder.BindModelAsync(modelBindingContext); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/ArrayModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/ArrayModelBinderTest.cs index 86c7752e51..f05bfbd649 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/ArrayModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/ArrayModelBinderTest.cs @@ -37,13 +37,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test } [Fact] - public async Task ArrayModelBinder_DoesNotCreateCollection_ForTopLevelModel_OnFirstPass() + public async Task ArrayModelBinder_DoesNotCreateCollection_IfIsTopLevelObjectAndIsFirstChanceBinding() { // Arrange var binder = new ArrayModelBinder(); var context = CreateContext(); - context.ModelName = "param"; + context.IsFirstChanceBinding = true; + context.IsTopLevelObject = true; + + // Explicit prefix and empty model name both ignored. + context.BinderModelName = "prefix"; + context.ModelName = string.Empty; var metadataProvider = context.OperationBindingContext.MetadataProvider; context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(string[])); @@ -58,13 +63,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test } [Fact] - public async Task ArrayModelBinder_CreatesEmptyCollection_ForTopLevelModel_OnFallback() + public async Task ArrayModelBinder_CreatesEmptyCollection_IfIsTopLevelObjectAndNotIsFirstChanceBinding() { // Arrange var binder = new ArrayModelBinder(); var context = CreateContext(); - context.ModelName = string.Empty; + context.IsTopLevelObject = true; + + // Lack of prefix and non-empty model name both ignored. + context.ModelName = "modelName"; var metadataProvider = context.OperationBindingContext.MetadataProvider; context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(string[])); @@ -78,37 +86,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test Assert.NotNull(result); Assert.Empty(Assert.IsType(result.Model)); - Assert.Equal(string.Empty, result.Key); - Assert.True(result.IsModelSet); - - Assert.Same(result.ValidationNode.Model, result.Model); - Assert.Same(result.ValidationNode.Key, result.Key); - Assert.Same(result.ValidationNode.ModelMetadata, context.ModelMetadata); - } - - [Fact] - public async Task ArrayModelBinder_CreatesEmptyCollection_ForTopLevelModel_WithExplicitPrefix() - { - // Arrange - var binder = new ArrayModelBinder(); - - var context = CreateContext(); - context.ModelName = "prefix"; - context.BinderModelName = "prefix"; - - var metadataProvider = context.OperationBindingContext.MetadataProvider; - context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(string[])); - - context.ValueProvider = new TestValueProvider(new Dictionary()); - - // Act - var result = await binder.BindModelAsync(context); - - // Assert - Assert.NotNull(result); - - Assert.Empty(Assert.IsType(result.Model)); - Assert.Equal("prefix", result.Key); + Assert.Equal("modelName", result.Key); Assert.True(result.IsModelSet); Assert.Same(result.ValidationNode.Model, result.Model); @@ -119,7 +97,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test [Theory] [InlineData("")] [InlineData("param")] - public async Task ArrayModelBinder_DoesNotCreateCollection_ForNonTopLevelModel(string prefix) + public async Task ArrayModelBinder_DoesNotCreateCollection_IfNotIsTopLevelObject(string prefix) { // Arrange var binder = new ArrayModelBinder(); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BodyModelBinderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BodyModelBinderTests.cs index 646520d35a..b2f5f3c0ef 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BodyModelBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BodyModelBinderTests.cs @@ -10,7 +10,6 @@ using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Http.Internal; using Microsoft.AspNet.Routing; -using Microsoft.Framework.DependencyInjection; using Microsoft.Net.Http.Headers; using Moq; using Xunit; @@ -279,6 +278,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var bindingContext = new ModelBindingContext { + IsTopLevelObject = true, ModelMetadata = metadataProvider.GetMetadataForType(modelType), ModelName = "someName", ValueProvider = Mock.Of(), diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs index dbde65ee76..8b80636b7c 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs @@ -218,13 +218,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test } [Fact] - public async Task CollectionModelBinder_DoesNotCreateCollection_ForTopLevelModel_OnFirstPass() + public async Task CollectionModelBinder_DoesNotCreateCollection_IfIsTopLevelObjectAndIsFirstChanceBinding() { // Arrange var binder = new CollectionModelBinder(); var context = CreateContext(); - context.ModelName = "param"; + context.IsTopLevelObject = true; + context.IsFirstChanceBinding = true; + + // Explicit prefix and empty model name both ignored. + context.BinderModelName = "prefix"; + context.ModelName = string.Empty; var metadataProvider = context.OperationBindingContext.MetadataProvider; context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(List)); @@ -239,13 +244,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test } [Fact] - public async Task CollectionModelBinder_CreatesEmptyCollection_ForTopLevelModel_OnFallback() + public async Task CollectionModelBinder_CreatesEmptyCollection_IfIsTopLevelObjectAndNotIsFirstChanceBinding() { // Arrange var binder = new CollectionModelBinder(); var context = CreateContext(); - context.ModelName = string.Empty; + context.IsTopLevelObject = true; + + // Lack of prefix and non-empty model name both ignored. + context.ModelName = "modelName"; var metadataProvider = context.OperationBindingContext.MetadataProvider; context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(List)); @@ -259,37 +267,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test Assert.NotNull(result); Assert.Empty(Assert.IsType>(result.Model)); - Assert.Equal(string.Empty, result.Key); - Assert.True(result.IsModelSet); - - Assert.Same(result.ValidationNode.Model, result.Model); - Assert.Same(result.ValidationNode.Key, result.Key); - Assert.Same(result.ValidationNode.ModelMetadata, context.ModelMetadata); - } - - [Fact] - public async Task CollectionModelBinder_CreatesEmptyCollection_ForTopLevelModel_WithExplicitPrefix() - { - // Arrange - var binder = new CollectionModelBinder(); - - var context = CreateContext(); - context.ModelName = "prefix"; - context.BinderModelName = "prefix"; - - var metadataProvider = context.OperationBindingContext.MetadataProvider; - context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(List)); - - context.ValueProvider = new TestValueProvider(new Dictionary()); - - // Act - var result = await binder.BindModelAsync(context); - - // Assert - Assert.NotNull(result); - - Assert.Empty(Assert.IsType>(result.Model)); - Assert.Equal("prefix", result.Key); + Assert.Equal("modelName", result.Key); Assert.True(result.IsModelSet); Assert.Same(result.ValidationNode.Model, result.Model); @@ -300,7 +278,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test [Theory] [InlineData("")] [InlineData("param")] - public async Task CollectionModelBinder_DoesNotCreateCollection_ForNonTopLevelModel(string prefix) + public async Task CollectionModelBinder_DoesNotCreateCollection_IfNotIsTopLevelObject(string prefix) { // Arrange var binder = new CollectionModelBinder(); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/DictionaryModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/DictionaryModelBinderTest.cs index 4ad8d02f32..b20d8c46f8 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/DictionaryModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/DictionaryModelBinderTest.cs @@ -65,13 +65,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test } [Fact] - public async Task DictionaryModelBinder_DoesNotCreateCollection_ForTopLevelModel_OnFirstPass() + public async Task DictionaryModelBinder_DoesNotCreateCollection_IfIsTopLevelObjectAndIsFirstChanceBinding() { // Arrange var binder = new DictionaryModelBinder(); var context = CreateContext(); - context.ModelName = "param"; + context.IsTopLevelObject = true; + context.IsFirstChanceBinding = true; + + // Explicit prefix and empty model name both ignored. + context.BinderModelName = "prefix"; + context.ModelName = string.Empty; var metadataProvider = context.OperationBindingContext.MetadataProvider; context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(Dictionary)); @@ -86,13 +91,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test } [Fact] - public async Task DictionaryModelBinder_CreatesEmptyCollection_ForTopLevelModel_OnFallback() + public async Task DictionaryModelBinder_CreatesEmptyCollection_IfIsTopLevelObjectAndNotIsFirstChanceBinding() { // Arrange var binder = new DictionaryModelBinder(); var context = CreateContext(); - context.ModelName = string.Empty; + context.IsTopLevelObject = true; + + // Lack of prefix and non-empty model name both ignored. + context.ModelName = "modelName"; var metadataProvider = context.OperationBindingContext.MetadataProvider; context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(Dictionary)); @@ -106,37 +114,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test Assert.NotNull(result); Assert.Empty(Assert.IsType>(result.Model)); - Assert.Equal(string.Empty, result.Key); - Assert.True(result.IsModelSet); - - Assert.Same(result.ValidationNode.Model, result.Model); - Assert.Same(result.ValidationNode.Key, result.Key); - Assert.Same(result.ValidationNode.ModelMetadata, context.ModelMetadata); - } - - [Fact] - public async Task DictionaryModelBinder_CreatesEmptyCollection_ForTopLevelModel_WithExplicitPrefix() - { - // Arrange - var binder = new DictionaryModelBinder(); - - var context = CreateContext(); - context.ModelName = "prefix"; - context.BinderModelName = "prefix"; - - var metadataProvider = context.OperationBindingContext.MetadataProvider; - context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(Dictionary)); - - context.ValueProvider = new TestValueProvider(new Dictionary()); - - // Act - var result = await binder.BindModelAsync(context); - - // Assert - Assert.NotNull(result); - - Assert.Empty(Assert.IsType>(result.Model)); - Assert.Equal("prefix", result.Key); + Assert.Equal("modelName", result.Key); Assert.True(result.IsModelSet); Assert.Same(result.ValidationNode.Model, result.Model); @@ -147,7 +125,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test [Theory] [InlineData("")] [InlineData("param")] - public async Task DictionaryModelBinder_DoesNotCreateCollection_ForNonTopLevelModel(string prefix) + public async Task DictionaryModelBinder_DoesNotCreateCollection_IfNotIsTopLevelObject(string prefix) { // Arrange var binder = new DictionaryModelBinder(); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/KeyValuePairModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/KeyValuePairModelBinderTest.cs index d3dbd30d0e..df1c2ef042 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/KeyValuePairModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/KeyValuePairModelBinderTest.cs @@ -148,13 +148,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test } [Fact] - public async Task KeyValuePairModelBinder_DoesNotCreateCollection_ForTopLevelModel_OnFirstPass() + public async Task KeyValuePairModelBinder_DoesNotCreateCollection_IfIsTopLevelObjectAndIsFirstChanceBinding() { // Arrange var binder = new KeyValuePairModelBinder(); var context = CreateContext(); - context.ModelName = "param"; + context.IsTopLevelObject = true; + context.IsFirstChanceBinding = true; + + // Explicit prefix and empty model name both ignored. + context.BinderModelName = "prefix"; + context.ModelName = string.Empty; var metadataProvider = context.OperationBindingContext.MetadataProvider; context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(KeyValuePair)); @@ -169,13 +174,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test } [Fact] - public async Task KeyValuePairModelBinder_CreatesEmptyCollection_ForTopLevelModel_OnFallback() + public async Task KeyValuePairModelBinder_CreatesEmptyCollection_IfIsTopLevelObjectAndNotIsFirstChanceBinding() { // Arrange var binder = new KeyValuePairModelBinder(); var context = CreateContext(); - context.ModelName = string.Empty; + context.IsTopLevelObject = true; + + // Lack of prefix and non-empty model name both ignored. + context.ModelName = "modelName"; var metadataProvider = context.OperationBindingContext.MetadataProvider; context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(KeyValuePair)); @@ -189,37 +197,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test Assert.NotNull(result); Assert.Equal(default(KeyValuePair), Assert.IsType>(result.Model)); - Assert.Equal(string.Empty, result.Key); - Assert.True(result.IsModelSet); - - Assert.Equal(result.ValidationNode.Model, result.Model); - Assert.Same(result.ValidationNode.Key, result.Key); - Assert.Same(result.ValidationNode.ModelMetadata, context.ModelMetadata); - } - - [Fact] - public async Task KeyValuePairModelBinder_CreatesEmptyCollection_ForTopLevelModel_WithExplicitPrefix() - { - // Arrange - var binder = new KeyValuePairModelBinder(); - - var context = CreateContext(); - context.ModelName = "prefix"; - context.BinderModelName = "prefix"; - - var metadataProvider = context.OperationBindingContext.MetadataProvider; - context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(KeyValuePair)); - - context.ValueProvider = new TestValueProvider(new Dictionary()); - - // Act - var result = await binder.BindModelAsync(context); - - // Assert - Assert.NotNull(result); - - Assert.Equal(default(KeyValuePair), Assert.IsType>(result.Model)); - Assert.Equal("prefix", result.Key); + Assert.Equal("modelName", result.Key); Assert.True(result.IsModelSet); Assert.Equal(result.ValidationNode.Model, result.Model); @@ -230,7 +208,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test [Theory] [InlineData("")] [InlineData("param")] - public async Task KeyValuePairModelBinder_DoesNotCreateCollection_ForNonTopLevelModel(string prefix) + public async Task KeyValuePairModelBinder_DoesNotCreateCollection_IfNotIsTopLevelObject(string prefix) { // Arrange var binder = new KeyValuePairModelBinder(); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs index 918ecb850e..c0c44a6d64 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs @@ -20,23 +20,42 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public class MutableObjectModelBinderTest { [Theory] - [InlineData(typeof(Person), true)] - [InlineData(typeof(Person), false)] - [InlineData(typeof(EmptyModel), true)] - [InlineData(typeof(EmptyModel), false)] - public async Task CanCreateModel_CreatesModel_ForTopLevelObjectIfThereIsExplicitPrefix( - Type modelType, - bool isPrefixProvided) + [InlineData(true, true, "", "", false)] + [InlineData(true, false, "", "", true)] + [InlineData(false, true, "", "", false)] // !isTopLevelObject && isFirstChanceBinding cases are unexpected + [InlineData(false, false, "", "", false)] + [InlineData(true, true, "prefix", "", false)] + [InlineData(true, false, "prefix", "", true)] + [InlineData(false, true, "prefix", "", false)] + [InlineData(false, false, "prefix", "", false)] + [InlineData(true, true, "", "dummyModelName", false)] + [InlineData(true, false, "", "dummyModelName", true)] + [InlineData(false, true, "", "dummyModelName", false)] + [InlineData(false, false, "", "dummyModelName", false)] + [InlineData(true, true, "prefix", "dummyModelName", false)] + [InlineData(true, false, "prefix", "dummyModelName", true)] + [InlineData(false, true, "prefix", "dummyModelName", false)] + [InlineData(false, false, "prefix", "dummyModelName", false)] + public async Task CanCreateModel_ReturnsTrue_IfIsTopLevelObjectAndNotIsFirstChanceBinding( + bool isTopLevelObject, + bool isFirstChanceBinding, + string binderModelName, + string modelName, + bool expectedCanCreate) { var mockValueProvider = new Mock(); - mockValueProvider.Setup(o => o.ContainsPrefixAsync(It.IsAny())) - .Returns(Task.FromResult(false)); + mockValueProvider + .Setup(o => o.ContainsPrefixAsync(It.IsAny())) + .Returns(Task.FromResult(false)); var metadataProvider = new TestModelMetadataProvider(); var bindingContext = new MutableObjectBinderContext { ModelBindingContext = new ModelBindingContext { + IsTopLevelObject = isTopLevelObject, + IsFirstChanceBinding = isFirstChanceBinding, + // Random type. ModelMetadata = metadataProvider.GetMetadataForType(typeof(Person)), ValueProvider = mockValueProvider.Object, @@ -47,65 +66,25 @@ namespace Microsoft.AspNet.Mvc.ModelBinding ValidatorProvider = Mock.Of(), }, - // Setting it to empty ensures that model does not get created becasue of no model name. - ModelName = "dummyModelName", - BinderModelName = isPrefixProvided ? "prefix" : null, - } + // CanCreateModel() ignores the BinderModelName and ModelName properties. + BinderModelName = binderModelName, + ModelName = modelName, + }, }; var mutableBinder = new TestableMutableObjectModelBinder(); bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties( - bindingContext.ModelBindingContext); + bindingContext.ModelBindingContext); // Act - var retModel = await mutableBinder.CanCreateModel(bindingContext); + var canCreate = await mutableBinder.CanCreateModel(bindingContext); // Assert - Assert.Equal(isPrefixProvided, retModel); - } - - [Theory] - [InlineData(typeof(Person), true)] - [InlineData(typeof(Person), false)] - [InlineData(typeof(EmptyModel), true)] - [InlineData(typeof(EmptyModel), false)] - public async Task - CanCreateModel_CreatesModel_ForTopLevelObjectIfThereIsEmptyModelName(Type modelType, bool emptyModelName) - { - var mockValueProvider = new Mock(); - mockValueProvider.Setup(o => o.ContainsPrefixAsync(It.IsAny())) - .Returns(Task.FromResult(false)); - - var bindingContext = new MutableObjectBinderContext - { - ModelBindingContext = new ModelBindingContext - { - // Random type. - ModelMetadata = GetMetadataForType(typeof(Person)), - ValueProvider = mockValueProvider.Object, - OperationBindingContext = new OperationBindingContext - { - ValidatorProvider = Mock.Of(), - ValueProvider = mockValueProvider.Object, - MetadataProvider = TestModelMetadataProvider.CreateDefaultProvider() - } - } - }; - - bindingContext.ModelBindingContext.ModelName = emptyModelName ? string.Empty : "dummyModelName"; - var mutableBinder = new TestableMutableObjectModelBinder(); - bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties( - bindingContext.ModelBindingContext); - - // Act - var retModel = await mutableBinder.CanCreateModel(bindingContext); - - // Assert - Assert.Equal(emptyModelName, retModel); + Assert.Equal(expectedCanCreate, canCreate); } [Fact] - public async Task CanCreateModel_ReturnsFalse_ForNonTopLevelModel_IfModelIsMarkedWithBinderMetadata() + public async Task CanCreateModel_ReturnsFalse_IfNotIsTopLevelObjectAndModelIsMarkedWithBinderMetadata() { // Get the property metadata so that it is not a top level object. var modelMetadata = GetMetadataForType(typeof(Document)) @@ -135,13 +114,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } [Fact] - public async Task CanCreateModel_ReturnsTrue_ForTopLevelModel_IfModelIsMarkedWithBinderMetadata() + public async Task CanCreateModel_ReturnsTrue_IfIsTopLevelObjectAndModelIsMarkedWithBinderMetadata() { var bindingContext = new MutableObjectBinderContext { ModelBindingContext = new ModelBindingContext { // Here the metadata represents a top level object. + IsTopLevelObject = true, + ModelMetadata = GetMetadataForType(typeof(Document)), OperationBindingContext = new OperationBindingContext { @@ -198,7 +179,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding [Theory] [InlineData(true)] [InlineData(false)] - public async Task CanCreateModel_ReturnsTrue_ForNonTopLevelModel_BasedOnValueAvailability(bool valueAvailable) + public async Task CanCreateModel_ReturnsTrue_IfNotIsTopLevelObject_BasedOnValueAvailability( + bool valueAvailable) { // Arrange var mockValueProvider = new Mock(MockBehavior.Strict); @@ -434,7 +416,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } [Fact] - public async Task BindModel_InitsInstance_ForEmptyModelName() + public async Task BindModel_InitsInstance_IfIsTopLevelObjectAndNotIsFirstChanceBinding() { // Arrange var mockValueProvider = new Mock(); @@ -444,6 +426,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var mockDtoBinder = new Mock(); var bindingContext = new ModelBindingContext { + IsTopLevelObject = true, ModelMetadata = GetMetadataForType(typeof(Person)), ModelName = "", ValueProvider = mockValueProvider.Object, diff --git a/test/Microsoft.AspNet.Mvc.Extensions.Test/Rendering/DefaultHtmlGeneratorTest.cs b/test/Microsoft.AspNet.Mvc.Extensions.Test/Rendering/DefaultHtmlGeneratorTest.cs index 4b988b51be..06f70bb54a 100644 --- a/test/Microsoft.AspNet.Mvc.Extensions.Test/Rendering/DefaultHtmlGeneratorTest.cs +++ b/test/Microsoft.AspNet.Mvc.Extensions.Test/Rendering/DefaultHtmlGeneratorTest.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Globalization; using System.IO; using System.Linq; using Microsoft.AspNet.DataProtection; diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs index ec625e5f90..8a836da386 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs @@ -181,10 +181,9 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests }; // Need to have a key here so that the GenericModelBinder will recurse to bind elements. - var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => - { - request.QueryString = new QueryString("?parameter.index=0"); - }); + var operationContext = ModelBindingTestHelper.GetOperationBindingContext( + request => request.QueryString = new QueryString("?parameter.index=0"), + options => options.ModelBinders.Add(new AddressBinder())); var modelState = new ModelStateDictionary(); @@ -204,6 +203,41 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.True(modelState.IsValid); } + // Similar to the GenericModelBinder_BindsCollection_ElementTypeUsesGreedyModelBinder_WithPrefix_Success + // scenario but mis-configured. Model using a BindingSource for which no ModelBinder is enabled. + [Fact] + public async Task GenericModelBinder_BindsCollection_ElementTypeUsesGreedyBindingSource_WithPrefix_NullElement() + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(Address[]) + }; + + // Need to have a key here so that the GenericModelBinder will recurse to bind elements. + var operationContext = ModelBindingTestHelper.GetOperationBindingContext( + request => request.QueryString = new QueryString("?parameter.index=0")); + + var modelState = new ModelStateDictionary(); + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext); + + // Assert + Assert.NotNull(modelBindingResult); + Assert.True(modelBindingResult.IsModelSet); + + var model = Assert.IsType(modelBindingResult.Model); + Assert.Equal(1, model.Length); + Assert.Null(model[0]); + + Assert.Equal(0, modelState.Count); + Assert.Equal(0, modelState.ErrorCount); + Assert.True(modelState.IsValid); + } + // This is part of a random sampling of scenarios where a GenericModelBinder is used // recursively. [Fact] diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/ValidationIntegrationTests.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/ValidationIntegrationTests.cs index 0c5dc8a502..e69c65cdda 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/ValidationIntegrationTests.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/ValidationIntegrationTests.cs @@ -4,7 +4,6 @@ using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.IO; -using System.Linq; using System.Text; using System.Threading.Tasks; using Microsoft.AspNet.Http;