From 236ef5d1d1d12a2503c6da0900b3c40ca5470985 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Fri, 22 Sep 2017 10:13:48 +0100 Subject: [PATCH] Support validation and BindBehavior on top-level action parameters and bound properties. Fixes #6790 --- .../Metadata/ModelMetadataIdentity.cs | 32 +- .../Metadata/ModelMetadataKind.cs | 5 + .../ModelBinding/ModelMetadataProvider.cs | 37 ++ .../MvcCoreServiceCollectionExtensions.cs | 9 +- .../ControllerBinderDelegateProvider.cs | 35 +- .../DefaultBindingMetadataProvider.cs | 40 +- .../ModelBinding/BindNeverAttribute.cs | 2 +- .../ModelBinding/BindRequiredAttribute.cs | 2 +- .../ModelBinding/BindingBehaviorAttribute.cs | 2 +- .../BindingMetadataProviderContext.cs | 6 + .../Metadata/DefaultModelMetadata.cs | 6 +- .../Metadata/DefaultModelMetadataProvider.cs | 83 ++-- .../ModelBinding/Metadata/ModelAttributes.cs | 99 +++-- .../ModelBinding/ParameterBinder.cs | 103 ++++- .../Validation/ValidationVisitor.cs | 23 +- .../Internal/DataAnnotationsModelValidator.cs | 3 +- .../ControllerActionInvokerCacheTest.cs | 2 +- .../ControllerBinderDelegateProviderTest.cs | 359 ++++++++++++++---- .../DefaultBindingMetadataProviderTest.cs | 204 ++++++++-- .../Internal/MiddlewareFilterTest.cs | 2 +- .../BindingSourceMetadataProviderTests.cs | 2 +- .../DefaultModelMetadataProviderTest.cs | 62 +++ .../Metadata/DefaultModelMetadataTest.cs | 58 +-- .../DefaultValidationMetadataProviderTest.cs | 16 +- .../ExcludeBindingMetadataProviderTest.cs | 4 +- .../Metadata/ModelAttributesTest.cs | 47 +++ .../ModelBinding/ParameterBinderTest.cs | 245 +++++++++++- .../DataAnnotationsMetadataProviderTest.cs | 60 +-- .../DataAnnotationsModelValidatorTest.cs | 48 +++ ...mberRequiredBindingMetadataProviderTest.cs | 10 +- .../Internal/ModelMetadataProviderTest.cs | 4 +- .../InputValidationTests.cs | 120 ++++++ .../ActionParametersIntegrationTest.cs | 156 ++++++++ .../BindPropertyIntegrationTest.cs | 141 +++++++ .../ModelBindingTestHelper.cs | 30 +- .../ParameterBinderExtensions.cs | 26 ++ .../Internal/PageActionInvokerProviderTest.cs | 2 +- .../Internal/PageActionInvokerTest.cs | 15 +- .../Internal/PagePropertyBinderFactoryTest.cs | 14 +- .../TestModelMetadataProvider.cs | 2 +- .../TopLevelValidationController.cs | 62 +++ 41 files changed, 1874 insertions(+), 304 deletions(-) create mode 100644 src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadataProvider.cs create mode 100644 test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputValidationTests.cs create mode 100644 test/WebSites/FormatterWebSite/Controllers/TopLevelValidationController.cs diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Metadata/ModelMetadataIdentity.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Metadata/ModelMetadataIdentity.cs index a824e171d6..782b84b2cd 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Metadata/ModelMetadataIdentity.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Metadata/ModelMetadataIdentity.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Reflection; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.Extensions.Internal; @@ -65,6 +66,21 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata }; } + public static ModelMetadataIdentity ForParameter(ParameterInfo parameter) + { + if (parameter == null) + { + throw new ArgumentNullException(nameof(parameter)); + } + + return new ModelMetadataIdentity() + { + Name = parameter.Name, + ModelType = parameter.ParameterType, + ParameterInfo = parameter, + }; + } + /// /// Gets the defining the model property represented by the current /// instance, or null if the current instance does not represent a property. @@ -83,7 +99,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata { get { - if (ContainerType != null && Name != null) + if (ParameterInfo != null) + { + return ModelMetadataKind.Parameter; + } + else if (ContainerType != null && Name != null) { return ModelMetadataKind.Property; } @@ -100,13 +120,20 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata /// public string Name { get; private set; } + /// + /// Gets a descriptor for the parameter, or null if this instance + /// does not represent a parameter. + /// + public ParameterInfo ParameterInfo { get; private set; } + /// public bool Equals(ModelMetadataIdentity other) { return ContainerType == other.ContainerType && ModelType == other.ModelType && - Name == other.Name; + Name == other.Name && + ParameterInfo == other.ParameterInfo; } /// @@ -123,6 +150,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata hash.Add(ContainerType); hash.Add(ModelType); hash.Add(Name, StringComparer.Ordinal); + hash.Add(ParameterInfo); return hash; } } diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Metadata/ModelMetadataKind.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Metadata/ModelMetadataKind.cs index 3eb836cc18..4756f85226 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Metadata/ModelMetadataKind.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Metadata/ModelMetadataKind.cs @@ -17,5 +17,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata /// Used for for a property. /// Property, + + /// + /// Used for for a parameter. + /// + Parameter, } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadataProvider.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadataProvider.cs new file mode 100644 index 0000000000..f32bcf00e2 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadataProvider.cs @@ -0,0 +1,37 @@ +// 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.Reflection; +using Microsoft.AspNetCore.Mvc.Abstractions; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding +{ + /// + /// A provider that can supply instances of . + /// + public abstract class ModelMetadataProvider : IModelMetadataProvider + { + /// + /// Supplies metadata describing the properties of a . + /// + /// The . + /// A set of instances describing properties of the . + public abstract IEnumerable GetMetadataForProperties(Type modelType); + + /// + /// Supplies metadata describing a . + /// + /// The . + /// A instance describing the . + public abstract ModelMetadata GetMetadataForType(Type modelType); + + /// + /// Supplies metadata describing a parameter. + /// + /// The . + /// A instance describing properties of the . + public abstract ModelMetadata GetMetadataForParameter(ParameterInfo parameter); + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index a6dfebfc1e..223ada2cdd 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -231,7 +231,14 @@ namespace Microsoft.Extensions.DependencyInjection return new DefaultObjectValidator(metadataProvider, options.ModelValidatorProviders); }); services.TryAddSingleton(); - services.TryAddSingleton(); + services.TryAddSingleton(s => + { + var options = s.GetRequiredService>().Value; + var metadataProvider = s.GetRequiredService(); + var modelBinderFactory = s.GetRequiredService(); + var modelValidatorProvider = new CompositeModelValidatorProvider(options.ModelValidatorProviders); + return new ParameterBinder(metadataProvider, modelBinderFactory, modelValidatorProvider); + }); // // Random Infrastructure diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerBinderDelegateProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerBinderDelegateProvider.cs index 16bfca522b..06c493b79c 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerBinderDelegateProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ControllerBinderDelegateProvider.cs @@ -51,13 +51,19 @@ namespace Microsoft.AspNetCore.Mvc.Internal { var parameter = parameters[i]; var bindingInfo = parameterBindingInfo[i]; + var modelMetadata = bindingInfo.ModelMetadata; + + if (!modelMetadata.IsBindingAllowed) + { + continue; + } var result = await parameterBinder.BindModelAsync( controllerContext, bindingInfo.ModelBinder, valueProvider, parameter, - bindingInfo.ModelMetadata, + modelMetadata, value: null); if (result.IsModelSet) @@ -71,13 +77,19 @@ namespace Microsoft.AspNetCore.Mvc.Internal { var property = properties[i]; var bindingInfo = propertyBindingInfo[i]; + var modelMetadata = bindingInfo.ModelMetadata; + + if (!modelMetadata.IsBindingAllowed) + { + continue; + } var result = await parameterBinder.BindModelAsync( controllerContext, bindingInfo.ModelBinder, valueProvider, property, - bindingInfo.ModelMetadata, + modelMetadata, value: null); if (result.IsModelSet) @@ -103,7 +115,24 @@ namespace Microsoft.AspNetCore.Mvc.Internal for (var i = 0; i < parameters.Count; i++) { var parameter = parameters[i]; - var metadata = modelMetadataProvider.GetMetadataForType(parameter.ParameterType); + + ModelMetadata metadata; + if (modelMetadataProvider is ModelMetadataProvider modelMetadataProviderBase + && parameter is ControllerParameterDescriptor controllerParameterDescriptor) + { + // The default model metadata provider derives from ModelMetadataProvider + // and can therefore supply information about attributes applied to parameters. + metadata = modelMetadataProviderBase.GetMetadataForParameter(controllerParameterDescriptor.ParameterInfo); + } + else + { + // For backward compatibility, if there's a custom model metadata provider that + // only implements the older IModelMetadataProvider interface, access the more + // limited metadata information it supplies. In this scenario, validation attributes + // are not supported on parameters. + metadata = modelMetadataProvider.GetMetadataForType(parameter.ParameterType); + } + var binder = modelBinderFactory.CreateBinder(new ModelBinderFactoryContext { BindingInfo = parameter.BindingInfo, diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultBindingMetadataProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultBindingMetadataProvider.cs index 7d034e1200..8e0546b992 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultBindingMetadataProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultBindingMetadataProvider.cs @@ -68,25 +68,31 @@ namespace Microsoft.AspNetCore.Mvc.Internal context.BindingMetadata.PropertyFilterProvider = composite; } - if (context.Key.MetadataKind == ModelMetadataKind.Property) + var bindingBehavior = FindBindingBehavior(context); + if (bindingBehavior != null) { - // BindingBehavior can fall back to attributes on the Container Type, but we should ignore - // attributes on the Property Type. - var bindingBehavior = context.PropertyAttributes.OfType().FirstOrDefault(); - if (bindingBehavior == null) - { - bindingBehavior = - context.Key.ContainerType.GetTypeInfo() - .GetCustomAttributes(typeof(BindingBehaviorAttribute), inherit: true) - .OfType() - .FirstOrDefault(); - } + context.BindingMetadata.IsBindingAllowed = bindingBehavior.Behavior != BindingBehavior.Never; + context.BindingMetadata.IsBindingRequired = bindingBehavior.Behavior == BindingBehavior.Required; + } + } - if (bindingBehavior != null) - { - context.BindingMetadata.IsBindingAllowed = bindingBehavior.Behavior != BindingBehavior.Never; - context.BindingMetadata.IsBindingRequired = bindingBehavior.Behavior == BindingBehavior.Required; - } + private static BindingBehaviorAttribute FindBindingBehavior(BindingMetadataProviderContext context) + { + switch (context.Key.MetadataKind) + { + case ModelMetadataKind.Property: + // BindingBehavior can fall back to attributes on the Container Type, but we should ignore + // attributes on the Property Type. + var matchingAttributes = context.PropertyAttributes.OfType(); + return matchingAttributes.FirstOrDefault() + ?? context.Key.ContainerType.GetTypeInfo() + .GetCustomAttributes(typeof(BindingBehaviorAttribute), inherit: true) + .OfType() + .FirstOrDefault(); + case ModelMetadataKind.Parameter: + return context.ParameterAttributes.OfType().FirstOrDefault(); + default: + return null; } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindNeverAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindNeverAttribute.cs index 3bcdfab9ff..12a5077d41 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindNeverAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindNeverAttribute.cs @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// system excludes that property. When applied to a type, the model binding system excludes all properties that /// type defines. /// - [AttributeUsage(AttributeTargets.Class | AttributeTargets.Property, AllowMultiple = false, Inherited = true)] + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Property | AttributeTargets.Parameter, AllowMultiple = false, Inherited = true)] public sealed class BindNeverAttribute : BindingBehaviorAttribute { /// diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindRequiredAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindRequiredAttribute.cs index b98f120e12..247dde0e26 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindRequiredAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindRequiredAttribute.cs @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// requires a value for that property. When applied to a type, the model binding system requires values for all /// properties that type defines. /// - [AttributeUsage(AttributeTargets.Class | AttributeTargets.Property, AllowMultiple = false, Inherited = true)] + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Property | AttributeTargets.Parameter, AllowMultiple = false, Inherited = true)] public sealed class BindRequiredAttribute : BindingBehaviorAttribute { /// diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindingBehaviorAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindingBehaviorAttribute.cs index 84492d5806..b27c8451d4 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindingBehaviorAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindingBehaviorAttribute.cs @@ -8,7 +8,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// /// Specifies the that should be applied. /// - [AttributeUsage(AttributeTargets.Class | AttributeTargets.Property, AllowMultiple = false, Inherited = true)] + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Property | AttributeTargets.Parameter, AllowMultiple = false, Inherited = true)] public class BindingBehaviorAttribute : Attribute { /// diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/BindingMetadataProviderContext.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/BindingMetadataProviderContext.cs index 24fdc2475f..587cf6fb69 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/BindingMetadataProviderContext.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/BindingMetadataProviderContext.cs @@ -27,6 +27,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata Key = key; Attributes = attributes.Attributes; + ParameterAttributes = attributes.ParameterAttributes; PropertyAttributes = attributes.PropertyAttributes; TypeAttributes = attributes.TypeAttributes; @@ -43,6 +44,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata /// public ModelMetadataIdentity Key { get; } + /// + /// Gets the parameter attributes. + /// + public IReadOnlyList ParameterAttributes { get; } + /// /// Gets the property attributes. /// diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs index 91069d3d3e..3b8da4a8c8 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs @@ -259,13 +259,13 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata { get { - if (MetadataKind == ModelMetadataKind.Property) + if (MetadataKind == ModelMetadataKind.Type) { - return BindingMetadata.IsBindingAllowed; + return true; } else { - return true; + return BindingMetadata.IsBindingAllowed; } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadataProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadataProvider.cs index a3e829c105..e71563e60a 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadataProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadataProvider.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; using System.Reflection; +using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.Extensions.Internal; using Microsoft.Extensions.Options; @@ -13,7 +14,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata /// /// A default implementation of based on reflection. /// - public class DefaultModelMetadataProvider : IModelMetadataProvider + public class DefaultModelMetadataProvider : ModelMetadataProvider { private readonly TypeCache _typeCache = new TypeCache(); private readonly Func _cacheEntryFactory; @@ -68,7 +69,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata protected DefaultModelBindingMessageProvider ModelBindingMessageProvider { get; } /// - public virtual IEnumerable GetMetadataForProperties(Type modelType) + public override IEnumerable GetMetadataForProperties(Type modelType) { if (modelType == null) { @@ -97,8 +98,20 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata return cacheEntry.Details.Properties; } + public override ModelMetadata GetMetadataForParameter(ParameterInfo parameter) + { + if (parameter == null) + { + throw new ArgumentNullException(nameof(parameter)); + } + + var cacheEntry = GetCacheEntry(parameter); + + return cacheEntry.Metadata; + } + /// - public virtual ModelMetadata GetMetadataForType(Type modelType) + public override ModelMetadata GetMetadataForType(Type modelType) { if (modelType == null) { @@ -139,9 +152,25 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata return cacheEntry; } + private ModelMetadataCacheEntry GetCacheEntry(ParameterInfo parameter) + { + return _typeCache.GetOrAdd( + ModelMetadataIdentity.ForParameter(parameter), + _cacheEntryFactory); + } + private ModelMetadataCacheEntry CreateCacheEntry(ModelMetadataIdentity key) { - var details = CreateTypeDetails(key); + DefaultMetadataDetails details; + if (key.MetadataKind == ModelMetadataKind.Parameter) + { + details = CreateParameterDetails(key); + } + else + { + details = CreateTypeDetails(key); + } + var metadata = CreateModelMetadata(details); return new ModelMetadataCacheEntry(metadata, details); } @@ -232,15 +261,20 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata /// protected virtual DefaultMetadataDetails CreateTypeDetails(ModelMetadataIdentity key) { - return new DefaultMetadataDetails(key, ModelAttributes.GetAttributesForType(key.ModelType)); + return new DefaultMetadataDetails( + key, + ModelAttributes.GetAttributesForType(key.ModelType)); + } + + protected virtual DefaultMetadataDetails CreateParameterDetails(ModelMetadataIdentity key) + { + return new DefaultMetadataDetails( + key, + ModelAttributes.GetAttributesForParameter(key.ParameterInfo)); } private class TypeCache : ConcurrentDictionary { - public TypeCache() - : base(ModelMetadataIdentityComparer.Instance) - { - } } private struct ModelMetadataCacheEntry @@ -255,36 +289,5 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata public DefaultMetadataDetails Details { get; } } - - private class ModelMetadataIdentityComparer : IEqualityComparer - { - public static readonly ModelMetadataIdentityComparer Instance = new ModelMetadataIdentityComparer(); - - public bool Equals(ModelMetadataIdentity x, ModelMetadataIdentity y) - { - return - x.ContainerType == y.ContainerType && - x.ModelType == y.ModelType && - x.Name == y.Name; - } - - public int GetHashCode(ModelMetadataIdentity obj) - { - var hash = 17; - hash = hash * 23 + obj.ModelType.GetHashCode(); - - if (obj.ContainerType != null) - { - hash = hash * 23 + obj.ContainerType.GetHashCode(); - } - - if (obj.Name != null) - { - hash = hash * 23 + obj.Name.GetHashCode(); - } - - return hash; - } - } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ModelAttributes.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ModelAttributes.cs index f1e30efd87..0276163272 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ModelAttributes.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ModelAttributes.cs @@ -5,27 +5,26 @@ using System; using System.Collections.Generic; using System.Linq; using System.Reflection; +using Microsoft.AspNetCore.Mvc.Controllers; +using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; namespace Microsoft.AspNetCore.Mvc.ModelBinding { /// - /// Provides access to the combined list of attributes associated a or property. + /// Provides access to the combined list of attributes associated with a , property, or parameter. /// public class ModelAttributes { + private static readonly IEnumerable _emptyAttributesCollection = Enumerable.Empty(); + /// /// Creates a new for a . /// /// The set of attributes for the . + [Obsolete("This constructor is obsolete and will be removed in a future version. The recommended alternative is " + nameof(ModelAttributes) + "." + nameof(GetAttributesForType) + ".")] public ModelAttributes(IEnumerable typeAttributes) + : this(typeAttributes, null, null) { - if (typeAttributes == null) - { - throw new ArgumentNullException(nameof(typeAttributes)); - } - - Attributes = typeAttributes.ToArray(); - TypeAttributes = Attributes; } /// @@ -35,21 +34,57 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// /// The set of attributes for the property's . See . /// + [Obsolete("This constructor is obsolete and will be removed in a future version. The recommended alternative is " + nameof(ModelAttributes) + "." + nameof(GetAttributesForProperty) + ".")] public ModelAttributes(IEnumerable propertyAttributes, IEnumerable typeAttributes) + : this(typeAttributes, propertyAttributes, null) { - if (propertyAttributes == null) - { - throw new ArgumentNullException(nameof(propertyAttributes)); - } + } - if (typeAttributes == null) + /// + /// Creates a new . + /// + /// + /// If this instance represents a type, the set of attributes for that type. + /// If this instance represents a property, the set of attributes for the property's . + /// Otherwise, null. + /// + /// + /// If this instance represents a property, the set of attributes for that property. + /// Otherwise, null. + /// + /// + /// If this instance represents a parameter, the set of attributes for that parameter. + /// Otherwise, null. + /// + public ModelAttributes(IEnumerable typeAttributes, IEnumerable propertyAttributes, IEnumerable parameterAttributes) + { + if (propertyAttributes != null) { - throw new ArgumentNullException(nameof(typeAttributes)); - } + // Represents a property + if (typeAttributes == null) + { + throw new ArgumentNullException(nameof(typeAttributes)); + } - PropertyAttributes = propertyAttributes.ToArray(); - TypeAttributes = typeAttributes.ToArray(); - Attributes = PropertyAttributes.Concat(TypeAttributes).ToArray(); + PropertyAttributes = propertyAttributes.ToArray(); + TypeAttributes = typeAttributes.ToArray(); + Attributes = PropertyAttributes.Concat(TypeAttributes).ToArray(); + } + else if (parameterAttributes != null) + { + // Represents a parameter + Attributes = ParameterAttributes = parameterAttributes.ToArray(); + } + else if (typeAttributes != null) + { + // Represents a type + if (typeAttributes == null) + { + throw new ArgumentNullException(nameof(typeAttributes)); + } + + Attributes = TypeAttributes = typeAttributes.ToArray(); + } } /// @@ -59,15 +94,22 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public IReadOnlyList Attributes { get; } /// - /// Gets the set of attributes on the property, or null if this instance represents the attributes - /// for a . + /// Gets the set of attributes on the property, or null if this instance does not represent the attributes + /// for a property. /// public IReadOnlyList PropertyAttributes { get; } + /// + /// Gets the set of attributes on the parameter, or null if this instance does not represent the attributes + /// for a parameter. + /// + public IReadOnlyList ParameterAttributes { get; } + /// /// Gets the set of attributes on the . If this instance represents a property, /// then contains attributes retrieved from - /// . + /// . If this instance represents a parameter, then + /// the value is null. /// public IReadOnlyList TypeAttributes { get; } @@ -104,7 +146,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } } - return new ModelAttributes(propertyAttributes, typeAttributes); + return new ModelAttributes(typeAttributes, propertyAttributes, null); } /// @@ -128,7 +170,18 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding attributes = attributes.Concat(metadataType.GetTypeInfo().GetCustomAttributes()); } - return new ModelAttributes(attributes); + return new ModelAttributes(attributes, null, null); + } + + /// + /// Gets the attributes for the given . + /// + /// The for which attributes need to be resolved. + /// + /// A instance with the attributes of the . + public static ModelAttributes GetAttributesForParameter(ParameterInfo parameterInfo) + { + return new ModelAttributes(null, null, parameterInfo.GetCustomAttributes()); } private static Type GetMetadataType(Type type) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ParameterBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ParameterBinder.cs index cabb45c096..f751be5f96 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ParameterBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ParameterBinder.cs @@ -4,6 +4,7 @@ using System; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; namespace Microsoft.AspNetCore.Mvc.ModelBinding @@ -15,18 +16,55 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { private readonly IModelMetadataProvider _modelMetadataProvider; private readonly IModelBinderFactory _modelBinderFactory; - private readonly IObjectModelValidator _validator; + private readonly IObjectModelValidator _validatorForBackCompatOnly; + private readonly IModelValidatorProvider _validatorProvider; + private readonly ValidatorCache _validatorCache; /// - /// Initializes a new instance of . + /// Initializes a new instance of . + /// + /// The . + /// The . + /// The . + public ParameterBinder( + IModelMetadataProvider modelMetadataProvider, + IModelBinderFactory modelBinderFactory, + IModelValidatorProvider validatorProvider) + : this(modelMetadataProvider, modelBinderFactory, validatorProvider, null) + { + if (validatorProvider == null) + { + throw new ArgumentNullException(nameof(validatorProvider)); + } + } + + /// + /// Initializes a new instance of . /// /// The . /// The . /// The . + [Obsolete("This constructor is obsolete and will be removed in a future version. The recommended alternative is the overload that takes a " + nameof(IModelValidatorProvider) + " instead of a " + nameof(IObjectModelValidator) + ".")] public ParameterBinder( IModelMetadataProvider modelMetadataProvider, IModelBinderFactory modelBinderFactory, IObjectModelValidator validator) + : this(modelMetadataProvider, modelBinderFactory, null, validator) + { + // Note: When this obsolete constructor overload is removed, also remember + // to remove the validatorForBackCompatOnly field. + + if (validator == null) + { + throw new ArgumentNullException(nameof(validator)); + } + } + + private ParameterBinder( + IModelMetadataProvider modelMetadataProvider, + IModelBinderFactory modelBinderFactory, + IModelValidatorProvider validatorProvider, + IObjectModelValidator validatorForBackCompatOnly) { if (modelMetadataProvider == null) { @@ -38,14 +76,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding throw new ArgumentNullException(nameof(modelBinderFactory)); } - if (validator == null) - { - throw new ArgumentNullException(nameof(validator)); - } - _modelMetadataProvider = modelMetadataProvider; _modelBinderFactory = modelBinderFactory; - _validator = validator; + _validatorProvider = validatorProvider; + _validatorForBackCompatOnly = validatorForBackCompatOnly; + _validatorCache = new ValidatorCache(); } /// @@ -185,16 +220,58 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding await modelBinder.BindModelAsync(modelBindingContext); var modelBindingResult = modelBindingContext.Result; - if (modelBindingResult.IsModelSet) + + if (_validatorForBackCompatOnly != null) { - _validator.Validate( + // Since we don't have access to an IModelValidatorProvider, fall back + // on back-compatibility logic. In this scenario, top-level validation + // attributes will be ignored like they were historically. + if (modelBindingResult.IsModelSet) + { + _validatorForBackCompatOnly.Validate( + actionContext, + modelBindingContext.ValidationState, + modelBindingContext.ModelName, + modelBindingResult.Model); + } + } + else + { + EnforceBindRequiredAndValidate( actionContext, - modelBindingContext.ValidationState, - modelBindingContext.ModelName, - modelBindingResult.Model); + metadata, + modelBindingContext, + modelBindingResult); } return modelBindingResult; } + + private void EnforceBindRequiredAndValidate(ActionContext actionContext, ModelMetadata metadata, ModelBindingContext modelBindingContext, ModelBindingResult modelBindingResult) + { + if (!modelBindingResult.IsModelSet && metadata.IsBindingRequired) + { + // Enforce BindingBehavior.Required (e.g., [BindRequired]) + var modelName = modelBindingContext.FieldName; + var message = metadata.ModelBindingMessageProvider.MissingBindRequiredValueAccessor(modelName); + actionContext.ModelState.TryAddModelError(modelName, message); + } + else if (modelBindingResult.IsModelSet || metadata.IsRequired) + { + // Enforce any other validation rules + var visitor = new ValidationVisitor( + actionContext, + _validatorProvider, + _validatorCache, + _modelMetadataProvider, + modelBindingContext.ValidationState); + + visitor.Validate( + metadata, + modelBindingContext.ModelName, + modelBindingResult.Model, + alwaysValidateAtTopLevel: metadata.IsRequired); + } + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs index 76b43e8a7c..4e721c5731 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs @@ -79,7 +79,20 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation /// true if the object is valid, otherwise false. public bool Validate(ModelMetadata metadata, string key, object model) { - if (model == null && key != null) + return Validate(metadata, key, model, alwaysValidateAtTopLevel: false); + } + + /// + /// Validates a object. + /// + /// The associated with the model. + /// The model prefix key. + /// The model object. + /// If true, applies validation rules even if the top-level value is null. + /// true if the object is valid, otherwise false. + public bool Validate(ModelMetadata metadata, string key, object model, bool alwaysValidateAtTopLevel) + { + if (model == null && key != null && !alwaysValidateAtTopLevel) { var entry = _modelState[key]; if (entry != null && entry.ValidationState != ModelValidationState.Valid) @@ -128,6 +141,14 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation { var result = results[i]; var key = ModelNames.CreatePropertyModelName(_key, result.MemberName); + + // If this is a top-level parameter/property, the key would be empty, + // so use the name of the top-level property + if (string.IsNullOrEmpty(key) && _metadata.PropertyName != null) + { + key = _metadata.PropertyName; + } + _modelState.TryAddModelError(key, result.Message); } } diff --git a/src/Microsoft.AspNetCore.Mvc.DataAnnotations/Internal/DataAnnotationsModelValidator.cs b/src/Microsoft.AspNetCore.Mvc.DataAnnotations/Internal/DataAnnotationsModelValidator.cs index ad992a0d4f..790356eba3 100644 --- a/src/Microsoft.AspNetCore.Mvc.DataAnnotations/Internal/DataAnnotationsModelValidator.cs +++ b/src/Microsoft.AspNetCore.Mvc.DataAnnotations/Internal/DataAnnotationsModelValidator.cs @@ -15,6 +15,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal /// public class DataAnnotationsModelValidator : IModelValidator { + private static readonly object _emptyValidationContextInstance = new object(); private readonly IStringLocalizer _stringLocalizer; private readonly IValidationAttributeAdapterProvider _validationAttributeAdapterProvider; @@ -83,7 +84,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var container = validationContext.Container; var context = new ValidationContext( - instance: container ?? validationContext.Model, + instance: container ?? validationContext.Model ?? _emptyValidationContextInstance, serviceProvider: validationContext.ActionContext?.HttpContext?.RequestServices, items: null) { diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerCacheTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerCacheTest.cs index 7a0bdf5295..32bf7c2614 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerCacheTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerActionInvokerCacheTest.cs @@ -105,7 +105,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal new ParameterBinder( modelMetadataProvider, modelBinderFactory, - Mock.Of()), + Mock.Of()), modelBinderFactory, modelMetadataProvider, filterProviders, diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs index cc34fcfeb5..a4f90e77eb 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/ControllerBinderDelegateProviderTest.cs @@ -13,15 +13,110 @@ using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Binders; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; -using Microsoft.AspNetCore.Mvc.RazorPages; +using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.AspNetCore.Routing; using Moq; using Xunit; +using System.Linq; namespace Microsoft.AspNetCore.Mvc.Internal { public class ControllerBinderDelegateProviderTest { + [Fact] + public async Task BindActionArgumentsAsync_DoesNotAddActionArgumentsOrCallBinderOrValidator_IfBindingIsNotAllowed_OnParameter() + { + // Arrange + var actionDescriptor = GetActionDescriptor(); + actionDescriptor.Parameters.Add( + new ControllerParameterDescriptor + { + Name = "foo", + ParameterType = typeof(object), + BindingInfo = new BindingInfo(), + ParameterInfo = ParameterInfos.BindNeverParameterInfo + }); + + var controllerContext = GetControllerContext(actionDescriptor); + var arguments = new Dictionary(StringComparer.Ordinal); + + var binder = new Mock(); + binder + .Setup(b => b.BindModelAsync(It.IsAny())) + .Verifiable(); + + var mockValidator = CreateMockValidator(); + var factory = GetModelBinderFactory(binder.Object); + var controller = new TestController(); + var parameterBinder = GetParameterBinder(factory); + + // Act + var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate( + parameterBinder, + factory, + TestModelMetadataProvider.CreateDefaultProvider(), + actionDescriptor); + + await binderDelegate(controllerContext, controller, arguments); + + // Assert + Assert.Empty(arguments); + binder + .Verify(o => o.BindModelAsync( + It.IsAny()), + Times.Never()); + mockValidator + .Verify(o => o.Validate( + It.IsAny()), + Times.Never()); + } + + [Fact] + public async Task BindActionArgumentsAsync_DoesNotAddActionArgumentsOrCallBinderOrValidator_IfBindingIsNotAllowed_OnProperty() + { + // Arrange + var actionDescriptor = GetActionDescriptor(); + actionDescriptor.BoundProperties.Add( + new ParameterDescriptor + { + Name = nameof(TestController.RequiredButBindNeverProperty), + ParameterType = typeof(object) + }); + + var controllerContext = GetControllerContext(actionDescriptor); + var arguments = new Dictionary(StringComparer.Ordinal); + + var binder = new Mock(); + binder + .Setup(b => b.BindModelAsync(It.IsAny())) + .Verifiable(); + + var mockValidator = CreateMockValidator(); + var factory = GetModelBinderFactory(binder.Object); + var controller = new TestController(); + var parameterBinder = GetParameterBinder(factory); + + // Act + var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate( + parameterBinder, + factory, + TestModelMetadataProvider.CreateDefaultProvider(), + actionDescriptor); + + await binderDelegate(controllerContext, controller, arguments); + + // Assert + Assert.Empty(arguments); + binder + .Verify(o => o.BindModelAsync( + It.IsAny()), + Times.Never()); + mockValidator + .Verify(o => o.Validate( + It.IsAny()), + Times.Never()); + } + [Fact] public async Task BindActionArgumentsAsync_DoesNotAddActionArguments_IfBinderReturnsNull() { @@ -100,7 +195,6 @@ namespace Microsoft.AspNetCore.Mvc.Internal public async Task BindActionArgumentsAsync_AddsActionArguments_IfBinderReturnsNotNull() { // Arrange - Func method = foo => 1; var actionDescriptor = GetActionDescriptor(); actionDescriptor.Parameters.Add( new ParameterDescriptor @@ -143,31 +237,120 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal(value, arguments["foo"]); } + [Fact] + public async Task BindActionArgumentsAsync_GetsMetadataFromParameter() + { + // Arrange + var actionDescriptor = GetActionDescriptor(); + actionDescriptor.Parameters.Add( + new ControllerParameterDescriptor + { + Name = "foo", + ParameterType = typeof(object), + ParameterInfo = ParameterInfos.NoAttributesParameterInfo + }); + + var controllerContext = GetControllerContext(actionDescriptor); + + var mockBinder = new Mock(); + var factory = GetModelBinderFactory(mockBinder.Object); + + var parameterBinder = GetParameterBinder(factory, CreateMockValidator().Object); + var controller = new TestController(); + var arguments = new Dictionary(StringComparer.Ordinal); + + var modelMetadata = new Mock(); + modelMetadata.Setup(m => m.IsBindingAllowed).Returns(true); + var mockMetadataProvider = new Mock( + Mock.Of()); + mockMetadataProvider + .Setup(p => p.GetMetadataForParameter(ParameterInfos.NoAttributesParameterInfo)) + .Returns(modelMetadata.Object); + + // Act + var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate( + parameterBinder, + factory, + mockMetadataProvider.Object, + actionDescriptor); + + await binderDelegate(controllerContext, controller, arguments); + + // Assert + mockBinder + .Verify(o => o.BindModelAsync( + It.Is(context => context.ModelMetadata == modelMetadata.Object)), + Times.Once()); + } + + [Fact] + public async Task BindActionArgumentsAsync_GetsMetadataFromType_IsMetadataProviderIsNotDefaultMetadataProvider() + { + // Arrange + var actionDescriptor = GetActionDescriptor(); + actionDescriptor.Parameters.Add( + new ControllerParameterDescriptor + { + Name = "foo", + ParameterType = typeof(Person) + }); + + var controllerContext = GetControllerContext(actionDescriptor); + + var mockBinder = new Mock(); + var factory = GetModelBinderFactory(mockBinder.Object); + + var parameterBinder = GetParameterBinder(factory, CreateMockValidator().Object); + var controller = new TestController(); + var arguments = new Dictionary(StringComparer.Ordinal); + + var modelMetadata = new Mock(); + modelMetadata.Setup(m => m.IsBindingAllowed).Returns(true); + var mockMetadataProvider = new Mock(); + mockMetadataProvider + .Setup(p => p.GetMetadataForType(typeof(Person))) + .Returns(modelMetadata.Object); + + // Act + var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate( + parameterBinder, + factory, + mockMetadataProvider.Object, + actionDescriptor); + + await binderDelegate(controllerContext, controller, arguments); + + // Assert + mockBinder + .Verify(o => o.BindModelAsync( + It.Is(context => context.ModelMetadata == modelMetadata.Object)), + Times.Once()); + } + [Fact] public async Task BindActionArgumentsAsync_CallsValidator_IfModelBinderSucceeds() { // Arrange var actionDescriptor = GetActionDescriptor(); actionDescriptor.Parameters.Add( - new ParameterDescriptor + new ControllerParameterDescriptor { Name = "foo", ParameterType = typeof(object), + ParameterInfo = ParameterInfos.CustomValidationParameterInfo }); var controllerContext = GetControllerContext(actionDescriptor); var factory = GetModelBinderFactory("Hello"); - var mockValidator = new Mock(MockBehavior.Strict); + var mockValidator = CreateMockValidator(); mockValidator - .Setup(o => o.Validate( - It.IsAny(), - It.IsAny(), - It.IsAny(), - It.IsAny())); + .Setup(o => o.Validate(It.IsAny())) + .Returns(new[] { new ModelValidationResult("memberName", "some message") }); - var parameterBinder = GetParameterBinder(factory, mockValidator.Object); + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var parameterBinder = GetParameterBinder(factory, mockValidator.Object, modelMetadataProvider); var controller = new TestController(); var arguments = new Dictionary(StringComparer.Ordinal); @@ -175,7 +358,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate( parameterBinder, factory, - TestModelMetadataProvider.CreateDefaultProvider(), + modelMetadataProvider, actionDescriptor); await binderDelegate(controllerContext, controller, arguments); @@ -183,18 +366,19 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert mockValidator .Verify(o => o.Validate( - It.IsAny(), - It.IsAny(), - It.IsAny(), - It.IsAny()), + It.IsAny()), Times.Once()); + + Assert.False(controllerContext.ModelState.IsValid); + Assert.Equal( + "some message", + controllerContext.ModelState["memberName"].Errors.Single().ErrorMessage); } [Fact] public async Task BindActionArgumentsAsync_DoesNotCallValidator_IfModelBinderFails() { // Arrange - Func method = foo => 1; var actionDescriptor = GetActionDescriptor(); actionDescriptor.Parameters.Add( new ParameterDescriptor @@ -212,14 +396,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal .Setup(b => b.BindModelAsync(It.IsAny())) .Returns(Task.CompletedTask); - var mockValidator = new Mock(MockBehavior.Strict); - mockValidator - .Setup(o => o.Validate( - It.IsAny(), - It.IsAny(), - It.IsAny(), - It.IsAny())); - + var mockValidator = CreateMockValidator(); var factory = GetModelBinderFactory(binder.Object); var controller = new TestController(); var parameterBinder = GetParameterBinder(factory, mockValidator.Object); @@ -236,10 +413,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert mockValidator .Verify(o => o.Validate( - It.IsAny(), - It.IsAny(), - It.IsAny(), - It.IsAny()), + It.IsAny()), Times.Never()); } @@ -251,7 +425,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal actionDescriptor.BoundProperties.Add( new ParameterDescriptor { - Name = nameof(TestController.StringProperty), + Name = nameof(TestController.ValidatedProperty), ParameterType = typeof(string), }); @@ -259,22 +433,20 @@ namespace Microsoft.AspNetCore.Mvc.Internal var controller = new TestController(); var arguments = new Dictionary(StringComparer.Ordinal); - var mockValidator = new Mock(MockBehavior.Strict); + var mockValidator = CreateMockValidator(); mockValidator - .Setup(o => o.Validate( - It.IsAny(), - It.IsAny(), - It.IsAny(), - It.IsAny())); + .Setup(o => o.Validate(It.IsAny())) + .Returns(new[] { new ModelValidationResult("memberName", "some message") }); var factory = GetModelBinderFactory("Hello"); - var parameterBinder = GetParameterBinder(factory, mockValidator.Object); + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var parameterBinder = GetParameterBinder(factory, mockValidator.Object, modelMetadataProvider); // Act var binderDelegate = ControllerBinderDelegateProvider.CreateBinderDelegate( parameterBinder, factory, - TestModelMetadataProvider.CreateDefaultProvider(), + modelMetadataProvider, actionDescriptor); await binderDelegate(controllerContext, controller, arguments); @@ -282,18 +454,18 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert mockValidator .Verify(o => o.Validate( - It.IsAny(), - It.IsAny(), - It.IsAny(), - It.IsAny()), + It.IsAny()), Times.Once()); + Assert.False(controllerContext.ModelState.IsValid); + Assert.Equal( + "some message", + controllerContext.ModelState["memberName"].Errors.Single().ErrorMessage); } [Fact] public async Task BindActionArgumentsAsync_DoesNotCallValidator_ForControllerProperties_IfModelBinderFails() { // Arrange - Func method = foo => 1; var actionDescriptor = GetActionDescriptor(); actionDescriptor.BoundProperties.Add( new ParameterDescriptor @@ -311,14 +483,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal .Setup(b => b.BindModelAsync(It.IsAny())) .Returns(Task.CompletedTask); - var mockValidator = new Mock(MockBehavior.Strict); - mockValidator - .Setup(o => o.Validate( - It.IsAny(), - It.IsAny(), - It.IsAny(), - It.IsAny())); - + var mockValidator = CreateMockValidator(); var factory = GetModelBinderFactory(binder.Object); var parameterBinder = GetParameterBinder(factory, mockValidator.Object); @@ -334,10 +499,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Assert mockValidator .Verify(o => o.Validate( - It.IsAny(), - It.IsAny(), - It.IsAny(), - It.IsAny()), + It.IsAny()), Times.Never()); } @@ -845,10 +1007,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal var modelMetadataProvider = new EmptyModelMetadataProvider(); var modelBinderProvider = new BodyModelBinderProvider(new[] { Mock.Of() }, Mock.Of()); var factory = TestModelBinderFactory.CreateDefault(modelBinderProvider); + var modelValidatorProvider = new Mock(MockBehavior.Strict).Object; var parameterBinder = new Mock( new EmptyModelMetadataProvider(), factory, - CreateMockValidator()); + modelValidatorProvider); parameterBinder.Setup(p => p.BindModelAsync( It.IsAny(), It.IsAny(), @@ -959,11 +1122,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal private static ParameterBinder GetParameterBinder( IModelBinderFactory factory = null, - IObjectModelValidator validator = null) + IModelValidator validator = null, + IModelMetadataProvider modelMetadataProvider = null) { if (validator == null) { - validator = CreateMockValidator(); + validator = CreateMockValidator().Object; } if (factory == null) @@ -971,22 +1135,31 @@ namespace Microsoft.AspNetCore.Mvc.Internal factory = TestModelBinderFactory.CreateDefault(); } + var validatorProvider = new Mock(); + validatorProvider + .Setup(p => p.CreateValidators(It.IsAny())) + .Callback(context => + { + foreach (var result in context.Results) + { + result.Validator = validator; + result.IsReusable = true; + } + }); + return new ParameterBinder( - TestModelMetadataProvider.CreateDefaultProvider(), + modelMetadataProvider ?? TestModelMetadataProvider.CreateDefaultProvider(), factory, - validator); + validatorProvider.Object); } - private static IObjectModelValidator CreateMockValidator() + private static Mock CreateMockValidator() { - var mockValidator = new Mock(MockBehavior.Strict); + var mockValidator = new Mock(MockBehavior.Strict); mockValidator .Setup(o => o.Validate( - It.IsAny(), - It.IsAny(), - It.IsAny(), - It.IsAny())); - return mockValidator.Object; + It.IsAny())); + return mockValidator; } // No need for bind-related attributes on properties in this controller class. Properties are added directly @@ -1008,6 +1181,12 @@ namespace Microsoft.AspNetCore.Mvc.Internal public int NonNullableProperty { get; set; } public int? NullableProperty { get; set; } + + [CustomValidation("Test message")] public string ValidatedProperty { get; set; } + + // Despite being "required", the BindNever means this property won't be involved + // in binding, so no validation will be performed + [Required, BindNever] public string RequiredButBindNeverProperty { get; set; } } private class Person : IEquatable, IEquatable @@ -1034,5 +1213,53 @@ namespace Microsoft.AspNetCore.Mvc.Internal { public BindingSource BindingSource { get { return BindingSource.Query; } } } + + private class CustomValidationAttribute : Attribute, IModelValidator + { + public string Message { get; } + + public CustomValidationAttribute(string message) + { + Message = message; + } + + public IEnumerable Validate(ModelValidationContext context) + { + yield return new ModelValidationResult(context.ModelMetadata.BinderModelName, Message); + } + } + + private class ParameterInfos + { + public void Method( + object param1, + [BindNever] object param2, + [CustomValidation("some message")] string param3) + { + } + + public static ParameterInfo NoAttributesParameterInfo + = typeof(ParameterInfos) + .GetMethod(nameof(ParameterInfos.Method)) + .GetParameters()[0]; + + public static ParameterInfo BindNeverParameterInfo + = typeof(ParameterInfos) + .GetMethod(nameof(ParameterInfos.Method)) + .GetParameters()[1]; + + public static ParameterInfo CustomValidationParameterInfo + = typeof(ParameterInfos) + .GetMethod(nameof(ParameterInfos.Method)) + .GetParameters()[2]; + } + + public abstract class FakeModelMetadata : ModelMetadata + { + public FakeModelMetadata() + : base(ModelMetadataIdentity.ForType(typeof(string))) + { + } + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultBindingMetadataProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultBindingMetadataProviderTest.cs index 9e553ea05d..327f8aa66f 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultBindingMetadataProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultBindingMetadataProviderTest.cs @@ -1,6 +1,8 @@ // 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.Reflection; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Binders; using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; @@ -22,7 +24,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForType(typeof(string)), - new ModelAttributes(attributes)); + new ModelAttributes(attributes, null, null)); var provider = new DefaultBindingMetadataProvider(); @@ -46,7 +48,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForType(typeof(string)), - new ModelAttributes(attributes)); + new ModelAttributes(attributes, null, null)); var provider = new DefaultBindingMetadataProvider(); @@ -69,7 +71,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForType(typeof(string)), - new ModelAttributes(attributes)); + new ModelAttributes(attributes, null, null)); var provider = new DefaultBindingMetadataProvider(); @@ -93,7 +95,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForType(typeof(string)), - new ModelAttributes(attributes)); + new ModelAttributes(attributes, null, null)); var provider = new DefaultBindingMetadataProvider(); @@ -116,7 +118,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForType(typeof(string)), - new ModelAttributes(attributes)); + new ModelAttributes(attributes, null, null)); var provider = new DefaultBindingMetadataProvider(); @@ -140,7 +142,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForType(typeof(string)), - new ModelAttributes(attributes)); + new ModelAttributes(attributes, null, null)); var provider = new DefaultBindingMetadataProvider(); @@ -162,7 +164,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)), - new ModelAttributes(propertyAttributes, typeAttributes: new object[0])); + new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -185,7 +187,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)), - new ModelAttributes(propertyAttributes, typeAttributes: new object[0])); + new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -208,7 +210,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)), - new ModelAttributes(propertyAttributes, typeAttributes: new object[0])); + new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -231,7 +233,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)), - new ModelAttributes(propertyAttributes, typeAttributes: new object[0])); + new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -254,7 +256,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)), - new ModelAttributes(propertyAttributes, typeAttributes: new object[0])); + new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -266,6 +268,147 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.True(context.BindingMetadata.IsBindingRequired); } + [Fact] + public void CreateBindingDetails_FindsBindingBehaviorNever_OnParameter() + { + // Arrange + var parameterAttributes = new object[] + { + new BindingBehaviorAttribute(BindingBehavior.Never), + }; + + var context = new BindingMetadataProviderContext( + ModelMetadataIdentity.ForParameter(ParameterInfos.SampleParameterInfo), + new ModelAttributes(null, null, parameterAttributes)); + + var provider = new DefaultBindingMetadataProvider(); + + // Act + provider.CreateBindingMetadata(context); + + // Assert + Assert.False(context.BindingMetadata.IsBindingAllowed); + Assert.False(context.BindingMetadata.IsBindingRequired); + } + + [Fact] + public void CreateBindingDetails_FindsBindNever_OnParameter() + { + // Arrange + var parameterAttributes = new object[] + { + new BindNeverAttribute(), + }; + + var context = new BindingMetadataProviderContext( + ModelMetadataIdentity.ForParameter(ParameterInfos.SampleParameterInfo), + new ModelAttributes(null, null, parameterAttributes)); + + var provider = new DefaultBindingMetadataProvider(); + + // Act + provider.CreateBindingMetadata(context); + + // Assert + Assert.False(context.BindingMetadata.IsBindingAllowed); + Assert.False(context.BindingMetadata.IsBindingRequired); + } + + [Fact] + public void CreateBindingDetails_FindsBindingBehaviorOptional_OnParameter() + { + // Arrange + var parameterAttributes = new object[] + { + new BindingBehaviorAttribute(BindingBehavior.Optional), + }; + + var context = new BindingMetadataProviderContext( + ModelMetadataIdentity.ForParameter(ParameterInfos.SampleParameterInfo), + new ModelAttributes(null, null, parameterAttributes)); + + var provider = new DefaultBindingMetadataProvider(); + + // Act + provider.CreateBindingMetadata(context); + + // Assert + Assert.True(context.BindingMetadata.IsBindingAllowed); + Assert.False(context.BindingMetadata.IsBindingRequired); + } + + [Fact] + public void CreateBindingDetails_FindsBindingBehaviorRequired_OnParameter() + { + // Arrange + var parameterAttributes = new object[] + { + new BindingBehaviorAttribute(BindingBehavior.Required), + }; + + var context = new BindingMetadataProviderContext( + ModelMetadataIdentity.ForParameter(ParameterInfos.SampleParameterInfo), + new ModelAttributes(null, null, parameterAttributes)); + + var provider = new DefaultBindingMetadataProvider(); + + // Act + provider.CreateBindingMetadata(context); + + // Assert + Assert.True(context.BindingMetadata.IsBindingAllowed); + Assert.True(context.BindingMetadata.IsBindingRequired); + } + + [Fact] + public void CreateBindingDetails_FindsBindRequired_OnParameter() + { + // Arrange + var parameterAttributes = new object[] + { + new BindRequiredAttribute(), + }; + + var context = new BindingMetadataProviderContext( + ModelMetadataIdentity.ForParameter(ParameterInfos.SampleParameterInfo), + new ModelAttributes(null, null, parameterAttributes)); + + var provider = new DefaultBindingMetadataProvider(); + + // Act + provider.CreateBindingMetadata(context); + + // Assert + Assert.True(context.BindingMetadata.IsBindingAllowed); + Assert.True(context.BindingMetadata.IsBindingRequired); + } + + [Fact] + public void CreateBindingDetails_FindsCustomAttributes_OnParameter() + { + // Arrange + var parameterAttributes = new object[] + { + new CustomAttribute { Identifier = "Instance1" }, + new CustomAttribute { Identifier = "Instance2" } + }; + + var context = new BindingMetadataProviderContext( + ModelMetadataIdentity.ForParameter(ParameterInfos.SampleParameterInfo), + new ModelAttributes(null, null, parameterAttributes)); + + var provider = new DefaultBindingMetadataProvider(); + + // Act + provider.CreateBindingMetadata(context); + + // Assert + Assert.Collection(context.Attributes, + a => Assert.Equal("Instance1", ((CustomAttribute)a).Identifier), + a => Assert.Equal("Instance2", ((CustomAttribute)a).Identifier)); + Assert.Equal(2, context.ParameterAttributes.Count); + } + // These attributes have conflicting behavior - the 'required' behavior should be used because // of ordering. [Fact] @@ -280,7 +423,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)), - new ModelAttributes(propertyAttributes, typeAttributes: new object[0])); + new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -298,7 +441,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Arrange var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindRequiredOnClass)), - new ModelAttributes(propertyAttributes: new object[0], typeAttributes: new object[0])); + new ModelAttributes(new object[0], new object[0], null)); var provider = new DefaultBindingMetadataProvider(); @@ -316,7 +459,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Arrange var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindNeverOnClass)), - new ModelAttributes(propertyAttributes: new object[0], typeAttributes: new object[0])); + new ModelAttributes(new object[0], new object[0], null)); var provider = new DefaultBindingMetadataProvider(); @@ -334,7 +477,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Arrange var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(InheritedBindNeverOnClass)), - new ModelAttributes(propertyAttributes: new object[0], typeAttributes: new object[0])); + new ModelAttributes(new object[0], new object[0], null)); var provider = new DefaultBindingMetadataProvider(); @@ -357,7 +500,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindNeverOnClass)), - new ModelAttributes(propertyAttributes, typeAttributes: new object[0])); + new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -380,7 +523,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindNeverOnClass)), - new ModelAttributes(propertyAttributes, typeAttributes: new object[0])); + new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -403,7 +546,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(InheritedBindNeverOnClass)), - new ModelAttributes(propertyAttributes, typeAttributes: new object[0])); + new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -426,7 +569,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindRequiredOnClass)), - new ModelAttributes(propertyAttributes, typeAttributes: new object[0])); + new ModelAttributes(new object[0], propertyAttributes, null)); var provider = new DefaultBindingMetadataProvider(); @@ -445,7 +588,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal // Arrange var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindRequiredOverridesInheritedBindNever)), - new ModelAttributes(propertyAttributes: new object[0], typeAttributes: new object[0])); + new ModelAttributes(new object[0], new object[0], null)); var provider = new DefaultBindingMetadataProvider(); @@ -470,7 +613,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForType(typeof(string)), - new ModelAttributes(attributes)); + new ModelAttributes(attributes, null, null)); // These values shouldn't be changed since this is a Type-Metadata context.BindingMetadata.IsBindingAllowed = initialValue; @@ -501,7 +644,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal var context = new BindingMetadataProviderContext( ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)), - new ModelAttributes(propertyAttributes: new object[0], typeAttributes: typeAttributes)); + new ModelAttributes(typeAttributes, new object[0], null)); // These values shouldn't be changed since this is a Type-Metadata context.BindingMetadata.IsBindingAllowed = initialValue; @@ -545,5 +688,22 @@ namespace Microsoft.AspNetCore.Mvc.Internal BindingSource = bindingSource; } } + + private class ParameterInfos + { + public void Method(object param1) + { + } + + public static ParameterInfo SampleParameterInfo + = typeof(ParameterInfos) + .GetMethod(nameof(ParameterInfos.Method)) + .GetParameters()[0]; + } + + private class CustomAttribute : Attribute + { + public string Identifier { get; set; } + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MiddlewareFilterTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MiddlewareFilterTest.cs index 9270ec6b64..2767f6c054 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MiddlewareFilterTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/MiddlewareFilterTest.cs @@ -448,7 +448,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal : base( new EmptyModelMetadataProvider(), TestModelBinderFactory.CreateDefault(), - Mock.Of()) + Mock.Of()) { _actionParameters = actionParameters; } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/BindingSourceMetadataProviderTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/BindingSourceMetadataProviderTests.cs index 0d255fa660..5cf12bb5f0 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/BindingSourceMetadataProviderTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/BindingSourceMetadataProviderTests.cs @@ -15,7 +15,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var key = ModelMetadataIdentity.ForType(typeof(Test)); - var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0])); + var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0], null)); // Act provider.CreateBindingMetadata(context); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataProviderTest.cs index 7c0215b4f6..45f2f4ca4f 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataProviderTest.cs @@ -197,6 +197,61 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata } } + [Fact] + public void GetMetadataForParameter_SuppliesEmptyAttributes_WhenParameterHasNoAttributes() + { + // Arrange + var provider = CreateProvider(); + var parameters = typeof(ModelType) + .GetMethod(nameof(ModelType.Method1)) + .GetParameters(); + + // Act + var metadata = provider.GetMetadataForParameter(parameters[0]); + + // Assert + var defaultMetadata = Assert.IsType(metadata); + Assert.Empty(defaultMetadata.Attributes.Attributes); + } + + [Fact] + public void GetMetadataForParameter_SuppliesAttributes_WhenParamHasAttributes() + { + // Arrange + var provider = CreateProvider(); + var parameters = typeof(ModelType) + .GetMethod(nameof(ModelType.Method1)) + .GetParameters(); + + // Act + var metadata = provider.GetMetadataForParameter(parameters[1]); + + // Assert + var defaultMetadata = Assert.IsType(metadata); + Assert.Equal(2, defaultMetadata.Attributes.Attributes.Count); + var attribute1 = Assert.IsType(defaultMetadata.Attributes.Attributes[0]); + Assert.Equal("ParamAttrib1", attribute1.Value); + var attribute2 = Assert.IsType(defaultMetadata.Attributes.Attributes[1]); + Assert.Equal("ParamAttrib2", attribute2.Value); + } + + [Fact] + public void GetMetadataForParameter_Cached() + { + // Arrange + var provider = CreateProvider(); + var parameter = typeof(ModelType) + .GetMethod(nameof(ModelType.Method1)) + .GetParameters()[1]; + + // Act + var metadata1 = provider.GetMetadataForParameter(parameter); + var metadata2 = provider.GetMetadataForParameter(parameter); + + // Assert + Assert.Same(metadata1, metadata2); + } + private static DefaultModelMetadataProvider CreateProvider() { return new DefaultModelMetadataProvider( @@ -211,6 +266,12 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata public PropertyType Property1 { get; } = new PropertyType(); public PropertyType Property2 { get; set; } + + public void Method1( + object paramWithNoAttributes, + [Model("ParamAttrib1"), Model("ParamAttrib2")] object paramWithTwoAttributes) + { + } } [Model("OnPropertyType")] @@ -218,6 +279,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata { } + [AttributeUsage(AttributeTargets.All, AllowMultiple = true)] private class ModelAttribute : Attribute { public ModelAttribute(string value) 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 88892e3d26..5369d979e3 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataTest.cs @@ -25,7 +25,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata Enumerable.Empty()); var key = ModelMetadataIdentity.ForType(typeof(string)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); // Act var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); @@ -85,7 +85,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata Enumerable.Empty()); var key = ModelMetadataIdentity.ForType(typeof(Exception)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); // Act var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); @@ -104,7 +104,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); var key = ModelMetadataIdentity.ForProperty(typeof(string), "Message", typeof(Exception)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], new object[0], null)); // Act var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); @@ -127,7 +127,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); var key = ModelMetadataIdentity.ForType(modelType); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); @@ -154,7 +154,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); var key = ModelMetadataIdentity.ForType(modelType); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); @@ -188,7 +188,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); var key = ModelMetadataIdentity.ForType(modelType); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); cache.BindingMetadata = new BindingMetadata() { IsBindingAllowed = false, // Will be ignored. @@ -213,7 +213,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); var key = ModelMetadataIdentity.ForType(modelType); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); cache.BindingMetadata = new BindingMetadata() { IsBindingRequired = true, // Will be ignored. @@ -239,7 +239,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); var key = ModelMetadataIdentity.ForType(modelType); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); @@ -260,7 +260,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); var key = ModelMetadataIdentity.ForType(modelType); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); @@ -285,13 +285,13 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata detailsProvider, new DefaultMetadataDetails( ModelMetadataIdentity.ForProperty(typeof(int), "Prop1", typeof(string)), - attributes: new ModelAttributes(new object[0], new object[0]))), + attributes: new ModelAttributes(new object[0], new object[0], null))), new DefaultModelMetadata( provider.Object, detailsProvider, new DefaultMetadataDetails( ModelMetadataIdentity.ForProperty(typeof(int), "Prop2", typeof(string)), - attributes: new ModelAttributes(new object[0], new object[0]))), + attributes: new ModelAttributes(new object[0], new object[0], null))), }; provider @@ -299,7 +299,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata .Returns(expectedProperties); var key = ModelMetadataIdentity.ForType(typeof(string)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); var metadata = new DefaultModelMetadata(provider.Object, detailsProvider, cache); @@ -361,7 +361,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata detailsProvider, new DefaultMetadataDetails( ModelMetadataIdentity.ForProperty(typeof(int), originalName, typeof(string)), - attributes: new ModelAttributes(new object[0], new object[0])))); + attributes: new ModelAttributes(new object[0], new object[0], null)))); } provider @@ -369,7 +369,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata .Returns(expectedProperties); var key = ModelMetadataIdentity.ForType(typeof(string)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); var metadata = new DefaultModelMetadata(provider.Object, detailsProvider, cache); @@ -461,7 +461,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata { var propertyCache = new DefaultMetadataDetails( ModelMetadataIdentity.ForProperty(typeof(int), kvp.Key, typeof(string)), - attributes: new ModelAttributes(new object[0], new object[0])); + attributes: new ModelAttributes(new object[0], new object[0], null)); propertyCache.DisplayMetadata = new DisplayMetadata(); propertyCache.DisplayMetadata.Order = kvp.Value; @@ -477,7 +477,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata .Returns(expectedProperties); var key = ModelMetadataIdentity.ForType(typeof(string)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); var metadata = new DefaultModelMetadata(provider.Object, detailsProvider, cache); @@ -497,7 +497,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); var key = ModelMetadataIdentity.ForType(typeof(string)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); @@ -518,7 +518,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); var key = ModelMetadataIdentity.ForType(typeof(string)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); @@ -539,7 +539,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new DefaultModelMetadataProvider(detailsProvider); var key = ModelMetadataIdentity.ForType(typeof(int[])); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); cache.BindingMetadata = new BindingMetadata() { IsReadOnly = true, // Will be ignored. @@ -562,7 +562,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new DefaultModelMetadataProvider(detailsProvider); var key = ModelMetadataIdentity.ForType(typeof(TypeWithProperties)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); @@ -581,7 +581,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new DefaultModelMetadataProvider(detailsProvider); var key = ModelMetadataIdentity.ForType(typeof(TypeWithProperties)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); @@ -600,7 +600,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new DefaultModelMetadataProvider(detailsProvider); var key = ModelMetadataIdentity.ForType(typeof(TypeWithProperties)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); @@ -624,7 +624,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new DefaultModelMetadataProvider(detailsProvider); var key = ModelMetadataIdentity.ForType(modelType); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); @@ -651,7 +651,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new DefaultModelMetadataProvider(detailsProvider); var key = ModelMetadataIdentity.ForType(typeof(TypeWithProperties)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); @@ -683,7 +683,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new DefaultModelMetadataProvider(detailsProvider); var key = ModelMetadataIdentity.ForType(typeof(int)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); cache.ValidationMetadata = new ValidationMetadata { PropertyValidationFilter = value, @@ -706,7 +706,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new DefaultModelMetadataProvider(detailsProvider); var key = ModelMetadataIdentity.ForType(typeof(int)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); cache.ValidationMetadata = new ValidationMetadata() { ValidateChildren = true, @@ -729,7 +729,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var provider = new DefaultModelMetadataProvider(detailsProvider); var key = ModelMetadataIdentity.ForType(typeof(XmlDocument)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); cache.ValidationMetadata = new ValidationMetadata() { ValidateChildren = false, @@ -750,7 +750,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata // Arrange var detailsProvider = new Mock(); var key = ModelMetadataIdentity.ForType(typeof(string)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); var metadataProvider = new Mock(); metadataProvider .Setup(mp => mp.GetMetadataForType(typeof(string))) @@ -770,7 +770,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata // Arrange var detailsProvider = new Mock(); var key = ModelMetadataIdentity.ForType(typeof(string)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0], null, null)); var metadataProvider = new Mock(); metadataProvider .Setup(mp => mp.GetMetadataForProperties(typeof(Exception))) diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultValidationMetadataProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultValidationMetadataProviderTest.cs index 16fe4f3b37..d7299a1a3b 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultValidationMetadataProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultValidationMetadataProviderTest.cs @@ -19,7 +19,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var attributes = new Attribute[] { new ValidateNeverAttribute() }; var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); - var context = new ValidationMetadataProviderContext(key, new ModelAttributes(attributes, new object[0])); + var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); // Act provider.CreateValidationMetadata(context); @@ -39,7 +39,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var attributes = new Attribute[] { new ValidateNeverAttribute() }; var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); - var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], attributes)); + var context = new ValidationMetadataProviderContext(key, new ModelAttributes(attributes, new object[0], null)); // Act provider.CreateValidationMetadata(context); @@ -56,7 +56,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var attributes = new Attribute[] { new ValidateNeverAttribute() }; var key = ModelMetadataIdentity.ForType(typeof(ValidateNeverClass)); - var context = new ValidationMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new ValidationMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateValidationMetadata(context); @@ -75,7 +75,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata typeof(string), nameof(ValidateNeverClass.ClassName), typeof(ValidateNeverClass)); - var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0])); + var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0], null)); // Act provider.CreateValidationMetadata(context); @@ -97,7 +97,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata typeof(string), nameof(ValidateNeverSubclass.SubclassName), typeof(ValidateNeverSubclass)); - var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0])); + var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0], null)); // Act provider.CreateValidationMetadata(context); @@ -118,7 +118,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var attribute = new TestClientModelValidationAttribute(); var attributes = new Attribute[] { attribute }; var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); - var context = new ValidationMetadataProviderContext(key, new ModelAttributes(attributes, new object[0])); + var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); // Act provider.CreateValidationMetadata(context); @@ -137,7 +137,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var attribute = new TestModelValidationAttribute(); var attributes = new Attribute[] { attribute }; var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); - var context = new ValidationMetadataProviderContext(key, new ModelAttributes(attributes, new object[0])); + var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); // Act provider.CreateValidationMetadata(context); @@ -156,7 +156,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata var attribute = new TestValidationAttribute(); var attributes = new Attribute[] { attribute }; var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); - var context = new ValidationMetadataProviderContext(key, new ModelAttributes(attributes, new object[0])); + var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); context.ValidationMetadata.ValidatorMetadata.Add(attribute); // Act diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/ExcludeBindingMetadataProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/ExcludeBindingMetadataProviderTest.cs index a380c17399..8d2e6d10f3 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/ExcludeBindingMetadataProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/ExcludeBindingMetadataProviderTest.cs @@ -20,7 +20,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata nameof(Person.Age), typeof(Person)); - var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0])); + var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0], null)); context.BindingMetadata.IsBindingAllowed = initialValue; @@ -44,7 +44,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata nameof(Person.Age), typeof(Person)); - var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0])); + var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0], null)); context.BindingMetadata.IsBindingAllowed = initialValue; diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/ModelAttributesTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/ModelAttributesTest.cs index 786ff0fec6..69c146726e 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/ModelAttributesTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/ModelAttributesTest.cs @@ -178,6 +178,40 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Assert.IsType(attribute); } + [Fact] + public void GetAttributesForParameter_NoAttributes() + { + // Arrange & Act + var attributes = ModelAttributes.GetAttributesForParameter( + typeof(MethodWithParamAttributesType) + .GetMethod(nameof(MethodWithParamAttributesType.Method)) + .GetParameters()[0]); + + // Assert + Assert.Empty(attributes.Attributes); + Assert.Empty(attributes.ParameterAttributes); + Assert.Null(attributes.TypeAttributes); + Assert.Null(attributes.PropertyAttributes); + } + + [Fact] + public void GetAttributesForParameter_SomeAttributes() + { + // Arrange & Act + var attributes = ModelAttributes.GetAttributesForParameter( + typeof(MethodWithParamAttributesType) + .GetMethod(nameof(MethodWithParamAttributesType.Method)) + .GetParameters()[1]); + + // Assert + Assert.IsType(attributes.Attributes[0]); + Assert.IsType(attributes.Attributes[1]); + Assert.IsType(attributes.ParameterAttributes[0]); + Assert.IsType(attributes.ParameterAttributes[1]); + Assert.Null(attributes.TypeAttributes); + Assert.Null(attributes.PropertyAttributes); + } + [ClassValidator] private class BaseModel { @@ -266,5 +300,18 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding private class MetadataPropertyType { } + + [IrrelevantAttribute] // We verify this is ignored + private class MethodWithParamAttributesType + { + [IrrelevantAttribute] // We verify this is ignored + public void Method(object noAttribs, [Required, Range(1, 100)] int validationAttribs) + { + } + } + + private class IrrelevantAttribute : Attribute + { + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ParameterBinderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ParameterBinderTest.cs index 39a48657ec..902187908b 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ParameterBinderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ParameterBinderTest.cs @@ -2,9 +2,14 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.ComponentModel.DataAnnotations; +using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Abstractions; +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 Moq; using Xunit; @@ -89,7 +94,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding var parameterBinder = new ParameterBinder( metadataProvider, factory.Object, - CreateMockValidator()); + CreateMockValidatorProvider()); var controllerContext = new ControllerContext(); @@ -139,7 +144,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding var argumentBinder = new ParameterBinder( metadataProvider, factory.Object, - CreateMockValidator()); + CreateMockValidatorProvider()); var valueProvider = new SimpleValueProvider { @@ -154,15 +159,237 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Assert.True(binderExecuted); } - private static IObjectModelValidator CreateMockValidator() + [Fact] + public async Task BindModelAsync_EnforcesTopLevelBindRequired() { - var mockValidator = new Mock(); + // Arrange + var actionContext = new ControllerContext(); + + var mockModelMetadata = CreateMockModelMetadata(); + mockModelMetadata.Setup(o => o.IsBindingRequired).Returns(true); + mockModelMetadata.Setup(o => o.DisplayName).Returns("Ignored Display Name"); // Bind attribute errors are phrased in terms of the model name, not display name + + var parameterBinder = CreateParameterBinder(mockModelMetadata.Object, validator: null); + var modelBindingResult = ModelBindingResult.Failed(); + + // Act + var result = await parameterBinder.BindModelAsync( + actionContext, + CreateMockModelBinder(modelBindingResult), + CreateMockValueProvider(), + new ParameterDescriptor { Name = "myParam", ParameterType = typeof(Person) }, + mockModelMetadata.Object, + "ignoredvalue"); + + // Assert + Assert.False(actionContext.ModelState.IsValid); + Assert.Equal("myParam", actionContext.ModelState.Single().Key); + Assert.Equal( + new DefaultModelBindingMessageProvider().MissingBindRequiredValueAccessor("myParam"), + actionContext.ModelState.Single().Value.Errors.Single().ErrorMessage); + } + + [Fact] + public async Task BindModelAsync_EnforcesTopLevelRequired() + { + // Arrange + var actionContext = new ControllerContext(); + var mockModelMetadata = CreateMockModelMetadata(); + mockModelMetadata.Setup(o => o.IsRequired).Returns(true); + mockModelMetadata.Setup(o => o.DisplayName).Returns("My Display Name"); + mockModelMetadata.Setup(o => o.ValidatorMetadata).Returns(new[] + { + new RequiredAttribute() + }); + + var validator = new DataAnnotationsModelValidator( + new ValidationAttributeAdapterProvider(), + new RequiredAttribute(), + stringLocalizer: null); + + var parameterBinder = CreateParameterBinder( + mockModelMetadata.Object, + validator); + var modelBindingResult = ModelBindingResult.Success(null); + + // Act + var result = await parameterBinder.BindModelAsync( + actionContext, + CreateMockModelBinder(modelBindingResult), + CreateMockValueProvider(), + new ParameterDescriptor { Name = "myParam", ParameterType = typeof(Person) }, + mockModelMetadata.Object, + "ignoredvalue"); + + // Assert + Assert.False(actionContext.ModelState.IsValid); + Assert.Equal("myParam", actionContext.ModelState.Single().Key); + Assert.Equal( + new RequiredAttribute().FormatErrorMessage("My Display Name"), + actionContext.ModelState.Single().Value.Errors.Single().ErrorMessage); + } + + [Fact] + public async Task BindModelAsync_EnforcesTopLevelDataAnnotationsAttribute() + { + // Arrange + var actionContext = new ControllerContext(); + var mockModelMetadata = CreateMockModelMetadata(); + var validationAttribute = new RangeAttribute(1, 100); + mockModelMetadata.Setup(o => o.DisplayName).Returns("My Display Name"); + mockModelMetadata.Setup(o => o.ValidatorMetadata).Returns(new[] { + validationAttribute + }); + + var validator = new DataAnnotationsModelValidator( + new ValidationAttributeAdapterProvider(), + validationAttribute, + stringLocalizer: null); + + var parameterBinder = CreateParameterBinder( + mockModelMetadata.Object, + validator); + var modelBindingResult = ModelBindingResult.Success(123); + + // Act + var result = await parameterBinder.BindModelAsync( + actionContext, + CreateMockModelBinder(modelBindingResult), + CreateMockValueProvider(), + new ParameterDescriptor { Name = "myParam", ParameterType = typeof(Person) }, + mockModelMetadata.Object, + 50); // This value is ignored, because test explicitly set the ModelBindingResult + + // Assert + Assert.False(actionContext.ModelState.IsValid); + Assert.Equal("myParam", actionContext.ModelState.Single().Key); + Assert.Equal( + validationAttribute.FormatErrorMessage("My Display Name"), + actionContext.ModelState.Single().Value.Errors.Single().ErrorMessage); + } + + [Fact] + public async Task BindModelAsync_SupportsIObjectModelValidatorForBackCompat() + { + // Arrange + var actionContext = new ControllerContext(); + + var mockValidator = new Mock(MockBehavior.Strict); mockValidator .Setup(o => o.Validate( It.IsAny(), It.IsAny(), It.IsAny(), - It.IsAny())); + It.IsAny())) + .Callback((ActionContext context, ValidationStateDictionary validationState, string prefix, object model) => + { + context.ModelState.AddModelError(prefix, "Test validation message"); + }); + + var modelMetadata = CreateMockModelMetadata().Object; + var parameterBinder = CreateBackCompatParameterBinder( + modelMetadata, + mockValidator.Object); + var modelBindingResult = ModelBindingResult.Success(123); + + // Act + var result = await parameterBinder.BindModelAsync( + actionContext, + CreateMockModelBinder(modelBindingResult), + CreateMockValueProvider(), + new ParameterDescriptor { Name = "myParam", ParameterType = typeof(Person) }, + modelMetadata, + "ignored"); + + // Assert + Assert.False(actionContext.ModelState.IsValid); + Assert.Equal("myParam", actionContext.ModelState.Single().Key); + Assert.Equal( + "Test validation message", + actionContext.ModelState.Single().Value.Errors.Single().ErrorMessage); + } + + private static Mock CreateMockModelMetadata() + { + var mockModelMetadata = new Mock(); + mockModelMetadata + .Setup(o => o.ModelBindingMessageProvider) + .Returns(new DefaultModelBindingMessageProvider()); + return mockModelMetadata; + } + + private static IModelBinder CreateMockModelBinder(ModelBindingResult modelBinderResult) + { + var mockBinder = new Mock(MockBehavior.Strict); + mockBinder + .Setup(o => o.BindModelAsync(It.IsAny())) + .Returns(context => + { + context.Result = modelBinderResult; + return Task.CompletedTask; + }); + return mockBinder.Object; + } + + private static ParameterBinder CreateParameterBinder( + ModelMetadata modelMetadata, + IModelValidator validator) + { + var mockModelMetadataProvider = new Mock(MockBehavior.Strict); + mockModelMetadataProvider + .Setup(o => o.GetMetadataForType(typeof(Person))) + .Returns(modelMetadata); + + var mockModelBinderFactory = new Mock(MockBehavior.Strict); + return new ParameterBinder( + mockModelMetadataProvider.Object, + mockModelBinderFactory.Object, + CreateMockValidatorProvider(validator)); + } + + private static ParameterBinder CreateBackCompatParameterBinder( + ModelMetadata modelMetadata, + IObjectModelValidator validator) + { + var mockModelMetadataProvider = new Mock(MockBehavior.Strict); + mockModelMetadataProvider + .Setup(o => o.GetMetadataForType(typeof(Person))) + .Returns(modelMetadata); + + var mockModelBinderFactory = new Mock(MockBehavior.Strict); +#pragma warning disable CS0618 // Type or member is obsolete + return new ParameterBinder( + mockModelMetadataProvider.Object, + mockModelBinderFactory.Object, + validator); +#pragma warning restore CS0618 // Type or member is obsolete + } + + private static IValueProvider CreateMockValueProvider() + { + var mockValueProvider = new Mock(MockBehavior.Strict); + mockValueProvider + .Setup(o => o.ContainsPrefix(It.IsAny())) + .Returns(true); + return mockValueProvider.Object; + } + + private static IModelValidatorProvider CreateMockValidatorProvider(IModelValidator validator = null) + { + var mockValidator = new Mock(); + mockValidator + .Setup(o => o.CreateValidators( + It.IsAny())) + .Callback(context => + { + if (validator != null) + { + foreach (var result in context.Results) + { + result.Validator = validator; + } + } + }); return mockValidator.Object; } @@ -180,5 +407,13 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding return Equals(obj as Person); } } + + public abstract class FakeModelMetadata : ModelMetadata + { + public FakeModelMetadata() + : base(ModelMetadataIdentity.ForType(typeof(string))) + { + } + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/DataAnnotationsMetadataProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/DataAnnotationsMetadataProviderTest.cs index 4b551655f9..a00e3e4482 100644 --- a/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/DataAnnotationsMetadataProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/DataAnnotationsMetadataProviderTest.cs @@ -69,7 +69,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal stringLocalizerFactory: null); var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(new object[] { attribute })); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(new object[] { attribute }, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -92,7 +92,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new[] { dataType, }; var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -117,7 +117,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { dataType, displayFormat, }; var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -138,7 +138,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { editable }; var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new BindingMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new BindingMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateBindingMetadata(context); @@ -159,7 +159,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { editable }; var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new BindingMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new BindingMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateBindingMetadata(context); @@ -186,7 +186,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { display, displayName }; var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -213,7 +213,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { display, displayName }; var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -240,7 +240,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { display, displayName }; var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -277,7 +277,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { displayName }; var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -314,7 +314,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { display }; var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -346,7 +346,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { display }; var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -383,7 +383,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { display }; var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -420,7 +420,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { display }; var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -451,7 +451,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { display }; var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -488,7 +488,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { display }; var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -519,7 +519,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { display }; var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -567,7 +567,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { display }; var key = ModelMetadataIdentity.ForType(typeof(DataAnnotationsMetadataProviderTest)); - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -614,7 +614,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var key = ModelMetadataIdentity.ForType(type); var attributes = new object[0]; - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -650,7 +650,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var key = ModelMetadataIdentity.ForType(type); var attributes = new object[0]; - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -784,7 +784,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var key = ModelMetadataIdentity.ForType(type); var attributes = new object[0]; - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -812,7 +812,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new object[0]; var key = ModelMetadataIdentity.ForType(type); - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); var stringLocalizer = new Mock(MockBehavior.Strict); stringLocalizer @@ -964,7 +964,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var key = ModelMetadataIdentity.ForType(type); var attributes = new object[0]; - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -994,7 +994,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var key = ModelMetadataIdentity.ForType(typeof(EnumWithDisplayOrder)); var attributes = new object[0]; - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); // Act provider.CreateDisplayMetadata(context); @@ -1098,7 +1098,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { required }; var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); - var context = new ValidationMetadataProviderContext(key, new ModelAttributes(attributes, new object[0])); + var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); // Act provider.CreateValidationMetadata(context); @@ -1120,7 +1120,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { }; var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); - var context = new ValidationMetadataProviderContext(key, new ModelAttributes(attributes, new object[0])); + var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); context.ValidationMetadata.IsRequired = initialValue; // Act @@ -1143,7 +1143,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { new RequiredAttribute() }; var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); - var context = new BindingMetadataProviderContext(key, new ModelAttributes(attributes, new object[0])); + var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); context.BindingMetadata.IsBindingRequired = initialValue; // Act @@ -1166,7 +1166,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attributes = new Attribute[] { }; var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); - var context = new BindingMetadataProviderContext(key, new ModelAttributes(attributes, new object[0])); + var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); context.BindingMetadata.IsReadOnly = initialValue; // Act @@ -1187,7 +1187,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attribute = new TestValidationAttribute(); var attributes = new Attribute[] { attribute }; var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); - var context = new ValidationMetadataProviderContext(key, new ModelAttributes(attributes, new object[0])); + var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); // Act provider.CreateValidationMetadata(context); @@ -1208,7 +1208,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var attribute = new TestValidationAttribute(); var attributes = new Attribute[] { attribute }; var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); - var context = new ValidationMetadataProviderContext(key, new ModelAttributes(attributes, new object[0])); + var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); context.ValidationMetadata.ValidatorMetadata.Add(attribute); // Act @@ -1227,7 +1227,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal var key = ModelMetadataIdentity.ForType(typeof(EnumWithLocalizedDisplayNames)); var attributes = new object[0]; - var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes)); + var context = new DisplayMetadataProviderContext(key, new ModelAttributes(attributes, null, null)); provider.CreateDisplayMetadata(context); return context.DisplayMetadata.EnumGroupedDisplayNamesAndValues; diff --git a/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/DataAnnotationsModelValidatorTest.cs b/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/DataAnnotationsModelValidatorTest.cs index 977d730b2f..8930283bce 100644 --- a/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/DataAnnotationsModelValidatorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/DataAnnotationsModelValidatorTest.cs @@ -196,6 +196,54 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal Assert.Empty(result); } + [Fact] + public void Validate_RequiredButNullAtTopLevel_Invalid() + { + // Arrange + var metadata = _metadataProvider.GetMetadataForProperty(typeof(string), "Length"); + var validator = new DataAnnotationsModelValidator( + new ValidationAttributeAdapterProvider(), + new RequiredAttribute(), + stringLocalizer: null); + var validationContext = new ModelValidationContext( + actionContext: new ActionContext(), + modelMetadata: metadata, + metadataProvider: _metadataProvider, + container: null, + model: null); + + // Act + var result = validator.Validate(validationContext); + + // Assert + var validationResult = result.Single(); + Assert.Empty(validationResult.MemberName); + Assert.Equal(new RequiredAttribute().FormatErrorMessage("Length"), validationResult.Message); + } + + [Fact] + public void Validate_RequiredAndNotNullAtTopLevel_Valid() + { + // Arrange + var metadata = _metadataProvider.GetMetadataForProperty(typeof(string), "Length"); + var validator = new DataAnnotationsModelValidator( + new ValidationAttributeAdapterProvider(), + new RequiredAttribute(), + stringLocalizer: null); + var validationContext = new ModelValidationContext( + actionContext: new ActionContext(), + modelMetadata: metadata, + metadataProvider: _metadataProvider, + container: null, + model: 123); + + // Act + var result = validator.Validate(validationContext); + + // Assert + Assert.Empty(result); + } + public static TheoryData, IEnumerable> Valdate_ReturnsExpectedResults_Data { diff --git a/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/DataMemberRequiredBindingMetadataProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/DataMemberRequiredBindingMetadataProviderTest.cs index 10dfc3cbd9..48dc323474 100644 --- a/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/DataMemberRequiredBindingMetadataProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/DataMemberRequiredBindingMetadataProviderTest.cs @@ -25,7 +25,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal typeof(string), nameof(ClassWithDataMemberIsRequiredTrue.StringProperty), typeof(ClassWithDataMemberIsRequiredTrue)); - var context = new BindingMetadataProviderContext(key, new ModelAttributes(attributes, new object[0])); + var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); // Act provider.CreateBindingMetadata(context); @@ -51,7 +51,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal typeof(string), nameof(ClassWithDataMemberIsRequiredFalse.StringProperty), typeof(ClassWithDataMemberIsRequiredFalse)); - var context = new BindingMetadataProviderContext(key, new ModelAttributes(attributes, new object[0])); + var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); context.BindingMetadata.IsBindingRequired = initialValue; @@ -76,7 +76,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal }; var key = ModelMetadataIdentity.ForType(typeof(ClassWithDataMemberIsRequiredTrue)); - var context = new BindingMetadataProviderContext(key, new ModelAttributes(attributes, new object[0])); + var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); context.BindingMetadata.IsBindingRequired = initialValue; @@ -99,7 +99,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal typeof(string), nameof(ClassWithoutAttributes.StringProperty), typeof(ClassWithoutAttributes)); - var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0])); + var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0], null)); context.BindingMetadata.IsBindingRequired = initialValue; @@ -127,7 +127,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal typeof(string), nameof(ClassWithDataMemberIsRequiredTrueWithoutDataContract.StringProperty), typeof(ClassWithDataMemberIsRequiredTrueWithoutDataContract)); - var context = new BindingMetadataProviderContext(key, new ModelAttributes(attributes, new object[0])); + var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], attributes, null)); context.BindingMetadata.IsBindingRequired = initialValue; diff --git a/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/ModelMetadataProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/ModelMetadataProviderTest.cs index b42894bdbf..925cc144a4 100644 --- a/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/ModelMetadataProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.DataAnnotations.Test/Internal/ModelMetadataProviderTest.cs @@ -1066,7 +1066,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal { return new DefaultMetadataDetails( key, - new ModelAttributes(_attributes.Concat(entry.ModelAttributes.TypeAttributes).ToArray())); + new ModelAttributes(_attributes.Concat(entry.ModelAttributes.TypeAttributes).ToArray(), null, null)); } return entry; @@ -1079,7 +1079,7 @@ namespace Microsoft.AspNetCore.Mvc.DataAnnotations.Internal { return new DefaultMetadataDetails( e.Key, - new ModelAttributes(_attributes.Concat(e.ModelAttributes.PropertyAttributes), e.ModelAttributes.TypeAttributes)); + new ModelAttributes(e.ModelAttributes.TypeAttributes, _attributes.Concat(e.ModelAttributes.PropertyAttributes), null)); }) .ToArray(); } diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputValidationTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputValidationTests.cs new file mode 100644 index 0000000000..4a8583d345 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/InputValidationTests.cs @@ -0,0 +1,120 @@ +// 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.Linq; +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.FunctionalTests +{ + public class InputValidationTests : IClassFixture> + { + public InputValidationTests(MvcTestFixture fixture) + { + Client = fixture.Client; + } + + public HttpClient Client { get; } + + [Fact] + public async Task ValidRequest_IsAccepted() + { + // Arrange + var content = new FormUrlEncodedContent(new Dictionary + { + { "RequiredProp", "1" }, + { "BindRequiredProp", "2" }, + { "RequiredAndBindRequiredProp", "3" }, + { "requiredParam", "4" }, + { "bindRequiredParam", "5" }, + { "requiredAndBindRequiredParam", "6" }, + { "UnboundRequiredProp", "100" }, // Value should not be used + { "UnboundBindRequiredProp", "101" }, // Value should not be used + { "BindNeverRequiredProp", "ignoredValue" }, // Value should not be used + }); + + // Act + var response = await Client.PostAsync("http://localhost/TopLevelValidation", content); + var responseText = await response.Content.ReadAsStringAsync(); + + // Assert + Assert.Contains("[OptionalProp:0]", responseText); + Assert.Contains("[RequiredProp:1]", responseText); + Assert.Contains("[BindRequiredProp:2]", responseText); + Assert.Contains("[RequiredAndBindRequiredProp:3]", responseText); + Assert.Contains("[OptionalStringLengthProp:]", responseText); + Assert.Contains("[OptionalRangeDisplayNameProp:0]", responseText); + Assert.Contains("[UnboundRequiredProp:0]", responseText); + Assert.Contains("[UnboundBindRequiredProp:0]", responseText); + Assert.Contains("[BindNeverRequiredProp:]", responseText); + Assert.Contains("[optionalParam:0]", responseText); + Assert.Contains("[requiredParam:4]", responseText); + Assert.Contains("[bindRequiredParam:5]", responseText); + Assert.Contains("[requiredAndBindRequiredParam:6]", responseText); + Assert.Contains("[optionalStringLengthParam:]", responseText); + Assert.Contains("[optionalRangeDisplayNameParam:0]", responseText); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + [Fact] + public async Task InvalidRequest_IsRejected() + { + // Arrange + var content = new FormUrlEncodedContent(new Dictionary + { + { "OptionalStringLengthProp", "ThisStringIsTooLongForTheProperty" }, + { "OptionalRangeDisplayNameProp", "123" }, + { "optionalStringLengthParam", "ThisStringIsTooLongForTheParameter" }, + { "optionalRangeDisplayNameParam", "456" }, + }); + + // Act + var response = await Client.PostAsync("http://localhost/TopLevelValidation", content); + var responseText = await response.Content.ReadAsStringAsync(); + var errors = JsonConvert.DeserializeObject(responseText) + .Properties() + .ToDictionary( + prop => prop.Name, + prop => ((JArray)prop.Value).Single().Value()); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + Assert.Equal(10, errors.Count); + Assert.Equal( + "The RequiredProp field is required.", + errors["RequiredProp"]); + Assert.Equal( + "A value for the 'BindRequiredProp' property was not provided.", + errors["BindRequiredProp"]); + Assert.Equal( + "A value for the 'RequiredAndBindRequiredProp' property was not provided.", + errors["RequiredAndBindRequiredProp"]); + Assert.Equal( + "The field OptionalStringLengthProp must be a string with a maximum length of 5.", + errors["OptionalStringLengthProp"]); + Assert.Equal( + "The field Some Display Name For Prop must be between 1 and 100.", + errors["OptionalRangeDisplayNameProp"]); + Assert.Equal( + "The requiredParam field is required.", + errors["requiredParam"]); + Assert.Equal( + "A value for the 'bindRequiredParam' property was not provided.", + errors["bindRequiredParam"]); + Assert.Equal( + "A value for the 'requiredAndBindRequiredParam' property was not provided.", + errors["requiredAndBindRequiredParam"]); + Assert.Equal( + "The field optionalStringLengthParam must be a string with a maximum length of 5.", + errors["optionalStringLengthParam"]); + Assert.Equal( + "The field Some Display Name For Param must be between 1 and 100.", + errors["optionalRangeDisplayNameParam"]); + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ActionParametersIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ActionParametersIntegrationTest.cs index 41a09a475a..d8521ee52c 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ActionParametersIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ActionParametersIntegrationTest.cs @@ -3,7 +3,10 @@ using System; using System.Collections.Generic; +using System.ComponentModel; +using System.ComponentModel.DataAnnotations; using System.Linq; +using System.Reflection; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; @@ -489,6 +492,132 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.True(modelState.IsValid); } + [Fact] + public async Task ActionParameter_WithBindNever_DoesNotGetBound() + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameter = new ParameterDescriptor() + { + Name = BindingAndValidationController.BindNeverParamInfo.Name, + ParameterType = typeof(int) + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.QueryString = QueryString.Create(parameter.Name, "123"); + }); + + var modelState = testContext.ModelState; + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var modelMetadata = modelMetadataProvider + .GetMetadataForParameter(BindingAndValidationController.BindNeverParamInfo); + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync( + parameter, + testContext, + modelMetadataProvider, + modelMetadata); + + // Assert + Assert.False(modelBindingResult.IsModelSet); + Assert.True(modelState.IsValid); + } + + [Theory] + [InlineData(123, true)] + [InlineData(null, false)] + public async Task ActionParameter_EnforcesBindRequired(int? input, bool isValid) + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameter = new ParameterDescriptor() + { + Name = BindingAndValidationController.BindRequiredParamInfo.Name, + ParameterType = typeof(int) + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + if (input.HasValue) + { + request.QueryString = QueryString.Create(parameter.Name, input.Value.ToString()); + } + }); + + var modelState = testContext.ModelState; + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var modelMetadata = modelMetadataProvider + .GetMetadataForParameter(BindingAndValidationController.BindRequiredParamInfo); + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync( + parameter, + testContext, + modelMetadataProvider, + modelMetadata); + + // Assert + Assert.Equal(input.HasValue, modelBindingResult.IsModelSet); + Assert.Equal(isValid, modelState.IsValid); + if (isValid) + { + Assert.Equal(input.Value, Assert.IsType(modelBindingResult.Model)); + } + } + + [Theory] + [InlineData("requiredAndStringLengthParam", null, false)] + [InlineData("requiredAndStringLengthParam", "", false)] + [InlineData("requiredAndStringLengthParam", "abc", true)] + [InlineData("requiredAndStringLengthParam", "abcTooLong", false)] + [InlineData("displayNameStringLengthParam", null, true)] + [InlineData("displayNameStringLengthParam", "", true)] + [InlineData("displayNameStringLengthParam", "abc", true)] + [InlineData("displayNameStringLengthParam", "abcTooLong", false, "My Display Name")] + public async Task ActionParameter_EnforcesDataAnnotationsAttributes( + string paramName, string input, bool isValid, string displayName = null) + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameterInfo = BindingAndValidationController.GetParameterInfo(paramName); + var parameter = new ParameterDescriptor() + { + Name = parameterInfo.Name, + ParameterType = parameterInfo.ParameterType + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + if (input != null) + { + request.QueryString = QueryString.Create(parameter.Name, input); + } + }); + + var modelState = testContext.ModelState; + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var modelMetadata = modelMetadataProvider + .GetMetadataForParameter(parameterInfo); + + // Act + var modelBindingResult = await parameterBinder.BindModelAsync( + parameter, + testContext, + modelMetadataProvider, + modelMetadata); + + // Assert + Assert.Equal(input != null, modelBindingResult.IsModelSet); + Assert.Equal(isValid, modelState.IsValid); + if (!isValid) + { + var message = modelState[paramName].Errors.Single().ErrorMessage; + Assert.Contains(displayName ?? parameter.Name, message); + } + } + private struct PointStruct { public PointStruct(double x, double y) @@ -533,6 +662,33 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests public string Name { get; set; } } + private class BindingAndValidationController + { + public void MyAction( + [BindNever] int bindNeverParam, + [BindRequired] int bindRequiredParam, + [Required, StringLength(3)] string requiredAndStringLengthParam, + [Display(Name = "My Display Name"), StringLength(3)] string displayNameStringLengthParam) + { + } + + private static MethodInfo MyActionMethodInfo + => typeof(BindingAndValidationController).GetMethod(nameof(MyAction)); + + public static ParameterInfo BindNeverParamInfo + => MyActionMethodInfo.GetParameters()[0]; + + public static ParameterInfo BindRequiredParamInfo + => MyActionMethodInfo.GetParameters()[1]; + + public static ParameterInfo GetParameterInfo(string parameterName) + { + return MyActionMethodInfo + .GetParameters() + .Single(p => p.Name.Equals(parameterName, StringComparison.Ordinal)); + } + } + private class CustomReadOnlyCollection : ICollection { private ICollection _original; diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BindPropertyIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BindPropertyIntegrationTest.cs index 40df2d96b5..1b42c219f0 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BindPropertyIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/BindPropertyIntegrationTest.cs @@ -2,6 +2,9 @@ // 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; +using System.ComponentModel.DataAnnotations; +using System.Linq; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; @@ -97,5 +100,143 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests // Assert Assert.False(result.IsModelSet); } + + [Fact] + public async Task BindModelAsync_WithBindProperty_BindNever_DoesNotBindModel() + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameter = new ParameterDescriptor() + { + Name = nameof(TestController.BindNeverProp), + ParameterType = typeof(string), + BindingInfo = BindingInfo.GetBindingInfo(new[] { new BindPropertyAttribute() }), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.Method = "POST"; + request.QueryString = new QueryString($"?{parameter.Name}=Joey"); + }); + + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var modelMetadata = modelMetadataProvider + .GetMetadataForProperty(typeof(TestController), parameter.Name); + + // Act + var result = await parameterBinder.BindModelAsync( + parameter, + testContext, + modelMetadataProvider, + modelMetadata); + + // Assert + Assert.False(result.IsModelSet); + Assert.True(testContext.ModelState.IsValid); + } + + [Theory] + [InlineData(null, false)] + [InlineData(123, true)] + public async Task BindModelAsync_WithBindProperty_EnforcesBindRequired(int? input, bool isValid) + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameter = new ParameterDescriptor() + { + Name = nameof(TestController.BindRequiredProp), + ParameterType = typeof(string), + BindingInfo = BindingInfo.GetBindingInfo(new[] { new BindPropertyAttribute() }), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.Method = "POST"; + + if (input.HasValue) + { + request.QueryString = new QueryString($"?{parameter.Name}={input.Value}"); + } + }); + + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var modelMetadata = modelMetadataProvider + .GetMetadataForProperty(typeof(TestController), parameter.Name); + + // Act + var result = await parameterBinder.BindModelAsync( + parameter, + testContext, + modelMetadataProvider, + modelMetadata); + + // Assert + Assert.Equal(input.HasValue, result.IsModelSet); + Assert.Equal(isValid, testContext.ModelState.IsValid); + if (isValid) + { + Assert.Equal(input.Value, Assert.IsType(result.Model)); + } + } + + [Theory] + [InlineData("RequiredAndStringLengthProp", null, false)] + [InlineData("RequiredAndStringLengthProp", "", false)] + [InlineData("RequiredAndStringLengthProp", "abc", true)] + [InlineData("RequiredAndStringLengthProp", "abcTooLong", false)] + [InlineData("DisplayNameStringLengthProp", null, true)] + [InlineData("DisplayNameStringLengthProp", "", true)] + [InlineData("DisplayNameStringLengthProp", "abc", true)] + [InlineData("DisplayNameStringLengthProp", "abcTooLong", false, "My Display Name")] + public async Task BindModelAsync_WithBindProperty_EnforcesDataAnnotationsAttributes( + string propertyName, string input, bool isValid, string displayName = null) + { + // Arrange + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var parameter = new ParameterDescriptor() + { + Name = propertyName, + ParameterType = typeof(string), + BindingInfo = BindingInfo.GetBindingInfo(new[] { new BindPropertyAttribute() }), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(request => + { + request.Method = "POST"; + + if (input != null) + { + request.QueryString = new QueryString($"?{parameter.Name}={input}"); + } + }); + + var modelMetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var modelMetadata = modelMetadataProvider + .GetMetadataForProperty(typeof(TestController), parameter.Name); + + // Act + var result = await parameterBinder.BindModelAsync( + parameter, + testContext, + modelMetadataProvider, + modelMetadata); + + // Assert + Assert.Equal(input != null, result.IsModelSet); + Assert.Equal(isValid, testContext.ModelState.IsValid); + if (!isValid) + { + var message = testContext.ModelState[propertyName].Errors.Single().ErrorMessage; + Assert.Contains(displayName ?? parameter.Name, message); + } + } + + class TestController + { + [BindNever] public string BindNeverProp { get; set; } + [BindRequired] public int BindRequiredProp { get; set; } + [Required, StringLength(3)] public string RequiredAndStringLengthProp { get; set; } + [DisplayName("My Display Name"), StringLength(3)] public string DisplayNameStringLengthProp { get; set; } + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs index 7ccd40514b..634c9547be 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ModelBindingTestHelper.cs @@ -76,25 +76,41 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests return new ParameterBinder( metadataProvider, - new ModelBinderFactory(metadataProvider, options), - GetObjectValidator(metadataProvider, options)); + GetModelBinderFactory(metadataProvider, options), + new CompositeModelValidatorProvider(GetModelValidatorProviders(options))); + } + + public static IModelBinderFactory GetModelBinderFactory( + IModelMetadataProvider metadataProvider, + IOptions options = null) + { + if (options == null) + { + options = GetServices().GetRequiredService>(); + } + + return new ModelBinderFactory(metadataProvider, options); } public static IObjectModelValidator GetObjectValidator( IModelMetadataProvider metadataProvider, IOptions options = null) { - IList validatorProviders; + return new DefaultObjectValidator( + metadataProvider, + GetModelValidatorProviders(options)); + } + + private static IList GetModelValidatorProviders(IOptions options) + { if (options == null) { - validatorProviders = TestModelValidatorProvider.CreateDefaultProvider().ValidatorProviders; + return TestModelValidatorProvider.CreateDefaultProvider().ValidatorProviders; } else { - validatorProviders = options.Value.ModelValidatorProviders; + return options.Value.ModelValidatorProviders; } - - return new DefaultObjectValidator(metadataProvider, validatorProviders); } private static HttpContext GetHttpContext( diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ParameterBinderExtensions.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ParameterBinderExtensions.cs index 7fd951c667..8d839c83a3 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ParameterBinderExtensions.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ParameterBinderExtensions.cs @@ -18,5 +18,31 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests return await parameterBinder.BindModelAsync(context, valueProvider, parameter); } + public static async Task BindModelAsync( + this ParameterBinder parameterBinder, + ParameterDescriptor parameter, + ControllerContext context, + IModelMetadataProvider modelMetadataProvider, + ModelMetadata modelMetadata) + { + var valueProvider = await CompositeValueProvider.CreateAsync(context); + + var modelBinderFactory = ModelBindingTestHelper.GetModelBinderFactory(modelMetadataProvider); + + var modelBinder = modelBinderFactory.CreateBinder(new ModelBinderFactoryContext + { + BindingInfo = parameter.BindingInfo, + Metadata = modelMetadata, + CacheToken = parameter, + }); + + return await parameterBinder.BindModelAsync( + context, + modelBinder, + valueProvider, + parameter, + modelMetadata, + value: null); + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs index 88de3835d2..3b823077ba 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerProviderTest.cs @@ -475,7 +475,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal var parameterBinder = new ParameterBinder( modelMetadataProvider, TestModelBinderFactory.CreateDefault(), - Mock.Of()); + Mock.Of()); return new PageActionInvokerProvider( loader, diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs index 677d3c8416..4cd7c46907 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PageActionInvokerTest.cs @@ -1249,11 +1249,11 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal private static ParameterBinder GetParameterBinder( IModelBinderFactory factory = null, - IObjectModelValidator validator = null) + IModelValidatorProvider validator = null) { if (validator == null) { - validator = CreateMockValidator(); + validator = CreateMockValidatorProvider(); } if (factory == null) @@ -1267,15 +1267,12 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal validator); } - private static IObjectModelValidator CreateMockValidator() + private static IModelValidatorProvider CreateMockValidatorProvider() { - var mockValidator = new Mock(MockBehavior.Strict); + var mockValidator = new Mock(MockBehavior.Strict); mockValidator - .Setup(o => o.Validate( - It.IsAny(), - It.IsAny(), - It.IsAny(), - It.IsAny())); + .Setup(o => o.CreateValidators( + It.IsAny())); return mockValidator.Object; } diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PagePropertyBinderFactoryTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PagePropertyBinderFactoryTest.cs index c8a3bdb1f2..5427b649ac 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PagePropertyBinderFactoryTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/PagePropertyBinderFactoryTest.cs @@ -29,7 +29,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal var binder = new ParameterBinder( modelMetadataProvider, TestModelBinderFactory.CreateDefault(), - Mock.Of()); + Mock.Of()); // Act var factory = PagePropertyBinderFactory.CreateBinder(binder, modelMetadataProvider, actionDescriptor); @@ -52,7 +52,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal var binder = new ParameterBinder( TestModelMetadataProvider.CreateDefaultProvider(), TestModelBinderFactory.CreateDefault(), - Mock.Of()); + Mock.Of()); // Act var factory = PagePropertyBinderFactory.CreateBinder(binder, modelMetadataProvider, actionDescriptor); @@ -73,7 +73,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal var binder = new ParameterBinder( modelMetadataProvider, TestModelBinderFactory.CreateDefault(), - Mock.Of()); + Mock.Of()); // Act var factory = PagePropertyBinderFactory.CreateBinder(binder, modelMetadataProvider, actionDescriptor); @@ -95,7 +95,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal var binder = new ParameterBinder( modelMetadataProvider, TestModelBinderFactory.CreateDefault(), - Mock.Of()); + Mock.Of()); // Act var factory = PagePropertyBinderFactory.CreateBinder(binder, modelMetadataProvider, actionDescriptor); @@ -116,7 +116,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal var binder = new ParameterBinder( modelMetadataProvider, TestModelBinderFactory.CreateDefault(), - Mock.Of()); + Mock.Of()); // Act var factory = PagePropertyBinderFactory.CreateBinder(binder, modelMetadataProvider, actionDescriptor); @@ -138,7 +138,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal var binder = new ParameterBinder( modelMetadataProvider, TestModelBinderFactory.CreateDefault(), - Mock.Of()); + Mock.Of()); // Act var factory = PagePropertyBinderFactory.CreateBinder(binder, modelMetadataProvider, actionDescriptor); @@ -460,7 +460,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal : base( TestModelMetadataProvider.CreateDefaultProvider(), TestModelBinderFactory.CreateDefault(), - Mock.Of()) + Mock.Of()) { _args = args; } diff --git a/test/Microsoft.AspNetCore.Mvc.TestCommon/TestModelMetadataProvider.cs b/test/Microsoft.AspNetCore.Mvc.TestCommon/TestModelMetadataProvider.cs index 360baf953b..e14bcdbf25 100644 --- a/test/Microsoft.AspNetCore.Mvc.TestCommon/TestModelMetadataProvider.cs +++ b/test/Microsoft.AspNetCore.Mvc.TestCommon/TestModelMetadataProvider.cs @@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public class TestModelMetadataProvider : DefaultModelMetadataProvider { // Creates a provider with all the defaults - includes data annotations - public static IModelMetadataProvider CreateDefaultProvider(IStringLocalizerFactory stringLocalizerFactory = null) + public static ModelMetadataProvider CreateDefaultProvider(IStringLocalizerFactory stringLocalizerFactory = null) { var detailsProviders = new IMetadataDetailsProvider[] { diff --git a/test/WebSites/FormatterWebSite/Controllers/TopLevelValidationController.cs b/test/WebSites/FormatterWebSite/Controllers/TopLevelValidationController.cs new file mode 100644 index 0000000000..92b27adaf4 --- /dev/null +++ b/test/WebSites/FormatterWebSite/Controllers/TopLevelValidationController.cs @@ -0,0 +1,62 @@ +// 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.ComponentModel; +using System.ComponentModel.DataAnnotations; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.ModelBinding; + +namespace FormatterWebSite.Controllers +{ + public class TopLevelValidationController : Controller + { + [BindProperty] public int OptionalProp { get; set; } + [BindProperty, Required] public int RequiredProp { get; set; } + [BindProperty, BindRequired] public int BindRequiredProp { get; set; } + [BindProperty, Required, BindRequired] public int RequiredAndBindRequiredProp { get; set; } + [BindProperty, StringLength(5)] public string OptionalStringLengthProp { get; set; } + [BindProperty, Range(1, 100), DisplayName("Some Display Name For Prop")] public int OptionalRangeDisplayNameProp { get; set; } + + // Despite the Required/BindRequired attributes, these properties won't be validated + // because they aren't [BindProperty] properties (hence aren't involved in binding). + [Required] public int UnboundRequiredProp { get; set; } + [BindRequired] public int UnboundBindRequiredProp { get; set; } + + // The [BindNever] overrides [BindProperty], meaning [Required] will not apply + // (nor will any incoming value be used) + [BindProperty, BindNever, Required] public string BindNeverRequiredProp { get; set; } + + public IActionResult Index( + int optionalParam, + [Required] int requiredParam, + [BindRequired] int bindRequiredParam, + [Required, BindRequired] int requiredAndBindRequiredParam, + [StringLength(5)] string optionalStringLengthParam, + [Range(1, 100), Display(Name = "Some Display Name For Param")] int optionalRangeDisplayNameParam) + { + if (ModelState.IsValid) + { + return Content($@" + [{ nameof(OptionalProp) }:{ OptionalProp }] + [{ nameof(RequiredProp) }:{ RequiredProp }] + [{ nameof(BindRequiredProp) }:{ BindRequiredProp }] + [{ nameof(RequiredAndBindRequiredProp) }:{ RequiredAndBindRequiredProp }] + [{ nameof(OptionalStringLengthProp) }:{ OptionalStringLengthProp }] + [{ nameof(OptionalRangeDisplayNameProp) }:{ OptionalRangeDisplayNameProp }] + [{ nameof(UnboundRequiredProp) }:{ UnboundRequiredProp }] + [{ nameof(UnboundBindRequiredProp) }:{ UnboundBindRequiredProp }] + [{ nameof(BindNeverRequiredProp) }:{ BindNeverRequiredProp }] + [{ nameof(optionalParam) }:{ optionalParam }] + [{ nameof(requiredParam) }:{ requiredParam }] + [{ nameof(bindRequiredParam) }:{ bindRequiredParam }] + [{ nameof(requiredAndBindRequiredParam) }:{ requiredAndBindRequiredParam }] + [{ nameof(optionalStringLengthParam) }:{ optionalStringLengthParam }] + [{ nameof(optionalRangeDisplayNameParam) }:{ optionalRangeDisplayNameParam }]"); + } + else + { + return BadRequest(ModelState); + } + } + } +}