diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Metadata/BindingMetadata.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Metadata/BindingMetadata.cs index 6183b883a7..e7540642f2 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Metadata/BindingMetadata.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Metadata/BindingMetadata.cs @@ -38,11 +38,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata /// /// 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 does not represent a property. - /// See . If null, the value of - /// will be computed based on - /// . + /// See . /// - public bool? IsBindingRequired { get; set; } + public bool IsBindingRequired { get; set; } /// /// Gets or sets a value indicating whether or not the model is read-only. Will be ignored diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs index 4a83480299..28d3e1a695 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Metadata/DefaultModelMetadata.cs @@ -341,14 +341,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata { _isBindingRequired = false; } - else if (BindingMetadata.IsBindingRequired.HasValue) - { - _isBindingRequired = BindingMetadata.IsBindingRequired; - } else { - // Default to IsBindingRequired = true for value types. - _isBindingRequired = !AllowsNullValue(ModelType); + _isBindingRequired = BindingMetadata.IsBindingRequired; } } diff --git a/src/Microsoft.AspNet.Mvc.Formatters.Xml/DataMemberRequiredBindingMetadataProvider.cs b/src/Microsoft.AspNet.Mvc.Formatters.Xml/DataMemberRequiredBindingMetadataProvider.cs index e4eb241ffd..a9b6884c61 100644 --- a/src/Microsoft.AspNet.Mvc.Formatters.Xml/DataMemberRequiredBindingMetadataProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Formatters.Xml/DataMemberRequiredBindingMetadataProvider.cs @@ -22,7 +22,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata return; } - if (context.BindingMetadata.IsBindingRequired == true) + if (context.BindingMetadata.IsBindingRequired) { // This value is already required, no need to look at attributes. return; diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataTest.cs index f55875ebbc..5939d875fb 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Metadata/DefaultModelMetadataTest.cs @@ -39,7 +39,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata Assert.False(metadata.HideSurroundingHtml); Assert.True(metadata.HtmlEncode); Assert.True(metadata.IsBindingAllowed); - Assert.False(metadata.IsBindingRequired); // Defaults to false for reference types + Assert.False(metadata.IsBindingRequired); Assert.False(metadata.IsComplexType); Assert.False(metadata.IsCollectionType); Assert.False(metadata.IsEnum); @@ -223,49 +223,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Metadata Assert.False(isBindingRequired); } - [Theory] - [InlineData(typeof(string))] - [InlineData(typeof(IDisposable))] - [InlineData(typeof(Nullable))] - public void IsBindingRequired_ReturnsFalse_ForNullablePropertyTypes(Type modelType) - { - // Arrange - var provider = new EmptyModelMetadataProvider(); - var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); - - var key = ModelMetadataIdentity.ForProperty(modelType, "Test", typeof(string)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); - - var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); - - // Act - var isBindingRequired = metadata.IsBindingRequired; - - // Assert - Assert.False(isBindingRequired); - } - - [Theory] - [InlineData(typeof(int))] - [InlineData(typeof(DayOfWeek))] - public void IsBindingRequired_ReturnsTrue_ForNonNullablePropertyTypes(Type modelType) - { - // Arrange - var provider = new EmptyModelMetadataProvider(); - var detailsProvider = new EmptyCompositeMetadataDetailsProvider(); - - var key = ModelMetadataIdentity.ForProperty(modelType, "Test", typeof(string)); - var cache = new DefaultMetadataDetails(key, new ModelAttributes(new object[0])); - - var metadata = new DefaultModelMetadata(provider, detailsProvider, cache); - - // Act - var isBindingRequired = metadata.IsBindingRequired; - - // Assert - Assert.True(isBindingRequired); - } - [Theory] [InlineData(typeof(string))] [InlineData(typeof(IDisposable))] diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs index 5fbc79b1f9..a7f82e0424 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs @@ -1050,7 +1050,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } [Fact] - public void ProcessDto_ValueTypeProperty_NoValue_Error() + public void ProcessDto_ValueTypeProperty_NoValue_NoError() { // Arrange var model = new Person(); @@ -1083,14 +1083,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding testableBinder.ProcessDto(bindingContext, dto, modelValidationNode); // Assert - Assert.False(modelState.IsValid); - - var entry = modelState["theModel." + nameof(Person.ValueTypeRequiredWithDefaultValue)]; - var error = Assert.Single(entry.Errors); - Assert.Equal( - $"A value for the '{nameof(Person.ValueTypeRequiredWithDefaultValue)}' property was not provided.", - error.ErrorMessage); - Assert.Null(error.Exception); + Assert.True(modelState.IsValid); } [Fact] diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/HtmlGenerationTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/HtmlGenerationTest.cs index 2536f0c926..a5d4990fee 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/HtmlGenerationTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/HtmlGenerationTest.cs @@ -196,12 +196,11 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var request = new HttpRequestMessage(HttpMethod.Post, "http://localhost/Customer/HtmlGeneration_Customer"); var nameValueCollection = new List> { - new KeyValuePair("Number", "0"), + new KeyValuePair("Number", string.Empty), new KeyValuePair("Name", string.Empty), new KeyValuePair("Email", string.Empty), new KeyValuePair("PhoneNumber", string.Empty), - new KeyValuePair("Password", string.Empty), - new KeyValuePair("Gender", "Female"), + new KeyValuePair("Password", string.Empty) }; request.Content = new FormUrlEncodedContent(nameValueCollection); diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingFromQueryTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingFromQueryTest.cs index f512f3240e..dd1496b377 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingFromQueryTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingFromQueryTest.cs @@ -122,12 +122,8 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests // Assert var body = await response.Content.ReadAsStringAsync(); var result = JsonConvert.DeserializeObject(body); - - Assert.Collection( - result.ModelStateErrors, - e => Assert.Equal("TestEmployees[0].EmployeeId", e), - e => Assert.Equal("TestEmployees[0].EmployeeTaxId", e), - e => Assert.Equal("TestEmployees[0].Age", e)); + var error = Assert.Single(result.ModelStateErrors); + Assert.Equal("TestEmployees[0].EmployeeId", error); } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs index af7257a2d5..696f366d9d 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs @@ -2218,7 +2218,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests // Arrange var server = TestHelper.CreateServer(_app, SiteName, _configureServices); var client = server.CreateClient(); - var url = "http://localhost/TryUpdateModel/TryUpdateModel_ClearsModelStateEntries?id=5&price=1"; + var url = "http://localhost/TryUpdateModel/TryUpdateModel_ClearsModelStateEntries"; // Act var response = await client.GetAsync(url); diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Customer.Index.html b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Customer.Index.html index a32dfd7f63..7d6cf27c87 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Customer.Index.html +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Customer.Index.html @@ -3,8 +3,8 @@
- - The field Number must be between 1 and 100. + + The value '' is invalid.
@@ -27,10 +27,10 @@
Male - Female + Female
-
  • The field Number must be between 1 and 100.
    • +
    • The value '' is invalid.
    • The Password field is required.
    • diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs index db88f551b7..31838694c8 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs @@ -108,7 +108,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests private class Person4 { [FromBody] - [BindingBehavior(BindingBehavior.Optional)] + [Required] public int Address { get; set; } } diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs index 8301dcd1d4..1037f03c09 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs @@ -552,7 +552,6 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests private class Address4 { - [BindingBehavior(BindingBehavior.Optional)] public int Zip { get; set; } public string Street { get; set; } @@ -611,7 +610,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests private class Address5 { - public int? Zip { get; set; } + public int Zip { get; set; } [StringLength(3)] public string Street { get; set; } diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs index f107438e27..d3f709ec4c 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs @@ -26,7 +26,6 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests private class Order1 { - [BindingBehavior(BindingBehavior.Optional)] public int ProductId { get; set; } public Person1 Customer { get; set; } @@ -262,7 +261,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests private class Order2 { - public int? ProductId { get; set; } + public int ProductId { get; set; } public Person2 Customer { get; set; } } @@ -436,7 +435,6 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests private class Order3 { - [BindingBehavior(BindingBehavior.Optional)] public int ProductId { get; set; } public Person3 Customer { get; set; } @@ -580,7 +578,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests private class Order4 { - public int? ProductId { get; set; } + public int ProductId { get; set; } public Person4 Customer { get; set; } } @@ -1343,7 +1341,6 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests { public string Name { get; set; } - [BindingBehavior(BindingBehavior.Optional)] public KeyValuePair ProductId { get; set; } } @@ -2033,6 +2030,93 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.Equal("123", entry.Value.AttemptedValue); } + private class Order14 + { + public int ProductId { get; set; } + } + + // This covers the case where a key is present, but has an empty value. The type converter + // will report an error. + [Fact] + public async Task MutableObjectModelBinder_BindsPOCO_TypeConvertedPropertyNonConvertableValue_GetsError() + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(Order14) + }; + + // Need to have a key here so that the MutableObjectModelBinder will recurse to bind elements. + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.QueryString = new QueryString("?parameter.ProductId="); + }); + + var modelState = new ModelStateDictionary(); + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext); + + // Assert + Assert.NotNull(modelBindingResult); + Assert.True(modelBindingResult.IsModelSet); + + var model = Assert.IsType(modelBindingResult.Model); + Assert.NotNull(model); + Assert.Equal(0, model.ProductId); + + Assert.Equal(1, modelState.Count); + Assert.Equal(1, modelState.ErrorCount); + Assert.False(modelState.IsValid); + + var entry = Assert.Single(modelState, e => e.Key == "parameter.ProductId").Value; + Assert.Equal(string.Empty, entry.Value.AttemptedValue); + Assert.Equal(string.Empty, entry.Value.RawValue); + + var error = Assert.Single(entry.Errors); + Assert.Equal("The value '' is invalid.", error.ErrorMessage); + Assert.Null(error.Exception); + } + + // This covers the case where a key is present, but has no value. The type converter + // will report an error. + [Fact] + public async Task MutableObjectModelBinder_BindsPOCO_TypeConvertedPropertyWithNoValue_NoError() + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(Order14) + }; + + // Need to have a key here so that the MutableObjectModelBinder will recurse to bind elements. + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.QueryString = new QueryString("?parameter.ProductId"); + }); + + var modelState = new ModelStateDictionary(); + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext); + + // Assert + Assert.NotNull(modelBindingResult); + Assert.True(modelBindingResult.IsModelSet); + + var model = Assert.IsType(modelBindingResult.Model); + Assert.NotNull(model); + Assert.Equal(0, model.ProductId); + + Assert.Equal(0, modelState.Count); + Assert.Equal(0, modelState.ErrorCount); + Assert.True(modelState.IsValid); + } + private static void SetJsonBodyContent(HttpRequest request, string content) { var stream = new MemoryStream(new UTF8Encoding(encoderShouldEmitUTF8Identifier: false).GetBytes(content)); diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/TypeConverterModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/TypeConverterModelBinderIntegrationTest.cs index 8170475234..3b87dbbd12 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/TypeConverterModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/TypeConverterModelBinderIntegrationTest.cs @@ -159,6 +159,58 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState); } + + [Fact] + public async Task BindParameter_NonConvertableValue_GetsError() + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor() + { + Name = "Parameter1", + BindingInfo = new BindingInfo(), + + ParameterType = typeof(int) + }; + + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.QueryString = QueryString.Create("Parameter1", "abcd"); + }); + + var modelState = new ModelStateDictionary(); + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext); + + // Assert + + // ModelBindingResult + Assert.NotNull(modelBindingResult); + Assert.False(modelBindingResult.IsModelSet); + + // Model + Assert.Null(modelBindingResult.Model); + + // ModelState + Assert.False(modelState.IsValid); + Assert.Equal(1, modelState.Count); + Assert.Equal(1, modelState.ErrorCount); + + var key = Assert.Single(modelState.Keys); + Assert.Equal("Parameter1", key); + + var entry = modelState[key]; + Assert.NotNull(entry.Value); + Assert.Equal("abcd", entry.Value.AttemptedValue); + Assert.Equal("abcd", entry.Value.RawValue); + Assert.Equal(ModelValidationState.Invalid, entry.ValidationState); + + var error = Assert.Single(entry.Errors); + Assert.Null(error.Exception); + Assert.Equal("The value 'abcd' is not valid for Parameter1.", error.ErrorMessage); + } + [Theory] [InlineData(typeof(int))] [InlineData(typeof(bool))] diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/ValidationIntegrationTests.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/ValidationIntegrationTests.cs index ebde9dae1a..013aeb8475 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/ValidationIntegrationTests.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/ValidationIntegrationTests.cs @@ -195,7 +195,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests private class Person3 { - public int? Age { get; set; } + public int Age { get; set; } [Required] public string Name { get; set; } @@ -1018,12 +1018,12 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests private class Address { - public int? Street { get; set; } + public int Street { get; set; } public string State { get; set; } [Range(10000, 99999)] - public int? Zip { get; set; } + public int Zip { get; set; } public Country Country { get; set; } }