diff --git a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingResult.cs b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingResult.cs index b9c5ef2a7a..c50ecc7ca3 100644 --- a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingResult.cs +++ b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelBindingResult.cs @@ -16,10 +16,25 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// A value that represents if the model has been set by the /// . public ModelBindingResult(object model, string key, bool isModelSet) + : this (model, key, isModelSet, validationNode: null) + { + } + + /// + /// Creates a new . + /// + /// The model which was created by the . + /// The key using which was used to attempt binding the model. + /// A value that represents if the model has been set by the + /// . + /// A which captures the validation information. + /// + public ModelBindingResult(object model, string key, bool isModelSet, ModelValidationNode validationNode) { Model = model; Key = key; IsModelSet = isModelSet; + ValidationNode = validationNode; } /// @@ -47,5 +62,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// /// public bool IsModelSet { get; } + + /// + /// A associated with the current . + /// + public ModelValidationNode ValidationNode { get; } } } diff --git a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelValidationNode.cs b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelValidationNode.cs new file mode 100644 index 0000000000..1109f1352a --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelValidationNode.cs @@ -0,0 +1,76 @@ +// 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 Microsoft.Framework.Internal; + +namespace Microsoft.AspNet.Mvc.ModelBinding +{ + /// + /// Captures the validation information for a particular model. + /// + public class ModelValidationNode + { + /// + /// Creates a new instance of . + /// + /// The key that will be used by the validation system to find + /// entries. + /// The for the . + /// The model object which is to be validated. + public ModelValidationNode([NotNull] string key, [NotNull] ModelMetadata modelMetadata, object model) + : this (key, modelMetadata, model, new List()) + { + } + + /// + /// Creates a new instance of . + /// + /// The key that will be used by the validation system to add + /// entries. + /// The for the . + /// The model object which will be validated. + /// A collection of child nodes. + public ModelValidationNode( + [NotNull] string key, + [NotNull] ModelMetadata modelMetadata, + object model, + [NotNull] IList childNodes) + { + Key = key; + ModelMetadata = modelMetadata; + ChildNodes = childNodes; + Model = model; + } + + /// + /// Gets the key used for adding entries. + /// + public string Key { get; } + + /// + /// Gets the . + /// + public ModelMetadata ModelMetadata { get; } + + /// + /// Gets the model instance which is to be validated. + /// + public object Model { get; } + + /// + /// Gets the child nodes. + /// + public IList ChildNodes { get; } + + /// + /// Gets or sets a value that indicates whether all properties of the model should be validated. + /// + public bool ValidateAllProperties { get; set; } + + /// + /// Gets or sets a value that indicates whether validation should be suppressed. + /// + public bool SuppressValidation { get; set; } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/Validation/ModelValidationContext.cs b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/Validation/ModelValidationContext.cs index 61b52c1e36..ff1a2bda7d 100644 --- a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/Validation/ModelValidationContext.cs +++ b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/Validation/ModelValidationContext.cs @@ -10,23 +10,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation public ModelValidationContext( [NotNull] ModelBindingContext bindingContext, [NotNull] ModelExplorer modelExplorer) - : this(bindingContext.ModelName, - bindingContext.BindingSource, - bindingContext.OperationBindingContext.ValidatorProvider, - bindingContext.ModelState, - modelExplorer) + : this( + bindingContext.BindingSource, + bindingContext.OperationBindingContext.ValidatorProvider, + bindingContext.ModelState, + modelExplorer) { } public ModelValidationContext( - string rootPrefix, BindingSource bindingSource, [NotNull] IModelValidatorProvider validatorProvider, [NotNull] ModelStateDictionary modelState, [NotNull] ModelExplorer modelExplorer) { ModelState = modelState; - RootPrefix = rootPrefix; ValidatorProvider = validatorProvider; ModelExplorer = modelExplorer; BindingSource = bindingSource; @@ -45,7 +43,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation [NotNull] ModelExplorer modelExplorer) { return new ModelValidationContext( - parentContext.RootPrefix, modelExplorer.Metadata.BindingSource, parentContext.ValidatorProvider, parentContext.ModelState, @@ -56,8 +53,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation public ModelStateDictionary ModelState { get; } - public string RootPrefix { get; set; } - public BindingSource BindingSource { get; set; } public IModelValidatorProvider ValidatorProvider { get; } diff --git a/src/Microsoft.AspNet.Mvc.Core/Controller.cs b/src/Microsoft.AspNet.Mvc.Core/Controller.cs index 5d3ae6f43f..7ea570efe0 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Controller.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Controller.cs @@ -1336,13 +1336,17 @@ namespace Microsoft.AspNet.Mvc modelName); var validationContext = new ModelValidationContext( - modelName, bindingSource: null, validatorProvider: BindingContext.ValidatorProvider, modelState: ModelState, modelExplorer: modelExplorer); - ObjectValidator.Validate(validationContext); + ObjectValidator.Validate( + validationContext, + new ModelValidationNode(modelName, modelExplorer.Metadata, model) + { + ValidateAllProperties = true + }); return ModelState.IsValid; } diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs index aacf99716e..9609154295 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs @@ -90,12 +90,11 @@ namespace Microsoft.AspNet.Mvc modelBindingResult.Model); var validationContext = new ModelValidationContext( - key, modelBindingContext.BindingSource, operationContext.ValidatorProvider, modelState, modelExplorer); - _validator.Validate(validationContext); + _validator.Validate(validationContext, modelBindingResult.ValidationNode); } return modelBindingResult; diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BinderTypeBasedModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BinderTypeBasedModelBinder.cs index 349d2147e1..2b3c004abd 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BinderTypeBasedModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BinderTypeBasedModelBinder.cs @@ -46,7 +46,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await modelBinder.BindModelAsync(bindingContext); var modelBindingResult = result != null ? - new ModelBindingResult(result.Model, result.Key, result.IsModelSet) : + new ModelBindingResult(result.Model, result.Key, result.IsModelSet, result.ValidationNode) : new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false); // A model binder was specified by metadata and this binder handles all such cases. diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BindingSourceModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BindingSourceModelBinder.cs index 42108a488f..f01a2c0135 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BindingSourceModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BindingSourceModelBinder.cs @@ -81,7 +81,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var modelBindingResult = result != null ? - new ModelBindingResult(result.Model, result.Key, result.IsModelSet) : + new ModelBindingResult(result.Model, result.Key, result.IsModelSet, result.ValidationNode) : new ModelBindingResult(model: null, key: context.ModelName, isModelSet: false); // This model binder is the only handler for its binding source. diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs index 8326ee316c..eb9552b02c 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/BodyModelBinder.cs @@ -54,11 +54,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var isTopLevelObject = bindingContext.ModelMetadata.ContainerType == null; - // For compatibility with MVC 5.0 for top level object we want to consider an empty key instead of + // For compatibility with MVC 5.0 for top level object we want to consider an empty key instead of // the parameter name/a custom name. In all other cases (like when binding body to a property) we // consider the entire ModelName as a prefix. var modelBindingKey = isTopLevelObject ? string.Empty : bindingContext.ModelName; - return new ModelBindingResult(model, key: modelBindingKey, isModelSet: true); + + var validationNode = new ModelValidationNode(modelBindingKey, bindingContext.ModelMetadata, model) + { + ValidateAllProperties = true + }; + + return new ModelBindingResult( + model, + key: modelBindingKey, + isModelSet: true, + validationNode: validationNode); } catch (Exception ex) { diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ByteArrayModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ByteArrayModelBinder.cs index 075e0c771b..651ee7fd57 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ByteArrayModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ByteArrayModelBinder.cs @@ -39,7 +39,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding try { var model = Convert.FromBase64String(value); - return new ModelBindingResult(model, bindingContext.ModelName, isModelSet: true); + + // We do not need to set an explict ModelValidationNode since CompositeModelBinder does that automatically. + return new ModelBindingResult( + model, + bindingContext.ModelName, + isModelSet: true); } catch (Exception ex) { diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs index f9258543d1..040f389dd4 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs @@ -31,16 +31,19 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var valueProviderResult = await bindingContext.ValueProvider.GetValueAsync(bindingContext.ModelName); IEnumerable boundCollection; + CollectionResult result; if (valueProviderResult == null) { - boundCollection = await BindComplexCollection(bindingContext); + result = await BindComplexCollection(bindingContext); + boundCollection = result.Model; } else { - boundCollection = await BindSimpleCollection( + result = await BindSimpleCollection( bindingContext, valueProviderResult.RawValue, valueProviderResult.Culture); + boundCollection = result.Model; } var model = bindingContext.Model; @@ -54,12 +57,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding CopyToModel(model, boundCollection); } - return new ModelBindingResult(model, bindingContext.ModelName, isModelSet: true); + return new ModelBindingResult( + model, + bindingContext.ModelName, + isModelSet: true, + validationNode: result?.ValidationNode); } // Used when the ValueProvider contains the collection to be bound as a single element, e.g. the raw value // is [ "1", "2" ] and needs to be converted to an int[]. - internal async Task> BindSimpleCollection( + internal async Task BindSimpleCollection( ModelBindingContext bindingContext, object rawValue, CultureInfo culture) @@ -74,6 +81,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider; var elementMetadata = metadataProvider.GetMetadataForType(typeof(TElement)); + var validationNode = new ModelValidationNode( + bindingContext.ModelName, + bindingContext.ModelMetadata, + boundCollection); var rawValueArray = RawValueToObjectArray(rawValue); foreach (var rawValueElement in rawValueArray) { @@ -91,18 +102,26 @@ namespace Microsoft.AspNet.Mvc.ModelBinding object boundValue = null; var result = await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(innerBindingContext); - if (result != null) + if (result != null && result.IsModelSet) { boundValue = result.Model; + if (result.ValidationNode != null) + { + validationNode.ChildNodes.Add(result.ValidationNode); + } } boundCollection.Add(ModelBindingHelper.CastOrDefault(boundValue)); } - return boundCollection; + return new CollectionResult + { + ValidationNode = validationNode, + Model = boundCollection + }; } // Used when the ValueProvider contains the collection to be bound as multiple elements, e.g. foo[0], foo[1]. - private async Task> BindComplexCollection(ModelBindingContext bindingContext) + private async Task BindComplexCollection(ModelBindingContext bindingContext) { var indexPropertyName = ModelNames.CreatePropertyModelName(bindingContext.ModelName, "index"); var valueProviderResultIndex = await bindingContext.ValueProvider.GetValueAsync(indexPropertyName); @@ -111,7 +130,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return await BindComplexCollectionFromIndexes(bindingContext, indexNames); } - internal async Task> BindComplexCollectionFromIndexes( + internal async Task BindComplexCollectionFromIndexes( ModelBindingContext bindingContext, IEnumerable indexNames) { @@ -131,6 +150,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var elementMetadata = metadataProvider.GetMetadataForType(typeof(TElement)); var boundCollection = new List(); + var validationNode = new ModelValidationNode( + bindingContext.ModelName, + bindingContext.ModelMetadata, + boundCollection); foreach (var indexName in indexNames) { var fullChildName = ModelNames.CreateIndexModelName(bindingContext.ModelName, indexName); @@ -146,10 +169,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(childBindingContext); - if (result != null) + if (result != null && result.IsModelSet) { didBind = true; boundValue = result.Model; + if (result.ValidationNode != null) + { + validationNode.ChildNodes.Add(result.ValidationNode); + } } // infinite size collection stops on first bind failure @@ -161,7 +188,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding boundCollection.Add(ModelBindingHelper.CastOrDefault(boundValue)); } - return boundCollection; + return new CollectionResult + { + ValidationNode = validationNode, + Model = boundCollection + }; + } + + internal class CollectionResult + { + public ModelValidationNode ValidationNode { get; set; } + + public IEnumerable Model { get; set; } } /// diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs index 034b4c51f6..a9d0e92b86 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CompositeModelBinder.cs @@ -55,6 +55,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding bindingContext.OperationBindingContext.BodyBindingState = newBindingContext.OperationBindingContext.BodyBindingState; + var bindingKey = bindingContext.ModelName; if (modelBindingResult.IsModelSet) { // Update the model state key if we are bound using an empty prefix and it is a complex type. @@ -78,15 +79,28 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // In this case, for the model parameter the key would be SimpleType instead of model.SimpleType. // (i.e here the prefix for the model key is empty). // For the id parameter the key would be id. - return modelBindingResult; + bindingKey = string.Empty; } } - // Fall through to update the ModelBindingResult's key. + // 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, - bindingContext.ModelName, - modelBindingResult.IsModelSet); + bindingKey, + modelBindingResult.IsModelSet, + modelValidationNode); } private async Task TryBind(ModelBindingContext bindingContext) diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs index 6791ee2988..2997e7f8b1 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs @@ -20,7 +20,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(bindingContext); var modelBindingResult = result != null ? - new ModelBindingResult(result.Model, result.Key, result.IsModelSet) : + new ModelBindingResult(result.Model, result.Key, result.IsModelSet, result.ValidationNode) : new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false); // Were able to resolve a binder type. diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/HeaderModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/HeaderModelBinder.cs index 1044bb6c59..6d8c267dfb 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/HeaderModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/HeaderModelBinder.cs @@ -51,7 +51,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } } - return Task.FromResult(new ModelBindingResult(model, bindingContext.ModelName, isModelSet: model != null)); + ModelValidationNode validationNode = null; + if (model != null) + { + validationNode = new ModelValidationNode( + bindingContext.ModelName, + bindingContext.ModelMetadata, + model); + } + + return Task.FromResult( + new ModelBindingResult( + model, + bindingContext.ModelName, + isModelSet: model != null, + validationNode: validationNode)); } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/KeyValuePairModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/KeyValuePairModelBinder.cs index 8481da9152..12e285e6b4 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/KeyValuePairModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/KeyValuePairModelBinder.cs @@ -15,8 +15,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding typeof(KeyValuePair), allowNullModel: true); - var keyResult = await TryBindStrongModel(bindingContext, "Key"); - var valueResult = await TryBindStrongModel(bindingContext, "Value"); + var childNodes = new List(); + var keyResult = await TryBindStrongModel(bindingContext, "Key", childNodes); + var valueResult = await TryBindStrongModel(bindingContext, "Value", childNodes); if (keyResult.IsModelSet && valueResult.IsModelSet) { @@ -24,8 +25,20 @@ namespace Microsoft.AspNet.Mvc.ModelBinding ModelBindingHelper.CastOrDefault(keyResult.Model), ModelBindingHelper.CastOrDefault(valueResult.Model)); + // Update the model for the top level validation node. + var modelValidationNode = + new ModelValidationNode( + bindingContext.ModelName, + bindingContext.ModelMetadata, + model, + childNodes); + // Success - return new ModelBindingResult(model, bindingContext.ModelName, isModelSet: true); + return new ModelBindingResult( + model, + bindingContext.ModelName, + isModelSet: true, + validationNode: modelValidationNode); } else if (!keyResult.IsModelSet && valueResult.IsModelSet) { @@ -55,8 +68,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } } - internal async Task TryBindStrongModel(ModelBindingContext parentBindingContext, - string propertyName) + internal async Task TryBindStrongModel( + ModelBindingContext parentBindingContext, + string propertyName, + List childNodes) { var propertyModelMetadata = parentBindingContext.OperationBindingContext.MetadataProvider.GetMetadataForType(typeof(TModel)); @@ -72,6 +87,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding propertyBindingContext); if (modelBindingResult != null) { + if (modelBindingResult.ValidationNode != null) + { + childNodes.Add(modelBindingResult.ValidationNode); + } + return modelBindingResult; } diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs index 03caa4f61f..0fd66a0f25 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs @@ -44,12 +44,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding EnsureModel(bindingContext); var result = await CreateAndPopulateDto(bindingContext, mutableObjectBinderContext.PropertyMetadata); + var validationNode = new ModelValidationNode( + bindingContext.ModelName, + bindingContext.ModelMetadata, + bindingContext.Model); + // post-processing, e.g. property setters and hooking up validation - ProcessDto(bindingContext, (ComplexModelDto)result.Model); + ProcessDto(bindingContext, (ComplexModelDto)result.Model, validationNode); return new ModelBindingResult( bindingContext.Model, bindingContext.ModelName, - isModelSet: true); + isModelSet: true, + validationNode: validationNode); } /// @@ -359,11 +365,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return validationInfo; } - internal void ProcessDto(ModelBindingContext bindingContext, ComplexModelDto dto) + // Internal for testing. + internal ModelValidationNode ProcessDto( + ModelBindingContext bindingContext, + ComplexModelDto dto, + ModelValidationNode validationNode) { var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider; var modelExplorer = metadataProvider.GetModelExplorerForType(bindingContext.ModelType, bindingContext.Model); - var validationInfo = GetPropertyValidationInfo(bindingContext); // Eliminate provided properties from requiredProperties; leaving just *missing* required properties. @@ -415,8 +424,20 @@ namespace Microsoft.AspNet.Mvc.ModelBinding out requiredValidator); SetProperty(bindingContext, modelExplorer, propertyMetadata, dtoResult, requiredValidator); + + var dtoValidationNode = dtoResult.ValidationNode; + if (dtoValidationNode == null) + { + // Make sure that irrespective of if 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); + } + + validationNode.ChildNodes.Add(dtoValidationNode); } } + + return validationNode; } /// diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ServicesModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ServicesModelBinder.cs index 2dd41f88fe..69d3ae1e1c 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ServicesModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ServicesModelBinder.cs @@ -26,7 +26,17 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { var requestServices = bindingContext.OperationBindingContext.HttpContext.RequestServices; var model = requestServices.GetRequiredService(bindingContext.ModelType); - return Task.FromResult(new ModelBindingResult(model, bindingContext.ModelName, isModelSet: true)); + var validationNode = + new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, model) + { + SuppressValidation = true + }; + + return Task.FromResult(new ModelBindingResult( + model, + bindingContext.ModelName, + isModelSet: true, + validationNode: validationNode)); } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeConverterModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeConverterModelBinder.cs index f548c06c53..16cafa5083 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeConverterModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeConverterModelBinder.cs @@ -30,7 +30,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { newModel = valueProviderResult.ConvertTo(bindingContext.ModelType); ModelBindingHelper.ReplaceEmptyStringWithNull(bindingContext.ModelMetadata, ref newModel); - return new ModelBindingResult(newModel, bindingContext.ModelName, isModelSet: true); + + // We do not need to set an explict ModelValidationNode since CompositeModelBinder does that automatically. + return new ModelBindingResult( + newModel, + bindingContext.ModelName, + isModelSet: true); } catch (Exception ex) { diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeMatchModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeMatchModelBinder.cs index ea1c90d587..58a18c99a1 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeMatchModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeMatchModelBinder.cs @@ -19,7 +19,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding bindingContext.ModelState.SetModelValue(bindingContext.ModelName, valueProviderResult); var model = valueProviderResult.RawValue; ModelBindingHelper.ReplaceEmptyStringWithNull(bindingContext.ModelMetadata, ref model); - return new ModelBindingResult(model, bindingContext.ModelName, isModelSet: true); + + // We do not need to set an explict ModelValidationNode since CompositeModelBinder does that automatically. + return new ModelBindingResult( + model, + bindingContext.ModelName, + isModelSet: true); } 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 0aa43cb445..4a3c5ed0a0 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/DefaultObjectValidator.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/DefaultObjectValidator.cs @@ -35,16 +35,19 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation } /// - public void Validate([NotNull] ModelValidationContext modelValidationContext) + public void Validate( + [NotNull] ModelValidationContext modelValidationContext, + [NotNull] ModelValidationNode validationNode) { var validationContext = new ValidationContext() { ModelValidationContext = modelValidationContext, Visited = new HashSet(ReferenceEqualityComparer.Instance), + ValidationNode = validationNode }; ValidateNonVisitedNodeAndChildren( - modelValidationContext.RootPrefix, + validationNode.Key, validationContext, validators: null); } @@ -54,19 +57,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation ValidationContext validationContext, IList validators) { - var modelValidationContext = validationContext.ModelValidationContext; - var modelExplorer = modelValidationContext.ModelExplorer; - // Recursion guard to avoid stack overflows RuntimeHelpers.EnsureSufficientExecutionStack(); + var modelValidationContext = validationContext.ModelValidationContext; + var modelExplorer = modelValidationContext.ModelExplorer; var modelState = modelValidationContext.ModelState; - - var bindingSource = modelValidationContext.BindingSource; - if (bindingSource != null && !bindingSource.IsFromRequest) + var currentValidationNode = validationContext.ValidationNode; + if (currentValidationNode.SuppressValidation) { - // Short circuit if the metadata represents something that was not bound using request data. - // For example model bound using [FromServices]. Treat such objects as skipped. + // Short circuit if the node is marked to be suppressed var validationState = modelState.GetFieldValidationState(modelKey); if (validationState == ModelValidationState.Unvalidated) { @@ -88,22 +88,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation { // The validators are not null in the case of validating an array. Since the validators are // the same for all the elements of the array, we do not do GetValidators for each element, - // instead we just pass them over. See ValidateElements function. - var validatorProvider = modelValidationContext.ValidatorProvider; - var validatorProviderContext = new ModelValidatorProviderContext(modelExplorer.Metadata); - validatorProvider.GetValidators(validatorProviderContext); - - validators = validatorProviderContext.Validators; + // instead we just pass them over. + validators = GetValidators(modelValidationContext.ValidatorProvider, modelExplorer.Metadata); } - // We don't need to recursively traverse the graph for null values - if (modelExplorer.Model == null) + // We don't need to recursively traverse the graph if there are no child nodes. + if (currentValidationNode.ChildNodes.Count == 0 && !currentValidationNode.ValidateAllProperties) { return ShallowValidate(modelKey, modelExplorer, validationContext, validators); } // We don't need to recursively traverse the graph for types that shouldn't be validated - var modelType = modelExplorer.Model.GetType(); + var modelType = modelExplorer.ModelType; if (IsTypeExcludedFromValidation(_excludeFilters, modelType)) { var result = ShallowValidate(modelKey, modelExplorer, validationContext, validators); @@ -112,24 +108,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation } // Check to avoid infinite recursion. This can happen with cycles in an object graph. + // Note that this is only applicable in case the model is pre-existing (like in case of TryUpdateModel). if (validationContext.Visited.Contains(modelExplorer.Model)) { return true; } validationContext.Visited.Add(modelExplorer.Model); - - // Validate the children first - depth-first traversal - var enumerableModel = modelExplorer.Model as IEnumerable; - if (enumerableModel == null) - { - isValid = ValidateProperties(modelKey, modelExplorer, validationContext); - } - else - { - isValid = ValidateElements(modelKey, enumerableModel, validationContext); - } - + isValid = ValidateChildNodes(modelKey, modelExplorer, validationContext); if (isValid) { // Don't bother to validate this node if children failed. @@ -166,32 +152,49 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation } } - private bool ValidateProperties( + private IList GetValidators(IModelValidatorProvider provider, ModelMetadata metadata) + { + var validatorProviderContext = new ModelValidatorProviderContext(metadata); + provider.GetValidators(validatorProviderContext); + return validatorProviderContext.Validators; + } + + private bool ValidateChildNodes( string currentModelKey, ModelExplorer modelExplorer, ValidationContext validationContext) { var isValid = true; + ExpandValidationNode(validationContext, modelExplorer); - foreach (var property in modelExplorer.Metadata.Properties) + IList validators = null; + if (modelExplorer.Metadata.IsCollectionType && modelExplorer.Model != null) { - var propertyExplorer = modelExplorer.GetExplorerForProperty(property.PropertyName); - var propertyMetadata = propertyExplorer.Metadata; + var enumerableModel = (IEnumerable)modelExplorer.Model; + var elementType = GetElementType(enumerableModel.GetType()); + var elementMetadata = _modelMetadataProvider.GetMetadataForType(elementType); + validators = GetValidators(validationContext.ModelValidationContext.ValidatorProvider, elementMetadata); + } + + foreach (var childNode in validationContext.ValidationNode.ChildNodes) + { + var childModelExplorer = childNode.ModelMetadata.MetadataKind == Metadata.ModelMetadataKind.Type ? + _modelMetadataProvider.GetModelExplorerForType(childNode.ModelMetadata.ModelType, childNode.Model) : + modelExplorer.GetExplorerForProperty(childNode.ModelMetadata.PropertyName); + var propertyValidationContext = new ValidationContext() { ModelValidationContext = ModelValidationContext.GetChildValidationContext( validationContext.ModelValidationContext, - propertyExplorer), - Visited = validationContext.Visited + childModelExplorer), + Visited = validationContext.Visited, + ValidationNode = childNode }; - var propertyBindingName = propertyMetadata.BinderModelName ?? propertyMetadata.PropertyName; - var childKey = ModelNames.CreatePropertyModelName(currentModelKey, propertyBindingName); - if (!ValidateNonVisitedNodeAndChildren( - childKey, - propertyValidationContext, - validators: null)) + childNode.Key, + propertyValidationContext, + validators)) { isValid = false; } @@ -200,51 +203,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation return isValid; } - private bool ValidateElements(string currentKey, IEnumerable model, ValidationContext validationContext) - { - var elementType = GetElementType(model.GetType()); - var elementMetadata = _modelMetadataProvider.GetMetadataForType(elementType); - - var validatorProvider = validationContext.ModelValidationContext.ValidatorProvider; - var validatorProviderContext = new ModelValidatorProviderContext(elementMetadata); - validatorProvider.GetValidators(validatorProviderContext); - - var validators = validatorProviderContext.Validators; - - // If there are no validators or the object is null we bail out quickly - // when there are large arrays of null, this will save a significant amount of processing - // with minimal impact to other scenarios. - var anyValidatorsDefined = validators.Any(); - var index = 0; - var isValid = true; - foreach (var element in model) - { - // If the element is non null, the recursive calls might find more validators. - // If it's null, then a shallow validation will be performed. - if (element != null || anyValidatorsDefined) - { - var elementExplorer = new ModelExplorer(_modelMetadataProvider, elementMetadata, element); - var elementKey = ModelNames.CreateIndexModelName(currentKey, index); - var elementValidationContext = new ValidationContext() - { - ModelValidationContext = ModelValidationContext.GetChildValidationContext( - validationContext.ModelValidationContext, - elementExplorer), - Visited = validationContext.Visited - }; - - if (!ValidateNonVisitedNodeAndChildren(elementKey, elementValidationContext, validators)) - { - isValid = false; - } - } - - index++; - } - - return isValid; - } - // Validates a single node (not including children) // Returns true if validation passes successfully private static bool ShallowValidate( @@ -312,6 +270,54 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation return filters.Any(filter => filter.IsTypeExcluded(type)); } + private void ExpandValidationNode(ValidationContext context, ModelExplorer modelExplorer) + { + var validationNode = context.ValidationNode; + if (validationNode.ChildNodes.Count != 0 || + !validationNode.ValidateAllProperties || + validationNode.Model == null) + { + return; + } + + if (!modelExplorer.Metadata.IsCollectionType) + { + foreach (var property in validationNode.ModelMetadata.Properties) + { + var propertyExplorer = modelExplorer.GetExplorerForProperty(property.PropertyName); + var propertyBindingName = property.BinderModelName ?? property.PropertyName; + var childKey = ModelNames.CreatePropertyModelName(validationNode.Key, propertyBindingName); + var childNode = new ModelValidationNode(childKey, property, propertyExplorer.Model) + { + ValidateAllProperties = true + }; + validationNode.ChildNodes.Add(childNode); + } + } + else + { + var enumerableModel = (IEnumerable)modelExplorer.Model; + var elementType = GetElementType(enumerableModel.GetType()); + var elementMetadata = _modelMetadataProvider.GetMetadataForType(elementType); + + // An integer index is incorrect in scenarios where there is a custom index provided by the user. + // However those scenarios are supported by createing a ModelValidationNode with the right keys. + var index = 0; + foreach (var element in enumerableModel) + { + var elementExplorer = new ModelExplorer(_modelMetadataProvider, elementMetadata, element); + var elementKey = ModelNames.CreateIndexModelName(validationNode.Key, index); + var childNode = new ModelValidationNode(elementKey, elementMetadata, elementExplorer.Model) + { + ValidateAllProperties = true + }; + + validationNode.ChildNodes.Add(childNode); + index++; + } + } + } + private static Type GetElementType(Type type) { Debug.Assert(typeof(IEnumerable).GetTypeInfo().IsAssignableFrom(type.GetTypeInfo())); @@ -337,6 +343,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation public ModelValidationContext ModelValidationContext { get; set; } public HashSet Visited { get; set; } + + public ModelValidationNode ValidationNode { get; set; } } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/IObjectModelValidator.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/IObjectModelValidator.cs index 259bdcc64f..54c01631a8 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/IObjectModelValidator.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/IObjectModelValidator.cs @@ -13,6 +13,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation /// /// The associated with the current call. /// - void Validate(ModelValidationContext validationContext); + /// The for the model which gets validated. + /// + void Validate(ModelValidationContext validationContext, ModelValidationNode validationNode); } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ParameterBinding/ModelBindingHelper.cs b/src/Microsoft.AspNet.Mvc.Core/ParameterBinding/ModelBindingHelper.cs index 9d14dbf59a..f56a66513d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ParameterBinding/ModelBindingHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ParameterBinding/ModelBindingHelper.cs @@ -312,8 +312,12 @@ namespace Microsoft.AspNet.Mvc { var modelExplorer = new ModelExplorer(metadataProvider, modelMetadata, modelBindingResult.Model); var modelValidationContext = new ModelValidationContext(modelBindingContext, modelExplorer); - modelValidationContext.RootPrefix = prefix; - objectModelValidator.Validate(modelValidationContext); + objectModelValidator.Validate( + modelValidationContext, + new ModelValidationNode(prefix, modelBindingContext.ModelMetadata, modelBindingResult.Model) + { + ValidateAllProperties = true + }); return modelState.IsValid; } diff --git a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs index 4e00bdb9f2..2af1b2136b 100644 --- a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs +++ b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs @@ -417,13 +417,17 @@ namespace System.Web.Http var modelExplorer = MetadataProvider.GetModelExplorerForType(typeof(TEntity), entity); var modelValidationContext = new ModelValidationContext( - keyPrefix, bindingSource: null, validatorProvider: BindingContext.ValidatorProvider, modelState: ModelState, modelExplorer: modelExplorer); - ObjectValidator.Validate(modelValidationContext); + ObjectValidator.Validate( + modelValidationContext, + new ModelValidationNode(keyPrefix, modelExplorer.Metadata, entity) + { + ValidateAllProperties = true + }); } protected virtual void Dispose(bool disposing) diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BinderTypeBasedModelBinderModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BinderTypeBasedModelBinderModelBinderTest.cs index 130093b92b..ea175c58de 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BinderTypeBasedModelBinderModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BinderTypeBasedModelBinderModelBinderTest.cs @@ -51,7 +51,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var bindingContext = GetBindingContext(typeof(Person), binderType: typeof(TrueModelBinder)); var model = new Person(); - var innerModelBinder = new TrueModelBinder(); var serviceProvider = new ServiceCollection() .AddSingleton(typeof(IModelBinder)) .BuildServiceProvider(); @@ -67,6 +66,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var p = (Person)binderResult.Model; Assert.Equal(model.Age, p.Age); Assert.Equal(model.Name, p.Name); + Assert.NotNull(binderResult.ValidationNode); + Assert.Equal(bindingContext.ModelName, binderResult.ValidationNode.Key); + Assert.Same(binderResult.Model, binderResult.ValidationNode.Model); } [Fact] @@ -138,7 +140,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test public Task BindModelAsync(ModelBindingContext bindingContext) { - return Task.FromResult(new ModelBindingResult(_model, bindingContext.ModelName, true)); + var validationNode = + new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, _model); + return Task.FromResult(new ModelBindingResult(_model, bindingContext.ModelName, true, validationNode)); } } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BodyModelBinderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BodyModelBinderTests.cs index 9d867bd735..14f83ac6af 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BodyModelBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BodyModelBinderTests.cs @@ -50,6 +50,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding mockInputFormatter.Verify(v => v.ReadAsync(It.IsAny()), Times.Once); Assert.NotNull(binderResult); Assert.True(binderResult.IsModelSet); + Assert.NotNull(binderResult.ValidationNode); + Assert.True(binderResult.ValidationNode.ValidateAllProperties); + Assert.False(binderResult.ValidationNode.SuppressValidation); + Assert.Empty(binderResult.ValidationNode.ChildNodes); + Assert.Equal(binderResult.Key, binderResult.ValidationNode.Key); + Assert.Equal(bindingContext.ModelMetadata, binderResult.ValidationNode.ModelMetadata); + Assert.Same(binderResult.Model, binderResult.ValidationNode.Model); } [Fact] @@ -71,6 +78,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Returns true because it understands the metadata type. Assert.NotNull(binderResult); Assert.False(binderResult.IsModelSet); + Assert.Null(binderResult.ValidationNode); Assert.Null(binderResult.Model); Assert.True(bindingContext.ModelState.ContainsKey("someName")); } @@ -92,6 +100,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Assert Assert.NotNull(binderResult); Assert.False(binderResult.IsModelSet); + Assert.Null(binderResult.ValidationNode); } [Fact] @@ -159,6 +168,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Returns true because it understands the metadata type. Assert.NotNull(binderResult); Assert.False(binderResult.IsModelSet); + Assert.Null(binderResult.ValidationNode); Assert.Null(binderResult.Model); Assert.True(bindingContext.ModelState.ContainsKey("someName")); var errorMessage = bindingContext.ModelState["someName"].Errors[0].Exception.Message; @@ -192,6 +202,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding Assert.NotNull(binderResult); Assert.False(binderResult.IsModelSet); Assert.Null(binderResult.Model); + Assert.Null(binderResult.ValidationNode); Assert.True(bindingContext.ModelState.ContainsKey("someName")); var errorMessage = bindingContext.ModelState["someName"].Errors[0].ErrorMessage; Assert.Equal("Unsupported content type 'text/xyz'.", errorMessage); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs index e5ba4d5fe5..c0f4af2331 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs @@ -33,7 +33,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var boundCollection = await binder.BindComplexCollectionFromIndexes(bindingContext, new[] { "foo", "bar", "baz" }); // Assert - Assert.Equal(new[] { 42, 0, 200 }, boundCollection.ToArray()); + Assert.Equal(new[] { 42, 0, 200 }, boundCollection.Model.ToArray()); + Assert.Equal( + new[] { "someName[foo]", "someName[baz]" }, + boundCollection.ValidationNode.ChildNodes.Select(o => o.Key).ToArray()); } [Fact] @@ -53,7 +56,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var boundCollection = await binder.BindComplexCollectionFromIndexes(bindingContext, indexNames: null); // Assert - Assert.Equal(new[] { 42, 100 }, boundCollection.ToArray()); + Assert.Equal(new[] { 42, 100 }, boundCollection.Model.ToArray()); + Assert.Equal( + new[] { "someName[0]", "someName[1]" }, + boundCollection.ValidationNode.ChildNodes.Select(o => o.Key).ToArray()); } [Theory] @@ -193,8 +199,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var boundCollection = await binder.BindSimpleCollection(context, rawValue: new object[0], culture: null); // Assert - Assert.NotNull(boundCollection); - Assert.Empty(boundCollection); + Assert.NotNull(boundCollection.Model); + Assert.Empty(boundCollection.Model); } [Fact] @@ -217,13 +223,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test // Arrange var culture = new CultureInfo("fr-FR"); var bindingContext = GetModelBindingContext(new SimpleHttpValueProvider()); - + ModelValidationNode childValidationNode = null; Mock.Get(bindingContext.OperationBindingContext.ModelBinder) .Setup(o => o.BindModelAsync(It.IsAny())) .Returns((ModelBindingContext mbc) => { Assert.Equal("someName", mbc.ModelName); - return Task.FromResult(new ModelBindingResult(42, mbc.ModelName, true)); + childValidationNode = new ModelValidationNode("someName", mbc.ModelMetadata, mbc.Model); + return Task.FromResult(new ModelBindingResult(42, mbc.ModelName, true, childValidationNode)); }); var modelBinder = new CollectionModelBinder(); @@ -231,7 +238,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var boundCollection = await modelBinder.BindSimpleCollection(bindingContext, new int[1], culture); // Assert - Assert.Equal(new[] { 42 }, boundCollection.ToArray()); + Assert.Equal(new[] { 42 }, boundCollection.Model.ToArray()); + Assert.Equal(new[] { childValidationNode }, boundCollection.ValidationNode.ChildNodes.ToArray()); } private static ModelBindingContext GetModelBindingContext( @@ -267,7 +275,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test if (value != null) { var model = value.ConvertTo(mbc.ModelType); - return new ModelBindingResult(model, mbc.ModelName, true); + var modelValidationNode = new ModelValidationNode(mbc.ModelName, mbc.ModelMetadata, model); + return new ModelBindingResult(model, mbc.ModelName, true, modelValidationNode); } return null; diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CompositeModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CompositeModelBinderTest.cs index 7f4b89e221..1927e48dc2 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CompositeModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CompositeModelBinderTest.cs @@ -374,6 +374,26 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var model = Assert.IsType(result.Model); Assert.Equal("firstName-value", model.FirstName); Assert.Equal("lastName-value", model.LastName); + + Assert.NotNull(result.ValidationNode); + Assert.Equal(2, result.ValidationNode.ChildNodes.Count); + Assert.Equal("", result.ValidationNode.Key); + Assert.Equal(bindingContext.ModelMetadata, result.ValidationNode.ModelMetadata); + model = Assert.IsType(result.ValidationNode.Model); + Assert.Equal("firstName-value", model.FirstName); + Assert.Equal("lastName-value", model.LastName); + + Assert.Equal(2, result.ValidationNode.ChildNodes.Count); + + var validationNode = result.ValidationNode.ChildNodes[0]; + Assert.Equal("FirstName", validationNode.Key); + Assert.Equal("firstName-value", validationNode.Model); + Assert.Empty(validationNode.ChildNodes); + + validationNode = result.ValidationNode.ChildNodes[1]; + Assert.Equal("LastName", validationNode.Key); + Assert.Equal("lastName-value", validationNode.Model); + Assert.Empty(validationNode.ChildNodes); } [Fact] @@ -414,6 +434,79 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test Assert.Equal(new byte[] { 227, 233, 133, 121, 58, 119, 180, 241 }, model.Resume); } + [Fact] + public async Task BindModel_DoesNotAddAValidationNode_IfModelIsNotSet() + { + // Arrange + var valueProvider = new SimpleHttpValueProvider(); + var mockBinder = new Mock(); + mockBinder + .Setup(o => o.BindModelAsync(It.IsAny())) + .Returns( + delegate (ModelBindingContext context) + { + return Task.FromResult( + new ModelBindingResult(model: 42, key: "someName", isModelSet: false)); + }); + var binder = CreateCompositeBinder(mockBinder.Object); + var bindingContext = CreateBindingContext(binder, valueProvider, typeof(SimplePropertiesModel)); + + // Act + var result = await binder.BindModelAsync(bindingContext); + + // Assert + // The result is null because of issue #2473 + Assert.Null(result); + } + + [Fact] + public async Task BindModel_DoesNotAddAValidationNode_IfModelBindingResultIsNull() + { + // Arrange + var mockBinder = new Mock(); + mockBinder + .Setup(o => o.BindModelAsync(It.IsAny())) + .Returns(Task.FromResult(null)); + var binder = CreateCompositeBinder(mockBinder.Object); + var valueProvider = new SimpleHttpValueProvider(); + var bindingContext = CreateBindingContext(binder, valueProvider, typeof(SimplePropertiesModel)); + + // Act + var result = await binder.BindModelAsync(bindingContext); + + // Assert + Assert.Null(result); + } + + [Fact] + public async Task BindModel_UsesTheValidationNodeOnModelBindingResult_IfPresent() + { + // Arrange + var valueProvider = new SimpleHttpValueProvider(); + ModelValidationNode validationNode = null; + + var mockBinder = new Mock(); + mockBinder + .Setup(o => o.BindModelAsync(It.IsAny())) + .Returns( + delegate (ModelBindingContext context) + { + validationNode = new ModelValidationNode("someName", context.ModelMetadata, 42); + return Task.FromResult( + new ModelBindingResult(42, "someName", isModelSet: true, validationNode: validationNode)); + }); + var binder = CreateCompositeBinder(mockBinder.Object); + var bindingContext = CreateBindingContext(binder, valueProvider, typeof(SimplePropertiesModel)); + + // Act + var result = await binder.BindModelAsync(bindingContext); + + // Assert + Assert.NotNull(result); + Assert.True(result.IsModelSet); + Assert.Same(validationNode, result.ValidationNode); + } + private static ModelBindingContext CreateBindingContext(IModelBinder binder, IValueProvider valueProvider, Type type, diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/KeyValuePairModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/KeyValuePairModelBinderTest.cs index 570f39c2e4..1b9b463e3e 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/KeyValuePairModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/KeyValuePairModelBinderTest.cs @@ -31,6 +31,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test Assert.NotNull(result); Assert.Null(result.Model); Assert.False(bindingContext.ModelState.IsValid); + Assert.Null(result.ValidationNode); Assert.Equal("someName", bindingContext.ModelName); var error = Assert.Single(bindingContext.ModelState["someName.Key"].Errors); Assert.Equal("A value is required.", error.ErrorMessage); @@ -53,6 +54,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test Assert.NotNull(result); Assert.Null(result.Model); Assert.False(bindingContext.ModelState.IsValid); + Assert.Null(result.ValidationNode); Assert.Equal("someName", bindingContext.ModelName); Assert.Equal(bindingContext.ModelState["someName.Value"].Errors.First().ErrorMessage, "A value is required."); } @@ -97,17 +99,31 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test // Assert Assert.NotNull(result); Assert.Equal(new KeyValuePair(42, "some-value"), result.Model); + Assert.NotNull(result.ValidationNode); + Assert.Equal(new KeyValuePair(42, "some-value"), result.ValidationNode.Model); + Assert.Equal("someName", result.ValidationNode.Key); + + var validationNode = result.ValidationNode.ChildNodes[0]; + Assert.Equal("someName.Key", validationNode.Key); + Assert.Equal(42, validationNode.Model); + Assert.Empty(validationNode.ChildNodes); + + validationNode = result.ValidationNode.ChildNodes[1]; + Assert.Equal("someName.Value", validationNode.Key); + Assert.Equal("some-value", validationNode.Model); + Assert.Empty(validationNode.ChildNodes); } [Fact] public async Task TryBindStrongModel_BinderExists_BinderReturnsCorrectlyTypedObject_ReturnsTrue() { // Arrange - ModelBindingContext bindingContext = GetBindingContext(new SimpleHttpValueProvider()); + var bindingContext = GetBindingContext(new SimpleHttpValueProvider()); var binder = new KeyValuePairModelBinder(); + var modelValidationNodeList = new List(); // Act - var result = await binder.TryBindStrongModel(bindingContext, "key"); + var result = await binder.TryBindStrongModel(bindingContext, "key", modelValidationNodeList); // Assert Assert.True(result.IsModelSet); @@ -131,9 +147,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var binder = new KeyValuePairModelBinder(); + var modelValidationNodeList = new List(); // Act - var result = await binder.TryBindStrongModel(bindingContext, "key"); + var result = await binder.TryBindStrongModel(bindingContext, "key", modelValidationNodeList); // Assert Assert.True(result.IsModelSet); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs index 708db0be61..2c8d23e119 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs @@ -776,11 +776,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding "John Doe", isModelSet: true, key: ""); - + var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); var testableBinder = new TestableMutableObjectModelBinder(); // Act - testableBinder.ProcessDto(bindingContext, dto); + testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); // Assert var modelStateDictionary = bindingContext.ModelState; @@ -827,10 +827,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding isModelSet: true, key: ""); + var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); var testableBinder = new TestableMutableObjectModelBinder(); // Act - testableBinder.ProcessDto(bindingContext, dto); + testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); // Assert var modelStateDictionary = bindingContext.ModelState; @@ -888,8 +889,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding isModelSet: true, key: "theModel.Age"); + var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); + // Act - testableBinder.ProcessDto(bindingContext, dto); + testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); // Assert var modelStateDictionary = bindingContext.ModelState; @@ -920,9 +923,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Set no properties though Age (a non-Nullable struct) and City (a class) properties are required. var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties); var testableBinder = new TestableMutableObjectModelBinder(); + var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); // Act - testableBinder.ProcessDto(bindingContext, dto); + testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); // Assert var modelStateDictionary = bindingContext.ModelState; @@ -973,9 +977,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding null, isModelSet: true, key: "theModel.City"); + var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); // Act - testableBinder.ProcessDto(bindingContext, dto); + testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); // Assert var modelStateDictionary = bindingContext.ModelState; @@ -1004,9 +1009,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Set no properties though ValueTypeRequired (a non-Nullable struct) property is required. var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties); var testableBinder = new TestableMutableObjectModelBinder(); + var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); // Act - testableBinder.ProcessDto(bindingContext, dto); + testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); // Assert var modelStateDictionary = bindingContext.ModelState; @@ -1074,9 +1080,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding model: null, isModelSet: isModelSet, key: "theModel." + nameof(Person.ValueTypeRequiredWithDefaultValue)); + var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); // Act - testableBinder.ProcessDto(bindingContext, dto); + testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); // Assert Assert.False(modelStateDictionary.IsValid); @@ -1149,9 +1156,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding model: null, isModelSet: false, key: "theModel." + nameof(Person.PropertyWithDefaultValue)); + var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model); // Act - testableBinder.ProcessDto(bindingContext, dto); + testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); // Assert Assert.True(modelStateDictionary.IsValid); @@ -1191,17 +1199,30 @@ namespace Microsoft.AspNet.Mvc.ModelBinding 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); + testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); // Assert Assert.Equal("John", model.FirstName); Assert.Equal("Doe", model.LastName); Assert.Equal(dob, model.DateOfBirth); Assert.True(bindingContext.ModelState.IsValid); + + // 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()); + + 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); } [Fact] diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Validation/DataAnnotationsModelValidatorTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Validation/DataAnnotationsModelValidatorTest.cs index d36efd3447..2248930b80 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Validation/DataAnnotationsModelValidatorTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Validation/DataAnnotationsModelValidatorTest.cs @@ -231,7 +231,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation private static ModelValidationContext CreateValidationContext(ModelExplorer modelExplorer) { return new ModelValidationContext( - rootPrefix: null, bindingSource: null, modelState: null, validatorProvider: null, diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Validation/DefaultObjectValidatorTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Validation/DefaultObjectValidatorTests.cs index e9a96740d4..33e1525209 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Validation/DefaultObjectValidatorTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Validation/DefaultObjectValidatorTests.cs @@ -212,9 +212,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation var context = GetModelValidationContext(model, type); var validator = new DefaultObjectValidator(context.ExcludeFilters, context.ModelMetadataProvider); + var topLevelValidationNode = + new ModelValidationNode(string.Empty, context.ModelValidationContext.ModelExplorer.Metadata, model) + { + ValidateAllProperties = true + }; // Act - validator.Validate(context.ModelValidationContext); + validator.Validate(context.ModelValidationContext, topLevelValidationNode); // Assert var actualErrors = new Dictionary(); @@ -240,6 +245,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation { // Arrange var testValidationContext = GetModelValidationContext(new Uri("/api/values", UriKind.Relative), typeof(Uri)); + var topLevelValidationNode = + new ModelValidationNode( + string.Empty, + testValidationContext.ModelValidationContext.ModelExplorer.Metadata, + testValidationContext.ModelValidationContext.ModelExplorer.Model) + { + ValidateAllProperties = true + }; // Act & Assert Assert.Throws( @@ -249,7 +262,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation new DefaultObjectValidator( testValidationContext.ExcludeFilters, testValidationContext.ModelMetadataProvider) - .Validate(testValidationContext.ModelValidationContext); + .Validate(testValidationContext.ModelValidationContext, topLevelValidationNode); }); } @@ -265,7 +278,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation yield return new object[] { new Dictionary { { "values", new Uri("/api/values", UriKind.Relative) }, { "hello", new Uri("/api/hello", UriKind.Relative) } - }, typeof(Uri), new List() { typeof(Uri) } }; + }, typeof(Dictionary), new List() { typeof(Uri) } }; } } @@ -276,13 +289,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation object input, Type type, List excludedTypes) { // Arrange - var testValidationContext = GetModelValidationContext(input, type, string.Empty, excludedTypes); + var testValidationContext = GetModelValidationContext(input, type, excludedTypes); + var topLevelValidationNode = + new ModelValidationNode( + string.Empty, + testValidationContext.ModelValidationContext.ModelExplorer.Metadata, + testValidationContext.ModelValidationContext.ModelExplorer.Model) + { + ValidateAllProperties = true + }; // Act & Assert (does not throw) new DefaultObjectValidator( testValidationContext.ExcludeFilters, testValidationContext.ModelMetadataProvider) - .Validate(testValidationContext.ModelValidationContext); + .Validate(testValidationContext.ModelValidationContext, topLevelValidationNode); Assert.True(testValidationContext.ModelValidationContext.ModelState.IsValid); } @@ -294,6 +315,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation var testValidationContext = GetModelValidationContext( new Uri("/api/values", UriKind.Relative), typeof(Uri)); var validationContext = testValidationContext.ModelValidationContext; + var topLevelValidationNode = + new ModelValidationNode( + string.Empty, + testValidationContext.ModelValidationContext.ModelExplorer.Metadata, + testValidationContext.ModelValidationContext.ModelExplorer.Model) + { + ValidateAllProperties = true + }; // Act & Assert Assert.Throws( @@ -302,7 +331,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation new DefaultObjectValidator( testValidationContext.ExcludeFilters, testValidationContext.ModelMetadataProvider) - .Validate(validationContext); + .Validate(validationContext, topLevelValidationNode); }); Assert.True(validationContext.ModelState.IsValid); } @@ -315,12 +344,20 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation var model = new Address() { Street = "Microsoft Way" }; var testValidationContext = GetModelValidationContext(model, model.GetType()); var validationContext = testValidationContext.ModelValidationContext; + var topLevelValidationNode = + new ModelValidationNode( + string.Empty, + testValidationContext.ModelValidationContext.ModelExplorer.Metadata, + testValidationContext.ModelValidationContext.ModelExplorer.Model) + { + ValidateAllProperties = true + }; // Act (does not throw) new DefaultObjectValidator( testValidationContext.ExcludeFilters, testValidationContext.ModelMetadataProvider) - .Validate(validationContext); + .Validate(validationContext, topLevelValidationNode); // Assert Assert.Contains("Street", validationContext.ModelState.Keys); @@ -340,12 +377,20 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation new TypeThatOverridesEquals { Funny = "hehe" } }; var testValidationContext = GetModelValidationContext(instance, typeof(TypeThatOverridesEquals[])); + var topLevelValidationNode = + new ModelValidationNode( + string.Empty, + testValidationContext.ModelValidationContext.ModelExplorer.Metadata, + testValidationContext.ModelValidationContext.ModelExplorer.Model) + { + ValidateAllProperties = true + }; // Act & Assert (does not throw) new DefaultObjectValidator( testValidationContext.ExcludeFilters, testValidationContext.ModelMetadataProvider) - .Validate(testValidationContext.ModelValidationContext); + .Validate(testValidationContext.ModelValidationContext, topLevelValidationNode); } [Fact] @@ -361,7 +406,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation var testValidationContext = GetModelValidationContext( user, typeof(User), - "user", new List { typeof(string) }); var validationContext = testValidationContext.ModelValidationContext; @@ -370,9 +414,17 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation var validator = new DefaultObjectValidator( testValidationContext.ExcludeFilters, testValidationContext.ModelMetadataProvider); + var topLevelValidationNode = + new ModelValidationNode( + "user", + testValidationContext.ModelValidationContext.ModelExplorer.Metadata, + testValidationContext.ModelValidationContext.ModelExplorer.Model) + { + ValidateAllProperties = true + }; // Act - validator.Validate(validationContext); + validator.Validate(validationContext, topLevelValidationNode); // Assert Assert.Equal(new[] { "key1", "user.Password", "", "user.ConfirmPassword" }, @@ -380,7 +432,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation var modelState = validationContext.ModelState["user.ConfirmPassword"]; Assert.Empty(modelState.Errors); Assert.Equal(modelState.ValidationState, ModelValidationState.Skipped); - + var error = Assert.Single(validationContext.ModelState[""].Errors); Assert.IsType(error.Exception); } @@ -398,15 +450,22 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation var testValidationContext = GetModelValidationContext( user, typeof(User), - "user", new List { typeof(User) }); var validationContext = testValidationContext.ModelValidationContext; var validator = new DefaultObjectValidator( testValidationContext.ExcludeFilters, testValidationContext.ModelMetadataProvider); + var topLevelValidationNode = + new ModelValidationNode( + "user", + testValidationContext.ModelValidationContext.ModelExplorer.Metadata, + testValidationContext.ModelValidationContext.ModelExplorer.Model) + { + ValidateAllProperties = true + }; // Act - validator.Validate(validationContext); + validator.Validate(validationContext, topLevelValidationNode); // Assert Assert.False(validationContext.ModelState.ContainsKey("user.Password")); @@ -429,7 +488,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation var testValidationContext = GetModelValidationContext( user, typeof(User), - "user", new List { typeof(User) }); var validationContext = testValidationContext.ModelValidationContext; @@ -440,9 +498,17 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation var validator = new DefaultObjectValidator( testValidationContext.ExcludeFilters, testValidationContext.ModelMetadataProvider); + var topLevelValidationNode = + new ModelValidationNode( + "user", + testValidationContext.ModelValidationContext.ModelExplorer.Metadata, + testValidationContext.ModelValidationContext.ModelExplorer.Model) + { + ValidateAllProperties = true + }; // Act - validator.Validate(validationContext); + validator.Validate(validationContext, topLevelValidationNode); // Assert var modelState = validationContext.ModelState["user.Password"]; @@ -455,21 +521,36 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation } [Fact] - public void NonRequestBoundModel_MarkedAsSkipped() + public void Validate_IfSuppressIsSet_MarkedAsSkipped() { // Arrange var testValidationContext = GetModelValidationContext( new TestServiceProvider(), - typeof(TestServiceProvider), - "serviceProvider"); + typeof(TestServiceProvider)); var validationContext = testValidationContext.ModelValidationContext; var validator = new DefaultObjectValidator( testValidationContext.ExcludeFilters, testValidationContext.ModelMetadataProvider); + var modelExplorer = testValidationContext.ModelValidationContext.ModelExplorer; + var topLevelValidationNode = new ModelValidationNode( + "serviceProvider", + modelExplorer.Metadata, + modelExplorer.Model); + + var propertyExplorer = modelExplorer.GetExplorerForProperty("TestService"); + var childNode = new ModelValidationNode( + "serviceProvider.TestService", + propertyExplorer.Metadata, + propertyExplorer.Model) + { + SuppressValidation = true + }; + + topLevelValidationNode.ChildNodes.Add(childNode); // Act - validator.Validate(validationContext); + validator.Validate(validationContext, topLevelValidationNode); // Assert Assert.True(validationContext.ModelState.IsValid); @@ -496,7 +577,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation var testValidationContext = GetModelValidationContext( model, type, - "items", excludedTypes: null, modelStateDictionary: modelStateDictionary); @@ -509,9 +589,17 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation var validator = new DefaultObjectValidator( testValidationContext.ExcludeFilters, testValidationContext.ModelMetadataProvider); + var topLevelValidationNode = + new ModelValidationNode( + "items", + testValidationContext.ModelValidationContext.ModelExplorer.Metadata, + testValidationContext.ModelValidationContext.ModelExplorer.Model) + { + ValidateAllProperties = true + }; // Act - validator.Validate(validationContext); + validator.Validate(validationContext, topLevelValidationNode); // Assert Assert.True(validationContext.ModelState.IsValid); @@ -538,7 +626,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation var testValidationContext = GetModelValidationContext( model, typeof(Dictionary), - "items", excludedTypes: null, modelStateDictionary: modelStateDictionary); @@ -552,8 +639,17 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation testValidationContext.ExcludeFilters, testValidationContext.ModelMetadataProvider); + var topLevelValidationNode = + new ModelValidationNode( + "items", + testValidationContext.ModelValidationContext.ModelExplorer.Metadata, + testValidationContext.ModelValidationContext.ModelExplorer.Model) + { + ValidateAllProperties = true + }; + // Act - validator.Validate(validationContext); + validator.Validate(validationContext, topLevelValidationNode); // Assert Assert.True(validationContext.ModelState.IsValid); @@ -569,19 +665,88 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation Assert.Equal(modelState.ValidationState, ModelValidationState.Skipped); } - private TestModelValidationContext GetModelValidationContext( - object model, - Type type, - string key = "", - List excludedTypes = null) + [Fact] + public void Validator_IfValidateAllPropertiesIsNotSet_DoesNotAutoExpand() { - return GetModelValidationContext(model, type, key, excludedTypes, new ModelStateDictionary()); + // Arrange + var testValidationContext = GetModelValidationContext( + LonelyPerson, + typeof(Person)); + + var validationContext = testValidationContext.ModelValidationContext; + var validator = new DefaultObjectValidator( + testValidationContext.ExcludeFilters, + testValidationContext.ModelMetadataProvider); + var modelExplorer = testValidationContext.ModelValidationContext.ModelExplorer; + + // No ChildNode added + var topLevelValidationNode = new ModelValidationNode( + "person", + modelExplorer.Metadata, + modelExplorer.Model); + + // Act + validator.Validate(validationContext, topLevelValidationNode); + + // Assert + Assert.True(validationContext.ModelState.IsValid); + var key = Assert.Single(validationContext.ModelState.Keys); + Assert.Equal("person", key); + } + + [Fact] + public void Validator_IfValidateAllPropertiesSet_WithChildNodes_DoesNotAutoExpand() + { + // Arrange + var testValidationContext = GetModelValidationContext( + LonelyPerson, + typeof(Person)); + + var validationContext = testValidationContext.ModelValidationContext; + var validator = new DefaultObjectValidator( + testValidationContext.ExcludeFilters, + testValidationContext.ModelMetadataProvider); + var modelExplorer = testValidationContext.ModelValidationContext.ModelExplorer; + + var topLevelValidationNode = new ModelValidationNode( + "person", + modelExplorer.Metadata, + modelExplorer.Model) + { + ValidateAllProperties = true + }; + + var propertyExplorer = modelExplorer.GetExplorerForProperty("Profession"); + var childNode = new ModelValidationNode( + "person.Profession", + propertyExplorer.Metadata, + propertyExplorer.Model); + + topLevelValidationNode.ChildNodes.Add(childNode); + + // Act + validator.Validate(validationContext, topLevelValidationNode); + + // Assert + var modelState = validationContext.ModelState; + Assert.False(modelState.IsValid); + + // Since the model is invalid at property level there is no entry in the model state for top level node. + Assert.Single(modelState.Keys, k => k == "person.Profession"); + Assert.Equal(1, modelState.Count); + } + + private TestModelValidationContext GetModelValidationContext( + object model, + Type type, + List excludedTypes = null) + { + return GetModelValidationContext(model, type, excludedTypes, new ModelStateDictionary()); } private TestModelValidationContext GetModelValidationContext( object model, Type type, - string key, List excludedTypes, ModelStateDictionary modelStateDictionary) { @@ -603,7 +768,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation return new TestModelValidationContext { ModelValidationContext = new ModelValidationContext( - key, null, TestModelValidatorProvider.CreateDefaultProvider(), modelStateDictionary, diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs index aee377fad0..f39ee0f6a4 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs @@ -154,7 +154,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test var mockValidatorProvider = new Mock(MockBehavior.Strict); mockValidatorProvider - .Setup(o => o.Validate(It.IsAny())) + .Setup(o => o.Validate(It.IsAny(), It.IsAny())) .Verifiable(); var argumentBinder = GetArgumentBinder(mockValidatorProvider.Object); @@ -164,7 +164,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test // Assert mockValidatorProvider.Verify( - o => o.Validate(It.IsAny()), Times.Once()); + o => o.Validate(It.IsAny(), It.IsAny()), Times.Once()); } [Fact] @@ -197,8 +197,9 @@ namespace Microsoft.AspNet.Mvc.Core.Test }; var mockValidatorProvider = new Mock(MockBehavior.Strict); - mockValidatorProvider.Setup(o => o.Validate(It.IsAny())) - .Verifiable(); + mockValidatorProvider + .Setup(o => o.Validate(It.IsAny(), It.IsAny())) + .Verifiable(); var argumentBinder = GetArgumentBinder(mockValidatorProvider.Object); // Act @@ -206,7 +207,9 @@ namespace Microsoft.AspNet.Mvc.Core.Test .BindActionArgumentsAsync(actionContext, actionBindingContext, new TestController()); // Assert - mockValidatorProvider.Verify(o => o.Validate(It.IsAny()), Times.Never()); + mockValidatorProvider.Verify( + o => o.Validate(It.IsAny(), It.IsAny()), + Times.Never()); } [Fact] @@ -226,7 +229,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test var mockValidatorProvider = new Mock(MockBehavior.Strict); mockValidatorProvider - .Setup(o => o.Validate(It.IsAny())) + .Setup(o => o.Validate(It.IsAny(), It.IsAny())) .Verifiable(); var argumentBinder = GetArgumentBinder(mockValidatorProvider.Object); @@ -236,7 +239,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test // Assert mockValidatorProvider.Verify( - o => o.Validate(It.IsAny()), Times.Once()); + o => o.Validate(It.IsAny(), It.IsAny()), Times.Once()); } [Fact] @@ -268,8 +271,10 @@ namespace Microsoft.AspNet.Mvc.Core.Test }; var mockValidatorProvider = new Mock(MockBehavior.Strict); - mockValidatorProvider.Setup(o => o.Validate(It.IsAny())) - .Verifiable(); + mockValidatorProvider + .Setup(o => o.Validate(It.IsAny(), It.IsAny())) + .Verifiable(); + var argumentBinder = GetArgumentBinder(mockValidatorProvider.Object); // Act @@ -277,7 +282,9 @@ namespace Microsoft.AspNet.Mvc.Core.Test .BindActionArgumentsAsync(actionContext, actionBindingContext, new TestController()); // Assert - mockValidatorProvider.Verify(o => o.Validate(It.IsAny()), Times.Never()); + mockValidatorProvider.Verify( + o => o.Validate(It.IsAny(), It.IsAny()), + Times.Never()); } [Fact] @@ -596,7 +603,8 @@ namespace Microsoft.AspNet.Mvc.Core.Test if (validator == null) { var mockValidator = new Mock(MockBehavior.Strict); - mockValidator.Setup(o => o.Validate(It.IsAny())); + mockValidator.Setup( + o => o.Validate(It.IsAny(), It.IsAny())); validator = mockValidator.Object; } diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/BinderTypeBasedModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/BinderTypeBasedModelBinderIntegrationTest.cs index db36daacc5..19fd5fac9a 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/BinderTypeBasedModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/BinderTypeBasedModelBinderIntegrationTest.cs @@ -281,7 +281,12 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests { public Task BindModelAsync(ModelBindingContext bindingContext) { - return Task.FromResult(new ModelBindingResult("Success", bindingContext.ModelName, true)); + var model = "Success"; + var modelValidationNode = new ModelValidationNode( + bindingContext.ModelName, + bindingContext.ModelMetadata, + model); + return Task.FromResult(new ModelBindingResult(model, bindingContext.ModelName, true, modelValidationNode)); } } diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs index aa02af55ed..74b4a2a27a 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs @@ -2,8 +2,11 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; +using System.IO; +using System.Text; using System.Threading.Tasks; using Microsoft.AspNet.Http; +using Microsoft.AspNet.Http.Internal; using Microsoft.AspNet.Mvc.ModelBinding; using Xunit; @@ -536,5 +539,63 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.Equal(0, modelState.ErrorCount); Assert.True(modelState.IsValid); } + + private class Person4 + { + public IList Addresses { get; set; } + } + + private class Address4 + { + public int Zip { get; set; } + + public string Street { get; set; } + } + + [Fact(Skip = "Extra ModelState key because of #2446")] + public async Task CollectionModelBinder_UsesCustomIndexes() + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(Person4) + }; + + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + var formCollection = new FormCollection(new Dictionary() + { + { "Addresses.index", new [] { "Key1", "Key2" } }, + { "Addresses[Key1].Street", new [] { "Street1" } }, + { "Addresses[Key2].Street", new [] { "Street2" } }, + }); + + request.Form = formCollection; + request.ContentType = "application/x-www-form-urlencoded"; + }); + + var modelState = new ModelStateDictionary(); + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext); + + // Assert + Assert.NotNull(modelBindingResult); + Assert.True(modelBindingResult.IsModelSet); + Assert.IsType(modelBindingResult.Model); + + Assert.Equal(2, modelState.Count); + Assert.Equal(0, modelState.ErrorCount); + Assert.True(modelState.IsValid); + var entry = Assert.Single(modelState, kvp => kvp.Key == "Addresses[Key1].Street").Value; + Assert.Equal("Street1", entry.Value.AttemptedValue); + Assert.Equal("Street1", entry.Value.RawValue); + + entry = Assert.Single(modelState, kvp => kvp.Key == "Addresses[Key2].Street").Value; + Assert.Equal("Street2", entry.Value.AttemptedValue); + Assert.Equal("Street2", entry.Value.RawValue); + } } } \ No newline at end of file