diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs index f0abc8f4a5..47080996bf 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelMetadata.cs @@ -8,6 +8,7 @@ using System.Diagnostics; using System.Reflection; using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.Extensions.Internal; namespace Microsoft.AspNetCore.Mvc.ModelBinding @@ -295,6 +296,13 @@ public string PropertyName /// public abstract string TemplateHint { get; } + /// + /// Gets an implementation that indicates whether this model should be + /// validated. If null, properties with this are validated. + /// + /// Defaults to null. + public virtual IPropertyValidationFilter PropertyValidationFilter => null; + /// /// Gets a value that indicates whether properties or elements of the model should be validated. /// diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Validation/IPropertyValidationFilter.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Validation/IPropertyValidationFilter.cs new file mode 100644 index 0000000000..622b41f70c --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Validation/IPropertyValidationFilter.cs @@ -0,0 +1,22 @@ +// 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. + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation +{ + /// + /// Contract for attributes that determine whether associated properties should be validated. When the attribute is + /// applied to a property, the validation system calls to determine whether to + /// validate that property. When applied to a type, the validation system calls + /// for each property that type defines to determine whether to validate it. + /// + public interface IPropertyValidationFilter + { + /// + /// Gets an indication whether the should be validated. + /// + /// to check. + /// containing . + /// true if should be validated; false otherwise. + bool ShouldValidateEntry(ValidationEntry entry, ValidationEntry parentEntry); + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Validation/ValidationEntry.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Validation/ValidationEntry.cs index d757239356..a873273ca7 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Validation/ValidationEntry.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/Validation/ValidationEntry.cs @@ -10,6 +10,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation /// public struct ValidationEntry { + private object _model; + private Func _modelAccessor; + /// /// Creates a new . /// @@ -30,7 +33,37 @@ public ValidationEntry(ModelMetadata metadata, string key, object model) Metadata = metadata; Key = key; - Model = model; + _model = model; + _modelAccessor = null; + } + + /// + /// Creates a new . + /// + /// The associated with the . + /// The model prefix associated with the . + /// A delegate that will return the . + public ValidationEntry(ModelMetadata metadata, string key, Func modelAccessor) + { + if (metadata == null) + { + throw new ArgumentNullException(nameof(metadata)); + } + + if (key == null) + { + throw new ArgumentNullException(nameof(key)); + } + + if (modelAccessor == null) + { + throw new ArgumentNullException(nameof(modelAccessor)); + } + + Metadata = metadata; + Key = key; + _model = null; + _modelAccessor = modelAccessor; } /// @@ -46,6 +79,18 @@ public ValidationEntry(ModelMetadata metadata, string key, object model) /// /// The model object. /// - public object Model { get; } + public object Model + { + get + { + if (_modelAccessor != null) + { + _model = _modelAccessor(); + _modelAccessor = null; + } + + return _model; + } + } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultComplexObjectValidationStrategy.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultComplexObjectValidationStrategy.cs index a0744bba58..135b13d2cf 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultComplexObjectValidationStrategy.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultComplexObjectValidationStrategy.cs @@ -84,36 +84,20 @@ public bool MoveNext() var propertyName = property.BinderModelName ?? property.PropertyName; var key = ModelNames.CreatePropertyModelName(_key, propertyName); - object model; - - // Our property accessors don't work on Mono 4.0.4 - see https://github.com/aspnet/External/issues/44 - // This is a workaround for what the PropertyGetter does in the background. - if (IsMono) + if (_model == null) + { + // Performance: Never create a delegate when container is null. + _entry = new ValidationEntry(property, key, model: null); + } + else if (IsMono) { - if (_model == null) - { - model = null; - } - else - { - var propertyInfo = _model.GetType().GetRuntimeProperty(property.PropertyName); - try - { - model = propertyInfo.GetValue(_model); - } - catch (TargetInvocationException ex) - { - throw ex.InnerException; - } - } + _entry = new ValidationEntry(property, key, () => GetModelOnMono(_model, property.PropertyName)); } else { - model = property.PropertyGetter(_model); + _entry = new ValidationEntry(property, key, () => GetModel(_model, property)); } - _entry = new ValidationEntry(property, key, model); - return true; } @@ -125,6 +109,26 @@ public void Reset() { throw new NotImplementedException(); } + + private static object GetModel(object container, ModelMetadata property) + { + return property.PropertyGetter(container); + } + + // Our property accessors don't work on Mono 4.0.4 - see https://github.com/aspnet/External/issues/44 + // This is a workaround for what the PropertyGetter does in the background. + private static object GetModelOnMono(object container, string propertyName) + { + var propertyInfo = container.GetType().GetRuntimeProperty(propertyName); + try + { + return propertyInfo.GetValue(container); + } + catch (TargetInvocationException ex) + { + throw ex.InnerException; + } + } } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultValidationMetadataProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultValidationMetadataProvider.cs index 2e3116185e..03faa4be62 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultValidationMetadataProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultValidationMetadataProvider.cs @@ -2,6 +2,8 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Linq; +using System.Reflection; using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; @@ -34,6 +36,24 @@ public void CreateValidationMetadata(ValidationMetadataProviderContext context) } } } + + // IPropertyValidationFilter attributes on a type affect properties in that type, not properties that have + // that type. Thus, we ignore context.TypeAttributes for properties and not check at all for types. + if (context.Key.MetadataKind == ModelMetadataKind.Property) + { + var validationFilter = context.PropertyAttributes.OfType().FirstOrDefault(); + if (validationFilter == null) + { + // No IPropertyValidationFilter attributes on the property. + // Check if container has such an attribute. + validationFilter = context.Key.ContainerType.GetTypeInfo() + .GetCustomAttributes(inherit: true) + .OfType() + .FirstOrDefault(); + } + + context.ValidationMetadata.PropertyValidationFilter = validationFilter; + } } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindNeverAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindNeverAttribute.cs index 6fa4dcf60a..3bcdfab9ff 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindNeverAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindNeverAttribute.cs @@ -7,8 +7,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { /// /// Indicates that a property should be excluded from model binding. When applied to a property, the model binding - /// system excludes that property. When applied to a type, the model binding system excludes all properties of that - /// type. + /// 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)] 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 0770991b54..b98f120e12 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindRequiredAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/BindRequiredAttribute.cs @@ -8,7 +8,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// /// Indicates that a property is required for model binding. When applied to a property, the model binding system /// requires a value for that property. When applied to a type, the model binding system requires values for all - /// properties of that type. + /// properties that type defines. /// [AttributeUsage(AttributeTargets.Class | AttributeTargets.Property, AllowMultiple = false, Inherited = true)] public sealed class BindRequiredAttribute : BindingBehaviorAttribute diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs index b99d128441..f9ef945887 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Collections.ObjectModel; using System.Linq; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata { @@ -531,6 +532,15 @@ public override string TemplateHint } } + /// + public override IPropertyValidationFilter PropertyValidationFilter + { + get + { + return ValidationMetadata.PropertyValidationFilter; + } + } + /// public override bool ValidateChildren { diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ExcludeBindingMetadataProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ExcludeBindingMetadataProvider.cs index f0c236cc21..a4f4a51550 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ExcludeBindingMetadataProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ExcludeBindingMetadataProvider.cs @@ -18,7 +18,7 @@ public class ExcludeBindingMetadataProvider : IBindingMetadataProvider /// Creates a new for the given . /// /// - /// The . All properties of this will have + /// The . All properties with this will have /// set to false. /// public ExcludeBindingMetadataProvider(Type type) diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ValidationMetadata.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ValidationMetadata.cs index 57c38cef5f..bd4ff327aa 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ValidationMetadata.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ValidationMetadata.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata { @@ -18,9 +19,15 @@ public class ValidationMetadata /// public bool? IsRequired { get; set; } + /// + /// Gets or sets an implementation that indicates whether this model + /// should be validated. See . + /// + public IPropertyValidationFilter PropertyValidationFilter { get; set; } + /// /// Gets or sets a value that indicates whether children of the model should be validated. If null - /// then will be true if either of + /// then will be true if either of /// or is true; /// false otherwise. /// diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidateNeverAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidateNeverAttribute.cs new file mode 100644 index 0000000000..a0c7181f3f --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidateNeverAttribute.cs @@ -0,0 +1,22 @@ +// 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; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation +{ + /// + /// implementation that unconditionally indicates a property should be + /// excluded from validation. When applied to a property, the validation system excludes that property. When + /// applied to a type, the validation system excludes all properties within that type. + /// + [AttributeUsage(AttributeTargets.Class | AttributeTargets.Property, AllowMultiple = false, Inherited = true)] + public sealed class ValidateNeverAttribute : Attribute, IPropertyValidationFilter + { + /// + public bool ShouldValidateEntry(ValidationEntry entry, ValidationEntry parentEntry) + { + return false; + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs index 7ad300021f..356a51800c 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs @@ -185,52 +185,26 @@ private bool Visit(ModelMetadata metadata, string key, object model) { if (_metadata.IsEnumerableType) { - return VisitEnumerableType(); + return VisitComplexType(DefaultCollectionValidationStrategy.Instance); } - else if (_metadata.IsComplexType) - { - return VisitComplexType(); - } - else + + if (_metadata.IsComplexType) { - return VisitSimpleType(); + return VisitComplexType(DefaultComplexObjectValidationStrategy.Instance); } - } - } - - private bool VisitEnumerableType() - { - var isValid = true; - if (_model != null && _metadata.ValidateChildren) - { - var strategy = _strategy ?? DefaultCollectionValidationStrategy.Instance; - isValid = VisitChildren(strategy); + return VisitSimpleType(); } - else if (_model != null) - { - // Suppress validation for the entries matching this prefix. This will temporarily set - // the current node to 'skipped' but we're going to visit it right away, so subsequent - // code will set it to 'valid' or 'invalid' - SuppressValidation(_key); - } - - // Double-checking HasReachedMaxErrors just in case this model has no elements. - if (isValid && !_modelState.HasReachedMaxErrors) - { - isValid &= ValidateNode(); - } - - return isValid; } - private bool VisitComplexType() + // Covers everything VisitSimpleType does not i.e. both enumerations and complex types. + private bool VisitComplexType(IValidationStrategy defaultStrategy) { var isValid = true; if (_model != null && _metadata.ValidateChildren) { - var strategy = _strategy ?? DefaultComplexObjectValidationStrategy.Instance; + var strategy = _strategy ?? defaultStrategy; isValid = VisitChildren(strategy); } else if (_model != null) @@ -265,14 +239,20 @@ private bool VisitChildren(IValidationStrategy strategy) { var isValid = true; var enumerator = strategy.GetChildren(_metadata, _key, _model); + var parentEntry = new ValidationEntry(_metadata, _key, _model); while (enumerator.MoveNext()) { - var metadata = enumerator.Current.Metadata; - var model = enumerator.Current.Model; - var key = enumerator.Current.Key; + var entry = enumerator.Current; + var metadata = entry.Metadata; + var key = entry.Key; + if (metadata.PropertyValidationFilter?.ShouldValidateEntry(entry, parentEntry) == false) + { + SuppressValidation(key); + continue; + } - isValid &= Visit(metadata, key, model); + isValid &= Visit(metadata, key, entry.Model); } return isValid; diff --git a/src/Microsoft.AspNetCore.Mvc.DataAnnotations/HiddenInputAttribute.cs b/src/Microsoft.AspNetCore.Mvc.DataAnnotations/HiddenInputAttribute.cs index ea5525f470..e88393d48c 100644 --- a/src/Microsoft.AspNetCore.Mvc.DataAnnotations/HiddenInputAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.DataAnnotations/HiddenInputAttribute.cs @@ -6,8 +6,8 @@ namespace Microsoft.AspNetCore.Mvc { /// - /// Indicates associated property or all properties of associated type should be edited using an <input> - /// element of type "hidden". + /// Indicates associated property or all properties with the associated type should be edited using an + /// <input> element of type "hidden". /// /// /// When overriding a inherited from a base class, should apply both diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/ViewDataInfo.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/ViewDataInfo.cs index 62e273a4ae..d2f5b928d3 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/ViewDataInfo.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/ViewDataInfo.cs @@ -27,8 +27,8 @@ public ViewDataInfo(object container, object value) /// /// Initializes a new instance of the class with info about a - /// lookup which is evaluated when is read. - /// It uses on + /// lookup which is evaluated when is read. + /// It uses on /// passing parameter to lazily evaluate the value. /// /// The that will be evaluated from. @@ -45,7 +45,7 @@ public ViewDataInfo(object container, PropertyInfo propertyInfo) /// /// The that has the . /// The that represents 's property. - /// + /// A delegate that will return the . public ViewDataInfo(object container, PropertyInfo propertyInfo, Func valueAccessor) { Container = container; diff --git a/test/Microsoft.AspNetCore.Mvc.Abstractions.Test/ModelBinding/ModelMetadataTest.cs b/test/Microsoft.AspNetCore.Mvc.Abstractions.Test/ModelBinding/ModelMetadataTest.cs index 823bb15efc..1f93ca93f0 100644 --- a/test/Microsoft.AspNetCore.Mvc.Abstractions.Test/ModelBinding/ModelMetadataTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Abstractions.Test/ModelBinding/ModelMetadataTest.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Collections.ObjectModel; using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Xunit; namespace Microsoft.AspNetCore.Mvc.ModelBinding @@ -576,6 +577,14 @@ public override string TemplateHint } } + public override IPropertyValidationFilter PropertyValidationFilter + { + get + { + throw new NotImplementedException(); + } + } + public override bool ValidateChildren { get diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultComplexObjectValidationStrategyTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultComplexObjectValidationStrategyTest.cs index 2cc4168685..9ed691d26d 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultComplexObjectValidationStrategyTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultComplexObjectValidationStrategyTest.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System; using System.Collections.Generic; using System.Linq; using Microsoft.AspNetCore.Mvc.ModelBinding; @@ -12,7 +13,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal public class DefaultComplexObjectValidationStrategyTest { [Fact] - public void EnumerateElements() + public void GetChildren_ReturnsExpectedElements() { // Arrange var model = new Person() @@ -31,23 +32,92 @@ public void EnumerateElements() // Assert Assert.Collection( BufferEntries(enumerator).OrderBy(e => e.Key), - e => + entry => { - Assert.Equal("prefix.Age", e.Key); - Assert.Equal(23, e.Model); - Assert.Same(metadata.Properties["Age"], e.Metadata); + Assert.Equal("prefix.Age", entry.Key); + Assert.Equal(23, entry.Model); + Assert.Same(metadata.Properties["Age"], entry.Metadata); }, - e => + entry => { - Assert.Equal("prefix.Id", e.Key); - Assert.Equal(1, e.Model); - Assert.Same(metadata.Properties["Id"], e.Metadata); + Assert.Equal("prefix.Id", entry.Key); + Assert.Equal(1, entry.Model); + Assert.Same(metadata.Properties["Id"], entry.Metadata); }, - e => + entry => { - Assert.Equal("prefix.Name", e.Key); - Assert.Equal("Joey", e.Model); - Assert.Same(metadata.Properties["Name"], e.Metadata); + Assert.Equal("prefix.Name", entry.Key); + Assert.Equal("Joey", entry.Model); + Assert.Same(metadata.Properties["Name"], entry.Metadata); + }); + } + + [Fact] + public void GetChildren_SetsModelNull_IfContainerNull() + { + // Arrange + Person model = null; + var metadata = TestModelMetadataProvider.CreateDefaultProvider().GetMetadataForType(typeof(Person)); + var strategy = DefaultComplexObjectValidationStrategy.Instance; + + // Act + var enumerator = strategy.GetChildren(metadata, "prefix", model); + + // Assert + Assert.Collection( + BufferEntries(enumerator).OrderBy(e => e.Key), + entry => + { + Assert.Equal("prefix.Age", entry.Key); + Assert.Null(entry.Model); + Assert.Same(metadata.Properties["Age"], entry.Metadata); + }, + entry => + { + Assert.Equal("prefix.Id", entry.Key); + Assert.Null(entry.Model); + Assert.Same(metadata.Properties["Id"], entry.Metadata); + }, + entry => + { + Assert.Equal("prefix.Name", entry.Key); + Assert.Null(entry.Model); + Assert.Same(metadata.Properties["Name"], entry.Metadata); + }); + } + + [Fact] + public void GetChildren_LazyLoadsModel() + { + // Arrange + var model = new LazyPerson(input: null); + var metadata = TestModelMetadataProvider.CreateDefaultProvider().GetMetadataForType(typeof(LazyPerson)); + var strategy = DefaultComplexObjectValidationStrategy.Instance; + + // Act + var enumerator = strategy.GetChildren(metadata, "prefix", model); + + // Assert + // Note: NREs are not thrown until the Model property is accessed. + Assert.Collection( + BufferEntries(enumerator).OrderBy(e => e.Key), + entry => + { + Assert.Equal("prefix.Age", entry.Key); + Assert.Equal(23, entry.Model); + Assert.Same(metadata.Properties["Age"], entry.Metadata); + }, + entry => + { + Assert.Equal("prefix.Id", entry.Key); + Assert.Throws(() => entry.Model); + Assert.Same(metadata.Properties["Id"], entry.Metadata); + }, + entry => + { + Assert.Equal("prefix.Name", entry.Key); + Assert.Throws(() => entry.Model); + Assert.Same(metadata.Properties["Name"], entry.Metadata); }); } @@ -70,5 +140,21 @@ private class Person public string Name { get; set; } } + + private class LazyPerson + { + private string _string; + + public LazyPerson(string input) + { + _string = input; + } + + public int Id => _string.Length; + + public int Age => 23; + + public string Name => _string.Substring(3, 5); + } } } 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 57def74719..62a0bfbcca 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataTest.cs @@ -8,6 +8,7 @@ using System.Linq; using System.Xml; using Microsoft.AspNetCore.Mvc.Internal; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Moq; using Xunit; @@ -50,6 +51,7 @@ public void DefaultValues() Assert.False(metadata.IsRequired); // Defaults to false for reference types Assert.True(metadata.ShowForDisplay); Assert.True(metadata.ShowForEdit); + Assert.False(metadata.ValidateChildren); // Defaults to true for complex and enumerable types. Assert.Null(metadata.DataTypeName); Assert.Null(metadata.Description); @@ -60,9 +62,10 @@ public void DefaultValues() Assert.Null(metadata.EnumGroupedDisplayNamesAndValues); Assert.Null(metadata.EnumNamesAndValues); Assert.Null(metadata.NullDisplayText); - Assert.Null(metadata.TemplateHint); + Assert.Null(metadata.PropertyValidationFilter); Assert.Null(metadata.SimpleDisplayProperty); Assert.Null(metadata.Placeholder); + Assert.Null(metadata.TemplateHint); Assert.Equal(10000, ModelMetadata.DefaultOrder); Assert.Equal(ModelMetadata.DefaultOrder, metadata.Order); @@ -659,6 +662,42 @@ public void ValidateChildren_ComplexAndEnumerableTypes(Type modelType) Assert.True(validateChildren); } + public static TheoryData ValidationFilterData + { + get + { + return new TheoryData + { + null, + new ValidateNeverAttribute(), + }; + } + } + + [Theory] + [MemberData(nameof(ValidationFilterData))] + public void PropertyValidationFilter_ReflectsFilter_FromValidationMetadata(IPropertyValidationFilter value) + { + // Arrange + var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); + var provider = new DefaultModelMetadataProvider(detailsProvider); + + var key = ModelMetadataIdentity.ForType(typeof(int)); + var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); + cache.ValidationMetadata = new ValidationMetadata + { + PropertyValidationFilter = value, + }; + + var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); + + // Act + var validationFilter = metadata.PropertyValidationFilter; + + // Assert + Assert.Same(value, validationFilter); + } + [Fact] public void ValidateChildren_OverrideTrue() { 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 475c92f11b..16fe4f3b37 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultValidationMetadataProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/DefaultValidationMetadataProviderTest.cs @@ -11,6 +11,104 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata { public class DefaultValidationMetadataProviderTest { + [Fact] + public void PropertyValidationFilter_ShouldValidateEntry_False_IfPropertyHasValidateNever() + { + // Arrange + var provider = new DefaultValidationMetadataProvider(); + + 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])); + + // Act + provider.CreateValidationMetadata(context); + + // Assert + Assert.NotNull(context.ValidationMetadata.PropertyValidationFilter); + Assert.False(context.ValidationMetadata.PropertyValidationFilter.ShouldValidateEntry( + new ValidationEntry(), + new ValidationEntry())); + } + + [Fact] + public void PropertyValidationFilter_Null_IfPropertyHasValidateNeverOnItsType() + { + // Arrange + var provider = new DefaultValidationMetadataProvider(); + + 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)); + + // Act + provider.CreateValidationMetadata(context); + + // Assert + Assert.Null(context.ValidationMetadata.PropertyValidationFilter); + } + + [Fact] + public void PropertyValidationFilter_Null_ForType() + { + // Arrange + var provider = new DefaultValidationMetadataProvider(); + + var attributes = new Attribute[] { new ValidateNeverAttribute() }; + var key = ModelMetadataIdentity.ForType(typeof(ValidateNeverClass)); + var context = new ValidationMetadataProviderContext(key, new ModelAttributes(attributes)); + + // Act + provider.CreateValidationMetadata(context); + + // Assert + Assert.Null(context.ValidationMetadata.PropertyValidationFilter); + } + + [Fact] + public void PropertyValidationFilter_ShouldValidateEntry_False_IfContainingTypeHasValidateNever() + { + // Arrange + var provider = new DefaultValidationMetadataProvider(); + + var key = ModelMetadataIdentity.ForProperty( + typeof(string), + nameof(ValidateNeverClass.ClassName), + typeof(ValidateNeverClass)); + var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0])); + + // Act + provider.CreateValidationMetadata(context); + + // Assert + Assert.NotNull(context.ValidationMetadata.PropertyValidationFilter); + Assert.False(context.ValidationMetadata.PropertyValidationFilter.ShouldValidateEntry( + new ValidationEntry(), + new ValidationEntry())); + } + + [Fact] + public void PropertyValidationFilter_ShouldValidateEntry_False_IfContainingTypeInheritsValidateNever() + { + // Arrange + var provider = new DefaultValidationMetadataProvider(); + + var key = ModelMetadataIdentity.ForProperty( + typeof(string), + nameof(ValidateNeverSubclass.SubclassName), + typeof(ValidateNeverSubclass)); + var context = new ValidationMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0])); + + // Act + provider.CreateValidationMetadata(context); + + // Assert + Assert.NotNull(context.ValidationMetadata.PropertyValidationFilter); + Assert.False(context.ValidationMetadata.PropertyValidationFilter.ShouldValidateEntry( + new ValidationEntry(), + new ValidationEntry())); + } + [Fact] public void GetValidationDetails_MarkedWithClientValidator_ReturnsValidator() { @@ -69,6 +167,17 @@ public void GetValidationDetails_Validator_AlreadyInContext_Ignores() Assert.Same(attribute, validatorMetadata); } + [ValidateNever] + private class ValidateNeverClass + { + public string ClassName { get; set; } + } + + private class ValidateNeverSubclass : ValidateNeverClass + { + public string SubclassName { get; set; } + } + private class TestModelValidationAttribute : Attribute, IModelValidator { public IEnumerable Validate(ModelValidationContext context) diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ValidationIntegrationTests.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ValidationIntegrationTests.cs index d55d09496b..6f66a9e0e2 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ValidationIntegrationTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ValidationIntegrationTests.cs @@ -12,6 +12,8 @@ using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Newtonsoft.Json.Linq; using Xunit; @@ -1222,6 +1224,347 @@ public async Task Validation_OverflowException_ShowsInvalidValueMessage_OnSimple Assert.Equal("The value '-123' is not valid for Zip.", error.ErrorMessage); } + private class NeverValid : IValidatableObject + { + public string NeverValidProperty { get; set; } + + public IEnumerable Validate(ValidationContext validationContext) + { + return new[] { new ValidationResult("This is not valid.") }; + } + } + + private class NeverValidAttribute : ValidationAttribute + { + protected override ValidationResult IsValid(object value, ValidationContext validationContext) + { + // By default, ValidationVisitor visits _all_ properties within a non-null complex object. + // But, like most reasonable ValidationAttributes, NeverValidAttribute ignores null property values. + if (value == null) + { + return ValidationResult.Success; + } + + return new ValidationResult("Properties with this are not valid."); + } + } + + private class ValidateSomeProperties + { + public NeverValid NeverValid { get; set; } + + [NeverValid] + public string NeverValidBecauseAttribute { get; set; } + + [ValidateNever] + [NeverValid] + public string ValidateNever { get; set; } + + [ValidateNever] + public int ValidateNeverLength => ValidateNever.Length; + } + + [ValidateNever] + private class ValidateNoProperties : ValidateSomeProperties + { + } + + [Fact] + public async Task IValidatableObject_IsValidated() + { + // Arrange + var parameter = new ParameterDescriptor + { + Name = "parameter", + ParameterType = typeof(ValidateSomeProperties), + }; + + var testContext = ModelBindingTestHelper.GetTestContext( + request => request.QueryString + = new QueryString($"?{nameof(ValidateSomeProperties.NeverValid)}.{nameof(NeverValid.NeverValidProperty)}=1")); + + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var modelState = testContext.ModelState; + + // Act + var result = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(result.IsModelSet); + var model = Assert.IsType(result.Model); + Assert.Equal("1", model.NeverValid.NeverValidProperty); + + Assert.False(modelState.IsValid); + Assert.Equal(1, modelState.ErrorCount); + Assert.Collection( + modelState, + state => + { + Assert.Equal(nameof(ValidateSomeProperties.NeverValid), state.Key); + Assert.Equal(ModelValidationState.Invalid, state.Value.ValidationState); + + var error = Assert.Single(state.Value.Errors); + Assert.Equal("This is not valid.", error.ErrorMessage); + Assert.Null(error.Exception); + }, + state => + { + Assert.Equal( + $"{nameof(ValidateSomeProperties.NeverValid)}.{nameof(NeverValid.NeverValidProperty)}", + state.Key); + Assert.Equal(ModelValidationState.Valid, state.Value.ValidationState); + }); + } + + [Fact] + public async Task CustomValidationAttribute_IsValidated() + { + // Arrange + var parameter = new ParameterDescriptor + { + Name = "parameter", + ParameterType = typeof(ValidateSomeProperties), + }; + + var testContext = ModelBindingTestHelper.GetTestContext( + request => request.QueryString + = new QueryString($"?{nameof(ValidateSomeProperties.NeverValidBecauseAttribute)}=1")); + + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var modelState = testContext.ModelState; + + // Act + var result = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(result.IsModelSet); + var model = Assert.IsType(result.Model); + Assert.Equal("1", model.NeverValidBecauseAttribute); + + Assert.False(modelState.IsValid); + Assert.Equal(1, modelState.ErrorCount); + var kvp = Assert.Single(modelState); + Assert.Equal(nameof(ValidateSomeProperties.NeverValidBecauseAttribute), kvp.Key); + var state = kvp.Value; + Assert.NotNull(state); + Assert.Equal(ModelValidationState.Invalid, state.ValidationState); + var error = Assert.Single(state.Errors); + Assert.Equal("Properties with this are not valid.", error.ErrorMessage); + Assert.Null(error.Exception); + } + + [Fact] + public async Task ValidateNeverProperty_IsSkipped() + { + // Arrange + var parameter = new ParameterDescriptor + { + Name = "parameter", + ParameterType = typeof(ValidateSomeProperties), + }; + + var testContext = ModelBindingTestHelper.GetTestContext( + request => request.QueryString + = new QueryString($"?{nameof(ValidateSomeProperties.ValidateNever)}=1")); + + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var modelState = testContext.ModelState; + + // Act + var result = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(result.IsModelSet); + var model = Assert.IsType(result.Model); + Assert.Equal("1", model.ValidateNever); + + Assert.True(modelState.IsValid); + var kvp = Assert.Single(modelState); + Assert.Equal(nameof(ValidateSomeProperties.ValidateNever), kvp.Key); + var state = kvp.Value; + Assert.NotNull(state); + Assert.Equal(ModelValidationState.Skipped, state.ValidationState); + } + + [Fact] + public async Task ValidateNeverProperty_IsSkippedWithoutAccessingModel() + { + // Arrange + var parameter = new ParameterDescriptor + { + Name = "parameter", + ParameterType = typeof(ValidateSomeProperties), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(); + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var modelState = testContext.ModelState; + + // Act + var result = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(result.IsModelSet); + var model = Assert.IsType(result.Model); + + // Note this Exception is not thrown earlier. + Assert.Throws(() => model.ValidateNeverLength); + + Assert.True(modelState.IsValid); + Assert.Empty(modelState); + } + + [Theory] + [InlineData(nameof(ValidateSomeProperties.NeverValid) + "." + nameof(NeverValid.NeverValidProperty))] + [InlineData(nameof(ValidateSomeProperties.NeverValidBecauseAttribute))] + [InlineData(nameof(ValidateSomeProperties.ValidateNever))] + public async Task PropertyWithinValidateNeverType_IsSkipped(string propertyName) + { + // Arrange + var parameter = new ParameterDescriptor + { + Name = "parameter", + ParameterType = typeof(ValidateNoProperties), + }; + + var testContext = ModelBindingTestHelper.GetTestContext( + request => request.QueryString = new QueryString($"?{propertyName}=1")); + + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var modelState = testContext.ModelState; + + // Act + var result = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(result.IsModelSet); + Assert.IsType(result.Model); + + Assert.True(modelState.IsValid); + var kvp = Assert.Single(modelState); + Assert.Equal(propertyName, kvp.Key); + var state = kvp.Value; + Assert.NotNull(state); + Assert.Equal(ModelValidationState.Skipped, state.ValidationState); + } + + private class ValidateSometimesAttribute : Attribute, IPropertyValidationFilter + { + private readonly string _otherProperty; + + public ValidateSometimesAttribute(string otherProperty) + { + // Would null-check otherProperty in real life. + _otherProperty = otherProperty; + } + + public bool ShouldValidateEntry(ValidationEntry entry, ValidationEntry parentEntry) + { + if (entry.Metadata.MetadataKind == ModelMetadataKind.Property && + parentEntry.Metadata != null) + { + // In real life, would throw an InvalidOperationException if otherProperty were null i.e. the + // property was not known. Could also assert container is non-null (see ValidationVisitor). + var container = parentEntry.Model; + var otherProperty = parentEntry.Metadata.Properties[_otherProperty]; + if (otherProperty.PropertyGetter(container) == null) + { + return false; + } + } + + return true; + } + } + + private class ValidateSomePropertiesSometimes + { + public string Control { get; set; } + + [ValidateSometimes(nameof(Control))] + public int ControlLength => Control.Length; + } + + [Fact] + public async Task PropertyToSometimesSkip_IsSkipped_IfControlIsNull() + { + // Arrange + var parameter = new ParameterDescriptor + { + Name = "parameter", + ParameterType = typeof(ValidateSomePropertiesSometimes), + }; + + var testContext = ModelBindingTestHelper.GetTestContext(); + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var modelState = testContext.ModelState; + + // Add an entry for the ControlLength property so that we can observe Skipped versus Valid states. + modelState.SetModelValue( + nameof(ValidateSomePropertiesSometimes.ControlLength), + rawValue: null, + attemptedValue: null); + + // Act + var result = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(result.IsModelSet); + var model = Assert.IsType(result.Model); + Assert.Null(model.Control); + + // Note this Exception is not thrown earlier. + Assert.Throws(() => model.ControlLength); + + Assert.True(modelState.IsValid); + var kvp = Assert.Single(modelState); + Assert.Equal(nameof(ValidateSomePropertiesSometimes.ControlLength), kvp.Key); + Assert.Equal(ModelValidationState.Skipped, kvp.Value.ValidationState); + } + + [Fact] + public async Task PropertyToSometimesSkip_IsValidated_IfControlIsNotNull() + { + // Arrange + var parameter = new ParameterDescriptor + { + Name = "parameter", + ParameterType = typeof(ValidateSomePropertiesSometimes), + }; + + var testContext = ModelBindingTestHelper.GetTestContext( + request => request.QueryString = new QueryString( + $"?{nameof(ValidateSomePropertiesSometimes.Control)}=1")); + + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var modelState = testContext.ModelState; + + // Add an entry for the ControlLength property so that we can observe Skipped versus Valid states. + modelState.SetModelValue( + nameof(ValidateSomePropertiesSometimes.ControlLength), + rawValue: null, + attemptedValue: null); + + // Act + var result = await argumentBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(result.IsModelSet); + var model = Assert.IsType(result.Model); + Assert.Equal("1", model.Control); + Assert.Equal(1, model.ControlLength); + + Assert.True(modelState.IsValid); + Assert.Collection( + modelState, + state => Assert.Equal(nameof(ValidateSomePropertiesSometimes.Control), state.Key), + state => + { + Assert.Equal(nameof(ValidateSomePropertiesSometimes.ControlLength), state.Key); + Assert.Equal(ModelValidationState.Valid, state.Value.ValidationState); + }); + } + private class Order11 { public IEnumerable
ShippingAddresses { get; set; }