diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs index f589c33eb4..064473fdd9 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs @@ -355,6 +355,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { validationInfo.RequiredProperties.Add(propertyName); } + else if (propertyMetadata.IsBindingRequired) + { + validationInfo.RequiredProperties.Add(propertyName); + } } return validationInfo; diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/BindingMetadata.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/BindingMetadata.cs index 7a82f22c94..d44cc74175 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/BindingMetadata.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/BindingMetadata.cs @@ -28,6 +28,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata /// public Type BinderType { get; set; } + /// + /// 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. + /// See . + /// + public bool IsBindingRequired { get; set; } + /// /// Gets or sets a value indicating whether or not the model is read-only. Will be ignored /// if the model metadata being created is not a property. If null then @@ -36,14 +43,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata /// public bool? IsReadOnly { get; set; } - /// - /// Gets or sets a value indicating whether or not the model is a required value. Will be ignored - /// if the model metadata being created is not a property. If null then - /// will be computed based on the model . - /// See . - /// - public bool? IsRequired { get; set; } - /// /// Gets or sets the . /// See . diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataAnnotationsMetadataProvider.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataAnnotationsMetadataProvider.cs index 6f5a2d0ab3..28f29e7bc3 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataAnnotationsMetadataProvider.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataAnnotationsMetadataProvider.cs @@ -22,12 +22,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata /// public void GetBindingMetadata([NotNull] BindingMetadataProviderContext context) { - var requiredAttribute = context.Attributes.OfType().FirstOrDefault(); - if (requiredAttribute != null) - { - context.BindingMetadata.IsRequired = true; - } - var editableAttribute = context.Attributes.OfType().FirstOrDefault(); if (editableAttribute != null) { @@ -211,6 +205,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata /// public void GetValidationMetadata([NotNull] ValidationMetadataProviderContext context) { + // RequiredAttribute marks a property as required by validation - this means that it + // must have a non-null value on the model during validation. + var requiredAttribute = context.Attributes.OfType().FirstOrDefault(); + if (requiredAttribute != null) + { + context.ValidationMetadata.IsRequired = true; + } + foreach (var attribute in context.Attributes.OfType()) { // If another provider has already added this attribute, do not repeat it. diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataMemberRequiredBindingMetadataProvider.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataMemberRequiredBindingMetadataProvider.cs index 3d848a6c50..81db4cf98e 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataMemberRequiredBindingMetadataProvider.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DataMemberRequiredBindingMetadataProvider.cs @@ -22,7 +22,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata return; } - if (context.BindingMetadata.IsRequired == true) + if (context.BindingMetadata.IsBindingRequired) { // This value is already required, no need to look at attributes. return; @@ -44,7 +44,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata { // We don't need to add a validator, just to set IsRequired = true. The validation // system will do the right thing. - context.BindingMetadata.IsRequired = true; + context.BindingMetadata.IsBindingRequired = true; } } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DefaultModelMetadata.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DefaultModelMetadata.cs index a1f7857865..dffe4a422f 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DefaultModelMetadata.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/DefaultModelMetadata.cs @@ -255,6 +255,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata } } + /// + public override bool IsBindingRequired + { + get + { + return BindingMetadata.IsBindingRequired; + } + } + /// public override bool IsEnum { @@ -305,9 +314,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata { if (!_isRequired.HasValue) { - if (BindingMetadata.IsRequired.HasValue) + if (ValidationMetadata.IsRequired.HasValue) { - _isRequired = BindingMetadata.IsRequired; + _isRequired = ValidationMetadata.IsRequired; } else { diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ValidationMetadata.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ValidationMetadata.cs index f6031272e6..6bab9aa27a 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ValidationMetadata.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/Metadata/ValidationMetadata.cs @@ -10,6 +10,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata /// public class ValidationMetadata { + /// + /// Gets or sets a value indicating whether or not the model is a required value. Will be ignored + /// if the model metadata being created is not a property. If null then + /// will be computed based on the model . + /// See . + /// + public bool? IsRequired { get; set; } + /// /// Gets a list of metadata items for validators. /// diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelMetadata.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelMetadata.cs index a23fc98ab2..e60d183ed1 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelMetadata.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelMetadata.cs @@ -166,6 +166,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// public abstract bool HideSurroundingHtml { get; } + /// + /// 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. + /// + /// + /// If true then the model value is considered required by model-binding and must have a value + /// supplied in the request to be considered valid. + /// + public abstract bool IsBindingRequired { get; } + /// /// Gets a value indicating whether or Nullable.GetUnderlyingType(ModelType) is /// for an . @@ -197,6 +207,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// Gets a value indicating whether or not the model value is required. This is only applicable when /// the current instance represents a property. /// + /// + /// + /// If true then the model value is considered required by validators. + /// + /// + /// By default an implicit will be added + /// if not present when true.. + /// + /// public abstract bool IsRequired { get; } /// diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingBindingBehaviorTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingBindingBehaviorTest.cs new file mode 100644 index 0000000000..d58d9946f3 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingBindingBehaviorTest.cs @@ -0,0 +1,128 @@ +// 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; +using System.Collections.Generic; +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using Microsoft.AspNet.Builder; +using Microsoft.Framework.DependencyInjection; +using ModelBindingWebSite; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using Xunit; + +namespace Microsoft.AspNet.Mvc.FunctionalTests +{ + public class ModelBindingBindingBehaviorTest + { + private const string SiteName = nameof(ModelBindingWebSite); + private readonly Action _app = new Startup().Configure; + private readonly Action _configureServices = new Startup().ConfigureServices; + + [Fact] + public async Task BindingBehavior_MissingRequiredProperties_ValidationErrors() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + + var url = "http://localhost/BindingBehavior/EchoModelValues"; + var request = new HttpRequestMessage(HttpMethod.Post, url); + var formData = new List> + { + new KeyValuePair("model.BehaviourOptionalProperty", "Hi"), + }; + + request.Content = new FormUrlEncodedContent(formData); + + // Act + var response = await client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + + var body = await response.Content.ReadAsStringAsync(); + var errors = JsonConvert.DeserializeObject(body); + + Assert.Equal(2, errors.Count); + + var error = Assert.Single(errors, kvp => kvp.Key == "model.BehaviourRequiredProperty"); + Assert.Equal("The 'BehaviourRequiredProperty' property is required.", ((JArray)error.Value)[0].Value()); + + error = Assert.Single(errors, kvp => kvp.Key == "model.BindRequiredProperty"); + Assert.Equal("The 'BindRequiredProperty' property is required.", ((JArray)error.Value)[0].Value()); + } + + [Fact] + public async Task BindingBehavior_OptionalIsOptional() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + + var url = "http://localhost/BindingBehavior/EchoModelValues"; + var request = new HttpRequestMessage(HttpMethod.Post, url); + var formData = new List> + { + new KeyValuePair("model.BehaviourRequiredProperty", "Hello"), + new KeyValuePair("model.BindRequiredProperty", "World!"), + }; + + request.Content = new FormUrlEncodedContent(formData); + + // Act + var response = await client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var body = await response.Content.ReadAsStringAsync(); + var model = JsonConvert.DeserializeObject(body); + + Assert.Null(model.BehaviourNeverProperty); + Assert.Null(model.BehaviourOptionalProperty); + Assert.Equal("Hello", model.BehaviourRequiredProperty); + Assert.Equal("World!", model.BindRequiredProperty); + Assert.Null(model.BindNeverProperty); + } + + [Fact] + public async Task BindingBehavior_Never_IsNotBound() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + + var url = "http://localhost/BindingBehavior/EchoModelValues"; + var request = new HttpRequestMessage(HttpMethod.Post, url); + var formData = new List> + { + + new KeyValuePair("model.BehaviourNeverProperty", "Ignored"), + new KeyValuePair("model.BehaviourOptionalProperty", "Optional"), + new KeyValuePair("model.BehaviourRequiredProperty", "Hello"), + new KeyValuePair("model.BindRequiredProperty", "World!"), + new KeyValuePair("model.BindNeverProperty", "Ignored"), + }; + + request.Content = new FormUrlEncodedContent(formData); + + // Act + var response = await client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var body = await response.Content.ReadAsStringAsync(); + var model = JsonConvert.DeserializeObject(body); + + Assert.Null(model.BehaviourNeverProperty); + Assert.Equal("Optional", model.BehaviourOptionalProperty); + Assert.Equal("Hello", model.BehaviourRequiredProperty); + Assert.Equal("World!", model.BindRequiredProperty); + Assert.Null(model.BindNeverProperty); + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingDataMemberRequiredTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingDataMemberRequiredTest.cs new file mode 100644 index 0000000000..ed04d5cf23 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingDataMemberRequiredTest.cs @@ -0,0 +1,87 @@ +// 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; +using System.Collections.Generic; +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using Microsoft.AspNet.Builder; +using Microsoft.Framework.DependencyInjection; +using ModelBindingWebSite; +using Newtonsoft.Json; +using Newtonsoft.Json.Linq; +using Xunit; + +namespace Microsoft.AspNet.Mvc.FunctionalTests +{ + public class ModelBindingDataMemberRequiredTest + { + private const string SiteName = nameof(ModelBindingWebSite); + private readonly Action _app = new Startup().Configure; + private readonly Action _configureServices = new Startup().ConfigureServices; + + [Fact] + public async Task DataMember_MissingRequiredProperty_ValidationError() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + + var url = "http://localhost/DataMemberRequired/EchoModelValues"; + var request = new HttpRequestMessage(HttpMethod.Post, url); + var formData = new List> + { + new KeyValuePair("model.ExplicitlyOptionalProperty", "Hi"), + }; + + request.Content = new FormUrlEncodedContent(formData); + + // Act + var response = await client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.BadRequest, response.StatusCode); + + var body = await response.Content.ReadAsStringAsync(); + var errors = JsonConvert.DeserializeObject(body); + + Assert.Equal(1, errors.Count); + + var error = Assert.Single(errors, kvp => kvp.Key == "model.RequiredProperty"); + Assert.Equal("The 'RequiredProperty' property is required.", ((JArray)error.Value)[0].Value()); + } + + [Fact] + public async Task DataMember_RequiredPropertyProvided_Success() + { + // Arrange + var server = TestHelper.CreateServer(_app, SiteName, _configureServices); + var client = server.CreateClient(); + + var url = "http://localhost/DataMemberRequired/EchoModelValues"; + var request = new HttpRequestMessage(HttpMethod.Post, url); + var formData = new List> + { + new KeyValuePair("model.ImplicitlyOptionalProperty", "Hello"), + new KeyValuePair("model.ExplicitlyOptionalProperty", "World!"), + new KeyValuePair("model.RequiredProperty", "Required!"), + }; + + request.Content = new FormUrlEncodedContent(formData); + + // Act + var response = await client.SendAsync(request); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var body = await response.Content.ReadAsStringAsync(); + var model = JsonConvert.DeserializeObject(body); + + Assert.Equal("Hello", model.ImplicitlyOptionalProperty); + Assert.Equal("World!", model.ExplicitlyOptionalProperty); + Assert.Equal("Required!", model.RequiredProperty); + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/MutableObjectModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/MutableObjectModelBinderTest.cs index 20f6599def..f4bed4fa10 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/MutableObjectModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/MutableObjectModelBinderTest.cs @@ -7,6 +7,7 @@ using System.Collections.Generic; using System.ComponentModel; using System.ComponentModel.DataAnnotations; using System.Linq; +using System.Runtime.Serialization; using System.Threading.Tasks; using Microsoft.AspNet.Http.Core; using Microsoft.AspNet.Mvc.ModelBinding.Validation; @@ -821,6 +822,56 @@ namespace Microsoft.AspNet.Mvc.ModelBinding Assert.Equal("The 'Age' property is required.", modelError.ErrorMessage); } + [Fact] + [ReplaceCulture] + public void ProcessDto_DataMemberIsRequiredFieldMissing_RaisesModelError() + { + // Arrange + var model = new ModelWithDataMemberIsRequired + { + Name = "original value", + Age = -20 + }; + + var containerMetadata = GetMetadataForType(model.GetType()); + var bindingContext = new ModelBindingContext + { + Model = model, + ModelMetadata = containerMetadata, + ModelName = "theModel", + OperationBindingContext = new OperationBindingContext + { + MetadataProvider = TestModelMetadataProvider.CreateDefaultProvider(), + ValidatorProvider = Mock.Of() + } + }; + var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties); + + var nameProperty = dto.PropertyMetadata.Single(o => o.PropertyName == "Name"); + dto.Results[nameProperty] = new ModelBindingResult( + "John Doe", + isModelSet: true, + key: ""); + + var testableBinder = new TestableMutableObjectModelBinder(); + + // Act + testableBinder.ProcessDto(bindingContext, dto); + + // Assert + var modelStateDictionary = bindingContext.ModelState; + Assert.False(modelStateDictionary.IsValid); + Assert.Single(modelStateDictionary); + + // Check Age error. + ModelState modelState; + Assert.True(modelStateDictionary.TryGetValue("theModel.Age", out modelState)); + var modelError = Assert.Single(modelState.Errors); + Assert.Null(modelError.Exception); + Assert.NotNull(modelError.ErrorMessage); + Assert.Equal("The 'Age' property is required.", modelError.ErrorMessage); + } + [Fact] [ReplaceCulture] public void ProcessDto_BindRequiredFieldNull_RaisesModelError() @@ -1638,6 +1689,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding public int Age { get; set; } } + [DataContract] + private class ModelWithDataMemberIsRequired + { + public string Name { get; set; } + + [DataMember(IsRequired = true)] + public int Age { get; set; } + } + [BindRequired] private class ModelWithMixedBindingBehaviors { diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataAnnotationsMetadataProviderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataAnnotationsMetadataProviderTest.cs index 96f0a510d8..836151bf59 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataAnnotationsMetadataProviderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataAnnotationsMetadataProviderTest.cs @@ -48,7 +48,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata [Theory] [MemberData(nameof(DisplayDetailsData))] - public void GetDisplayDetails_SimpleAttributes( + public void GetDisplayMetadata_SimpleAttributes( object attribute, Func accessor, object expected) @@ -68,7 +68,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata } [Fact] - public void GetDisplayDetails_FindsDisplayFormat_FromDataType() + public void GetDisplayMetadata_FindsDisplayFormat_FromDataType() { // Arrange var provider = new DataAnnotationsMetadataProvider(); @@ -88,7 +88,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata } [Fact] - public void GetDisplayDetails_FindsDisplayFormat_OverridingDataType() + public void GetDisplayMetadata_FindsDisplayFormat_OverridingDataType() { // Arrange var provider = new DataAnnotationsMetadataProvider(); @@ -111,7 +111,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata } [Fact] - public void GetDisplayDetails_EditableAttributeFalse_SetsReadOnlyTrue() + public void GetBindingMetadata_EditableAttributeFalse_SetsReadOnlyTrue() { // Arrange var provider = new DataAnnotationsMetadataProvider(); @@ -130,7 +130,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata } [Fact] - public void GetDisplayDetails_EditableAttributeTrue_SetsReadOnlyFalse() + public void GetBindingMetadata_EditableAttributeTrue_SetsReadOnlyFalse() { // Arrange var provider = new DataAnnotationsMetadataProvider(); @@ -148,28 +148,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata Assert.False(context.BindingMetadata.IsReadOnly); } - [Fact] - public void GetDisplayDetails_RequiredAttribute_SetsRequired() - { - // Arrange - var provider = new DataAnnotationsMetadataProvider(); - - var required = new RequiredAttribute(); - - var attributes = new Attribute[] { required }; - var key = ModelMetadataIdentity.ForType(typeof(string)); - var context = new BindingMetadataProviderContext(key, attributes); - - // Act - provider.GetBindingMetadata(context); - - // Assert - Assert.Equal(true, context.BindingMetadata.IsRequired); - } // This is IMPORTANT. Product code needs to use GetName() instead of .Name. It's easy to regress. [Fact] - public void GetDisplayDetails_DisplayAttribute_NameFromResources() + public void GetDisplayMetadata_DisplayAttribute_NameFromResources() { // Arrange var provider = new DataAnnotationsMetadataProvider(); @@ -198,7 +180,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata // This is IMPORTANT. Product code needs to use GetDescription() instead of .Description. It's easy to regress. [Fact] - public void GetDisplayDetails_DisplayAttribute_DescriptionFromResources() + public void GetDisplayMetadata_DisplayAttribute_DescriptionFromResources() { // Arrange var provider = new DataAnnotationsMetadataProvider(); @@ -243,7 +225,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata [InlineData(typeof(StructWithFields), false)] [InlineData(typeof(StructWithFields?), false)] [InlineData(typeof(StructWithProperties), false)] - public void GetDisplayDetails_IsEnum_ReflectsModelType(Type type, bool expectedIsEnum) + public void GetDisplayMetadata_IsEnum_ReflectsModelType(Type type, bool expectedIsEnum) { // Arrange var provider = new DataAnnotationsMetadataProvider(); @@ -277,7 +259,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata [InlineData(typeof(StructWithFields), false)] [InlineData(typeof(StructWithFields?), false)] [InlineData(typeof(StructWithProperties), false)] - public void GetDisplayDetails_IsFlagsEnum_ReflectsModelType(Type type, bool expectedIsFlagsEnum) + public void GetDisplayMetadata_IsFlagsEnum_ReflectsModelType(Type type, bool expectedIsFlagsEnum) { // Arrange var provider = new DataAnnotationsMetadataProvider(); @@ -407,7 +389,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata [Theory] [MemberData(nameof(EnumNamesData))] - public void GetDisplayDetails_EnumNamesAndValues_ReflectsModelType( + public void GetDisplayMetadata_EnumNamesAndValues_ReflectsModelType( Type type, IReadOnlyDictionary expectedDictionary) { @@ -541,7 +523,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata [Theory] [MemberData(nameof(EnumDisplayNamesData))] - public void GetDisplayDetails_EnumDisplayNamesAndValues_ReflectsModelType( + public void GetDisplayMetadata_EnumDisplayNamesAndValues_ReflectsModelType( Type type, IEnumerable> expectedKeyValuePairs) { @@ -560,7 +542,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata } [Fact] - public void GetBindingDetails_RequiredAttribute_SetsIsRequiredToTrue() + public void GetValidationMetadata_RequiredAttribute_SetsIsRequiredToTrue() { // Arrange var provider = new DataAnnotationsMetadataProvider(); @@ -569,34 +551,55 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata var attributes = new Attribute[] { required }; var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); - var context = new BindingMetadataProviderContext(key, attributes); + var context = new ValidationMetadataProviderContext(key, attributes); // Act - provider.GetBindingMetadata(context); + provider.GetValidationMetadata(context); // Assert - Assert.True(context.BindingMetadata.IsRequired); + Assert.True(context.ValidationMetadata.IsRequired); } [Theory] [InlineData(true)] [InlineData(false)] [InlineData(null)] - public void GetBindingDetails_NoRequiredAttribute_IsRequiredLeftAlone(bool? initialValue) + public void GetValidationMetadata_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 ValidationMetadataProviderContext(key, attributes); + context.ValidationMetadata.IsRequired = initialValue; + + // Act + provider.GetValidationMetadata(context); + + // Assert + Assert.Equal(initialValue, context.ValidationMetadata.IsRequired); + } + + // [Required] has no effect on IsBindingRequired + [Theory] + [InlineData(true)] + [InlineData(false)] + public void GetBindingMetadata_RequiredAttribute_IsBindingRequiredLeftAlone(bool initialValue) + { + // Arrange + var provider = new DataAnnotationsMetadataProvider(); + + var attributes = new Attribute[] { new RequiredAttribute() }; + var key = ModelMetadataIdentity.ForProperty(typeof(int), "Length", typeof(string)); var context = new BindingMetadataProviderContext(key, attributes); - context.BindingMetadata.IsRequired = initialValue; + context.BindingMetadata.IsBindingRequired = initialValue; // Act provider.GetBindingMetadata(context); // Assert - Assert.Equal(initialValue, context.BindingMetadata.IsRequired); + Assert.Equal(initialValue, context.BindingMetadata.IsBindingRequired); } [Theory] diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataMemberRequiredBindingMetadataProviderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataMemberRequiredBindingMetadataProviderTest.cs index 9b26516bc5..15cdc1683e 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataMemberRequiredBindingMetadataProviderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DataMemberRequiredBindingMetadataProviderTest.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata public class DataMemberRequiredBindingMetadataProviderTest { [Fact] - public void IsRequired_SetToTrue_WithDataMemberIsRequiredTrue() + public void IsBindingRequired_SetToTrue_WithDataMemberIsRequiredTrue() { // Arrange var provider = new DataMemberRequiredBindingMetadataProvider(); @@ -29,14 +29,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata provider.GetBindingMetadata(context); // Assert - Assert.True(context.BindingMetadata.IsRequired); + Assert.True(context.BindingMetadata.IsBindingRequired); } [Theory] [InlineData(true)] [InlineData(false)] - [InlineData(null)] - public void IsRequired_LeftAlone_DataMemberIsRequiredFalse(bool? initialValue) + public void IsBindingRequired_LeftAlone_DataMemberIsRequiredFalse(bool initialValue) { // Arrange var provider = new DataMemberRequiredBindingMetadataProvider(); @@ -52,20 +51,19 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata typeof(ClassWithDataMemberIsRequiredFalse)); var context = new BindingMetadataProviderContext(key, attributes); - context.BindingMetadata.IsRequired = initialValue; + context.BindingMetadata.IsBindingRequired = initialValue; // Act provider.GetBindingMetadata(context); // Assert - Assert.Equal(initialValue, context.BindingMetadata.IsRequired); + Assert.Equal(initialValue, context.BindingMetadata.IsBindingRequired); } [Theory] [InlineData(true)] [InlineData(false)] - [InlineData(null)] - public void IsRequired_LeftAlone_ForNonPropertyMetadata(bool? initialValue) + public void IsBindingRequired_LeftAlone_ForNonPropertyMetadata(bool initialValue) { // Arrange var provider = new DataMemberRequiredBindingMetadataProvider(); @@ -78,20 +76,19 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata var key = ModelMetadataIdentity.ForType(typeof(ClassWithDataMemberIsRequiredTrue)); var context = new BindingMetadataProviderContext(key, attributes); - context.BindingMetadata.IsRequired = initialValue; + context.BindingMetadata.IsBindingRequired = initialValue; // Act provider.GetBindingMetadata(context); // Assert - Assert.Equal(initialValue, context.BindingMetadata.IsRequired); + Assert.Equal(initialValue, context.BindingMetadata.IsBindingRequired); } [Theory] [InlineData(true)] [InlineData(false)] - [InlineData(null)] - public void IsRequired_LeftAlone_WithoutDataMemberAttribute(bool? initialValue) + public void IsBindingRequired_LeftAlone_WithoutDataMemberAttribute(bool initialValue) { // Arrange var provider = new DataMemberRequiredBindingMetadataProvider(); @@ -102,20 +99,19 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata typeof(ClassWithoutAttributes)); var context = new BindingMetadataProviderContext(key, attributes: new object[0]); - context.BindingMetadata.IsRequired = initialValue; + context.BindingMetadata.IsBindingRequired = initialValue; // Act provider.GetBindingMetadata(context); // Assert - Assert.Equal(initialValue, context.BindingMetadata.IsRequired); + Assert.Equal(initialValue, context.BindingMetadata.IsBindingRequired); } [Theory] [InlineData(true)] [InlineData(false)] - [InlineData(null)] - public void IsRequired_LeftAlone_WithoutDataContractAttribute(bool? initialValue) + public void IsBindingRequired_LeftAlone_WithoutDataContractAttribute(bool initialValue) { // Arrange var provider = new DataMemberRequiredBindingMetadataProvider(); @@ -131,13 +127,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata typeof(ClassWithDataMemberIsRequiredTrueWithoutDataContract)); var context = new BindingMetadataProviderContext(key, attributes); - context.BindingMetadata.IsRequired = initialValue; + context.BindingMetadata.IsBindingRequired = initialValue; // Act provider.GetBindingMetadata(context); // Assert - Assert.Equal(initialValue, context.BindingMetadata.IsRequired); + Assert.Equal(initialValue, context.BindingMetadata.IsBindingRequired); } [DataContract] diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DefaultModelMetadataTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DefaultModelMetadataTest.cs index 7fbe56a6ba..79e43c72e7 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DefaultModelMetadataTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/DefaultModelMetadataTest.cs @@ -36,6 +36,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata Assert.False(metadata.HasNonDefaultEditFormat); Assert.False(metadata.HideSurroundingHtml); Assert.True(metadata.HtmlEncode); + Assert.False(metadata.IsBindingRequired); Assert.False(metadata.IsComplexType); Assert.False(metadata.IsCollectionType); Assert.False(metadata.IsEnum); diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataProviderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataProviderTest.cs index 84f3b60158..2c10466e94 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataProviderTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataProviderTest.cs @@ -637,7 +637,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata } [Fact] - public void IsRequired_DataMemberIsRequiredTrue_SetsIsRequiredToTrue() + public void IsBindingRequired_DataMemberIsRequiredTrue_SetsIsBindingRequiredToTrue() { // Arrange var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); @@ -648,11 +648,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata nameof(ClassWithDataMemberIsRequiredTrue.StringProperty)); // Assert - Assert.True(metadata.IsRequired); + Assert.True(metadata.IsBindingRequired); } [Fact] - public void IsRequired_DataMemberIsRequiredFalse_FalseForReferenceType() + public void IsBindingRequired_DataMemberIsRequiredFalse_False() { // Arrange var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); @@ -663,26 +663,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata nameof(ClassWithDataMemberIsRequiredFalse.StringProperty)); // Assert - Assert.False(metadata.IsRequired); + Assert.False(metadata.IsBindingRequired); } [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() + public void IsBindingRequiredDataMemberIsRequiredTrueWithoutDataContract_False() { // Arrange var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); @@ -693,7 +678,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata nameof(ClassWithDataMemberIsRequiredTrueWithoutDataContract.StringProperty)); // Assert - Assert.False(metadata.IsRequired); + Assert.False(metadata.IsBindingRequired); } [Fact] diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataTest.cs index 2ec98eea4b..26d6f281dd 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataTest.cs +++ b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Metadata/ModelMetadataTest.cs @@ -309,6 +309,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } } + public override bool IsBindingRequired + { + get + { + throw new NotImplementedException(); + } + } + public override bool IsEnum { get diff --git a/test/WebSites/ModelBindingWebSite/Controllers/BindingBehaviorController.cs b/test/WebSites/ModelBindingWebSite/Controllers/BindingBehaviorController.cs new file mode 100644 index 0000000000..fd2e0ff56b --- /dev/null +++ b/test/WebSites/ModelBindingWebSite/Controllers/BindingBehaviorController.cs @@ -0,0 +1,23 @@ +// 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 Microsoft.AspNet.Mvc; + +namespace ModelBindingWebSite +{ + [Route("BindingBehavior")] + public class BindingBehaviorController : Controller + { + // If the results are valid you get back the model, otherwise you get the ModelState errors. + [HttpPost("EchoModelValues")] + public object EchoModelValues(BindingBehaviorModel model) + { + if (!ModelState.IsValid) + { + return HttpBadRequest(ModelState); + } + + return model; + } + } +} diff --git a/test/WebSites/ModelBindingWebSite/Controllers/DataMemberRequiredController.cs b/test/WebSites/ModelBindingWebSite/Controllers/DataMemberRequiredController.cs new file mode 100644 index 0000000000..ab05f86d48 --- /dev/null +++ b/test/WebSites/ModelBindingWebSite/Controllers/DataMemberRequiredController.cs @@ -0,0 +1,23 @@ +// 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 Microsoft.AspNet.Mvc; + +namespace ModelBindingWebSite +{ + [Route("DataMemberRequired")] + public class DataMemberRequiredController : Controller + { + // If the results are valid you get back the model, otherwise you get the ModelState errors. + [HttpPost("EchoModelValues")] + public object EchoModelValues(DataMemberRequiredModel model) + { + if (!ModelState.IsValid) + { + return HttpBadRequest(ModelState); + } + + return model; + } + } +} diff --git a/test/WebSites/ModelBindingWebSite/Models/BindingBehaviorModel.cs b/test/WebSites/ModelBindingWebSite/Models/BindingBehaviorModel.cs new file mode 100644 index 0000000000..1a88cb9c55 --- /dev/null +++ b/test/WebSites/ModelBindingWebSite/Models/BindingBehaviorModel.cs @@ -0,0 +1,25 @@ +// 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 Microsoft.AspNet.Mvc.ModelBinding; + +namespace ModelBindingWebSite +{ + public class BindingBehaviorModel + { + [BindingBehavior(BindingBehavior.Never)] + public string BehaviourNeverProperty { get; set; } + + [BindingBehavior(BindingBehavior.Optional)] + public string BehaviourOptionalProperty { get; set; } + + [BindingBehavior(BindingBehavior.Required)] + public string BehaviourRequiredProperty { get; set; } + + [BindRequired] + public string BindRequiredProperty { get; set; } + + [BindNever] + public string BindNeverProperty { get; set; } + } +} diff --git a/test/WebSites/ModelBindingWebSite/Models/DataMemberRequiredModel.cs b/test/WebSites/ModelBindingWebSite/Models/DataMemberRequiredModel.cs new file mode 100644 index 0000000000..8f0d347cff --- /dev/null +++ b/test/WebSites/ModelBindingWebSite/Models/DataMemberRequiredModel.cs @@ -0,0 +1,20 @@ +// 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; + +namespace ModelBindingWebSite +{ + [DataContract] + public class DataMemberRequiredModel + { + [DataMember] + public string ImplicitlyOptionalProperty { get; set; } + + [DataMember(IsRequired = false)] + public string ExplicitlyOptionalProperty { get; set; } + + [DataMember(IsRequired = true)] + public string RequiredProperty { get; set; } + } +}