Address #2526; remove use of a required validator when validating `[BindRequired]`

- only use MVC error message when `[BindRequired]` is violated
- update that error message to more clearly describe the problem
- enable all tests skipped due to dupe bug #2493
- update expectations of a few tests using the old messages

nits:
- rename `ModelBinding_MissingRequiredMember` to `ModelBinding_MissingBindRequiredMember`
- remove `<param>` description of removed `requiredValidator` parameter
- remove unused `MutableObjectModelBinderTest.GetRequiredValidator()`

src/Microsoft.AspNet.Mvc.Core/ModelBinding/MutableObjectModelBinder.cs

@@ -3,7 +3,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.ComponentModel;
 using System.Linq;
 using System.Reflection;
 using System.Threading.Tasks;
@@ -364,42 +363,19 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             var modelExplorer = metadataProvider.GetModelExplorerForType(bindingContext.ModelType, bindingContext.Model);
             var validationInfo = GetPropertyValidationInfo(bindingContext);
 
-            // Eliminate provided properties from requiredProperties; leaving just *missing* required properties.
+            // Eliminate provided properties from RequiredProperties; leaving just *missing* required properties.
             var boundProperties = dto.Results.Where(p => p.Value.IsModelSet).Select(p => p.Key.PropertyName);
             validationInfo.RequiredProperties.ExceptWith(boundProperties);
 
             foreach (var missingRequiredProperty in validationInfo.RequiredProperties)
             {
-                var addedError = false;
+                var propertyExplorer = modelExplorer.GetExplorerForProperty(missingRequiredProperty);
-
-                // We want to provide the 'null' value, not the value of model.Property,
-                // so avoiding modelExplorer.GetProperty here which would call the actual getter on the
-                // model. This avoids issues with value types, or properties with pre-initialized values.
-                var propertyExplorer = modelExplorer.GetExplorerForProperty(missingRequiredProperty, model: null);
-
                 var propertyName = propertyExplorer.Metadata.BinderModelName ?? missingRequiredProperty;
-                var modelStateKey = ModelNames.CreatePropertyModelName(
+                var modelStateKey = ModelNames.CreatePropertyModelName(bindingContext.ModelName, propertyName);
-                    bindingContext.ModelName,
-                    propertyName);
 
-                // Get the first 'required' validator (if any) to get custom error message.
+                bindingContext.ModelState.TryAddModelError(
-                var validatorProviderContext = new ModelValidatorProviderContext(propertyExplorer.Metadata);
+                    modelStateKey,
-                bindingContext.OperationBindingContext.ValidatorProvider.GetValidators(validatorProviderContext);
+                    Resources.FormatModelBinding_MissingBindRequiredMember(propertyName));
-                var validator = validatorProviderContext.Validators.FirstOrDefault(v => v.IsRequired);
-
-                if (validator != null)
-                {
-                    addedError = RunValidator(validator, bindingContext, propertyExplorer, modelStateKey);
-                }
-
-                // Fall back to default message if BindingBehaviorAttribute required this property and we have no
-                // actual validator for it.
-                if (!addedError)
-                {
-                    bindingContext.ModelState.TryAddModelError(
-                        modelStateKey,
-                        Resources.FormatModelBinding_MissingRequiredMember(propertyName));
-                }
             }
 
             // For each property that ComplexModelDtoModelBinder attempted to bind, call the setter, recording
@@ -436,10 +412,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
         /// </param>
         /// <param name="propertyMetadata">The <see cref="ModelMetadata"/> for the property to set.</param>
         /// <param name="dtoResult">The <see cref="ModelBindingResult"/> for the property's new value.</param>
-        /// <param name="requiredValidator">
-        /// The <see cref="IModelValidator"/> which ensures the value is not <c>null</c>. Or <c>null</c> if no such
-        /// requirement exists.
-        /// </param>
         /// <remarks>Should succeed in all cases that <see cref="CanUpdateProperty"/> returns <c>true</c>.</remarks>
         protected virtual void SetProperty(
             [NotNull] ModelBindingContext bindingContext,

src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs

@@ -1995,19 +1995,19 @@ namespace Microsoft.AspNet.Mvc.Core
         }
 
         /// <summary>
-        /// The '{0}' property is required.
+        /// A value for the '{0}' property was not provided.
         /// </summary>
-        internal static string ModelBinding_MissingRequiredMember
+        internal static string ModelBinding_MissingBindRequiredMember
         {
-            get { return GetString("ModelBinding_MissingRequiredMember"); }
+            get { return GetString("ModelBinding_MissingBindRequiredMember"); }
         }
 
         /// <summary>
-        /// The '{0}' property is required.
+        /// A value for the '{0}' property was not provided.
         /// </summary>
-        internal static string FormatModelBinding_MissingRequiredMember(object p0)
+        internal static string FormatModelBinding_MissingBindRequiredMember(object p0)
         {
-            return string.Format(CultureInfo.CurrentCulture, GetString("ModelBinding_MissingRequiredMember"), p0);
+            return string.Format(CultureInfo.CurrentCulture, GetString("ModelBinding_MissingBindRequiredMember"), p0);
         }
 
         /// <summary>

src/Microsoft.AspNet.Mvc.Core/Resources.resx

@@ -1,17 +1,17 @@
 <?xml version="1.0" encoding="utf-8"?>
 <root>
-  <!-- 
+  <!--
-    Microsoft ResX Schema 
+    Microsoft ResX Schema
-    
+
     Version 2.0
-    
+
-    The primary goals of this format is to allow a simple XML format 
+    The primary goals of this format is to allow a simple XML format
-    that is mostly human readable. The generation and parsing of the 
+    that is mostly human readable. The generation and parsing of the
-    various data types are done through the TypeConverter classes 
+    various data types are done through the TypeConverter classes
     associated with the data types.
-    
+
     Example:
-    
+
     ... ado.net/XML headers & schema ...
     <resheader name="resmimetype">text/microsoft-resx</resheader>
     <resheader name="version">2.0</resheader>
@@ -26,36 +26,36 @@
         <value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
         <comment>This is a comment</comment>
     </data>
-                
+
-    There are any number of "resheader" rows that contain simple 
+    There are any number of "resheader" rows that contain simple
     name/value pairs.
-    
+
-    Each data row contains a name, and value. The row also contains a 
+    Each data row contains a name, and value. The row also contains a
-    type or mimetype. Type corresponds to a .NET class that support 
+    type or mimetype. Type corresponds to a .NET class that support
-    text/value conversion through the TypeConverter architecture. 
+    text/value conversion through the TypeConverter architecture.
-    Classes that don't support this are serialized and stored with the 
+    Classes that don't support this are serialized and stored with the
     mimetype set.
-    
+
-    The mimetype is used for serialized objects, and tells the 
+    The mimetype is used for serialized objects, and tells the
-    ResXResourceReader how to depersist the object. This is currently not 
+    ResXResourceReader how to depersist the object. This is currently not
     extensible. For a given mimetype the value must be set accordingly:
-    
+
-    Note - application/x-microsoft.net.object.binary.base64 is the format 
+    Note - application/x-microsoft.net.object.binary.base64 is the format
-    that the ResXResourceWriter will generate, however the reader can 
+    that the ResXResourceWriter will generate, however the reader can
     read any of the formats listed below.
-    
+
     mimetype: application/x-microsoft.net.object.binary.base64
-    value   : The object must be serialized with 
+    value   : The object must be serialized with
             : System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
             : and then encoded with base64 encoding.
-    
+
     mimetype: application/x-microsoft.net.object.soap.base64
-    value   : The object must be serialized with 
+    value   : The object must be serialized with
             : System.Runtime.Serialization.Formatters.Soap.SoapFormatter
             : and then encoded with base64 encoding.
 
     mimetype: application/x-microsoft.net.object.bytearray.base64
-    value   : The object must be serialized into a byte array 
+    value   : The object must be serialized into a byte array
             : using a System.ComponentModel.TypeConverter
             : and then encoded with base64 encoding.
     -->
@@ -499,8 +499,8 @@
   <data name="ModelBinderUtil_ValueInvalidGeneric" xml:space="preserve">
     <value>The supplied value is invalid for {0}.</value>
   </data>
-  <data name="ModelBinding_MissingRequiredMember" xml:space="preserve">
+  <data name="ModelBinding_MissingBindRequiredMember" xml:space="preserve">
-    <value>The '{0}' property is required.</value>
+    <value>A value for the '{0}' property was not provided.</value>
   </data>
   <data name="ModelBinding_ValueRequired" xml:space="preserve">
     <value>A value is required.</value>

test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/MutableObjectModelBinderTest.cs

@@ -793,7 +793,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             var modelError = Assert.Single(modelState.Errors);
             Assert.Null(modelError.Exception);
             Assert.NotNull(modelError.ErrorMessage);
-            Assert.Equal("The 'Age' property is required.", modelError.ErrorMessage);
+            Assert.Equal("A value for the 'Age' property was not provided.", modelError.ErrorMessage);
         }
 
         [Fact]
@@ -844,7 +844,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             var modelError = Assert.Single(modelState.Errors);
             Assert.Null(modelError.Exception);
             Assert.NotNull(modelError.ErrorMessage);
-            Assert.Equal("The 'Age' property is required.", modelError.ErrorMessage);
+            Assert.Equal("A value for the 'Age' property was not provided.", modelError.ErrorMessage);
         }
 
         [Fact]
@@ -1147,9 +1147,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             Assert.Equal(0m, model.PropertyWithDefaultValue);     // [DefaultValue] has no effect
         }
 
-        // This uses [Required] with [BindRequired] to provide a custom validation messsage.
+        // [Required] cannot provide a custom validation for [BindRequired] errors.
         [Fact]
-        public void ProcessDto_ValueTypePropertyWithBindRequired_CustomValidationMessage()
+        public void ProcessDto_ValueTypePropertyWithBindRequired_RequiredValidatorIgnored()
         {
             // Arrange
             var model = new ModelWithBindRequiredAndRequiredAttribute();
@@ -1191,16 +1191,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 .Value;
             var error = Assert.Single(entry.Errors);
             Assert.Null(error.Exception);
-            Assert.Equal("Custom Message ValueTypeProperty", error.ErrorMessage);
+            Assert.Equal("A value for the 'ValueTypeProperty' property was not provided.", error.ErrorMessage);
 
             // Model gets provided values.
             Assert.Equal(0, model.ValueTypeProperty);
             Assert.Equal("value", model.ReferenceTypeProperty);
         }
 
-        // This uses [Required] with [BindRequired] to provide a custom validation messsage.
+        // [Required] cannot provide a custom validation for [BindRequired] errors.
         [Fact]
-        public void ProcessDto_ReferenceTypePropertyWithBindRequired_CustomValidationMessage()
+        public void ProcessDto_ReferenceTypePropertyWithBindRequired_RequiredValidatorIgnored()
         {
             // Arrange
             var model = new ModelWithBindRequiredAndRequiredAttribute();
@@ -1242,7 +1242,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 .Value;
             var error = Assert.Single(entry.Errors);
             Assert.Null(error.Exception);
-            Assert.Equal("Custom Message ReferenceTypeProperty", error.ErrorMessage);
+            Assert.Equal("A value for the 'ReferenceTypeProperty' property was not provided.", error.ErrorMessage);
 
             // Model gets provided values.
             Assert.Equal(17, model.ValueTypeProperty);
@@ -1715,15 +1715,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             };
         }
 
-        private static IModelValidator GetRequiredValidator(ModelBindingContext bindingContext, ModelMetadata propertyMetadata)
-        {
-            var validatorProvider = bindingContext.OperationBindingContext.ValidatorProvider;
-            var validatorProviderContext = new ModelValidatorProviderContext(propertyMetadata);
-            validatorProvider.GetValidators(validatorProviderContext);
-
-            return validatorProviderContext.Validators.FirstOrDefault(v => v.IsRequired);
-        }
-
         private static ModelMetadata GetMetadataForCanUpdateProperty(string propertyName)
         {
             var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider();

test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingBindingBehaviorTest.cs

@@ -49,10 +49,14 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
             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<string>());
+            Assert.Equal(
+                "A value for the 'BehaviourRequiredProperty' property was not provided.",
+                ((JArray)error.Value)[0].Value<string>());
 
             error = Assert.Single(errors, kvp => kvp.Key == "model.BindRequiredProperty");
-            Assert.Equal("The 'BindRequiredProperty' property is required.", ((JArray)error.Value)[0].Value<string>());
+            Assert.Equal(
+                "A value for the 'BindRequiredProperty' property was not provided.",
+                ((JArray)error.Value)[0].Value<string>());
         }
 
         [Fact]

test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingDataMemberRequiredTest.cs

@@ -49,7 +49,9 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
             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<string>());
+            Assert.Equal(
+                "A value for the 'RequiredProperty' property was not provided.",
+                ((JArray)error.Value)[0].Value<string>());
         }
 
         [Fact]

test/Microsoft.AspNet.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs

@@ -1582,7 +1582,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
             public string Name { get; set; }
         }
 
-        [Fact(Skip = "Error message is incorrect #2493.")]
+        [Fact]
         public async Task MutableObjectModelBinder_WithRequiredComplexProperty_NoData_GetsErrors()
         {
             // Arrange
@@ -1617,7 +1617,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
             var entry = Assert.Single(modelState, e => e.Key == "Customer").Value;
             Assert.Null(entry.Value);
             var error = Assert.Single(modelState["Customer"].Errors);
-            Assert.Equal("The Customer field is required.", error.ErrorMessage);
+            Assert.Equal("A value for the 'Customer' property was not provided.", error.ErrorMessage);
         }
 
         private class Order11
@@ -1633,7 +1633,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
             public string Name { get; set; }
         }
 
-        [Fact(Skip = "Error message is incorrect #2493.")]
+        [Fact]
         public async Task MutableObjectModelBinder_WithNestedRequiredProperty_WithPartialData_GetsErrors()
         {
             // Arrange
@@ -1676,10 +1676,10 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
             entry = Assert.Single(modelState, e => e.Key == "parameter.Customer.Name").Value;
             Assert.Null(entry.Value);
             var error = Assert.Single(modelState["parameter.Customer.Name"].Errors);
-            Assert.Equal("The Name field is required.", error.ErrorMessage);
+            Assert.Equal("A value for the 'Name' property was not provided.", error.ErrorMessage);
         }
 
-        [Fact(Skip = "Error message is incorrect #2493.")]
+        [Fact]
         public async Task MutableObjectModelBinder_WithNestedRequiredProperty_WithData_EmptyPrefix_GetsErrors()
         {
             // Arrange
@@ -1722,10 +1722,10 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
             entry = Assert.Single(modelState, e => e.Key == "Customer.Name").Value;
             Assert.Null(entry.Value);
             var error = Assert.Single(modelState["Customer.Name"].Errors);
-            Assert.Equal("The Name field is required.", error.ErrorMessage);
+            Assert.Equal("A value for the 'Name' property was not provided.", error.ErrorMessage);
         }
 
-        [Fact(Skip = "Error message is incorrect #2493.")]
+        [Fact]
         public async Task MutableObjectModelBinder_WithNestedRequiredProperty_WithData_CustomPrefix_GetsErrors()
         {
             // Arrange
@@ -1772,7 +1772,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
             entry = Assert.Single(modelState, e => e.Key == "customParameter.Customer.Name").Value;
             Assert.Null(entry.Value);
             var error = Assert.Single(modelState["customParameter.Customer.Name"].Errors);
-            Assert.Equal("The Name field is required.", error.ErrorMessage);
+            Assert.Equal("A value for the 'Name' property was not provided.", error.ErrorMessage);
         }
 
         private class Order12
@@ -1781,7 +1781,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
             public string ProductName { get; set; }
         }
 
-        [Fact(Skip = "Error message is incorrect #2493.")]
+        [Fact]
         public async Task MutableObjectModelBinder_WithRequiredProperty_NoData_GetsErrors()
         {
             // Arrange
@@ -1817,10 +1817,10 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
             var entry = Assert.Single(modelState, e => e.Key == "ProductName").Value;
             Assert.Null(entry.Value);
             var error = Assert.Single(modelState["ProductName"].Errors);
-            Assert.Equal("The ProductName field is required.", error.ErrorMessage);
+            Assert.Equal("A value for the 'ProductName' property was not provided.", error.ErrorMessage);
         }
 
-        [Fact(Skip = "Error message is incorrect #2493.")]
+        [Fact]
         public async Task MutableObjectModelBinder_WithRequiredProperty_NoData_CustomPrefix_GetsErros()
         {
             // Arrange
@@ -1860,7 +1860,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
             var entry = Assert.Single(modelState, e => e.Key == "customParameter.ProductName").Value;
             Assert.Null(entry.Value);
             var error = Assert.Single(modelState["customParameter.ProductName"].Errors);
-            Assert.Equal("The ProductName field is required.", error.ErrorMessage);
+            Assert.Equal("A value for the 'ProductName' property was not provided.", error.ErrorMessage);
         }
 
         [Fact]
@@ -1908,7 +1908,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
             public List<int> OrderIds { get; set; }
         }
 
-        [Fact(Skip = "Error message is incorrect #2493.")]
+        [Fact]
         public async Task MutableObjectModelBinder_WithRequiredCollectionProperty_NoData_GetsErros()
         {
             // Arrange
@@ -1944,10 +1944,10 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
             var entry = Assert.Single(modelState, e => e.Key == "OrderIds").Value;
             Assert.Null(entry.Value);
             var error = Assert.Single(modelState["OrderIds"].Errors);
-            Assert.Equal("The OrderIds field is required.", error.ErrorMessage);
+            Assert.Equal("A value for the 'OrderIds' property was not provided.", error.ErrorMessage);
         }
 
-        [Fact(Skip = "Error message is incorrect #2493.")]
+        [Fact]
         public async Task MutableObjectModelBinder_WithRequiredCollectionProperty_NoData_CustomPrefix_GetsErros()
         {
             // Arrange
@@ -1987,7 +1987,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
             var entry = Assert.Single(modelState, e => e.Key == "customParameter.OrderIds").Value;
             Assert.Null(entry.Value);
             var error = Assert.Single(modelState["customParameter.OrderIds"].Errors);
-            Assert.Equal("The OrderIds field is required.", error.ErrorMessage);
+            Assert.Equal("A value for the 'OrderIds' property was not provided.", error.ErrorMessage);
         }
 
         [Fact]