huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix ModelMetadata.IsRequired 

The DataAnnotationsMetadataProvider was setting the bool? IsRequired, all of the
time instead of only setting it to true when we found a RequiredAttribute.
So we never actually executed the fallback logic here. Found
this while working on removing some reflection code from the validator,
and wanted to split it out because it's simple.
This commit is contained in:
Ryan Nowak 2015-03-22 00:17:16 -07:00
parent a5da5b3acd
commit 784021cf85
8 changed files with 403 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataAnnotationsMetadataProvider.cs
View File

@ -22,7 +22,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
/// <inheritdoc />
public void GetBindingMetadata([NotNull] BindingMetadataProviderContext context)
{
context.BindingMetadata.IsRequired = context.Attributes.OfType<RequiredAttribute>().Any();
var requiredAttribute = context.Attributes.OfType<RequiredAttribute>().FirstOrDefault();
if (requiredAttribute != null)
{
context.BindingMetadata.IsRequired = true;
}
var editableAttribute = context.Attributes.OfType<EditableAttribute>().FirstOrDefault();
if (editableAttribute != null)

8
src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataMemberBindingMetadataProvider.cs → src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataMemberRequiredBindingMetadataProvider.cs
View File

@ -11,7 +11,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
/// <summary>
/// An <see cref="IBindingMetadataProvider"/> for <see cref="DataMemberAttribute.IsRequired"/>.
/// </summary>
public class DataMemberRequiredValidationMetadataProvider : IBindingMetadataProvider
public class DataMemberRequiredBindingMetadataProvider : IBindingMetadataProvider
{
/// <inheritdoc />
public void GetBindingMetadata([NotNull] BindingMetadataProviderContext context)
@ -22,6 +22,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
return;
}
if (context.BindingMetadata.IsRequired == true)
{
// This value is already required, no need to look at attributes.
return;
}
var dataMemberAttribute = context
.Attributes
.OfType<DataMemberAttribute>()

2
src/Microsoft.AspNet.Mvc/MvcOptionsSetup.cs
View File

@ -66,7 +66,7 @@ namespace Microsoft.AspNet.Mvc
options.ModelMetadataDetailsProviders.Add(new DefaultBindingMetadataProvider());
options.ModelMetadataDetailsProviders.Add(new DefaultValidationMetadataProvider());
options.ModelMetadataDetailsProviders.Add(new DataAnnotationsMetadataProvider());
options.ModelMetadataDetailsProviders.Add(new DataMemberRequiredValidationMetadataProvider());
options.ModelMetadataDetailsProviders.Add(new DataMemberRequiredBindingMetadataProvider());
// Set up validators
options.ModelValidatorProviders.Add(new DefaultModelValidatorProvider());

84
test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataAnnotationsMetadataProviderTest.cs
View File

@ -110,7 +110,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
}
[Fact]
public void GetDisplayDetails_EditableAttribute_SetsReadOnly()
public void GetDisplayDetails_EditableAttributeFalse_SetsReadOnlyTrue()
{
// Arrange
var provider = new DataAnnotationsMetadataProvider();
@ -125,7 +125,26 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
provider.GetBindingMetadata(context);
// Assert
Assert.Equal(true, context.BindingMetadata.IsReadOnly);
Assert.True(context.BindingMetadata.IsReadOnly);
}
[Fact]
public void GetDisplayDetails_EditableAttributeTrue_SetsReadOnlyFalse()
{
// Arrange
var provider = new DataAnnotationsMetadataProvider();
var editable = new EditableAttribute(allowEdit: true);
var attributes = new Attribute[] { editable };
var key = ModelMetadataIdentity.ForType(typeof(string));
var context = new BindingMetadataProviderContext(key, attributes);
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.False(context.BindingMetadata.IsReadOnly);
}
[Fact]
@ -539,6 +558,67 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
Assert.Equal(expectedKeyValuePairs, context.DisplayMetadata.EnumDisplayNamesAndValues);
}
[Fact]
public void GetBindingDetails_RequiredAttribute_SetsIsRequiredToTrue()
{
// Arrange
var provider = new DataAnnotationsMetadataProvider();
var required = new RequiredAttribute();
var attributes = new Attribute[] { required };
var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string));
var context = new BindingMetadataProviderContext(key, attributes);
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.True(context.BindingMetadata.IsRequired);
}
[Theory]
[InlineData(true)]
[InlineData(false)]
[InlineData(null)]
public void GetBindingDetails_NoRequiredAttribute_IsRequiredLeftAlone(bool? initialValue)
{
// Arrange
var provider = new DataAnnotationsMetadataProvider();
var attributes = new Attribute[] { };
var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string));
var context = new BindingMetadataProviderContext(key, attributes);
context.BindingMetadata.IsRequired = initialValue;
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.Equal(initialValue, context.BindingMetadata.IsRequired);
}
[Theory]
[InlineData(true)]
[InlineData(false)]
[InlineData(null)]
public void GetBindingDetails_NoEditableAttribute_IsReadOnlyLeftAlone(bool? initialValue)
{
// Arrange
var provider = new DataAnnotationsMetadataProvider();
var attributes = new Attribute[] { };
var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string));
var context = new BindingMetadataProviderContext(key, attributes);
context.BindingMetadata.IsReadOnly = initialValue;
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.Equal(initialValue, context.BindingMetadata.IsReadOnly);
}
private class EmptyClass
{
}

96
test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataMemberBindingMetadataProviderTest.cs
View File

@ -1,96 +0,0 @@
// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System.Runtime.Serialization;
using Xunit;
namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
{
public class DataMemberBindingMetadataProviderTest
{
[Fact]
public void ClassWithoutAttributes_NotRequired()
{
// Arrange
var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
// Act
var metadata = metadataProvider.GetMetadataForProperty(
typeof(ClassWithoutAttributes),
"TheProperty");
// Assert
Assert.False(metadata.IsRequired);
}
private class ClassWithoutAttributes
{
public string TheProperty { get; set; }
}
[Fact]
public void ClassWithDataMemberIsRequiredTrue_Validator()
{
// Arrange
var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
// Act
var metadata = metadataProvider.GetMetadataForProperty(
typeof(ClassWithDataMemberIsRequiredTrue),
"TheProperty");
// Assert
Assert.True(metadata.IsRequired);
}
[DataContract]
private class ClassWithDataMemberIsRequiredTrue
{
[DataMember(IsRequired = true)]
public string TheProperty { get; set; }
}
[Fact]
public void ClassWithDataMemberIsRequiredFalse_NoValidator()
{
// Arrange
var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
// Act
var metadata = metadataProvider.GetMetadataForProperty(
typeof(ClassWithDataMemberIsRequiredFalse),
"TheProperty");
// Assert
Assert.False(metadata.IsRequired);
}
[DataContract]
private class ClassWithDataMemberIsRequiredFalse
{
[DataMember(IsRequired = false)]
public int TheProperty { get; set; }
}
[Fact]
public void ClassWithDataMemberIsRequiredTrueWithoutDataContract_NoValidator()
{
// Arrange
var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
// Act
var metadata = metadataProvider.GetMetadataForProperty(
typeof(ClassWithDataMemberIsRequiredTrueWithoutDataContract),
"TheProperty");
// Assert
Assert.False(metadata.IsRequired);
}
private class ClassWithDataMemberIsRequiredTrueWithoutDataContract
{
[DataMember(IsRequired = true)]
public int TheProperty { get; set; }
}
}
}

168
test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataMemberRequiredBindingMetadataProviderTest.cs Normal file
View File

@ -0,0 +1,168 @@
// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System.Runtime.Serialization;
using Xunit;
namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
{
public class DataMemberRequiredBindingMetadataProviderTest
{
[Fact]
public void IsRequired_SetToTrue_WithDataMemberIsRequiredTrue()
{
// Arrange
var provider = new DataMemberRequiredBindingMetadataProvider();
var attributes = new object[]
{
new DataMemberAttribute() { IsRequired = true, }
};
var key = ModelMetadataIdentity.ForProperty(
typeof(string),
nameof(ClassWithDataMemberIsRequiredTrue.StringProperty),
typeof(ClassWithDataMemberIsRequiredTrue));
var context = new BindingMetadataProviderContext(key, attributes);
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.True(context.BindingMetadata.IsRequired);
}
[Theory]
[InlineData(true)]
[InlineData(false)]
[InlineData(null)]
public void IsRequired_LeftAlone_DataMemberIsRequiredFalse(bool? initialValue)
{
// Arrange
var provider = new DataMemberRequiredBindingMetadataProvider();
var attributes = new object[]
{
new DataMemberAttribute() { IsRequired = false, }
};
var key = ModelMetadataIdentity.ForProperty(
typeof(string),
nameof(ClassWithDataMemberIsRequiredFalse.StringProperty),
typeof(ClassWithDataMemberIsRequiredFalse));
var context = new BindingMetadataProviderContext(key, attributes);
context.BindingMetadata.IsRequired = initialValue;
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.Equal(initialValue, context.BindingMetadata.IsRequired);
}
[Theory]
[InlineData(true)]
[InlineData(false)]
[InlineData(null)]
public void IsRequired_LeftAlone_ForNonPropertyMetadata(bool? initialValue)
{
// Arrange
var provider = new DataMemberRequiredBindingMetadataProvider();
var attributes = new object[]
{
new DataMemberAttribute() { IsRequired = true, }
};
var key = ModelMetadataIdentity.ForType(typeof(ClassWithDataMemberIsRequiredTrue));
var context = new BindingMetadataProviderContext(key, attributes);
context.BindingMetadata.IsRequired = initialValue;
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.Equal(initialValue, context.BindingMetadata.IsRequired);
}
[Theory]
[InlineData(true)]
[InlineData(false)]
[InlineData(null)]
public void IsRequired_LeftAlone_WithoutDataMemberAttribute(bool? initialValue)
{
// Arrange
var provider = new DataMemberRequiredBindingMetadataProvider();
var key = ModelMetadataIdentity.ForProperty(
typeof(string),
nameof(ClassWithoutAttributes.StringProperty),
typeof(ClassWithoutAttributes));
var context = new BindingMetadataProviderContext(key, attributes: new object[0]);
context.BindingMetadata.IsRequired = initialValue;
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.Equal(initialValue, context.BindingMetadata.IsRequired);
}
[Theory]
[InlineData(true)]
[InlineData(false)]
[InlineData(null)]
public void IsRequired_LeftAlone_WithoutDataContractAttribute(bool? initialValue)
{
// Arrange
var provider = new DataMemberRequiredBindingMetadataProvider();
var attributes = new object[]
{
new DataMemberAttribute() { IsRequired = true, }
};
var key = ModelMetadataIdentity.ForProperty(
typeof(string),
nameof(ClassWithDataMemberIsRequiredTrueWithoutDataContract.StringProperty),
typeof(ClassWithDataMemberIsRequiredTrueWithoutDataContract));
var context = new BindingMetadataProviderContext(key, attributes);
context.BindingMetadata.IsRequired = initialValue;
// Act
provider.GetBindingMetadata(context);
// Assert
Assert.Equal(initialValue, context.BindingMetadata.IsRequired);
}
[DataContract]
private class ClassWithDataMemberIsRequiredTrue
{
[DataMember(IsRequired = true)]
public string StringProperty { get; set; }
}
[DataContract]
private class ClassWithDataMemberIsRequiredFalse
{
[DataMember(IsRequired = false)]
public string StringProperty { get; set; }
}
private class ClassWithDataMemberIsRequiredTrueWithoutDataContract
{
[DataMember(IsRequired = true)]
public string StringProperty { get; set; }
}
private class ClassWithoutAttributes
{
public string StringProperty { get; set; }
}
}
}

147
test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataProviderTest.cs
View File

@ -6,6 +6,7 @@ using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Linq;
using System.Reflection;
using System.Runtime.Serialization;
using Microsoft.Framework.Internal;
using Xunit;
@ -586,11 +587,143 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
Assert.Same(typeof(int), metadata.BinderType);
}
[Fact]
public void IsRequired_DefaultsToTrueForValueType()
{
// Arrange
var attributes = new object[]
{
};
var provider = CreateProvider(attributes);
// Act
var metadata = provider.GetMetadataForProperty(typeof(string), "Length");
// Assert
Assert.True(metadata.IsRequired);
}
[Fact]
public void IsRequired_DefaultsToFalseForReferenceType()
{
// Arrange
var attributes = new object[]
{
};
var provider = CreateProvider(attributes);
// Act
var metadata = provider.GetMetadataForProperty(typeof(Person), nameof(Person.Parent));
// Assert
Assert.False(metadata.IsRequired);
}
[Fact]
public void IsRequired_WithRequiredAttribute()
{
// Arrange
var provider = TestModelMetadataProvider.CreateDefaultProvider();
// Act
var metadata = provider.GetMetadataForProperty(
typeof(RequiredMember),
nameof(RequiredMember.RequiredProperty));
// Assert
Assert.True(metadata.IsRequired);
}
[Fact]
public void IsRequired_DataMemberIsRequiredTrue_SetsIsRequiredToTrue()
{
// Arrange
var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
// Act
var metadata = metadataProvider.GetMetadataForProperty(
typeof(ClassWithDataMemberIsRequiredTrue),
nameof(ClassWithDataMemberIsRequiredTrue.StringProperty));
// Assert
Assert.True(metadata.IsRequired);
}
[Fact]
public void IsRequired_DataMemberIsRequiredFalse_FalseForReferenceType()
{
// Arrange
var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
// Act
var metadata = metadataProvider.GetMetadataForProperty(
typeof(ClassWithDataMemberIsRequiredFalse),
nameof(ClassWithDataMemberIsRequiredFalse.StringProperty));
// Assert
Assert.False(metadata.IsRequired);
}
[Fact]
public void IsRequired_DataMemberIsRequiredFalse_TrueForValueType()
{
// Arrange
var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
// Act
var metadata = metadataProvider.GetMetadataForProperty(
typeof(ClassWithDataMemberIsRequiredFalse),
nameof(ClassWithDataMemberIsRequiredFalse.IntProperty));
// Assert
Assert.True(metadata.IsRequired);
}
[Fact]
public void IsRequired_DataMemberIsRequiredTrueWithoutDataContract_False()
{
// Arrange
var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();
// Act
var metadata = metadataProvider.GetMetadataForProperty(
typeof(ClassWithDataMemberIsRequiredTrueWithoutDataContract),
nameof(ClassWithDataMemberIsRequiredTrueWithoutDataContract.StringProperty));
// Assert
Assert.False(metadata.IsRequired);
}
private IModelMetadataProvider CreateProvider(params object[] attributes)
{
return new AttributeInjectModelMetadataProvider(attributes);
}
[DataContract]
private class ClassWithDataMemberIsRequiredTrue
{
[DataMember(IsRequired = true)]
public string StringProperty { get; set; }
}
[DataContract]
private class ClassWithDataMemberIsRequiredFalse
{
[DataMember(IsRequired = false)]
public string StringProperty { get; set; }
[DataMember(IsRequired = false)]
public int IntProperty { get; set; }
}
private class ClassWithDataMemberIsRequiredTrueWithoutDataContract
{
[DataMember(IsRequired = true)]
public string StringProperty { get; set; }
}
private class TestBinderTypeProvider : IBinderTypeProviderMetadata
{
public Type BinderType { get; set; }
@ -621,14 +754,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
}
}
private void ActionWithoutBindAttribute(User param)
{
}
private void ActionWithBindAttribute([Bind(new string[] { "IsAdmin" }, Prefix = "ParameterPrefix")] User param)
{
}
public class TypeBasedBinderAttribute : Attribute, IModelNameProvider
{
public string Name { get; set; }
@ -693,6 +818,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata
public int NotIncludedOrExcluded { get; set; }
}
private class RequiredMember
{
[Required]
public string RequiredProperty { get; set; }
}
private class AttributeInjectModelMetadataProvider : DefaultModelMetadataProvider
{
private readonly object[] _attributes;

2
test/Microsoft.AspNet.Mvc.TestCommon/TestModelMetadataProvider.cs
View File

@ -19,7 +19,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
new DefaultBindingMetadataProvider(),
new DefaultValidationMetadataProvider(),
new DataAnnotationsMetadataProvider(),
new DataMemberRequiredValidationMetadataProvider(),
new DataMemberRequiredBindingMetadataProvider(),
};
var compositeDetailsProvider = new DefaultCompositeMetadataDetailsProvider(detailsProviders);