From 2d33c321876294cf23ec3e268b4e3db754db350d Mon Sep 17 00:00:00 2001 From: bordecal Date: Sun, 5 May 2019 10:08:25 -0400 Subject: [PATCH] Allow ValidationVisitor.ValidateComplexTypesIfChildValidationFails to be configured via MvcOptions (#9312) * Allow ValidationVisitor.ValidateComplexTypesIfChildValidationFails to be configured via MvcOptions (#8519) * Regenerated reference source for Mvc.Core to add MvcOptions.ValidateComplexTypesIfChildValidationFails * Simplified functional tests for MvcOptions.ValidateComplexTypesIfChildValidationFails usage scenarios --- ...osoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs | 1 + .../Validation/DefaultObjectValidator.cs | 1 + src/Mvc/Mvc.Core/src/MvcOptions.cs | 10 ++ .../Validation/DefaultObjectValidatorTests.cs | 43 ++++++ .../InputObjectValidationTests.cs | 1 + .../InputParentValidationTests.cs | 132 ++++++++++++++++++ .../TryValidateModelIntegrationTest.cs | 55 ++------ .../Controllers/ValidationController.cs | 8 ++ .../FormatterWebSite/Models/InvalidModel.cs | 16 +++ .../StartupWithComplexParentValidation.cs | 29 ++++ 10 files changed, 253 insertions(+), 43 deletions(-) create mode 100644 src/Mvc/test/Mvc.FunctionalTests/InputParentValidationTests.cs create mode 100644 src/Mvc/test/WebSites/FormatterWebSite/Models/InvalidModel.cs create mode 100644 src/Mvc/test/WebSites/FormatterWebSite/StartupWithComplexParentValidation.cs diff --git a/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs b/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs index 78d9ab2109..136cf9e1e0 100644 --- a/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs +++ b/src/Mvc/Mvc.Core/ref/Microsoft.AspNetCore.Mvc.Core.netcoreapp3.0.cs @@ -886,6 +886,7 @@ namespace Microsoft.AspNetCore.Mvc public bool SuppressAsyncSuffixInActionNames { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public bool SuppressInputFormatterBuffering { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public bool SuppressOutputFormatterBuffering { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } + public bool ValidateComplexTypesIfChildValidationFails { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } } public System.Collections.Generic.IList ValueProviderFactories { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } System.Collections.Generic.IEnumerator System.Collections.Generic.IEnumerable.GetEnumerator() { throw null; } System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; } diff --git a/src/Mvc/Mvc.Core/src/ModelBinding/Validation/DefaultObjectValidator.cs b/src/Mvc/Mvc.Core/src/ModelBinding/Validation/DefaultObjectValidator.cs index dfa132e4a9..666ff7a6c8 100644 --- a/src/Mvc/Mvc.Core/src/ModelBinding/Validation/DefaultObjectValidator.cs +++ b/src/Mvc/Mvc.Core/src/ModelBinding/Validation/DefaultObjectValidator.cs @@ -42,6 +42,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation validationState) { MaxValidationDepth = _mvcOptions.MaxValidationDepth, + ValidateComplexTypesIfChildValidationFails = _mvcOptions.ValidateComplexTypesIfChildValidationFails, }; return visitor; diff --git a/src/Mvc/Mvc.Core/src/MvcOptions.cs b/src/Mvc/Mvc.Core/src/MvcOptions.cs index f9e13f6317..d3add19c02 100644 --- a/src/Mvc/Mvc.Core/src/MvcOptions.cs +++ b/src/Mvc/Mvc.Core/src/MvcOptions.cs @@ -227,6 +227,16 @@ namespace Microsoft.AspNetCore.Mvc } } + /// + /// Gets or sets a value that determines whether the validation visitor will perform validation of a complex type + /// if validation fails for any of its children. + /// + /// + /// + /// The default value is . + /// + public bool ValidateComplexTypesIfChildValidationFails { get; set; } + /// /// Gets or sets a value that determines if MVC will remove the suffix "Async" applied to /// controller action names. diff --git a/src/Mvc/Mvc.Core/test/ModelBinding/Validation/DefaultObjectValidatorTests.cs b/src/Mvc/Mvc.Core/test/ModelBinding/Validation/DefaultObjectValidatorTests.cs index 505bb6cf8b..15d225a3da 100644 --- a/src/Mvc/Mvc.Core/test/ModelBinding/Validation/DefaultObjectValidatorTests.cs +++ b/src/Mvc/Mvc.Core/test/ModelBinding/Validation/DefaultObjectValidatorTests.cs @@ -1328,6 +1328,33 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation Assert.NotNull(ex.HelpLink); } + [Theory] + [InlineData(false, ModelValidationState.Unvalidated)] + [InlineData(true, ModelValidationState.Invalid)] + public void Validate_RespectsMvcOptionsConfiguration_WhenChildValidationFails(bool optionValue, ModelValidationState expectedParentValidationState) + { + // Arrange + _options.ValidateComplexTypesIfChildValidationFails = optionValue; + + var actionContext = new ActionContext(); + var validationState = new ValidationStateDictionary(); + var validator = CreateValidator(); + + var model = (object)new SelfValidatableModelContainer + { + IsParentValid = false, + ValidatableModelProperty = new ValidatableModel() + }; + + // Act + validator.Validate(actionContext, validationState, prefix: string.Empty, model); + + // Assert + var modelState = actionContext.ModelState; + Assert.False(modelState.IsValid); + Assert.Equal(expectedParentValidationState, modelState.Root.ValidationState); + } + [Fact] public void Validate_TypeWithoutValidators() { @@ -1522,6 +1549,22 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation public ValidatableModel ValidatableModelProperty { get; set; } } + private class SelfValidatableModelContainer : IValidatableObject + { + public bool IsParentValid { get; set; } = true; + + [Display(Name = "Never valid")] + public ValidatableModel ValidatableModelProperty { get; set; } + + public IEnumerable Validate(ValidationContext validationContext) + { + if (!IsParentValid) + { + yield return new ValidationResult("Parent not valid"); + } + } + } + private class TypeThatOverridesEquals { [StringLength(2)] diff --git a/src/Mvc/test/Mvc.FunctionalTests/InputObjectValidationTests.cs b/src/Mvc/test/Mvc.FunctionalTests/InputObjectValidationTests.cs index a91091265b..78c3ce254c 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/InputObjectValidationTests.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/InputObjectValidationTests.cs @@ -8,6 +8,7 @@ using System.Net.Http; using System.Text; using System.Threading.Tasks; using FormatterWebSite; +using FormatterWebSite.Models; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Testing.xunit; using Newtonsoft.Json; diff --git a/src/Mvc/test/Mvc.FunctionalTests/InputParentValidationTests.cs b/src/Mvc/test/Mvc.FunctionalTests/InputParentValidationTests.cs new file mode 100644 index 0000000000..9759f3842f --- /dev/null +++ b/src/Mvc/test/Mvc.FunctionalTests/InputParentValidationTests.cs @@ -0,0 +1,132 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Linq; +using System.Net.Http; +using System.Text; +using System.Threading.Tasks; +using FormatterWebSite.Models; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; +using Newtonsoft.Json; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.FunctionalTests +{ + /// + /// Functional tests for verifying the impact of using + /// + public class InputParentValidationTests + { + public abstract class BaseTests : IClassFixture> + where TStartup : class + { + protected BaseTests(MvcTestFixture fixture) + { + var factory = fixture.Factories.FirstOrDefault() ?? fixture.WithWebHostBuilder(builder => + builder.UseStartup()); + + Client = factory.CreateDefaultClient(); + } + + protected abstract bool ShouldParentBeValidatedWhenChildIsInvalid { get; } + + private HttpClient Client { get; } + + [Fact] + public async Task ParentObjectValidation_RespectsMvcOptions_WhenChildIsInvalid() + { + // Arrange + var content = CreateInvalidModel(false); + var expectedErrors = this.GetExpectedErrors(this.ShouldParentBeValidatedWhenChildIsInvalid, true); + + // Act + var response = await Client.PostAsync("http://localhost/Validation/CreateInvalidModel", content); + + // Assert + Assert.Equal(StatusCodes.Status400BadRequest, (int)response.StatusCode); + + var responseContent = await response.Content.ReadAsStringAsync(); + var actualErrors = JsonConvert.DeserializeObject>(responseContent); + + Assert.Equal(expectedErrors, actualErrors); + } + + [Fact] + public async Task ParentObjectIsValidated_WhenChildIsValid() + { + // Arrange + var content = CreateInvalidModel(true); + var expectedErrors = this.GetExpectedErrors(true, false); + + // Act + var response = await Client.PostAsync("http://localhost/Validation/CreateInvalidModel", content); + + // Assert + Assert.Equal(StatusCodes.Status400BadRequest, (int)response.StatusCode); + + var responseContent = await response.Content.ReadAsStringAsync(); + var actualErrors = JsonConvert.DeserializeObject>(responseContent); + + Assert.Equal(expectedErrors, actualErrors); + } + + private StringContent CreateInvalidModel(bool isChildValid) + { + var model = new InvalidModel() + { + Name = (isChildValid ? "Valid Name" : null) + }; + + return new StringContent(JsonConvert.SerializeObject(model), Encoding.UTF8, "application/json"); + } + + private IDictionary GetExpectedErrors(bool parentInvalid, bool childInvalid) + { + var result = new Dictionary(); + + if (parentInvalid) + { + result.Add(string.Empty, new string[] { "The model is not valid." }); + } + + if (childInvalid) + { + result.Add("Name", new string[] { "The Name field is required." }); + } + + return result; + } + } + + /// + /// Scenarios for verifying the impact of setting + /// to + /// + public class ParentValidationScenarios : BaseTests + { + public ParentValidationScenarios(MvcTestFixture fixture) + : base(fixture) + { + } + + protected override bool ShouldParentBeValidatedWhenChildIsInvalid => true; + } + + /// + /// Scenarios for verifying the impact of leaving + /// to its default value + /// + public class ParentNonValidationScenarios : BaseTests + { + public ParentNonValidationScenarios(MvcTestFixture fixture) + : base(fixture) + { + } + + protected override bool ShouldParentBeValidatedWhenChildIsInvalid => false; + } + } +} \ No newline at end of file diff --git a/src/Mvc/test/Mvc.IntegrationTests/TryValidateModelIntegrationTest.cs b/src/Mvc/test/Mvc.IntegrationTests/TryValidateModelIntegrationTest.cs index 26584b2cde..e60b447fd2 100644 --- a/src/Mvc/test/Mvc.IntegrationTests/TryValidateModelIntegrationTest.cs +++ b/src/Mvc/test/Mvc.IntegrationTests/TryValidateModelIntegrationTest.cs @@ -6,7 +6,6 @@ using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.Linq; using Microsoft.AspNetCore.Mvc.ModelBinding; -using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; @@ -141,11 +140,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var testContext = ModelBindingTestHelper.GetTestContext(); var modelState = testContext.ModelState; var model = new ModelLevelErrorTest(); - var controller = CreateController(testContext, testContext.MetadataProvider); - controller.ObjectValidator = new CustomObjectValidator(testContext.MetadataProvider, TestModelValidatorProvider.CreateDefaultProvider().ValidatorProviders) - { - ValidateComplexTypesIfChildValidationFails = true - }; + var controller = CreateController(testContext, testContext.MetadataProvider, o => o.ValidateComplexTypesIfChildValidationFails = true); // Act var result = controller.TryValidateModel(model); @@ -166,11 +161,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var testContext = ModelBindingTestHelper.GetTestContext(); var modelState = testContext.ModelState; var model = new ModelLevelErrorTest(); - var controller = CreateController(testContext, testContext.MetadataProvider); - controller.ObjectValidator = new CustomObjectValidator(testContext.MetadataProvider, TestModelValidatorProvider.CreateDefaultProvider().ValidatorProviders) - { - ValidateComplexTypesIfChildValidationFails= false - }; + var controller = CreateController(testContext, testContext.MetadataProvider, o => o.ValidateComplexTypesIfChildValidationFails = false); // Act var result = controller.TryValidateModel(model); @@ -213,8 +204,18 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests private TestController CreateController( ActionContext actionContext, IModelMetadataProvider metadataProvider) + { + return CreateController(actionContext, metadataProvider, _ => { }); + } + + private TestController CreateController( + ActionContext actionContext, + IModelMetadataProvider metadataProvider, + Action optionsConfigurator + ) { var options = actionContext.HttpContext.RequestServices.GetRequiredService>(); + optionsConfigurator.Invoke(options.Value); var controller = new TestController(); controller.ControllerContext = new ControllerContext(actionContext); @@ -249,37 +250,5 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests private class TestController : Controller { } - - private class CustomObjectValidator : IObjectModelValidator - { - private readonly IModelMetadataProvider _modelMetadataProvider; - private readonly IList _validatorProviders; - private ValidatorCache _validatorCache; - private CompositeModelValidatorProvider _validatorProvider; - - public CustomObjectValidator(IModelMetadataProvider modelMetadataProvider, IList validatorProviders) - { - _modelMetadataProvider = modelMetadataProvider; - _validatorProviders = validatorProviders; - _validatorCache = new ValidatorCache(); - _validatorProvider = new CompositeModelValidatorProvider(validatorProviders); - } - - public void Validate(ActionContext actionContext, ValidationStateDictionary validationState, string prefix, object model) - { - var visitor = new ValidationVisitor( - actionContext, - _validatorProvider, - _validatorCache, - _modelMetadataProvider, - validationState); - - var metadata = model == null ? null : _modelMetadataProvider.GetMetadataForType(model.GetType()); - visitor.ValidateComplexTypesIfChildValidationFails = ValidateComplexTypesIfChildValidationFails; - visitor.Validate(metadata, prefix, model); - } - - public bool ValidateComplexTypesIfChildValidationFails { get; set; } - } } } diff --git a/src/Mvc/test/WebSites/FormatterWebSite/Controllers/ValidationController.cs b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/ValidationController.cs index 5b0f0ca4a0..997bdf4b2d 100644 --- a/src/Mvc/test/WebSites/FormatterWebSite/Controllers/ValidationController.cs +++ b/src/Mvc/test/WebSites/FormatterWebSite/Controllers/ValidationController.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; +using FormatterWebSite.Models; using Microsoft.AspNetCore.Mvc; namespace FormatterWebSite @@ -84,5 +85,12 @@ namespace FormatterWebSite { return Ok(); } + + [HttpPost] + [ModelStateValidationFilter] + public IActionResult CreateInvalidModel([FromBody] InvalidModel model) + { + return Ok(model); + } } } \ No newline at end of file diff --git a/src/Mvc/test/WebSites/FormatterWebSite/Models/InvalidModel.cs b/src/Mvc/test/WebSites/FormatterWebSite/Models/InvalidModel.cs new file mode 100644 index 0000000000..deda9398a3 --- /dev/null +++ b/src/Mvc/test/WebSites/FormatterWebSite/Models/InvalidModel.cs @@ -0,0 +1,16 @@ +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; + +namespace FormatterWebSite.Models +{ + public class InvalidModel : IValidatableObject + { + [Required] + public string Name { get; set; } + + public IEnumerable Validate(ValidationContext validationContext) + { + yield return new ValidationResult("The model is not valid."); + } + } +} diff --git a/src/Mvc/test/WebSites/FormatterWebSite/StartupWithComplexParentValidation.cs b/src/Mvc/test/WebSites/FormatterWebSite/StartupWithComplexParentValidation.cs new file mode 100644 index 0000000000..44d066a4aa --- /dev/null +++ b/src/Mvc/test/WebSites/FormatterWebSite/StartupWithComplexParentValidation.cs @@ -0,0 +1,29 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.DependencyInjection; + +namespace FormatterWebSite +{ + public class StartupWithComplexParentValidation + { + public void ConfigureServices(IServiceCollection services) + { + services + .AddControllers(options => options.ValidateComplexTypesIfChildValidationFails = true) + .AddNewtonsoftJson(options => options.SerializerSettings.Converters.Insert(0, new IModelConverter())) + .SetCompatibilityVersion(CompatibilityVersion.Latest); + } + + public void Configure(IApplicationBuilder app) + { + app.UseRouting(); + app.UseEndpoints(endpoints => + { + endpoints.MapDefaultControllerRoute(); + }); + } + } +} \ No newline at end of file