diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataAnnotationsMetadataProvider.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataAnnotationsMetadataProvider.cs index 57317f3dad..10233763d6 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataAnnotationsMetadataProvider.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataAnnotationsMetadataProvider.cs @@ -22,7 +22,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata /// public void GetBindingMetadata([NotNull] BindingMetadataProviderContext context) { - context.BindingMetadata.IsRequired = context.Attributes.OfType().Any(); + var requiredAttribute = context.Attributes.OfType().FirstOrDefault(); + if (requiredAttribute != null) + { + context.BindingMetadata.IsRequired = true; + } var editableAttribute = context.Attributes.OfType().FirstOrDefault(); if (editableAttribute != null) diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataMemberBindingMetadataProvider.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataMemberRequiredBindingMetadataProvider.cs similarity index 85% rename from src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataMemberBindingMetadataProvider.cs rename to src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataMemberRequiredBindingMetadataProvider.cs index 7d31b558b8..3d848a6c50 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataMemberBindingMetadataProvider.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataMemberRequiredBindingMetadataProvider.cs @@ -11,7 +11,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata /// /// An for . /// - public class DataMemberRequiredValidationMetadataProvider : IBindingMetadataProvider + public class DataMemberRequiredBindingMetadataProvider : IBindingMetadataProvider { /// 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() diff --git a/src/Microsoft.AspNet.Mvc/MvcOptionsSetup.cs b/src/Microsoft.AspNet.Mvc/MvcOptionsSetup.cs index e9b0623fe0..8423865cfb 100644 --- a/src/Microsoft.AspNet.Mvc/MvcOptionsSetup.cs +++ b/src/Microsoft.AspNet.Mvc/MvcOptionsSetup.cs @@ -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()); diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataAnnotationsMetadataProviderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataAnnotationsMetadataProviderTest.cs index 0b50d8ed40..a9c994d46e 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataAnnotationsMetadataProviderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataAnnotationsMetadataProviderTest.cs @@ -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 { } diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataMemberBindingMetadataProviderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataMemberBindingMetadataProviderTest.cs deleted file mode 100644 index 8c41a59c31..0000000000 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataMemberBindingMetadataProviderTest.cs +++ /dev/null @@ -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; } - } - } -} diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataMemberRequiredBindingMetadataProviderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataMemberRequiredBindingMetadataProviderTest.cs new file mode 100644 index 0000000000..9b26516bc5 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataMemberRequiredBindingMetadataProviderTest.cs @@ -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; } + } + } +} diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataProviderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataProviderTest.cs index 91ebb4e296..8b7f231bfb 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataProviderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataProviderTest.cs @@ -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; diff --git a/test/Microsoft.AspNet.Mvc.TestCommon/TestModelMetadataProvider.cs b/test/Microsoft.AspNet.Mvc.TestCommon/TestModelMetadataProvider.cs index ab27911a72..01675244da 100644 --- a/test/Microsoft.AspNet.Mvc.TestCommon/TestModelMetadataProvider.cs +++ b/test/Microsoft.AspNet.Mvc.TestCommon/TestModelMetadataProvider.cs @@ -19,7 +19,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding new DefaultBindingMetadataProvider(), new DefaultValidationMetadataProvider(), new DataAnnotationsMetadataProvider(), - new DataMemberRequiredValidationMetadataProvider(), + new DataMemberRequiredBindingMetadataProvider(), }; var compositeDetailsProvider = new DefaultCompositeMetadataDetailsProvider(detailsProviders);