diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ComplexModelDto.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ComplexModelDto.cs deleted file mode 100644 index bb4e7628b9..0000000000 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ComplexModelDto.cs +++ /dev/null @@ -1,32 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.Collections.Generic; -using System.Collections.ObjectModel; -using System.Linq; -using Microsoft.Framework.Internal; - -namespace Microsoft.AspNet.Mvc.ModelBinding -{ - // Describes a complex model, but uses a collection rather than individual properties as the data store. - public class ComplexModelDto - { - public ComplexModelDto([NotNull] ModelMetadata modelMetadata, - [NotNull] IEnumerable propertyMetadata) - { - ModelMetadata = modelMetadata; - PropertyMetadata = new Collection(propertyMetadata.ToList()); - Results = new Dictionary(); - } - - public ModelMetadata ModelMetadata { get; private set; } - - public Collection PropertyMetadata { get; private set; } - - // Contains entries corresponding to each property against which binding was - // attempted. If binding failed, the entry's value will be null. If binding - // was never attempted, this dictionary will not contain a corresponding - // entry. - public IDictionary Results { get; private set; } - } -} diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ComplexModelDtoModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ComplexModelDtoModelBinder.cs deleted file mode 100644 index 9314d5a591..0000000000 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ComplexModelDtoModelBinder.cs +++ /dev/null @@ -1,48 +0,0 @@ -// 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.Threading.Tasks; - -namespace Microsoft.AspNet.Mvc.ModelBinding -{ - public sealed class ComplexModelDtoModelBinder : IModelBinder - { - public async Task BindModelAsync(ModelBindingContext bindingContext) - { - if (bindingContext.ModelType != typeof(ComplexModelDto)) - { - return null; - } - - ModelBindingHelper.ValidateBindingContext(bindingContext, typeof(ComplexModelDto), allowNullModel: false); - - var dto = (ComplexModelDto)bindingContext.Model; - foreach (var propertyMetadata in dto.PropertyMetadata) - { - var propertyModelName = ModelNames.CreatePropertyModelName( - bindingContext.ModelName, - propertyMetadata.BinderModelName ?? propertyMetadata.PropertyName); - - var propertyBindingContext = ModelBindingContext.GetChildModelBindingContext( - bindingContext, - propertyModelName, - propertyMetadata); - - var modelBindingResult = - await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(propertyBindingContext); - if (modelBindingResult == null) - { - // Could not bind. Let MutableObjectModelBinder know explicitly. - dto.Results[propertyMetadata] = - new ModelBindingResult(model: null, key: propertyModelName, isModelSet: false); - } - else - { - dto.Results[propertyMetadata] = modelBindingResult; - } - } - - return new ModelBindingResult(dto, bindingContext.ModelName, isModelSet: true); - } - } -} diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs index a86826e433..8561264aa5 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs @@ -40,18 +40,22 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return null; } - EnsureModel(bindingContext); - var result = await CreateAndPopulateDto(bindingContext, mutableObjectBinderContext.PropertyMetadata); + // Create model first (if necessary) to avoid reporting errors about properties when activation fails. + var model = GetModel(bindingContext); + + var results = await BindPropertiesAsync(bindingContext, mutableObjectBinderContext.PropertyMetadata); var validationNode = new ModelValidationNode( bindingContext.ModelName, bindingContext.ModelMetadata, - bindingContext.Model); + model); + + // Post-processing e.g. property setters and hooking up validation. + bindingContext.Model = model; + ProcessResults(bindingContext, results, validationNode); - // post-processing, e.g. property setters and hooking up validation - ProcessDto(bindingContext, (ComplexModelDto)result.Model, validationNode); return new ModelBindingResult( - bindingContext.Model, + model, bindingContext.ModelName, isModelSet: true, validationNode: validationNode); @@ -192,7 +196,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var bindingSource = bindingContext.BindingSource; if (bindingSource != null && !bindingSource.IsGreedy) { - var rootValueProvider = bindingContext.OperationBindingContext.ValueProvider as IBindingSourceValueProvider; + var rootValueProvider = + bindingContext.OperationBindingContext.ValueProvider as IBindingSourceValueProvider; if (rootValueProvider != null) { valueProvider = rootValueProvider.Filter(bindingSource); @@ -225,12 +230,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return false; } - if (modelMetadata.ModelType == typeof(ComplexModelDto)) - { - // forbidden type - will cause a stack overflow if we try binding this type - return false; - } - return true; } @@ -265,24 +264,50 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return true; } - private async Task CreateAndPopulateDto( + // Returned dictionary contains entries corresponding to properties against which binding was attempted. If + // binding failed, the entry's value will have IsModelSet == false. Binding is attempted for all elements of + // propertyMetadatas. + private async Task> BindPropertiesAsync( ModelBindingContext bindingContext, IEnumerable propertyMetadatas) { - // create a DTO and call into the DTO binder - var dto = new ComplexModelDto(bindingContext.ModelMetadata, propertyMetadatas); + var results = new Dictionary(); + foreach (var propertyMetadata in propertyMetadatas) + { + var propertyModelName = ModelNames.CreatePropertyModelName( + bindingContext.ModelName, + propertyMetadata.BinderModelName ?? propertyMetadata.PropertyName); + var childContext = ModelBindingContext.GetChildModelBindingContext( + bindingContext, + propertyModelName, + propertyMetadata); - var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider; - var dtoMetadata = metadataProvider.GetMetadataForType(typeof(ComplexModelDto)); + // ModelBindingContext.Model property values may be non-null when invoked via TryUpdateModel(). Pass + // complex (including collection) values down so that binding system does not unnecessarily recreate + // instances or overwrite inner properties that are not bound. No need for this with simple values + // because they will be overwritten if binding succeeds. Arrays are never reused because they cannot + // be resized. + // + // ModelMetadata.PropertyGetter is not null safe; use it only if Model is non-null. + if (bindingContext.Model != null && + propertyMetadata.PropertyGetter != null && + propertyMetadata.IsComplexType && + !propertyMetadata.ModelType.IsArray) + { + childContext.Model = propertyMetadata.PropertyGetter(bindingContext.Model); + } - var childContext = ModelBindingContext.GetChildModelBindingContext( - bindingContext, - bindingContext.ModelName, - dtoMetadata); + var result = await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(childContext); + if (result == null) + { + // Could not bind. Let ProcessResult() know explicitly. + result = new ModelBindingResult(model: null, key: propertyModelName, isModelSet: false); + } - childContext.Model = dto; + results[propertyMetadata] = result; + } - return await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(childContext); + return results; } /// @@ -298,16 +323,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } /// - /// Ensures is not null in given - /// . + /// Get if that property is not null. Otherwise activate a + /// new instance of . /// /// The . - protected virtual void EnsureModel([NotNull] ModelBindingContext bindingContext) + protected virtual object GetModel([NotNull] ModelBindingContext bindingContext) { - if (bindingContext.Model == null) + if (bindingContext.Model != null) { - bindingContext.Model = CreateModel(bindingContext); + return bindingContext.Model; } + + return CreateModel(bindingContext); } /// @@ -365,17 +392,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } // Internal for testing. - internal ModelValidationNode ProcessDto( + internal ModelValidationNode ProcessResults( ModelBindingContext bindingContext, - ComplexModelDto dto, + IDictionary results, ModelValidationNode validationNode) { var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider; - var modelExplorer = metadataProvider.GetModelExplorerForType(bindingContext.ModelType, bindingContext.Model); + var modelExplorer = + metadataProvider.GetModelExplorerForType(bindingContext.ModelType, bindingContext.Model); var validationInfo = GetPropertyValidationInfo(bindingContext); // Eliminate provided properties from RequiredProperties; leaving just *missing* required properties. - var boundProperties = dto.Results.Where(p => p.Value.IsModelSet).Select(p => p.Key.PropertyName); + var boundProperties = results.Where(p => p.Value.IsModelSet).Select(p => p.Key.PropertyName); validationInfo.RequiredProperties.ExceptWith(boundProperties); foreach (var missingRequiredProperty in validationInfo.RequiredProperties) @@ -389,25 +417,25 @@ namespace Microsoft.AspNet.Mvc.ModelBinding Resources.FormatModelBinding_MissingBindRequiredMember(propertyName)); } - // For each property that ComplexModelDtoModelBinder attempted to bind, call the setter, recording + // For each property that BindPropertiesAsync() attempted to bind, call the setter, recording // exceptions as necessary. - foreach (var entry in dto.Results) + foreach (var entry in results) { - var dtoResult = entry.Value; - if (dtoResult != null) + var result = entry.Value; + if (result != null) { var propertyMetadata = entry.Key; - SetProperty(bindingContext, modelExplorer, propertyMetadata, dtoResult); + SetProperty(bindingContext, modelExplorer, propertyMetadata, result); - var dtoValidationNode = dtoResult.ValidationNode; - if (dtoValidationNode == null) + var propertyValidationNode = result.ValidationNode; + if (propertyValidationNode == null) { - // Make sure that irrespective of if the properties of the model were bound with a value, + // Make sure that irrespective of whether the properties of the model were bound with a value, // create a validation node so that these get validated. - dtoValidationNode = new ModelValidationNode(dtoResult.Key, entry.Key, dtoResult.Model); + propertyValidationNode = new ModelValidationNode(result.Key, entry.Key, result.Model); } - validationNode.ChildNodes.Add(dtoValidationNode); + validationNode.ChildNodes.Add(propertyValidationNode); } } @@ -422,18 +450,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// The for the model containing property to set. /// /// The for the property to set. - /// The for the property's new value. + /// The for the property's new value. /// Should succeed in all cases that returns true. protected virtual void SetProperty( [NotNull] ModelBindingContext bindingContext, [NotNull] ModelExplorer modelExplorer, [NotNull] ModelMetadata propertyMetadata, - [NotNull] ModelBindingResult dtoResult) + [NotNull] ModelBindingResult result) { var bindingFlags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.IgnoreCase; - var property = bindingContext.ModelType.GetProperty( - propertyMetadata.PropertyName, - bindingFlags); + var property = bindingContext.ModelType.GetProperty(propertyMetadata.PropertyName, bindingFlags); if (property == null) { @@ -441,33 +467,27 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return; } + if (!result.IsModelSet) + { + // If we don't have a value, don't set it on the model and trounce a pre-initialized value. + return; + } + if (!property.CanWrite) { // Try to handle as a collection if property exists but is not settable. - AddToProperty(bindingContext, modelExplorer, property, dtoResult); - return; - } - - object value = null; - if (dtoResult.IsModelSet) - { - value = dtoResult.Model; - } - - if (!dtoResult.IsModelSet) - { - // If we don't have a value, don't set it on the model and trounce a pre-initialized - // value. + AddToProperty(bindingContext, modelExplorer, property, result); return; } + var value = result.Model; try { propertyMetadata.PropertySetter(bindingContext.Model, value); } catch (Exception exception) { - AddModelError(exception, bindingContext, dtoResult); + AddModelError(exception, bindingContext, result); } } @@ -475,18 +495,24 @@ namespace Microsoft.AspNet.Mvc.ModelBinding ModelBindingContext bindingContext, ModelExplorer modelExplorer, PropertyInfo property, - ModelBindingResult dtoResult) + ModelBindingResult result) { var propertyExplorer = modelExplorer.GetExplorerForProperty(property.Name); var target = propertyExplorer.Model; - var source = dtoResult.Model; - if (!dtoResult.IsModelSet || target == null || source == null) + var source = result.Model; + if (target == null || source == null) { // Cannot copy to or from a null collection. return; } + if (target == source) + { + // Added to the target collection in BindPropertiesAsync(). + return; + } + // Determine T if this is an ICollection property. No need for a T[] case because CanUpdateProperty() // ensures property is either settable or not an array. Underlying assumption is that CanUpdateProperty() // and SetProperty() are overridden together. @@ -507,7 +533,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } catch (Exception exception) { - AddModelError(exception, bindingContext, dtoResult); + AddModelError(exception, bindingContext, result); } } @@ -529,7 +555,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding private static void AddModelError( Exception exception, ModelBindingContext bindingContext, - ModelBindingResult dtoResult) + ModelBindingResult result) { var targetInvocationException = exception as TargetInvocationException; if (targetInvocationException != null && targetInvocationException.InnerException != null) @@ -539,7 +565,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Do not add an error message if a binding error has already occurred for this property. var modelState = bindingContext.ModelState; - var modelStateKey = dtoResult.Key; + var modelStateKey = result.Key; var validationState = modelState.GetFieldValidationState(modelStateKey); if (validationState == ModelValidationState.Unvalidated) { diff --git a/src/Microsoft.AspNet.Mvc.Core/MvcCoreMvcOptionsSetup.cs b/src/Microsoft.AspNet.Mvc.Core/MvcCoreMvcOptionsSetup.cs index 5d92d989c8..d0177b9cf5 100644 --- a/src/Microsoft.AspNet.Mvc.Core/MvcCoreMvcOptionsSetup.cs +++ b/src/Microsoft.AspNet.Mvc.Core/MvcCoreMvcOptionsSetup.cs @@ -36,7 +36,6 @@ namespace Microsoft.AspNet.Mvc options.ModelBinders.Add(new FormCollectionModelBinder()); options.ModelBinders.Add(new GenericModelBinder()); options.ModelBinders.Add(new MutableObjectModelBinder()); - options.ModelBinders.Add(new ComplexModelDtoModelBinder()); // Set up default output formatters. options.OutputFormatters.Add(new HttpNoContentOutputFormatter()); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/ComplexModelDtoTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/ComplexModelDtoTest.cs deleted file mode 100644 index e9ab9bca83..0000000000 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/ComplexModelDtoTest.cs +++ /dev/null @@ -1,32 +0,0 @@ -// 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.Linq; -using Xunit; - -namespace Microsoft.AspNet.Mvc.ModelBinding.Test -{ - public class ComplexModelDtoTest - { - [Fact] - public void ConstructorSetsProperties() - { - // Arrange - ModelMetadata modelMetadata = GetModelMetadata(); - ModelMetadata[] propertyMetadata = new ModelMetadata[0]; - - // Act - ComplexModelDto dto = new ComplexModelDto(modelMetadata, propertyMetadata); - - // Assert - Assert.Equal(modelMetadata, dto.ModelMetadata); - Assert.Equal(propertyMetadata, dto.PropertyMetadata.ToArray()); - Assert.Empty(dto.Results); - } - - private static ModelMetadata GetModelMetadata() - { - return new EmptyModelMetadataProvider().GetMetadataForProperty(typeof(string), "Length"); - } - } -} diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CompositeModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CompositeModelBinderTest.cs index 1195aa75a7..5a924f4af5 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CompositeModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CompositeModelBinderTest.cs @@ -534,7 +534,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test new TypeMatchModelBinder(), new ByteArrayModelBinder(), new GenericModelBinder(), - new ComplexModelDtoModelBinder(), new TypeConverterModelBinder(), new MutableObjectModelBinder() }; diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/DictionaryModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/DictionaryModelBinderTest.cs index ec67e53eff..617bd67337 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/DictionaryModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/DictionaryModelBinderTest.cs @@ -433,7 +433,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test new TypeConverterModelBinder(), new TypeMatchModelBinder(), new MutableObjectModelBinder(), - new ComplexModelDtoModelBinder(), }; return new CompositeModelBinder(binders); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs index a7f82e0424..08c7e95cd6 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs @@ -73,8 +73,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding }; var mutableBinder = new TestableMutableObjectModelBinder(); - bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties( - bindingContext.ModelBindingContext).ToArray(); + bindingContext.PropertyMetadata = + mutableBinder.GetMetadataForProperties(bindingContext.ModelBindingContext).ToArray(); // Act var canCreate = await mutableBinder.CanCreateModel(bindingContext); @@ -160,14 +160,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding MetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(), }, - // Setting it to empty ensures that model does not get created becasue of no model name. + // Setting it to empty ensures that model does not get created because of no model name. ModelName = "dummyModelName", }, }; var mutableBinder = new TestableMutableObjectModelBinder(); - bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties( - bindingContext.ModelBindingContext).ToArray(); + bindingContext.PropertyMetadata = + mutableBinder.GetMetadataForProperties(bindingContext.ModelBindingContext).ToArray(); // Act var retModel = await mutableBinder.CanCreateModel(bindingContext); @@ -232,8 +232,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding }; var mutableBinder = new TestableMutableObjectModelBinder(); - bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties( - bindingContext.ModelBindingContext).ToArray(); + bindingContext.PropertyMetadata = + mutableBinder.GetMetadataForProperties(bindingContext.ModelBindingContext).ToArray(); // Act var canCreate = await mutableBinder.CanCreateModel(bindingContext); @@ -256,8 +256,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding }; var mutableBinder = new TestableMutableObjectModelBinder(); - bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties( - bindingContext.ModelBindingContext).ToArray(); + bindingContext.PropertyMetadata = + mutableBinder.GetMetadataForProperties(bindingContext.ModelBindingContext).ToArray(); // Act var retModel = await mutableBinder.CanCreateModel(bindingContext); @@ -299,8 +299,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding }; var mutableBinder = new TestableMutableObjectModelBinder(); - bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties( - bindingContext.ModelBindingContext).ToArray(); + bindingContext.PropertyMetadata = + mutableBinder.GetMetadataForProperties(bindingContext.ModelBindingContext).ToArray(); // Act var retModel = await mutableBinder.CanCreateModel(bindingContext); @@ -359,8 +359,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding }; var mutableBinder = new TestableMutableObjectModelBinder(); - bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties( - bindingContext.ModelBindingContext).ToArray(); + bindingContext.PropertyMetadata = + mutableBinder.GetMetadataForProperties(bindingContext.ModelBindingContext).ToArray(); // Act var retModel = await mutableBinder.CanCreateModel(bindingContext); @@ -374,7 +374,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), true)] [InlineData(typeof(TypeWithNoBinderMetadata), false)] [InlineData(typeof(TypeWithNoBinderMetadata), true)] - public async Task CanCreateModel_UnmarkedProperties_UsesCurrentValueProvider(Type modelType, bool valueProviderProvidesValue) + public async Task CanCreateModel_UnmarkedProperties_UsesCurrentValueProvider( + Type modelType, + bool valueProviderProvidesValue) { var mockValueProvider = new Mock(); mockValueProvider.Setup(o => o.ContainsPrefixAsync(It.IsAny())) @@ -402,8 +404,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding }; var mutableBinder = new TestableMutableObjectModelBinder(); - bindingContext.PropertyMetadata = mutableBinder.GetMetadataForProperties( - bindingContext.ModelBindingContext).ToArray(); + bindingContext.PropertyMetadata = + mutableBinder.GetMetadataForProperties(bindingContext.ModelBindingContext).ToArray(); // Act var retModel = await mutableBinder.CanCreateModel(bindingContext); @@ -417,10 +419,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { // Arrange var mockValueProvider = new Mock(); - mockValueProvider.Setup(o => o.ContainsPrefixAsync(It.IsAny())) - .Returns(Task.FromResult(true)); + mockValueProvider + .Setup(o => o.ContainsPrefixAsync(It.IsAny())) + .Returns(Task.FromResult(true)); + + // Mock binder fails to bind all properties. + var mockBinder = new Mock(); + mockBinder + .Setup(o => o.BindModelAsync(It.IsAny())) + .Returns(Task.FromResult(null)); - var mockDtoBinder = new Mock(); var bindingContext = new ModelBindingContext { ModelMetadata = GetMetadataForType(typeof(Person)), @@ -428,26 +436,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding ValueProvider = mockValueProvider.Object, OperationBindingContext = new OperationBindingContext { - ModelBinder = mockDtoBinder.Object, + ModelBinder = mockBinder.Object, MetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(), ValidatorProvider = Mock.Of() } }; - mockDtoBinder - .Setup(o => o.BindModelAsync(It.IsAny())) - .Returns((ModelBindingContext mbc) => - { - // just return the DTO unchanged - return Task.FromResult(new ModelBindingResult(mbc.Model, mbc.ModelName, true)); - }); - var model = new Person(); var testableBinder = new Mock { CallBase = true }; testableBinder - .Setup(o => o.EnsureModelPublic(bindingContext)) - .Callback(c => c.Model = model) + .Setup(o => o.GetModelPublic(bindingContext)) + .Returns(model) .Verifiable(); testableBinder .Setup(o => o.GetMetadataForProperties(bindingContext)) @@ -469,38 +469,36 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { // Arrange 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)); + + // Mock binder fails to bind all properties. + var mockBinder = new Mock(); + mockBinder + .Setup(o => o.BindModelAsync(It.IsAny())) + .Returns(Task.FromResult(null)); - var mockDtoBinder = new Mock(); var bindingContext = new ModelBindingContext { IsTopLevelObject = true, ModelMetadata = GetMetadataForType(typeof(Person)), - ModelName = "", + ModelName = string.Empty, ValueProvider = mockValueProvider.Object, OperationBindingContext = new OperationBindingContext { - ModelBinder = mockDtoBinder.Object, + ModelBinder = mockBinder.Object, MetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(), ValidatorProvider = Mock.Of() } }; - mockDtoBinder - .Setup(o => o.BindModelAsync(It.IsAny())) - .Returns((ModelBindingContext mbc) => - { - // just return the DTO unchanged - return Task.FromResult(new ModelBindingResult(mbc.Model, mbc.ModelName, true)); - }); - var model = new Person(); var testableBinder = new Mock { CallBase = true }; testableBinder - .Setup(o => o.EnsureModelPublic(bindingContext)) - .Callback(c => c.Model = model) + .Setup(o => o.GetModelPublic(bindingContext)) + .Returns(model) .Verifiable(); testableBinder @@ -576,7 +574,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } [Fact] - public void EnsureModel_ModelIsNotNull_DoesNothing() + public void GetModel_ModelIsNotNull_DoesNothing() { // Arrange var bindingContext = new ModelBindingContext @@ -585,19 +583,20 @@ namespace Microsoft.AspNet.Mvc.ModelBinding ModelMetadata = GetMetadataForType(typeof(Person)) }; + var originalModel = bindingContext.Model; var testableBinder = new Mock { CallBase = true }; // Act - var originalModel = bindingContext.Model; - testableBinder.Object.EnsureModelPublic(bindingContext); + var newModel = testableBinder.Object.GetModelPublic(bindingContext); // Assert Assert.Same(originalModel, bindingContext.Model); + Assert.Same(originalModel, newModel); testableBinder.Verify(o => o.CreateModelPublic(bindingContext), Times.Never()); } [Fact] - public void EnsureModel_ModelIsNull_CallsCreateModel() + public void GetModel_ModelIsNull_CallsCreateModel() { // Arrange var bindingContext = new ModelBindingContext @@ -605,17 +604,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding ModelMetadata = GetMetadataForType(typeof(Person)) }; + var originalModel = bindingContext.Model; var testableBinder = new Mock { CallBase = true }; testableBinder.Setup(o => o.CreateModelPublic(bindingContext)) .Returns(new Person()).Verifiable(); // Act - var originalModel = bindingContext.Model; - testableBinder.Object.EnsureModelPublic(bindingContext); - var newModel = bindingContext.Model; + var newModel = testableBinder.Object.GetModelPublic(bindingContext); + // Assert Assert.Null(originalModel); + Assert.Null(bindingContext.Model); Assert.IsType(newModel); testableBinder.Verify(); } @@ -779,7 +779,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding [Fact] [ReplaceCulture] - public void ProcessDto_BindRequiredFieldMissing_RaisesModelError() + public void ProcessResults_BindRequiredFieldMissing_RaisesModelError() { // Arrange var model = new ModelWithBindRequired @@ -800,18 +800,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding ValidatorProvider = Mock.Of() } }; - var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties); - var nameProperty = dto.PropertyMetadata.Single(o => o.PropertyName == "Name"); - dto.Results[nameProperty] = new ModelBindingResult( - "John Doe", - isModelSet: true, - key: ""); + var results = containerMetadata.Properties.ToDictionary( + property => property, + property => new ModelBindingResult(model: null, key: property.PropertyName, isModelSet: false)); + var nameProperty = containerMetadata.Properties[nameof(model.Name)]; + results[nameProperty] = new ModelBindingResult("John Doe", isModelSet: true, key: string.Empty); + var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); var testableBinder = new TestableMutableObjectModelBinder(); // Act - testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); + testableBinder.ProcessResults(bindingContext, results, modelValidationNode); // Assert var modelStateDictionary = bindingContext.ModelState; @@ -829,7 +829,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding [Fact] [ReplaceCulture] - public void ProcessDto_DataMemberIsRequiredFieldMissing_RaisesModelError() + public void ProcessResults_DataMemberIsRequiredFieldMissing_RaisesModelError() { // Arrange var model = new ModelWithDataMemberIsRequired @@ -850,19 +850,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding ValidatorProvider = Mock.Of() } }; - var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties); - var nameProperty = dto.PropertyMetadata.Single(o => o.PropertyName == "Name"); - dto.Results[nameProperty] = new ModelBindingResult( - "John Doe", - isModelSet: true, - key: ""); + var results = containerMetadata.Properties.ToDictionary( + property => property, + property => new ModelBindingResult(model: null, key: property.PropertyName, isModelSet: false)); + var nameProperty = containerMetadata.Properties[nameof(model.Name)]; + results[nameProperty] = new ModelBindingResult("John Doe", isModelSet: true, key: string.Empty); var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); var testableBinder = new TestableMutableObjectModelBinder(); // Act - testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); + testableBinder.ProcessResults(bindingContext, results, modelValidationNode); // Assert var modelStateDictionary = bindingContext.ModelState; @@ -880,7 +879,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding [Fact] [ReplaceCulture] - public void ProcessDto_ValueTypePropertyWithBindRequired_SetToNull_CapturesException() + public void ProcessResults_ValueTypePropertyWithBindRequired_SetToNull_CapturesException() { // Arrange var model = new ModelWithBindRequired @@ -903,27 +902,22 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } }; - var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties); - var testableBinder = new TestableMutableObjectModelBinder(); - - var propertyMetadata = dto.PropertyMetadata.Single(o => o.PropertyName == "Name"); - dto.Results[propertyMetadata] = new ModelBindingResult( - "John Doe", - isModelSet: true, - key: "theModel.Name"); + var results = containerMetadata.Properties.ToDictionary( + property => property, + property => new ModelBindingResult(model: null, key: property.PropertyName, isModelSet: false)); + var propertyMetadata = containerMetadata.Properties[nameof(model.Name)]; + results[propertyMetadata] = new ModelBindingResult("John Doe", isModelSet: true, key: "theModel.Name"); // Attempt to set non-Nullable property to null. BindRequiredAttribute should not be relevant in this // case because the binding exists. - propertyMetadata = dto.PropertyMetadata.Single(o => o.PropertyName == "Age"); - dto.Results[propertyMetadata] = new ModelBindingResult( - null, - isModelSet: true, - key: "theModel.Age"); + propertyMetadata = containerMetadata.Properties[nameof(model.Age)]; + results[propertyMetadata] = new ModelBindingResult(model: null, isModelSet: true, key: "theModel.Age"); + var testableBinder = new TestableMutableObjectModelBinder(); var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); // Act - testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); + testableBinder.ProcessResults(bindingContext, results, modelValidationNode); // Assert var modelStateDictionary = bindingContext.ModelState; @@ -941,19 +935,22 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } [Fact] - public void ProcessDto_ValueTypeProperty_WithBindingOptional_NoValueSet_NoError() + public void ProcessResults_ValueTypeProperty_WithBindingOptional_NoValueSet_NoError() { // Arrange var model = new BindingOptionalProperty(); var containerMetadata = GetMetadataForType(model.GetType()); var bindingContext = CreateContext(containerMetadata, model); - var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties); + var results = containerMetadata.Properties.ToDictionary( + property => property, + property => new ModelBindingResult(model: null, key: property.PropertyName, isModelSet: false)); + var testableBinder = new TestableMutableObjectModelBinder(); var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); // Act - testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); + testableBinder.ProcessResults(bindingContext, results, modelValidationNode); // Assert var modelStateDictionary = bindingContext.ModelState; @@ -961,19 +958,22 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } [Fact] - public void ProcessDto_NullableValueTypeProperty_NoValueSet_NoError() + public void ProcessResults_NullableValueTypeProperty_NoValueSet_NoError() { // Arrange var model = new NullableValueTypeProperty(); var containerMetadata = GetMetadataForType(model.GetType()); var bindingContext = CreateContext(containerMetadata, model); - - var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties); + + var results = containerMetadata.Properties.ToDictionary( + property => property, + property => new ModelBindingResult(model: null, key: property.PropertyName, isModelSet: false)); + var testableBinder = new TestableMutableObjectModelBinder(); var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); // Act - testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); + testableBinder.ProcessResults(bindingContext, results, modelValidationNode); // Assert var modelStateDictionary = bindingContext.ModelState; @@ -981,7 +981,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } [Fact] - public void ProcessDto_ValueTypeProperty_TriesToSetNullModel_CapturesException() + public void ProcessResults_ValueTypeProperty_TriesToSetNullModel_CapturesException() { // Arrange var model = new Person(); @@ -990,23 +990,24 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var bindingContext = CreateContext(containerMetadata, model); var modelStateDictionary = bindingContext.ModelState; - var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties); + var results = containerMetadata.Properties.ToDictionary( + property => property, + property => new ModelBindingResult(model: null, key: property.PropertyName, isModelSet: false)); var testableBinder = new TestableMutableObjectModelBinder(); // The [DefaultValue] on ValueTypeRequiredWithDefaultValue is ignored by model binding. var expectedValue = 0; // Make ValueTypeRequired invalid. - var propertyMetadata = dto.PropertyMetadata.Single(p => p.PropertyName == nameof(Person.ValueTypeRequired)); - dto.Results[propertyMetadata] = new ModelBindingResult( - null, + var propertyMetadata = containerMetadata.Properties[nameof(Person.ValueTypeRequired)]; + results[propertyMetadata] = new ModelBindingResult( + model: null, isModelSet: true, key: "theModel." + nameof(Person.ValueTypeRequired)); // Make ValueTypeRequiredWithDefaultValue invalid - propertyMetadata = dto.PropertyMetadata - .Single(p => p.PropertyName == nameof(Person.ValueTypeRequiredWithDefaultValue)); - dto.Results[propertyMetadata] = new ModelBindingResult( + propertyMetadata = containerMetadata.Properties[nameof(Person.ValueTypeRequiredWithDefaultValue)]; + results[propertyMetadata] = new ModelBindingResult( model: null, isModelSet: true, key: "theModel." + nameof(Person.ValueTypeRequiredWithDefaultValue)); @@ -1014,7 +1015,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); // Act - testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); + testableBinder.ProcessResults(bindingContext, results, modelValidationNode); // Assert Assert.False(modelStateDictionary.IsValid); @@ -1050,7 +1051,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } [Fact] - public void ProcessDto_ValueTypeProperty_NoValue_NoError() + public void ProcessResults_ValueTypeProperty_NoValue_NoError() { // Arrange var model = new Person(); @@ -1059,20 +1060,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var bindingContext = CreateContext(containerMetadata, model); var modelState = bindingContext.ModelState; - var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties); + var results = containerMetadata.Properties.ToDictionary( + property => property, + property => new ModelBindingResult(model: null, key: property.PropertyName, isModelSet: false)); var testableBinder = new TestableMutableObjectModelBinder(); // Make ValueTypeRequired invalid. - var propertyMetadata = dto.PropertyMetadata.Single(p => p.PropertyName == nameof(Person.ValueTypeRequired)); - dto.Results[propertyMetadata] = new ModelBindingResult( - null, + var propertyMetadata = containerMetadata.Properties[nameof(Person.ValueTypeRequired)]; + results[propertyMetadata] = new ModelBindingResult( + model: null, isModelSet: false, key: "theModel." + nameof(Person.ValueTypeRequired)); // Make ValueTypeRequiredWithDefaultValue invalid - propertyMetadata = dto.PropertyMetadata - .Single(p => p.PropertyName == nameof(Person.ValueTypeRequiredWithDefaultValue)); - dto.Results[propertyMetadata] = new ModelBindingResult( + propertyMetadata = containerMetadata.Properties[nameof(Person.ValueTypeRequiredWithDefaultValue)]; + results[propertyMetadata] = new ModelBindingResult( model: null, isModelSet: false, key: "theModel." + nameof(Person.ValueTypeRequiredWithDefaultValue)); @@ -1080,14 +1082,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); // Act - testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); + testableBinder.ProcessResults(bindingContext, results, modelValidationNode); // Assert Assert.True(modelState.IsValid); } [Fact] - public void ProcessDto_ProvideRequiredFields_Success() + public void ProcessResults_ProvideRequiredFields_Success() { // Arrange var model = new Person(); @@ -1096,36 +1098,35 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var bindingContext = CreateContext(containerMetadata, model); var modelStateDictionary = bindingContext.ModelState; - var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties); + var results = containerMetadata.Properties.ToDictionary( + property => property, + property => new ModelBindingResult(model: null, key: property.PropertyName, isModelSet: false)); var testableBinder = new TestableMutableObjectModelBinder(); // Make ValueTypeRequired valid. - var propertyMetadata = dto.PropertyMetadata - .Single(p => p.PropertyName == nameof(Person.ValueTypeRequired)); - dto.Results[propertyMetadata] = new ModelBindingResult( - 41, + var propertyMetadata = containerMetadata.Properties[nameof(Person.ValueTypeRequired)]; + results[propertyMetadata] = new ModelBindingResult( + model: 41, isModelSet: true, key: "theModel." + nameof(Person.ValueTypeRequired)); // Make ValueTypeRequiredWithDefaultValue valid. - propertyMetadata = dto.PropertyMetadata - .Single(p => p.PropertyName == nameof(Person.ValueTypeRequiredWithDefaultValue)); - dto.Results[propertyMetadata] = new ModelBindingResult( + propertyMetadata = containerMetadata.Properties[nameof(Person.ValueTypeRequiredWithDefaultValue)]; + results[propertyMetadata] = new ModelBindingResult( model: 57, isModelSet: true, key: "theModel." + nameof(Person.ValueTypeRequiredWithDefaultValue)); - // Also remind ProcessDto about PropertyWithDefaultValue -- as ComplexModelDtoModelBinder would. - propertyMetadata = dto.PropertyMetadata - .Single(p => p.PropertyName == nameof(Person.PropertyWithDefaultValue)); - dto.Results[propertyMetadata] = new ModelBindingResult( + // Also remind ProcessResults about PropertyWithDefaultValue -- as BindPropertiesAsync() would. + propertyMetadata = containerMetadata.Properties[nameof(Person.PropertyWithDefaultValue)]; + results[propertyMetadata] = new ModelBindingResult( model: null, isModelSet: false, key: "theModel." + nameof(Person.PropertyWithDefaultValue)); var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); // Act - testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); + testableBinder.ProcessResults(bindingContext, results, modelValidationNode); // Assert Assert.True(modelStateDictionary.IsValid); @@ -1139,7 +1140,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // [Required] cannot provide a custom validation for [BindRequired] errors. [Fact] - public void ProcessDto_ValueTypePropertyWithBindRequired_RequiredValidatorIgnored() + public void ProcessResults_ValueTypePropertyWithBindRequired_RequiredValidatorIgnored() { // Arrange var model = new ModelWithBindRequiredAndRequiredAttribute(); @@ -1148,21 +1149,23 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var bindingContext = CreateContext(containerMetadata, model); var modelStateDictionary = bindingContext.ModelState; - var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties); + var results = containerMetadata.Properties.ToDictionary( + property => property, + property => new ModelBindingResult(model: null, key: property.PropertyName, isModelSet: false)); var testableBinder = new TestableMutableObjectModelBinder(); // Make ValueTypeProperty not have a value. var propertyMetadata = containerMetadata .Properties[nameof(ModelWithBindRequiredAndRequiredAttribute.ValueTypeProperty)]; - dto.Results[propertyMetadata] = new ModelBindingResult( - null, + results[propertyMetadata] = new ModelBindingResult( + model: null, isModelSet: false, key: "theModel." + nameof(ModelWithBindRequiredAndRequiredAttribute.ValueTypeProperty)); // Make ReferenceTypeProperty have a value. propertyMetadata = containerMetadata .Properties[nameof(ModelWithBindRequiredAndRequiredAttribute.ReferenceTypeProperty)]; - dto.Results[propertyMetadata] = new ModelBindingResult( + results[propertyMetadata] = new ModelBindingResult( model: "value", isModelSet: true, key: "theModel." + nameof(ModelWithBindRequiredAndRequiredAttribute.ReferenceTypeProperty)); @@ -1170,7 +1173,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); // Act - testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); + testableBinder.ProcessResults(bindingContext, results, modelValidationNode); // Assert Assert.False(modelStateDictionary.IsValid); @@ -1190,7 +1193,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // [Required] cannot provide a custom validation for [BindRequired] errors. [Fact] - public void ProcessDto_ReferenceTypePropertyWithBindRequired_RequiredValidatorIgnored() + public void ProcessResults_ReferenceTypePropertyWithBindRequired_RequiredValidatorIgnored() { // Arrange var model = new ModelWithBindRequiredAndRequiredAttribute(); @@ -1199,21 +1202,23 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var bindingContext = CreateContext(containerMetadata, model); var modelStateDictionary = bindingContext.ModelState; - var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties); + var results = containerMetadata.Properties.ToDictionary( + property => property, + property => new ModelBindingResult(model: null, key: property.PropertyName, isModelSet: false)); var testableBinder = new TestableMutableObjectModelBinder(); // Make ValueTypeProperty have a value. var propertyMetadata = containerMetadata .Properties[nameof(ModelWithBindRequiredAndRequiredAttribute.ValueTypeProperty)]; - dto.Results[propertyMetadata] = new ModelBindingResult( - 17, + results[propertyMetadata] = new ModelBindingResult( + model: 17, isModelSet: true, key: "theModel." + nameof(ModelWithBindRequiredAndRequiredAttribute.ValueTypeProperty)); // Make ReferenceTypeProperty not have a value. propertyMetadata = containerMetadata .Properties[nameof(ModelWithBindRequiredAndRequiredAttribute.ReferenceTypeProperty)]; - dto.Results[propertyMetadata] = new ModelBindingResult( + results[propertyMetadata] = new ModelBindingResult( model: null, isModelSet: false, key: "theModel." + nameof(ModelWithBindRequiredAndRequiredAttribute.ReferenceTypeProperty)); @@ -1221,7 +1226,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); // Act - testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); + testableBinder.ProcessResults(bindingContext, results, modelValidationNode); // Assert Assert.False(modelStateDictionary.IsValid); @@ -1241,7 +1246,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding [Fact] - public void ProcessDto_Success() + public void ProcessResults_Success() { // Arrange var dob = new DateTime(2001, 1, 1); @@ -1252,28 +1257,27 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var containerMetadata = GetMetadataForType(model.GetType()); var bindingContext = CreateContext(containerMetadata, model); - var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties); + var results = containerMetadata.Properties.ToDictionary( + property => property, + property => new ModelBindingResult(model: null, key: property.PropertyName, isModelSet: false)); - var firstNameProperty = dto.PropertyMetadata.Single(o => o.PropertyName == "FirstName"); - dto.Results[firstNameProperty] = new ModelBindingResult( - "John", + var firstNameProperty = containerMetadata.Properties[nameof(model.FirstName)]; + results[firstNameProperty] = new ModelBindingResult( + model: "John", isModelSet: true, - key: ""); + key: nameof(model.FirstName)); - var lastNameProperty = dto.PropertyMetadata.Single(o => o.PropertyName == "LastName"); - dto.Results[lastNameProperty] = new ModelBindingResult( - "Doe", + var lastNameProperty = containerMetadata.Properties[nameof(model.LastName)]; + results[lastNameProperty] = new ModelBindingResult( + model: "Doe", isModelSet: true, - key: ""); + key: nameof(model.LastName)); - var dobProperty = dto.PropertyMetadata.Single(o => o.PropertyName == "DateOfBirth"); - dto.Results[dobProperty] = null; var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); - var testableBinder = new TestableMutableObjectModelBinder(); // Act - testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); + testableBinder.ProcessResults(bindingContext, results, modelValidationNode); // Assert Assert.Equal("John", model.FirstName); @@ -1283,15 +1287,34 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Ensure that we add child nodes for all the nodes which have a result (irrespective of if they // are bound or not). - Assert.Equal(2, modelValidationNode.ChildNodes.Count()); + Assert.Equal(5, modelValidationNode.ChildNodes.Count); - var validationNode = modelValidationNode.ChildNodes[0]; - Assert.Equal("", validationNode.Key); - Assert.Equal("John", validationNode.Model); - - validationNode = modelValidationNode.ChildNodes[1]; - Assert.Equal("", validationNode.Key); - Assert.Equal("Doe", validationNode.Model); + Assert.Collection(modelValidationNode.ChildNodes, + child => + { + Assert.Equal(nameof(model.DateOfBirth), child.Key); + Assert.Equal(null, child.Model); + }, + child => + { + Assert.Equal(nameof(model.DateOfDeath), child.Key); + Assert.Equal(null, child.Model); + }, + child => + { + Assert.Equal(nameof(model.FirstName), child.Key); + Assert.Equal("John", child.Model); + }, + child => + { + Assert.Equal(nameof(model.LastName), child.Key); + Assert.Equal("Doe", child.Model); + }, + child => + { + Assert.Equal(nameof(model.NonUpdateableProperty), child.Key); + Assert.Equal(null, child.Model); + }); } [Fact] @@ -1303,21 +1326,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider; var modelExplorer = metadataProvider.GetModelExplorerForType(typeof(Person), model); - var propertyMetadata = bindingContext.ModelMetadata.Properties["PropertyWithDefaultValue"]; - - var dtoResult = new ModelBindingResult( - model: null, - isModelSet: false, - key: "foo"); + var propertyMetadata = bindingContext.ModelMetadata.Properties[nameof(model.PropertyWithDefaultValue)]; + var result = new ModelBindingResult(model: null, isModelSet: false, key: "foo"); var testableBinder = new TestableMutableObjectModelBinder(); // Act - testableBinder.SetProperty( - bindingContext, - modelExplorer, - propertyMetadata, - dtoResult); + testableBinder.SetProperty(bindingContext, modelExplorer, propertyMetadata, result); // Assert var person = Assert.IsType(bindingContext.Model); @@ -1334,22 +1349,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider; var modelExplorer = metadataProvider.GetModelExplorerForType(typeof(Person), model); - var propertyMetadata = bindingContext.ModelMetadata.Properties["PropertyWithInitializedValue"]; + var propertyMetadata = bindingContext.ModelMetadata.Properties[nameof(model.PropertyWithInitializedValue)]; // This value won't be used because IsModelBound = false. - var dtoResult = new ModelBindingResult( - model: "bad-value", - isModelSet: false, - key: "foo"); + var result = new ModelBindingResult(model: "bad-value", isModelSet: false, key: "foo"); var testableBinder = new TestableMutableObjectModelBinder(); // Act - testableBinder.SetProperty( - bindingContext, - modelExplorer, - propertyMetadata, - dtoResult); + testableBinder.SetProperty(bindingContext, modelExplorer, propertyMetadata, result); // Assert var person = Assert.IsType(bindingContext.Model); @@ -1366,22 +1374,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider; var modelExplorer = metadataProvider.GetModelExplorerForType(typeof(Person), model); - var propertyMetadata = bindingContext.ModelMetadata.Properties["PropertyWithInitializedValueAndDefault"]; + var propertyMetadata = + bindingContext.ModelMetadata.Properties[nameof(model.PropertyWithInitializedValueAndDefault)]; // This value won't be used because IsModelBound = false. - var dtoResult = new ModelBindingResult( - model: "bad-value", - isModelSet: false, - key: "foo"); + var result = new ModelBindingResult(model: "bad-value", isModelSet: false, key: "foo"); var testableBinder = new TestableMutableObjectModelBinder(); // Act - testableBinder.SetProperty( - bindingContext, - modelExplorer, - propertyMetadata, - dtoResult); + testableBinder.SetProperty(bindingContext, modelExplorer, propertyMetadata, result); // Assert var person = Assert.IsType(bindingContext.Model); @@ -1398,21 +1400,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider; var modelExplorer = metadataProvider.GetModelExplorerForType(typeof(Person), model); - var propertyMetadata = bindingContext.ModelMetadata.Properties["NonUpdateableProperty"]; - - var dtoResult = new ModelBindingResult( - model: null, - isModelSet: false, - key: "foo"); + var propertyMetadata = bindingContext.ModelMetadata.Properties[nameof(model.NonUpdateableProperty)]; + var result = new ModelBindingResult(model: null, isModelSet: false, key: "foo"); var testableBinder = new TestableMutableObjectModelBinder(); // Act - testableBinder.SetProperty( - bindingContext, - modelExplorer, - propertyMetadata, - dtoResult); + testableBinder.SetProperty(bindingContext, modelExplorer, propertyMetadata, result); // Assert // If didn't throw, success! @@ -1453,7 +1447,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var modelExplorer = metadataProvider.GetModelExplorerForType(type, model); var propertyMetadata = bindingContext.ModelMetadata.Properties[propertyName]; - var dtoResult = new ModelBindingResult( + var result = new ModelBindingResult( model: new Simple { Name = "Hanna" }, isModelSet: true, key: propertyName); @@ -1461,11 +1455,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var testableBinder = new TestableMutableObjectModelBinder(); // Act - testableBinder.SetProperty( - bindingContext, - modelExplorer, - propertyMetadata, - dtoResult); + testableBinder.SetProperty(bindingContext, modelExplorer, propertyMetadata, result); // Assert Assert.Equal("Joe", propertAccessor(model)); @@ -1535,16 +1525,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var modelExplorer = metadataProvider.GetModelExplorerForType(type, model); var propertyMetadata = bindingContext.ModelMetadata.Properties[propertyName]; - var dtoResult = new ModelBindingResult(model: collection, isModelSet: true, key: propertyName); - + var result = new ModelBindingResult(model: collection, isModelSet: true, key: propertyName); var testableBinder = new TestableMutableObjectModelBinder(); // Act - testableBinder.SetProperty( - bindingContext, - modelExplorer, - propertyMetadata, - dtoResult); + testableBinder.SetProperty(bindingContext, modelExplorer, propertyMetadata, result); // Assert Assert.Equal(collection, propertyAccessor(model)); @@ -1561,21 +1546,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider; var modelExplorer = metadataProvider.GetModelExplorerForType(typeof(Person), model); - var propertyMetadata = bindingContext.ModelMetadata.Properties["DateOfBirth"]; - - var dtoResult = new ModelBindingResult( - new DateTime(2001, 1, 1), - key: "foo", - isModelSet: true); + var propertyMetadata = bindingContext.ModelMetadata.Properties[nameof(model.DateOfBirth)]; + var result = new ModelBindingResult(new DateTime(2001, 1, 1), key: "foo", isModelSet: true); var testableBinder = new TestableMutableObjectModelBinder(); // Act - testableBinder.SetProperty( - bindingContext, - modelExplorer, - propertyMetadata, - dtoResult); + testableBinder.SetProperty(bindingContext, modelExplorer, propertyMetadata, result); // Assert Assert.True(bindingContext.ModelState.IsValid); @@ -1596,21 +1573,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider; var modelExplorer = metadataProvider.GetModelExplorerForType(typeof(Person), model); - var propertyMetadata = bindingContext.ModelMetadata.Properties["DateOfDeath"]; - - var dtoResult = new ModelBindingResult( - new DateTime(1800, 1, 1), - isModelSet: true, - key: "foo"); + var propertyMetadata = bindingContext.ModelMetadata.Properties[nameof(model.DateOfDeath)]; + var result = new ModelBindingResult(new DateTime(1800, 1, 1), isModelSet: true, key: "foo"); var testableBinder = new TestableMutableObjectModelBinder(); // Act - testableBinder.SetProperty( - bindingContext, - modelExplorer, - propertyMetadata, - dtoResult); + testableBinder.SetProperty(bindingContext, modelExplorer, propertyMetadata, result); // Assert Assert.Equal("Date of death can't be before date of birth." + Environment.NewLine @@ -1629,21 +1598,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider; var modelExplorer = metadataProvider.GetModelExplorerForType(typeof(Person), model); - var propertyMetadata = bindingContext.ModelMetadata.Properties["DateOfBirth"]; - - var dtoResult = new ModelBindingResult( - model: null, - isModelSet: true, - key: "foo.DateOfBirth"); + var propertyMetadata = bindingContext.ModelMetadata.Properties[nameof(model.DateOfBirth)]; + var result = new ModelBindingResult(model: null, isModelSet: true, key: "foo.DateOfBirth"); var testableBinder = new TestableMutableObjectModelBinder(); // Act - testableBinder.SetProperty( - bindingContext, - modelExplorer, - propertyMetadata, - dtoResult); + testableBinder.SetProperty(bindingContext, modelExplorer, propertyMetadata, result); // Assert Assert.False(bindingContext.ModelState.IsValid); @@ -1665,21 +1626,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider; var modelExplorer = metadataProvider.GetModelExplorerForType(typeof(ModelWhosePropertySetterThrows), model); - var propertyMetadata = bindingContext.ModelMetadata.Properties["NameNoAttribute"]; - - var dtoResult = new ModelBindingResult( - model: null, - isModelSet: true, - key: "foo.NameNoAttribute"); + var propertyMetadata = bindingContext.ModelMetadata.Properties[nameof(model.NameNoAttribute)]; + var result = new ModelBindingResult(model: null, isModelSet: true, key: "foo.NameNoAttribute"); var testableBinder = new TestableMutableObjectModelBinder(); // Act - testableBinder.SetProperty( - bindingContext, - modelExplorer, - propertyMetadata, - dtoResult); + testableBinder.SetProperty(bindingContext, modelExplorer, propertyMetadata, result); // Assert Assert.False(bindingContext.ModelState.IsValid); @@ -2014,14 +1967,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return CreateModelPublic(bindingContext); } - public virtual void EnsureModelPublic(ModelBindingContext bindingContext) + public virtual object GetModelPublic(ModelBindingContext bindingContext) { - base.EnsureModel(bindingContext); + return base.GetModel(bindingContext); } - protected override void EnsureModel(ModelBindingContext bindingContext) + protected override object GetModel(ModelBindingContext bindingContext) { - EnsureModelPublic(bindingContext); + return GetModelPublic(bindingContext); } public virtual new IEnumerable GetMetadataForProperties(ModelBindingContext bindingContext) @@ -2033,13 +1986,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding ModelBindingContext bindingContext, ModelExplorer modelExplorer, ModelMetadata propertyMetadata, - ModelBindingResult dtoResult) + ModelBindingResult result) { - base.SetProperty( - bindingContext, - modelExplorer, - propertyMetadata, - dtoResult); + base.SetProperty(bindingContext, modelExplorer, propertyMetadata, result); } } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBindingHelperTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBindingHelperTest.cs index 87018515af..044e8adbfa 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBindingHelperTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBindingHelperTest.cs @@ -55,7 +55,6 @@ namespace Microsoft.AspNet.Mvc.Test var binders = new IModelBinder[] { new TypeConverterModelBinder(), - new ComplexModelDtoModelBinder(), new MutableObjectModelBinder() }; @@ -95,7 +94,6 @@ namespace Microsoft.AspNet.Mvc.Test var binders = new IModelBinder[] { new TypeConverterModelBinder(), - new ComplexModelDtoModelBinder(), new MutableObjectModelBinder() }; @@ -168,7 +166,6 @@ namespace Microsoft.AspNet.Mvc.Test var binders = new IModelBinder[] { new TypeConverterModelBinder(), - new ComplexModelDtoModelBinder(), new MutableObjectModelBinder() }; @@ -256,7 +253,6 @@ namespace Microsoft.AspNet.Mvc.Test var binders = new IModelBinder[] { new TypeConverterModelBinder(), - new ComplexModelDtoModelBinder(), new MutableObjectModelBinder() }; @@ -309,7 +305,6 @@ namespace Microsoft.AspNet.Mvc.Test var binders = new IModelBinder[] { new TypeConverterModelBinder(), - new ComplexModelDtoModelBinder(), new MutableObjectModelBinder() }; @@ -513,7 +508,6 @@ namespace Microsoft.AspNet.Mvc.Test var binders = new IModelBinder[] { new TypeConverterModelBinder(), - new ComplexModelDtoModelBinder(), new MutableObjectModelBinder() }; @@ -603,7 +597,6 @@ namespace Microsoft.AspNet.Mvc.Test var binders = new IModelBinder[] { new TypeConverterModelBinder(), - new ComplexModelDtoModelBinder(), new MutableObjectModelBinder() }; diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs index 782cac6fea..d7c97b7853 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Linq; using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Mvc.ModelBinding; @@ -16,10 +15,12 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests private class Address { public string Street { get; set; } + + public string City { get; set; } } [Fact] - public async Task TryUpdateModel_ExistingModel_EmptyPrefix_GetsOverWritten() + public async Task TryUpdateModel_ExistingModel_EmptyPrefix_OverwritesBoundValues() { // Arrange var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => @@ -28,7 +29,11 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests }); var modelState = new ModelStateDictionary(); - var model = new Address { Street = "DefaultStreet" }; + var model = new Address + { + Street = "DefaultStreet", + City = "Toronto", + }; var oldModel = model; // Act @@ -40,17 +45,19 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests // Model Assert.Same(oldModel, model); Assert.Equal("SomeStreet", model.Street); + Assert.Equal("Toronto", model.City); // ModelState Assert.True(modelState.IsValid); - Assert.Equal(1, modelState.Keys.Count); - var key = Assert.Single(modelState.Keys, k => k == "Street"); - Assert.NotNull(modelState[key].Value); - Assert.Equal("SomeStreet", modelState[key].Value.AttemptedValue); - Assert.Equal("SomeStreet", modelState[key].Value.RawValue); - Assert.Empty(modelState[key].Errors); - Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState); + var entry = Assert.Single(modelState); + Assert.Equal("Street", entry.Key); + var state = entry.Value; + Assert.NotNull(state.Value); + Assert.Equal("SomeStreet", state.Value.AttemptedValue); + Assert.Equal("SomeStreet", state.Value.RawValue); + Assert.Empty(state.Errors); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); } [Fact] @@ -64,6 +71,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var modelState = new ModelStateDictionary(); var model = new Address(); + // Act var result = await TryUpdateModel(model, string.Empty, operationContext, modelState); @@ -72,17 +80,72 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests // Model Assert.Equal("SomeStreet", model.Street); + Assert.Null(model.City); // ModelState Assert.True(modelState.IsValid); - Assert.Equal(1, modelState.Keys.Count); - var key = Assert.Single(modelState.Keys, k => k == "Street"); - Assert.NotNull(modelState[key].Value); - Assert.Equal("SomeStreet", modelState[key].Value.AttemptedValue); - Assert.Equal("SomeStreet", modelState[key].Value.RawValue); - Assert.Empty(modelState[key].Errors); - Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState); + var entry = Assert.Single(modelState); + Assert.Equal("Street", entry.Key); + var state = entry.Value; + Assert.NotNull(state.Value); + Assert.Equal("SomeStreet", state.Value.AttemptedValue); + Assert.Equal("SomeStreet", state.Value.RawValue); + Assert.Empty(state.Errors); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); + } + + private class Person1 + { + public string Name { get; set; } + + public Address Address { get; set; } + } + + [Fact] + public async Task TryUpdateModel_NestedPoco_EmptyPrefix_DoesNotTrounceUnboundValues() + { + // Arrange + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.QueryString = QueryString.Create("Address.Street", "SomeStreet"); + }); + + var modelState = new ModelStateDictionary(); + var model = new Person1 + { + Name = "Joe", + Address = new Address + { + Street = "DefaultStreet", + City = "Toronto", + }, + }; + var oldModel = model; + + // Act + var result = await TryUpdateModel(model, string.Empty, operationContext, modelState); + + // Assert + Assert.True(result); + + // Model + Assert.Same(oldModel, model); + Assert.Equal("Joe", model.Name); + Assert.Equal("SomeStreet", model.Address.Street); + Assert.Equal("Toronto", model.Address.City); + + // ModelState + Assert.True(modelState.IsValid); + + var entry = Assert.Single(modelState); + Assert.Equal("Address.Street", entry.Key); + var state = entry.Value; + Assert.NotNull(state.Value); + Assert.Equal("SomeStreet", state.Value.AttemptedValue); + Assert.Equal("SomeStreet", state.Value.RawValue); + Assert.Empty(state.Errors); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); } private class Person2 @@ -91,7 +154,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests } [Fact] - public async Task TryUpdateModel_SettableCollectionModel_EmptyPrefix_GetsBound() + public async Task TryUpdateModel_SettableCollectionModel_EmptyPrefix_CreatesCollection() { // Arrange var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => @@ -101,6 +164,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var modelState = new ModelStateDictionary(); var model = new Person2(); + // Act var result = await TryUpdateModel(model, string.Empty, operationContext, modelState); @@ -111,17 +175,61 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.NotNull(model.Address); Assert.Equal(1, model.Address.Count); Assert.Equal("SomeStreet", model.Address[0].Street); + Assert.Null(model.Address[0].City); // ModelState Assert.True(modelState.IsValid); - Assert.Equal(1, modelState.Keys.Count); - var key = Assert.Single(modelState.Keys, k => k == "Address[0].Street"); - Assert.NotNull(modelState[key].Value); - Assert.Equal("SomeStreet", modelState[key].Value.AttemptedValue); - Assert.Equal("SomeStreet", modelState[key].Value.RawValue); - Assert.Empty(modelState[key].Errors); - Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState); + var entry = Assert.Single(modelState); + Assert.Equal("Address[0].Street", entry.Key); + var state = entry.Value; + Assert.NotNull(state.Value); + Assert.Equal("SomeStreet", state.Value.AttemptedValue); + Assert.Equal("SomeStreet", state.Value.RawValue); + Assert.Empty(state.Errors); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); + } + + [Fact] + public async Task TryUpdateModel_SettableCollectionModel_EmptyPrefix_MaintainsCollectionIfNonNull() + { + // Arrange + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.QueryString = QueryString.Create("Address[0].Street", "SomeStreet"); + }); + + var modelState = new ModelStateDictionary(); + var model = new Person2 + { + Address = new List
(), + }; + var collection = model.Address; + + // Act + var result = await TryUpdateModel(model, string.Empty, operationContext, modelState); + + // Assert + Assert.True(result); + + // Model + Assert.NotNull(model.Address); + Assert.Same(collection, model.Address); + Assert.Equal(1, model.Address.Count); + Assert.Equal("SomeStreet", model.Address[0].Street); + Assert.Null(model.Address[0].City); + + // ModelState + Assert.True(modelState.IsValid); + + var entry = Assert.Single(modelState); + Assert.Equal("Address[0].Street", entry.Key); + var state = entry.Value; + Assert.NotNull(state.Value); + Assert.Equal("SomeStreet", state.Value.AttemptedValue); + Assert.Equal("SomeStreet", state.Value.RawValue); + Assert.Empty(state.Errors); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); } private class Person3 @@ -144,28 +252,46 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests }); var modelState = new ModelStateDictionary(); - var model = new Person3(); + var model = new Person3 + { + Address = + { + new Address + { + Street = "Old street", + City = "Redmond", + }, + new Address + { + Street = "Older street", + City = "Toronto", + }, + }, + }; + // Act var result = await TryUpdateModel(model, string.Empty, operationContext, modelState); // Assert Assert.True(result); - // Model + // Model (collection is cleared and new members created from scratch). Assert.NotNull(model.Address); Assert.Equal(1, model.Address.Count); Assert.Equal("SomeStreet", model.Address[0].Street); + Assert.Null(model.Address[0].City); // ModelState Assert.True(modelState.IsValid); - Assert.Equal(1, modelState.Keys.Count); - var key = Assert.Single(modelState.Keys, k => k == "Address[0].Street"); - Assert.NotNull(modelState[key].Value); - Assert.Equal("SomeStreet", modelState[key].Value.AttemptedValue); - Assert.Equal("SomeStreet", modelState[key].Value.RawValue); - Assert.Empty(modelState[key].Errors); - Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState); + var entry = Assert.Single(modelState); + Assert.Equal("Address[0].Street", entry.Key); + var state = entry.Value; + Assert.NotNull(state.Value); + Assert.Equal("SomeStreet", state.Value.AttemptedValue); + Assert.Equal("SomeStreet", state.Value.RawValue); + Assert.Empty(state.Errors); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); } private class Person6 @@ -213,7 +339,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests } [Fact] - public async Task TryUpdateModel_SettableArrayModel_EmptyPrefix_GetsBound() + public async Task TryUpdateModel_SettableArrayModel_EmptyPrefix_CreatesArray() { // Arrange var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => @@ -223,6 +349,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var modelState = new ModelStateDictionary(); var model = new Person4(); + // Act var result = await TryUpdateModel(model, string.Empty, operationContext, modelState); @@ -231,19 +358,70 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests // Model Assert.NotNull(model.Address); - Assert.Equal(1, model.Address.Count()); + Assert.Equal(1, model.Address.Length); Assert.Equal("SomeStreet", model.Address[0].Street); + Assert.Null(model.Address[0].City); // ModelState Assert.True(modelState.IsValid); - Assert.Equal(1, modelState.Keys.Count); - var key = Assert.Single(modelState.Keys, k => k == "Address[0].Street"); - Assert.NotNull(modelState[key].Value); - Assert.Equal("SomeStreet", modelState[key].Value.AttemptedValue); - Assert.Equal("SomeStreet", modelState[key].Value.RawValue); - Assert.Empty(modelState[key].Errors); - Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState); + var entry = Assert.Single(modelState); + Assert.Equal("Address[0].Street", entry.Key); + var state = entry.Value; + Assert.NotNull(state.Value); + Assert.Equal("SomeStreet", state.Value.AttemptedValue); + Assert.Equal("SomeStreet", state.Value.RawValue); + Assert.Empty(state.Errors); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); + } + + [Fact] + public async Task TryUpdateModel_SettableArrayModel_EmptyPrefix_OverwritesArray() + { + // Arrange + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.QueryString = QueryString.Create("Address[0].Street", "SomeStreet"); + }); + + var modelState = new ModelStateDictionary(); + var model = new Person4 + { + Address = new Address[] + { + new Address + { + Street = "Old street", + City = "Toronto", + }, + }, + }; + var collection = model.Address; + + // Act + var result = await TryUpdateModel(model, string.Empty, operationContext, modelState); + + // Assert + Assert.True(result); + + // Model + Assert.NotNull(model.Address); + Assert.NotSame(collection, model.Address); + Assert.Equal(1, model.Address.Length); + Assert.Equal("SomeStreet", model.Address[0].Street); + Assert.Null(model.Address[0].City); + + // ModelState + Assert.True(modelState.IsValid); + + var entry = Assert.Single(modelState); + Assert.Equal("Address[0].Street", entry.Key); + var state = entry.Value; + Assert.NotNull(state.Value); + Assert.Equal("SomeStreet", state.Value.AttemptedValue); + Assert.Equal("SomeStreet", state.Value.RawValue); + Assert.Empty(state.Errors); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); } private class Person5 @@ -262,6 +440,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var modelState = new ModelStateDictionary(); var model = new Person5(); + // Act var result = await TryUpdateModel(model, string.Empty, operationContext, modelState); @@ -272,16 +451,16 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.NotNull(model.Address); // Arrays should not be updated. - Assert.Equal(0, model.Address.Count()); + Assert.Equal(0, model.Address.Length); // ModelState Assert.True(modelState.IsValid); - Assert.Empty(modelState.Keys); + Assert.Empty(modelState); } [Fact] - public async Task TryUpdateModel_ExistingModel_WithPrefix_GetsOverWritten() + public async Task TryUpdateModel_ExistingModel_WithPrefix_ValuesGetOverwritten() { // Arrange var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => @@ -290,7 +469,11 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests }); var modelState = new ModelStateDictionary(); - var model = new Address { Street = "DefaultStreet" }; + var model = new Address + { + Street = "DefaultStreet", + City = "Toronto", + }; var oldModel = model; // Act @@ -302,17 +485,19 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests // Model Assert.Same(oldModel, model); Assert.Equal("SomeStreet", model.Street); + Assert.Equal("Toronto", model.City); // ModelState Assert.True(modelState.IsValid); - Assert.Equal(1, modelState.Keys.Count); - var key = Assert.Single(modelState.Keys, k => k == "prefix.Street"); - Assert.NotNull(modelState[key].Value); - Assert.Equal("SomeStreet", modelState[key].Value.AttemptedValue); - Assert.Equal("SomeStreet", modelState[key].Value.RawValue); - Assert.Empty(modelState[key].Errors); - Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState); + var entry = Assert.Single(modelState); + Assert.Equal("prefix.Street", entry.Key); + var state = entry.Value; + Assert.NotNull(state.Value); + Assert.Equal("SomeStreet", state.Value.AttemptedValue); + Assert.Equal("SomeStreet", state.Value.RawValue); + Assert.Empty(state.Errors); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); } [Fact] @@ -326,6 +511,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var modelState = new ModelStateDictionary(); var model = new Address(); + // Act var result = await TryUpdateModel(model, "prefix", operationContext, modelState); @@ -334,21 +520,69 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests // Model Assert.Equal("SomeStreet", model.Street); + Assert.Null(model.City); // ModelState Assert.True(modelState.IsValid); - Assert.Equal(1, modelState.Keys.Count); - var key = Assert.Single(modelState.Keys, k => k == "prefix.Street"); - Assert.NotNull(modelState[key].Value); - Assert.Equal("SomeStreet", modelState[key].Value.AttemptedValue); - Assert.Equal("SomeStreet", modelState[key].Value.RawValue); - Assert.Empty(modelState[key].Errors); - Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState); + var entry = Assert.Single(modelState); + Assert.Equal("prefix.Street", entry.Key); + var state = entry.Value; + Assert.NotNull(state.Value); + Assert.Equal("SomeStreet", state.Value.AttemptedValue); + Assert.Equal("SomeStreet", state.Value.RawValue); + Assert.Empty(state.Errors); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); } [Fact] - public async Task TryUpdateModel_SettableCollectionModel_WithPrefix_GetsBound() + public async Task TryUpdateModel_NestedPoco_WithPrefix_DoesNotTrounceUnboundValues() + { + // Arrange + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.QueryString = QueryString.Create("prefix.Address.Street", "SomeStreet"); + }); + + var modelState = new ModelStateDictionary(); + var model = new Person1 + { + Name = "Joe", + Address = new Address + { + Street = "DefaultStreet", + City = "Toronto", + }, + }; + var oldModel = model; + + // Act + var result = await TryUpdateModel(model, "prefix", operationContext, modelState); + + // Assert + Assert.True(result); + + // Model + Assert.Same(oldModel, model); + Assert.Equal("Joe", model.Name); + Assert.Equal("SomeStreet", model.Address.Street); + Assert.Equal("Toronto", model.Address.City); + + // ModelState + Assert.True(modelState.IsValid); + + var entry = Assert.Single(modelState); + Assert.Equal("prefix.Address.Street", entry.Key); + var state = entry.Value; + Assert.NotNull(state.Value); + Assert.Equal("SomeStreet", state.Value.AttemptedValue); + Assert.Equal("SomeStreet", state.Value.RawValue); + Assert.Empty(state.Errors); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); + } + + [Fact] + public async Task TryUpdateModel_SettableCollectionModel_WithPrefix_CreatesCollection() { // Arrange var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => @@ -358,6 +592,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var modelState = new ModelStateDictionary(); var model = new Person2(); + // Act var result = await TryUpdateModel(model, "prefix", operationContext, modelState); @@ -368,17 +603,61 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.NotNull(model.Address); Assert.Equal(1, model.Address.Count); Assert.Equal("SomeStreet", model.Address[0].Street); + Assert.Null(model.Address[0].City); // ModelState Assert.True(modelState.IsValid); - Assert.Equal(1, modelState.Keys.Count); - var key = Assert.Single(modelState.Keys, k => k == "prefix.Address[0].Street"); - Assert.NotNull(modelState[key].Value); - Assert.Equal("SomeStreet", modelState[key].Value.AttemptedValue); - Assert.Equal("SomeStreet", modelState[key].Value.RawValue); - Assert.Empty(modelState[key].Errors); - Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState); + var entry = Assert.Single(modelState); + Assert.Equal("prefix.Address[0].Street", entry.Key); + var state = entry.Value; + Assert.NotNull(state.Value); + Assert.Equal("SomeStreet", state.Value.AttemptedValue); + Assert.Equal("SomeStreet", state.Value.RawValue); + Assert.Empty(state.Errors); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); + } + + [Fact] + public async Task TryUpdateModel_SettableCollectionModel_WithPrefix_MaintainsCollectionIfNonNull() + { + // Arrange + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.QueryString = QueryString.Create("prefix.Address[0].Street", "SomeStreet"); + }); + + var modelState = new ModelStateDictionary(); + var model = new Person2 + { + Address = new List
(), + }; + var collection = model.Address; + + // Act + var result = await TryUpdateModel(model, "prefix", operationContext, modelState); + + // Assert + Assert.True(result); + + // Model + Assert.NotNull(model.Address); + Assert.Same(collection, model.Address); + Assert.Equal(1, model.Address.Count); + Assert.Equal("SomeStreet", model.Address[0].Street); + Assert.Null(model.Address[0].City); + + // ModelState + Assert.True(modelState.IsValid); + + var entry = Assert.Single(modelState); + Assert.Equal("prefix.Address[0].Street", entry.Key); + var state = entry.Value; + Assert.NotNull(state.Value); + Assert.Equal("SomeStreet", state.Value.AttemptedValue); + Assert.Equal("SomeStreet", state.Value.RawValue); + Assert.Empty(state.Errors); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); } [Fact] @@ -391,28 +670,46 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests }); var modelState = new ModelStateDictionary(); - var model = new Person3(); + var model = new Person3 + { + Address = + { + new Address + { + Street = "Old street", + City = "Redmond", + }, + new Address + { + Street = "Older street", + City = "Toronto", + }, + }, + }; + // Act var result = await TryUpdateModel(model, "prefix", operationContext, modelState); // Assert Assert.True(result); - // Model + // Model (collection is cleared and new members created from scratch). Assert.NotNull(model.Address); Assert.Equal(1, model.Address.Count); Assert.Equal("SomeStreet", model.Address[0].Street); + Assert.Null(model.Address[0].City); // ModelState Assert.True(modelState.IsValid); - Assert.Equal(1, modelState.Keys.Count); - var key = Assert.Single(modelState.Keys, k => k == "prefix.Address[0].Street"); - Assert.NotNull(modelState[key].Value); - Assert.Equal("SomeStreet", modelState[key].Value.AttemptedValue); - Assert.Equal("SomeStreet", modelState[key].Value.RawValue); - Assert.Empty(modelState[key].Errors); - Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState); + var entry = Assert.Single(modelState); + Assert.Equal("prefix.Address[0].Street", entry.Key); + var state = entry.Value; + Assert.NotNull(state.Value); + Assert.Equal("SomeStreet", state.Value.AttemptedValue); + Assert.Equal("SomeStreet", state.Value.RawValue); + Assert.Empty(state.Errors); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); } [Fact(Skip = "Validation incorrect for collections when using TryUpdateModel, #2941")] @@ -450,7 +747,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests } [Fact] - public async Task TryUpdateModel_SettableArrayModel_WithPrefix_GetsBound() + public async Task TryUpdateModel_SettableArrayModel_WithPrefix_CreatesArray() { // Arrange var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => @@ -460,6 +757,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var modelState = new ModelStateDictionary(); var model = new Person4(); + // Act var result = await TryUpdateModel(model, "prefix", operationContext, modelState); @@ -468,23 +766,74 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests // Model Assert.NotNull(model.Address); - Assert.Equal(1, model.Address.Count()); + Assert.Equal(1, model.Address.Length); Assert.Equal("SomeStreet", model.Address[0].Street); + Assert.Null(model.Address[0].City); // ModelState Assert.True(modelState.IsValid); - Assert.Equal(1, modelState.Keys.Count); - var key = Assert.Single(modelState.Keys, k => k == "prefix.Address[0].Street"); - Assert.NotNull(modelState[key].Value); - Assert.Equal("SomeStreet", modelState[key].Value.AttemptedValue); - Assert.Equal("SomeStreet", modelState[key].Value.RawValue); - Assert.Empty(modelState[key].Errors); - Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState); + var entry = Assert.Single(modelState); + Assert.Equal("prefix.Address[0].Street", entry.Key); + var state = entry.Value; + Assert.NotNull(state.Value); + Assert.Equal("SomeStreet", state.Value.AttemptedValue); + Assert.Equal("SomeStreet", state.Value.RawValue); + Assert.Empty(state.Errors); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); } [Fact] - public async Task TryUpdateModel_NonSettableArrayModel_WithPrefix_DoesNotGetBound() + public async Task TryUpdateModel_SettableArrayModel_WithPrefix_OverwritesArray() + { + // Arrange + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.QueryString = QueryString.Create("prefix.Address[0].Street", "SomeStreet"); + }); + + var modelState = new ModelStateDictionary(); + var model = new Person4 + { + Address = new Address[] + { + new Address + { + Street = "Old street", + City = "Toronto", + }, + }, + }; + var collection = model.Address; + + // Act + var result = await TryUpdateModel(model, "prefix", operationContext, modelState); + + // Assert + Assert.True(result); + + // Model + Assert.NotNull(model.Address); + Assert.NotSame(collection, model.Address); + Assert.Equal(1, model.Address.Length); + Assert.Equal("SomeStreet", model.Address[0].Street); + Assert.Null(model.Address[0].City); + + // ModelState + Assert.True(modelState.IsValid); + + var entry = Assert.Single(modelState); + Assert.Equal("prefix.Address[0].Street", entry.Key); + var state = entry.Value; + Assert.NotNull(state.Value); + Assert.Equal("SomeStreet", state.Value.AttemptedValue); + Assert.Equal("SomeStreet", state.Value.RawValue); + Assert.Empty(state.Errors); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); + } + + [Fact] + public async Task TryUpdateModel_NonSettableArrayModel_WithPrefix_GetsBound() { // Arrange var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => @@ -494,6 +843,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var modelState = new ModelStateDictionary(); var model = new Person5(); + // Act var result = await TryUpdateModel(model, "prefix", operationContext, modelState); @@ -504,11 +854,11 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.NotNull(model.Address); // Arrays should not be updated. - Assert.Equal(0, model.Address.Count()); + Assert.Equal(0, model.Address.Length); // ModelState Assert.True(modelState.IsValid); - Assert.Empty(modelState.Keys); + Assert.Empty(modelState); } private class CustomReadOnlyCollection : ICollection diff --git a/test/Microsoft.AspNet.Mvc.Test/MvcOptionsSetupTest.cs b/test/Microsoft.AspNet.Mvc.Test/MvcOptionsSetupTest.cs index 7da30076d3..1f55e2b444 100644 --- a/test/Microsoft.AspNet.Mvc.Test/MvcOptionsSetupTest.cs +++ b/test/Microsoft.AspNet.Mvc.Test/MvcOptionsSetupTest.cs @@ -39,7 +39,7 @@ namespace Microsoft.AspNet.Mvc // Assert var i = 0; - Assert.Equal(13, options.ModelBinders.Count); + Assert.Equal(12, options.ModelBinders.Count); Assert.IsType(typeof(BinderTypeBasedModelBinder), options.ModelBinders[i++]); Assert.IsType(typeof(ServicesModelBinder), options.ModelBinders[i++]); Assert.IsType(typeof(BodyModelBinder), options.ModelBinders[i++]); @@ -52,7 +52,6 @@ namespace Microsoft.AspNet.Mvc Assert.IsType(typeof(FormCollectionModelBinder), options.ModelBinders[i++]); Assert.IsType(typeof(GenericModelBinder), options.ModelBinders[i++]); Assert.IsType(typeof(MutableObjectModelBinder), options.ModelBinders[i++]); - Assert.IsType(typeof(ComplexModelDtoModelBinder), options.ModelBinders[i++]); } [Fact]