From f19c2e493d338c6d8724e3a16bbad0ac92a03054 Mon Sep 17 00:00:00 2001 From: Harsh Gupta Date: Wed, 4 Feb 2015 11:37:48 -0800 Subject: [PATCH] Merging Model Validation for body and non body validation. This also fixes #1503. Currently all model binders except mutable object binder are independent of validation code. The mutable object binder which needs to do some validation ( for scenarios involving [BindRequired] and [BindNever]). We would be going with an approach where required validaiton happens in input formatters and model binders. This is needed as validation for value types can best be done at creation time. Followup PRs: Introduce support for skipping validation (and not binding) for a particular property/type etc. --- Mvc.sln | 12 + src/Microsoft.AspNet.Mvc.Core/Controller.cs | 20 +- .../DefaultControllerActionArgumentBinder.cs | 17 +- .../ModelBinders/BodyModelBinder.cs | 21 +- .../ModelBinders/ServicesModelBinder.cs | 6 +- .../ParameterBinding/ModelBindingHelper.cs | 20 +- .../Binders/ArrayModelBinder.cs | 10 +- .../Binders/BinderTypeBasedModelBinder.cs | 18 +- .../Binders/BindingSourceModelBinder.cs | 15 +- .../Binders/ByteArrayModelBinder.cs | 13 +- .../Binders/CancellationTokenModelBinder.cs | 8 +- .../Binders/CollectionModelBinder.cs | 52 +- .../Binders/ComplexModelDto.cs | 4 +- .../Binders/ComplexModelDtoModelBinder.cs | 55 +- .../Binders/ComplexModelDtoResult.cs | 29 - .../Binders/CompositeModelBinder.cs | 106 ++-- .../Binders/DictionaryModelBinder.cs | 28 +- .../Binders/FormCollectionModelBinder.cs | 13 +- .../Binders/FormFileModelBinder.cs | 18 +- .../Binders/GenericModelBinder.cs | 12 +- .../Binders/HeaderModelBinder.cs | 10 +- .../Binders/IModelBinder.cs | 11 +- .../Binders/KeyValuePairModelBinder.cs | 58 +- .../Binders/MutableObjectModelBinder.cs | 71 +-- .../Binders/TypeConverterModelBinder.cs | 10 +- .../Binders/TypeMatchModelBinder.cs | 8 +- .../IObjectModelValidator.cs | 18 + .../Internal/CollectionModelBinderUtil.cs | 19 +- .../Internal/ModelBindingHelper.cs | 6 +- .../ModelBindingContext.cs | 48 -- .../ModelBindingResult.cs | 45 ++ .../ModelStateDictionary.cs | 43 +- .../ModelValidationState.cs | 1 + .../Properties/Resources.Designer.cs | 32 + .../Resources.resx | 6 + .../SimpleTypesExcludeFilter.cs | 1 - .../Validation/DefaultBodyModelValidator.cs | 264 -------- .../Validation/DefaultObjectValidator.cs | 299 +++++++++ .../Validation/IBodyModelValidator.cs | 22 - .../IValidationExcludeFiltersProvider.cs | 0 .../Validation/ModelValidatedEventArgs.cs | 21 - .../Validation/ModelValidatingEventArgs.cs | 21 - .../Validation/ModelValidationContext.cs | 29 +- .../Validation/ModelValidationNode.cs | 209 ------- .../ApiController.cs | 16 +- .../HttpRequestMessageModelBinder.cs | 8 +- ...piCompatShimServiceCollectionExtensions.cs | 6 + src/Microsoft.AspNet.Mvc/MvcOptionsSetup.cs | 6 + src/Microsoft.AspNet.Mvc/MvcServices.cs | 2 +- .../BodyModelBinderTests.cs | 36 +- .../ControllerActionInvokerTest.cs | 21 +- .../ControllerTests.cs | 23 +- .../DefaultControllerFactoryTest.cs | 5 +- .../DefaultModelBindersProviderTest.cs | 2 +- .../ModelBinderDescriptorTest.cs | 2 +- .../ControllerActionArgumentBinderTests.cs | 142 ++++- .../ModelBindingHelperTest.cs | 147 ++--- .../InputObjectValidationTests.cs | 4 +- .../ModelBindingTest.cs | 41 +- .../Binders/ArrayModelBinderTest.cs | 14 +- ...nderTypeBasedModelBinderModelBinderTest.cs | 19 +- .../Binders/BindingSourceModelBinderTest.cs | 11 +- .../Binders/ByteArrayModelBinderTests.cs | 13 +- .../CancellationTokenModelBinderTests.cs | 7 +- .../Binders/CollectionModelBinderTest.cs | 25 +- .../Binders/ComplexModelDtoResultTest.cs | 35 -- .../Binders/CompositeModelBinderTest.cs | 181 ++---- .../Binders/DictionaryModelBinderTest.cs | 12 +- .../Binders/FormCollectionModelBinderTest.cs | 16 +- .../Binders/FormFileModelBinderTest.cs | 28 +- .../Binders/HeaderModelBinderTests.cs | 10 +- .../Binders/KeyValuePairModelBinderTest.cs | 93 ++- .../Binders/ModelBindingContextTest.cs | 34 +- .../Binders/ModelBindingResultTest.cs | 27 + .../Binders/MutableObjectModelBinderTest.cs | 217 +++---- .../Binders/TypeConverterModelBinderTest.cs | 18 +- ...ests.cs => DefaultObjectValidatorTests.cs} | 264 ++++++-- .../Validation/ModelStateDictionaryTest.cs | 150 ++++- .../Validation/ModelValidationNodeTest.cs | 568 ------------------ .../MvcOptionsSetupTest.cs | 40 +- .../ModelBinderAttribute_ProductController.cs | 19 +- .../Controllers/ValidationController.cs | 31 + .../ModelBindingWebSite/Models/Address.cs | 4 + .../ModelBindingWebSite/Models/Resident.cs | 18 + test/WebSites/ModelBindingWebSite/Startup.cs | 1 + .../TestBindingSourceModelBinder.cs | 10 +- .../Views/Home/Index.cshtml | 2 +- .../Views/Home/Layout.cshtml | 2 +- .../Views/Home/_ViewStart.cshtml | 1 + 89 files changed, 1810 insertions(+), 2250 deletions(-) delete mode 100644 src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ComplexModelDtoResult.cs create mode 100644 src/Microsoft.AspNet.Mvc.ModelBinding/IObjectModelValidator.cs create mode 100644 src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingResult.cs delete mode 100644 src/Microsoft.AspNet.Mvc.ModelBinding/Validation/DefaultBodyModelValidator.cs create mode 100644 src/Microsoft.AspNet.Mvc.ModelBinding/Validation/DefaultObjectValidator.cs delete mode 100644 src/Microsoft.AspNet.Mvc.ModelBinding/Validation/IBodyModelValidator.cs rename src/{Microsoft.AspNet.Mvc.Core => Microsoft.AspNet.Mvc.ModelBinding/Validation}/IValidationExcludeFiltersProvider.cs (100%) delete mode 100644 src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidatedEventArgs.cs delete mode 100644 src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidatingEventArgs.cs delete mode 100644 test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ComplexModelDtoResultTest.cs create mode 100644 test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ModelBindingResultTest.cs rename test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/{DefaultBodyModelValidatorTests.cs => DefaultObjectValidatorTests.cs} (61%) delete mode 100644 test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelValidationNodeTest.cs create mode 100644 test/WebSites/ModelBindingWebSite/Controllers/ValidationController.cs create mode 100644 test/WebSites/ModelBindingWebSite/Models/Resident.cs diff --git a/Mvc.sln b/Mvc.sln index eef6514905..098e30615f 100644 --- a/Mvc.sln +++ b/Mvc.sln @@ -828,6 +828,18 @@ Global {C651F432-4EBE-41A6-BAD2-3E07CCBA209C}.Release|Mixed Platforms.Build.0 = Release|Any CPU {C651F432-4EBE-41A6-BAD2-3E07CCBA209C}.Release|x86.ActiveCfg = Release|Any CPU {C651F432-4EBE-41A6-BAD2-3E07CCBA209C}.Release|x86.Build.0 = Release|Any CPU + {22019146-BDFA-442E-8C8E-345FB9644578}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {22019146-BDFA-442E-8C8E-345FB9644578}.Debug|Any CPU.Build.0 = Debug|Any CPU + {22019146-BDFA-442E-8C8E-345FB9644578}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU + {22019146-BDFA-442E-8C8E-345FB9644578}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU + {22019146-BDFA-442E-8C8E-345FB9644578}.Debug|x86.ActiveCfg = Debug|Any CPU + {22019146-BDFA-442E-8C8E-345FB9644578}.Debug|x86.Build.0 = Debug|Any CPU + {22019146-BDFA-442E-8C8E-345FB9644578}.Release|Any CPU.ActiveCfg = Release|Any CPU + {22019146-BDFA-442E-8C8E-345FB9644578}.Release|Any CPU.Build.0 = Release|Any CPU + {22019146-BDFA-442E-8C8E-345FB9644578}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU + {22019146-BDFA-442E-8C8E-345FB9644578}.Release|Mixed Platforms.Build.0 = Release|Any CPU + {22019146-BDFA-442E-8C8E-345FB9644578}.Release|x86.ActiveCfg = Release|Any CPU + {22019146-BDFA-442E-8C8E-345FB9644578}.Release|x86.Build.0 = Release|Any CPU {A19022EF-9BA3-4349-94E4-F48E13E1C8AE}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {A19022EF-9BA3-4349-94E4-F48E13E1C8AE}.Debug|Any CPU.Build.0 = Debug|Any CPU {A19022EF-9BA3-4349-94E4-F48E13E1C8AE}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU diff --git a/src/Microsoft.AspNet.Mvc.Core/Controller.cs b/src/Microsoft.AspNet.Mvc.Core/Controller.cs index 39a4541c0c..1bb65494ad 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Controller.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Controller.cs @@ -135,6 +135,9 @@ namespace Microsoft.AspNet.Mvc [FromServices] public IUrlHelper Url { get; set; } + [FromServices] + public IObjectModelValidator ObjectValidator { get; set; } + /// /// Gets or sets the for user associated with the executing action. /// @@ -938,6 +941,7 @@ namespace Microsoft.AspNet.Mvc MetadataProvider, BindingContext.ModelBinder, valueProvider, + ObjectValidator, BindingContext.ValidatorProvider); } @@ -975,6 +979,7 @@ namespace Microsoft.AspNet.Mvc MetadataProvider, BindingContext.ModelBinder, BindingContext.ValueProvider, + ObjectValidator, BindingContext.ValidatorProvider, includeExpressions); } @@ -1012,6 +1017,7 @@ namespace Microsoft.AspNet.Mvc MetadataProvider, BindingContext.ModelBinder, BindingContext.ValueProvider, + ObjectValidator, BindingContext.ValidatorProvider, predicate); } @@ -1052,6 +1058,7 @@ namespace Microsoft.AspNet.Mvc MetadataProvider, BindingContext.ModelBinder, valueProvider, + ObjectValidator, BindingContext.ValidatorProvider, includeExpressions); } @@ -1091,6 +1098,7 @@ namespace Microsoft.AspNet.Mvc MetadataProvider, BindingContext.ModelBinder, valueProvider, + ObjectValidator, BindingContext.ValidatorProvider, predicate); } @@ -1128,21 +1136,15 @@ namespace Microsoft.AspNet.Mvc modelAccessor: () => model, modelType: model.GetType()); + var modelName = prefix ?? string.Empty; var validationContext = new ModelValidationContext( - MetadataProvider, + modelName, BindingContext.ValidatorProvider, ModelState, modelMetadata, containerMetadata: null); - var modelName = prefix ?? string.Empty; - - var validationNode = new ModelValidationNode(modelMetadata, modelName) - { - ValidateAllProperties = true - }; - validationNode.Validate(validationContext); - + ObjectValidator.Validate(validationContext); return ModelState.IsValid; } diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs index 0096ce1373..59661da018 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs @@ -18,13 +18,16 @@ namespace Microsoft.AspNet.Mvc { private readonly IModelMetadataProvider _modelMetadataProvider; private readonly MvcOptions _options; + private readonly IObjectModelValidator _validator; public DefaultControllerActionArgumentBinder( IModelMetadataProvider modelMetadataProvider, + IObjectModelValidator validator, IOptions optionsAccessor) { _modelMetadataProvider = modelMetadataProvider; _options = optionsAccessor.Options; + _validator = validator; } public async Task> GetActionArgumentsAsync( @@ -91,15 +94,21 @@ namespace Microsoft.AspNet.Mvc var modelState = actionContext.ModelState; modelState.MaxAllowedErrors = _options.MaxModelValidationErrors; - foreach (var parameter in parameterMetadata) { var parameterType = parameter.ModelType; var modelBindingContext = GetModelBindingContext(parameter, modelState, operationBindingContext); - if (await bindingContext.ModelBinder.BindModelAsync(modelBindingContext) && - modelBindingContext.IsModelSet) + var modelBindingResult = await bindingContext.ModelBinder.BindModelAsync(modelBindingContext); + if (modelBindingResult != null && modelBindingResult.IsModelSet) { - arguments[parameter.PropertyName] = modelBindingContext.Model; + arguments[parameter.PropertyName] = modelBindingResult.Model; + var validationContext = new ModelValidationContext( + modelBindingResult.Key, + bindingContext.ValidatorProvider, + actionContext.ModelState, + parameter, + containerMetadata: null); + _validator.Validate(validationContext); } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinders/BodyModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinders/BodyModelBinder.cs index e6f23cf57b..98ad787728 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinders/BodyModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinders/BodyModelBinder.cs @@ -19,7 +19,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding private readonly ActionContext _actionContext; private readonly IScopedInstance _bindingContext; private readonly IInputFormatterSelector _formatterSelector; - private readonly IBodyModelValidator _bodyModelValidator; private readonly IValidationExcludeFiltersProvider _bodyValidationExcludeFiltersProvider; /// @@ -28,26 +27,23 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// An accessor to the . /// An accessor to the . /// The . - /// The . /// /// The . /// public BodyModelBinder([NotNull] IScopedInstance context, [NotNull] IScopedInstance bindingContext, [NotNull] IInputFormatterSelector selector, - [NotNull] IBodyModelValidator bodyModelValidator, [NotNull] IValidationExcludeFiltersProvider bodyValidationExcludeFiltersProvider) : base(BindingSource.Body) { _actionContext = context.Value; _bindingContext = bindingContext; _formatterSelector = selector; - _bodyModelValidator = bodyModelValidator; _bodyValidationExcludeFiltersProvider = bodyValidationExcludeFiltersProvider; } /// - protected async override Task BindModelCoreAsync([NotNull] ModelBindingContext bindingContext) + protected async override Task BindModelCoreAsync([NotNull] ModelBindingContext bindingContext) { var formatters = _bindingContext.Value.InputFormatters; @@ -59,20 +55,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var unsupportedContentType = Resources.FormatUnsupportedContentType( bindingContext.OperationBindingContext.HttpContext.Request.ContentType); bindingContext.ModelState.AddModelError(bindingContext.ModelName, unsupportedContentType); - return; + return new ModelBindingResult(null, bindingContext.ModelName, isModelSet: false); } - bindingContext.Model = await formatter.ReadAsync(formatterContext); - - // Validate the deserialized object - var validationContext = new ModelValidationContext( - bindingContext.OperationBindingContext.MetadataProvider, - bindingContext.OperationBindingContext.ValidatorProvider, - bindingContext.ModelState, - bindingContext.ModelMetadata, - containerMetadata: null, - excludeFromValidationFilters: _bodyValidationExcludeFiltersProvider.ExcludeFilters); - _bodyModelValidator.Validate(validationContext, bindingContext.ModelName); + var model = await formatter.ReadAsync(formatterContext); + return new ModelBindingResult(model, bindingContext.ModelName, isModelSet: true); } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinders/ServicesModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinders/ServicesModelBinder.cs index 07f4f809b4..79e65e0480 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinders/ServicesModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinders/ServicesModelBinder.cs @@ -21,11 +21,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } /// - protected override Task BindModelCoreAsync([NotNull] ModelBindingContext bindingContext) + protected override Task BindModelCoreAsync([NotNull] ModelBindingContext bindingContext) { var requestServices = bindingContext.OperationBindingContext.HttpContext.RequestServices; - bindingContext.Model = requestServices.GetRequiredService(bindingContext.ModelType); - return Task.FromResult(true); + var model = requestServices.GetRequiredService(bindingContext.ModelType); + return Task.FromResult(new ModelBindingResult(model, bindingContext.ModelName, true)); } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ParameterBinding/ModelBindingHelper.cs b/src/Microsoft.AspNet.Mvc.Core/ParameterBinding/ModelBindingHelper.cs index 86775f3a85..5285541e1f 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ParameterBinding/ModelBindingHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ParameterBinding/ModelBindingHelper.cs @@ -29,6 +29,8 @@ namespace Microsoft.AspNet.Mvc /// The provider used for reading metadata for the model type. /// The used for binding. /// The used for looking up values. + /// The used for validating the + /// bound values. /// The used for executing validation /// on the model instance. /// A that on completion returns true if the update is successful @@ -40,6 +42,7 @@ namespace Microsoft.AspNet.Mvc [NotNull] IModelMetadataProvider metadataProvider, [NotNull] IModelBinder modelBinder, [NotNull] IValueProvider valueProvider, + [NotNull] IObjectModelValidator objectModelValidator, [NotNull] IModelValidatorProvider validatorProvider) where TModel : class { @@ -52,6 +55,7 @@ namespace Microsoft.AspNet.Mvc metadataProvider, modelBinder, valueProvider, + objectModelValidator, validatorProvider, predicate: (context, propertyName) => true); } @@ -71,6 +75,8 @@ namespace Microsoft.AspNet.Mvc /// The provider used for reading metadata for the model type. /// The used for binding. /// The used for looking up values. + /// The used for validating the + /// bound values. /// The used for executing validation /// on the model /// instance. @@ -85,6 +91,7 @@ namespace Microsoft.AspNet.Mvc [NotNull] IModelMetadataProvider metadataProvider, [NotNull] IModelBinder modelBinder, [NotNull] IValueProvider valueProvider, + [NotNull] IObjectModelValidator objectModelValidator, [NotNull] IModelValidatorProvider validatorProvider, [NotNull] params Expression>[] includeExpressions) where TModel : class @@ -100,6 +107,7 @@ namespace Microsoft.AspNet.Mvc metadataProvider, modelBinder, valueProvider, + objectModelValidator, validatorProvider, predicate: predicate); } @@ -119,6 +127,8 @@ namespace Microsoft.AspNet.Mvc /// The provider used for reading metadata for the model type. /// The used for binding. /// The used for looking up values. + /// /// The used for validating the + /// bound values. /// The used for executing validation /// on the model instance. /// A predicate which can be used to @@ -132,12 +142,13 @@ namespace Microsoft.AspNet.Mvc [NotNull] IModelMetadataProvider metadataProvider, [NotNull] IModelBinder modelBinder, [NotNull] IValueProvider valueProvider, + [NotNull] IObjectModelValidator objectModelValidator, [NotNull] IModelValidatorProvider validatorProvider, [NotNull] Func predicate) where TModel : class { var modelMetadata = metadataProvider.GetMetadataForType( - modelAccessor: null, + modelAccessor: () => model, modelType: model.GetType()); var operationBindingContext = new OperationBindingContext @@ -152,7 +163,6 @@ namespace Microsoft.AspNet.Mvc { ModelMetadata = modelMetadata, ModelName = prefix, - Model = model, ModelState = modelState, ValueProvider = valueProvider, FallbackToEmptyPrefix = true, @@ -160,8 +170,12 @@ namespace Microsoft.AspNet.Mvc PropertyFilter = predicate }; - if (await modelBinder.BindModelAsync(modelBindingContext)) + var modelBindingResult = await modelBinder.BindModelAsync(modelBindingContext); + if (modelBindingResult != null) { + var modelValidationContext = new ModelValidationContext(modelBindingContext, modelMetadata); + modelValidationContext.RootPrefix = prefix; + objectModelValidator.Validate(modelValidationContext); return modelState.IsValid; } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ArrayModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ArrayModelBinder.cs index e231b5c00b..414903b27c 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ArrayModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ArrayModelBinder.cs @@ -9,21 +9,19 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { public class ArrayModelBinder : CollectionModelBinder { - public override Task BindModelAsync(ModelBindingContext bindingContext) + public override Task BindModelAsync(ModelBindingContext bindingContext) { if (bindingContext.ModelMetadata.IsReadOnly) { - return Task.FromResult(false); + return Task.FromResult(null); } return base.BindModelAsync(bindingContext); } - protected override bool CreateOrReplaceCollection(ModelBindingContext bindingContext, - IList newCollection) + protected override object GetModel(IEnumerable newCollection) { - bindingContext.Model = newCollection.ToArray(); - return true; + return newCollection.ToArray(); } } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/BinderTypeBasedModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/BinderTypeBasedModelBinder.cs index 714325c68a..9d595443df 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/BinderTypeBasedModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/BinderTypeBasedModelBinder.cs @@ -21,13 +21,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding new ConcurrentDictionary(); - public async Task BindModelAsync(ModelBindingContext bindingContext) + public async Task BindModelAsync(ModelBindingContext bindingContext) { if (bindingContext.ModelMetadata.BinderType == null) { - // Return false so that we are able to continue with the default set of model binders, + // Return null so that we are able to continue with the default set of model binders, // if there is no specific model binder provided. - return false; + return null; } var requestServices = bindingContext.OperationBindingContext.HttpContext.RequestServices; @@ -52,11 +52,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } } - await modelBinder.BindModelAsync(bindingContext); + var result = await modelBinder.BindModelAsync(bindingContext); - // return true here, because this binder will handle all cases where the model binder is - // specified by metadata. - return true; + var modelBindingResult = result != null ? + new ModelBindingResult(result.Model, result.Key, result.IsModelSet) : + new ModelBindingResult(null, bindingContext.ModelName, false); + + // return a non null modelbinding result here, because this binder will handle all cases where the + // model binder is specified by metadata. + return modelBindingResult; } } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/BindingSourceModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/BindingSourceModelBinder.cs index b89d479c98..bc1967782f 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/BindingSourceModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/BindingSourceModelBinder.cs @@ -58,10 +58,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// /// A which will complete when model binding has completed. /// - protected abstract Task BindModelCoreAsync([NotNull] ModelBindingContext bindingContext); + protected abstract Task BindModelCoreAsync([NotNull] ModelBindingContext bindingContext); /// - public async Task BindModelAsync(ModelBindingContext context) + public async Task BindModelAsync(ModelBindingContext context) { var bindingSourceMetadata = context.ModelMetadata.BinderMetadata as IBindingSourceMetadata; var allowedBindingSource = bindingSourceMetadata?.BindingSource; @@ -70,14 +70,19 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { // Binding Sources are opt-in. This model either didn't specify one or specified something // incompatible so let other binders run. - return false; + return null; } - await BindModelCoreAsync(context); + var result = await BindModelCoreAsync(context); + + var modelBindingResult = + result != null ? + new ModelBindingResult(result.Model, result.Key, result.IsModelSet) : + new ModelBindingResult(null, context.ModelName, false); // Prevent other model binders from running because this model binder is the only handler for // its binding source. - return true; + return modelBindingResult; } } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ByteArrayModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ByteArrayModelBinder.cs index 6c8bcabf60..6a39463452 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ByteArrayModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ByteArrayModelBinder.cs @@ -13,11 +13,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public class ByteArrayModelBinder : IModelBinder { /// - public async Task BindModelAsync([NotNull] ModelBindingContext bindingContext) + public async Task BindModelAsync([NotNull] ModelBindingContext bindingContext) { if (bindingContext.ModelType != typeof(byte[])) { - return false; + return null; } var valueProviderResult = await bindingContext.ValueProvider.GetValueAsync(bindingContext.ModelName); @@ -25,7 +25,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // case 1: there was no element containing this data if (valueProviderResult == null) { - return false; + return null; } var value = valueProviderResult.AttemptedValue; @@ -33,19 +33,20 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // case 2: there was an element but it was left blank if (string.IsNullOrEmpty(value)) { - return false; + return null; } try { - bindingContext.Model = Convert.FromBase64String(value); + var model = Convert.FromBase64String(value); + return new ModelBindingResult(model, bindingContext.ModelName, true); } catch (Exception ex) { bindingContext.ModelState.TryAddModelError(bindingContext.ModelName, ex); } - return true; + return new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false); } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CancellationTokenModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CancellationTokenModelBinder.cs index 9313dc17ab..0eb879f649 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CancellationTokenModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CancellationTokenModelBinder.cs @@ -12,15 +12,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public class CancellationTokenModelBinder : IModelBinder { /// - public Task BindModelAsync(ModelBindingContext bindingContext) + public Task BindModelAsync(ModelBindingContext bindingContext) { if (bindingContext.ModelType == typeof(CancellationToken)) { - bindingContext.Model = bindingContext.OperationBindingContext.HttpContext.RequestAborted; - return Task.FromResult(true); + var model = bindingContext.OperationBindingContext.HttpContext.RequestAborted; + return Task.FromResult(new ModelBindingResult(model, bindingContext.ModelName, true)); } - return Task.FromResult(false); + return Task.FromResult(null); } } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CollectionModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CollectionModelBinder.cs index f2e6ff25d1..26d579d5dd 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CollectionModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CollectionModelBinder.cs @@ -13,13 +13,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { public class CollectionModelBinder : IModelBinder { - public virtual async Task BindModelAsync(ModelBindingContext bindingContext) + public virtual async Task BindModelAsync(ModelBindingContext bindingContext) { ModelBindingHelper.ValidateBindingContext(bindingContext); if (!await bindingContext.ValueProvider.ContainsPrefixAsync(bindingContext.ModelName)) { - return false; + return null; } var valueProviderResult = await bindingContext.ValueProvider.GetValueAsync(bindingContext.ModelName); @@ -27,13 +27,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding BindSimpleCollection(bindingContext, valueProviderResult.RawValue, valueProviderResult.Culture) : BindComplexCollection(bindingContext); var boundCollection = await bindCollectionTask; - - return CreateOrReplaceCollection(bindingContext, boundCollection); + var model = GetModel(boundCollection); + return new ModelBindingResult(model, bindingContext.ModelName, true); } // 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(ModelBindingContext bindingContext, + internal async Task> BindSimpleCollection(ModelBindingContext bindingContext, object rawValue, CultureInfo culture) { @@ -62,10 +62,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding }; object boundValue = null; - if (await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(innerBindingContext)) + var result = await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(innerBindingContext); + if (result != null) { - boundValue = innerBindingContext.Model; - bindingContext.ValidationNode.ChildNodes.Add(innerBindingContext.ValidationNode); + boundValue = result.Model; } boundCollection.Add(ModelBindingHelper.CastOrDefault(boundValue)); } @@ -74,7 +74,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } // 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 = ModelBindingHelper.CreatePropertyModelName(bindingContext.ModelName, "index"); var valueProviderResultIndex = await bindingContext.ValueProvider.GetValueAsync(indexPropertyName); @@ -82,7 +82,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return await BindComplexCollectionFromIndexes(bindingContext, indexNames); } - internal async Task> BindComplexCollectionFromIndexes(ModelBindingContext bindingContext, + internal async Task> BindComplexCollectionFromIndexes(ModelBindingContext bindingContext, IEnumerable indexNames) { bool indexNamesIsFinite; @@ -110,13 +110,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var modelType = bindingContext.ModelType; - if (await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(childBindingContext)) + var result = await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(childBindingContext); + if (result != null) { didBind = true; - boundValue = childBindingContext.Model; - - // merge validation up - bindingContext.ValidationNode.ChildNodes.Add(childBindingContext.ValidationNode); + boundValue = result.Model; } // infinite size collection stops on first bind failure @@ -133,11 +131,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Extensibility point that allows the bound collection to be manipulated or transformed before // being returned from the binder. - protected virtual bool CreateOrReplaceCollection(ModelBindingContext bindingContext, - IList newCollection) + protected virtual object GetModel(IEnumerable newCollection) { - CreateOrReplaceCollection(bindingContext, newCollection, () => new List()); - return true; + return newCollection; } internal static object[] RawValueToObjectArray(object rawValue) @@ -165,23 +161,5 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // fallback return new[] { rawValue }; } - - internal static void CreateOrReplaceCollection(ModelBindingContext bindingContext, - IEnumerable incomingElements, - Func> creator) - { - var collection = bindingContext.Model as ICollection; - if (collection == null || collection.IsReadOnly) - { - collection = creator(); - bindingContext.Model = collection; - } - - collection.Clear(); - foreach (var element in incomingElements) - { - collection.Add(element); - } - } } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ComplexModelDto.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ComplexModelDto.cs index 543f6de1fa..a3c2e92b34 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ComplexModelDto.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ComplexModelDto.cs @@ -15,7 +15,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { ModelMetadata = modelMetadata; PropertyMetadata = new Collection(propertyMetadata.ToList()); - Results = new Dictionary(); + Results = new Dictionary(); } public ModelMetadata ModelMetadata { get; private set; } @@ -26,6 +26,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // attempted. If binding failed, the entry's value will be null. If binding // was never attempted, this dictionary will not contain a corresponding // entry. - public IDictionary Results { get; private set; } + public IDictionary Results { get; private set; } } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ComplexModelDtoModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ComplexModelDtoModelBinder.cs index c92fe2540b..7e65069dcc 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ComplexModelDtoModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ComplexModelDtoModelBinder.cs @@ -8,37 +8,38 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { public sealed class ComplexModelDtoModelBinder : IModelBinder { - public async Task BindModelAsync(ModelBindingContext bindingContext) + public async Task BindModelAsync(ModelBindingContext bindingContext) { - if (bindingContext.ModelType == typeof(ComplexModelDto)) + if (bindingContext.ModelType != typeof(ComplexModelDto)) { - ModelBindingHelper.ValidateBindingContext(bindingContext, - typeof(ComplexModelDto), - allowNullModel: false); - - var dto = (ComplexModelDto)bindingContext.Model; - foreach (var propertyMetadata in dto.PropertyMetadata) - { - var propertyModelName = ModelBindingHelper.CreatePropertyModelName(bindingContext.ModelName, - propertyMetadata.PropertyName); - - var propertyBindingContext = new ModelBindingContext(bindingContext, - propertyModelName, - propertyMetadata); - - // bind and propagate the values - // If we can't bind then leave the result missing (don't add a null). - if (await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(propertyBindingContext)) - { - var result = ComplexModelDtoResult.FromBindingContext(propertyBindingContext); - dto.Results[propertyMetadata] = result; - } - } - - return true; + return null; } - return false; + ModelBindingHelper.ValidateBindingContext(bindingContext, + typeof(ComplexModelDto), + allowNullModel: false); + + var dto = (ComplexModelDto)bindingContext.ModelMetadata.Model; + foreach (var propertyMetadata in dto.PropertyMetadata) + { + var propertyModelName = ModelBindingHelper.CreatePropertyModelName(bindingContext.ModelName, + propertyMetadata.PropertyName); + + var propertyBindingContext = new ModelBindingContext(bindingContext, + propertyModelName, + propertyMetadata); + + // bind and propagate the values + // If we can't bind then leave the result missing (don't add a null). + var modelBindingResult = + await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(propertyBindingContext); + if (modelBindingResult != null) + { + dto.Results[propertyMetadata] = modelBindingResult; + } + } + + return new ModelBindingResult(dto, bindingContext.ModelName, isModelSet: true); } } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ComplexModelDtoResult.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ComplexModelDtoResult.cs deleted file mode 100644 index cd298dd477..0000000000 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ComplexModelDtoResult.cs +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -namespace Microsoft.AspNet.Mvc.ModelBinding -{ - public sealed class ComplexModelDtoResult - { - public static ComplexModelDtoResult FromBindingContext([NotNull] ModelBindingContext context) - { - return new ComplexModelDtoResult(context.Model, context.IsModelSet, context.ValidationNode); - } - - public ComplexModelDtoResult( - object model, - bool isModelBound, - [NotNull] ModelValidationNode validationNode) - { - Model = model; - IsModelBound = isModelBound; - ValidationNode = validationNode; - } - - public bool IsModelBound { get; } - - public object Model { get; set; } - - public ModelValidationNode ValidationNode { get; set; } - } -} diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CompositeModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CompositeModelBinder.cs index a39638b1cc..e34104e597 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CompositeModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CompositeModelBinder.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Runtime.CompilerServices; using System.Threading.Tasks; +using Microsoft.Framework.DependencyInjection; namespace Microsoft.AspNet.Mvc.ModelBinding { @@ -30,97 +31,86 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// public IReadOnlyList ModelBinders { get; } - public virtual async Task BindModelAsync([NotNull] ModelBindingContext bindingContext) + public virtual async Task BindModelAsync([NotNull] ModelBindingContext bindingContext) { var newBindingContext = CreateNewBindingContext(bindingContext, - bindingContext.ModelName, - reuseValidationNode: true); + bindingContext.ModelName); - var boundSuccessfully = await TryBind(newBindingContext); - if (!boundSuccessfully && !string.IsNullOrEmpty(bindingContext.ModelName) + var modelBindingResult = await TryBind(newBindingContext); + if (modelBindingResult == null && !string.IsNullOrEmpty(bindingContext.ModelName) && bindingContext.FallbackToEmptyPrefix) { // fallback to empty prefix? newBindingContext = CreateNewBindingContext(bindingContext, - modelName: string.Empty, - reuseValidationNode: false); - boundSuccessfully = await TryBind(newBindingContext); + modelName: string.Empty); + modelBindingResult = await TryBind(newBindingContext); } - if (!boundSuccessfully) + if (modelBindingResult == null) { - return false; // something went wrong - } - - // Only perform validation at the root of the object graph. ValidationNode will recursively walk the graph. - // Ignore ComplexModelDto since it essentially wraps the primary object. - if (newBindingContext.IsModelSet && IsBindingAtRootOfObjectGraph(newBindingContext)) - { - // run validation and return the model - // If we fell back to an empty prefix above and are dealing with simple types, - // propagate the non-blank model name through for user clarity in validation errors. - // Complex types will reveal their individual properties as model names and do not require this. - if (!newBindingContext.ModelMetadata.IsComplexType && - string.IsNullOrEmpty(newBindingContext.ModelName)) - { - newBindingContext.ValidationNode = new ModelValidationNode(newBindingContext.ModelMetadata, - bindingContext.ModelName); - } - - var validationContext = new ModelValidationContext( - bindingContext.OperationBindingContext.MetadataProvider, - bindingContext.OperationBindingContext.ValidatorProvider, - bindingContext.ModelState, - bindingContext.ModelMetadata, - containerMetadata: null); - - newBindingContext.ValidationNode.Validate(validationContext, parentNode: null); + return null; // something went wrong } bindingContext.OperationBindingContext.BodyBindingState = newBindingContext.OperationBindingContext.BodyBindingState; - if (newBindingContext.IsModelSet) + if (modelBindingResult.IsModelSet) { - bindingContext.Model = newBindingContext.Model; + bindingContext.ModelMetadata.Model = modelBindingResult.Model; + + // Update the model state key if we are bound using an empty prefix and it is a complex type. + // This is needed as validation uses the model state key to log errors. The client validation expects + // the errors with property names rather than the full name. + if (newBindingContext.ModelMetadata.IsComplexType && string.IsNullOrEmpty(modelBindingResult.Key)) + { + // For non-complex types, if we fell back to the empty prefix, we should still be using the name + // of the parameter/property. Complex types have their own property names which acts as model + // state keys and do not need special treatment. + // For example : + // + // public class Model + // { + // public int SimpleType { get; set; } + // } + // public void Action(int id, Model model) + // { + // } + // + // 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; + } } - return true; + return new ModelBindingResult( + modelBindingResult.Model, + bindingContext.ModelName, + modelBindingResult.IsModelSet); } - private async Task TryBind(ModelBindingContext bindingContext) + private async Task TryBind(ModelBindingContext bindingContext) { RuntimeHelpers.EnsureSufficientExecutionStack(); foreach (var binder in ModelBinders) { - if (await binder.BindModelAsync(bindingContext)) + var result = await binder.BindModelAsync(bindingContext); + if (result != null) { - return true; + return result; } } // Either we couldn't find a binder, or the binder couldn't bind. Distinction is not important. - return false; - } - - private static bool IsBindingAtRootOfObjectGraph(ModelBindingContext bindingContext) - { - // We're at the root of the object graph if the model does does not have a container. - // This statement is true for complex types at the root twice over - once with the actual model - // and once when when it is represented by a ComplexModelDto. Ignore the latter case. - - return bindingContext.ModelMetadata.ContainerType == null && - bindingContext.ModelMetadata.ModelType != typeof(ComplexModelDto); + return null; } private static ModelBindingContext CreateNewBindingContext(ModelBindingContext oldBindingContext, - string modelName, - bool reuseValidationNode) + string modelName) { var newBindingContext = new ModelBindingContext { - IsModelSet = oldBindingContext.IsModelSet, ModelMetadata = oldBindingContext.ModelMetadata, ModelName = modelName, ModelState = oldBindingContext.ModelState, @@ -129,12 +119,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding PropertyFilter = oldBindingContext.PropertyFilter, }; - // validation is expensive to create, so copy it over if we can - if (reuseValidationNode) - { - newBindingContext.ValidationNode = oldBindingContext.ValidationNode; - } - newBindingContext.OperationBindingContext.BodyBindingState = GetBodyBindingState(oldBindingContext); // If the property has a specified data binding sources, we need to filter the set of value providers diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/DictionaryModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/DictionaryModelBinder.cs index 53cea8a8d0..328670a8f4 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/DictionaryModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/DictionaryModelBinder.cs @@ -3,37 +3,15 @@ using System; using System.Collections.Generic; +using System.Linq; namespace Microsoft.AspNet.Mvc.ModelBinding { public class DictionaryModelBinder : CollectionModelBinder> { - protected override bool CreateOrReplaceCollection(ModelBindingContext bindingContext, - IList> newCollection) + protected override object GetModel(IEnumerable> newCollection) { - CreateOrReplaceDictionary(bindingContext, newCollection, () => new Dictionary()); - return true; - } - - private static void CreateOrReplaceDictionary(ModelBindingContext bindingContext, - IEnumerable> incomingElements, - Func> creator) - { - var dictionary = bindingContext.Model as IDictionary; - if (dictionary == null || dictionary.IsReadOnly) - { - dictionary = creator(); - bindingContext.Model = dictionary; - } - - dictionary.Clear(); - foreach (var element in incomingElements) - { - if (element.Key != null) - { - dictionary[element.Key] = element.Value; - } - } + return newCollection.ToDictionary(kvp => kvp.Key, kvp => kvp.Value); } } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/FormCollectionModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/FormCollectionModelBinder.cs index 50a2407e28..31cb016a29 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/FormCollectionModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/FormCollectionModelBinder.cs @@ -16,35 +16,36 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public class FormCollectionModelBinder : IModelBinder { /// - public async Task BindModelAsync([NotNull] ModelBindingContext bindingContext) + public async Task BindModelAsync([NotNull] ModelBindingContext bindingContext) { if (bindingContext.ModelType != typeof(IFormCollection) && bindingContext.ModelType != typeof(FormCollection)) { - return false; + return null; } + object model = null; var request = bindingContext.OperationBindingContext.HttpContext.Request; if (request.HasFormContentType) { var form = await request.ReadFormAsync(); if (bindingContext.ModelType.IsAssignableFrom(form.GetType())) { - bindingContext.Model = form; + model = form; } else { var formValuesLookup = form.ToDictionary(p => p.Key, p => p.Value); - bindingContext.Model = new FormCollection(formValuesLookup, form.Files); + model = new FormCollection(formValuesLookup, form.Files); } } else { - bindingContext.Model = new FormCollection(new Dictionary()); + model = new FormCollection(new Dictionary()); } - return true; + return new ModelBindingResult(model, bindingContext.ModelName, true); } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/FormFileModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/FormFileModelBinder.cs index e8796a45b4..d04ed36833 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/FormFileModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/FormFileModelBinder.cs @@ -18,33 +18,23 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public class FormFileModelBinder : IModelBinder { /// - public async Task BindModelAsync([NotNull] ModelBindingContext bindingContext) + public async Task BindModelAsync([NotNull] ModelBindingContext bindingContext) { if (bindingContext.ModelType == typeof(IFormFile)) { var postedFiles = await GetFormFilesAsync(bindingContext); var value = postedFiles.FirstOrDefault(); - if (value != null) - { - bindingContext.Model = value; - } - - return true; + return new ModelBindingResult(value, bindingContext.ModelName, value != null); } else if (typeof(IEnumerable).GetTypeInfo().IsAssignableFrom( bindingContext.ModelType.GetTypeInfo())) { var postedFiles = await GetFormFilesAsync(bindingContext); var value = ModelBindingHelper.ConvertValuesToCollectionType(bindingContext.ModelType, postedFiles); - if (value != null) - { - bindingContext.Model = value; - } - - return true; + return new ModelBindingResult(value, bindingContext.ModelName, value != null); } - return false; + return null; } private async Task> GetFormFilesAsync(ModelBindingContext bindingContext) diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/GenericModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/GenericModelBinder.cs index 649dd32c67..286d5f261a 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/GenericModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/GenericModelBinder.cs @@ -12,20 +12,24 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { public class GenericModelBinder : IModelBinder { - public async Task BindModelAsync(ModelBindingContext bindingContext) + public async Task BindModelAsync(ModelBindingContext bindingContext) { var binderType = ResolveBinderType(bindingContext.ModelType); if (binderType != null) { var binder = (IModelBinder)Activator.CreateInstance(binderType); - await binder.BindModelAsync(bindingContext); + var result = await binder.BindModelAsync(bindingContext); + + var modelBindingResult = result != null ? + new ModelBindingResult(result.Model, result.Key, result.IsModelSet) : + new ModelBindingResult(null, bindingContext.ModelName, false); // Was able to resolve a binder type, hence we should tell the model binding system to return // true so that none of the other model binders participate. - return true; + return modelBindingResult; } - return false; + return null; } private static Type ResolveBinderType(Type modelType) diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/HeaderModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/HeaderModelBinder.cs index 89c0192458..c8fa10028b 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/HeaderModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/HeaderModelBinder.cs @@ -23,19 +23,20 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } /// - protected override Task BindModelCoreAsync([NotNull] ModelBindingContext bindingContext) + protected override Task BindModelCoreAsync([NotNull] ModelBindingContext bindingContext) { var request = bindingContext.OperationBindingContext.HttpContext.Request; var modelMetadata = bindingContext.ModelMetadata; // Property name can be null if the model metadata represents a type (rahter than a property or parameter). var headerName = modelMetadata.BinderModelName ?? modelMetadata.PropertyName ?? bindingContext.ModelName; + object model = null; if (bindingContext.ModelType == typeof(string)) { var value = request.Headers.Get(headerName); if (value != null) { - bindingContext.Model = value; + model = value; } } else if (typeof(IEnumerable).GetTypeInfo().IsAssignableFrom( @@ -44,14 +45,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var values = request.Headers.GetCommaSeparatedValues(headerName); if (values != null) { - bindingContext.Model = ModelBindingHelper.ConvertValuesToCollectionType( + model = ModelBindingHelper.ConvertValuesToCollectionType( bindingContext.ModelType, values); } } - // Always return true as header model binder is supposed to always handle IHeaderBinderMetadata. - return Task.FromResult(true); + return Task.FromResult(new ModelBindingResult(model, bindingContext.ModelName, model != null)); } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/IModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/IModelBinder.cs index 538110dc17..e8724068e8 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/IModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/IModelBinder.cs @@ -14,7 +14,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// Async function to bind to a particular model. /// /// The binding context which has the object to be bound. - /// A Task with a bool implying the success or failure of the operation. - Task BindModelAsync(ModelBindingContext bindingContext); + /// A Task which on completion returns a which represents the result + /// of the model binding process. + /// + /// + /// A null return value means that this model binder was not able to handle the request. + /// Returning null ensures that subsequent model binders are run. If a non null value indicates + /// that the model binder was able to handle the request. + /// + Task BindModelAsync(ModelBindingContext bindingContext); } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/KeyValuePairModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/KeyValuePairModelBinder.cs index e04a17d282..4a75fbd432 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/KeyValuePairModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/KeyValuePairModelBinder.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using Microsoft.AspNet.Mvc.ModelBinding.Internal; @@ -9,23 +10,39 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { public sealed class KeyValuePairModelBinder : IModelBinder { - public async Task BindModelAsync(ModelBindingContext bindingContext) + public async Task BindModelAsync(ModelBindingContext bindingContext) { ModelBindingHelper.ValidateBindingContext(bindingContext, typeof(KeyValuePair), allowNullModel: true); - var keyResult = await TryBindStrongModel(bindingContext, "key"); - var valueResult = await TryBindStrongModel(bindingContext, "value"); - - if (keyResult.Success && valueResult.Success) + var keyResult = await TryBindStrongModel(bindingContext, "Key"); + var valueResult = await TryBindStrongModel(bindingContext, "Value"); + var model = bindingContext.ModelMetadata.Model; + var isModelSet = false; + if (keyResult.IsModelSet && valueResult.IsModelSet) { - bindingContext.Model = new KeyValuePair(keyResult.Model, valueResult.Model); + model = new KeyValuePair( + ModelBindingHelper.CastOrDefault(keyResult.Model), + ModelBindingHelper.CastOrDefault(valueResult.Model)); + isModelSet = true; } - return keyResult.Success || valueResult.Success; + else if (!keyResult.IsModelSet && valueResult.IsModelSet) + { + bindingContext.ModelState.TryAddModelError(keyResult.Key, + Resources.KeyValuePair_BothKeyAndValueMustBePresent); + } + else if (keyResult.IsModelSet && !valueResult.IsModelSet) + { + bindingContext.ModelState.TryAddModelError(valueResult.Key, + Resources.KeyValuePair_BothKeyAndValueMustBePresent); + } + + var result = new ModelBindingResult(model, bindingContext.ModelName, isModelSet); + return (keyResult.IsModelSet || valueResult.IsModelSet) ? result : null; } - internal async Task> TryBindStrongModel(ModelBindingContext parentBindingContext, + internal async Task TryBindStrongModel(ModelBindingContext parentBindingContext, string propertyName) { var propertyModelMetadata = @@ -35,29 +52,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding ModelBindingHelper.CreatePropertyModelName(parentBindingContext.ModelName, propertyName); var propertyBindingContext = new ModelBindingContext(parentBindingContext, propertyModelName, propertyModelMetadata); - - if (await propertyBindingContext.OperationBindingContext.ModelBinder.BindModelAsync(propertyBindingContext)) + var modelBindingResult = + await propertyBindingContext.OperationBindingContext.ModelBinder.BindModelAsync(propertyBindingContext); + if (modelBindingResult != null) { - var untypedModel = propertyBindingContext.Model; - var model = ModelBindingHelper.CastOrDefault(untypedModel); - parentBindingContext.ValidationNode.ChildNodes.Add(propertyBindingContext.ValidationNode); - return new BindResult(success: true, model: model); + return modelBindingResult; } - return new BindResult(success: false, model: default(TModel)); - } - - internal sealed class BindResult - { - public BindResult(bool success, TModel model) - { - Success = success; - Model = model; - } - - public bool Success { get; private set; } - - public TModel Model { get; private set; } + return new ModelBindingResult(model: default(TModel), key: propertyModelName, isModelSet: false); } } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs index 7c6a4458d8..8b98d5b1ca 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs @@ -14,12 +14,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { public class MutableObjectModelBinder : IModelBinder { - public virtual async Task BindModelAsync(ModelBindingContext bindingContext) + public virtual async Task BindModelAsync(ModelBindingContext bindingContext) { ModelBindingHelper.ValidateBindingContext(bindingContext); if (!CanBindType(bindingContext.ModelType)) { - return false; + return null; } var mutableObjectBinderContext = new MutableObjectBinderContext() @@ -30,17 +30,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding if (!(await CanCreateModel(mutableObjectBinderContext))) { - return false; + return null; } EnsureModel(bindingContext); - var dto = await CreateAndPopulateDto(bindingContext, mutableObjectBinderContext.PropertyMetadata); + var result = await CreateAndPopulateDto(bindingContext, mutableObjectBinderContext.PropertyMetadata); // post-processing, e.g. property setters and hooking up validation - ProcessDto(bindingContext, dto); - // complex models require full validation - bindingContext.ValidationNode.ValidateAllProperties = true; - return true; + ProcessDto(bindingContext, (ComplexModelDto)result.Model); + return + new ModelBindingResult(bindingContext.ModelMetadata.Model, bindingContext.ModelName, isModelSet: true); } protected virtual bool CanUpdateProperty(ModelMetadata propertyMetadata) @@ -236,7 +235,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return true; } - private async Task CreateAndPopulateDto(ModelBindingContext bindingContext, + private async Task CreateAndPopulateDto(ModelBindingContext bindingContext, IEnumerable propertyMetadatas) { // create a DTO and call into the DTO binder @@ -247,8 +246,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var dtoBindingContext = new ModelBindingContext(bindingContext, bindingContext.ModelName, complexModelDtoMetadata); - await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(dtoBindingContext); - return (ComplexModelDto)dtoBindingContext.Model; + return await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(dtoBindingContext); } protected virtual object CreateModel(ModelBindingContext bindingContext) @@ -258,36 +256,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return Activator.CreateInstance(bindingContext.ModelType); } - // Called when the property setter null check failed, allows us to add our own error message to ModelState. - internal static EventHandler CreateNullCheckFailedHandler(ModelMetadata modelMetadata, - object incomingValue) - { - return (sender, e) => - { - var validationNode = (ModelValidationNode)sender; - var modelState = e.ValidationContext.ModelState; - var validationState = modelState.GetFieldValidationState(validationNode.ModelStateKey); - - if (validationState == ModelValidationState.Unvalidated) - { - // TODO: https://github.com/aspnet/Mvc/issues/450 Revive ModelBinderConfig - // var errorMessage = ModelBinderConfig.ValueRequiredErrorMessageProvider(e.ValidationContext, - // modelMetadata, - // incomingValue); - var errorMessage = Resources.ModelBinderConfig_ValueRequired; - if (errorMessage != null) - { - modelState.TryAddModelError(validationNode.ModelStateKey, errorMessage); - } - } - }; - } - protected virtual void EnsureModel(ModelBindingContext bindingContext) { - if (bindingContext.Model == null) + if (bindingContext.ModelMetadata.Model == null) { - bindingContext.Model = CreateModel(bindingContext); + bindingContext.ModelMetadata.Model = CreateModel(bindingContext); } } @@ -378,14 +351,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var validationInfo = GetPropertyValidationInfo(bindingContext); // Eliminate provided properties from requiredProperties; leaving just *missing* required properties. - var boundProperties = dto.Results.Where(p => p.Value.IsModelBound).Select(p => p.Key.PropertyName); + var boundProperties = dto.Results.Where(p => p.Value.IsModelSet).Select(p => p.Key.PropertyName); validationInfo.RequiredProperties.ExceptWith(boundProperties); foreach (var missingRequiredProperty in validationInfo.RequiredProperties) { var addedError = false; var modelStateKey = ModelBindingHelper.CreatePropertyModelName( - bindingContext.ValidationNode.ModelStateKey, missingRequiredProperty); + bindingContext.ModelName, missingRequiredProperty); // Update Model as SetProperty() would: Place null value where validator will check for non-null. This // ensures a failure result from a required validator (if any) even for a non-nullable property. @@ -421,14 +394,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding validationInfo.RequiredValidators.TryGetValue(propertyMetadata.PropertyName, out requiredValidator); SetProperty(bindingContext, propertyMetadata, dtoResult, requiredValidator); - bindingContext.ValidationNode.ChildNodes.Add(dtoResult.ValidationNode); } } } protected virtual void SetProperty(ModelBindingContext bindingContext, ModelMetadata propertyMetadata, - ComplexModelDtoResult dtoResult, + ModelBindingResult dtoResult, IModelValidator requiredValidator) { var bindingFlags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.IgnoreCase; @@ -443,7 +415,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding object value; var hasDefaultValue = false; - if (dtoResult.IsModelBound) + if (dtoResult.IsModelSet) { value = dtoResult.Model; } @@ -458,7 +430,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // the property setters throw, e.g. if we're setting entity keys to null. if (value == null) { - var modelStateKey = dtoResult.ValidationNode.ModelStateKey; + var modelStateKey = dtoResult.Key; var validationState = bindingContext.ModelState.GetFieldValidationState(modelStateKey); if (validationState == ModelValidationState.Unvalidated) { @@ -473,7 +445,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } } - if (!dtoResult.IsModelBound && !hasDefaultValue) + if (!dtoResult.IsModelSet && !hasDefaultValue) { // If we don't have a value, don't set it on the model and trounce a pre-initialized // value. @@ -484,7 +456,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { try { - property.SetValue(bindingContext.Model, value); + property.SetValue(bindingContext.ModelMetadata.Model, value); } catch (Exception ex) { @@ -495,7 +467,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { ex = targetInvocationException.InnerException; } - var modelStateKey = dtoResult.ValidationNode.ModelStateKey; + var modelStateKey = dtoResult.Key; var validationState = bindingContext.ModelState.GetFieldValidationState(modelStateKey); if (validationState == ModelValidationState.Unvalidated) { @@ -506,11 +478,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding else { // trying to set a non-nullable value type to null, need to make sure there's a message - var modelStateKey = dtoResult.ValidationNode.ModelStateKey; + var modelStateKey = dtoResult.Key; var validationState = bindingContext.ModelState.GetFieldValidationState(modelStateKey); if (validationState == ModelValidationState.Unvalidated) { - dtoResult.ValidationNode.Validated += CreateNullCheckFailedHandler(propertyMetadata, value); + var errorMessage = Resources.ModelBinderConfig_ValueRequired; + bindingContext.ModelState.TryAddModelError(modelStateKey, errorMessage); } } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeConverterModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeConverterModelBinder.cs index 8a02d22683..bc9090aa33 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeConverterModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeConverterModelBinder.cs @@ -9,20 +9,20 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { public sealed class TypeConverterModelBinder : IModelBinder { - public async Task BindModelAsync(ModelBindingContext bindingContext) + public async Task BindModelAsync(ModelBindingContext bindingContext) { ModelBindingHelper.ValidateBindingContext(bindingContext); if (!TypeHelper.HasStringConverter(bindingContext.ModelType)) { // this type cannot be converted - return false; + return null; } var valueProviderResult = await bindingContext.ValueProvider.GetValueAsync(bindingContext.ModelName); if (valueProviderResult == null) { - return false; // no entry + return null; // no entry } object newModel; @@ -31,14 +31,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { newModel = valueProviderResult.ConvertTo(bindingContext.ModelType); ModelBindingHelper.ReplaceEmptyStringWithNull(bindingContext.ModelMetadata, ref newModel); - bindingContext.Model = newModel; + return new ModelBindingResult(newModel, bindingContext.ModelName, true); } catch (Exception ex) { bindingContext.ModelState.TryAddModelError(bindingContext.ModelName, ex); } - return true; + return new ModelBindingResult(null, bindingContext.ModelName, false); } } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeMatchModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeMatchModelBinder.cs index b23b1897f5..9c7085054f 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeMatchModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/TypeMatchModelBinder.cs @@ -8,21 +8,19 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { public sealed class TypeMatchModelBinder : IModelBinder { - public async Task BindModelAsync(ModelBindingContext bindingContext) + public async Task BindModelAsync(ModelBindingContext bindingContext) { var valueProviderResult = await GetCompatibleValueProviderResult(bindingContext); if (valueProviderResult == null) { // conversion would have failed - return false; + return null; } bindingContext.ModelState.SetModelValue(bindingContext.ModelName, valueProviderResult); var model = valueProviderResult.RawValue; ModelBindingHelper.ReplaceEmptyStringWithNull(bindingContext.ModelMetadata, ref model); - bindingContext.Model = model; - - return true; + return new ModelBindingResult(model, bindingContext.ModelName, true); } internal static async Task GetCompatibleValueProviderResult(ModelBindingContext context) diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/IObjectModelValidator.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/IObjectModelValidator.cs new file mode 100644 index 0000000000..f1f593b2aa --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/IObjectModelValidator.cs @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNet.Mvc.ModelBinding +{ + /// + /// Provides methods to validate an object graph. + /// + public interface IObjectModelValidator + { + /// + /// Validates the given model in . + /// + /// The associated with the current call. + /// + void Validate(ModelValidationContext validationContext); + } +} diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/CollectionModelBinderUtil.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/CollectionModelBinderUtil.cs index dff0d1a4e6..a95a874c51 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/CollectionModelBinderUtil.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/CollectionModelBinderUtil.cs @@ -19,25 +19,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Internal indexNames = indexes; } } + return indexNames; } - - public static void CreateOrReplaceCollection(ModelBindingContext bindingContext, - IEnumerable incomingElements, - Func> creator) - { - var collection = bindingContext.Model as ICollection; - if (collection == null || collection.IsReadOnly) - { - collection = creator(); - bindingContext.Model = collection; - } - - collection.Clear(); - foreach (var element in incomingElements) - { - collection.Add(element); - } - } } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/ModelBindingHelper.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/ModelBindingHelper.cs index 37ea739747..60236648a4 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/ModelBindingHelper.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Internal/ModelBindingHelper.cs @@ -80,17 +80,17 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Internal throw new ArgumentException(message, "bindingContext"); } - if (!allowNullModel && bindingContext.Model == null) + if (!allowNullModel && bindingContext.ModelMetadata.Model == null) { var message = Resources.FormatModelBinderUtil_ModelCannotBeNull(requiredType); throw new ArgumentException(message, "bindingContext"); } - if (bindingContext.Model != null && + if (bindingContext.ModelMetadata.Model != null && !bindingContext.ModelType.GetTypeInfo().IsAssignableFrom(requiredType.GetTypeInfo())) { var message = Resources.FormatModelBinderUtil_ModelInstanceIsWrong( - bindingContext.Model.GetType(), + bindingContext.ModelMetadata.Model.GetType(), requiredType); throw new ArgumentException(message, "bindingContext"); } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingContext.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingContext.cs index 98799eb181..8bb3505b60 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingContext.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingContext.cs @@ -17,7 +17,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding private string _modelName; private ModelStateDictionary _modelState; - private ModelValidationNode _validationNode; private Func _propertyFilter; /// @@ -55,36 +54,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// public OperationBindingContext OperationBindingContext { get; set; } - /// - /// Gets or sets the model associated with this context. - /// - /// - /// The property must be set to access this property. - /// - public object Model - { - get - { - EnsureModelMetadata(); - return ModelMetadata.Model; - } - set - { - IsModelSet = true; - - EnsureModelMetadata(); - ModelMetadata.Model = value; - } - } - - /// - /// Gets or sets a value indicating whether or not the value has been set. - /// - /// This property can be used to distinguish between a model binder which does not find a value and - /// the case where a model binder sets the null value. - /// - public bool IsModelSet { get; set; } - /// /// Gets or sets the metadata for the model associated with this context. /// @@ -164,23 +133,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding set { _propertyFilter = value; } } - /// - /// Gets or sets the instance used as a container for - /// validation information. - /// - public ModelValidationNode ValidationNode - { - get - { - if (_validationNode == null) - { - _validationNode = new ModelValidationNode(ModelMetadata, ModelName); - } - return _validationNode; - } - set { _validationNode = value; } - } - private void EnsureModelMetadata() { if (ModelMetadata == null) diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingResult.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingResult.cs new file mode 100644 index 0000000000..afb14eb06d --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingResult.cs @@ -0,0 +1,45 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNet.Mvc.ModelBinding +{ + /// + /// Contains the result of model binding. + /// + public class ModelBindingResult + { + /// + /// 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 + /// . + public ModelBindingResult(object model, string key, bool isModelSet) + { + Model = model; + Key = key; + IsModelSet = isModelSet; + } + + /// + /// Gets or sets the model associated with this context. + /// + public object Model { get; } + + /// + /// Gets or sets the model name which was used to bind the model. + /// + /// This property can be used during validation to add model state for a bound model. + /// + public string Key { get; } + + /// + /// Gets or sets a value indicating whether or not the value has been set. + /// + /// This property can be used to distinguish between a model binder which does not find a value and + /// the case where a model binder sets the null value. + /// + public bool IsModelSet { get; } + } +} diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs index 90530144ab..9abd1515bf 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelStateDictionary.cs @@ -132,10 +132,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding get { return _innerDictionary.Values; } } - /// + /// + /// Gets a value that indicates whether any model state values in this model state dictionary is invalid or not validated. + /// public bool IsValid { - get { return ValidationState == ModelValidationState.Valid; } + get + { + return ValidationState == ModelValidationState.Valid || ValidationState == ModelValidationState.Skipped; + } } /// @@ -284,6 +289,24 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return GetValidity(entries); } + /// + /// Returns for the . + /// + /// The key to look up model state errors for. + /// Returns if no entry is found for the specified + /// key, if an instance is found with one or more model + /// state errors; otherwise. + public ModelValidationState GetValidationState([NotNull] string key) + { + ModelState validationState; + if (TryGetValue(key, out validationState)) + { + return validationState.ValidationState; + } + + return ModelValidationState.Unvalidated; + } + /// /// Marks the for the entry with the specified /// as . @@ -300,6 +323,22 @@ namespace Microsoft.AspNet.Mvc.ModelBinding modelState.ValidationState = ModelValidationState.Valid; } + /// + /// Marks the for the entry with the specified + /// as . + /// + /// The key of the to mark as skipped. + public void MarkFieldSkipped([NotNull] string key) + { + var modelState = GetModelStateForKey(key); + if (modelState.ValidationState == ModelValidationState.Invalid) + { + throw new InvalidOperationException(Resources.Validation_InvalidFieldCannotBeReset_ToSkipped); + } + + modelState.ValidationState = ModelValidationState.Skipped; + } + /// /// Copies the values from the specified into this instance, overwriting /// existing values if keys are the same. diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelValidationState.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelValidationState.cs index e8bd13a964..93cdfc021e 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelValidationState.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelValidationState.cs @@ -8,5 +8,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding Unvalidated, Invalid, Valid, + Skipped } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Properties/Resources.Designer.cs index ccb8a02331..b4bd26fbbb 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Properties/Resources.Designer.cs @@ -42,6 +42,22 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return GetString("ArgumentCannotBeNullOrEmpty"); } + /// + /// A value is required. + /// + internal static string KeyValuePair_BothKeyAndValueMustBePresent + { + get { return GetString("KeyValuePair_BothKeyAndValueMustBePresent"); } + } + + /// + /// A value is required. + /// + internal static string FormatKeyValuePair_BothKeyAndValueMustBePresent() + { + return GetString("KeyValuePair_BothKeyAndValueMustBePresent"); + } + /// /// The property {0}.{1} could not be found. /// @@ -330,6 +346,22 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return GetString("Validation_InvalidFieldCannotBeReset"); } + /// + /// A field previously marked invalid should not be marked skipped. + /// + internal static string Validation_InvalidFieldCannotBeReset_ToSkipped + { + get { return GetString("Validation_InvalidFieldCannotBeReset_ToSkipped"); } + } + + /// + /// A field previously marked invalid should not be marked skipped. + /// + internal static string FormatValidation_InvalidFieldCannotBeReset_ToSkipped() + { + return GetString("Validation_InvalidFieldCannotBeReset_ToSkipped"); + } + /// /// A value is required but was not present in the request. /// diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Resources.resx b/src/Microsoft.AspNet.Mvc.ModelBinding/Resources.resx index fa74ff2293..d1104cd28e 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Resources.resx @@ -123,6 +123,9 @@ Value cannot be null or empty. + + A value is required. + The property {0}.{1} could not be found. @@ -177,6 +180,9 @@ A field previously marked invalid should not be marked valid. + + A field previously marked invalid should not be marked skipped. + A value is required but was not present in the request. diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/SimpleTypesExcludeFilter.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/SimpleTypesExcludeFilter.cs index 85316acd77..9d86aa5d19 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/SimpleTypesExcludeFilter.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/SimpleTypesExcludeFilter.cs @@ -40,7 +40,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding foreach (var actualType in actualTypes) { var underlyingType = Nullable.GetUnderlyingType(actualType) ?? actualType; - if (!IsSimpleType(underlyingType)) { return false; diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/DefaultBodyModelValidator.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/DefaultBodyModelValidator.cs deleted file mode 100644 index c42550e7d8..0000000000 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/DefaultBodyModelValidator.cs +++ /dev/null @@ -1,264 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Collections; -using System.Collections.Generic; -using System.Diagnostics; -using System.Linq; -using System.Runtime.CompilerServices; -using Microsoft.AspNet.Mvc.ModelBinding.Internal; - -namespace Microsoft.AspNet.Mvc.ModelBinding -{ - /// - /// Recursively validate an object. - /// - public class DefaultBodyModelValidator : IBodyModelValidator - { - /// - public bool Validate( - [NotNull] ModelValidationContext modelValidationContext, - string keyPrefix) - { - var metadata = modelValidationContext.ModelMetadata; - var validationContext = new ValidationContext() - { - ModelValidationContext = modelValidationContext, - Visited = new HashSet(ReferenceEqualityComparer.Instance), - KeyBuilders = new Stack(), - RootPrefix = keyPrefix - }; - - return ValidateNonVisitedNodeAndChildren(metadata, validationContext, validators: null); - } - - private bool ValidateNonVisitedNodeAndChildren( - ModelMetadata metadata, ValidationContext validationContext, IEnumerable validators) - { - // Recursion guard to avoid stack overflows - RuntimeHelpers.EnsureSufficientExecutionStack(); - - var isValid = true; - if (validators == null) - { - // 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. - validators = validationContext.ModelValidationContext.ValidatorProvider.GetValidators(metadata); - } - - // We don't need to recursively traverse the graph for null values - if (metadata.Model == null) - { - return ShallowValidate(metadata, validationContext, validators); - } - - // We don't need to recursively traverse the graph for types that shouldn't be validated - var modelType = metadata.Model.GetType(); - if (IsTypeExcludedFromValidation( - validationContext.ModelValidationContext.ExcludeFromValidationFilters, - modelType)) - { - return ShallowValidate(metadata, validationContext, validators); - } - - // Check to avoid infinite recursion. This can happen with cycles in an object graph. - if (validationContext.Visited.Contains(metadata.Model)) - { - return true; - } - - validationContext.Visited.Add(metadata.Model); - - // Validate the children first - depth-first traversal - var enumerableModel = metadata.Model as IEnumerable; - if (enumerableModel == null) - { - isValid = ValidateProperties(metadata, validationContext); - } - else - { - isValid = ValidateElements(enumerableModel, validationContext); - } - - if (isValid) - { - // Don't bother to validate this node if children failed. - isValid = ShallowValidate(metadata, validationContext, validators); - } - - // Pop the object so that it can be validated again in a different path - validationContext.Visited.Remove(metadata.Model); - - return isValid; - } - - private bool ValidateProperties(ModelMetadata metadata, ValidationContext validationContext) - { - var isValid = true; - var propertyScope = new PropertyScope(); - validationContext.KeyBuilders.Push(propertyScope); - foreach (var childMetadata in - validationContext.ModelValidationContext.MetadataProvider.GetMetadataForProperties( - metadata.Model, metadata.RealModelType)) - { - propertyScope.PropertyName = childMetadata.PropertyName; - if (!ValidateNonVisitedNodeAndChildren(childMetadata, validationContext, validators: null)) - { - isValid = false; - } - } - - validationContext.KeyBuilders.Pop(); - return isValid; - } - - private bool ValidateElements(IEnumerable model, ValidationContext validationContext) - { - var isValid = true; - var elementType = GetElementType(model.GetType()); - var elementMetadata = - validationContext.ModelValidationContext.MetadataProvider.GetMetadataForType( - modelAccessor: null, modelType: elementType); - - var elementScope = new ElementScope() { Index = 0 }; - validationContext.KeyBuilders.Push(elementScope); - var validators = validationContext.ModelValidationContext.ValidatorProvider.GetValidators(elementMetadata); - - // 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(); - - 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) - { - elementMetadata.Model = element; - - if (!ValidateNonVisitedNodeAndChildren(elementMetadata, validationContext, validators)) - { - isValid = false; - } - } - - elementScope.Index++; - } - - validationContext.KeyBuilders.Pop(); - return isValid; - } - - // Validates a single node (not including children) - // Returns true if validation passes successfully - private static bool ShallowValidate( - ModelMetadata metadata, - ValidationContext validationContext, - [NotNull] IEnumerable validators) - { - var isValid = true; - string modelKey = null; - - // When the are no validators we bail quickly. This saves a GetEnumerator allocation. - // In a large array (tens of thousands or more) scenario it's very significant. - var validatorsAsCollection = validators as ICollection; - if (validatorsAsCollection != null && validatorsAsCollection.Count == 0) - { - return isValid; - } - - var modelValidationContext = - new ModelValidationContext(validationContext.ModelValidationContext, metadata); - foreach (var validator in validators) - { - foreach (var error in validator.Validate(modelValidationContext)) - { - if (modelKey == null) - { - modelKey = validationContext.RootPrefix; - // This constructs the object heirarchy - // Example: prefix.Parent.Child - foreach (var keyBuilder in validationContext.KeyBuilders.Reverse()) - { - modelKey = keyBuilder.AppendTo(modelKey); - } - } - - var errorKey = ModelBindingHelper.CreatePropertyModelName(modelKey, error.MemberName); - validationContext.ModelValidationContext.ModelState.AddModelError(errorKey, error.Message); - isValid = false; - } - } - - return isValid; - } - - private bool IsTypeExcludedFromValidation( - IReadOnlyList filters, Type type) - { - // This can be set to null in ModelBinding scenarios which does not flow through this path. - if (filters == null) - { - return false; - } - - return filters.Any(filter => filter.IsTypeExcluded(type)); - } - - private static Type GetElementType(Type type) - { - Debug.Assert(typeof(IEnumerable).IsAssignableFrom(type)); - if (type.IsArray) - { - return type.GetElementType(); - } - - foreach (var implementedInterface in type.GetInterfaces()) - { - if (implementedInterface.IsGenericType() && - implementedInterface.GetGenericTypeDefinition() == typeof(IEnumerable<>)) - { - return implementedInterface.GetGenericArguments()[0]; - } - } - - return typeof(object); - } - - private interface IKeyBuilder - { - string AppendTo(string prefix); - } - - private class PropertyScope : IKeyBuilder - { - public string PropertyName { get; set; } - - public string AppendTo(string prefix) - { - return ModelBindingHelper.CreatePropertyModelName(prefix, PropertyName); - } - } - - private class ElementScope : IKeyBuilder - { - public int Index { get; set; } - - public string AppendTo(string prefix) - { - return ModelBindingHelper.CreateIndexModelName(prefix, Index); - } - } - - private class ValidationContext - { - public ModelValidationContext ModelValidationContext { get; set; } - public HashSet Visited { get; set; } - public Stack KeyBuilders { get; set; } - public string RootPrefix { get; set; } - } - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/DefaultObjectValidator.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/DefaultObjectValidator.cs new file mode 100644 index 0000000000..d7349d95f2 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/DefaultObjectValidator.cs @@ -0,0 +1,299 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Runtime.CompilerServices; +using Microsoft.AspNet.Mvc.ModelBinding.Internal; + +namespace Microsoft.AspNet.Mvc.ModelBinding +{ + /// + /// Recursively validate an object. + /// + public class DefaultObjectValidator : IObjectModelValidator + { + private readonly IValidationExcludeFiltersProvider _excludeFilterProvider; + private readonly IModelMetadataProvider _modelMetadataProvider; + + public DefaultObjectValidator( + IValidationExcludeFiltersProvider excludeFilterProvider, + IModelMetadataProvider modelMetadataProvider) + { + _excludeFilterProvider = excludeFilterProvider; + _modelMetadataProvider = modelMetadataProvider; + } + + /// + public void Validate([NotNull] ModelValidationContext modelValidationContext) + { + var metadata = modelValidationContext.ModelMetadata; + var validationContext = new ValidationContext() + { + ModelValidationContext = modelValidationContext, + Visited = new HashSet(ReferenceEqualityComparer.Instance), + }; + + ValidateNonVisitedNodeAndChildren( + modelValidationContext.RootPrefix, metadata, validationContext, validators: null); + } + + private bool ValidateNonVisitedNodeAndChildren(string modelKey, + ModelMetadata metadata, ValidationContext validationContext, IEnumerable validators) + { + // Recursion guard to avoid stack overflows + RuntimeHelpers.EnsureSufficientExecutionStack(); + + var modelState = validationContext.ModelValidationContext.ModelState; + var bindingSourceMetadata = metadata.BinderMetadata as IBindingSourceMetadata; + var bindingSource = bindingSourceMetadata?.BindingSource; + + if (bindingSource != null && !bindingSource.IsFromRequest) + { + // 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. + var validationState = modelState.GetFieldValidationState(modelKey); + if (validationState == ModelValidationState.Unvalidated) + { + validationContext.ModelValidationContext.ModelState.MarkFieldSkipped(modelKey); + } + + // For validation purposes this model is valid. + return true; + } + + if (modelState.HasReachedMaxErrors) + { + // Short circuit if max errors have been recorded. In which case we treat this as invalid. + return false; + } + + var isValid = true; + if (validators == null) + { + // 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. + validators = validationContext.ModelValidationContext.ValidatorProvider.GetValidators(metadata); + } + + // We don't need to recursively traverse the graph for null values + if (metadata.Model == null) + { + return ShallowValidate(modelKey, metadata, validationContext, validators); + } + + // We don't need to recursively traverse the graph for types that shouldn't be validated + var modelType = metadata.Model.GetType(); + if (IsTypeExcludedFromValidation(_excludeFilterProvider.ExcludeFilters, modelType)) + { + var result = ShallowValidate(modelKey, metadata, validationContext, validators); + MarkPropertiesAsSkipped(modelKey, metadata, validationContext); + return result; + } + + // Check to avoid infinite recursion. This can happen with cycles in an object graph. + if (validationContext.Visited.Contains(metadata.Model)) + { + return true; + } + + validationContext.Visited.Add(metadata.Model); + + // Validate the children first - depth-first traversal + var enumerableModel = metadata.Model as IEnumerable; + if (enumerableModel == null) + { + isValid = ValidateProperties(modelKey, metadata, validationContext); + } + else + { + isValid = ValidateElements(modelKey, enumerableModel, validationContext); + } + + if (isValid) + { + // Don't bother to validate this node if children failed. + isValid = ShallowValidate(modelKey, metadata, validationContext, validators); + } + + // Pop the object so that it can be validated again in a different path + validationContext.Visited.Remove(metadata.Model); + return isValid; + } + + private void MarkPropertiesAsSkipped(string currentModelKey, ModelMetadata metadata, ValidationContext validationContext) + { + var modelState = validationContext.ModelValidationContext.ModelState; + var fieldValidationState = modelState.GetFieldValidationState(currentModelKey); + + // Since shallow validation is done, if the modelvalidation state is still marked as unvalidated, + // it is because some properties in the subtree are marked as unvalidated. Mark all such properties + // as skipped. Models which have their subtrees as Valid or Invalid do not need to be marked as skipped. + if (fieldValidationState != ModelValidationState.Unvalidated) + { + return; + } + + foreach (var childMetadata in metadata.Properties) + { + var childKey = ModelBindingHelper.CreatePropertyModelName(currentModelKey, childMetadata.PropertyName); + var validationState = modelState.GetFieldValidationState(childKey); + + if (validationState == ModelValidationState.Unvalidated) + { + validationContext.ModelValidationContext.ModelState.MarkFieldSkipped(childKey); + } + } + } + + private bool ValidateProperties(string currentModelKey, ModelMetadata metadata, ValidationContext validationContext) + { + var isValid = true; + + foreach (var childMetadata in metadata.Properties) + { + var childKey = ModelBindingHelper.CreatePropertyModelName(currentModelKey, childMetadata.PropertyName); + if (!ValidateNonVisitedNodeAndChildren(childKey, childMetadata, validationContext, validators: null)) + { + isValid = false; + } + } + + return isValid; + } + + private bool ValidateElements(string currentKey, IEnumerable model, ValidationContext validationContext) + { + var elementType = GetElementType(model.GetType()); + var elementMetadata = _modelMetadataProvider.GetMetadataForType( + modelAccessor: null, + modelType: elementType); + + var validators = validationContext.ModelValidationContext.ValidatorProvider.GetValidators(elementMetadata); + + // 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) + { + elementMetadata.Model = element; + var elementKey = ModelBindingHelper.CreateIndexModelName(currentKey, index); + if (!ValidateNonVisitedNodeAndChildren(elementKey, elementMetadata, validationContext, validators)) + { + isValid = false; + } + } + + index++; + } + + return isValid; + } + + // Validates a single node (not including children) + // Returns true if validation passes successfully + private static bool ShallowValidate( + string modelKey, + ModelMetadata metadata, + ValidationContext validationContext, + IEnumerable validators) + { + var isValid = true; + + // When the are no validators we bail quickly. This saves a GetEnumerator allocation. + // In a large array (tens of thousands or more) scenario it's very significant. + var validatorsAsCollection = validators as ICollection; + if (validatorsAsCollection == null || validatorsAsCollection.Count > 0) + { + var modelValidationContext = + new ModelValidationContext(validationContext.ModelValidationContext, metadata); + var modelState = validationContext.ModelValidationContext.ModelState; + var modelValidationState = modelState.GetValidationState(modelKey); + var fieldValidationState = modelState.GetFieldValidationState(modelKey); + + // If either the model or its properties are unvalidated, validate them now. + if (modelValidationState == ModelValidationState.Unvalidated || + fieldValidationState == ModelValidationState.Unvalidated) + { + foreach (var validator in validators) + { + foreach (var error in validator.Validate(modelValidationContext)) + { + var errorKey = ModelBindingHelper.CreatePropertyModelName(modelKey, error.MemberName); + if (!modelState.TryAddModelError(errorKey, error.Message) && + 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. + modelState.MarkFieldSkipped(errorKey); + } + + isValid = false; + } + } + } + else if (fieldValidationState == ModelValidationState.Invalid) + { + isValid = false; + } + } + + if (isValid) + { + validationContext.ModelValidationContext.ModelState.MarkFieldValid(modelKey); + } + + return isValid; + } + + private bool IsTypeExcludedFromValidation(IReadOnlyList filters, Type type) + { + // This can be set to null in ModelBinding scenarios which does not flow through this path. + if (filters == null) + { + return false; + } + + return filters.Any(filter => filter.IsTypeExcluded(type)); + } + + private static Type GetElementType(Type type) + { + Debug.Assert(typeof(IEnumerable).IsAssignableFrom(type)); + if (type.IsArray) + { + return type.GetElementType(); + } + + foreach (var implementedInterface in type.GetInterfaces()) + { + if (implementedInterface.IsGenericType() && + implementedInterface.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + { + return implementedInterface.GetGenericArguments()[0]; + } + } + + return typeof(object); + } + + private class ValidationContext + { + public ModelValidationContext ModelValidationContext { get; set; } + + public HashSet Visited { get; set; } + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/IBodyModelValidator.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/IBodyModelValidator.cs deleted file mode 100644 index 398f399219..0000000000 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/IBodyModelValidator.cs +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -namespace Microsoft.AspNet.Mvc.ModelBinding -{ - /// - /// Validates the body parameter of an action after the parameter - /// has been read by the Input Formatters. - /// - public interface IBodyModelValidator - { - /// - /// Determines whether the Model is valid - /// and adds any validation errors to the - /// - /// The validation context which contains the model, metadata - /// and the validator providers. - /// The to append to the key for any validation errors. - /// trueif the model is valid, false otherwise. - bool Validate(ModelValidationContext modelValidationContext, string keyPrefix); - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/IValidationExcludeFiltersProvider.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/IValidationExcludeFiltersProvider.cs similarity index 100% rename from src/Microsoft.AspNet.Mvc.Core/IValidationExcludeFiltersProvider.cs rename to src/Microsoft.AspNet.Mvc.ModelBinding/Validation/IValidationExcludeFiltersProvider.cs diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidatedEventArgs.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidatedEventArgs.cs deleted file mode 100644 index cddc32b7d8..0000000000 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidatedEventArgs.cs +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; - -namespace Microsoft.AspNet.Mvc.ModelBinding -{ - public sealed class ModelValidatedEventArgs : EventArgs - { - public ModelValidatedEventArgs([NotNull] ModelValidationContext validationContext, - [NotNull] ModelValidationNode parentNode) - { - ValidationContext = validationContext; - ParentNode = parentNode; - } - - public ModelValidationContext ValidationContext { get; private set; } - - public ModelValidationNode ParentNode { get; private set; } - } -} diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidatingEventArgs.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidatingEventArgs.cs deleted file mode 100644 index 59ae077fff..0000000000 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidatingEventArgs.cs +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System.ComponentModel; - -namespace Microsoft.AspNet.Mvc.ModelBinding -{ - public sealed class ModelValidatingEventArgs : CancelEventArgs - { - public ModelValidatingEventArgs([NotNull] ModelValidationContext validationContext, - [NotNull] ModelValidationNode parentNode) - { - ValidationContext = validationContext; - ParentNode = parentNode; - } - - public ModelValidationContext ValidationContext { get; private set; } - - public ModelValidationNode ParentNode { get; private set; } - } -} diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidationContext.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidationContext.cs index cda6ddb4d5..8a5af786bc 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidationContext.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidationContext.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { public ModelValidationContext([NotNull] ModelBindingContext bindingContext, [NotNull] ModelMetadata metadata) - : this(bindingContext.OperationBindingContext.MetadataProvider, + : this(bindingContext.ModelName, bindingContext.OperationBindingContext.ValidatorProvider, bindingContext.ModelState, metadata, @@ -17,33 +17,17 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { } - public ModelValidationContext([NotNull] IModelMetadataProvider metadataProvider, + public ModelValidationContext(string rootPrefix, [NotNull] IModelValidatorProvider validatorProvider, [NotNull] ModelStateDictionary modelState, [NotNull] ModelMetadata metadata, ModelMetadata containerMetadata) - : this(metadataProvider, - validatorProvider, - modelState, - metadata, - containerMetadata, - excludeFromValidationFilters: null) - { - } - - public ModelValidationContext([NotNull] IModelMetadataProvider metadataProvider, - [NotNull] IModelValidatorProvider validatorProvider, - [NotNull] ModelStateDictionary modelState, - [NotNull] ModelMetadata metadata, - ModelMetadata containerMetadata, - IReadOnlyList excludeFromValidationFilters) { ModelMetadata = metadata; ModelState = modelState; - MetadataProvider = metadataProvider; + RootPrefix = rootPrefix; ValidatorProvider = validatorProvider; ContainerMetadata = containerMetadata; - ExcludeFromValidationFilters = excludeFromValidationFilters; } public ModelValidationContext([NotNull] ModelValidationContext parentContext, @@ -52,9 +36,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding ModelMetadata = metadata; ContainerMetadata = parentContext.ModelMetadata; ModelState = parentContext.ModelState; - MetadataProvider = parentContext.MetadataProvider; + RootPrefix = parentContext.RootPrefix; ValidatorProvider = parentContext.ValidatorProvider; - ExcludeFromValidationFilters = parentContext.ExcludeFromValidationFilters; } public ModelMetadata ModelMetadata { get; } @@ -63,10 +46,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public ModelStateDictionary ModelState { get; } - public IModelMetadataProvider MetadataProvider { get; } + public string RootPrefix { get; set; } public IModelValidatorProvider ValidatorProvider { get; } - - public IReadOnlyList ExcludeFromValidationFilters { get; } } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidationNode.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidationNode.cs index 0dfafb3875..9c7500c229 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidationNode.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Validation/ModelValidationNode.cs @@ -8,213 +8,4 @@ using Microsoft.AspNet.Mvc.ModelBinding.Internal; namespace Microsoft.AspNet.Mvc.ModelBinding { - public class ModelValidationNode - { - private readonly List _childNodes; - - public ModelValidationNode(ModelMetadata modelMetadata, string modelStateKey) - : this(modelMetadata, modelStateKey, null) - { - } - - public ModelValidationNode([NotNull] ModelMetadata modelMetadata, - [NotNull] string modelStateKey, - IEnumerable childNodes) - { - ModelMetadata = modelMetadata; - ModelStateKey = modelStateKey; - _childNodes = (childNodes != null) ? childNodes.ToList() : new List(); - } - - public event EventHandler Validated; - - public event EventHandler Validating; - - public ICollection ChildNodes - { - get { return _childNodes; } - } - - public ModelMetadata ModelMetadata { get; private set; } - - public string ModelStateKey { get; private set; } - - public bool ValidateAllProperties { get; set; } - - public bool SuppressValidation { get; set; } - - public void CombineWith(ModelValidationNode otherNode) - { - if (otherNode != null && !otherNode.SuppressValidation) - { - Validated += otherNode.Validated; - Validating += otherNode.Validating; - var otherChildNodes = otherNode._childNodes; - for (var i = 0; i < otherChildNodes.Count; i++) - { - var childNode = otherChildNodes[i]; - _childNodes.Add(childNode); - } - } - } - - private void OnValidated(ModelValidatedEventArgs e) - { - if (Validated != null) - { - Validated(this, e); - } - } - - private void OnValidating(ModelValidatingEventArgs e) - { - if (Validating != null) - { - Validating(this, e); - } - } - - private object TryConvertContainerToMetadataType(ModelValidationNode parentNode) - { - if (parentNode != null) - { - var containerInstance = parentNode.ModelMetadata.Model; - if (containerInstance != null) - { - var expectedContainerType = ModelMetadata.ContainerType; - if (expectedContainerType != null) - { - if (expectedContainerType.IsCompatibleWith(containerInstance)) - { - return containerInstance; - } - } - } - } - - return null; - } - - public void Validate(ModelValidationContext validationContext) - { - Validate(validationContext, parentNode: null); - } - - public void Validate([NotNull] ModelValidationContext validationContext, ModelValidationNode parentNode) - { - if (SuppressValidation || validationContext.ModelState.HasReachedMaxErrors) - { - // Short circuit if validation does not need to be applied or if we've reached the max number of - // validation errors. - return; - } - - // pre-validation steps - var validatingEventArgs = new ModelValidatingEventArgs(validationContext, parentNode); - OnValidating(validatingEventArgs); - if (validatingEventArgs.Cancel) - { - return; - } - - ValidateChildren(validationContext); - ValidateThis(validationContext, parentNode); - - // post-validation steps - var validatedEventArgs = new ModelValidatedEventArgs(validationContext, parentNode); - OnValidated(validatedEventArgs); - - var modelState = validationContext.ModelState; - if (modelState.GetFieldValidationState(ModelStateKey) != ModelValidationState.Invalid) - { - // If a node or its subtree were not marked invalid, we can consider it valid at this point. - modelState.MarkFieldValid(ModelStateKey); - } - } - - private void ValidateChildren(ModelValidationContext validationContext) - { - for (var i = 0; i < _childNodes.Count; i++) - { - var child = _childNodes[i]; - var childValidationContext = new ModelValidationContext(validationContext, child.ModelMetadata); - child.Validate(childValidationContext, this); - } - - if (ValidateAllProperties) - { - ValidateProperties(validationContext); - } - } - - private void ValidateProperties(ModelValidationContext validationContext) - { - var modelState = validationContext.ModelState; - - var model = ModelMetadata.Model; - var updatedMetadata = validationContext.MetadataProvider.GetMetadataForType(() => model, - ModelMetadata.ModelType); - - foreach (var propertyMetadata in updatedMetadata.Properties) - { - // Only want to add errors to ModelState if something doesn't already exist for the property node, - // else we could end up with duplicate or irrelevant error messages. - var propertyKeyRoot = ModelBindingHelper.CreatePropertyModelName(ModelStateKey, - propertyMetadata.PropertyName); - - if (modelState.GetFieldValidationState(propertyKeyRoot) == ModelValidationState.Unvalidated) - { - var propertyValidators = GetValidators(validationContext, propertyMetadata); - var propertyValidationContext = new ModelValidationContext(validationContext, propertyMetadata); - foreach (var propertyValidator in propertyValidators) - { - foreach (var propertyResult in propertyValidator.Validate(propertyValidationContext)) - { - var thisErrorKey = ModelBindingHelper.CreatePropertyModelName(propertyKeyRoot, - propertyResult.MemberName); - modelState.TryAddModelError(thisErrorKey, propertyResult.Message); - } - } - } - } - } - - private void ValidateThis(ModelValidationContext validationContext, ModelValidationNode parentNode) - { - var modelState = validationContext.ModelState; - if (modelState.GetFieldValidationState(ModelStateKey) == ModelValidationState.Invalid) - { - // If any item in the key's subtree has been identified as invalid, short-circuit - return; - } - - // If the Model at the current node is null and there is no parent, we cannot validate, and the - // DataAnnotationsModelValidator will throw. So we intercept here to provide a catch-all value-required - // validation error - if (parentNode == null && ModelMetadata.Model == null) - { - modelState.TryAddModelError(ModelStateKey, Resources.Validation_ValueNotFound); - return; - } - - var container = TryConvertContainerToMetadataType(parentNode); - var validators = GetValidators(validationContext, ModelMetadata).ToArray(); - for (var i = 0; i < validators.Length; i++) - { - var validator = validators[i]; - foreach (var validationResult in validator.Validate(validationContext)) - { - var currentModelStateKey = ModelBindingHelper.CreatePropertyModelName(ModelStateKey, - validationResult.MemberName); - modelState.TryAddModelError(currentModelStateKey, validationResult.Message); - } - } - } - - private static IEnumerable GetValidators(ModelValidationContext validationContext, - ModelMetadata metadata) - { - return validationContext.ValidatorProvider.GetValidators(metadata); - } - } } diff --git a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs index 8397ac997e..905195823f 100644 --- a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs +++ b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/ApiController.cs @@ -54,6 +54,10 @@ namespace System.Web.Http [FromServices] public IModelMetadataProvider MetadataProvider { get; set; } + + [FromServices] + public IObjectModelValidator ObjectValidator { get; set; } + /// /// Gets model state after the model binding process. This ModelState will be empty before model binding /// happens. @@ -417,18 +421,14 @@ namespace System.Web.Http { var modelMetadata = MetadataProvider.GetMetadataForType(() => entity, typeof(TEntity)); - var bodyValidationExcludeFiltersProvider = Context.RequestServices - .GetRequiredService(); - var validator = Context.RequestServices.GetRequiredService(); - var modelValidationContext = new ModelValidationContext( - MetadataProvider, + keyPrefix, BindingContext.ValidatorProvider, ModelState, modelMetadata, - containerMetadata: null, - excludeFromValidationFilters: bodyValidationExcludeFiltersProvider.ExcludeFilters); - validator.Validate(modelValidationContext, keyPrefix); + containerMetadata: null); + + ObjectValidator.Validate(modelValidationContext); } protected virtual void Dispose(bool disposing) diff --git a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/HttpRequestMessage/HttpRequestMessageModelBinder.cs b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/HttpRequestMessage/HttpRequestMessageModelBinder.cs index e9b7f7f212..9917d25936 100644 --- a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/HttpRequestMessage/HttpRequestMessageModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/HttpRequestMessage/HttpRequestMessageModelBinder.cs @@ -9,15 +9,15 @@ namespace Microsoft.AspNet.Mvc.WebApiCompatShim { public class HttpRequestMessageModelBinder : IModelBinder { - public Task BindModelAsync(ModelBindingContext bindingContext) + public Task BindModelAsync(ModelBindingContext bindingContext) { if (bindingContext.ModelType == typeof(HttpRequestMessage)) { - bindingContext.Model = bindingContext.OperationBindingContext.HttpContext.GetHttpRequestMessage(); - return Task.FromResult(true); + var model = bindingContext.OperationBindingContext.HttpContext.GetHttpRequestMessage(); + return Task.FromResult(new ModelBindingResult(model, bindingContext.ModelName, true)); } - return Task.FromResult(false); + return Task.FromResult(null); } } } diff --git a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/WebApiCompatShimServiceCollectionExtensions.cs b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/WebApiCompatShimServiceCollectionExtensions.cs index ea098a4377..6367785703 100644 --- a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/WebApiCompatShimServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/WebApiCompatShimServiceCollectionExtensions.cs @@ -1,7 +1,9 @@ // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Net.Http; using System.Net.Http.Formatting; +using Microsoft.AspNet.Mvc; using Microsoft.AspNet.Mvc.WebApiCompatShim; namespace Microsoft.Framework.DependencyInjection @@ -15,6 +17,10 @@ namespace Microsoft.Framework.DependencyInjection // The constructors on DefaultContentNegotiator aren't DI friendly, so just // new it up. services.AddInstance(new DefaultContentNegotiator()); + services.Configure(options => + { + options.ValidationExcludeFilters.Add(typeof(HttpRequestMessage)); + }); return services; } diff --git a/src/Microsoft.AspNet.Mvc/MvcOptionsSetup.cs b/src/Microsoft.AspNet.Mvc/MvcOptionsSetup.cs index b1dd8eb837..749dcf7340 100644 --- a/src/Microsoft.AspNet.Mvc/MvcOptionsSetup.cs +++ b/src/Microsoft.AspNet.Mvc/MvcOptionsSetup.cs @@ -68,6 +68,12 @@ namespace Microsoft.AspNet.Mvc options.ValidationExcludeFilters.Add(typeof(XObject)); options.ValidationExcludeFilters.Add(typeof(Type)); options.ValidationExcludeFilters.Add(typeof(JToken)); + + // Any 'known' types that we bind should be marked as excluded from validation. + options.ValidationExcludeFilters.Add(typeof(System.Threading.CancellationToken)); + options.ValidationExcludeFilters.Add(typeof(Http.IFormFile)); + options.ValidationExcludeFilters.Add(typeof(Http.IFormCollection)); + options.ValidationExcludeFilters.Add(typeFullName: "System.Xml.XmlNode"); } } diff --git a/src/Microsoft.AspNet.Mvc/MvcServices.cs b/src/Microsoft.AspNet.Mvc/MvcServices.cs index c72994092d..42ed31f887 100644 --- a/src/Microsoft.AspNet.Mvc/MvcServices.cs +++ b/src/Microsoft.AspNet.Mvc/MvcServices.cs @@ -62,6 +62,7 @@ namespace Microsoft.AspNet.Mvc yield return describe.Singleton(); yield return describe.Scoped(); yield return describe.Transient(); + yield return describe.Transient(); yield return describe.Transient, ControllerActionDescriptorProvider>(); @@ -91,7 +92,6 @@ namespace Microsoft.AspNet.Mvc yield return describe.Instance(new JsonOutputFormatter()); yield return describe.Transient(); - yield return describe.Transient(); yield return describe.Transient(); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/BodyModelBinderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/BodyModelBinderTests.cs index 29e86d034b..c8ebccd6b5 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/BodyModelBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/BodyModelBinderTests.cs @@ -17,13 +17,9 @@ namespace Microsoft.AspNet.Mvc public class BodyModelBinderTests { [Fact] - public async Task BindModel_CallsValidationAndSelectedInputFormatterOnce() + public async Task BindModel_CallsSelectedInputFormatterOnce() { // Arrange - var mockValidator = new Mock(); - mockValidator.Setup(o => o.Validate(It.IsAny(), It.IsAny())) - .Returns(true) - .Verifiable(); var mockInputFormatter = new Mock(); mockInputFormatter.Setup(o => o.ReadAsync(It.IsAny())) .Returns(Task.FromResult(new Person())) @@ -32,13 +28,12 @@ namespace Microsoft.AspNet.Mvc var bindingContext = GetBindingContext(typeof(Person), inputFormatter: mockInputFormatter.Object); bindingContext.ModelMetadata.BinderMetadata = new FromBodyAttribute(); - var binder = GetBodyBinder(mockInputFormatter.Object, mockValidator.Object); + var binder = GetBodyBinder(mockInputFormatter.Object); // Act var binderResult = await binder.BindModelAsync(bindingContext); // Assert - mockValidator.Verify(v => v.Validate(It.IsAny(), It.IsAny()), Times.Once); mockInputFormatter.Verify(v => v.ReadAsync(It.IsAny()), Times.Once); } @@ -57,8 +52,9 @@ namespace Microsoft.AspNet.Mvc // Assert // Returns true because it understands the metadata type. - Assert.True(binderResult); - Assert.Null(bindingContext.Model); + Assert.NotNull(binderResult); + Assert.False(binderResult.IsModelSet); + Assert.Null(binderResult.Model); Assert.True(bindingContext.ModelState.ContainsKey("someName")); } @@ -78,7 +74,8 @@ namespace Microsoft.AspNet.Mvc var binderResult = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(binderResult); + Assert.NotNull(binderResult); + Assert.False(binderResult.IsModelSet); } [Fact] @@ -97,7 +94,7 @@ namespace Microsoft.AspNet.Mvc var binderResult = await binder.BindModelAsync(bindingContext); // Assert - Assert.False(binderResult); + Assert.Null(binderResult); } [Fact] @@ -116,7 +113,7 @@ namespace Microsoft.AspNet.Mvc var binderResult = await binder.BindModelAsync(bindingContext); // Assert - Assert.False(binderResult); + Assert.Null(binderResult); } private static ModelBindingContext GetBindingContext(Type modelType, IInputFormatter inputFormatter) @@ -124,7 +121,7 @@ namespace Microsoft.AspNet.Mvc var metadataProvider = new EmptyModelMetadataProvider(); var operationBindingContext = new OperationBindingContext { - ModelBinder = GetBodyBinder(inputFormatter, null), + ModelBinder = GetBodyBinder(inputFormatter), MetadataProvider = metadataProvider, HttpContext = new DefaultHttpContext(), }; @@ -141,8 +138,7 @@ namespace Microsoft.AspNet.Mvc return bindingContext; } - private static BodyModelBinder GetBodyBinder( - IInputFormatter inputFormatter, IBodyModelValidator validator) + private static BodyModelBinder GetBodyBinder(IInputFormatter inputFormatter) { var actionContext = CreateActionContext(new DefaultHttpContext()); var inputFormatterSelector = new Mock(); @@ -152,15 +148,6 @@ namespace Microsoft.AspNet.Mvc It.IsAny())) .Returns(inputFormatter); - if (validator == null) - { - var mockValidator = new Mock(); - mockValidator.Setup(o => o.Validate(It.IsAny(), It.IsAny())) - .Returns(true) - .Verifiable(); - validator = mockValidator.Object; - } - var bodyValidationPredicatesProvider = new Mock(); bodyValidationPredicatesProvider.SetupGet(o => o.ExcludeFilters) .Returns(new List()); @@ -179,7 +166,6 @@ namespace Microsoft.AspNet.Mvc actionContext, bindingContextAccessor, inputFormatterSelector.Object, - validator, bodyValidationPredicatesProvider.Object); return binder; diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionInvokerTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionInvokerTest.cs index b94c4bbfa4..0da32e5b73 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionInvokerTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerActionInvokerTest.cs @@ -1990,7 +1990,9 @@ namespace Microsoft.AspNet.Mvc var inputFormattersProvider = new Mock(); inputFormattersProvider.SetupGet(o => o.InputFormatters) .Returns(new List()); - + var excludeFilterProvider = new Mock(); + excludeFilterProvider.SetupGet(o => o.ExcludeFilters) + .Returns(new List()); var invoker = new TestControllerActionInvoker( actionContext, filterProvider.Object, @@ -2029,7 +2031,7 @@ namespace Microsoft.AspNet.Mvc var binder = new Mock(); binder.Setup(b => b.BindModelAsync(It.IsAny())) - .Returns(Task.FromResult(result: false)); + .Returns(Task.FromResult(result: null)); var context = new Mock(); context.SetupGet(c => c.Items) .Returns(new Dictionary()); @@ -2042,18 +2044,21 @@ namespace Microsoft.AspNet.Mvc var inputFormattersProvider = new Mock(); inputFormattersProvider.SetupGet(o => o.InputFormatters) .Returns(new List()); - + var metadataProvider = new EmptyModelMetadataProvider(); var invoker = new ControllerActionInvoker( actionContext, Mock.Of>(), controllerFactory.Object, actionDescriptor, inputFormattersProvider.Object, - new DefaultControllerActionArgumentBinder(new EmptyModelMetadataProvider(), new MockMvcOptionsAccessor()), - new MockModelBinderProvider() { ModelBinders = new List() { binder.Object } }, - new MockModelValidatorProviderProvider(), - new MockValueProviderFactoryProvider(), - new MockScopedInstance()); + new DefaultControllerActionArgumentBinder( + metadataProvider, + new DefaultObjectValidator(Mock.Of(), metadataProvider), + new MockMvcOptionsAccessor()), + new MockModelBinderProvider() { ModelBinders = new List() { binder.Object } }, + new MockModelValidatorProviderProvider(), + new MockValueProviderFactoryProvider(), + new MockScopedInstance()); // Act await invoker.InvokeAsync(); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs index 4b1815604d..78a96eae8d 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ControllerTests.cs @@ -13,6 +13,8 @@ using Microsoft.AspNet.Mvc.ModelBinding; using Microsoft.AspNet.Routing; using Microsoft.AspNet.Testing; using Microsoft.AspNet.WebUtilities; +using Microsoft.AspNet.Http.Core; +using Microsoft.Framework.DependencyInjection; #if ASPNET50 using Moq; #endif @@ -917,7 +919,7 @@ namespace Microsoft.AspNet.Mvc.Test Assert.True(context.PropertyFilter(context, "Property1")); Assert.True(context.PropertyFilter(context, "Property2")); }) - .Returns(Task.FromResult(false)) + .Returns(Task.FromResult(null)) .Verifiable(); var controller = GetController(binder.Object, valueProvider); @@ -950,7 +952,7 @@ namespace Microsoft.AspNet.Mvc.Test Assert.True(context.PropertyFilter(context, "Property1")); Assert.True(context.PropertyFilter(context, "Property2")); }) - .Returns(Task.FromResult(false)) + .Returns(Task.FromResult(null)) .Verifiable(); var controller = GetController(binder.Object, valueProvider); @@ -982,7 +984,7 @@ namespace Microsoft.AspNet.Mvc.Test Assert.True(context.PropertyFilter(context, "Property1")); Assert.True(context.PropertyFilter(context, "Property2")); }) - .Returns(Task.FromResult(false)) + .Returns(Task.FromResult(null)) .Verifiable(); var controller = GetController(binder.Object, provider: null); @@ -1019,7 +1021,7 @@ namespace Microsoft.AspNet.Mvc.Test Assert.False(context.PropertyFilter(context, "exclude1")); Assert.False(context.PropertyFilter(context, "exclude2")); }) - .Returns(Task.FromResult(true)) + .Returns(Task.FromResult(null)) .Verifiable(); var controller = GetController(binder.Object, valueProvider); @@ -1056,7 +1058,7 @@ namespace Microsoft.AspNet.Mvc.Test Assert.False(context.PropertyFilter(context, "exclude1")); Assert.False(context.PropertyFilter(context, "exclude2")); }) - .Returns(Task.FromResult(true)) + .Returns(Task.FromResult(null)) .Verifiable(); var controller = GetController(binder.Object, provider: null); @@ -1090,7 +1092,7 @@ namespace Microsoft.AspNet.Mvc.Test Assert.False(context.PropertyFilter(context, "exclude1")); Assert.False(context.PropertyFilter(context, "exclude2")); }) - .Returns(Task.FromResult(true)) + .Returns(Task.FromResult(null)) .Verifiable(); @@ -1125,7 +1127,7 @@ namespace Microsoft.AspNet.Mvc.Test Assert.False(context.PropertyFilter(context, "exclude1")); Assert.False(context.PropertyFilter(context, "exclude2")); }) - .Returns(Task.FromResult(true)) + .Returns(Task.FromResult(null)) .Verifiable(); var controller = GetController(binder.Object, provider: null); @@ -1336,7 +1338,8 @@ namespace Microsoft.AspNet.Mvc.Test private static Controller GetController(IModelBinder binder, IValueProvider provider) { var metadataProvider = new DataAnnotationsModelMetadataProvider(); - var actionContext = new ActionContext(Mock.Of(), new RouteData(), new ActionDescriptor()); + var httpContext = new DefaultHttpContext(); + var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); var viewData = new ViewDataDictionary(metadataProvider, new ModelStateDictionary()); @@ -1344,6 +1347,7 @@ namespace Microsoft.AspNet.Mvc.Test { ModelBinder = binder, ValueProvider = provider, + ValidatorProvider = new DataAnnotationsModelValidatorProvider() }; return new TestableController() @@ -1351,7 +1355,8 @@ namespace Microsoft.AspNet.Mvc.Test ActionContext = actionContext, BindingContext = bindingContext, MetadataProvider = metadataProvider, - ViewData = viewData + ViewData = viewData, + ObjectValidator = new DefaultObjectValidator(Mock.Of(), metadataProvider) }; } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultControllerFactoryTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultControllerFactoryTest.cs index 1dfc9b5272..8560032f4f 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultControllerFactoryTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultControllerFactoryTest.cs @@ -268,13 +268,16 @@ namespace Microsoft.AspNet.Mvc.Core private IServiceProvider GetServices() { + var metadataProvider = new EmptyModelMetadataProvider(); var services = new Mock(); services.Setup(s => s.GetService(typeof(IUrlHelper))) .Returns(Mock.Of()); services.Setup(s => s.GetService(typeof(IModelMetadataProvider))) - .Returns(new EmptyModelMetadataProvider()); + .Returns(metadataProvider); services.Setup(s => s.GetService(typeof(TestService))) .Returns(new TestService()); + services.Setup(s => s.GetService(typeof(IObjectModelValidator))) + .Returns(new DefaultObjectValidator(Mock.Of(), metadataProvider)); services .Setup(s => s.GetService(typeof(IScopedInstance))) .Returns(new MockScopedInstance()); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/OptionDescriptors/DefaultModelBindersProviderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/OptionDescriptors/DefaultModelBindersProviderTest.cs index 82c6005224..1ed61fb367 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/OptionDescriptors/DefaultModelBindersProviderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/OptionDescriptors/DefaultModelBindersProviderTest.cs @@ -53,7 +53,7 @@ namespace Microsoft.AspNet.Mvc.OptionDescriptors public ITestService Service { get; private set; } - public Task BindModelAsync(ModelBindingContext bindingContext) + public Task BindModelAsync(ModelBindingContext bindingContext) { throw new NotImplementedException(); } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/OptionDescriptors/ModelBinderDescriptorTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/OptionDescriptors/ModelBinderDescriptorTest.cs index 8d1dc7c4dd..a016f41503 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/OptionDescriptors/ModelBinderDescriptorTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/OptionDescriptors/ModelBinderDescriptorTest.cs @@ -53,7 +53,7 @@ namespace Microsoft.AspNet.Mvc.OptionDescriptors private class TestModelBinder : IModelBinder { - public Task BindModelAsync(ModelBindingContext bindingContext) + public Task BindModelAsync(ModelBindingContext bindingContext) { throw new NotImplementedException(); } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs index 9eda87caa1..bf913cc8d5 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs @@ -169,12 +169,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test var binder = new Mock(); binder .Setup(b => b.BindModelAsync(It.IsAny())) - .Callback(c => - { - // This value won't go into the arguments, because we return false. - c.Model = "Hello"; - }) - .Returns(Task.FromResult(result: false)); + .Returns(Task.FromResult(result: null)); var actionContext = new ActionContext( new DefaultHttpContext(), @@ -186,13 +181,14 @@ namespace Microsoft.AspNet.Mvc.Core.Test ModelBinder = binder.Object, }; + var modelMetadataProvider = new DataAnnotationsModelMetadataProvider(); var inputFormattersProvider = new Mock(); inputFormattersProvider .SetupGet(o => o.InputFormatters) .Returns(new List()); - var invoker = new DefaultControllerActionArgumentBinder( - new DataAnnotationsModelMetadataProvider(), + modelMetadataProvider, + new DefaultObjectValidator(Mock.Of(), modelMetadataProvider), new MockMvcOptionsAccessor()); // Act @@ -223,11 +219,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test var binder = new Mock(); binder .Setup(b => b.BindModelAsync(It.IsAny())) - .Callback(c => - { - Assert.False(c.IsModelSet); - }) - .Returns(Task.FromResult(result: true)); + .Returns(Task.FromResult(new ModelBindingResult(null, "", false))); var actionContext = new ActionContext( new DefaultHttpContext(), @@ -244,8 +236,10 @@ namespace Microsoft.AspNet.Mvc.Core.Test .SetupGet(o => o.InputFormatters) .Returns(new List()); + var modelMetadataProvider = new DataAnnotationsModelMetadataProvider(); var invoker = new DefaultControllerActionArgumentBinder( - new DataAnnotationsModelMetadataProvider(), + modelMetadataProvider, + new DefaultObjectValidator(Mock.Of(), modelMetadataProvider), new MockMvcOptionsAccessor()); // Act @@ -284,10 +278,8 @@ namespace Microsoft.AspNet.Mvc.Core.Test context.ModelMetadata = metadataProvider.GetMetadataForType( modelAccessor: null, modelType: typeof(string)); - - context.Model = value; }) - .Returns(Task.FromResult(result: true)); + .Returns(Task.FromResult(result: new ModelBindingResult(value, "", true))); var actionContext = new ActionContext( new DefaultHttpContext(), @@ -299,8 +291,12 @@ namespace Microsoft.AspNet.Mvc.Core.Test ModelBinder = binder.Object, }; + var mockValidatorProvider = new Mock(MockBehavior.Strict); + mockValidatorProvider.Setup(o => o.Validate(It.IsAny())); + var invoker = new DefaultControllerActionArgumentBinder( metadataProvider, + mockValidatorProvider.Object, new MockMvcOptionsAccessor()); // Act @@ -311,6 +307,107 @@ namespace Microsoft.AspNet.Mvc.Core.Test Assert.Equal(value, result["foo"]); } + [Fact] + public async Task GetActionArgumentsAsync_CallsValidator_IfModelBinderSucceeds() + { + // Arrange + Func method = foo => 1; + var actionDescriptor = new ControllerActionDescriptor + { + MethodInfo = method.Method, + Parameters = new List + { + new ParameterDescriptor + { + Name = "foo", + ParameterType = typeof(object), + } + } + }; + + var actionContext = new ActionContext( + new DefaultHttpContext(), + new RouteData(), + actionDescriptor); + + var binder = new Mock(); + binder + .Setup(b => b.BindModelAsync(It.IsAny())) + .Returns(Task.FromResult(result: new ModelBindingResult( + model: null, + key: string.Empty, + isModelSet: true))); + + var actionBindingContext = new ActionBindingContext() + { + ModelBinder = binder.Object, + }; + + var mockValidatorProvider = new Mock(MockBehavior.Strict); + mockValidatorProvider + .Setup(o => o.Validate(It.IsAny())) + .Verifiable(); + var invoker = new DefaultControllerActionArgumentBinder( + new DataAnnotationsModelMetadataProvider(), + mockValidatorProvider.Object, + new MockMvcOptionsAccessor()); + + // Act + var result = await invoker.GetActionArgumentsAsync(actionContext, actionBindingContext); + + // Assert + mockValidatorProvider.Verify( + o => o.Validate(It.IsAny()), Times.Once()); + } + + [Fact] + public async Task GetActionArgumentsAsync_DoesNotCallValidator_IfModelBinderFails() + { + // Arrange + Func method = foo => 1; + var actionDescriptor = new ControllerActionDescriptor + { + MethodInfo = method.Method, + Parameters = new List + { + new ParameterDescriptor + { + Name = "foo", + ParameterType = typeof(object), + } + } + }; + + var actionContext = new ActionContext( + new DefaultHttpContext(), + new RouteData(), + actionDescriptor); + + var binder = new Mock(); + binder + .Setup(b => b.BindModelAsync(It.IsAny())) + .Returns(Task.FromResult(null)); + + var actionBindingContext = new ActionBindingContext() + { + ModelBinder = binder.Object, + }; + + var mockValidatorProvider = new Mock(MockBehavior.Strict); + mockValidatorProvider.Setup(o => o.Validate(It.IsAny())) + .Verifiable(); + var invoker = new DefaultControllerActionArgumentBinder( + new DataAnnotationsModelMetadataProvider(), + mockValidatorProvider.Object, + new MockMvcOptionsAccessor()); + + // Act + var result = await invoker.GetActionArgumentsAsync(actionContext, actionBindingContext); + + // Assert + mockValidatorProvider.Verify(o => o.Validate(It.IsAny()), Times.Never()); + } + [Fact] public async Task GetActionArgumentsAsync_SetsMaxModelErrors() { @@ -332,11 +429,8 @@ namespace Microsoft.AspNet.Mvc.Core.Test var binder = new Mock(); binder .Setup(b => b.BindModelAsync(It.IsAny())) - .Callback(c => - { - c.Model = "Hello"; - }) - .Returns(Task.FromResult(result: true)); + .Returns(Task.FromResult( + result: new ModelBindingResult(model: "Hello", key: string.Empty, isModelSet: true))); var actionContext = new ActionContext( new DefaultHttpContext(), @@ -355,9 +449,11 @@ namespace Microsoft.AspNet.Mvc.Core.Test var options = new MockMvcOptionsAccessor(); options.Options.MaxModelValidationErrors = 5; - + var mockValidatorProvider = new Mock(MockBehavior.Strict); + mockValidatorProvider.Setup(o => o.Validate(It.IsAny())); var invoker = new DefaultControllerActionArgumentBinder( new DataAnnotationsModelMetadataProvider(), + mockValidatorProvider.Object, options); // Act diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ModelBindingHelperTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ModelBindingHelperTest.cs index d918ec44d8..16acb3f3fd 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ModelBindingHelperTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ModelBindingHelperTest.cs @@ -24,25 +24,26 @@ namespace Microsoft.AspNet.Mvc.Core.Test { // Arrange var metadataProvider = new Mock(); - metadataProvider.Setup(m => m.GetMetadataForType(null, It.IsAny())) + metadataProvider.Setup(m => m.GetMetadataForType(It.IsAny>(), It.IsAny())) .Returns(new ModelMetadata(metadataProvider.Object, null, null, typeof(MyModel), null)) .Verifiable(); var binder = new Mock(); binder.Setup(b => b.BindModelAsync(It.IsAny())) - .Returns(Task.FromResult(false)); + .Returns(Task.FromResult(null)); var model = new MyModel(); - + // Act var result = await ModelBindingHelper.TryUpdateModelAsync( - model, - null, - Mock.Of(), - new ModelStateDictionary(), - metadataProvider.Object, - GetCompositeBinder(binder.Object), - Mock.Of(), - Mock.Of()); + model, + null, + Mock.Of(), + new ModelStateDictionary(), + metadataProvider.Object, + GetCompositeBinder(binder.Object), + Mock.Of(), + new DefaultObjectValidator(Mock.Of(), metadataProvider.Object), + Mock.Of()); // Assert Assert.False(result); @@ -71,17 +72,19 @@ namespace Microsoft.AspNet.Mvc.Core.Test { "", null } }; var valueProvider = new TestValueProvider(values); + var modelMetadataProvider = new DataAnnotationsModelMetadataProvider(); // Act var result = await ModelBindingHelper.TryUpdateModelAsync( - model, - "", - Mock.Of(), - modelStateDictionary, - new DataAnnotationsModelMetadataProvider(), - GetCompositeBinder(binders), - valueProvider, - validator); + model, + "", + Mock.Of(), + modelStateDictionary, + modelMetadataProvider, + GetCompositeBinder(binders), + valueProvider, + new DefaultObjectValidator(Mock.Of(), modelMetadataProvider), + validator); // Assert Assert.False(result); @@ -109,17 +112,19 @@ namespace Microsoft.AspNet.Mvc.Core.Test { "MyProperty", "MyPropertyValue" } }; var valueProvider = new TestValueProvider(values); + var metadataProvider = new DataAnnotationsModelMetadataProvider(); // Act var result = await ModelBindingHelper.TryUpdateModelAsync( - model, - "", - Mock.Of(), - modelStateDictionary, - new DataAnnotationsModelMetadataProvider(), - GetCompositeBinder(binders), - valueProvider, - validator); + model, + "", + Mock.Of(), + modelStateDictionary, + metadataProvider, + GetCompositeBinder(binders), + valueProvider, + new DefaultObjectValidator(Mock.Of(), metadataProvider), + validator); // Assert Assert.True(result); @@ -131,27 +136,28 @@ namespace Microsoft.AspNet.Mvc.Core.Test { // Arrange var metadataProvider = new Mock(); - metadataProvider.Setup(m => m.GetMetadataForType(null, It.IsAny())) + metadataProvider.Setup(m => m.GetMetadataForType(It.IsAny>(), It.IsAny())) .Returns(new ModelMetadata(metadataProvider.Object, null, null, typeof(MyModel), null)) .Verifiable(); var binder = new Mock(); binder.Setup(b => b.BindModelAsync(It.IsAny())) - .Returns(Task.FromResult(false)); + .Returns(Task.FromResult(null)); var model = new MyModel(); Func includePredicate = (context, propertyName) => true; // Act var result = await ModelBindingHelper.TryUpdateModelAsync( - model, - null, - Mock.Of(), - new ModelStateDictionary(), - metadataProvider.Object, - GetCompositeBinder(binder.Object), - Mock.Of(), - Mock.Of(), - includePredicate); + model, + null, + Mock.Of(), + new ModelStateDictionary(), + metadataProvider.Object, + GetCompositeBinder(binder.Object), + Mock.Of(), + Mock.Of(), + Mock.Of(), + includePredicate); // Assert Assert.False(result); @@ -194,18 +200,20 @@ namespace Microsoft.AspNet.Mvc.Core.Test string.Equals(propertyName, "MyProperty", StringComparison.OrdinalIgnoreCase); var valueProvider = new TestValueProvider(values); + var metadataProvider = new DataAnnotationsModelMetadataProvider(); // Act var result = await ModelBindingHelper.TryUpdateModelAsync( - model, - "", - Mock.Of(), - modelStateDictionary, - new DataAnnotationsModelMetadataProvider(), - GetCompositeBinder(binders), - valueProvider, - validator, - includePredicate); + model, + "", + Mock.Of(), + modelStateDictionary, + metadataProvider, + GetCompositeBinder(binders), + valueProvider, + new DefaultObjectValidator(Mock.Of(), metadataProvider), + validator, + includePredicate); // Assert Assert.True(result); @@ -219,13 +227,13 @@ namespace Microsoft.AspNet.Mvc.Core.Test { // Arrange var metadataProvider = new Mock(); - metadataProvider.Setup(m => m.GetMetadataForType(null, It.IsAny())) + metadataProvider.Setup(m => m.GetMetadataForType(It.IsAny>(), It.IsAny())) .Returns(new ModelMetadata(metadataProvider.Object, null, null, typeof(MyModel), null)) .Verifiable(); var binder = new Mock(); binder.Setup(b => b.BindModelAsync(It.IsAny())) - .Returns(Task.FromResult(false)); + .Returns(Task.FromResult(null)); var model = new MyModel(); // Act @@ -237,6 +245,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test metadataProvider.Object, GetCompositeBinder(binder.Object), Mock.Of(), + Mock.Of(), Mock.Of(), m => m.IncludedProperty ); @@ -277,19 +286,21 @@ namespace Microsoft.AspNet.Mvc.Core.Test }; var valueProvider = new TestValueProvider(values); + var metadataProvider = new DataAnnotationsModelMetadataProvider(); // Act var result = await ModelBindingHelper.TryUpdateModelAsync( - model, - "", - Mock.Of(), - modelStateDictionary, - new DataAnnotationsModelMetadataProvider(), - GetCompositeBinder(binders), - valueProvider, - validator, - m => m.IncludedProperty, - m => m.MyProperty); + model, + "", + Mock.Of(), + modelStateDictionary, + new DataAnnotationsModelMetadataProvider(), + GetCompositeBinder(binders), + valueProvider, + new DefaultObjectValidator(Mock.Of(), metadataProvider), + validator, + m => m.IncludedProperty, + m => m.MyProperty); // Assert Assert.True(result); @@ -327,17 +338,19 @@ namespace Microsoft.AspNet.Mvc.Core.Test }; var valueProvider = new TestValueProvider(values); + var metadataProvider = new DataAnnotationsModelMetadataProvider(); // Act var result = await ModelBindingHelper.TryUpdateModelAsync( - model, - "", - Mock.Of(), - modelStateDictionary, - new DataAnnotationsModelMetadataProvider(), - GetCompositeBinder(binders), - valueProvider, - validator); + model, + "", + Mock.Of(), + modelStateDictionary, + metadataProvider, + GetCompositeBinder(binders), + valueProvider, + new DefaultObjectValidator(Mock.Of(), metadataProvider), + validator); // Assert // Includes everything. diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/InputObjectValidationTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/InputObjectValidationTests.cs index b453cebe20..fc92465a57 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/InputObjectValidationTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/InputObjectValidationTests.cs @@ -135,7 +135,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var responseContent = await response.Content.ReadAsStringAsync(); var responseObject = JsonConvert.DeserializeObject>(responseContent); - var errorCollection = Assert.Single(responseObject); + var errorCollection = Assert.Single(responseObject, modelState => modelState.Value.Errors.Any()); var error = Assert.Single(errorCollection.Value.Errors); Assert.Equal(expectedModelStateErrorMessage, error.ErrorMessage); @@ -164,7 +164,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var responseContent = await response.Content.ReadAsStringAsync(); var responseObject = JsonConvert.DeserializeObject>(responseContent); - var errorCollection = Assert.Single(responseObject); + var errorCollection = Assert.Single(responseObject, modelState => modelState.Value.Errors.Any()); var error = Assert.Single(errorCollection.Value.Errors); Assert.Equal(expectedModelStateErrorMessage, error.ErrorMessage); } diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs index abe6e66c3d..a715fc02f8 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs @@ -26,6 +26,45 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests private readonly IServiceProvider _services = TestHelper.CreateServices("ModelBindingWebSite"); private readonly Action _app = new ModelBindingWebSite.Startup().Configure; + [Fact] + public async Task TypeBasedExclusion_ForBodyAndNonBodyBoundModels() + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.CreateClient(); + + // Make sure the body object gets created with an invalid zip. + var input = "{\"OfficeAddress.Zip\":\"45\"}"; + var content = new StringContent(input, Encoding.UTF8, "application/json"); + + // Act + // Make sure the non body based object gets created with an invalid zip. + var response = await client.PostAsync( + "http://localhost/Validation/SkipValidation?ShippingAddresses[0].Zip=45&HomeAddress.Zip=46", content); + + // Assert + var stringValue = await response.Content.ReadAsStringAsync(); + var isModelStateValid = JsonConvert.DeserializeObject(stringValue); + Assert.True(isModelStateValid); + } + + [Fact] + public async Task ModelValidation_DoesNotValidate_AnAlreadyValidatedObject() + { + // Arrange + var server = TestServer.Create(_services, _app); + var client = server.CreateClient(); + + // Act + var response = await client.GetAsync( + "http://localhost/Validation/AvoidRecursive?Name=selfish"); + + // Assert + var stringValue = await response.Content.ReadAsStringAsync(); + var isModelStateValid = JsonConvert.DeserializeObject(stringValue); + Assert.True(isModelStateValid); + } + [Theory] [InlineData("RestrictValueProvidersUsingFromRoute", "valueFromRoute")] [InlineData("RestrictValueProvidersUsingFromQuery", "valueFromQuery")] @@ -1012,7 +1051,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests // Act var response = await client.GetStringAsync("http://localhost/TryUpdateModel/" + "CreateAndUpdateUser" + - "?RegisterationMonth=March"); + "?RegistedeburationMonth=March"); // Assert var result = JsonConvert.DeserializeObject(response); diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ArrayModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ArrayModelBinderTest.cs index 938d6b5397..9e70a4c403 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ArrayModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ArrayModelBinderTest.cs @@ -26,9 +26,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var retVal = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(retVal); + Assert.NotNull(retVal); - int[] array = bindingContext.Model as int[]; + int[] array = retVal.Model as int[]; Assert.Equal(new[] { 42, 84 }, array); } @@ -43,7 +43,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var bound = await binder.BindModelAsync(bindingContext); // Assert - Assert.False(bound); + Assert.Null(bound); } [Fact] @@ -62,7 +62,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var bound = await binder.BindModelAsync(bindingContext); // Assert - Assert.False(bound); + Assert.Null(bound); } private static IModelBinder CreateIntBinder() @@ -75,10 +75,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var value = await mbc.ValueProvider.GetValueAsync(mbc.ModelName); if (value != null) { - mbc.Model = value.ConvertTo(mbc.ModelType); - return true; + var model = value.ConvertTo(mbc.ModelType); + return new ModelBindingResult(model, key: null, isModelSet: true); } - return false; + return null; }); return mockIntBinder.Object; } diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/BinderTypeBasedModelBinderModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/BinderTypeBasedModelBinderModelBinderTest.cs index 1dcdd9c66e..dbc82e465d 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/BinderTypeBasedModelBinderModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/BinderTypeBasedModelBinderModelBinderTest.cs @@ -27,7 +27,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var binderResult = await binder.BindModelAsync(bindingContext); // Assert - Assert.False(binderResult); + Assert.Null(binderResult); } [Fact] @@ -43,7 +43,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var binderResult = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(binderResult); + Assert.NotNull(binderResult); } [Fact] @@ -67,8 +67,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var binderResult = await binder.BindModelAsync(bindingContext); // Assert - var p = (Person)bindingContext.Model; - Assert.True(binderResult); + var p = (Person)binderResult.Model; Assert.Equal(model.Age, p.Age); Assert.Equal(model.Name, p.Name); } @@ -95,8 +94,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var binderResult = await binder.BindModelAsync(bindingContext); // Assert - var p = (Person)bindingContext.Model; - Assert.True(binderResult); + var p = (Person)binderResult.Model; Assert.Equal(model.Age, p.Age); Assert.Equal(model.Name, p.Name); } @@ -152,9 +150,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test private class FalseModelBinder : IModelBinder { - public Task BindModelAsync(ModelBindingContext bindingContext) + public Task BindModelAsync(ModelBindingContext bindingContext) { - return Task.FromResult(false); + return Task.FromResult(null); } } @@ -167,10 +165,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test _model = new Person(); } - public Task BindModelAsync(ModelBindingContext bindingContext) + public Task BindModelAsync(ModelBindingContext bindingContext) { - bindingContext.Model = _model; - return Task.FromResult(true); + return Task.FromResult(new ModelBindingResult(_model, bindingContext.ModelName, true)); } } diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/BindingSourceModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/BindingSourceModelBinderTest.cs index df559821ab..99409e76fc 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/BindingSourceModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/BindingSourceModelBinderTest.cs @@ -45,7 +45,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(context); // Assert - Assert.False(result); + Assert.Null(result); Assert.False(binder.WasBindModelCoreCalled); } @@ -69,7 +69,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(context); // Assert - Assert.False(result); + Assert.Null(result); Assert.False(binder.WasBindModelCoreCalled); } @@ -93,7 +93,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(context); // Assert - Assert.True(result); + Assert.NotNull(result); + Assert.True(result.IsModelSet); Assert.True(binder.WasBindModelCoreCalled); } @@ -106,10 +107,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { } - protected override Task BindModelCoreAsync([NotNull] ModelBindingContext bindingContext) + protected override Task BindModelCoreAsync([NotNull] ModelBindingContext bindingContext) { WasBindModelCoreCalled = true; - return Task.FromResult(true); + return Task.FromResult(new ModelBindingResult(null, bindingContext.ModelName, true)); } } } diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ByteArrayModelBinderTests.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ByteArrayModelBinderTests.cs index c13af2c32b..0757150669 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ByteArrayModelBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ByteArrayModelBinderTests.cs @@ -30,8 +30,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var binderResult = await binder.BindModelAsync(bindingContext); // Assert - Assert.False(binderResult); - Assert.Null(bindingContext.Model); + Assert.Null(binderResult); } [Fact] @@ -50,8 +49,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var binderResult = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(binderResult); - var bytes = Assert.IsType(bindingContext.Model); + Assert.NotNull(binderResult); + var bytes = Assert.IsType(binderResult.Model); Assert.Equal(new byte[] { 23, 43, 53 }, bytes); } @@ -75,7 +74,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var binderResult = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(binderResult); + Assert.NotNull(binderResult); Assert.False(bindingContext.ModelState.IsValid); var error = Assert.Single(bindingContext.ModelState["foo"].Errors); Assert.Equal(expected, error.ErrorMessage); @@ -97,7 +96,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var binderResult = await binder.BindModelAsync(bindingContext); // Assert - Assert.False(binderResult); + Assert.Null(binderResult); } [Fact] @@ -111,7 +110,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var binderResult = await binder.BindModelAsync(bindingContext); // Assert - Assert.False(binderResult); + Assert.Null(binderResult); } private static ModelBindingContext GetBindingContext(IValueProvider valueProvider, Type modelType) diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CancellationTokenModelBinderTests.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CancellationTokenModelBinderTests.cs index 8b3e601289..927dca4e51 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CancellationTokenModelBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CancellationTokenModelBinderTests.cs @@ -22,8 +22,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var bound = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(bound); - Assert.Equal(bindingContext.OperationBindingContext.HttpContext.RequestAborted, bindingContext.Model); + Assert.NotNull(bound); + Assert.Equal(bindingContext.OperationBindingContext.HttpContext.RequestAborted, bound.Model); } [Theory] @@ -40,8 +40,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var bound = await binder.BindModelAsync(bindingContext); // Assert - Assert.False(bound); - Assert.Null(bindingContext.Model); + Assert.Null(bound); } private static ModelBindingContext GetBindingContext(Type modelType) diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CollectionModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CollectionModelBinderTest.cs index bc50dda7fd..81309ae9a5 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CollectionModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CollectionModelBinderTest.cs @@ -34,7 +34,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test // Assert Assert.Equal(new[] { 42, 0, 200 }, boundCollection.ToArray()); - Assert.Equal(new[] { "someName[foo]", "someName[baz]" }, bindingContext.ValidationNode.ChildNodes.Select(o => o.ModelStateKey).ToArray()); } [Fact] @@ -55,7 +54,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test // Assert Assert.Equal(new[] { 42, 100 }, boundCollection.ToArray()); - Assert.Equal(new[] { "someName[0]", "someName[1]" }, bindingContext.ValidationNode.ChildNodes.Select(o => o.ModelStateKey).ToArray()); } [Fact] @@ -73,10 +71,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var binder = new CollectionModelBinder(); // Act - bool retVal = await binder.BindModelAsync(bindingContext); + var retVal = await binder.BindModelAsync(bindingContext); // Assert - Assert.Equal(new[] { 42, 100, 200 }, ((List)bindingContext.Model).ToArray()); + Assert.Equal(new[] { 42, 100, 200 }, ((List)retVal.Model).ToArray()); } [Fact] @@ -91,11 +89,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var binder = new CollectionModelBinder(); // Act - bool retVal = await binder.BindModelAsync(bindingContext); + var retVal = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(retVal); - Assert.Equal(new[] { 42, 100, 200 }, ((List)bindingContext.Model).ToArray()); + Assert.NotNull(retVal); + Assert.Equal(new[] { 42, 100, 200 }, ((List)retVal.Model).ToArray()); } #endif @@ -134,15 +132,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test 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); - childValidationNode = mbc.ValidationNode; - mbc.Model = 42; - return Task.FromResult(true); + return Task.FromResult(new ModelBindingResult(42, mbc.ModelName, true)); }); var modelBinder = new CollectionModelBinder(); @@ -151,7 +146,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test // Assert Assert.Equal(new[] { 42 }, boundCollection.ToArray()); - Assert.Equal(new[] { childValidationNode }, bindingContext.ValidationNode.ChildNodes.ToArray()); } private static ModelBindingContext GetModelBindingContext(IValueProvider valueProvider) @@ -181,10 +175,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var value = await mbc.ValueProvider.GetValueAsync(mbc.ModelName); if (value != null) { - mbc.Model = value.ConvertTo(mbc.ModelType); - return true; + var model = value.ConvertTo(mbc.ModelType); + return new ModelBindingResult(model, mbc.ModelName, true); } - return false; + + return null; }); return mockIntBinder.Object; } diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ComplexModelDtoResultTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ComplexModelDtoResultTest.cs deleted file mode 100644 index 0330680e16..0000000000 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ComplexModelDtoResultTest.cs +++ /dev/null @@ -1,35 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Xunit; - -namespace Microsoft.AspNet.Mvc.ModelBinding.Test -{ - public class ComplexModelDtoResultTest - { - [Fact] - public void Constructor_SetsProperties() - { - // Arrange - var validationNode = GetValidationNode(); - - // Act - var result = new ComplexModelDtoResult( - "some string", - isModelBound: true, - validationNode: validationNode); - - // Assert - Assert.Equal("some string", result.Model); - Assert.True(result.IsModelBound); - Assert.Equal(validationNode, result.ValidationNode); - } - - private static ModelValidationNode GetValidationNode() - { - var provider = new EmptyModelMetadataProvider(); - var metadata = provider.GetMetadataForType(null, typeof(object)); - return new ModelValidationNode(metadata, "someKey"); - } - } -} diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CompositeModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CompositeModelBinderTest.cs index e1f2ef22ff..fe3a88b329 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CompositeModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CompositeModelBinderTest.cs @@ -15,11 +15,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test public class CompositeModelBinderTest { [Fact] - public async Task BindModel_SuccessfulBind_RunsValidationAndReturnsModel() + public async Task BindModel_SuccessfulBind_ReturnsModel() { // Arrange - var validationCalled = false; - var bindingContext = new ModelBindingContext { FallbackToEmptyPrefix = true, @@ -46,28 +44,25 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test Assert.Equal("someName", context.ModelName); Assert.Same(bindingContext.ValueProvider, context.ValueProvider); - context.Model = 42; - bindingContext.ValidationNode.Validating += delegate { validationCalled = true; }; - return Task.FromResult(true); + + return Task.FromResult( + new ModelBindingResult(model: 42, key: "someName", isModelSet: true)); }); var shimBinder = CreateCompositeBinder(mockIntBinder.Object); // Act - var isBound = await shimBinder.BindModelAsync(bindingContext); + var result = await shimBinder.BindModelAsync(bindingContext); // Assert - Assert.True(isBound); - Assert.Equal(42, bindingContext.Model); - - Assert.True(validationCalled); - Assert.True(bindingContext.ModelState.IsValid); + Assert.NotNull(result); + Assert.True(result.IsModelSet); + Assert.Equal(42, result.Model); } [Fact] - public async Task BindModel_SuccessfulBind_ComplexTypeFallback_RunsValidationAndReturnsModel() + public async Task BindModel_SuccessfulBind_ComplexTypeFallback_ReturnsModel() { // Arrange - var validationCalled = false; var expectedModel = new List { 1, 2, 3, 4, 5 }; var bindingContext = new ModelBindingContext @@ -94,35 +89,32 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test { if (!string.IsNullOrEmpty(mbc.ModelName)) { - return Task.FromResult(false); + return Task.FromResult(null); } Assert.Same(bindingContext.ModelMetadata, mbc.ModelMetadata); Assert.Equal("", mbc.ModelName); Assert.Same(bindingContext.ValueProvider, mbc.ValueProvider); - mbc.Model = expectedModel; - mbc.ValidationNode.Validating += delegate { validationCalled = true; }; - return Task.FromResult(true); + return Task.FromResult(new ModelBindingResult(expectedModel, string.Empty, true)); }); var shimBinder = CreateCompositeBinder(mockIntBinder.Object); // Act - var isBound = await shimBinder.BindModelAsync(bindingContext); + var result = await shimBinder.BindModelAsync(bindingContext); // Assert - Assert.True(isBound); - Assert.Equal(expectedModel, bindingContext.Model); - Assert.True(validationCalled); - Assert.True(bindingContext.ModelState.IsValid); + Assert.NotNull(result); + Assert.True(result.IsModelSet); + Assert.Equal(string.Empty, result.Key); + Assert.Equal(expectedModel, result.Model); } [Fact] - public async Task ModelBinder_ReturnsTrue_WithoutSettingValue_SkipsValidation() + public async Task ModelBinder_ReturnsTrue_WithoutSettingValue() { // Arrange - var validationCalled = false; var bindingContext = new ModelBindingContext { @@ -143,31 +135,24 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var modelBinder = new Mock(); modelBinder .Setup(mb => mb.BindModelAsync(It.IsAny())) - .Callback(context => - { - context.ValidationNode.Validating += delegate { validationCalled = true; }; - }) - .Returns(Task.FromResult(true)); + .Returns(Task.FromResult(new ModelBindingResult(null, "someName", false))); var composite = CreateCompositeBinder(modelBinder.Object); // Act - var isBound = await composite.BindModelAsync(bindingContext); + var result = await composite.BindModelAsync(bindingContext); // Assert - Assert.True(isBound); - - Assert.Null(bindingContext.Model); - Assert.False(validationCalled); - Assert.False(bindingContext.IsModelSet); - Assert.True(bindingContext.ModelState.IsValid); + Assert.NotNull(result); + Assert.False(result.IsModelSet); + Assert.Equal("someName", result.Key); + Assert.Null(result.Model); } [Fact] - public async Task ModelBinder_ReturnsTrue_SetsNullValue_RunsValidation() + public async Task ModelBinder_ReturnsTrue_SetsNullValue_SetsModelStateKey() { // Arrange - var validationCalled = false; var bindingContext = new ModelBindingContext { @@ -188,25 +173,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var modelBinder = new Mock(); modelBinder .Setup(mb => mb.BindModelAsync(It.IsAny())) - .Callback(context => - { - context.Model = null; - context.ValidationNode.Validating += delegate { validationCalled = true; }; - }) - .Returns(Task.FromResult(true)); + .Returns(Task.FromResult(new ModelBindingResult(null, "someName", true))); var composite = CreateCompositeBinder(modelBinder.Object); // Act - var isBound = await composite.BindModelAsync(bindingContext); + var result = await composite.BindModelAsync(bindingContext); // Assert - Assert.True(isBound); - - Assert.Null(bindingContext.Model); - Assert.True(validationCalled); - Assert.True(bindingContext.IsModelSet); - Assert.False(bindingContext.ModelState.IsValid); + Assert.NotNull(result); + Assert.True(result.IsModelSet); + Assert.Equal("someName", result.Key); + Assert.Null(result.Model); } [Fact] @@ -215,7 +193,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test // Arrange var mockListBinder = new Mock(); mockListBinder.Setup(o => o.BindModelAsync(It.IsAny())) - .Returns(Task.FromResult(false)) + .Returns(Task.FromResult(null)) .Verifiable(); var shimBinder = mockListBinder.Object; @@ -227,11 +205,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test }; // Act - var isBound = await shimBinder.BindModelAsync(bindingContext); + var result = await shimBinder.BindModelAsync(bindingContext); // Assert - Assert.False(isBound); - Assert.Null(bindingContext.Model); + Assert.Null(result); Assert.True(bindingContext.ModelState.IsValid); mockListBinder.Verify(); } @@ -252,11 +229,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test }; // Act - var isBound = await shimBinder.BindModelAsync(bindingContext); + var result = await shimBinder.BindModelAsync(bindingContext); // Assert - Assert.False(isBound); - Assert.Null(bindingContext.Model); + Assert.Null(result); } [Fact] @@ -273,11 +249,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var bindingContext = CreateBindingContext(binder, valueProvider, typeof(SimplePropertiesModel)); // Act - var isBound = await binder.BindModelAsync(bindingContext); + var result = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(isBound); - var model = Assert.IsType(bindingContext.Model); + Assert.NotNull(result); + var model = Assert.IsType(result.Model); Assert.Equal("firstName-value", model.FirstName); Assert.Equal("lastName-value", model.LastName); } @@ -302,11 +278,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var bindingContext = CreateBindingContext(binder, valueProvider, typeof(Person)); // Act - var isBound = await binder.BindModelAsync(bindingContext); + var result = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(isBound); - var model = Assert.IsType(bindingContext.Model); + Assert.NotNull(result); + var model = Assert.IsType(result.Model); Assert.Equal("firstName-value", model.FirstName); Assert.Equal("lastName-value", model.LastName); Assert.Equal(2, model.Friends.Count); @@ -319,81 +295,6 @@ 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_WithDefaultValidators_ValidatesSubProperties() - { - // Arrange - var validatorProvider = new DataAnnotationsModelValidatorProvider(); - var binder = CreateBinderWithDefaults(); - var valueProvider = new SimpleHttpValueProvider - { - { "user.password", "password-val" }, - { "user.confirmpassword", "not-password-val" }, - }; - var bindingContext = CreateBindingContext(binder, valueProvider, typeof(User), validatorProvider); - bindingContext.ModelName = "user"; - - // Act - var isBound = await binder.BindModelAsync(bindingContext); - - // Assert - var error = Assert.Single(bindingContext.ModelState["user.confirmpassword"].Errors); - Assert.Equal("'ConfirmPassword' and 'Password' do not match.", error.ErrorMessage); - } - - [Fact] - public async Task BindModel_WithDefaultValidators_ValidatesInstance() - { - // Arrange - var validatorProvider = new DataAnnotationsModelValidatorProvider(); - var binder = CreateBinderWithDefaults(); - var valueProvider = new SimpleHttpValueProvider - { - { "user.password", "password" }, - { "user.confirmpassword", "password" }, - }; - var bindingContext = CreateBindingContext(binder, valueProvider, typeof(User), validatorProvider); - bindingContext.ModelName = "user"; - - // Act - var isBound = await binder.BindModelAsync(bindingContext); - - // Assert - var error = Assert.Single(bindingContext.ModelState["user"].Errors); - Assert.Equal("Password does not meet complexity requirements.", error.ErrorMessage); - } - - [Fact] - public async Task BindModel_UsesTryAddModelError() - { - // Arrange - var validatorProvider = new DataAnnotationsModelValidatorProvider(); - var binder = CreateBinderWithDefaults(); - var valueProvider = new SimpleHttpValueProvider - { - { "user.password", "password" }, - { "user.confirmpassword", "password2" }, - }; - var bindingContext = CreateBindingContext(binder, valueProvider, typeof(User), validatorProvider); - bindingContext.ModelState.MaxAllowedErrors = 2; - bindingContext.ModelState.AddModelError("key1", "error1"); - bindingContext.ModelName = "user"; - - // Act - await binder.BindModelAsync(bindingContext); - - // Assert - var modelState = bindingContext.ModelState["user.confirmpassword"]; - Assert.Empty(modelState.Errors); - - modelState = bindingContext.ModelState["user"]; - Assert.Empty(modelState.Errors); - - var error = Assert.Single(bindingContext.ModelState[""].Errors); - Assert.IsType(error.Exception); - } - - private static ModelBindingContext CreateBindingContext(IModelBinder binder, IValueProvider valueProvider, Type type, diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/DictionaryModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/DictionaryModelBinderTest.cs index 34b0dda257..c97bbe8eb1 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/DictionaryModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/DictionaryModelBinderTest.cs @@ -34,12 +34,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var binder = new DictionaryModelBinder(); // Act - bool retVal = await binder.BindModelAsync(bindingContext); + var retVal = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(retVal); + Assert.NotNull(retVal); - var dictionary = Assert.IsAssignableFrom>(bindingContext.Model); + var dictionary = Assert.IsAssignableFrom>(retVal.Model); Assert.NotNull(dictionary); Assert.Equal(2, dictionary.Count); Assert.Equal("forty-two", dictionary[42]); @@ -56,10 +56,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var value = await mbc.ValueProvider.GetValueAsync(mbc.ModelName); if (value != null) { - mbc.Model = value.ConvertTo(mbc.ModelType); - return true; + var model = value.ConvertTo(mbc.ModelType); + return new ModelBindingResult(model, key: null, isModelSet: true); } - return false; + return null; }); return mockKvpBinder.Object; } diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/FormCollectionModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/FormCollectionModelBinderTest.cs index e00e99d263..1cbfd052b8 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/FormCollectionModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/FormCollectionModelBinderTest.cs @@ -34,8 +34,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(result); - var form = Assert.IsAssignableFrom(bindingContext.Model); + Assert.NotNull(result); + var form = Assert.IsAssignableFrom(result.Model); Assert.Equal(2, form.Count); Assert.Equal("value1", form["field1"]); Assert.Equal("value2", form["field2"]); @@ -58,7 +58,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(bindingContext); // Assert - Assert.False(result); + Assert.Null(result); } [Fact] @@ -73,9 +73,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(result); - Assert.IsType(typeof(FormCollection), bindingContext.Model); - Assert.Empty((FormCollection)bindingContext.Model); + Assert.NotNull(result); + Assert.IsType(typeof(FormCollection), result.Model); + Assert.Empty((FormCollection)result.Model); } [Fact] @@ -95,8 +95,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(result); - var form = Assert.IsAssignableFrom(bindingContext.Model); + Assert.NotNull(result); + var form = Assert.IsAssignableFrom(result.Model); Assert.Equal(2, form.Count); Assert.Equal("value1", form["field1"]); Assert.Equal("value2", form["field2"]); diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/FormFileModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/FormFileModelBinderTest.cs index 68b2f3232a..24cb4925d9 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/FormFileModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/FormFileModelBinderTest.cs @@ -32,8 +32,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(result); - var files = Assert.IsAssignableFrom>(bindingContext.Model); + Assert.NotNull(result); + var files = Assert.IsAssignableFrom>(result.Model); Assert.Equal(2, files.Count); } @@ -52,8 +52,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(result); - var files = Assert.IsAssignableFrom>(bindingContext.Model); + Assert.NotNull(result); + var files = Assert.IsAssignableFrom>(result.Model); Assert.Equal(2, files.Count); } @@ -72,8 +72,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(result); - var file = Assert.IsAssignableFrom(bindingContext.Model); + Assert.NotNull(result); + var file = Assert.IsAssignableFrom(result.Model); Assert.Equal("form-data; name=file; filename=file1.txt", file.ContentDisposition); } @@ -91,8 +91,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(result); - Assert.Null(bindingContext.Model); + Assert.NotNull(result); + Assert.Null(result.Model); } [Fact] @@ -109,8 +109,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(result); - Assert.Null(bindingContext.Model); + Assert.NotNull(result); + Assert.Null(result.Model); } [Fact] @@ -127,8 +127,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(result); - Assert.Null(bindingContext.Model); + Assert.NotNull(result); + Assert.Null(result.Model); } [Fact] @@ -145,8 +145,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var result = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(result); - Assert.Null(bindingContext.Model); + Assert.NotNull(result); + Assert.Null(result.Model); } private static ModelBindingContext GetBindingContext(Type modelType, HttpContext httpContext) diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/HeaderModelBinderTests.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/HeaderModelBinderTests.cs index aedc02d95d..b96c2171c2 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/HeaderModelBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/HeaderModelBinderTests.cs @@ -27,7 +27,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var result = await binder.BindModelAsync(modelBindingContext); // Assert - Assert.True(result); + Assert.NotNull(result); } [Fact] @@ -47,8 +47,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var result = await binder.BindModelAsync(modelBindingContext); // Assert - Assert.True(result); - Assert.Equal(headerValue.Split(','), modelBindingContext.Model); + Assert.NotNull(result); + Assert.Equal(headerValue.Split(','), result.Model); } [Fact] @@ -68,8 +68,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var result = await binder.BindModelAsync(modelBindingContext); // Assert - Assert.True(result); - Assert.Equal(headerValue, modelBindingContext.Model); + Assert.NotNull(result); + Assert.Equal(headerValue, result.Model); } private static ModelBindingContext GetBindingContext(Type modelType) diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/KeyValuePairModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/KeyValuePairModelBinderTest.cs index 448f606a5e..fa0fb29754 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/KeyValuePairModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/KeyValuePairModelBinderTest.cs @@ -2,7 +2,9 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. #if ASPNET50 +using System; using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; using System.Linq; using System.Threading.Tasks; using Moq; @@ -13,37 +15,70 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test public class KeyValuePairModelBinderTest { [Fact] - public async Task BindModel_MissingKey_ReturnsFalse() + public async Task BindModel_MissingKey_ReturnsTrue_AndAddsModelValidationError() { // Arrange var valueProvider = new SimpleHttpValueProvider(); - var bindingContext = GetBindingContext(valueProvider, Mock.Of()); + + // Create string binder to create the value but not the key. + var bindingContext = GetBindingContext(valueProvider, CreateStringBinder()); var binder = new KeyValuePairModelBinder(); // Act - bool retVal = await binder.BindModelAsync(bindingContext); + var retVal = await binder.BindModelAsync(bindingContext); // Assert - Assert.False(retVal); - Assert.Null(bindingContext.Model); - Assert.Empty(bindingContext.ValidationNode.ChildNodes); + Assert.NotNull(retVal); + Assert.Null(retVal.Model); + Assert.False(bindingContext.ModelState.IsValid); + Assert.Equal("someName", bindingContext.ModelName); + var error = Assert.Single(bindingContext.ModelState["someName.Key"].Errors); + Assert.Equal("A value is required.", error.ErrorMessage); } [Fact] - public async Task BindModel_MissingValue_ReturnsTrue() + public async Task BindModel_MissingValue_ReturnsTrue_AndAddsModelValidationError() { // Arrange var valueProvider = new SimpleHttpValueProvider(); - var bindingContext = GetBindingContext(valueProvider); + + // Create int binder to create the value but not the key. + var bindingContext = GetBindingContext(valueProvider, CreateIntBinder()); var binder = new KeyValuePairModelBinder(); // Act - bool retVal = await binder.BindModelAsync(bindingContext); + var retVal = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(retVal); - Assert.Null(bindingContext.Model); - Assert.Equal(new[] { "someName.key" }, bindingContext.ValidationNode.ChildNodes.Select(n => n.ModelStateKey).ToArray()); + Assert.NotNull(retVal); + Assert.Null(retVal.Model); + Assert.False(bindingContext.ModelState.IsValid); + Assert.Equal("someName", bindingContext.ModelName); + Assert.Equal(bindingContext.ModelState["someName.Value"].Errors.First().ErrorMessage, "A value is required."); + } + + [Fact] + public async Task BindModel_MissingKeyAndMissingValue_DoNotAddModelStateError() + { + // Arrange + var valueProvider = new SimpleHttpValueProvider(); + + // Create int binder to create the value but not the key. + var bindingContext = GetBindingContext(valueProvider); + var mockBinder = new Mock(); + mockBinder.Setup(o => o.BindModelAsync(It.IsAny())) + .Returns(Task.FromResult(null)); + + bindingContext.OperationBindingContext.ModelBinder = mockBinder.Object; + var binder = new KeyValuePairModelBinder(); + + // Act + var retVal = await binder.BindModelAsync(bindingContext); + + // Assert + Assert.Null(retVal); + Assert.True(bindingContext.ModelState.IsValid); + Assert.Equal(0, bindingContext.ModelState.ErrorCount); } [Fact] @@ -60,9 +95,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var retVal = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(retVal); - Assert.Equal(new KeyValuePair(42, "some-value"), bindingContext.Model); - Assert.Equal(new[] { "someName.key", "someName.value" }, bindingContext.ValidationNode.ChildNodes.Select(n => n.ModelStateKey).ToArray()); + Assert.NotNull(retVal); + Assert.Equal(new KeyValuePair(42, "some-value"), retVal.Model); } [Fact] @@ -76,9 +110,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var retVal = await binder.TryBindStrongModel(bindingContext, "key"); // Assert - Assert.True(retVal.Success); + Assert.True(retVal.IsModelSet); Assert.Equal(42, retVal.Model); - Assert.Single(bindingContext.ValidationNode.ChildNodes); Assert.Empty(bindingContext.ModelState); } @@ -92,7 +125,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test .Returns((ModelBindingContext mbc) => { Assert.Equal("someName.key", mbc.ModelName); - return Task.FromResult(true); + return Task.FromResult(new ModelBindingResult(null, string.Empty, true)); }); var bindingContext = GetBindingContext(new SimpleHttpValueProvider(), innerBinder.Object); @@ -103,25 +136,27 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var retVal = await binder.TryBindStrongModel(bindingContext, "key"); // Assert - Assert.True(retVal.Success); - Assert.Equal(default(int), retVal.Model); - Assert.Single(bindingContext.ValidationNode.ChildNodes); + Assert.True(retVal.IsModelSet); + Assert.Null(retVal.Model); Assert.Empty(bindingContext.ModelState); } - private static ModelBindingContext GetBindingContext(IValueProvider valueProvider, IModelBinder innerBinder = null) + private static ModelBindingContext GetBindingContext( + IValueProvider valueProvider, + IModelBinder innerBinder = null, + Type keyValuePairType = null) { var metataProvider = new EmptyModelMetadataProvider(); var bindingContext = new ModelBindingContext { - ModelMetadata = metataProvider.GetMetadataForType(null, typeof(KeyValuePair)), + ModelMetadata = metataProvider.GetMetadataForType(null, keyValuePairType ?? typeof(KeyValuePair)), ModelName = "someName", ValueProvider = valueProvider, OperationBindingContext = new OperationBindingContext { ModelBinder = innerBinder ?? CreateIntBinder(), MetadataProvider = metataProvider, - ValidatorProvider = Mock.Of() + ValidatorProvider = new DataAnnotationsModelValidatorProvider() } }; return bindingContext; @@ -136,10 +171,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test { if (mbc.ModelType == typeof(int)) { - mbc.Model = 42; - return Task.FromResult(true); + return Task.FromResult(new ModelBindingResult(42, mbc.ModelName, true)); } - return Task.FromResult(false); + return Task.FromResult(null); }); return mockIntBinder.Object; } @@ -153,10 +187,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test { if (mbc.ModelType == typeof(string)) { - mbc.Model = "some-value"; - return Task.FromResult(true); + return Task.FromResult(new ModelBindingResult("some-value", mbc.ModelName, true)); } - return Task.FromResult(false); + return Task.FromResult(null); }); return mockStringBinder.Object; } diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ModelBindingContextTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ModelBindingContextTest.cs index 925f9fed1f..9d4996c41a 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ModelBindingContextTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ModelBindingContextTest.cs @@ -34,19 +34,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test } [Fact] - public void ModelProperty_ThrowsIfModelMetadataDoesNotExist() - { - // Arrange - var bindingContext = new ModelBindingContext(); - - // Act & assert - ExceptionAssert.Throws( - () => bindingContext.Model = null, - "The ModelMetadata property must be set before accessing this property."); - } - - [Fact] - public void ModelAndModelTypeAreFedFromModelMetadata() + public void ModelTypeAreFedFromModelMetadata() { // Act var bindingContext = new ModelBindingContext @@ -55,27 +43,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test }; // Assert - Assert.Equal(42, bindingContext.Model); Assert.Equal(typeof(int), bindingContext.ModelType); } - - [Fact] - public void ValidationNodeProperty_DefaultValues() - { - // Act - var bindingContext = new ModelBindingContext - { - ModelMetadata = new EmptyModelMetadataProvider().GetMetadataForType(() => 42, typeof(int)), - ModelName = "theInt" - }; - - // Act - var validationNode = bindingContext.ValidationNode; - - // Assert - Assert.NotNull(validationNode); - Assert.Equal(bindingContext.ModelMetadata, validationNode.ModelMetadata); - Assert.Equal(bindingContext.ModelName, validationNode.ModelStateKey); - } } } diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ModelBindingResultTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ModelBindingResultTest.cs new file mode 100644 index 0000000000..14b2d8622f --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/ModelBindingResultTest.cs @@ -0,0 +1,27 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Xunit; + +namespace Microsoft.AspNet.Mvc.ModelBinding.Test +{ + public class ModelBindingResultTest + { + [Fact] + public void Constructor_SetsProperties() + { + // Arrange + + // Act + var result = new ModelBindingResult( + "some string", + isModelSet: true, + key: "someName"); + + // Assert + Assert.Equal("some string", result.Model); + Assert.True(result.IsModelSet); + Assert.Equal("someName", result.Key); + } + } +} diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/MutableObjectModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/MutableObjectModelBinderTest.cs index 1801c8ae5a..8d11e9ab1c 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/MutableObjectModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/MutableObjectModelBinderTest.cs @@ -359,7 +359,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding .Returns((ModelBindingContext mbc) => { // just return the DTO unchanged - return Task.FromResult(true); + return Task.FromResult(new ModelBindingResult(mbc.ModelMetadata.Model, mbc.ModelName, true)); }); var testableBinder = new Mock { CallBase = true }; @@ -371,9 +371,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var retValue = await testableBinder.Object.BindModelAsync(bindingContext); // Assert - Assert.True(retValue); - Assert.IsType(bindingContext.Model); - Assert.True(bindingContext.ValidationNode.ValidateAllProperties); + Assert.NotNull(retValue); + Assert.True(retValue.IsModelSet); + Assert.NotNull(retValue.Model); + Assert.IsType(retValue.Model); testableBinder.Verify(); } @@ -404,7 +405,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding .Returns((ModelBindingContext mbc) => { // just return the DTO unchanged - return Task.FromResult(true); + return Task.FromResult(new ModelBindingResult(mbc.ModelMetadata.Model, mbc.ModelName, true)); }); var testableBinder = new Mock { CallBase = true }; @@ -416,9 +417,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var retValue = await testableBinder.Object.BindModelAsync(bindingContext); // Assert - Assert.True(retValue); - Assert.IsType(bindingContext.Model); - Assert.True(bindingContext.ValidationNode.ValidateAllProperties); + Assert.NotNull(retValue); + Assert.True(retValue.IsModelSet); + Assert.NotNull(retValue.Model); + Assert.IsType(retValue.Model); testableBinder.Verify(); } @@ -517,9 +519,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var testableBinder = new Mock { CallBase = true }; // Act - var originalModel = bindingContext.Model; + var originalModel = bindingContext.ModelMetadata.Model; testableBinder.Object.EnsureModelPublic(bindingContext); - var newModel = bindingContext.Model; + var newModel = bindingContext.ModelMetadata.Model; // Assert Assert.Same(originalModel, newModel); @@ -540,9 +542,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding .Returns(new Person()).Verifiable(); // Act - var originalModel = bindingContext.Model; + var originalModel = bindingContext.ModelMetadata.Model; testableBinder.Object.EnsureModelPublic(bindingContext); - var newModel = bindingContext.Model; + var newModel = bindingContext.ModelMetadata.Model; // Assert Assert.Null(originalModel); @@ -685,53 +687,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding Assert.Equal(new[] { "Never" }, validationInfo.SkipProperties); } - [Fact] - public void NullCheckFailedHandler_ModelStateAlreadyInvalid_DoesNothing() - { - // Arrange - var modelState = new ModelStateDictionary(); - modelState.AddModelError("foo.bar", "Some existing error."); - - var modelMetadata = GetMetadataForType(typeof(Person)); - var validationNode = new ModelValidationNode(modelMetadata, "foo"); - var validationContext = new ModelValidationContext(new DataAnnotationsModelMetadataProvider(), - Mock.Of(), - modelState, - modelMetadata, - null); - var e = new ModelValidatedEventArgs(validationContext, parentNode: null); - - // Act - var handler = MutableObjectModelBinder.CreateNullCheckFailedHandler(modelMetadata, incomingValue: null); - handler(validationNode, e); - - // Assert - Assert.False(modelState.ContainsKey("foo")); - } - - [Fact] - public void NullCheckFailedHandler_ModelStateValid_AddsErrorString() - { - // Arrange - var modelState = new ModelStateDictionary(); - var modelMetadata = GetMetadataForType(typeof(Person)); - var validationNode = new ModelValidationNode(modelMetadata, "foo"); - var validationContext = new ModelValidationContext(new DataAnnotationsModelMetadataProvider(), - Mock.Of(), - modelState, - modelMetadata, - null); - var e = new ModelValidatedEventArgs(validationContext, parentNode: null); - - // Act - var handler = MutableObjectModelBinder.CreateNullCheckFailedHandler(modelMetadata, incomingValue: null); - handler(validationNode, e); - - // Assert - Assert.True(modelState.ContainsKey("foo")); - Assert.Equal("A value is required.", modelState["foo"].Errors[0].ErrorMessage); - } - [Fact] [ReplaceCulture] public void ProcessDto_BindRequiredFieldMissing_RaisesModelError() @@ -756,10 +711,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties); var nameProperty = dto.PropertyMetadata.Single(o => o.PropertyName == "Name"); - dto.Results[nameProperty] = new ComplexModelDtoResult( + dto.Results[nameProperty] = new ModelBindingResult( "John Doe", - isModelBound: true, - validationNode: new ModelValidationNode(nameProperty, "")); + isModelSet: true, + key: ""); var testableBinder = new TestableMutableObjectModelBinder(); @@ -802,7 +757,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding ValidatorProvider = Mock.Of() } }; - var validationContext = new ModelValidationContext(new EmptyModelMetadataProvider(), + var validationContext = new ModelValidationContext("theModel", bindingContext.OperationBindingContext .ValidatorProvider, bindingContext.ModelState, @@ -813,35 +768,29 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var testableBinder = new TestableMutableObjectModelBinder(); var propertyMetadata = dto.PropertyMetadata.Single(o => o.PropertyName == "Name"); - dto.Results[propertyMetadata] = new ComplexModelDtoResult( + dto.Results[propertyMetadata] = new ModelBindingResult( "John Doe", - isModelBound: true, - validationNode: new ModelValidationNode(propertyMetadata, "theModel.Name")); + isModelSet: true, + key: "theModel.Name"); // Attempt to set non-Nullable property to null. BindRequiredAttribute should not be relevant in this // case because the binding exists. propertyMetadata = dto.PropertyMetadata.Single(o => o.PropertyName == "Age"); - dto.Results[propertyMetadata] = new ComplexModelDtoResult( + dto.Results[propertyMetadata] = new ModelBindingResult( null, - isModelBound: true, - validationNode: new ModelValidationNode(propertyMetadata, "theModel.Age")); + isModelSet: true, + key: "theModel.Age"); - // Act; must also Validate because null-check error handler is late-bound + // Act testableBinder.ProcessDto(bindingContext, dto); - bindingContext.ValidationNode.Validate(validationContext); // Assert var modelStateDictionary = bindingContext.ModelState; Assert.False(modelStateDictionary.IsValid); - Assert.Equal(2, modelStateDictionary.Count); - - // Check Name field - ModelState modelState; - Assert.True(modelStateDictionary.TryGetValue("theModel.Name", out modelState)); - Assert.Equal(0, modelState.Errors.Count); - Assert.Equal(ModelValidationState.Valid, modelState.ValidationState); + Assert.Equal(1, modelStateDictionary.Count); // Check Age error. + ModelState modelState; Assert.True(modelStateDictionary.TryGetValue("theModel.Age", out modelState)); Assert.Equal(ModelValidationState.Invalid, modelState.ValidationState); @@ -906,16 +855,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Make Age valid and City invalid. var propertyMetadata = dto.PropertyMetadata.Single(p => p.PropertyName == "Age"); - dto.Results[propertyMetadata] = new ComplexModelDtoResult( + dto.Results[propertyMetadata] = new ModelBindingResult( 23, - isModelBound: true, - validationNode: new ModelValidationNode(propertyMetadata, "theModel.Age")); + isModelSet: true, + key: "theModel.Age"); propertyMetadata = dto.PropertyMetadata.Single(p => p.PropertyName == "City"); - dto.Results[propertyMetadata] = new ComplexModelDtoResult( + dto.Results[propertyMetadata] = new ModelBindingResult( null, - isModelBound: true, - validationNode: new ModelValidationNode(propertyMetadata, "theModel.City")); + isModelSet: true, + key: "theModel.City"); // Act testableBinder.ProcessDto(bindingContext, dto); @@ -980,10 +929,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Make ValueTypeRequired invalid. var propertyMetadata = dto.PropertyMetadata.Single(p => p.PropertyName == "ValueTypeRequired"); - dto.Results[propertyMetadata] = new ComplexModelDtoResult( + dto.Results[propertyMetadata] = new ModelBindingResult( null, - isModelBound: true, - validationNode: new ModelValidationNode(propertyMetadata, "theModel.ValueTypeRequired")); + isModelSet: true, + key: "theModel.ValueTypeRequired"); // Act testableBinder.ProcessDto(bindingContext, dto); @@ -1018,16 +967,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties); var firstNameProperty = dto.PropertyMetadata.Single(o => o.PropertyName == "FirstName"); - dto.Results[firstNameProperty] = new ComplexModelDtoResult( + dto.Results[firstNameProperty] = new ModelBindingResult( "John", - isModelBound: true, - validationNode: new ModelValidationNode(firstNameProperty, "")); + isModelSet: true, + key: ""); var lastNameProperty = dto.PropertyMetadata.Single(o => o.PropertyName == "LastName"); - dto.Results[lastNameProperty] = new ComplexModelDtoResult( + dto.Results[lastNameProperty] = new ModelBindingResult( "Doe", - isModelBound: true, - validationNode: new ModelValidationNode(lastNameProperty, "")); + isModelSet: true, + key: ""); var dobProperty = dto.PropertyMetadata.Single(o => o.PropertyName == "DateOfBirth"); dto.Results[dobProperty] = null; @@ -1051,12 +1000,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var bindingContext = CreateContext(GetMetadataForObject(new Person())); var propertyMetadata = bindingContext.ModelMetadata.Properties.First(o => o.PropertyName == "PropertyWithDefaultValue"); - var validationNode = new ModelValidationNode(propertyMetadata, "foo"); - var dtoResult = new ComplexModelDtoResult( + var dtoResult = new ModelBindingResult( model: null, - isModelBound: false, - validationNode: validationNode); + isModelSet: false, + key: "foo"); var requiredValidator = bindingContext.OperationBindingContext .ValidatorProvider @@ -1069,7 +1017,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding testableBinder.SetProperty(bindingContext, propertyMetadata, dtoResult, requiredValidator); // Assert - var person = Assert.IsType(bindingContext.Model); + var person = Assert.IsType(bindingContext.ModelMetadata.Model); Assert.Equal(123.456m, person.PropertyWithDefaultValue); Assert.True(bindingContext.ModelState.IsValid); } @@ -1082,13 +1030,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var propertyMetadata = bindingContext.ModelMetadata.Properties.Single( o => o.PropertyName == "PropertyWithInitializedValue"); - var validationNode = new ModelValidationNode(propertyMetadata, "foo"); // This value won't be used because IsModelBound = false. - var dtoResult = new ComplexModelDtoResult( + var dtoResult = new ModelBindingResult( model: "bad-value", - isModelBound: false, - validationNode: validationNode); + isModelSet: false, + key: "foo"); var testableBinder = new TestableMutableObjectModelBinder(); @@ -1096,7 +1043,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding testableBinder.SetProperty(bindingContext, propertyMetadata, dtoResult, requiredValidator: null); // Assert - var person = Assert.IsType(bindingContext.Model); + var person = Assert.IsType(bindingContext.ModelMetadata.Model); Assert.Equal("preinitialized", person.PropertyWithInitializedValue); Assert.True(bindingContext.ModelState.IsValid); } @@ -1109,13 +1056,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var propertyMetadata = bindingContext.ModelMetadata.Properties.Single( o => o.PropertyName == "PropertyWithInitializedValueAndDefault"); - var validationNode = new ModelValidationNode(propertyMetadata, "foo"); // This value won't be used because IsModelBound = false. - var dtoResult = new ComplexModelDtoResult( + var dtoResult = new ModelBindingResult( model: "bad-value", - isModelBound: false, - validationNode: validationNode); + isModelSet: false, + key: "foo"); var testableBinder = new TestableMutableObjectModelBinder(); @@ -1123,7 +1069,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding testableBinder.SetProperty(bindingContext, propertyMetadata, dtoResult, requiredValidator: null); // Assert - var person = Assert.IsType(bindingContext.Model); + var person = Assert.IsType(bindingContext.ModelMetadata.Model); Assert.Equal("default", person.PropertyWithInitializedValueAndDefault); Assert.True(bindingContext.ModelState.IsValid); } @@ -1134,12 +1080,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Arrange var bindingContext = CreateContext(GetMetadataForObject(new Person())); var propertyMetadata = bindingContext.ModelMetadata.Properties.Single(o => o.PropertyName == "NonUpdateableProperty"); - var validationNode = new ModelValidationNode(propertyMetadata, "foo"); - var dtoResult = new ComplexModelDtoResult( + var dtoResult = new ModelBindingResult( model: null, - isModelBound: false, - validationNode: validationNode); + isModelSet: false, + key: "foo"); var testableBinder = new TestableMutableObjectModelBinder(); @@ -1158,12 +1103,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var bindingContext = CreateContext(GetMetadataForObject(model)); var propertyMetadata = bindingContext.ModelMetadata.Properties.Single(o => o.PropertyName == "DateOfBirth"); - var validationNode = new ModelValidationNode(propertyMetadata, "foo"); - var dtoResult = new ComplexModelDtoResult( + var dtoResult = new ModelBindingResult( new DateTime(2001, 1, 1), - isModelBound: true, - validationNode: validationNode); + key: "foo", + isModelSet: true); + var requiredValidator = bindingContext.OperationBindingContext .ValidatorProvider @@ -1177,7 +1122,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding testableBinder.SetProperty(bindingContext, propertyMetadata, dtoResult, requiredValidator); // Assert - validationNode.Validate(validationContext); Assert.True(bindingContext.ModelState.IsValid); Assert.Equal(new DateTime(2001, 1, 1), model.DateOfBirth); } @@ -1194,11 +1138,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var bindingContext = CreateContext(GetMetadataForObject(model)); var propertyMetadata = bindingContext.ModelMetadata.Properties.Single(o => o.PropertyName == "DateOfDeath"); - var validationNode = new ModelValidationNode(propertyMetadata, "foo"); - var dtoResult = new ComplexModelDtoResult( + var dtoResult = new ModelBindingResult( new DateTime(1800, 1, 1), - isModelBound: true, - validationNode: validationNode); + isModelSet: true, + key: "foo"); var testableBinder = new TestableMutableObjectModelBinder(); @@ -1218,11 +1161,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var bindingContext = CreateContext(GetMetadataForObject(new Person())); var propertyMetadata = bindingContext.ModelMetadata.Properties.Single(o => o.PropertyName == "DateOfBirth"); - var validationNode = new ModelValidationNode(propertyMetadata, "foo"); - var dtoResult = new ComplexModelDtoResult( + var dtoResult = new ModelBindingResult( model: null, - isModelBound: true, - validationNode: validationNode); + isModelSet: true, + key: "foo"); var requiredValidator = GetRequiredValidator(bindingContext, propertyMetadata); var validationContext = new ModelValidationContext(bindingContext, propertyMetadata); @@ -1233,8 +1175,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding testableBinder.SetProperty(bindingContext, propertyMetadata, dtoResult, requiredValidator); // Assert - Assert.True(bindingContext.ModelState.IsValid); - validationNode.Validate(validationContext, bindingContext.ValidationNode); Assert.False(bindingContext.ModelState.IsValid); } @@ -1246,11 +1186,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding bindingContext.ModelName = " foo"; var propertyMetadata = bindingContext.ModelMetadata.Properties.Single(o => o.PropertyName == "ValueTypeRequired"); - var validationNode = new ModelValidationNode(propertyMetadata, "foo.ValueTypeRequired"); - var dtoResult = new ComplexModelDtoResult( + var dtoResult = new ModelBindingResult( model: null, - isModelBound: true, - validationNode: validationNode); + isModelSet: true, + key: "foo.ValueTypeRequired"); var requiredValidator = GetRequiredValidator(bindingContext, propertyMetadata); @@ -1273,11 +1212,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding bindingContext.ModelName = "foo"; var propertyMetadata = bindingContext.ModelMetadata.Properties.Single(o => o.PropertyName == "NameNoAttribute"); - var validationNode = new ModelValidationNode(propertyMetadata, "foo.NameNoAttribute"); - var dtoResult = new ComplexModelDtoResult( + var dtoResult = new ModelBindingResult( model: null, - isModelBound: true, - validationNode: validationNode); + isModelSet: true, + key: "foo.NameNoAttribute"); var requiredValidator = GetRequiredValidator(bindingContext, propertyMetadata); @@ -1302,10 +1240,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding bindingContext.ModelName = "foo"; var propertyMetadata = bindingContext.ModelMetadata.Properties.Single(o => o.PropertyName == "Name"); - var validationNode = new ModelValidationNode(propertyMetadata, "foo.Name"); - var dtoResult = new ComplexModelDtoResult(model: null, - isModelBound: true, - validationNode: validationNode); + var dtoResult = new ModelBindingResult(model: null, + isModelSet: true, + key: "foo.Name"); var requiredValidator = GetRequiredValidator(bindingContext, propertyMetadata); @@ -1613,7 +1550,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public new void SetProperty(ModelBindingContext bindingContext, ModelMetadata propertyMetadata, - ComplexModelDtoResult dtoResult, + ModelBindingResult dtoResult, IModelValidator requiredValidator) { base.SetProperty(bindingContext, propertyMetadata, dtoResult, requiredValidator); diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/TypeConverterModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/TypeConverterModelBinderTest.cs index d69ac40bb6..6e1d87ad8d 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/TypeConverterModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/TypeConverterModelBinderTest.cs @@ -30,7 +30,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var retVal = await binder.BindModelAsync(bindingContext); // Assert - Assert.False(retVal); + Assert.Null(retVal); } [Theory] @@ -64,7 +64,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var retVal = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(retVal); + Assert.NotNull(retVal); } [Fact] @@ -84,8 +84,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var retVal = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(retVal); - Assert.Null(bindingContext.Model); + Assert.NotNull(retVal); + Assert.Null(retVal.Model); Assert.False(bindingContext.ModelState.IsValid); var error = Assert.Single(bindingContext.ModelState["theModelName"].Errors); Assert.Equal(message, error.ErrorMessage); @@ -102,7 +102,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var retVal = await binder.BindModelAsync(bindingContext); // Assert - Assert.False(retVal, "BindModel should have returned null."); + Assert.Null(retVal); Assert.Empty(bindingContext.ModelState); } @@ -122,8 +122,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var retVal = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(retVal); - Assert.Null(bindingContext.Model); + Assert.NotNull(retVal); + Assert.Null(retVal.Model); Assert.True(bindingContext.ModelState.ContainsKey("theModelName")); } @@ -143,8 +143,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var retVal = await binder.BindModelAsync(bindingContext); // Assert - Assert.True(retVal); - Assert.Equal(42, bindingContext.Model); + Assert.NotNull(retVal); + Assert.Equal(42, retVal.Model);; Assert.True(bindingContext.ModelState.ContainsKey("theModelName")); } diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/DefaultBodyModelValidatorTests.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/DefaultObjectValidatorTests.cs similarity index 61% rename from test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/DefaultBodyModelValidatorTests.cs rename to test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/DefaultObjectValidatorTests.cs index 67a811f600..bd96c5f0f6 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/DefaultBodyModelValidatorTests.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/DefaultObjectValidatorTests.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.ComponentModel.DataAnnotations; +using System.Globalization; using System.IO; using System.Linq; using System.Xml; @@ -15,11 +16,11 @@ using Xunit; namespace Microsoft.AspNet.Mvc.ModelBinding { - public class DefaultBodyModelValidatorTests + public class DefaultObjectValidatorTests { private static Person LonelyPerson; - static DefaultBodyModelValidatorTests() + static DefaultObjectValidatorTests() { LonelyPerson = new Person() { Name = "Reallllllllly Long Name" }; LonelyPerson.Friend = LonelyPerson; @@ -212,14 +213,17 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public void ExpectedValidationErrorsRaised(object model, Type type, Dictionary expectedErrors) { // Arrange - var validationContext = GetModelValidationContext(model, type); + var testValidationContext = GetModelValidationContext(model, type); // Act (does not throw) - new DefaultBodyModelValidator().Validate(validationContext, keyPrefix: string.Empty); + new DefaultObjectValidator( + testValidationContext.ExcludeFiltersProvider, + testValidationContext.ModelMetadataProvider) + .Validate(testValidationContext.ModelValidationContext); // Assert var actualErrors = new Dictionary(); - foreach (var keyStatePair in validationContext.ModelState) + foreach (var keyStatePair in testValidationContext.ModelValidationContext.ModelState) { foreach (var error in keyStatePair.Value.Errors) { @@ -237,17 +241,20 @@ namespace Microsoft.AspNet.Mvc.ModelBinding [Fact] [ReplaceCulture] - public void BodyValidator_Throws_IfPropertyAccessorThrows() + public void Validator_Throws_IfPropertyAccessorThrows() { // Arrange - var validationContext = GetModelValidationContext(new Uri("/api/values", UriKind.Relative), typeof(Uri)); + var testValidationContext = GetModelValidationContext(new Uri("/api/values", UriKind.Relative), typeof(Uri)); // Act & Assert Assert.Throws( typeof(InvalidOperationException), () => { - new DefaultBodyModelValidator().Validate(validationContext, keyPrefix: string.Empty); + new DefaultObjectValidator( + testValidationContext.ExcludeFiltersProvider, + testValidationContext.ModelMetadataProvider) + .Validate(testValidationContext.ModelValidationContext); }); } @@ -270,30 +277,37 @@ namespace Microsoft.AspNet.Mvc.ModelBinding [Theory] [MemberData(nameof(ObjectsWithPropertiesWhichThrowOnGet))] [ReplaceCulture] - public void BodyValidator_DoesNotThrow_IfExcludedPropertyAccessorsThrow( + public void Validator_DoesNotThrow_IfExcludedPropertyAccessorsThrow( object input, Type type, List excludedTypes) { // Arrange - var validationContext = GetModelValidationContext(input, type, excludedTypes); + var testValidationContext = GetModelValidationContext(input, type, string.Empty, excludedTypes); // Act & Assert (does not throw) - new DefaultBodyModelValidator().Validate(validationContext, keyPrefix: string.Empty); - Assert.True(validationContext.ModelState.IsValid); + new DefaultObjectValidator( + testValidationContext.ExcludeFiltersProvider, + testValidationContext.ModelMetadataProvider) + .Validate(testValidationContext.ModelValidationContext); + Assert.True(testValidationContext.ModelValidationContext.ModelState.IsValid); } [Fact] [ReplaceCulture] - public void BodyValidator_Throws_IfPropertyGetterThrows() + public void Validator_Throws_IfPropertyGetterThrows() { // Arrange - var validationContext = GetModelValidationContext( - new Uri("/api/values", UriKind.Relative), typeof(Uri), new List()); + var testValidationContext = GetModelValidationContext( + new Uri("/api/values", UriKind.Relative), typeof(Uri)); + var validationContext = testValidationContext.ModelValidationContext; // Act & Assert Assert.Throws( () => { - new DefaultBodyModelValidator().Validate(validationContext, keyPrefix: string.Empty); + new DefaultObjectValidator( + testValidationContext.ExcludeFiltersProvider, + testValidationContext.ModelMetadataProvider) + .Validate(validationContext); }); Assert.True(validationContext.ModelState.IsValid); } @@ -304,10 +318,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { // Arrange var model = new Address() { Street = "Microsoft Way" }; - var validationContext = GetModelValidationContext(model, model.GetType()); + var testValidationContext = GetModelValidationContext(model, model.GetType()); + var validationContext = testValidationContext.ModelValidationContext; // Act (does not throw) - new DefaultBodyModelValidator().Validate(validationContext, keyPrefix: string.Empty); + new DefaultObjectValidator( + testValidationContext.ExcludeFiltersProvider, + testValidationContext.ModelMetadataProvider) + .Validate(validationContext); // Assert Assert.Contains("Street", validationContext.ModelState.Keys); @@ -326,14 +344,146 @@ namespace Microsoft.AspNet.Mvc.ModelBinding new TypeThatOverridesEquals { Funny = "hehe" }, new TypeThatOverridesEquals { Funny = "hehe" } }; - var validationContext = GetModelValidationContext(instance, typeof(TypeThatOverridesEquals[])); + var testValidationContext = GetModelValidationContext(instance, typeof(TypeThatOverridesEquals[])); // Act & Assert (does not throw) - new DefaultBodyModelValidator().Validate(validationContext, keyPrefix: string.Empty); + new DefaultObjectValidator( + testValidationContext.ExcludeFiltersProvider, + testValidationContext.ModelMetadataProvider) + .Validate(testValidationContext.ModelValidationContext); } - private ModelValidationContext GetModelValidationContext( - object model, Type type, List excludedTypes = null) + [Fact] + public void Validation_ShortCircuit_WhenMaxErrorCountIsSet() + { + // Arrange + var user = new User() + { + Password = "password-val", + ConfirmPassword = "not-password-val" + }; + + var testValidationContext = GetModelValidationContext( + user, + typeof(User), + "user", + new List { typeof(string) }); + + var validationContext = testValidationContext.ModelValidationContext; + validationContext.ModelState.MaxAllowedErrors = 2; + validationContext.ModelState.AddModelError("key1", "error1"); + var validator = new DefaultObjectValidator( + testValidationContext.ExcludeFiltersProvider, + testValidationContext.ModelMetadataProvider); + + // Act + validator.Validate(validationContext); + + // Assert + Assert.Equal(new[] { "key1", "user.Password", "", "user.ConfirmPassword" }, + validationContext.ModelState.Keys.ToArray()); + 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); + } + + [Fact] + public void ForExcludedNonModelBoundType_Properties_NotMarkedAsSkiped() + { + // Arrange + var user = new User() + { + Password = "password-val", + ConfirmPassword = "not-password-val" + }; + + var testValidationContext = GetModelValidationContext( + user, + typeof(User), + "user", + new List { typeof(User) }); + var validationContext = testValidationContext.ModelValidationContext; + var validator = new DefaultObjectValidator( + testValidationContext.ExcludeFiltersProvider, + testValidationContext.ModelMetadataProvider); + + // Act + validator.Validate(validationContext); + + // Assert + Assert.False(validationContext.ModelState.ContainsKey("user.Password")); + Assert.False(validationContext.ModelState.ContainsKey("user.ConfirmPassword")); + var modelState = validationContext.ModelState["user"]; + Assert.Empty(modelState.Errors); + Assert.Equal(modelState.ValidationState, ModelValidationState.Valid); + } + + [Fact] + public void ForExcludedModelBoundTypes_Properties_MarkedAsSkipped() + { + // Arrange + var user = new User() + { + Password = "password-val", + ConfirmPassword = "not-password-val" + }; + + var testValidationContext = GetModelValidationContext( + user, + typeof(User), + "user", + new List { typeof(User) }); + var validationContext = testValidationContext.ModelValidationContext; + + // Set the value on model state as a model binder would. + validationContext.ModelState.SetModelValue( + "user.Password", + Mock.Of()); + var validator = new DefaultObjectValidator( + testValidationContext.ExcludeFiltersProvider, + testValidationContext.ModelMetadataProvider); + + // Act + validator.Validate(validationContext); + + // Assert + var modelState = validationContext.ModelState["user.Password"]; + Assert.Empty(modelState.Errors); + Assert.Equal(modelState.ValidationState, ModelValidationState.Skipped); + + modelState = validationContext.ModelState["user.ConfirmPassword"]; + Assert.Empty(modelState.Errors); + Assert.Equal(modelState.ValidationState, ModelValidationState.Skipped); + } + + [Fact] + public void NonRequestBoundModel_MarkedAsSkipped() + { + // Arrange + var testValidationContext = GetModelValidationContext( + new TestServiceProvider(), + typeof(TestServiceProvider), + "serviceProvider"); + + var validationContext = testValidationContext.ModelValidationContext; + var validator = new DefaultObjectValidator( + testValidationContext.ExcludeFiltersProvider, + testValidationContext.ModelMetadataProvider); + + // Act + validator.Validate(validationContext); + + // Assert + var modelState = validationContext.ModelState["serviceProvider.TestService"]; + Assert.Empty(modelState.Errors); + Assert.Equal(modelState.ValidationState, ModelValidationState.Skipped); + } + + private TestModelValidationContext GetModelValidationContext( + object model, Type type, string key = "", List excludedTypes = null) { var modelStateDictionary = new ModelStateDictionary(); var providers = new IModelValidatorProvider[] @@ -341,9 +491,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding new DataAnnotationsModelValidatorProvider(), new DataMemberModelValidatorProvider() }; - var modelMetadataProvider = new EmptyModelMetadataProvider(); + var modelMetadataProvider = new DataAnnotationsModelMetadataProvider(); var excludedValidationTypesPredicate = new List(); + if (excludedTypes != null) { var mockExcludeTypeFilter = new Mock(); @@ -354,18 +505,25 @@ namespace Microsoft.AspNet.Mvc.ModelBinding excludedValidationTypesPredicate.Add(mockExcludeTypeFilter.Object); } - return new ModelValidationContext( - modelMetadataProvider, - new CompositeModelValidatorProvider(providers), - modelStateDictionary, - new ModelMetadata( - provider: modelMetadataProvider, - containerType: typeof(object), - modelAccessor: () => model, - modelType: type, - propertyName: null), - containerMetadata: null, - excludeFromValidationFilters: excludedValidationTypesPredicate); + var mockValidationExcludeFiltersProvider = new Mock(); + mockValidationExcludeFiltersProvider.SetupGet(o => o.ExcludeFilters) + .Returns(excludedValidationTypesPredicate); + return new TestModelValidationContext + { + ModelValidationContext = new ModelValidationContext( + key, + new CompositeModelValidatorProvider(providers), + modelStateDictionary, + new ModelMetadata( + provider: modelMetadataProvider, + containerType: typeof(object), + modelAccessor: () => model, + modelType: type, + propertyName: null), + containerMetadata: null), + ModelMetadataProvider = modelMetadataProvider, + ExcludeFiltersProvider = mockValidationExcludeFiltersProvider.Object + }; } public class Person @@ -468,6 +626,42 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public Team Test { get; set; } } + + private class User : IValidatableObject + { + public string Password { get; set; } + + [Compare("Password")] + public string ConfirmPassword { get; set; } + + public IEnumerable Validate(ValidationContext validationContext) + { + if (Password == "password") + { + yield return new ValidationResult("Password does not meet complexity requirements."); + } + } + } + + public class TestServiceProvider + { + [FromServices] + [Required] + public ITestService TestService { get; set; } + } + + public interface ITestService + { + } + + private class TestModelValidationContext + { + public ModelValidationContext ModelValidationContext { get; set; } + + public IModelMetadataProvider ModelMetadataProvider { get; set; } + + public IValidationExcludeFiltersProvider ExcludeFiltersProvider { get; set; } + } } } #endif \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelStateDictionaryTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelStateDictionaryTest.cs index cb7d4897cb..904c19a14f 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelStateDictionaryTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelStateDictionaryTest.cs @@ -9,6 +9,140 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { public class ModelStateDictionaryTest { + [Theory] + [InlineData(ModelValidationState.Valid)] + [InlineData(ModelValidationState.Unvalidated)] + public void MarkFieldSkipped_MarksFieldAsSkipped_IfStateIsNotInValid(ModelValidationState validationState) + { + // Arrange + var modelState = new ModelState + { + Value = GetValueProviderResult("value"), + ValidationState = validationState + }; + + var source = new ModelStateDictionary + { + { "key", modelState } + }; + + // Act + source.MarkFieldSkipped("key"); + + // Assert + Assert.Equal(ModelValidationState.Skipped, source["key"].ValidationState); + } + + [Fact] + public void MarkFieldSkipped_MarksFieldAsSkipped_IfKeyIsNotPresent() + { + // Arrange + var modelState = new ModelState + { + Value = GetValueProviderResult("value"), + ValidationState = ModelValidationState.Valid + }; + + var source = new ModelStateDictionary + { + }; + + // Act + source.MarkFieldSkipped("key"); + + // Assert + Assert.Equal(0, source.ErrorCount); + Assert.Equal(1, source.Count); + Assert.Equal(ModelValidationState.Skipped, source["key"].ValidationState); + } + + [Fact] + public void MarkFieldSkipped_Throws_IfStateIsInvalid() + { + // Arrange + var modelState = new ModelState + { + Value = GetValueProviderResult("value"), + ValidationState = ModelValidationState.Invalid + }; + + var source = new ModelStateDictionary + { + { "key", modelState } + }; + + // Act + var exception = Assert.Throws(() => source.MarkFieldSkipped("key")); + + // Assert + Assert.Equal( + "A field previously marked invalid should not be marked skipped.", + exception.Message); + } + + [Theory] + [InlineData(ModelValidationState.Skipped)] + [InlineData(ModelValidationState.Unvalidated)] + public void MarkFieldValid_MarksFieldAsValid_IfStateIsNotInvalid(ModelValidationState validationState) + { + // Arrange + var modelState = new ModelState + { + Value = GetValueProviderResult("value"), + ValidationState = validationState + }; + + var source = new ModelStateDictionary + { + { "key", modelState } + }; + + // Act + source.MarkFieldValid("key"); + + // Assert + Assert.Equal(ModelValidationState.Valid, source["key"].ValidationState); + } + + [Fact] + public void MarkFieldValid_MarksFieldAsValid_IfKeyIsNotPresent() + { + // Arrange + var source = new ModelStateDictionary(); + + // Act + source.MarkFieldValid("key"); + + // Assert + Assert.Equal(0, source.ErrorCount); + Assert.Equal(1, source.Count); + Assert.Equal(ModelValidationState.Valid, source["key"].ValidationState); + } + + [Fact] + public void MarkFieldValid_Throws_IfStateIsInvalid() + { + // Arrange + var modelState = new ModelState + { + Value = GetValueProviderResult("value"), + ValidationState = ModelValidationState.Invalid + }; + + var source = new ModelStateDictionary + { + { "key", modelState } + }; + + // Act + var exception = Assert.Throws(() => source.MarkFieldValid("key")); + + // Assert + Assert.Equal( + "A field previously marked invalid should not be marked valid.", + exception.Message); + } + [Fact] public void CopyConstructor_CopiesModelStateData() { @@ -100,6 +234,20 @@ namespace Microsoft.AspNet.Mvc.ModelBinding Assert.Equal(ModelValidationState.Unvalidated, validationState); } + [Fact] + public void GetValidationState_ReturnsValidationStateForKey_IgnoresChildren() + { + // Arrange + var msd = new ModelStateDictionary(); + msd.AddModelError("foo.bar", "error text"); + + // Act + var validationState = msd.GetValidationState("foo"); + + // Assert + Assert.Equal(ModelValidationState.Unvalidated, validationState); + } + [Fact] public void GetFieldValidationState_ReturnsInvalidIfKeyChildContainsErrors() { @@ -193,7 +341,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding }, { "baz", new ModelState { - ValidationState = ModelValidationState.Valid, + ValidationState = ModelValidationState.Skipped, Value = GetValueProviderResult("quux", "bar") } } diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelValidationNodeTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelValidationNodeTest.cs deleted file mode 100644 index 2e952dd08f..0000000000 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Validation/ModelValidationNodeTest.cs +++ /dev/null @@ -1,568 +0,0 @@ -// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using System; -using System.Collections.Generic; -using System.ComponentModel.DataAnnotations; -using System.Linq; -using Microsoft.AspNet.Testing; -using Xunit; - -namespace Microsoft.AspNet.Mvc.ModelBinding -{ - public class ModelValidationNodeTest - { - [Fact] - public void ConstructorSetsCollectionInstance() - { - // Arrange - var metadata = GetModelMetadata(); - var modelStateKey = "someKey"; - var childNodes = new[] - { - new ModelValidationNode(metadata, "someKey0"), - new ModelValidationNode(metadata, "someKey1") - }; - - // Act - var node = new ModelValidationNode(metadata, modelStateKey, childNodes); - - // Assert - Assert.Equal(childNodes, node.ChildNodes.ToArray()); - } - - [Fact] - public void PropertiesAreSet() - { - // Arrange - var metadata = GetModelMetadata(); - var modelStateKey = "someKey"; - - // Act - var node = new ModelValidationNode(metadata, modelStateKey); - - // Assert - Assert.Equal(metadata, node.ModelMetadata); - Assert.Equal(modelStateKey, node.ModelStateKey); - Assert.NotNull(node.ChildNodes); - Assert.Empty(node.ChildNodes); - } - - [Fact] - public void CombineWith() - { - // Arrange - var expected = new[] - { - "Validating parent1.", - "Validating parent2.", - "Validated parent1.", - "Validated parent2." - }; - var log = new List(); - - var allChildNodes = new[] - { - new ModelValidationNode(GetModelMetadata(), "key1"), - new ModelValidationNode(GetModelMetadata(), "key2"), - new ModelValidationNode(GetModelMetadata(), "key3"), - }; - - var parentNode1 = new ModelValidationNode(GetModelMetadata(), "parent1"); - parentNode1.ChildNodes.Add(allChildNodes[0]); - parentNode1.Validating += (sender, e) => log.Add("Validating parent1."); - parentNode1.Validated += (sender, e) => log.Add("Validated parent1."); - - var parentNode2 = new ModelValidationNode(GetModelMetadata(), "parent2"); - parentNode2.ChildNodes.Add(allChildNodes[1]); - parentNode2.ChildNodes.Add(allChildNodes[2]); - parentNode2.Validating += (sender, e) => log.Add("Validating parent2."); - parentNode2.Validated += (sender, e) => log.Add("Validated parent2."); - var context = CreateContext(); - - // Act - parentNode1.CombineWith(parentNode2); - parentNode1.Validate(context); - - // Assert - Assert.Equal(expected, log); - Assert.Equal(allChildNodes, parentNode1.ChildNodes.ToArray()); - } - - [Fact] - public void CombineWith_OtherNodeIsSuppressed_DoesNothing() - { - // Arrange - var log = new List(); - - var allChildNodes = new[] - { - new ModelValidationNode(GetModelMetadata(), "key1"), - new ModelValidationNode(GetModelMetadata(), "key2"), - new ModelValidationNode(GetModelMetadata(), "key3"), - }; - - var expectedChildNodes = new[] - { - allChildNodes[0] - }; - - var parentNode1 = new ModelValidationNode(GetModelMetadata(), "parent1"); - parentNode1.ChildNodes.Add(allChildNodes[0]); - parentNode1.Validating += (sender, e) => log.Add("Validating parent1."); - parentNode1.Validated += (sender, e) => log.Add("Validated parent1."); - - var parentNode2 = new ModelValidationNode(GetModelMetadata(), "parent2"); - parentNode2.ChildNodes.Add(allChildNodes[1]); - parentNode2.ChildNodes.Add(allChildNodes[2]); - parentNode2.Validating += (sender, e) => log.Add("Validating parent2."); - parentNode2.Validated += (sender, e) => log.Add("Validated parent2."); - parentNode2.SuppressValidation = true; - var context = CreateContext(); - - // Act - parentNode1.CombineWith(parentNode2); - parentNode1.Validate(context); - - // Assert - Assert.Equal(new[] { "Validating parent1.", "Validated parent1." }, log.ToArray()); - Assert.Equal(expectedChildNodes, parentNode1.ChildNodes.ToArray()); - } - - [Fact] - public void Validate_Ordering() - { - // Proper order of invocation: - // 1. OnValidating() - // 2. Child validators - // 3. This validator - // 4. OnValidated() - - // Arrange - var expected = new[] - { - "In OnValidating()", - "In LoggingValidatonAttribute.IsValid()", - "In IValidatableObject.Validate()", - "In OnValidated()" - }; - var log = new List(); - var model = new LoggingValidatableObject(log); - var modelMetadata = GetModelMetadata(model); - var provider = new EmptyModelMetadataProvider(); - var childMetadata = provider.GetMetadataForProperty(() => model, model.GetType(), "ValidStringProperty"); - var node = new ModelValidationNode(modelMetadata, "theKey"); - node.Validating += (sender, e) => log.Add("In OnValidating()"); - node.Validated += (sender, e) => log.Add("In OnValidated()"); - node.ChildNodes.Add(new ModelValidationNode(childMetadata, "theKey.ValidStringProperty")); - var context = CreateContext(modelMetadata); - - // Act - node.Validate(context); - - // Assert - Assert.Equal(expected, log); - } - - // Validation order is primarily important when MaxAllowedErrors has been overridden. - [Fact] - public void Validate_OrdersUsingModelMetadata() - { - // Proper order of invocation: - // 1. OnValidating() - // 2. Child validators -- ordered using ModelMetadata.Order. - // 3. OnValidated() - - // Arrange - var expected = new[] - { - "In OnValidating()", - "In LoggingValidatonAttribute.IsValid(OrderedProperty3)", - "In LoggingValidatonAttribute.IsValid(OrderedProperty2)", - "In LoggingValidatonAttribute.IsValid(OrderedProperty1)", - "In LoggingValidatonAttribute.IsValid(Property3)", - "In LoggingValidatonAttribute.IsValid(Property1)", - "In LoggingValidatonAttribute.IsValid(Property2)", - "In LoggingValidatonAttribute.IsValid(LastProperty)", - "In OnValidated()" - }; - - var log = new List(); - var model = new LoggingNonValidatableObject(log); - var provider = new DataAnnotationsModelMetadataProvider(); - var modelMetadata = provider.GetMetadataForType(() => model, model.GetType()); - var node = new ModelValidationNode(modelMetadata, "theKey") - { - ValidateAllProperties = true, - }; - node.Validating += (sender, e) => log.Add("In OnValidating()"); - node.Validated += (sender, e) => log.Add("In OnValidated()"); - var context = CreateContext(modelMetadata, provider); - - // Act - node.Validate(context); - - // Assert - Assert.Equal(expected, log); - } - - [Fact] - public void Validate_ChildNodes_OverridesOrdering() - { - // Proper order of invocation: - // 1. OnValidating() - // 2. Child validators -- ordered using ChildNodes, then ModelMetadata.Order. - // 3. OnValidated() - - // Arrange - var expected = new[] - { - "In OnValidating()", - "In LoggingValidatonAttribute.IsValid(LastProperty)", - "In LoggingValidatonAttribute.IsValid(OrderedProperty3)", - "In LoggingValidatonAttribute.IsValid(OrderedProperty2)", - "In LoggingValidatonAttribute.IsValid(OrderedProperty1)", - "In LoggingValidatonAttribute.IsValid(Property3)", - "In LoggingValidatonAttribute.IsValid(Property1)", - "In LoggingValidatonAttribute.IsValid(Property2)", - "In OnValidated()" - }; - - var log = new List(); - var model = new LoggingNonValidatableObject(log); - var provider = new DataAnnotationsModelMetadataProvider(); - var modelMetadata = provider.GetMetadataForType(() => model, model.GetType()); - var childMetadata = modelMetadata.Properties.FirstOrDefault( - property => property.PropertyName == "LastProperty"); - Assert.NotNull(childMetadata); // Guard - - var node = new ModelValidationNode(modelMetadata, "theKey") - { - ChildNodes = - { - new ModelValidationNode(childMetadata, "theKey.LastProperty") - }, - ValidateAllProperties = true, - }; - node.Validating += (sender, e) => log.Add("In OnValidating()"); - node.Validated += (sender, e) => log.Add("In OnValidated()"); - var context = CreateContext(modelMetadata, provider); - - // Act - node.Validate(context); - - // Assert - Assert.Equal(expected, log); - } - - [Fact] - public void Validate_SkipsRemainingValidationIfModelStateIsInvalid() - { - // Because a property validator fails, the model validator shouldn't run - - // Arrange - var expected = new[] - { - "In OnValidating()", - "In IValidatableObject.Validate()", - "In OnValidated()" - }; - var log = new List(); - var model = new LoggingValidatableObject(log); - var modelMetadata = GetModelMetadata(model); - var provider = new EmptyModelMetadataProvider(); - var childMetadata = provider.GetMetadataForProperty(() => model, - model.GetType(), - "InvalidStringProperty"); - var node = new ModelValidationNode(modelMetadata, "theKey"); - node.ChildNodes.Add(new ModelValidationNode(childMetadata, "theKey.InvalidStringProperty")); - node.Validating += (sender, e) => log.Add("In OnValidating()"); - node.Validated += (sender, e) => log.Add("In OnValidated()"); - var context = CreateContext(modelMetadata); - - // Act - node.Validate(context); - - // Assert - Assert.Equal(expected, log); - Assert.Equal("Sample error message", - context.ModelState["theKey.InvalidStringProperty"].Errors[0].ErrorMessage); - } - - [Fact] - public void Validate_SkipsValidationIfHandlerCancels() - { - // Arrange - var log = new List(); - var model = new LoggingValidatableObject(log); - var modelMetadata = GetModelMetadata(model); - var node = new ModelValidationNode(modelMetadata, "theKey"); - node.Validating += (sender, e) => - { - log.Add("In OnValidating()"); - e.Cancel = true; - }; - node.Validated += (sender, e) => log.Add("In OnValidated()"); - var context = CreateContext(modelMetadata); - - // Act - node.Validate(context); - - // Assert - Assert.Equal(new[] { "In OnValidating()" }, log.ToArray()); - } - - [Fact] - public void Validate_SkipsValidationIfSuppressed() - { - // Arrange - var log = new List(); - var model = new LoggingValidatableObject(log); - var modelMetadata = GetModelMetadata(model); - var node = new ModelValidationNode(modelMetadata, "theKey") - { - SuppressValidation = true - }; - - node.Validating += (sender, e) => log.Add("In OnValidating()"); - node.Validated += (sender, e) => log.Add("In OnValidated()"); - var context = CreateContext(); - - // Act - node.Validate(context); - - // Assert - Assert.Empty(log); - } - - [Fact] - public void Validate_ValidatesIfModelIsNull() - { - // Arrange - var modelMetadata = GetModelMetadata(typeof(ValidateAllPropertiesModel)); - var node = new ModelValidationNode(modelMetadata, "theKey"); - - var context = CreateContext(modelMetadata); - - // Act - node.Validate(context); - - // Assert - var modelState = Assert.Single(context.ModelState); - Assert.Equal("theKey", modelState.Key); - Assert.Equal(ModelValidationState.Invalid, modelState.Value.ValidationState); - - var error = Assert.Single(modelState.Value.Errors); - Assert.Equal("A value is required but was not present in the request.", error.ErrorMessage); - } - - [Fact] - [ReplaceCulture] - public void Validate_ValidateAllProperties_AddsValidationErrors() - { - // Arrange - var model = new ValidateAllPropertiesModel - { - RequiredString = null /* error */, - RangedInt = 0 /* error */, - ValidString = "dog" - }; - - var modelMetadata = GetModelMetadata(model); - var node = new ModelValidationNode(modelMetadata, "theKey") - { - ValidateAllProperties = true - }; - var context = CreateContext(modelMetadata); - context.ModelState.AddModelError("theKey.RequiredString.Dummy", "existing Error Text"); - - // Act - node.Validate(context); - - // Assert - var modelState = context.ModelState["theKey.RequiredString.Dummy"]; - Assert.NotNull(modelState); - var error = Assert.Single(modelState.Errors); - Assert.Equal("existing Error Text", error.ErrorMessage); - - modelState = context.ModelState["theKey.RangedInt"]; - Assert.NotNull(modelState); - error = Assert.Single(modelState.Errors); - Assert.Equal("The field RangedInt must be between 10 and 30.", error.ErrorMessage); - - Assert.DoesNotContain("theKey.RequiredString", context.ModelState.Keys); - Assert.DoesNotContain("theKey.ValidString", context.ModelState.Keys); - Assert.DoesNotContain("theKey", context.ModelState.Keys); - } - - [Fact] - [ReplaceCulture] - public void Validate_ShortCircuits_IfModelStateHasReachedMaxNumberOfErrors() - { - // Arrange - var model = new ValidateAllPropertiesModel - { - RequiredString = null /* error */, - RangedInt = 0 /* error */, - ValidString = "cat" /* error */ - }; - var expectedMessage = ValidationAttributeUtil.GetRequiredErrorMessage("RequiredString"); - - var modelMetadata = GetModelMetadata(model); - var node = new ModelValidationNode(modelMetadata, "theKey") - { - ValidateAllProperties = true - }; - var context = CreateContext(modelMetadata); - context.ModelState.MaxAllowedErrors = 3; - context.ModelState.AddModelError("somekey", "error text"); - - // Act - node.Validate(context); - - // Assert - Assert.Equal(3, context.ModelState.Count); - var modelState = context.ModelState[string.Empty]; - Assert.NotNull(modelState); - var error = Assert.Single(modelState.Errors); - Assert.IsType(error.Exception); - - // RequiredString is validated first due to ModelMetadata.Properties ordering (Reflection-based). - modelState = context.ModelState["theKey.RequiredString"]; - Assert.NotNull(modelState); - error = Assert.Single(modelState.Errors); - Assert.Equal(expectedMessage, error.ErrorMessage); - - // No room for the other validation errors. - Assert.DoesNotContain("theKey.RangedInt", context.ModelState.Keys); - Assert.DoesNotContain("theKey.ValidString", context.ModelState.Keys); - } - - private static ModelMetadata GetModelMetadata() - { - return new EmptyModelMetadataProvider().GetMetadataForType(null, typeof(object)); - } - - private static ModelMetadata GetModelMetadata(object model) - { - return new DataAnnotationsModelMetadataProvider().GetMetadataForType(() => model, model.GetType()); - } - - private static ModelMetadata GetModelMetadata(Type type) - { - return new DataAnnotationsModelMetadataProvider().GetMetadataForType(modelAccessor: null, modelType: type); - } - - private static ModelValidationContext CreateContext( - ModelMetadata metadata = null, - IModelMetadataProvider metadataProvider = null) - { - var providers = new IModelValidatorProvider[] - { - new DataAnnotationsModelValidatorProvider(), - new DataMemberModelValidatorProvider() - }; - if (metadataProvider == null) - { - metadataProvider = new EmptyModelMetadataProvider(); - } - - return new ModelValidationContext(metadataProvider, - new CompositeModelValidatorProvider(providers), - new ModelStateDictionary(), - metadata, - null); - } - - private sealed class LoggingValidatableObject : IValidatableObject - { - private readonly IList _log; - - public LoggingValidatableObject(IList log) - { - _log = log; - } - - [LoggingValidation] - public string ValidStringProperty { get; set; } - public string InvalidStringProperty { get; set; } - - public IEnumerable Validate(ValidationContext validationContext) - { - Assert.Null(validationContext.MemberName); - _log.Add("In IValidatableObject.Validate()"); - yield return new ValidationResult("Sample error message", new[] { "InvalidStringProperty" }); - } - - private sealed class LoggingValidationAttribute : ValidationAttribute - { - protected override ValidationResult IsValid(object value, ValidationContext validationContext) - { - var validatableObject = Assert.IsType(value); - Assert.NotNull(validationContext); - Assert.Equal("ValidStringProperty", validationContext.MemberName); - validatableObject._log.Add("In LoggingValidatonAttribute.IsValid()"); - return ValidationResult.Success; - } - } - } - - private sealed class LoggingNonValidatableObject - { - private readonly IList _log; - - public LoggingNonValidatableObject(IList log) - { - _log = log; - } - - [LoggingValidation] - [Display(Order = 10001)] - public string LastProperty { get; set; } - - [LoggingValidation] - public string Property3 { get; set; } - [LoggingValidation] - public string Property1 { get; set; } - [LoggingValidation] - public string Property2 { get; set; } - - [LoggingValidation] - [Display(Order = 23)] - public string OrderedProperty3 { get; set; } - [LoggingValidation] - [Display(Order = 23)] - public string OrderedProperty2 { get; set; } - [LoggingValidation] - [Display(Order = 23)] - public string OrderedProperty1 { get; set; } - - private sealed class LoggingValidationAttribute : ValidationAttribute - { - protected override ValidationResult IsValid(object value, ValidationContext validationContext) - { - Assert.Null(value); - Assert.NotNull(validationContext); - var nonValidatableObject = - Assert.IsType(validationContext.ObjectInstance); - - nonValidatableObject._log.Add( - string.Format("In LoggingValidatonAttribute.IsValid({0})", validationContext.MemberName)); - return ValidationResult.Success; - } - } - } - - private class ValidateAllPropertiesModel - { - [Required] - public string RequiredString { get; set; } - - [Range(10, 30)] - public int RangedInt { get; set; } - - [RegularExpression("dog")] - public string ValidString { get; set; } - } - } -} diff --git a/test/Microsoft.AspNet.Mvc.Test/MvcOptionsSetupTest.cs b/test/Microsoft.AspNet.Mvc.Test/MvcOptionsSetupTest.cs index 0c89c2243f..3f16a5ad2d 100644 --- a/test/Microsoft.AspNet.Mvc.Test/MvcOptionsSetupTest.cs +++ b/test/Microsoft.AspNet.Mvc.Test/MvcOptionsSetupTest.cs @@ -147,28 +147,46 @@ namespace Microsoft.AspNet.Mvc setup.Configure(mvcOptions); // Assert - Assert.Equal(5, mvcOptions.ValidationExcludeFilters.Count); + Assert.Equal(8, mvcOptions.ValidationExcludeFilters.Count); + var i = 0; // Verify if the delegates registered by default exclude the given types. - Assert.Equal(typeof(SimpleTypesExcludeFilter), mvcOptions.ValidationExcludeFilters[0].OptionType); - Assert.Equal(typeof(DefaultTypeBasedExcludeFilter), mvcOptions.ValidationExcludeFilters[1].OptionType); + Assert.Equal(typeof(SimpleTypesExcludeFilter), mvcOptions.ValidationExcludeFilters[i++].OptionType); + Assert.Equal(typeof(DefaultTypeBasedExcludeFilter), mvcOptions.ValidationExcludeFilters[i].OptionType); var xObjectFilter - = Assert.IsType(mvcOptions.ValidationExcludeFilters[1].Instance); + = Assert.IsType(mvcOptions.ValidationExcludeFilters[i++].Instance); Assert.Equal(xObjectFilter.ExcludedType, typeof(XObject)); - Assert.Equal(typeof(DefaultTypeBasedExcludeFilter), mvcOptions.ValidationExcludeFilters[2].OptionType); - var typeFilter = - Assert.IsType(mvcOptions.ValidationExcludeFilters[2].Instance); + Assert.Equal(typeof(DefaultTypeBasedExcludeFilter), mvcOptions.ValidationExcludeFilters[i].OptionType); + var typeFilter + = Assert.IsType(mvcOptions.ValidationExcludeFilters[i++].Instance); Assert.Equal(typeFilter.ExcludedType, typeof(Type)); - Assert.Equal(typeof(DefaultTypeBasedExcludeFilter), mvcOptions.ValidationExcludeFilters[3].OptionType); + Assert.Equal(typeof(DefaultTypeBasedExcludeFilter), mvcOptions.ValidationExcludeFilters[i].OptionType); var jTokenFilter - = Assert.IsType(mvcOptions.ValidationExcludeFilters[3].Instance); + = Assert.IsType(mvcOptions.ValidationExcludeFilters[i++].Instance); Assert.Equal(jTokenFilter.ExcludedType, typeof(JToken)); - Assert.Equal(typeof(DefaultTypeNameBasedExcludeFilter), mvcOptions.ValidationExcludeFilters[4].OptionType); + Assert.Equal(typeof(DefaultTypeBasedExcludeFilter), mvcOptions.ValidationExcludeFilters[i].OptionType); + var cancellationTokenFilter + = Assert.IsType(mvcOptions.ValidationExcludeFilters[i++].Instance); + Assert.Equal(cancellationTokenFilter.ExcludedType, typeof(System.Threading.CancellationToken)); + + Assert.Equal(typeof(DefaultTypeBasedExcludeFilter), mvcOptions.ValidationExcludeFilters[i].OptionType); + var formFileFilter + = Assert.IsType(mvcOptions.ValidationExcludeFilters[i++].Instance); + Assert.Equal(formFileFilter.ExcludedType, typeof(Http.IFormFile)); + + Assert.Equal( + typeof(DefaultTypeBasedExcludeFilter), + mvcOptions.ValidationExcludeFilters[i].OptionType); + var formCollectionFilter + = Assert.IsType(mvcOptions.ValidationExcludeFilters[i++].Instance); + Assert.Equal(formCollectionFilter.ExcludedType, typeof(Http.IFormCollection)); + + Assert.Equal(typeof(DefaultTypeNameBasedExcludeFilter), mvcOptions.ValidationExcludeFilters[i].OptionType); var xmlNodeFilter = - Assert.IsType(mvcOptions.ValidationExcludeFilters[4].Instance); + Assert.IsType(mvcOptions.ValidationExcludeFilters[i++].Instance); Assert.Equal(xmlNodeFilter.ExcludedTypeName, "System.Xml.XmlNode"); } } diff --git a/test/WebSites/ModelBindingWebSite/Controllers/ModelBinderAttribute_ProductController.cs b/test/WebSites/ModelBindingWebSite/Controllers/ModelBinderAttribute_ProductController.cs index 7f12b1fdf9..8c9fb6ea97 100644 --- a/test/WebSites/ModelBindingWebSite/Controllers/ModelBinderAttribute_ProductController.cs +++ b/test/WebSites/ModelBindingWebSite/Controllers/ModelBinderAttribute_ProductController.cs @@ -81,7 +81,7 @@ namespace ModelBindingWebSite.Controllers private class OrderStatusBinder : IModelBinder { - public Task BindModelAsync(ModelBindingContext bindingContext) + public Task BindModelAsync(ModelBindingContext bindingContext) { if (typeof(OrderStatus).IsAssignableFrom(bindingContext.ModelType)) { @@ -90,21 +90,17 @@ namespace ModelBindingWebSite.Controllers // Doing something slightly different here to make sure we don't get accidentally bound // by the type converter binder. OrderStatus model; - if (Enum.TryParse("Status" + request.Query.Get("status"), out model)) - { - bindingContext.Model = model; - } - - return Task.FromResult(true); + var isModelSet = Enum.TryParse("Status" + request.Query.Get("status"), out model); + return Task.FromResult(new ModelBindingResult(model, "status", isModelSet)); } - return Task.FromResult(false); + return Task.FromResult(null); } } private class ProductModelBinder : IModelBinder { - public async Task BindModelAsync(ModelBindingContext bindingContext) + public async Task BindModelAsync(ModelBindingContext bindingContext) { if (typeof(Product).IsAssignableFrom(bindingContext.ModelType)) { @@ -120,11 +116,10 @@ namespace ModelBindingWebSite.Controllers var value = await bindingContext.ValueProvider.GetValueAsync(key); model.ProductId = (int)value.ConvertTo(typeof(int)); - bindingContext.Model = model; - return true; + return new ModelBindingResult(model, key, true); } - return false; + return null; } } diff --git a/test/WebSites/ModelBindingWebSite/Controllers/ValidationController.cs b/test/WebSites/ModelBindingWebSite/Controllers/ValidationController.cs new file mode 100644 index 0000000000..45836f4fff --- /dev/null +++ b/test/WebSites/ModelBindingWebSite/Controllers/ValidationController.cs @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.Linq; +using Microsoft.AspNet.Mvc; + +namespace ModelBindingWebSite.Controllers +{ + [Route("Validation/[Action]")] + public class ValidationController : Controller + { + public bool SkipValidation(Resident resident) + { + return ModelState.IsValid; + } + + public bool AvoidRecursive(SelfishPerson selfishPerson) + { + return ModelState.IsValid; + } + } + + public class SelfishPerson + { + public string Name { get; set; } + public SelfishPerson MySelf { get { return this; } } + } +} \ No newline at end of file diff --git a/test/WebSites/ModelBindingWebSite/Models/Address.cs b/test/WebSites/ModelBindingWebSite/Models/Address.cs index 27e328c65d..82dfce1d53 100644 --- a/test/WebSites/ModelBindingWebSite/Models/Address.cs +++ b/test/WebSites/ModelBindingWebSite/Models/Address.cs @@ -1,12 +1,16 @@ // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.ComponentModel.DataAnnotations; + namespace ModelBindingWebSite { public class Address { public int Street { get; set; } public string State { get; set; } + + [Range(10000, 99999)] public int Zip { get; set; } public Country Country { get; set; } diff --git a/test/WebSites/ModelBindingWebSite/Models/Resident.cs b/test/WebSites/ModelBindingWebSite/Models/Resident.cs new file mode 100644 index 0000000000..03098e77a4 --- /dev/null +++ b/test/WebSites/ModelBindingWebSite/Models/Resident.cs @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft Open Technologies, Inc. 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.AspNet.Mvc; + +namespace ModelBindingWebSite +{ + public class Resident : Person + { + public IEnumerable
ShippingAddresses { get; set; } + + public Address HomeAddress { get; set; } + + [FromBody] + public Address OfficeAddress { get; set; } + } +} \ No newline at end of file diff --git a/test/WebSites/ModelBindingWebSite/Startup.cs b/test/WebSites/ModelBindingWebSite/Startup.cs index 8b1724e451..51a0b3a3db 100644 --- a/test/WebSites/ModelBindingWebSite/Startup.cs +++ b/test/WebSites/ModelBindingWebSite/Startup.cs @@ -30,6 +30,7 @@ namespace ModelBindingWebSite m.ModelBinders.Insert(0, typeof(TestBindingSourceModelBinder)); m.AddXmlDataContractSerializerFormatter(); + m.ValidationExcludeFilters.Add(typeof(Address)); }); services.AddSingleton(); diff --git a/test/WebSites/ModelBindingWebSite/TestBindingSourceModelBinder.cs b/test/WebSites/ModelBindingWebSite/TestBindingSourceModelBinder.cs index abb7512c59..d5b8b3921a 100644 --- a/test/WebSites/ModelBindingWebSite/TestBindingSourceModelBinder.cs +++ b/test/WebSites/ModelBindingWebSite/TestBindingSourceModelBinder.cs @@ -15,17 +15,17 @@ namespace ModelBindingWebSite { } - protected override Task BindModelCoreAsync(ModelBindingContext bindingContext) + protected override Task BindModelCoreAsync(ModelBindingContext bindingContext) { var metadata = (FromTestAttribute)bindingContext.ModelMetadata.BinderMetadata; - bindingContext.Model = metadata.Value; - + var model = metadata.Value; if (!IsSimpleType(bindingContext.ModelType)) { - bindingContext.Model = Activator.CreateInstance(bindingContext.ModelType); + model = Activator.CreateInstance(bindingContext.ModelType); + return Task.FromResult(new ModelBindingResult(model, bindingContext.ModelName, true)); } - return Task.FromResult(true); + return Task.FromResult(new ModelBindingResult(null, bindingContext.ModelName, false)); } private bool IsSimpleType(Type type) diff --git a/test/WebSites/PrecompilationWebSite/Views/Home/Index.cshtml b/test/WebSites/PrecompilationWebSite/Views/Home/Index.cshtml index 77a9e96d2e..37e13cd4cb 100644 --- a/test/WebSites/PrecompilationWebSite/Views/Home/Index.cshtml +++ b/test/WebSites/PrecompilationWebSite/Views/Home/Index.cshtml @@ -1 +1 @@ -index:@GetType().GetTypeInfo().Assembly.GetName() \ No newline at end of file +index:@GetType().GetTypeInfo().Assembly.GetName() \ No newline at end of file diff --git a/test/WebSites/PrecompilationWebSite/Views/Home/Layout.cshtml b/test/WebSites/PrecompilationWebSite/Views/Home/Layout.cshtml index c068404da0..57c4e0541c 100644 --- a/test/WebSites/PrecompilationWebSite/Views/Home/Layout.cshtml +++ b/test/WebSites/PrecompilationWebSite/Views/Home/Layout.cshtml @@ -1,2 +1,2 @@ Layout:@GetType().GetTypeInfo().Assembly.FullName -@RenderBody() \ No newline at end of file +@RenderBody() \ No newline at end of file diff --git a/test/WebSites/PrecompilationWebSite/Views/Home/_ViewStart.cshtml b/test/WebSites/PrecompilationWebSite/Views/Home/_ViewStart.cshtml index 984c6ea0eb..80fc904470 100644 --- a/test/WebSites/PrecompilationWebSite/Views/Home/_ViewStart.cshtml +++ b/test/WebSites/PrecompilationWebSite/Views/Home/_ViewStart.cshtml @@ -1,3 +1,4 @@ @using System.Reflection @{ Layout = "/Views/Home/Layout.cshtml";} _viewstart:@GetType().GetTypeInfo().Assembly.FullName + \ No newline at end of file