Fix #1712 - remove reflection from validation code in MutableObjectModelBinder

This change moves [BindingBehavior(...)] and friends into the model
metadata layer, and removes the reflection code from
MutableObjectModelBinder that was looking for them previously.
This commit is contained in:
Ryan Nowak 2015-04-16 14:53:57 -07:00
parent 9d2b1822d9
commit f3679f214e
8 changed files with 566 additions and 35 deletions

View File

@ -8,7 +8,6 @@ using System.Linq;
using System.Reflection; using System.Reflection;
using System.Threading.Tasks; using System.Threading.Tasks;
using Microsoft.AspNet.Mvc.ModelBinding.Internal; using Microsoft.AspNet.Mvc.ModelBinding.Internal;
using Microsoft.AspNet.Mvc.ModelBinding.Metadata;
using Microsoft.AspNet.Mvc.ModelBinding.Validation; using Microsoft.AspNet.Mvc.ModelBinding.Validation;
namespace Microsoft.AspNet.Mvc.ModelBinding namespace Microsoft.AspNet.Mvc.ModelBinding
@ -313,19 +312,23 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
internal static PropertyValidationInfo GetPropertyValidationInfo(ModelBindingContext bindingContext) internal static PropertyValidationInfo GetPropertyValidationInfo(ModelBindingContext bindingContext)
{ {
var validationInfo = new PropertyValidationInfo(); var validationInfo = new PropertyValidationInfo();
var modelTypeInfo = bindingContext.ModelType.GetTypeInfo();
var typeAttribute = modelTypeInfo.GetCustomAttribute<BindingBehaviorAttribute>(); foreach (var propertyMetadata in bindingContext.ModelMetadata.Properties)
var properties = bindingContext.ModelType.GetProperties(BindingFlags.Public | BindingFlags.Instance);
foreach (var property in properties)
{ {
var propertyName = property.Name; var propertyName = propertyMetadata.PropertyName;
var propertyMetadata = bindingContext.ModelMetadata.Properties[propertyName];
if (propertyMetadata == null) if (!propertyMetadata.IsBindingAllowed)
{ {
// Skip indexer properties and others ModelMetadata ignores. // Nothing to do here if binding is not allowed.
validationInfo.SkipProperties.Add(propertyName);
continue; continue;
} }
if (propertyMetadata.IsBindingRequired)
{
validationInfo.RequiredProperties.Add(propertyName);
}
var validatorProviderContext = new ModelValidatorProviderContext(propertyMetadata); var validatorProviderContext = new ModelValidatorProviderContext(propertyMetadata);
bindingContext.OperationBindingContext.ValidatorProvider.GetValidators(validatorProviderContext); bindingContext.OperationBindingContext.ValidatorProvider.GetValidators(validatorProviderContext);
@ -334,29 +337,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
if (requiredValidator != null) if (requiredValidator != null)
{ {
validationInfo.RequiredValidators[propertyName] = requiredValidator; validationInfo.RequiredValidators[propertyName] = requiredValidator;
}
var propertyAttribute = property.GetCustomAttribute<BindingBehaviorAttribute>();
var bindingBehaviorAttribute = propertyAttribute ?? typeAttribute;
if (bindingBehaviorAttribute != null)
{
switch (bindingBehaviorAttribute.Behavior)
{
case BindingBehavior.Required:
validationInfo.RequiredProperties.Add(propertyName);
break;
case BindingBehavior.Never:
validationInfo.SkipProperties.Add(propertyName);
break;
}
}
else if (requiredValidator != null)
{
validationInfo.RequiredProperties.Add(propertyName);
}
else if (propertyMetadata.IsBindingRequired)
{
validationInfo.RequiredProperties.Add(propertyName); validationInfo.RequiredProperties.Add(propertyName);
} }
} }

View File

@ -28,9 +28,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
/// </summary> /// </summary>
public Type BinderType { get; set; } public Type BinderType { get; set; }
/// <summary>
/// Gets or sets a value indicating whether or not the property can be model bound.
/// Will be ignored if the model metadata being created does not represent a property.
/// See <see cref="ModelMetadata.IsBindingAllowed"/>.
/// </summary>
public bool IsBindingAllowed { get; set; } = true;
/// <summary> /// <summary>
/// Gets or sets a value indicating whether or not the request must contain a value for the model. /// Gets or sets a value indicating whether or not the request must contain a value for the model.
/// Will be ignored if the model metadata being created is not a property. /// Will be ignored if the model metadata being created does not represent a property.
/// See <see cref="ModelMetadata.IsBindingRequired"/>. /// See <see cref="ModelMetadata.IsBindingRequired"/>.
/// </summary> /// </summary>
public bool IsBindingRequired { get; set; } public bool IsBindingRequired { get; set; }

View File

@ -4,6 +4,7 @@
using System; using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.Linq; using System.Linq;
using System.Reflection;
using Microsoft.Framework.Internal; using Microsoft.Framework.Internal;
namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
@ -53,6 +54,27 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
context.BindingMetadata.PropertyBindingPredicateProvider = new CompositePredicateProvider( context.BindingMetadata.PropertyBindingPredicateProvider = new CompositePredicateProvider(
predicateProviders); predicateProviders);
} }
if (context.Key.MetadataKind == ModelMetadataKind.Property)
{
// BindingBehavior can fall back to attributes on the Container Type, but we should ignore
// attributes on the Property Type.
var bindingBehavior = context.PropertyAttributes.OfType<BindingBehaviorAttribute>().FirstOrDefault();
if (bindingBehavior == null)
{
bindingBehavior =
context.Key.ContainerType.GetTypeInfo()
.GetCustomAttributes(typeof(BindingBehaviorAttribute), inherit: true)
.OfType<BindingBehaviorAttribute>()
.FirstOrDefault();
}
if (bindingBehavior != null)
{
context.BindingMetadata.IsBindingAllowed = bindingBehavior.Behavior != BindingBehavior.Never;
context.BindingMetadata.IsBindingRequired = bindingBehavior.Behavior == BindingBehavior.Required;
}
}
} }
private class CompositePredicateProvider : IPropertyBindingPredicateProvider private class CompositePredicateProvider : IPropertyBindingPredicateProvider

View File

@ -255,12 +255,35 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
} }
} }
/// <inheritdoc />
public override bool IsBindingAllowed
{
get
{
if (MetadataKind == ModelMetadataKind.Property)
{
return BindingMetadata.IsBindingAllowed;
}
else
{
return true;
}
}
}
/// <inheritdoc /> /// <inheritdoc />
public override bool IsBindingRequired public override bool IsBindingRequired
{ {
get get
{ {
return BindingMetadata.IsBindingRequired; if (MetadataKind == ModelMetadataKind.Property)
{
return BindingMetadata.IsBindingRequired;
}
else
{
return false;
}
} }
} }

View File

@ -166,12 +166,22 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
/// </remarks> /// </remarks>
public abstract bool HideSurroundingHtml { get; } public abstract bool HideSurroundingHtml { get; }
/// <summary>
/// Gets a value indicating whether or not the model value can be bound by model binding. This is only
/// applicable when the current instance represents a property.
/// </summary>
/// <remarks>
/// If <c>true</c> then the model value is considered supported by model binding and can be set
/// based on provided input in the request.
/// </remarks>
public abstract bool IsBindingAllowed { get; }
/// <summary> /// <summary>
/// Gets a value indicating whether or not the model value is required by model binding. This is only /// Gets a value indicating whether or not the model value is required by model binding. This is only
/// applicable when the current instance represents a property. /// applicable when the current instance represents a property.
/// </summary> /// </summary>
/// <remarks> /// <remarks>
/// If <c>true</c> then the model value is considered required by model-binding and must have a value /// If <c>true</c> then the model value is considered required by model binding and must have a value
/// supplied in the request to be considered valid. /// supplied in the request to be considered valid.
/// </remarks> /// </remarks>
public abstract bool IsBindingRequired { get; } public abstract bool IsBindingRequired { get; }

View File

@ -147,5 +147,392 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
// Assert // Assert
Assert.Equal(BindingSource.Body, context.BindingMetadata.BindingSource); Assert.Equal(BindingSource.Body, context.BindingMetadata.BindingSource);
} }
[Fact]
public void GetBindingDetails_FindsBindingBehaviorNever_OnProperty()
{
// Arrange
var propertyAttributes = new object[]
{
new BindingBehaviorAttribute(BindingBehavior.Never),
};
var context = new BindingMetadataProviderContext(
ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)),
new ModelAttributes(propertyAttributes, typeAttributes: new object[0]));
var provider = new DefaultBindingMetadataProvider();
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.False(context.BindingMetadata.IsBindingAllowed);
Assert.False(context.BindingMetadata.IsBindingRequired);
}
[Fact]
public void GetBindingDetails_FindsBindNever_OnProperty()
{
// Arrange
var propertyAttributes = new object[]
{
new BindNeverAttribute(),
};
var context = new BindingMetadataProviderContext(
ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)),
new ModelAttributes(propertyAttributes, typeAttributes: new object[0]));
var provider = new DefaultBindingMetadataProvider();
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.False(context.BindingMetadata.IsBindingAllowed);
Assert.False(context.BindingMetadata.IsBindingRequired);
}
[Fact]
public void GetBindingDetails_FindsBindingBehaviorOptional_OnProperty()
{
// Arrange
var propertyAttributes = new object[]
{
new BindingBehaviorAttribute(BindingBehavior.Optional),
};
var context = new BindingMetadataProviderContext(
ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)),
new ModelAttributes(propertyAttributes, typeAttributes: new object[0]));
var provider = new DefaultBindingMetadataProvider();
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.True(context.BindingMetadata.IsBindingAllowed);
Assert.False(context.BindingMetadata.IsBindingRequired);
}
[Fact]
public void GetBindingDetails_FindsBindingBehaviorRequired_OnProperty()
{
// Arrange
var propertyAttributes = new object[]
{
new BindingBehaviorAttribute(BindingBehavior.Required),
};
var context = new BindingMetadataProviderContext(
ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)),
new ModelAttributes(propertyAttributes, typeAttributes: new object[0]));
var provider = new DefaultBindingMetadataProvider();
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.True(context.BindingMetadata.IsBindingAllowed);
Assert.True(context.BindingMetadata.IsBindingRequired);
}
[Fact]
public void GetBindingDetails_FindsBindRequired_OnProperty()
{
// Arrange
var propertyAttributes = new object[]
{
new BindRequiredAttribute(),
};
var context = new BindingMetadataProviderContext(
ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)),
new ModelAttributes(propertyAttributes, typeAttributes: new object[0]));
var provider = new DefaultBindingMetadataProvider();
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.True(context.BindingMetadata.IsBindingAllowed);
Assert.True(context.BindingMetadata.IsBindingRequired);
}
// These attributes have conflicting behavior - the 'required' behavior should be used because
// of ordering.
[Fact]
public void GetBindingDetails_UsesFirstAttribute()
{
// Arrange
var propertyAttributes = new object[]
{
new BindingBehaviorAttribute(BindingBehavior.Required),
new BindNeverAttribute(),
};
var context = new BindingMetadataProviderContext(
ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)),
new ModelAttributes(propertyAttributes, typeAttributes: new object[0]));
var provider = new DefaultBindingMetadataProvider();
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.True(context.BindingMetadata.IsBindingAllowed);
Assert.True(context.BindingMetadata.IsBindingRequired);
}
[Fact]
public void GetBindingDetails_FindsBindRequired_OnContainerClass()
{
// Arrange
var context = new BindingMetadataProviderContext(
ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindRequiredOnClass)),
new ModelAttributes(propertyAttributes: new object[0], typeAttributes: new object[0]));
var provider = new DefaultBindingMetadataProvider();
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.True(context.BindingMetadata.IsBindingAllowed);
Assert.True(context.BindingMetadata.IsBindingRequired);
}
[Fact]
public void GetBindingDetails_FindsBindNever_OnContainerClass()
{
// Arrange
var context = new BindingMetadataProviderContext(
ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindNeverOnClass)),
new ModelAttributes(propertyAttributes: new object[0], typeAttributes: new object[0]));
var provider = new DefaultBindingMetadataProvider();
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.False(context.BindingMetadata.IsBindingAllowed);
Assert.False(context.BindingMetadata.IsBindingRequired);
}
[Fact]
public void GetBindingDetails_FindsBindNever_OnBaseClass()
{
// Arrange
var context = new BindingMetadataProviderContext(
ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(InheritedBindNeverOnClass)),
new ModelAttributes(propertyAttributes: new object[0], typeAttributes: new object[0]));
var provider = new DefaultBindingMetadataProvider();
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.False(context.BindingMetadata.IsBindingAllowed);
Assert.False(context.BindingMetadata.IsBindingRequired);
}
[Fact]
public void GetBindingDetails_OverrideBehaviorOnClass_OverrideWithOptional()
{
// Arrange
var propertyAttributes = new object[]
{
new BindingBehaviorAttribute(BindingBehavior.Optional)
};
var context = new BindingMetadataProviderContext(
ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindNeverOnClass)),
new ModelAttributes(propertyAttributes, typeAttributes: new object[0]));
var provider = new DefaultBindingMetadataProvider();
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.True(context.BindingMetadata.IsBindingAllowed);
Assert.False(context.BindingMetadata.IsBindingRequired);
}
[Fact]
public void GetBindingDetails_OverrideBehaviorOnClass_OverrideWithRequired()
{
// Arrange
var propertyAttributes = new object[]
{
new BindRequiredAttribute()
};
var context = new BindingMetadataProviderContext(
ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindNeverOnClass)),
new ModelAttributes(propertyAttributes, typeAttributes: new object[0]));
var provider = new DefaultBindingMetadataProvider();
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.True(context.BindingMetadata.IsBindingAllowed);
Assert.True(context.BindingMetadata.IsBindingRequired);
}
[Fact]
public void GetBindingDetails_OverrideInheritedBehaviorOnClass_OverrideWithRequired()
{
// Arrange
var propertyAttributes = new object[]
{
new BindRequiredAttribute()
};
var context = new BindingMetadataProviderContext(
ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(InheritedBindNeverOnClass)),
new ModelAttributes(propertyAttributes, typeAttributes: new object[0]));
var provider = new DefaultBindingMetadataProvider();
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.True(context.BindingMetadata.IsBindingAllowed);
Assert.True(context.BindingMetadata.IsBindingRequired);
}
[Fact]
public void GetBindingDetails_OverrideBehaviorOnClass_OverrideWithNever()
{
// Arrange
var propertyAttributes = new object[]
{
new BindNeverAttribute(),
};
var context = new BindingMetadataProviderContext(
ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindRequiredOnClass)),
new ModelAttributes(propertyAttributes, typeAttributes: new object[0]));
var provider = new DefaultBindingMetadataProvider();
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.False(context.BindingMetadata.IsBindingAllowed);
Assert.False(context.BindingMetadata.IsBindingRequired);
}
// This overrides an inherited class-level attribute with a different class-level attribute.
[Fact]
public void GetBindingDetails_OverrideBehaviorOnBaseClass_OverrideWithRequired_OnClass()
{
// Arrange
var context = new BindingMetadataProviderContext(
ModelMetadataIdentity.ForProperty(typeof(string), "Property", typeof(BindRequiredOverridesInheritedBindNever)),
new ModelAttributes(propertyAttributes: new object[0], typeAttributes: new object[0]));
var provider = new DefaultBindingMetadataProvider();
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.True(context.BindingMetadata.IsBindingAllowed);
Assert.True(context.BindingMetadata.IsBindingRequired);
}
[Theory]
[InlineData(true)]
[InlineData(false)]
public void GetBindingDetails_BindingBehaviorLeftAlone_ForTypeMetadata(bool initialValue)
{
// Arrange
var attributes = new object[]
{
new BindingBehaviorAttribute(BindingBehavior.Required),
};
var context = new BindingMetadataProviderContext(
ModelMetadataIdentity.ForType(typeof(string)),
new ModelAttributes(attributes));
// These values shouldn't be changed since this is a Type-Metadata
context.BindingMetadata.IsBindingAllowed = initialValue;
context.BindingMetadata.IsBindingRequired = initialValue;
var provider = new DefaultBindingMetadataProvider();
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.Equal(initialValue, context.BindingMetadata.IsBindingAllowed);
Assert.Equal(initialValue, context.BindingMetadata.IsBindingRequired);
}
// Unlike most model metadata settings, BindingBehavior can be specified on the *container type*
// but not on the property type.
[Theory]
[InlineData(true)]
[InlineData(false)]
public void GetBindingDetails_BindingBehaviorLeftAlone_ForAttributeOnPropertyType(bool initialValue)
{
// Arrange
var typeAttributes = new object[]
{
new BindingBehaviorAttribute(BindingBehavior.Required),
};
var context = new BindingMetadataProviderContext(
ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)),
new ModelAttributes(propertyAttributes: new object[0], typeAttributes: typeAttributes));
// These values shouldn't be changed since this is a Type-Metadata
context.BindingMetadata.IsBindingAllowed = initialValue;
context.BindingMetadata.IsBindingRequired = initialValue;
var provider = new DefaultBindingMetadataProvider();
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.Equal(initialValue, context.BindingMetadata.IsBindingAllowed);
Assert.Equal(initialValue, context.BindingMetadata.IsBindingRequired);
}
[BindNever]
private class BindNeverOnClass
{
public string Property { get; set; }
}
private class InheritedBindNeverOnClass : BindNeverOnClass
{
}
[BindRequired]
private class BindRequiredOnClass
{
public string Property { get; set; }
}
[BindRequired]
private class BindRequiredOverridesInheritedBindNever : BindNeverOnClass
{
}
} }
} }

View File

@ -726,6 +726,85 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
Assert.Null(metadata.Properties[propertyName]); Assert.Null(metadata.Properties[propertyName]);
} }
[Fact]
public void BindingBehavior_GetMetadataForType_UsesDefaultValues()
{
// Arrange
var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
var type = typeof(BindingBehaviorModel);
// Act
var metadata = metadataProvider.GetMetadataForType(type);
// Assert
Assert.True(metadata.IsBindingAllowed);
Assert.False(metadata.IsBindingRequired);
}
[Fact]
public void BindingBehavior_Override_RequiredOnProperty()
{
// Arrange
var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
var type = typeof(BindingBehaviorModel);
var propertyName = nameof(BindingBehaviorModel.BindRequiredOverride);
// Act
var metadata = metadataProvider.GetMetadataForProperty(type, propertyName);
// Assert
Assert.True(metadata.IsBindingAllowed);
Assert.True(metadata.IsBindingRequired);
}
[Fact]
public void BindingBehavior_Override_OptionalOnProperty()
{
// Arrange
var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
var type = typeof(BindingBehaviorModel);
var propertyName = nameof(BindingBehaviorModel.BindingBehaviorOptionalOverride);
// Act
var metadata = metadataProvider.GetMetadataForProperty(type, propertyName);
// Assert
Assert.True(metadata.IsBindingAllowed);
Assert.False(metadata.IsBindingRequired);
}
[Fact]
public void BindingBehavior_Never_DuplicatedPropertyAndClass()
{
// Arrange
var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
var type = typeof(BindingBehaviorModel);
var propertyName = nameof(BindingBehaviorModel.BindingBehaviorNeverDuplicatedFromClass);
// Act
var metadata = metadataProvider.GetMetadataForProperty(type, propertyName);
// Assert
Assert.False(metadata.IsBindingAllowed);
Assert.False(metadata.IsBindingRequired);
}
[Fact]
public void BindingBehavior_Never_SetOnClass()
{
// Arrange
var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
var type = typeof(BindingBehaviorModel);
var propertyName = nameof(BindingBehaviorModel.BindingBehaviorNeverSetOnClass);
// Act
var metadata = metadataProvider.GetMetadataForProperty(type, propertyName);
// Assert
Assert.False(metadata.IsBindingAllowed);
Assert.False(metadata.IsBindingRequired);
}
private IModelMetadataProvider CreateProvider(params object[] attributes) private IModelMetadataProvider CreateProvider(params object[] attributes)
{ {
return new AttributeInjectModelMetadataProvider(attributes); return new AttributeInjectModelMetadataProvider(attributes);
@ -869,6 +948,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
public string RequiredProperty { get; set; } public string RequiredProperty { get; set; }
} }
[BindNever]
private class BindingBehaviorModel
{
[BindRequired]
public int BindRequiredOverride { get; set; }
[BindingBehavior(BindingBehavior.Optional)]
public int BindingBehaviorOptionalOverride { get; set; }
[BindingBehavior(BindingBehavior.Never)]
public int BindingBehaviorNeverDuplicatedFromClass { get; set; }
public int BindingBehaviorNeverSetOnClass { get; set; }
}
private class AttributeInjectModelMetadataProvider : DefaultModelMetadataProvider private class AttributeInjectModelMetadataProvider : DefaultModelMetadataProvider
{ {
private readonly object[] _attributes; private readonly object[] _attributes;

View File

@ -309,6 +309,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
} }
} }
public override bool IsBindingAllowed
{
get
{
throw new NotImplementedException();
}
}
public override bool IsBindingRequired public override bool IsBindingRequired
{ {
get get