From 4f419eee557bac41a1e802323fa9133e04d9def7 Mon Sep 17 00:00:00 2001 From: Harsh Gupta Date: Wed, 27 May 2015 12:16:11 -0700 Subject: [PATCH] Removing creating ModelValidationNode by default in CompositeModelBinder. This moves the responsibility of of MVN creation to individual model binders if they want the individual models to be validated. --- .../DefaultControllerActionArgumentBinder.cs | 16 ++++++++++------ .../ModelBinding/ByteArrayModelBinder.cs | 8 ++++++-- .../ModelBinding/CancellationTokenModelBinder.cs | 8 +++++++- .../ModelBinding/CompositeModelBinder.cs | 15 +-------------- .../ModelBinding/FormCollectionModelBinder.cs | 8 +++++++- .../ModelBinding/FormFileModelBinder.cs | 16 ++++++++++++++-- .../ModelBinding/TypeConverterModelBinder.cs | 8 ++++++-- .../ModelBinding/TypeMatchModelBinder.cs | 8 ++++++-- .../Validation/DefaultObjectValidator.cs | 4 ++-- .../ModelBinding/KeyValuePairModelBinderTest.cs | 8 ++++++-- .../ControllerActionArgumentBinderTests.cs | 12 ++++++++---- .../ModelBindingTest.cs | 2 +- .../ModelBinderAttribute_ProductController.cs | 14 +++++++++----- 13 files changed, 83 insertions(+), 44 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs index 9609154295..ed804246b3 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs @@ -89,12 +89,16 @@ namespace Microsoft.AspNet.Mvc metadata, modelBindingResult.Model); - var validationContext = new ModelValidationContext( - modelBindingContext.BindingSource, - operationContext.ValidatorProvider, - modelState, - modelExplorer); - _validator.Validate(validationContext, modelBindingResult.ValidationNode); + if (modelBindingResult.ValidationNode != null) + { + var validationContext = new ModelValidationContext( + modelBindingContext.BindingSource, + operationContext.ValidatorProvider, + modelState, + modelExplorer); + + _validator.Validate(validationContext, modelBindingResult.ValidationNode); + } } return modelBindingResult; diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ByteArrayModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ByteArrayModelBinder.cs index 651ee7fd57..0172489263 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ByteArrayModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ByteArrayModelBinder.cs @@ -39,12 +39,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding try { var model = Convert.FromBase64String(value); + var validationNode = new ModelValidationNode( + bindingContext.ModelName, + bindingContext.ModelMetadata, + model); - // We do not need to set an explict ModelValidationNode since CompositeModelBinder does that automatically. return new ModelBindingResult( model, bindingContext.ModelName, - isModelSet: true); + isModelSet: true, + validationNode: validationNode); } catch (Exception ex) { diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CancellationTokenModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CancellationTokenModelBinder.cs index c9ebe69a4a..716345c169 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CancellationTokenModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CancellationTokenModelBinder.cs @@ -17,7 +17,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding if (bindingContext.ModelType == typeof(CancellationToken)) { var model = bindingContext.OperationBindingContext.HttpContext.RequestAborted; - return Task.FromResult(new ModelBindingResult(model, bindingContext.ModelName, isModelSet: true)); + var validationNode = + new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, model); + return Task.FromResult(new ModelBindingResult( + model, + bindingContext.ModelName, + isModelSet: true, + validationNode: validationNode)); } return Task.FromResult(null); diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs index a9d0e92b86..d546a7d6d1 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs @@ -83,24 +83,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } } - // Update the model validation node if the model binding result was set but no validation node was provided. - // This would typically be the case where leaf level model binders, do not have to add a validation node - // for validation to take effect. The composite being the entry point for model binders, takes care or - // adding missing validation nodes. - var modelValidationNode = modelBindingResult.ValidationNode; - if (modelBindingResult.IsModelSet && modelValidationNode == null) - { - modelValidationNode = new ModelValidationNode( - bindingKey, - bindingContext.ModelMetadata, - modelBindingResult.Model); - } - return new ModelBindingResult( modelBindingResult.Model, bindingKey, modelBindingResult.IsModelSet, - modelValidationNode); + modelBindingResult.ValidationNode); } private async Task TryBind(ModelBindingContext bindingContext) diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs index cdc5731e29..4c37b27c40 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs @@ -46,7 +46,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding model = new FormCollection(new Dictionary()); } - return new ModelBindingResult(model, bindingContext.ModelName, isModelSet: true); + var validationNode = + new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, model); + return new ModelBindingResult( + model, + bindingContext.ModelName, + isModelSet: true, + validationNode: validationNode); } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormFileModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormFileModelBinder.cs index e1a0bd204d..e11821619d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormFileModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormFileModelBinder.cs @@ -24,14 +24,26 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { var postedFiles = await GetFormFilesAsync(bindingContext); var value = postedFiles.FirstOrDefault(); - return new ModelBindingResult(value, bindingContext.ModelName, isModelSet: value != null); + var validationNode = + new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, value); + return new ModelBindingResult( + value, + bindingContext.ModelName, + isModelSet: value != null, + validationNode: validationNode); } else if (typeof(IEnumerable).GetTypeInfo().IsAssignableFrom( bindingContext.ModelType.GetTypeInfo())) { var postedFiles = await GetFormFilesAsync(bindingContext); var value = ModelBindingHelper.ConvertValuesToCollectionType(bindingContext.ModelType, postedFiles); - return new ModelBindingResult(value, bindingContext.ModelName, isModelSet: value != null); + var validationNode = + new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, value); + return new ModelBindingResult( + value, + bindingContext.ModelName, + isModelSet: value != null, + validationNode: validationNode); } return null; diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeConverterModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeConverterModelBinder.cs index 16cafa5083..c0d5a59f96 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeConverterModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeConverterModelBinder.cs @@ -30,12 +30,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { newModel = valueProviderResult.ConvertTo(bindingContext.ModelType); ModelBindingHelper.ReplaceEmptyStringWithNull(bindingContext.ModelMetadata, ref newModel); + var validationNode = new ModelValidationNode( + bindingContext.ModelName, + bindingContext.ModelMetadata, + newModel); - // We do not need to set an explict ModelValidationNode since CompositeModelBinder does that automatically. return new ModelBindingResult( newModel, bindingContext.ModelName, - isModelSet: true); + isModelSet: true, + validationNode: validationNode); } catch (Exception ex) { diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeMatchModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeMatchModelBinder.cs index 58a18c99a1..4d034786e3 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeMatchModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeMatchModelBinder.cs @@ -19,12 +19,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding bindingContext.ModelState.SetModelValue(bindingContext.ModelName, valueProviderResult); var model = valueProviderResult.RawValue; ModelBindingHelper.ReplaceEmptyStringWithNull(bindingContext.ModelMetadata, ref model); + var validationNode = new ModelValidationNode( + bindingContext.ModelName, + bindingContext.ModelMetadata, + model); - // We do not need to set an explict ModelValidationNode since CompositeModelBinder does that automatically. return new ModelBindingResult( model, bindingContext.ModelName, - isModelSet: true); + isModelSet: true, + validationNode: validationNode); } internal static async Task GetCompatibleValueProviderResult(ModelBindingContext context) diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/DefaultObjectValidator.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/DefaultObjectValidator.cs index 57fd2d6f11..558bae6f3e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/DefaultObjectValidator.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/DefaultObjectValidator.cs @@ -247,8 +247,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation modelState.GetFieldValidationState(errorKey) == ModelValidationState.Unvalidated) { - // If we are not able to add a model error - // for instance when the max error count is reached, mark the model as skipped. + // If we are not able to add a model error + // for instance when the max error count is reached, mark the model as skipped. modelState.MarkFieldSkipped(errorKey); } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/KeyValuePairModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/KeyValuePairModelBinderTest.cs index 257eda9b21..b430df1ae1 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/KeyValuePairModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/KeyValuePairModelBinderTest.cs @@ -309,7 +309,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test { if (mbc.ModelType == typeof(int)) { - return Task.FromResult(new ModelBindingResult(42, mbc.ModelName, true)); + var model = 42; + var validationNode = new ModelValidationNode(mbc.ModelName, mbc.ModelMetadata, model); + return Task.FromResult(new ModelBindingResult(model, mbc.ModelName, true, validationNode)); } return Task.FromResult(null); }); @@ -325,7 +327,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test { if (mbc.ModelType == typeof(string)) { - return Task.FromResult(new ModelBindingResult("some-value", mbc.ModelName, true)); + var model = "some-value"; + var validationNode = new ModelValidationNode(mbc.ModelName, mbc.ModelMetadata, model); + return Task.FromResult(new ModelBindingResult(model, mbc.ModelName, true, validationNode)); } return Task.FromResult(null); }); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs index f39ee0f6a4..2c557abc18 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs @@ -588,10 +588,14 @@ namespace Microsoft.AspNet.Mvc.Core.Test private static ActionBindingContext GetActionBindingContext(object model) { var binder = new Mock(); - binder - .Setup(b => b.BindModelAsync(It.IsAny())) - .Returns(Task.FromResult( - result: new ModelBindingResult(model: model, key: string.Empty, isModelSet: true))); + binder.Setup(b => b.BindModelAsync(It.IsAny())) + .Returns(mbc => + { + var validationNode = new ModelValidationNode(string.Empty, mbc.ModelMetadata, model); + return Task.FromResult( + result: new ModelBindingResult(model, string.Empty, isModelSet: true, validationNode: validationNode)); + }); + return new ActionBindingContext() { ModelBinder = binder.Object, diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs index 91c55f95c5..e570d74772 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs @@ -1462,7 +1462,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests }; var url = "http://localhost/dealers/43/update-vehicle?dealer.name=TestCarDealer&dealer.location=NE"; - + // Act var response = await client.PostAsJsonAsync(url, postedContent); diff --git a/test/WebSites/ModelBindingWebSite/Controllers/ModelBinderAttribute_ProductController.cs b/test/WebSites/ModelBindingWebSite/Controllers/ModelBinderAttribute_ProductController.cs index 9d32ef8429..55c6418589 100644 --- a/test/WebSites/ModelBindingWebSite/Controllers/ModelBinderAttribute_ProductController.cs +++ b/test/WebSites/ModelBindingWebSite/Controllers/ModelBinderAttribute_ProductController.cs @@ -74,7 +74,9 @@ namespace ModelBindingWebSite.Controllers // by the type converter binder. OrderStatus model; var isModelSet = Enum.TryParse("Status" + request.Query.Get("status"), out model); - return Task.FromResult(new ModelBindingResult(model, "status", isModelSet)); + var validationNode = + new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, model); + return Task.FromResult(new ModelBindingResult(model, "status", isModelSet, validationNode)); } return Task.FromResult(null); @@ -91,15 +93,17 @@ namespace ModelBindingWebSite.Controllers model.BinderType = GetType(); - var key = - string.IsNullOrEmpty(bindingContext.ModelName) ? - "productId" : + var key = + string.IsNullOrEmpty(bindingContext.ModelName) ? + "productId" : bindingContext.ModelName + "." + "productId"; var value = await bindingContext.ValueProvider.GetValueAsync(key); model.ProductId = (int)value.ConvertTo(typeof(int)); - return new ModelBindingResult(model, key, true); + var validationNode = + new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, value); + return new ModelBindingResult(model, key, true, validationNode); } return null;