From fb57810f2960730e59db0235207b873c924bc3fd Mon Sep 17 00:00:00 2001 From: Pranav K Date: Thu, 4 Oct 2018 16:58:42 -0700 Subject: [PATCH] Shortcircuit validation when using default validator providers and no validation metadata is discovered Fixes https://github.com/aspnet/Mvc/issues/5887 --- .../BasicApi/Controllers/PetController.cs | 7 + benchmarkapps/BasicApi/benchmarks.json | 6 + .../ValidationVisitorBenchmarkBase.cs | 75 +++ .../ValidationVisitorByteArrayBenchmark.cs | 42 ++ ...tionVisitorModelWithValidatedProperties.cs | 92 +++ .../ModelBinding/ModelMetadata.cs | 9 + .../MvcCoreServiceCollectionExtensions.cs | 2 + .../MvcCoreMvcOptionsSetup.cs | 16 +- .../Metadata/DefaultModelMetadata.cs | 80 +++ ...HasValidatorsValidationMetadataProvider.cs | 53 ++ .../Metadata/ValidationMetadata.cs | 5 + .../DefaultModelValidatorProvider.cs | 23 +- .../IMetadataBasedModelValidatorProvider.cs | 30 + .../Validation/ValidationVisitor.cs | 18 + .../DataAnnotationsModelValidatorProvider.cs | 27 +- .../Properties/AssemblyInfo.cs | 6 + .../MvcCoreServiceCollectionExtensionsTest.cs | 1 + .../Internal/DefaultObjectValidatorTests.cs | 111 +++- .../Metadata/DefaultModelMetadataTest.cs | 382 +++++++++++ ...alidatorsValidationMetadataProviderTest.cs | 131 ++++ .../DefaultModelValidatorProviderTest.cs | 32 +- .../TestModelMetadataProvider.cs | 7 + .../TestModelValidatorProvider.cs | 2 - ...taAnnotationsModelValidatorProviderTest.cs | 54 +- .../InputObjectValidationTests.cs | 90 +++ .../ActionParametersIntegrationTest.cs | 9 +- ...lidationMetadataProviderIntegrationTest.cs | 53 ++ .../TryUpdateModelIntegrationTest.cs | 12 +- .../TryValidateModelIntegrationTest.cs | 2 - .../ValidationIntegrationTests.cs | 597 ++++++++++++++++++ .../MvcOptionsSetupTest.cs | 4 +- .../MvcServiceCollectionExtensionsTest.cs | 6 +- .../Controllers/TestApiController.cs | 16 + .../Models/BookModelWithNoValidation.cs | 20 + .../Models/RecursiveIdentifier.cs | 11 +- 35 files changed, 1992 insertions(+), 39 deletions(-) create mode 100644 benchmarks/Microsoft.AspNetCore.Mvc.Performance/ValidationVisitorBenchmarkBase.cs create mode 100644 benchmarks/Microsoft.AspNetCore.Mvc.Performance/ValidationVisitorByteArrayBenchmark.cs create mode 100644 benchmarks/Microsoft.AspNetCore.Mvc.Performance/ValidationVisitorModelWithValidatedProperties.cs rename src/Microsoft.AspNetCore.Mvc.Core/{Internal => Infrastructure}/MvcCoreMvcOptionsSetup.cs (86%) create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/HasValidatorsValidationMetadataProvider.cs rename src/Microsoft.AspNetCore.Mvc.Core/{Internal => ModelBinding/Validation}/DefaultModelValidatorProvider.cs (64%) create mode 100644 src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/IMetadataBasedModelValidatorProvider.cs rename src/Microsoft.AspNetCore.Mvc.DataAnnotations/{Internal => }/DataAnnotationsModelValidatorProvider.cs (84%) create mode 100644 test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/HasValidatorsValidationMetadataProviderTest.cs rename test/Microsoft.AspNetCore.Mvc.Core.Test/{Internal => ModelBinding/Validation}/DefaultModelValidatorProviderTest.cs (88%) rename test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/{Internal => }/DataAnnotationsModelValidatorProviderTest.cs (79%) create mode 100644 test/Microsoft.AspNetCore.Mvc.IntegrationTests/HasValidatorsValidationMetadataProviderIntegrationTest.cs create mode 100644 test/WebSites/FormatterWebSite/Controllers/TestApiController.cs create mode 100644 test/WebSites/FormatterWebSite/Models/BookModelWithNoValidation.cs diff --git a/benchmarkapps/BasicApi/Controllers/PetController.cs b/benchmarkapps/BasicApi/Controllers/PetController.cs index 8a30fb18c2..ed1ff8f5b9 100644 --- a/benchmarkapps/BasicApi/Controllers/PetController.cs +++ b/benchmarkapps/BasicApi/Controllers/PetController.cs @@ -132,6 +132,13 @@ namespace BasicApi.Controllers return new CreatedAtRouteResult("FindPetById", new { id = pet.Id }, pet); } + [Authorize("pet-store-writer")] + [HttpPost("add-pet")] + public ActionResult AddPetWithoutDb(Pet pet) + { + return pet; + } + [Authorize("pet-store-writer")] [HttpPut] public IActionResult EditPet(Pet pet) diff --git a/benchmarkapps/BasicApi/benchmarks.json b/benchmarkapps/BasicApi/benchmarks.json index 7b64057987..aff5eb280d 100644 --- a/benchmarkapps/BasicApi/benchmarks.json +++ b/benchmarkapps/BasicApi/benchmarks.json @@ -44,5 +44,11 @@ "Scripts": "https://raw.githubusercontent.com/aspnet/Mvc/release/2.2/benchmarkapps/BasicApi/postJsonWithToken.lua" }, "Path": "/pet" + }, + "BasicApi.PostWithoutDb": { + "Path": "/pet/add-pet", + "ClientProperties": { + "Scripts": "https://raw.githubusercontent.com/aspnet/Mvc/release/2.2/benchmarkapps/BasicApi/postJsonWithToken.lua" + } } } diff --git a/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ValidationVisitorBenchmarkBase.cs b/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ValidationVisitorBenchmarkBase.cs new file mode 100644 index 0000000000..cfa12d5f1b --- /dev/null +++ b/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ValidationVisitorBenchmarkBase.cs @@ -0,0 +1,75 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using BenchmarkDotNet.Attributes; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.DataAnnotations; +using Microsoft.AspNetCore.Mvc.Internal; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; +using Microsoft.AspNetCore.Routing; +using Microsoft.Extensions.Options; + +namespace Microsoft.AspNetCore.Mvc.Performance +{ + public abstract class ValidationVisitorBenchmarkBase + { + protected const int Iterations = 4; + + protected static readonly IModelValidatorProvider[] ValidatorProviders = new IModelValidatorProvider[] + { + new DefaultModelValidatorProvider(), + new DataAnnotationsModelValidatorProvider( + new ValidationAttributeAdapterProvider(), + Options.Create(new MvcDataAnnotationsLocalizationOptions()), + null), + }; + + protected static readonly CompositeModelValidatorProvider CompositeModelValidatorProvider = new CompositeModelValidatorProvider(ValidatorProviders); + + public abstract object Model { get; } + + public ModelMetadataProvider BaselineModelMetadataProvider { get; private set; } + public ModelMetadataProvider ModelMetadataProvider { get; private set; } + public ModelMetadata BaselineModelMetadata { get; private set; } + public ModelMetadata ModelMetadata { get; private set; } + public ActionContext ActionContext { get; private set; } + public ValidatorCache ValidatorCache { get; private set; } + + [GlobalSetup] + public void Setup() + { + BaselineModelMetadataProvider = CreateModelMetadataProvider(addHasValidatorsProvider: false); + ModelMetadataProvider = CreateModelMetadataProvider(addHasValidatorsProvider: true); + + BaselineModelMetadata = BaselineModelMetadataProvider.GetMetadataForType(Model.GetType()); + ModelMetadata = ModelMetadataProvider.GetMetadataForType(Model.GetType()); + ActionContext = GetActionContext(); + ValidatorCache = new ValidatorCache(); + } + + protected static ModelMetadataProvider CreateModelMetadataProvider(bool addHasValidatorsProvider) + { + var detailsProviders = new List + { + new DefaultValidationMetadataProvider(), + }; + + if (addHasValidatorsProvider) + { + detailsProviders.Add(new HasValidatorsValidationMetadataProvider(ValidatorProviders)); + } + + var compositeDetailsProvider = new DefaultCompositeMetadataDetailsProvider(detailsProviders); + return new DefaultModelMetadataProvider(compositeDetailsProvider, Options.Create(new MvcOptions())); + } + + protected static ActionContext GetActionContext() + { + return new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor()); + } + } +} diff --git a/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ValidationVisitorByteArrayBenchmark.cs b/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ValidationVisitorByteArrayBenchmark.cs new file mode 100644 index 0000000000..8880aa5a05 --- /dev/null +++ b/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ValidationVisitorByteArrayBenchmark.cs @@ -0,0 +1,42 @@ +// 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 BenchmarkDotNet.Attributes; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; + +namespace Microsoft.AspNetCore.Mvc.Performance +{ + public class ValidationVisitorByteArrayBenchmark : ValidationVisitorBenchmarkBase + { + public override object Model { get; } = new byte[30]; + + [Benchmark(Baseline = true, Description = "validation for byte arrays baseline", OperationsPerInvoke = Iterations)] + public void Baseline() + { + // Baseline for validating a byte array of size 30, without the ModelMetadata.HasValidators optimization. + // This is the behavior as of 2.1. + var validationVisitor = new ValidationVisitor( + ActionContext, + CompositeModelValidatorProvider, + ValidatorCache, + BaselineModelMetadataProvider, + new ValidationStateDictionary()); + + validationVisitor.Validate(BaselineModelMetadata, "key", Model); + } + + [Benchmark(Description = "validation for byte arrays", OperationsPerInvoke = Iterations)] + public void HasValidators() + { + // Validating a byte array of size 30, with the ModelMetadata.HasValidators optimization. + var validationVisitor = new ValidationVisitor( + ActionContext, + CompositeModelValidatorProvider, + ValidatorCache, + ModelMetadataProvider, + new ValidationStateDictionary()); + + validationVisitor.Validate(ModelMetadata, "key", Model); + } + } +} diff --git a/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ValidationVisitorModelWithValidatedProperties.cs b/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ValidationVisitorModelWithValidatedProperties.cs new file mode 100644 index 0000000000..58c4127232 --- /dev/null +++ b/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ValidationVisitorModelWithValidatedProperties.cs @@ -0,0 +1,92 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using BenchmarkDotNet.Attributes; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; + +namespace Microsoft.AspNetCore.Mvc.Performance +{ + public class ValidationVisitorModelWithValidatedProperties : ValidationVisitorBenchmarkBase + { + public class Person + { + [Required] + public int Id { get; set; } + + [Required] + [StringLength(20)] + public string Name { get; set; } + + public string Description { get; set; } + + public IList
Address { get; set; } + } + + public class Address + { + [Required] + public string Street { get; set; } + + public string Street2 { get; set; } + + public string Type { get; set; } + + [Required] + public string Zip { get; set; } + } + + public override object Model { get; } = new Person + { + Id = 10, + Name = "Test", + Address = new List
+ { + new Address + { + Street = "1 Microsoft Way", + Type = "Work", + Zip = "98056", + }, + new Address + { + Street = "15701 NE 39th St", + Type = "Home", + Zip = "98052", + } + }, + }; + + [Benchmark(Baseline = true, Description = "validation for a model with some validated properties - baseline", OperationsPerInvoke = Iterations)] + public void Visit_TypeWithSomeValidatedProperties_Baseline() + { + // Baseline for validating a typical model with some properties that require validation. + // This executes without the ModelMetadata.HasValidators optimization. + + var validationVisitor = new ValidationVisitor( + ActionContext, + CompositeModelValidatorProvider, + ValidatorCache, + BaselineModelMetadataProvider, + new ValidationStateDictionary()); + + validationVisitor.Validate(BaselineModelMetadata, "key", Model); + } + + [Benchmark(Description = "validation for a model with some validated properties", OperationsPerInvoke = Iterations)] + public void Visit_TypeWithSomeValidatedProperties() + { + // Validating a typical model with some properties that require validation. + // This executes with the ModelMetadata.HasValidators optimization. + var validationVisitor = new ValidationVisitor( + ActionContext, + CompositeModelValidatorProvider, + ValidatorCache, + ModelMetadataProvider, + new ValidationStateDictionary()); + + validationVisitor.Validate(ModelMetadata, "key", Model); + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs index f78815a7d1..8701d4f4a3 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs @@ -325,6 +325,15 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// public abstract bool ValidateChildren { get; } + /// + /// Gets a value that indicates if the model, or one of it's properties, or elements has associatated validators. + /// + /// + /// When , validation can be assume that the model is valid () without + /// inspecting the object graph. + /// + public virtual bool? HasValidators { get; } + /// /// Gets a collection of metadata items for validators. /// diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index a0c082fdf6..95a4afb21b 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -146,6 +146,8 @@ namespace Microsoft.Extensions.DependencyInjection ServiceDescriptor.Transient, MvcCoreMvcOptionsSetup>()); services.TryAddEnumerable( ServiceDescriptor.Transient, MvcOptionsConfigureCompatibilityOptions>()); + services.TryAddEnumerable( + ServiceDescriptor.Transient, MvcCoreMvcOptionsSetup>()); services.TryAddEnumerable( ServiceDescriptor.Transient, ApiBehaviorOptionsSetup>()); services.TryAddEnumerable( diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreMvcOptionsSetup.cs b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcCoreMvcOptionsSetup.cs similarity index 86% rename from src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreMvcOptionsSetup.cs rename to src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcCoreMvcOptionsSetup.cs index 4cb4f9f3bd..7b89b69ee6 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreMvcOptionsSetup.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Infrastructure/MvcCoreMvcOptionsSetup.cs @@ -8,24 +8,25 @@ using System.Threading; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Binders; using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; -namespace Microsoft.AspNetCore.Mvc.Internal +namespace Microsoft.AspNetCore.Mvc { /// /// Sets up default options for . /// - public class MvcCoreMvcOptionsSetup : IConfigureOptions + internal class MvcCoreMvcOptionsSetup : IConfigureOptions, IPostConfigureOptions { private readonly IHttpRequestStreamReaderFactory _readerFactory; private readonly ILoggerFactory _loggerFactory; - // Used in tests public MvcCoreMvcOptionsSetup(IHttpRequestStreamReaderFactory readerFactory) : this(readerFactory, NullLoggerFactory.Instance) { @@ -83,6 +84,15 @@ namespace Microsoft.AspNetCore.Mvc.Internal options.ModelValidatorProviders.Add(new DefaultModelValidatorProvider()); } + public void PostConfigure(string name, MvcOptions options) + { + // HasValidatorsValidationMetadataProvider uses the results of other ValidationMetadataProvider to determine if a model requires + // validation. It is imperative that this executes later than all other metadata provider. We'll register it as part of PostConfigure. + // This should ensure it appears later than all of the details provider registered by MVC and user configured details provider registered + // as part of ConfigureOptions. + options.ModelMetadataDetailsProviders.Add(new HasValidatorsValidationMetadataProvider(options.ModelValidatorProviders)); + } + internal static void ConfigureAdditionalModelMetadataDetailsProviders(IList modelMetadataDetailsProviders) { // Don't bind the Type class by default as it's expensive. A user can override this behavior diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs index bfb1bde8b5..277da94ddd 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Collections.ObjectModel; using System.Linq; +using System.Runtime.CompilerServices; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata @@ -29,6 +30,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata private bool? _isRequired; private ModelPropertyCollection _properties; private bool? _validateChildren; + private bool? _hasValidators; private ReadOnlyCollection _validatorMetadata; /// @@ -427,6 +429,84 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata } } + /// + public override bool? HasValidators + { + get + { + if (!_hasValidators.HasValue) + { + var visited = new HashSet(); + + _hasValidators = CalculateHasValidators(visited, this); + } + + return _hasValidators.Value; + } + } + + internal static bool CalculateHasValidators(HashSet visited, ModelMetadata metadata) + { + RuntimeHelpers.EnsureSufficientExecutionStack(); + + if (metadata?.GetType() != typeof(DefaultModelMetadata)) + { + // The calculation is valid only for DefaultModelMetadata instances. Null, other ModelMetadata instances + // or subtypes of DefaultModelMetadata will be treated as always requiring validation. + return true; + } + + var defaultModelMetadata = (DefaultModelMetadata)metadata; + + if (defaultModelMetadata._hasValidators.HasValue) + { + // Return a previously calculated value if available. + return defaultModelMetadata._hasValidators.Value; + } + + if (defaultModelMetadata.ValidationMetadata.HasValidators != false) + { + // Either the ModelMetadata instance has some validators (HasValidators = true) or it is non-deterministic (HasValidators = null). + // In either case, assume it has validators. + return true; + } + + // Before inspecting properties or elements of a collection, ensure we do not have a cycle. + // Consider a model like so + // + // Employee { BusinessDivision Division; int Id; string Name; } + // BusinessDivision { int Id; List Employees } + // + // If we get to the Employee element from Employee.Division.Employees, we can return false for that instance + // and allow other properties of BusinessDivision and Employee to determine if it has validators. + if (!visited.Add(defaultModelMetadata)) + { + return false; + } + + // We have inspected the current element. Inspect properties or elements that may contribute to this value. + if (defaultModelMetadata.IsEnumerableType) + { + if (CalculateHasValidators(visited, defaultModelMetadata.ElementMetadata)) + { + return true; + } + } + else if (defaultModelMetadata.IsComplexType) + { + foreach (var property in defaultModelMetadata.Properties) + { + if (CalculateHasValidators(visited, property)) + { + return true; + } + } + } + + // We've come this far. The ModelMetadata does not have any validation + return false; + } + /// public override IReadOnlyList ValidatorMetadata { diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/HasValidatorsValidationMetadataProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/HasValidatorsValidationMetadataProvider.cs new file mode 100644 index 0000000000..6378ba518f --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/HasValidatorsValidationMetadataProvider.cs @@ -0,0 +1,53 @@ +// 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 Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation +{ + internal class HasValidatorsValidationMetadataProvider : IValidationMetadataProvider + { + private readonly bool _hasOnlyMetadataBasedValidators; + private readonly IMetadataBasedModelValidatorProvider[] _validatorProviders; + + public HasValidatorsValidationMetadataProvider(IList modelValidatorProviders) + { + if (modelValidatorProviders.Count > 0 && modelValidatorProviders.All(p => p is IMetadataBasedModelValidatorProvider)) + { + _hasOnlyMetadataBasedValidators = true; + _validatorProviders = modelValidatorProviders.Cast().ToArray(); + } + } + + public void CreateValidationMetadata(ValidationMetadataProviderContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (!_hasOnlyMetadataBasedValidators) + { + return; + } + + for (var i = 0; i < _validatorProviders.Length; i++) + { + var provider = _validatorProviders[i]; + if (provider.HasValidators(context.Key.ModelType, context.ValidationMetadata.ValidatorMetadata)) + { + context.ValidationMetadata.HasValidators = true; + return; + } + } + + if (context.ValidationMetadata.HasValidators == null) + { + context.ValidationMetadata.HasValidators = false; + } + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ValidationMetadata.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ValidationMetadata.cs index bd4ff327aa..d205fdf172 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ValidationMetadata.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ValidationMetadata.cs @@ -41,5 +41,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata /// in this list, to be consumed later by an . /// public IList ValidatorMetadata { get; } = new List(); + + /// + /// Gets a value that indicates if the model has validators . + /// + public bool? HasValidators { get; set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultModelValidatorProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/DefaultModelValidatorProvider.cs similarity index 64% rename from src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultModelValidatorProvider.cs rename to src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/DefaultModelValidatorProvider.cs index 11d5739c02..2174be1417 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultModelValidatorProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/DefaultModelValidatorProvider.cs @@ -1,9 +1,10 @@ // 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.Mvc.ModelBinding.Validation; +using System; +using System.Collections.Generic; -namespace Microsoft.AspNetCore.Mvc.Internal +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation { /// /// A default . @@ -12,7 +13,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal /// The provides validators from /// instances in . /// - public class DefaultModelValidatorProvider : IModelValidatorProvider + internal sealed class DefaultModelValidatorProvider : IMetadataBasedModelValidatorProvider { /// public void CreateValidators(ModelValidatorProviderContext context) @@ -28,13 +29,25 @@ namespace Microsoft.AspNetCore.Mvc.Internal continue; } - var validator = validatorItem.ValidatorMetadata as IModelValidator; - if (validator != null) + if (validatorItem.ValidatorMetadata is IModelValidator validator) { validatorItem.Validator = validator; validatorItem.IsReusable = true; } } } + + public bool HasValidators(Type modelType, IList validatorMetadata) + { + for (var i = 0; i < validatorMetadata.Count; i++) + { + if (validatorMetadata[i] is IModelValidator) + { + return true; + } + } + + return false; + } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/IMetadataBasedModelValidatorProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/IMetadataBasedModelValidatorProvider.cs new file mode 100644 index 0000000000..4fb9bd0c6f --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/IMetadataBasedModelValidatorProvider.cs @@ -0,0 +1,30 @@ +// 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 Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation +{ + /// + /// An that provides instances + /// exclusively using values in or the model type. + /// + /// can be used to statically determine if a given + /// instance can incur any validation. The value for + /// can be calculated if all instances in are . + /// + /// + public interface IMetadataBasedModelValidatorProvider : IModelValidatorProvider + { + /// + /// Gets a value that determines if the can + /// produce any validators given the and . + /// + /// The of the model. + /// The list of metadata items for validators. . + /// + bool HasValidators(Type modelType, IList validatorMetadata); + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs index 164928c54a..e62f825887 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs @@ -265,6 +265,24 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation CurrentPath.Pop(model); return true; } + // If the metadata indicates that no validators exist AND the aggregate state for the key says that the model graph + // is not invalid (i.e. is one of Unvalidated, Valid, or Skipped) we can safely mark the graph as valid. + else if (metadata.HasValidators == false && + ModelState.GetFieldValidationState(key) != ModelValidationState.Invalid) + { + // No validators will be created for this graph of objects. Mark it as valid if it wasn't previously validated. + var entries = ModelState.FindKeysWithPrefix(key); + foreach (var item in entries) + { + if (item.Value.ValidationState == ModelValidationState.Unvalidated) + { + item.Value.ValidationState = ModelValidationState.Valid; + } + } + + CurrentPath.Pop(model); + return true; + } using (StateManager.Recurse(this, key ?? string.Empty, metadata, model, strategy)) { diff --git a/src/Microsoft.AspNetCore.Mvc.DataAnnotations/Internal/DataAnnotationsModelValidatorProvider.cs b/src/Microsoft.AspNetCore.Mvc.DataAnnotations/DataAnnotationsModelValidatorProvider.cs similarity index 84% rename from src/Microsoft.AspNetCore.Mvc.DataAnnotations/Internal/DataAnnotationsModelValidatorProvider.cs rename to src/Microsoft.AspNetCore.Mvc.DataAnnotations/DataAnnotationsModelValidatorProvider.cs index 5ece07f042..b6266958c3 100644 --- a/src/Microsoft.AspNetCore.Mvc.DataAnnotations/Internal/DataAnnotationsModelValidatorProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.DataAnnotations/DataAnnotationsModelValidatorProvider.cs @@ -2,19 +2,21 @@ // 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 Microsoft.AspNetCore.Mvc.DataAnnotations.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.Extensions.Localization; using Microsoft.Extensions.Options; -namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal +namespace Microsoft.AspNetCore.Mvc.DataAnnotations { /// /// An implementation of which provides validators /// for attributes which derive from . It also provides /// a validator for types which implement . /// - public class DataAnnotationsModelValidatorProvider : IModelValidatorProvider + internal sealed class DataAnnotationsModelValidatorProvider : IMetadataBasedModelValidatorProvider { private readonly IOptions _options; private readonly IStringLocalizerFactory _stringLocalizerFactory; @@ -66,8 +68,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal continue; } - var attribute = validatorItem.ValidatorMetadata as ValidationAttribute; - if (attribute == null) + if (!(validatorItem.ValidatorMetadata is ValidationAttribute attribute)) { continue; } @@ -98,5 +99,23 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal }); } } + + public bool HasValidators(Type modelType, IList validatorMetadata) + { + if (typeof(IValidatableObject).IsAssignableFrom(modelType)) + { + return true; + } + + for (var i = 0; i < validatorMetadata.Count; i++) + { + if (validatorMetadata[i] is ValidationAttribute) + { + return true; + } + } + + return false; + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.DataAnnotations/Properties/AssemblyInfo.cs b/src/Microsoft.AspNetCore.Mvc.DataAnnotations/Properties/AssemblyInfo.cs index c375950300..a0584f4754 100644 --- a/src/Microsoft.AspNetCore.Mvc.DataAnnotations/Properties/AssemblyInfo.cs +++ b/src/Microsoft.AspNetCore.Mvc.DataAnnotations/Properties/AssemblyInfo.cs @@ -4,3 +4,9 @@ using System.Runtime.CompilerServices; [assembly: InternalsVisibleTo("Microsoft.AspNetCore.Mvc.DataAnnotations.Test, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] +[assembly: InternalsVisibleTo("Microsoft.AspNetCore.Mvc.Test, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] +[assembly: InternalsVisibleTo("Microsoft.AspNetCore.Mvc.Core.Test, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] +[assembly: InternalsVisibleTo("Microsoft.AspNetCore.Mvc.Core.TestCommon, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] +[assembly: InternalsVisibleTo("Microsoft.AspNetCore.Mvc.ViewFeatures.Test, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] + +[assembly: InternalsVisibleTo("Microsoft.AspNetCore.Mvc.Performance, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f33a29044fa9d740c9b3213a93e57c84b472c84e0b8a0e1ae48e67a9f8f6de9d5f7f3d52ac23e48ac51801f1dc950abe901da34d2a9e3baadb141a17c77ef3c565dd5ee5054b91cf63bb3c6ab83f72ab3aafe93d0fc3c2348b764fafb0b1c0733de51459aeab46580384bf9d74c4e28164b7cde247f891ba07891c9d872ad2bb")] \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs index ba70d85c6e..e4e56bebd2 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/DependencyInjection/MvcCoreServiceCollectionExtensionsTest.cs @@ -248,6 +248,7 @@ namespace Microsoft.AspNetCore.Mvc new Type[] { typeof(MvcOptionsConfigureCompatibilityOptions), + typeof(MvcCoreMvcOptionsSetup), } }, { diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultObjectValidatorTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultObjectValidatorTests.cs index 5450c507c1..8fad61aadd 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultObjectValidatorTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultObjectValidatorTests.cs @@ -1170,11 +1170,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal var modelState = actionContext.ModelState; var validationState = new ValidationStateDictionary(); - var validator = CreateValidator(typeof(List)); + var validator = CreateValidator(typeof(List)); - var model = new List() + var model = new List() { - "15", + new ValidatedModel { Value = "15" }, }; modelState.SetModelValue("userIds[0]", "15", "15"); @@ -1192,6 +1192,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Empty(entry.Errors); } + private class ValidatedModel + { + [Required] + public string Value { get; set; } + } + [Fact] public void Validate_SuppressesValidation_ForExcludedType_Stream() { @@ -1317,7 +1323,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal { model, new ValidationStateEntry() } }; var method = GetType().GetMethod(nameof(Validate_Throws_ForTopLevelMetadataData), BindingFlags.NonPublic | BindingFlags.Instance); - var metadata = MetadataProvider.GetMetadataForParameter(method.GetParameters()[0]); // Act & Assert var ex = Assert.Throws(() => validator.Validate(actionContext, validationState, prefix: string.Empty, model)); @@ -1325,6 +1330,102 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.NotNull(ex.HelpLink); } + [Fact] + public void Validate_TypeWithoutValidators() + { + var actionContext = new ActionContext(); + var validator = CreateValidator(); + var model = new ModelWithoutValidation(); + var validationState = new ValidationStateDictionary + { + { model, new ValidationStateEntry() } + }; + + actionContext.ModelState.SetModelValue("Property1", new ValueProviderResult("value1")); + actionContext.ModelState.SetModelValue("Property2", new ValueProviderResult("value2")); + + // Act + validator.Validate(actionContext, validationState, string.Empty, model); + + // Assert + var modelState = actionContext.ModelState; + Assert.Equal(ModelValidationState.Valid, modelState.ValidationState); + Assert.True(modelState.IsValid); + + var entry = modelState["Property1"]; + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + + entry = modelState["Property2"]; + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + } + + [Fact] + public void Validate_TypeWithoutValidators_DoesNotUpdateValidationState() + { + var actionContext = new ActionContext(); + var validator = CreateValidator(); + var model = new ModelWithoutValidation(); + var validationState = new ValidationStateDictionary + { + { model, new ValidationStateEntry() } + }; + + var modelState = actionContext.ModelState; + modelState.SetModelValue("Property1", new ValueProviderResult("value1")); + modelState.SetModelValue("Property2", new ValueProviderResult("value2")); + modelState["Property2"].ValidationState = ModelValidationState.Skipped; + + // Act + validator.Validate(actionContext, validationState, string.Empty, model); + + // Assert + Assert.Equal(ModelValidationState.Valid, modelState.ValidationState); + Assert.True(modelState.IsValid); + + var entry = modelState["Property1"]; + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + + entry = modelState["Property2"]; + Assert.Equal(ModelValidationState.Skipped, entry.ValidationState); + } + + [Fact] + public void Validate_TypeWithoutValidators_DoesNotResetInvalidState() + { + var actionContext = new ActionContext(); + var validator = CreateValidator(); + var model = new ModelWithoutValidation(); + var validationState = new ValidationStateDictionary + { + { model, new ValidationStateEntry() } + }; + + var modelState = actionContext.ModelState; + modelState.SetModelValue("Property1", new ValueProviderResult("value1")); + modelState.SetModelValue("Property2", new ValueProviderResult("value2")); + modelState["Property2"].ValidationState = ModelValidationState.Invalid; + + // Act + validator.Validate(actionContext, validationState, string.Empty, model); + + // Assert + Assert.Equal(ModelValidationState.Invalid, modelState.ValidationState); + Assert.False(modelState.IsValid); + + var entry = modelState["Property1"]; + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + + entry = modelState["Property2"]; + Assert.Equal(ModelValidationState.Invalid, entry.ValidationState); + } + + private class ModelWithoutValidation + { + public string Property1 { get; set; } + + public string Property2 { get; set; } + } + private static DefaultObjectValidator CreateValidator(Type excludedType) { var excludeFilters = new List(); @@ -1352,6 +1453,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal private class ThrowingProperty { + [Required] public string WatchOut { get @@ -1520,6 +1622,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal Depth = depth; } + [Range(-10, 400)] public int Depth { get; } public int MaxAllowedDepth { get; } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataTest.cs index 6f3eb0830f..3352890707 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataTest.cs @@ -6,6 +6,7 @@ using System.Collections; using System.Collections.Generic; using System.Collections.ObjectModel; using System.Linq; +using System.Reflection; using System.Xml; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; @@ -909,6 +910,351 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata metadataProvider.VerifyAll(); } + [Fact] + public void CalculateHasValidators_ParameterMetadata_TypeHasNoValidators() + { + // Arrange + var parameter = GetType() + .GetMethod(nameof(CalculateHasValidators_ParameterMetadata_TypeHasNoValidatorsMethod), BindingFlags.Static | BindingFlags.NonPublic) + .GetParameters()[0]; + var modelIdentity = ModelMetadataIdentity.ForParameter(parameter); + var modelMetadata = CreateModelMetadata(modelIdentity, Mock.Of(), false); + + // Act + var result = DefaultModelMetadata.CalculateHasValidators(new HashSet(), modelMetadata); + + // Assert + Assert.False(result); + } + + private static void CalculateHasValidators_ParameterMetadata_TypeHasNoValidatorsMethod(string model) { } + + [Fact] + public void CalculateHasValidators_PropertyMetadata_TypeHasNoValidators() + { + // Arrange + var property = GetType() + .GetProperty(nameof(CalculateHasValidators_PropertyMetadata_TypeHasNoValidatorsProperty), BindingFlags.Static | BindingFlags.NonPublic); + var modelIdentity = ModelMetadataIdentity.ForProperty(property.PropertyType, property.Name, GetType()); + var modelMetadata = CreateModelMetadata(modelIdentity, Mock.Of(), false); + + // Act + var result = DefaultModelMetadata.CalculateHasValidators(new HashSet(), modelMetadata); + + // Assert + Assert.False(result); + } + + private static int CalculateHasValidators_PropertyMetadata_TypeHasNoValidatorsProperty { get; set; } + + [Fact] + public void CalculateHasValidators_TypeWithoutProperties_TypeHasNoValidators() + { + // Arrange + var modelIdentity = ModelMetadataIdentity.ForType(typeof(string)); + var modelMetadata = CreateModelMetadata(modelIdentity, Mock.Of(), false); + + // Act + var result = DefaultModelMetadata.CalculateHasValidators(new HashSet(), modelMetadata); + + // Assert + Assert.False(result); + } + + [Fact] + public void CalculateHasValidators_SimpleType_TypeHasValidators() + { + // Arrange + var modelIdentity = ModelMetadataIdentity.ForType(typeof(string)); + var modelMetadata = CreateModelMetadata(modelIdentity, Mock.Of(), true); + + // Act + var result = DefaultModelMetadata.CalculateHasValidators(new HashSet(), modelMetadata); + + // Assert + Assert.True(result); + } + + [Fact] + public void CalculateHasValidators_ReturnsTrue_SimpleType_TypeHasNonDeterministicValidators() + { + // Arrange + var modelIdentity = ModelMetadataIdentity.ForType(typeof(string)); + var modelMetadata = CreateModelMetadata(modelIdentity, Mock.Of(), null); + + // Act + var result = DefaultModelMetadata.CalculateHasValidators(new HashSet(), modelMetadata); + + // Assert + Assert.True(result); + } + + [Fact] + public void CalculateHasValidators_TypeWithProperties_PropertyIsNotDefaultModelMetadata() + { + // Arrange + var modelType = typeof(TypeWithProperties); + var modelIdentity = ModelMetadataIdentity.ForType(modelType); + var metadataProvider = new Mock(); + var modelMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); + + var propertyIdentity = ModelMetadataIdentity.ForProperty(typeof(int), nameof(TypeWithProperties.PublicGetPublicSetProperty), typeof(string)); + var propertyMetadata = new Mock(propertyIdentity); + + metadataProvider + .Setup(mp => mp.GetMetadataForProperties(modelType)) + .Returns(new[] { propertyMetadata.Object, }) + .Verifiable(); + + // Act + var result = DefaultModelMetadata.CalculateHasValidators(new HashSet(), modelMetadata); + + // Assert + Assert.True(result); + } + + [Fact] + public void CalculateHasValidators_TypeWithProperties_HasValidatorForAnyPropertyIsTrue() + { + // Arrange + var modelType = typeof(TypeWithProperties); + var modelIdentity = ModelMetadataIdentity.ForType(modelType); + var metadataProvider = new Mock(); + var modelMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); + + var property1Identity = ModelMetadataIdentity.ForProperty(typeof(int), nameof(TypeWithProperties.PublicGetPublicSetProperty), typeof(string)); + var property1Metadata = CreateModelMetadata(property1Identity, metadataProvider.Object, false); + + var property2Identity = ModelMetadataIdentity.ForProperty(typeof(int), nameof(TypeWithProperties.PublicGetProtectedSetProperty), typeof(string)); + var property2Metadata = CreateModelMetadata(property2Identity, metadataProvider.Object, true); + + metadataProvider + .Setup(mp => mp.GetMetadataForProperties(modelType)) + .Returns(new[] { property1Metadata, property2Metadata }) + .Verifiable(); + + // Act + var result = DefaultModelMetadata.CalculateHasValidators(new HashSet(), modelMetadata); + + // Assert + Assert.True(result); + } + + [Fact] + public void CalculateHasValidators_TypeWithProperties_HasValidatorsForPropertyIsNotDeterminstic() + { + // Arrange + var modelType = typeof(TypeWithProperties); + var modelIdentity = ModelMetadataIdentity.ForType(modelType); + var metadataProvider = new Mock(); + var modelMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); + + var propertyIdentity = ModelMetadataIdentity.ForProperty(typeof(int), nameof(TypeWithProperties.PublicGetPublicSetProperty), typeof(string)); + var propertyMetadata = CreateModelMetadata(propertyIdentity, metadataProvider.Object, null); + + metadataProvider + .Setup(mp => mp.GetMetadataForProperties(modelType)) + .Returns(new[] { propertyMetadata, }) + .Verifiable(); + + // Act + var result = DefaultModelMetadata.CalculateHasValidators(new HashSet(), modelMetadata); + + // Assert + Assert.True(result); + } + + [Fact] + public void CalculateHasValidators_TypeWithProperties_HasValidatorForAllPropertiesIsFalse() + { + // Arrange + var modelType = typeof(TypeWithProperties); + var modelIdentity = ModelMetadataIdentity.ForType(modelType); + var metadataProvider = new Mock(); + var modelMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); + + var property1Identity = ModelMetadataIdentity.ForProperty(typeof(int), nameof(TypeWithProperties.PublicGetPublicSetProperty), modelType); + var property1Metadata = CreateModelMetadata(property1Identity, metadataProvider.Object, false); + + var property2Identity = ModelMetadataIdentity.ForProperty(typeof(int), nameof(TypeWithProperties.PublicGetProtectedSetProperty), modelType); + var property2Metadata = CreateModelMetadata(property2Identity, metadataProvider.Object, false); + + metadataProvider + .Setup(mp => mp.GetMetadataForProperties(modelType)) + .Returns(new[] { property1Metadata, property2Metadata }) + .Verifiable(); + + // Act + var result = DefaultModelMetadata.CalculateHasValidators(new HashSet(), modelMetadata); + + // Assert + Assert.False(result); + } + + [Fact] + public void CalculateHasValidators_SelfReferencingType_HasValidatorOnNestedProperty() + { + // Arrange + var modelType = typeof(Employee); + var modelIdentity = ModelMetadataIdentity.ForType(modelType); + var metadataProvider = new Mock(); + var modelMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); + + var employeeId = ModelMetadataIdentity.ForProperty(typeof(int), nameof(Employee.Id), modelType); + var employeeIdMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); + var employeeUnit = ModelMetadataIdentity.ForProperty(typeof(BusinessUnit), nameof(Employee.Unit), modelType); + var employeeUnitMetadata = CreateModelMetadata(employeeUnit, metadataProvider.Object, false); + var employeeManager = ModelMetadataIdentity.ForProperty(typeof(Employee), nameof(Employee.Unit), modelType); + var employeeManagerMetadata = CreateModelMetadata(employeeManager, metadataProvider.Object, false); + var employeeEmployees = ModelMetadataIdentity.ForProperty(typeof(List), nameof(Employee.Employees), modelType); + var employeeEmployeesMetadata = CreateModelMetadata(employeeEmployees, metadataProvider.Object, false); + + var unitHead = ModelMetadataIdentity.ForProperty(typeof(Employee), nameof(BusinessUnit.Head), modelType); + var unitHeadMetadata = CreateModelMetadata(unitHead, metadataProvider.Object, false); + var unitId = ModelMetadataIdentity.ForProperty(typeof(int), nameof(BusinessUnit.Id), modelType); + var unitIdMetadata = CreateModelMetadata(unitId, metadataProvider.Object, true); // BusinessUnit.Id has validators. + + metadataProvider + .Setup(mp => mp.GetMetadataForProperties(modelType)) + .Returns(new[] { employeeIdMetadata, employeeUnitMetadata, employeeManagerMetadata, employeeEmployeesMetadata, }) + .Verifiable(); + + metadataProvider + .Setup(mp => mp.GetMetadataForProperties(typeof(BusinessUnit))) + .Returns(new[] { unitHeadMetadata, unitIdMetadata, }) + .Verifiable(); + + // Act + var result = DefaultModelMetadata.CalculateHasValidators(new HashSet(), modelMetadata); + + // Assert + Assert.True(result); + } + + [Fact] + public void CalculateHasValidators_SelfReferencingType_HasValidatorOnSelfReferencedProperty() + { + // Arrange + var modelType = typeof(Employee); + var modelIdentity = ModelMetadataIdentity.ForType(modelType); + var metadataProvider = new Mock(); + var modelMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); + + var employeeId = ModelMetadataIdentity.ForProperty(typeof(int), nameof(Employee.Id), modelType); + var employeeIdMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); + var employeeUnit = ModelMetadataIdentity.ForProperty(typeof(BusinessUnit), nameof(Employee.Unit), modelType); + var employeeUnitMetadata = CreateModelMetadata(employeeUnit, metadataProvider.Object, false); + var employeeManager = ModelMetadataIdentity.ForProperty(typeof(Employee), nameof(Employee.Unit), modelType); + var employeeManagerMetadata = CreateModelMetadata(employeeManager, metadataProvider.Object, false); + var employeeEmployees = ModelMetadataIdentity.ForProperty(typeof(List), nameof(Employee.Employees), modelType); + var employeeEmployeesMetadata = CreateModelMetadata(employeeEmployees, metadataProvider.Object, false); + + var unitHead = ModelMetadataIdentity.ForProperty(typeof(Employee), nameof(BusinessUnit.Head), modelType); + var unitHeadMetadata = CreateModelMetadata(unitHead, metadataProvider.Object, true); // BusinessUnit.Head has validators + var unitId = ModelMetadataIdentity.ForProperty(typeof(int), nameof(BusinessUnit.Id), modelType); + var unitIdMetadata = CreateModelMetadata(unitId, metadataProvider.Object, false); + + metadataProvider + .Setup(mp => mp.GetMetadataForProperties(modelType)) + .Returns(new[] { employeeIdMetadata, employeeUnitMetadata, employeeManagerMetadata, employeeEmployeesMetadata, }); + + metadataProvider + .Setup(mp => mp.GetMetadataForProperties(typeof(BusinessUnit))) + .Returns(new[] { unitHeadMetadata, unitIdMetadata, }); + + metadataProvider + .Setup(mp => mp.GetMetadataForType(modelType)) + .Returns(modelMetadata); + + // Act + var result = DefaultModelMetadata.CalculateHasValidators(new HashSet(), modelMetadata); + + // Assert + Assert.True(result); + } + + [Fact] + public void CalculateHasValidators_CollectionElementHasValidators() + { + // Arrange + var modelType = typeof(Employee); + var modelIdentity = ModelMetadataIdentity.ForType(modelType); + var metadataProvider = new Mock(); + var modelMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); + + var employeeId = ModelMetadataIdentity.ForProperty(typeof(int), nameof(Employee.Id), modelType); + var employeeIdMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); + var employeeEmployees = ModelMetadataIdentity.ForProperty(typeof(List), nameof(Employee.Employees), modelType); + var employeeEmployeesMetadata = CreateModelMetadata(employeeEmployees, metadataProvider.Object, false); + + metadataProvider + .Setup(mp => mp.GetMetadataForProperties(modelType)) + .Returns(new[] { employeeIdMetadata, employeeEmployeesMetadata, }); + + metadataProvider + .Setup(mp => mp.GetMetadataForType(modelType)) + .Returns(CreateModelMetadata(modelIdentity, metadataProvider.Object, true)); // Employees.Employee has validators + + // Act + var result = DefaultModelMetadata.CalculateHasValidators(new HashSet(), modelMetadata); + + // Assert + Assert.True(result); + } + + [Fact] + public void CalculateHasValidators_SelfReferencingType_NoValidatorsInGraph() + { + // Arrange + var modelType = typeof(Employee); + var modelIdentity = ModelMetadataIdentity.ForType(modelType); + var metadataProvider = new Mock(); + var modelMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); + + var employeeId = ModelMetadataIdentity.ForProperty(typeof(int), nameof(Employee.Id), modelType); + var employeeIdMetadata = CreateModelMetadata(modelIdentity, metadataProvider.Object, false); + var employeeUnit = ModelMetadataIdentity.ForProperty(typeof(BusinessUnit), nameof(Employee.Unit), modelType); + var employeeUnitMetadata = CreateModelMetadata(employeeUnit, metadataProvider.Object, false); + var employeeManager = ModelMetadataIdentity.ForProperty(typeof(Employee), nameof(Employee.Unit), modelType); + var employeeManagerMetadata = CreateModelMetadata(employeeManager, metadataProvider.Object, false); + var employeeEmployeesId = ModelMetadataIdentity.ForProperty(typeof(List), nameof(Employee.Employees), modelType); + var employeeEmployeesIdMetadata = CreateModelMetadata(employeeEmployeesId, metadataProvider.Object, false); + + var unitHead = ModelMetadataIdentity.ForProperty(typeof(Employee), nameof(BusinessUnit.Head), modelType); + var unitHeadMetadata = CreateModelMetadata(unitHead, metadataProvider.Object, false); + var unitId = ModelMetadataIdentity.ForProperty(typeof(int), nameof(BusinessUnit.Id), modelType); + var unitIdMetadata = CreateModelMetadata(unitId, metadataProvider.Object, false); + + metadataProvider + .Setup(mp => mp.GetMetadataForProperties(modelType)) + .Returns(new[] { employeeIdMetadata, employeeUnitMetadata, employeeManagerMetadata, employeeEmployeesIdMetadata, }); + + metadataProvider + .Setup(mp => mp.GetMetadataForProperties(typeof(BusinessUnit))) + .Returns(new[] { unitHeadMetadata, unitIdMetadata, }); + + metadataProvider + .Setup(mp => mp.GetMetadataForType(modelType)) + .Returns(modelMetadata); + + // Act + var result = DefaultModelMetadata.CalculateHasValidators(new HashSet(), modelMetadata); + + // Assert + Assert.False(result); + } + + private static DefaultModelMetadata CreateModelMetadata( + ModelMetadataIdentity modelIdentity, + IModelMetadataProvider metadataProvider, + bool? hasValidators) + { + return new DefaultModelMetadata( + metadataProvider, + new SetHasValidatorsCompositeMetadataDetailsProvider { HasValidators = hasValidators }, + new DefaultMetadataDetails(modelIdentity, new ModelAttributes(new object[0], new object[0], new object[0]))); + } + private void ActionMethod(string input) { } @@ -921,5 +1267,41 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata public int PublicGetPublicSetProperty { get; set; } } + + public class Employee + { + public int Id { get; set; } + + public BusinessUnit Unit { get; set; } + + public Employee Manager { get; set; } + + public List Employees { get; set; } + } + + public class BusinessUnit + { + public Employee Head { get; set; } + + public int Id { get; set; } + } + + private class SetHasValidatorsCompositeMetadataDetailsProvider : ICompositeMetadataDetailsProvider + { + public bool? HasValidators { get; set; } + + public void CreateBindingMetadata(BindingMetadataProviderContext context) + { + } + + public void CreateDisplayMetadata(DisplayMetadataProviderContext context) + { + } + + public void CreateValidationMetadata(ValidationMetadataProviderContext context) + { + context.ValidationMetadata.HasValidators = HasValidators; + } + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/HasValidatorsValidationMetadataProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/HasValidatorsValidationMetadataProviderTest.cs new file mode 100644 index 0000000000..94ebad7b3b --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/HasValidatorsValidationMetadataProviderTest.cs @@ -0,0 +1,131 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; +using Moq; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata +{ + public class HasValidatorsValidationMetadataProviderTest + { + [Fact] + public void CreateValidationMetadata_DoesNotSetHasValidators_IfNonMetadataBasedProviderExists() + { + // Arrange + var validationProviders = new IModelValidatorProvider[] + { + new DefaultModelValidatorProvider(), + Mock.Of(), + }; + var metadataProvider = new HasValidatorsValidationMetadataProvider(validationProviders); + + var key = ModelMetadataIdentity.ForType(typeof(object)); + var modelAttributes = new ModelAttributes(new object[0], new object[0], new object[0]); + var context = new ValidationMetadataProviderContext(key, modelAttributes); + + // Act + metadataProvider.CreateValidationMetadata(context); + + // Assert + Assert.Null(context.ValidationMetadata.HasValidators); + } + + [Fact] + public void CreateValidationMetadata_DoesNotSetHasValidators_IfProviderIsConfigured() + { + // Arrange + var validationProviders = new IModelValidatorProvider[0]; + var metadataProvider = new HasValidatorsValidationMetadataProvider(validationProviders); + + var key = ModelMetadataIdentity.ForType(typeof(object)); + var modelAttributes = new ModelAttributes(new object[0], new object[0], new object[0]); + var context = new ValidationMetadataProviderContext(key, modelAttributes); + + // Act + metadataProvider.CreateValidationMetadata(context); + + // Assert + Assert.Null(context.ValidationMetadata.HasValidators); + } + + [Fact] + public void CreateValidationMetadata_SetsHasValidatorsToTrue_IfProviderReturnsTrue() + { + // Arrange + var metadataBasedModelValidatorProvider = new Mock(); + metadataBasedModelValidatorProvider.Setup(p => p.HasValidators(typeof(object), It.IsAny>())) + .Returns(true) + .Verifiable(); + + var validationProviders = new IModelValidatorProvider[] + { + new DefaultModelValidatorProvider(), + metadataBasedModelValidatorProvider.Object, + + }; + var metadataProvider = new HasValidatorsValidationMetadataProvider(validationProviders); + + var key = ModelMetadataIdentity.ForType(typeof(object)); + var modelAttributes = new ModelAttributes(new object[0], new object[0], new object[0]); + var context = new ValidationMetadataProviderContext(key, modelAttributes); + + // Act + metadataProvider.CreateValidationMetadata(context); + + // Assert + Assert.True(context.ValidationMetadata.HasValidators); + metadataBasedModelValidatorProvider.Verify(); + } + + [Fact] + public void CreateValidationMetadata_SetsHasValidatorsToFalse_IfNoProviderReturnsTrue() + { + // Arrange + var provider = Mock.Of(p => p.HasValidators(typeof(object), It.IsAny>()) == false); + var validationProviders = new IModelValidatorProvider[] + { + new DefaultModelValidatorProvider(), + provider, + }; + var metadataProvider = new HasValidatorsValidationMetadataProvider(validationProviders); + + var key = ModelMetadataIdentity.ForType(typeof(object)); + var modelAttributes = new ModelAttributes(new object[0], new object[0], new object[0]); + var context = new ValidationMetadataProviderContext(key, modelAttributes); + + // Act + metadataProvider.CreateValidationMetadata(context); + + // Assert + Assert.False(context.ValidationMetadata.HasValidators); + } + + [Fact] + public void CreateValidationMetadata_DoesNotOverrideExistingHasValidatorsValue() + { + // Arrange + var provider = Mock.Of(p => p.HasValidators(typeof(object), It.IsAny>()) == false); + var validationProviders = new IModelValidatorProvider[] + { + new DefaultModelValidatorProvider(), + provider, + }; + var metadataProvider = new HasValidatorsValidationMetadataProvider(validationProviders); + + var key = ModelMetadataIdentity.ForType(typeof(object)); + var modelAttributes = new ModelAttributes(new object[0], new object[0], new object[0]); + var context = new ValidationMetadataProviderContext(key, modelAttributes); + + // Initialize this value. + context.ValidationMetadata.HasValidators = true; + + // Act + metadataProvider.CreateValidationMetadata(context); + + // Assert + Assert.True(context.ValidationMetadata.HasValidators); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultModelValidatorProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Validation/DefaultModelValidatorProviderTest.cs similarity index 88% rename from test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultModelValidatorProviderTest.cs rename to test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Validation/DefaultModelValidatorProviderTest.cs index 680d46ff30..bc8e9d4aa4 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultModelValidatorProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Validation/DefaultModelValidatorProviderTest.cs @@ -6,11 +6,9 @@ using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.Linq; using Microsoft.AspNetCore.Mvc.DataAnnotations.Internal; -using Microsoft.AspNetCore.Mvc.ModelBinding; -using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Xunit; -namespace Microsoft.AspNetCore.Mvc.Internal +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation { // Integration tests for the default configuration of ModelMetadata and Validation providers public class DefaultModelValidatorProviderTest @@ -145,6 +143,34 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Single(validatorItems, v => ((DataAnnotationsModelValidator)v.Validator).Attribute is StringLengthAttribute); } + [Fact] + public void HasValidators_ReturnsTrue_IfMetadataIsIModelValidator() + { + // Arrange + var validatorProvider = new DefaultModelValidatorProvider(); + var attributes = new object[] { new RequiredAttribute(), new CustomModelValidatorAttribute(), new BindRequiredAttribute(), }; + + // Act + var result = validatorProvider.HasValidators(typeof(object), attributes); + + // Assert + Assert.True(result); + } + + [Fact] + public void HasValidators_ReturnsFalse_IfNoMetadataIsIModelValidator() + { + // Arrange + var validatorProvider = new DefaultModelValidatorProvider(); + var attributes = new object[] { new RequiredAttribute(), new BindRequiredAttribute(), }; + + // Act + var result = validatorProvider.HasValidators(typeof(object), attributes); + + // Assert + Assert.False(result); + } + private static IList GetValidatorItems(ModelMetadata metadata) { return metadata.ValidatorMetadata.Select(v => new ValidatorItem(v)).ToList(); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.TestCommon/TestModelMetadataProvider.cs b/test/Microsoft.AspNetCore.Mvc.Core.TestCommon/TestModelMetadataProvider.cs index 2548dc26c3..684875d565 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.TestCommon/TestModelMetadataProvider.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.TestCommon/TestModelMetadataProvider.cs @@ -8,6 +8,7 @@ using Microsoft.AspNetCore.Mvc.DataAnnotations; using Microsoft.AspNetCore.Mvc.DataAnnotations.Internal; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.Extensions.Localization; using Microsoft.Extensions.Options; using Xunit; @@ -37,6 +38,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding MvcCoreMvcOptionsSetup.ConfigureAdditionalModelMetadataDetailsProviders(detailsProviders); + var validationProviders = TestModelValidatorProvider.CreateDefaultProvider(); + detailsProviders.Add(new HasValidatorsValidationMetadataProvider(validationProviders.ValidatorProviders)); + var compositeDetailsProvider = new DefaultCompositeMetadataDetailsProvider(detailsProviders); return new DefaultModelMetadataProvider(compositeDetailsProvider, Options.Create(new MvcOptions())); } @@ -57,6 +61,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding detailsProviders.AddRange(providers); + var validationProviders = TestModelValidatorProvider.CreateDefaultProvider(); + detailsProviders.Add(new HasValidatorsValidationMetadataProvider(validationProviders.ValidatorProviders)); + var compositeDetailsProvider = new DefaultCompositeMetadataDetailsProvider(detailsProviders); return new DefaultModelMetadataProvider(compositeDetailsProvider, Options.Create(new MvcOptions())); } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.TestCommon/TestModelValidatorProvider.cs b/test/Microsoft.AspNetCore.Mvc.Core.TestCommon/TestModelValidatorProvider.cs index 230f7970b4..4dab51dc37 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.TestCommon/TestModelValidatorProvider.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.TestCommon/TestModelValidatorProvider.cs @@ -3,8 +3,6 @@ using System.Collections.Generic; using Microsoft.AspNetCore.Mvc.DataAnnotations; -using Microsoft.AspNetCore.Mvc.DataAnnotations.Internal; -using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.Extensions.Localization; using Microsoft.Extensions.Options; diff --git a/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/DataAnnotationsModelValidatorProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/DataAnnotationsModelValidatorProviderTest.cs similarity index 79% rename from test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/DataAnnotationsModelValidatorProviderTest.cs rename to test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/DataAnnotationsModelValidatorProviderTest.cs index e0a156e8b1..4889e7c4da 100644 --- a/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/DataAnnotationsModelValidatorProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/DataAnnotationsModelValidatorProviderTest.cs @@ -1,16 +1,18 @@ // 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.ComponentModel.DataAnnotations; using System.Linq; +using Microsoft.AspNetCore.Mvc.DataAnnotations.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.Extensions.Options; using Moq; using Xunit; -namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal +namespace Microsoft.AspNetCore.Mvc.DataAnnotations { public class DataAnnotationsModelValidatorProviderTest { @@ -110,6 +112,56 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal Assert.Single(providerContext.Results); } + [Fact] + public void HasValidators_ReturnsTrue_IfModelIsIValidatableObject() + { + // Arrange + var provider = GetProvider(); + var mockValidatable = Mock.Of(); + + // Act + var result = provider.HasValidators(mockValidatable.GetType(), Array.Empty()); + + // Assert + Assert.True(result); + } + + [Fact] + public void HasValidators_ReturnsTrue_IfMetadataContainsValidationAttribute() + { + // Arrange + var provider = GetProvider(); + var attributes = new object[] { new BindNeverAttribute(), new DummyValidationAttribute() }; + + // Act + var result = provider.HasValidators(typeof(object), attributes); + + // Assert + Assert.True(result); + } + + [Fact] + public void HasValidators_ReturnsFalse_IfNoDataAnnotationsValidationIsAvailable() + { + // Arrange + var provider = GetProvider(); + var attributes = new object[] { new BindNeverAttribute(), }; + + // Act + var result = provider.HasValidators(typeof(object), attributes); + + // Assert + Assert.False(result); + } + + private static DataAnnotationsModelValidatorProvider GetProvider() + { + return new DataAnnotationsModelValidatorProvider( + new ValidationAttributeAdapterProvider(), + Options.Create(new MvcDataAnnotationsLocalizationOptions()), + stringLocalizerFactory: null); + } + private IList GetValidatorItems(ModelMetadata metadata) { var items = new List(metadata.ValidatorMetadata.Count); diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputObjectValidationTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputObjectValidationTests.cs index 138e70f6c8..a56ee08b8e 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputObjectValidationTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputObjectValidationTests.cs @@ -262,5 +262,95 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests var ex = await Assert.ThrowsAsync(() => Client.SendAsync(requestMessage)); Assert.Equal(expected, ex.Message); } + + [Fact] + public async Task ErrorsDeserializingMalformedJson_AreReportedForModelsWithoutAnyValidationAttributes() + { + // This test verifies that for a model with ModelMetadata.HasValidators = false, we continue to get an invalid ModelState + validation + // errors from json serialization errors + // Arrange + var input = "{Id = \"This string is incomplete"; + var requestMessage = new HttpRequestMessage(HttpMethod.Post, "TestApi/PostBookWithNoValidation") + { + Content = new StringContent(input, Encoding.UTF8, "application/json"), + }; + + // Act + var response = await Client.SendAsync(requestMessage); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest); + var responseContent = await response.Content.ReadAsStringAsync(); + var validationProblemDetails = JsonConvert.DeserializeObject(responseContent); + + Assert.Collection( + validationProblemDetails.Errors, + error => + { + Assert.Empty(error.Key); + Assert.Equal(new[] { "Invalid character after parsing property name. Expected ':' but got: =. Path '', line 1, position 4." }, error.Value); + }); + } + + [Fact] + public async Task JsonValidationErrors_AreReportedForModelsWithoutAnyValidationAttributes() + { + // This test verifies that for a model with ModelMetadata.HasValidators = false, we continue to get an invalid ModelState + validation + // errors from json serialization errors + // Arrange + var input = "{Id: \"0c92bb85-cfaf-4344-8a9d-f92e88716861\"}"; + var requestMessage = new HttpRequestMessage(HttpMethod.Post, "TestApi/PostBookWithNoValidation") + { + Content = new StringContent(input, Encoding.UTF8, "application/json"), + }; + + // Act + var response = await Client.SendAsync(requestMessage); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest); + var responseContent = await response.Content.ReadAsStringAsync(); + var validationProblemDetails = JsonConvert.DeserializeObject(responseContent); + + Assert.Collection( + validationProblemDetails.Errors, + error => + { + Assert.Empty(error.Key); + Assert.Equal(new[] { "Required property 'isbn' not found in JSON. Path '', line 1, position 44." }, error.Value); + }); + } + + [Fact] + public async Task ErrorsDeserializingMalformedXml_AreReportedForModelsWithoutAnyValidationAttributes() + { + // This test verifies that for a model with ModelMetadata.HasValidators = false, we continue to get an invalid ModelState + validation + // errors from json serialization errors + // Arrange + var input = "" + + "" + + "Incomplete element" + + ""; + var requestMessage = new HttpRequestMessage(HttpMethod.Post, "TestApi/PostBookWithNoValidation") + { + Content = new StringContent(input, Encoding.UTF8, "application/xml"), + }; + + // Act + var response = await Client.SendAsync(requestMessage); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.BadRequest); + var responseContent = await response.Content.ReadAsStringAsync(); + var validationProblemDetails = JsonConvert.DeserializeObject(responseContent); + + Assert.Collection( + validationProblemDetails.Errors, + error => + { + Assert.Empty(error.Key); + Assert.Equal(new[] { "An error occurred while deserializing input data." }, error.Value); + }); + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ActionParametersIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ActionParametersIntegrationTest.cs index fcd5923101..6f84a74c88 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ActionParametersIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ActionParametersIntegrationTest.cs @@ -111,13 +111,12 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests // Read-only collection should not be updated. Assert.Empty(boundModel.Address); - // ModelState (data is can't be validated). - Assert.False(modelState.IsValid); + Assert.True(modelState.IsValid); var entry = Assert.Single(modelState); Assert.Equal("Address[0].Street", entry.Key); var state = entry.Value; Assert.NotNull(state); - Assert.Equal(ModelValidationState.Unvalidated, state.ValidationState); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); Assert.Equal("SomeStreet", state.RawValue); Assert.Equal("SomeStreet", state.AttemptedValue); } @@ -292,12 +291,12 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.Empty(boundModel.Address); // ModelState (data cannot be validated). - Assert.False(modelState.IsValid); + Assert.True(modelState.IsValid); var entry = Assert.Single(modelState); Assert.Equal("prefix.Address[0].Street", entry.Key); var state = entry.Value; Assert.NotNull(state); - Assert.Equal(ModelValidationState.Unvalidated, state.ValidationState); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); Assert.Equal("SomeStreet", state.AttemptedValue); Assert.Equal("SomeStreet", state.RawValue); } diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HasValidatorsValidationMetadataProviderIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HasValidatorsValidationMetadataProviderIntegrationTest.cs new file mode 100644 index 0000000000..102ddff705 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HasValidatorsValidationMetadataProviderIntegrationTest.cs @@ -0,0 +1,53 @@ +// 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.Linq; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.ObjectPool; +using Microsoft.Extensions.Options; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.IntegrationTests +{ + public class HasValidatorsValidationMetadataProviderIntegrationTest + { + [Fact] + public void HasValidatorsValidationMetadataProvider_IsRegisteredAfterOtherMetadataProviders() + { + // HasValidatorsValidationMetadataProvider uses values populated by other details providers to query validator providers + // This test ensures all other detail providers have had an opportunity to modify validation metadata first. + // Arrange + var serviceCollection = new ServiceCollection(); + serviceCollection.AddLogging(); + serviceCollection.AddSingleton(); + serviceCollection.AddMvc(); + var services = serviceCollection.BuildServiceProvider(); + + // Act + var options = services.GetRequiredService>(); + + Assert.IsType(options.Value.ModelMetadataDetailsProviders.Last()); + } + + [Fact] + public void HasValidatorsValidationMetadataProvider_IsRegisteredAfterUserSpecifiedMetadataProvider() + { + // Arrange + var serviceCollection = new ServiceCollection(); + serviceCollection.AddLogging(); + serviceCollection.AddSingleton(); + serviceCollection.AddMvc(mvcOptions => + { + mvcOptions.ModelMetadataDetailsProviders.Add(new SuppressChildValidationMetadataProvider(typeof(IQueryable))); + }); + var services = serviceCollection.BuildServiceProvider(); + + // Act + var options = services.GetRequiredService>(); + + Assert.IsType(options.Value.ModelMetadataDetailsProviders.Last()); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs index 200d95914d..0bf1131a36 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs @@ -373,15 +373,15 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var result = await TryUpdateModelAsync(model, string.Empty, testContext); // Assert - Assert.False(result); + Assert.True(result); // ModelState - Assert.False(modelState.IsValid); + Assert.True(modelState.IsValid); var entry = Assert.Single(modelState); Assert.Equal("Address[0].Street", entry.Key); var state = entry.Value; Assert.NotNull(state); - Assert.Equal(ModelValidationState.Unvalidated, state.ValidationState); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); Assert.Equal("SomeStreet", state.RawValue); Assert.Equal("SomeStreet", state.AttemptedValue); } @@ -402,15 +402,15 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var result = await TryUpdateModelAsync(model, "prefix", testContext); // Assert - Assert.False(result); + Assert.True(result); // ModelState - Assert.False(modelState.IsValid); + Assert.True(modelState.IsValid); var entry = Assert.Single(modelState); Assert.Equal("prefix.Address[0].Street", entry.Key); var state = entry.Value; Assert.NotNull(state); - Assert.Equal(ModelValidationState.Unvalidated, state.ValidationState); + Assert.Equal(ModelValidationState.Valid, state.ValidationState); Assert.Equal("SomeStreet", state.RawValue); Assert.Equal("SomeStreet", state.AttemptedValue); } diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryValidateModelIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryValidateModelIntegrationTest.cs index d9bb172f0e..cae150842f 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryValidateModelIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryValidateModelIntegrationTest.cs @@ -158,7 +158,6 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.Equal(2, modelStateErrors.Count); AssertErrorEquals("Property", modelStateErrors["Message"]); AssertErrorEquals("Model", modelStateErrors[""]); - } [Fact] @@ -183,7 +182,6 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var modelStateErrors = GetModelStateErrors(modelState); Assert.Single(modelStateErrors); // single error from the required attribute AssertErrorEquals("Property", modelStateErrors.Single().Value); - } [ModelLevelError] diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ValidationIntegrationTests.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ValidationIntegrationTests.cs index 9482cf918e..25deb2477d 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ValidationIntegrationTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ValidationIntegrationTests.cs @@ -2,9 +2,13 @@ // 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.Collections.ObjectModel; using System.ComponentModel.DataAnnotations; using System.IO; +using System.Linq; +using System.Reflection; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -14,6 +18,7 @@ using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; +using Newtonsoft.Json; using Newtonsoft.Json.Linq; using Xunit; @@ -1488,6 +1493,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests public string Control { get; set; } [ValidateSometimes(nameof(Control))] + [Range(0, 10)] public int ControlLength => Control.Length; } @@ -1571,6 +1577,53 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests }); } + // This type has a IPropertyValidationFilter declared on a property, but no validators. + // We should expect validation to short-circuit + private class ValidateSomePropertiesSometimesWithoutValidation + { + public string Control { get; set; } + + [ValidateSometimes(nameof(Control))] + public int ControlLength => Control.Length; + } + + [Fact] + public async Task PropertyToSometimesSkip_IsNotValidated_IfNoValidationAttributesExistButPropertyValidationFilterExists() + { + // Arrange + var parameter = new ParameterDescriptor + { + Name = "parameter", + ParameterType = typeof(ValidateSomePropertiesSometimesWithoutValidation), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(); + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var modelState = testContext.ModelState; + + // Add an entry for the ControlLength property so that we can observe Skipped versus Valid states. + modelState.SetModelValue( + nameof(ValidateSomePropertiesSometimes.ControlLength), + rawValue: null, + attemptedValue: null); + + // Act + var result = await parameterBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(result.IsModelSet); + var model = Assert.IsType(result.Model); + Assert.Null(model.Control); + + // Note this Exception is not thrown earlier. + Assert.Throws(() => model.ControlLength); + + Assert.True(modelState.IsValid); + var kvp = Assert.Single(modelState); + Assert.Equal(nameof(ValidateSomePropertiesSometimesWithoutValidation.ControlLength), kvp.Key); + Assert.Equal(ModelValidationState.Valid, kvp.Value.ValidationState); + } + private class Order11 { public IEnumerable
ShippingAddresses { get; set; } @@ -1838,6 +1891,550 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests public string Message { get; set; } } + [Fact] + public async Task Validation_NoAttributeInGraphOfObjects_WithDefaultValidatorProviders() + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(Order12), + BindingInfo = new BindingInfo + { + BindingSource = BindingSource.Body + }, + }; + + var input = new Order12 + { + Id = 10, + OrderFile = new byte[40], + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.Body = new MemoryStream(Encoding.UTF8.GetBytes(JsonConvert.SerializeObject(input))); + request.ContentType = "application/json"; + }); + + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + + var model = Assert.IsType(modelBindingResult.Model); + Assert.Equal(input.Id, model.Id); + Assert.Equal(input.OrderFile, model.OrderFile); + Assert.Null(model.RelatedOrders); + + Assert.Empty(modelState); + Assert.Equal(ModelValidationState.Valid, modelState.ValidationState); + } + + private class Order12 + { + public int Id { get; set; } + + public byte[] OrderFile { get; set; } + + public IList RelatedOrders { get; set; } + } + + [Fact] + public async Task Validation_ListOfType_NoValidatorOnParameter() + { + // Arrange + var parameterInfo = GetType().GetMethod(nameof(Validation_ListOfType_NoValidatorOnParameterTestMethod), BindingFlags.NonPublic | BindingFlags.Static) + .GetParameters() + .First(); + + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var modelMetadata = modelMetadataProvider.GetMetadataForParameter(parameterInfo); + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(modelMetadataProvider); + + var parameter = new ParameterDescriptor() + { + Name = parameterInfo.Name, + ParameterType = parameterInfo.ParameterType, + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.QueryString = new QueryString("?[0]=1&[1]=2"); + }); + + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext, modelMetadataProvider, modelMetadata); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + + var model = Assert.IsType>(modelBindingResult.Model); + Assert.Equal(new[] { 1, 2 }, model); + + Assert.False(modelMetadata.HasValidators); + + Assert.True(modelState.IsValid); + Assert.Equal(ModelValidationState.Valid, modelState.ValidationState); + + var entry = Assert.Single(modelState, e => e.Key == "[0]").Value; + Assert.Equal("1", entry.AttemptedValue); + Assert.Equal("1", entry.RawValue); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + + entry = Assert.Single(modelState, e => e.Key == "[1]").Value; + Assert.Equal("2", entry.AttemptedValue); + Assert.Equal("2", entry.RawValue); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + } + + private static void Validation_ListOfType_NoValidatorOnParameterTestMethod(List parameter) { } + + [Fact] + public async Task Validation_ListOfType_ValidatorOnParameter() + { + // Arrange + var parameterInfo = GetType().GetMethod(nameof(Validation_ListOfType_ValidatorOnParameterTestMethod), BindingFlags.NonPublic | BindingFlags.Static) + .GetParameters() + .First(); + + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var modelMetadata = modelMetadataProvider.GetMetadataForParameter(parameterInfo); + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(modelMetadataProvider); + + var parameter = new ParameterDescriptor() + { + Name = parameterInfo.Name, + ParameterType = parameterInfo.ParameterType, + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.QueryString = new QueryString("?[0]=1&[1]=2"); + }); + + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext, modelMetadataProvider, modelMetadata); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + + var model = Assert.IsType>(modelBindingResult.Model); + Assert.Equal(new[] { 1, 2 }, model); + + Assert.True(modelMetadata.HasValidators); + + Assert.False(modelState.IsValid); + Assert.Equal(ModelValidationState.Invalid, modelState.ValidationState); + + var entry = Assert.Single(modelState, e => e.Key == "").Value; + Assert.Equal(ModelValidationState.Invalid, entry.ValidationState); + + entry = Assert.Single(modelState, e => e.Key == "[0]").Value; + Assert.Equal("1", entry.AttemptedValue); + Assert.Equal("1", entry.RawValue); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + + entry = Assert.Single(modelState, e => e.Key == "[1]").Value; + Assert.Equal("2", entry.AttemptedValue); + Assert.Equal("2", entry.RawValue); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + } + + private static void Validation_ListOfType_ValidatorOnParameterTestMethod([ConsistentMinLength(3)] List parameter) { } + + private class ConsistentMinLength : ValidationAttribute + { + private readonly int _length; + + public ConsistentMinLength(int length) + { + _length = length; + } + + public override bool IsValid(object value) + { + return value is ICollection collection && collection.Count >= _length; + } + } + + [Fact] + public async Task Validation_CollectionOfType_ValidatorOnElement() + { + // Arrange + var parameterInfo = GetType().GetMethod(nameof(Validation_CollectionOfType_ValidatorOnElementTestMethod), BindingFlags.NonPublic | BindingFlags.Static) + .GetParameters() + .First(); + + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var modelMetadata = modelMetadataProvider.GetMetadataForParameter(parameterInfo); + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(modelMetadataProvider); + + var parameter = new ParameterDescriptor() + { + Name = parameterInfo.Name, + ParameterType = parameterInfo.ParameterType, + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.QueryString = new QueryString("?p[0].Id=1&p[1].Id=2"); + }); + + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext, modelMetadataProvider, modelMetadata); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + + var model = Assert.IsType>(modelBindingResult.Model); + Assert.Equal(1, model[0].Id); + Assert.Equal(2, model[1].Id); + + Assert.True(modelMetadata.HasValidators); + + Assert.False(modelState.IsValid); + Assert.Equal(ModelValidationState.Invalid, modelState.ValidationState); + + var entry = Assert.Single(modelState, e => e.Key == "p[0].Id").Value; + Assert.Equal("1", entry.AttemptedValue); + Assert.Equal("1", entry.RawValue); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + + entry = Assert.Single(modelState, e => e.Key == "p[1]").Value; + Assert.Equal(ModelValidationState.Invalid, entry.ValidationState); + + entry = Assert.Single(modelState, e => e.Key == "p[1].Id").Value; + Assert.Equal("2", entry.AttemptedValue); + Assert.Equal("2", entry.RawValue); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + } + + private static void Validation_CollectionOfType_ValidatorOnElementTestMethod(Collection p) { } + + public class InvalidEvenIds : IValidatableObject + { + public int Id { get; set; } + + public IEnumerable Validate(ValidationContext validationContext) + { + if (Id % 2 == 0) + { + yield return new ValidationResult("Failed validation"); + } + } + } + + [Fact] + public async Task Validation_DictionaryType_NoValidators() + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(IDictionary) + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.QueryString = new QueryString("?parameter[0].Key=key0¶meter[0].Value=10"); + }); + + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + + var model = Assert.IsType>(modelBindingResult.Model); + Assert.Collection( + model.OrderBy(k => k.Key), + kvp => + { + Assert.Equal("key0", kvp.Key); + Assert.Equal(10, kvp.Value); + }); + + Assert.True(modelState.IsValid); + Assert.Equal(ModelValidationState.Valid, modelState.ValidationState); + + var entry = Assert.Single(modelState, e => e.Key == "parameter[0].Key").Value; + Assert.Equal("key0", entry.AttemptedValue); + Assert.Equal("key0", entry.RawValue); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + + entry = Assert.Single(modelState, e => e.Key == "parameter[0].Value").Value; + Assert.Equal("10", entry.AttemptedValue); + Assert.Equal("10", entry.RawValue); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + } + + [Fact] + public async Task Validation_DictionaryType_ValueHasValidators() + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(Dictionary) + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.QueryString = new QueryString("?parameter[0].Key=key0¶meter[0].Value.NeverValidProperty=value0"); + }); + + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + + var model = Assert.IsType>(modelBindingResult.Model); + Assert.Collection( + model.OrderBy(k => k.Key), + kvp => + { + Assert.Equal("key0", kvp.Key); + Assert.Equal("value0", kvp.Value.NeverValidProperty); + }); + + Assert.False(modelState.IsValid); + Assert.Equal(ModelValidationState.Invalid, modelState.ValidationState); + + var entry = Assert.Single(modelState, e => e.Key == "parameter[0].Key").Value; + Assert.Equal("key0", entry.AttemptedValue); + Assert.Equal("key0", entry.RawValue); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + + entry = Assert.Single(modelState, e => e.Key == "parameter[0].Value.NeverValidProperty").Value; + Assert.Equal("value0", entry.AttemptedValue); + Assert.Equal("value0", entry.RawValue); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + + entry = Assert.Single(modelState, e => e.Key == "parameter[0].Value").Value; + Assert.Equal(ModelValidationState.Invalid, entry.ValidationState); + Assert.Single(entry.Errors); + } + + [Fact] + public async Task Validation_TopLevelProperty_NoValidation() + { + // Arrange + var modelType = typeof(Validation_TopLevelPropertyController); + var propertyInfo = modelType.GetProperty(nameof(Validation_TopLevelPropertyController.Model)); + + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var modelMetadata = modelMetadataProvider.GetMetadataForProperty(propertyInfo, propertyInfo.PropertyType); + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(modelMetadataProvider); + + var parameter = new ParameterDescriptor() + { + Name = propertyInfo.Name, + ParameterType = propertyInfo.PropertyType, + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.QueryString = new QueryString("?Model.Id=12"); + }); + + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext, modelMetadataProvider, modelMetadata); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + + var model = Assert.IsType(modelBindingResult.Model); + Assert.Equal(12, model.Id); + + Assert.False(modelMetadata.HasValidators); + + Assert.True(modelState.IsValid); + Assert.Equal(ModelValidationState.Valid, modelState.ValidationState); + + var entry = Assert.Single(modelState, e => e.Key == "Model.Id").Value; + Assert.Equal("12", entry.AttemptedValue); + Assert.Equal("12", entry.RawValue); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + } + + public class Validation_TopLevelPropertyModel + { + public int Id { get; set; } + } + + private class Validation_TopLevelPropertyController + { + public Validation_TopLevelPropertyModel Model { get; set; } + } + + [Fact] + public async Task Validation_TopLevelProperty_ValidationOnProperty() + { + // Arrange + var modelType = typeof(Validation_TopLevelProperty_ValidationOnPropertyController); + var propertyInfo = modelType.GetProperty(nameof(Validation_TopLevelProperty_ValidationOnPropertyController.Model)); + + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var modelMetadata = modelMetadataProvider.GetMetadataForProperty(propertyInfo, propertyInfo.PropertyType); + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(modelMetadataProvider); + + var parameter = new ParameterDescriptor() + { + Name = propertyInfo.Name, + ParameterType = propertyInfo.PropertyType, + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.QueryString = new QueryString("?Model.Id=12"); + }); + + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext, modelMetadataProvider, modelMetadata); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + + var model = Assert.IsType(modelBindingResult.Model); + Assert.Equal(12, model.Id); + + Assert.True(modelMetadata.HasValidators); + + Assert.False(modelState.IsValid); + Assert.Equal(ModelValidationState.Invalid, modelState.ValidationState); + + var entry = Assert.Single(modelState, e => e.Key == "Model.Id").Value; + Assert.Equal("12", entry.AttemptedValue); + Assert.Equal("12", entry.RawValue); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + + entry = Assert.Single(modelState, e => e.Key == "Model").Value; + Assert.Equal(ModelValidationState.Invalid, entry.ValidationState); + } + + public class Validation_TopLevelProperty_ValidationOnPropertyController + { + [CustomValidation(typeof(Validation_TopLevelProperty_ValidationOnPropertyController), nameof(Validate))] + public Validation_TopLevelPropertyModel Model { get; set; } + + public static ValidationResult Validate(ValidationContext context) + { + return new ValidationResult("Invalid result"); + } + } + + [Fact] + public async Task Validation_InfinitelyRecursiveType_NoValidators() + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(RecursiveModel) + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.QueryString = new QueryString("?Property1=8"); + }); + + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + + var model = Assert.IsType(modelBindingResult.Model); + Assert.Equal(8, model.Property1); + + Assert.True(modelState.IsValid); + Assert.Equal(ModelValidationState.Valid, modelState.ValidationState); + + var entry = Assert.Single(modelState, e => e.Key == "Property1").Value; + Assert.Equal("8", entry.AttemptedValue); + Assert.Equal("8", entry.RawValue); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + } + + public class RecursiveModel + { + public int Property1 { get; set; } + + public RecursiveModel Property2 { get; set; } + + public RecursiveModel Property3 => new RecursiveModel { Property1 = Property1 }; + } + + [Fact] + public async Task Validation_InifnitelyRecursiveModel_ValidationOnTopLevelParameter() + { + // Arrange + var parameterInfo = GetType().GetMethod(nameof(Validation_InifnitelyRecursiveModel_ValidationOnTopLevelParameterMethod), BindingFlags.NonPublic | BindingFlags.Static) + .GetParameters() + .First(); + + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var modelMetadata = modelMetadataProvider.GetMetadataForParameter(parameterInfo); + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(modelMetadataProvider); + + var parameter = new ParameterDescriptor() + { + Name = parameterInfo.Name, + ParameterType = parameterInfo.ParameterType, + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.QueryString = new QueryString("?Property1=8"); + }); + + var modelState = testContext.ModelState; + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext, modelMetadataProvider, modelMetadata); + + // Assert + Assert.True(modelBindingResult.IsModelSet); + + var model = Assert.IsType(modelBindingResult.Model); + Assert.Equal(8, model.Property1); + + Assert.True(modelState.IsValid); + Assert.Equal(ModelValidationState.Valid, modelState.ValidationState); + + var entry = Assert.Single(modelState, e => e.Key == "Property1").Value; + Assert.Equal("8", entry.AttemptedValue); + Assert.Equal("8", entry.RawValue); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + } + + private static void Validation_InifnitelyRecursiveModel_ValidationOnTopLevelParameterMethod([Required] RecursiveModel model) { } + private static void AssertRequiredError(string key, ModelError error) { Assert.Equal(ValidationAttributeUtil.GetRequiredErrorMessage(key), error.ErrorMessage); diff --git a/test/Microsoft.AspNetCore.Mvc.Test/MvcOptionsSetupTest.cs b/test/Microsoft.AspNetCore.Mvc.Test/MvcOptionsSetupTest.cs index e43d50af95..53ba4f52fb 100644 --- a/test/Microsoft.AspNetCore.Mvc.Test/MvcOptionsSetupTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Test/MvcOptionsSetupTest.cs @@ -14,6 +14,7 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.JsonPatch; using Microsoft.AspNetCore.Mvc.ApplicationParts; +using Microsoft.AspNetCore.Mvc.DataAnnotations; using Microsoft.AspNetCore.Mvc.DataAnnotations.Internal; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Internal; @@ -251,7 +252,8 @@ namespace Microsoft.AspNetCore.Mvc { var excludeFilter = Assert.IsType(provider); Assert.Equal(typeof(XmlNode).FullName, excludeFilter.FullTypeName); - }); + }, + provider => Assert.IsType(provider)); } private static T GetOptions(Action action = null) diff --git a/test/Microsoft.AspNetCore.Mvc.Test/MvcServiceCollectionExtensionsTest.cs b/test/Microsoft.AspNetCore.Mvc.Test/MvcServiceCollectionExtensionsTest.cs index b4c06b2613..70fea5804a 100644 --- a/test/Microsoft.AspNetCore.Mvc.Test/MvcServiceCollectionExtensionsTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Test/MvcServiceCollectionExtensionsTest.cs @@ -18,6 +18,7 @@ using Microsoft.AspNetCore.Mvc.DataAnnotations.Internal; using Microsoft.AspNetCore.Mvc.Filters; using Microsoft.AspNetCore.Mvc.Formatters.Json; using Microsoft.AspNetCore.Mvc.Formatters.Json.Internal; +using Microsoft.AspNetCore.Mvc.Infrastructure; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.Razor; using Microsoft.AspNetCore.Mvc.Razor.Compilation; @@ -381,14 +382,15 @@ namespace Microsoft.AspNetCore.Mvc typeof(IPostConfigureOptions), new[] { - typeof(MvcOptions).Assembly.GetType("Microsoft.AspNetCore.Mvc.Infrastructure.MvcOptionsConfigureCompatibilityOptions", throwOnError: true), + typeof(MvcOptionsConfigureCompatibilityOptions), + typeof(MvcCoreMvcOptionsSetup), } }, { typeof(IPostConfigureOptions), new[] { - typeof(RazorPagesOptions).Assembly.GetType("Microsoft.AspNetCore.Mvc.RazorPages.RazorPagesOptionsConfigureCompatibilityOptions", throwOnError: true), + typeof(RazorPagesOptionsConfigureCompatibilityOptions), } }, { diff --git a/test/WebSites/FormatterWebSite/Controllers/TestApiController.cs b/test/WebSites/FormatterWebSite/Controllers/TestApiController.cs new file mode 100644 index 0000000000..832a5917c6 --- /dev/null +++ b/test/WebSites/FormatterWebSite/Controllers/TestApiController.cs @@ -0,0 +1,16 @@ +// 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 FormatterWebSite.Models; +using Microsoft.AspNetCore.Mvc; + +namespace FormatterWebSite.Controllers +{ + [ApiController] + [Route("[controller]/[action]")] + public class TestApiController : ControllerBase + { + [HttpPost] + public IActionResult PostBookWithNoValidation(BookModelWithNoValidation bookModel) => Ok(); + } +} diff --git a/test/WebSites/FormatterWebSite/Models/BookModelWithNoValidation.cs b/test/WebSites/FormatterWebSite/Models/BookModelWithNoValidation.cs new file mode 100644 index 0000000000..ad13253e03 --- /dev/null +++ b/test/WebSites/FormatterWebSite/Models/BookModelWithNoValidation.cs @@ -0,0 +1,20 @@ +// 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.Runtime.Serialization; +using Newtonsoft.Json; + +namespace FormatterWebSite.Models +{ + public class BookModelWithNoValidation + { + public Guid Id { get; set; } + + public string Title { get; set; } + + [JsonRequired] + [DataMember(IsRequired = true)] + public string ISBN { get; set; } + } +} diff --git a/test/WebSites/FormatterWebSite/Models/RecursiveIdentifier.cs b/test/WebSites/FormatterWebSite/Models/RecursiveIdentifier.cs index 847a01b428..49e8ab2e91 100644 --- a/test/WebSites/FormatterWebSite/Models/RecursiveIdentifier.cs +++ b/test/WebSites/FormatterWebSite/Models/RecursiveIdentifier.cs @@ -1,10 +1,14 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.Linq; + namespace FormatterWebSite { // A System.Security.Principal.SecurityIdentifier like type that works on xplat - public class RecursiveIdentifier + public class RecursiveIdentifier : IValidatableObject { public RecursiveIdentifier(string identifier) { @@ -14,5 +18,10 @@ namespace FormatterWebSite public string Value { get; } public RecursiveIdentifier AccountIdentifier => new RecursiveIdentifier(Value); + + public IEnumerable Validate(ValidationContext validationContext) + { + return Enumerable.Empty(); + } } } \ No newline at end of file