From 02cc82a0554d6e4c0ccf91e806d6fb87be0c5678 Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Fri, 21 Aug 2015 10:45:43 -0700 Subject: [PATCH] PR comments commit --- .../ModelBindingHelper.cs | 14 ++-- .../ModelBindingHelperTest.cs | 72 +++++++++---------- .../TryUpdateModelIntegrationTest.cs | 61 +++++++++++++++- 3 files changed, 106 insertions(+), 41 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBindingHelper.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBindingHelper.cs index c80ec1e80f..59fb7a58f2 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBindingHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBindingHelper.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using System.Linq.Expressions; using System.Reflection; @@ -313,9 +312,16 @@ namespace Microsoft.AspNet.Mvc var modelValidationContext = new ModelValidationContext(modelBindingContext, modelExplorer); var validationNode = modelBindingResult.ValidationNode; - Debug.Assert( - validationNode != null, - "ValidationNode should never be null in a successful ModelBindingResult."); + if (validationNode == null) + { + validationNode = new ModelValidationNode( + modelBindingResult.Key, + modelMetadata, + modelBindingResult.Model) + { + ValidateAllProperties = true, + }; + } objectModelValidator.Validate(modelValidationContext, validationNode); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBindingHelperTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBindingHelperTest.cs index fa9d8e0bc9..e3f643bb19 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBindingHelperTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBindingHelperTest.cs @@ -52,7 +52,7 @@ namespace Microsoft.AspNet.Mvc.Test GetCompositeBinder(binder.Object), Mock.Of(), new List(), - new DefaultObjectValidator(new IExcludeTypeValidationFilter[0], metadataProvider), + new Mock(MockBehavior.Strict).Object, Mock.Of()); // Assert @@ -164,7 +164,7 @@ namespace Microsoft.AspNet.Mvc.Test GetCompositeBinder(binder.Object), Mock.Of(), new List(), - Mock.Of(), + new Mock(MockBehavior.Strict).Object, Mock.Of(), includePredicate); @@ -245,17 +245,17 @@ namespace Microsoft.AspNet.Mvc.Test // Act var result = await ModelBindingHelper.TryUpdateModelAsync( - model, - null, - Mock.Of(), - new ModelStateDictionary(), - metadataProvider, - GetCompositeBinder(binder.Object), - Mock.Of(), - new List(), - Mock.Of(), - Mock.Of(), - m => m.IncludedProperty ); + model, + null, + Mock.Of(), + new ModelStateDictionary(), + metadataProvider, + GetCompositeBinder(binder.Object), + Mock.Of(), + new List(), + new Mock(MockBehavior.Strict).Object, + Mock.Of(), + m => m.IncludedProperty ); // Assert Assert.False(result); @@ -502,18 +502,18 @@ namespace Microsoft.AspNet.Mvc.Test // Act var result = await ModelBindingHelper.TryUpdateModelAsync( - model, - model.GetType(), - prefix: null, - httpContext: Mock.Of(), - modelState: new ModelStateDictionary(), - metadataProvider: metadataProvider, - modelBinder: GetCompositeBinder(binder.Object), - valueProvider: Mock.Of(), - inputFormatters: new List(), - objectModelValidator: Mock.Of(), - validatorProvider: Mock.Of(), - predicate: includePredicate); + model, + model.GetType(), + prefix: null, + httpContext: Mock.Of(), + modelState: new ModelStateDictionary(), + metadataProvider: metadataProvider, + modelBinder: GetCompositeBinder(binder.Object), + valueProvider: Mock.Of(), + inputFormatters: new List(), + objectModelValidator: new Mock(MockBehavior.Strict).Object, + validatorProvider: Mock.Of(), + predicate: includePredicate); // Assert Assert.False(result); @@ -596,17 +596,17 @@ namespace Microsoft.AspNet.Mvc.Test // Act var result = await ModelBindingHelper.TryUpdateModelAsync( - model, - modelType: model.GetType(), - prefix: null, - httpContext: Mock.Of(), - modelState: new ModelStateDictionary(), - metadataProvider: metadataProvider, - modelBinder: GetCompositeBinder(binder.Object), - valueProvider: Mock.Of(), - inputFormatters: new List(), - objectModelValidator: Mock.Of(), - validatorProvider: Mock.Of()); + model, + modelType: model.GetType(), + prefix: null, + httpContext: Mock.Of(), + modelState: new ModelStateDictionary(), + metadataProvider: metadataProvider, + modelBinder: GetCompositeBinder(binder.Object), + valueProvider: Mock.Of(), + inputFormatters: new List(), + objectModelValidator: new Mock(MockBehavior.Strict).Object, + validatorProvider: Mock.Of()); // Assert Assert.False(result); diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs index ff79f16f7e..cac95d23a1 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs @@ -138,7 +138,8 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.True(result); // Model - Assert.Collection(model, + Assert.Collection( + model, element => { Assert.Equal("One Name", element.Name); @@ -591,6 +592,64 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.Equal(ModelValidationState.Valid, state.ValidationState); } + [Fact] + public async Task TryUpdateModel_TopLevelCollection_WithPrefix_BindsAfterClearing() + { + // Arrange + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.QueryString = QueryString.Create(new Dictionary + { + { "prefix[0].Name", "One Name" }, + { "prefix[1].Address.Street", "Two Street" }, + }); + }); + + var modelState = new ModelStateDictionary(); + var model = new List + { + new Person1 + { + Name = "One", + Address = new Address + { + Street = "DefaultStreet", + City = "Toronto", + }, + }, + new Person1 { Name = "Two" }, + new Person1 { Name = "Three" }, + }; + + // Act + var result = await TryUpdateModel(model, "prefix", operationContext, modelState); + + // Assert + Assert.True(result); + + // Model + Assert.Collection( + model, + element => + { + Assert.Equal("One Name", element.Name); + Assert.Null(element.Address); + }, + element => + { + Assert.Null(element.Name); + Assert.NotNull(element.Address); + Assert.Equal("Two Street", element.Address.Street); + Assert.Null(element.Address.City); + }); + + // ModelState + Assert.True(modelState.IsValid); + Assert.Equal(2, modelState.Count); + Assert.NotNull(modelState["prefix[0].Name"]); + Assert.NotNull(modelState["prefix[1].Address.Street"]); + } + [Fact] public async Task TryUpdateModel_NestedPoco_WithPrefix_DoesNotTrounceUnboundValues() {