Fix #2407 - Part 1 - Make model binding behavior for [Required] compatible

with MVC5.

This change removes the behavior in model binding to validate values 'on
the wire' for requiredness instead of the looking at the model. This
restores the behavior of [Required] for model binding to the MVC5
semantics.

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

@@ -349,17 +349,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 {
                     validationInfo.RequiredProperties.Add(propertyName);
                 }
-
-                var validatorProviderContext = new ModelValidatorProviderContext(propertyMetadata);
-                bindingContext.OperationBindingContext.ValidatorProvider.GetValidators(validatorProviderContext);
-
-                var requiredValidator = validatorProviderContext.Validators
-                    .FirstOrDefault(v => v != null && v.IsRequired);
-                if (requiredValidator != null)
-                {
-                    validationInfo.RequiredValidators[propertyName] = requiredValidator;
-                    validationInfo.RequiredProperties.Add(propertyName);
-                }
             }
 
             return validationInfo;
@@ -393,15 +382,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                     bindingContext.ModelName,
                     propertyName);
 
-                // Execute validator (if any) to get custom error message.
+                // Get the first 'required' validator (if any) to get custom error message.
-                IModelValidator validator;
+                var validatorProviderContext = new ModelValidatorProviderContext(propertyExplorer.Metadata);
-                if (validationInfo.RequiredValidators.TryGetValue(missingRequiredProperty, out validator))
+                bindingContext.OperationBindingContext.ValidatorProvider.GetValidators(validatorProviderContext);
+                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 or validator
+                // Fall back to default message if BindingBehaviorAttribute required this property and we have no
-                // (oddly) succeeded.
+                // actual validator for it.
                 if (!addedError)
                 {
                     bindingContext.ModelState.TryAddModelError(
@@ -418,12 +410,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 if (dtoResult != null)
                 {
                     var propertyMetadata = entry.Key;
-                    IModelValidator requiredValidator;
+                    SetProperty(bindingContext, modelExplorer, propertyMetadata, dtoResult);
-                    validationInfo.RequiredValidators.TryGetValue(
-                        propertyMetadata.PropertyName,
-                        out requiredValidator);
-
-                    SetProperty(bindingContext, modelExplorer, propertyMetadata, dtoResult, requiredValidator);
 
                     var dtoValidationNode = dtoResult.ValidationNode;
                     if (dtoValidationNode == null)
@@ -458,8 +445,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             [NotNull] ModelBindingContext bindingContext,
             [NotNull] ModelExplorer modelExplorer,
             [NotNull] ModelMetadata propertyMetadata,
-            [NotNull] ModelBindingResult dtoResult,
+            [NotNull] ModelBindingResult dtoResult)
-            IModelValidator requiredValidator)
         {
             var bindingFlags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.IgnoreCase;
             var property = bindingContext.ModelType.GetProperty(
@@ -485,26 +471,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 value = dtoResult.Model;
             }
 
-            // 'Required' validators need to run first so that we can provide useful error messages if
-            // the property setters throw, e.g. if we're setting entity keys to null.
-            if (value == null)
-            {
-                var modelStateKey = dtoResult.Key;
-                var validationState = bindingContext.ModelState.GetFieldValidationState(modelStateKey);
-                if (validationState == ModelValidationState.Unvalidated)
-                {
-                    if (requiredValidator != null)
-                    {
-                        var propertyExplorer = modelExplorer.GetExplorerForExpression(propertyMetadata, model: null);
-                        var validationContext = new ModelValidationContext(bindingContext, propertyExplorer);
-                        foreach (var validationResult in requiredValidator.Validate(validationContext))
-                        {
-                            bindingContext.ModelState.TryAddModelError(modelStateKey, validationResult.Message);
-                        }
-                    }
-                }
-            }
-
             if (!dtoResult.IsModelSet)
             {
                 // If we don't have a value, don't set it on the model and trounce a pre-initialized
@@ -512,31 +478,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 return;
             }
 
-            if (value != null || TypeHelper.AllowsNullValue(property.PropertyType))
+            try
             {
-                try
+                propertyMetadata.PropertySetter(bindingContext.Model, value);
-                {
-                    propertyMetadata.PropertySetter(bindingContext.Model, value);
-                }
-                catch (Exception exception)
-                {
-                    AddModelError(exception, bindingContext, dtoResult);
-                }
             }
-            else
+            catch (Exception exception)
             {
-                // trying to set a non-nullable value type to null, need to make sure there's a message
+                AddModelError(exception, bindingContext, dtoResult);
-                var modelStateKey = dtoResult.Key;
-                var validationState = bindingContext.ModelState.GetFieldValidationState(modelStateKey);
-                if (validationState == ModelValidationState.Unvalidated)
-                {
-                    var errorMessage = Resources.ModelBinding_ValueRequired;
-                    bindingContext.ModelState.TryAddModelError(modelStateKey, errorMessage);
-                }
             }
         }
 
-        // Neither [DefaultValue] nor [Required] is relevant for a non-settable collection.
         private void AddToProperty(
             ModelBindingContext bindingContext,
             ModelExplorer modelExplorer,
@@ -642,14 +593,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             public PropertyValidationInfo()
             {
                 RequiredProperties = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
-                RequiredValidators = new Dictionary<string, IModelValidator>(StringComparer.OrdinalIgnoreCase);
                 SkipProperties = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
             }
 
             public HashSet<string> RequiredProperties { get; private set; }
 
-            public Dictionary<string, IModelValidator> RequiredValidators { get; private set; }
-
             public HashSet<string> SkipProperties { get; private set; }
         }
     }

src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/DefaultObjectValidator.cs

@@ -153,7 +153,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
         {
             var validatorProviderContext = new ModelValidatorProviderContext(metadata);
             provider.GetValidators(validatorProviderContext);
-            return validatorProviderContext.Validators;
+
+            return validatorProviderContext
+                .Validators
+                .OrderBy(v => v, ValidatorOrderComparer.Instance)
+                .ToList();
         }
 
         private bool ValidateChildNodes(
@@ -345,5 +349,19 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
 
             public ModelValidationNode ValidationNode { get; set; }
         }
+
+        // Sorts validators based on whether or not they are 'required'. We want to run
+        // 'required' validators first so that we get the best possible error message.
+        private class ValidatorOrderComparer : IComparer<IModelValidator>
+        {
+            public static readonly ValidatorOrderComparer Instance = new ValidatorOrderComparer();
+
+            public int Compare(IModelValidator x, IModelValidator y)
+            {
+                var xScore = x.IsRequired ? 0 : 1;
+                var yScore = y.IsRequired ? 0 : 1;
+                return xScore.CompareTo(yScore);
+            }
+        }
     }
 }

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

@@ -849,7 +849,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
 
         [Fact]
         [ReplaceCulture]
-        public void ProcessDto_BindRequiredFieldNull_RaisesModelError()
+        public void ProcessDto_ValueTypePropertyWithBindRequired_SetToNull_CapturesException()
         {
             // Arrange
             var model = new ModelWithBindRequired
@@ -905,14 +905,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             Assert.Equal(ModelValidationState.Invalid, modelState.ValidationState);
 
             var modelError = Assert.Single(modelState.Errors);
-            Assert.Null(modelError.Exception);
+            Assert.Equal(string.Empty, modelError.ErrorMessage);
-            Assert.NotNull(modelError.ErrorMessage);
+            Assert.IsType<NullReferenceException>(modelError.Exception);
-            Assert.Equal("A value is required.", modelError.ErrorMessage);
         }
 
         [Fact]
         [ReplaceCulture]
-        public void ProcessDto_RequiredFieldMissing_RaisesModelError()
+        public void ProcessDto_MissingDataForRequiredFields_NoErrors()
         {
             // Arrange
             var model = new ModelWithRequired();
@@ -930,32 +929,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
 
             // Assert
             var modelStateDictionary = bindingContext.ModelState;
-            Assert.False(modelStateDictionary.IsValid);
+            Assert.True(modelStateDictionary.IsValid);
-            Assert.Equal(2, modelStateDictionary.Count);
+            Assert.Empty(modelStateDictionary);
-
-            // Check Age error.
-            ModelState modelState;
-            Assert.True(modelStateDictionary.TryGetValue("theModel." + nameof(ModelWithRequired.Age), out modelState));
-
-            var modelError = Assert.Single(modelState.Errors);
-            Assert.Null(modelError.Exception);
-            Assert.NotNull(modelError.ErrorMessage);
-            var expected = ValidationAttributeUtil.GetRequiredErrorMessage(nameof(ModelWithRequired.Age));
-            Assert.Equal(expected, modelError.ErrorMessage);
-
-            // Check City error.
-            Assert.True(modelStateDictionary.TryGetValue("theModel." + nameof(ModelWithRequired.City), out modelState));
-
-            modelError = Assert.Single(modelState.Errors);
-            Assert.Null(modelError.Exception);
-            Assert.NotNull(modelError.ErrorMessage);
-            expected = ValidationAttributeUtil.GetRequiredErrorMessage(nameof(ModelWithRequired.City));
-            Assert.Equal(expected, modelError.ErrorMessage);
         }
 
         [Fact]
         [ReplaceCulture]
-        public void ProcessDto_RequiredFieldNull_RaisesModelError()
+        public void ProcessDto_ValueTypeProperty_WithRequiredAttribute_SetToNull_NoError()
         {
             // Arrange
             var model = new ModelWithRequired();
@@ -984,22 +964,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
 
             // Assert
             var modelStateDictionary = bindingContext.ModelState;
-            Assert.False(modelStateDictionary.IsValid);
+            Assert.True(modelStateDictionary.IsValid);
-            Assert.Single(modelStateDictionary);
+            Assert.Empty(modelStateDictionary);
-
-            // Check City error.
-            ModelState modelState;
-            Assert.True(modelStateDictionary.TryGetValue("theModel." + nameof(ModelWithRequired.City), out modelState));
-
-            var modelError = Assert.Single(modelState.Errors);
-            Assert.Null(modelError.Exception);
-            Assert.NotNull(modelError.ErrorMessage);
-            var expected = ValidationAttributeUtil.GetRequiredErrorMessage(nameof(ModelWithRequired.City));
-            Assert.Equal(expected, modelError.ErrorMessage);
         }
 
         [Fact]
-        public void ProcessDto_RequiredFieldMissing_RaisesModelErrorWithMessage()
+        public void ProcessDto_PropertyWithRequiredAttribute_NoPropertiesSet_NoError()
         {
             // Arrange
             var model = new Person();
@@ -1016,42 +986,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
 
             // Assert
             var modelStateDictionary = bindingContext.ModelState;
-            Assert.False(modelStateDictionary.IsValid);
+            Assert.True(modelStateDictionary.IsValid);
-            Assert.Equal(2, modelStateDictionary.Count);
-
-            // Check ValueTypeRequired error.
-            var modelStateEntry = Assert.Single(
-                modelStateDictionary,
-                entry => entry.Key == "theModel." + nameof(Person.ValueTypeRequired));
-            Assert.Equal("theModel." + nameof(Person.ValueTypeRequired), modelStateEntry.Key);
-
-            var modelState = modelStateEntry.Value;
-            Assert.Equal(ModelValidationState.Invalid, modelState.ValidationState);
-
-            var modelError = Assert.Single(modelState.Errors);
-            Assert.Null(modelError.Exception);
-            Assert.NotNull(modelError.ErrorMessage);
-            Assert.Equal("Sample message", modelError.ErrorMessage);
-
-            // Check ValueTypeRequiredWithDefaultValue error.
-            modelStateEntry = Assert.Single(
-                modelStateDictionary,
-                entry => entry.Key == "theModel." + nameof(Person.ValueTypeRequiredWithDefaultValue));
-            Assert.Equal("theModel." + nameof(Person.ValueTypeRequiredWithDefaultValue), modelStateEntry.Key);
-
-            modelState = modelStateEntry.Value;
-            Assert.Equal(ModelValidationState.Invalid, modelState.ValidationState);
-
-            modelError = Assert.Single(modelState.Errors);
-            Assert.Null(modelError.Exception);
-            Assert.NotNull(modelError.ErrorMessage);
-            Assert.Equal("Another sample message", modelError.ErrorMessage);
         }
 
-        [Theory]
+        [Fact]
-        [InlineData(false)]
+        public void ProcessDto_ValueTypeProperty_TriesToSetNullModel_CapturesException()
-        [InlineData(true)]
-        public void ProcessDto_RequiredFieldNull_RaisesModelErrorWithMessage(bool isModelSet)
         {
             // Arrange
             var model = new Person();
@@ -1070,7 +1009,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             var propertyMetadata = dto.PropertyMetadata.Single(p => p.PropertyName == nameof(Person.ValueTypeRequired));
             dto.Results[propertyMetadata] = new ModelBindingResult(
                 null,
-                isModelSet: isModelSet,
+                isModelSet: true,
                 key: "theModel." + nameof(Person.ValueTypeRequired));
 
             // Make ValueTypeRequiredWithDefaultValue invalid
@@ -1078,8 +1017,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 .Single(p => p.PropertyName == nameof(Person.ValueTypeRequiredWithDefaultValue));
             dto.Results[propertyMetadata] = new ModelBindingResult(
                 model: null,
-                isModelSet: isModelSet,
+                isModelSet: true,
                 key: "theModel." + nameof(Person.ValueTypeRequiredWithDefaultValue));
+
             var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model);
 
             // Act
@@ -1097,10 +1037,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             var modelState = modelStateEntry.Value;
             Assert.Equal(ModelValidationState.Invalid, modelState.ValidationState);
 
-            var modelError = Assert.Single(modelState.Errors);
+            var error = Assert.Single(modelState.Errors);
-            Assert.Null(modelError.Exception);
+            Assert.Equal(string.Empty, error.ErrorMessage);
-            Assert.NotNull(modelError.ErrorMessage);
+            Assert.IsType<NullReferenceException>(error.Exception);
-            Assert.Equal("Sample message", modelError.ErrorMessage);
 
             // Check ValueTypeRequiredWithDefaultValue error.
             modelStateEntry = Assert.Single(
@@ -1111,15 +1050,55 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             modelState = modelStateEntry.Value;
             Assert.Equal(ModelValidationState.Invalid, modelState.ValidationState);
 
-            modelError = Assert.Single(modelState.Errors);
+            error = Assert.Single(modelState.Errors);
-            Assert.Null(modelError.Exception);
+            Assert.Equal(string.Empty, error.ErrorMessage);
-            Assert.NotNull(modelError.ErrorMessage);
+            Assert.IsType<NullReferenceException>(error.Exception);
-            Assert.Equal("Another sample message", modelError.ErrorMessage);
 
             Assert.Equal(0, model.ValueTypeRequired);
             Assert.Equal(expectedValue, model.ValueTypeRequiredWithDefaultValue);
         }
 
+        [Fact]
+        public void ProcessDto_ValueTypeProperty_NoValue_NoError()
+        {
+            // Arrange
+            var model = new Person();
+            var containerMetadata = GetMetadataForType(model.GetType());
+
+            var bindingContext = CreateContext(containerMetadata, model);
+            var modelStateDictionary = bindingContext.ModelState;
+
+            var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties);
+            var testableBinder = new TestableMutableObjectModelBinder();
+
+            // The [DefaultValue] on ValueTypeRequiredWithDefaultValue is ignored by model binding.
+            var expectedValue = 0;
+
+            // Make ValueTypeRequired invalid.
+            var propertyMetadata = dto.PropertyMetadata.Single(p => p.PropertyName == nameof(Person.ValueTypeRequired));
+            dto.Results[propertyMetadata] = new ModelBindingResult(
+                null,
+                isModelSet: false,
+                key: "theModel." + nameof(Person.ValueTypeRequired));
+
+            // Make ValueTypeRequiredWithDefaultValue invalid
+            propertyMetadata = dto.PropertyMetadata
+                .Single(p => p.PropertyName == nameof(Person.ValueTypeRequiredWithDefaultValue));
+            dto.Results[propertyMetadata] = new ModelBindingResult(
+                model: null,
+                isModelSet: false,
+                key: "theModel." + nameof(Person.ValueTypeRequiredWithDefaultValue));
+
+            var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model);
+
+            // Act
+            testableBinder.ProcessDto(bindingContext, dto, modelValidationNode);
+
+            // Assert
+            Assert.True(modelStateDictionary.IsValid);
+            Assert.Empty(modelStateDictionary);
+        }
+
         [Fact]
         public void ProcessDto_ProvideRequiredFields_Success()
         {
@@ -1171,6 +1150,109 @@ 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.
+        [Fact]
+        public void ProcessDto_ValueTypePropertyWithBindRequired_CustomValidationMessage()
+        {
+            // Arrange
+            var model = new ModelWithBindRequiredAndRequiredAttribute();
+            var containerMetadata = GetMetadataForType(model.GetType());
+
+            var bindingContext = CreateContext(containerMetadata, model);
+            var modelStateDictionary = bindingContext.ModelState;
+
+            var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties);
+            var testableBinder = new TestableMutableObjectModelBinder();
+
+            // Make ValueTypeProperty not have a value.
+            var propertyMetadata = containerMetadata
+                .Properties[nameof(ModelWithBindRequiredAndRequiredAttribute.ValueTypeProperty)];
+            dto.Results[propertyMetadata] = new ModelBindingResult(
+                null,
+                isModelSet: false,
+                key: "theModel." + nameof(ModelWithBindRequiredAndRequiredAttribute.ValueTypeProperty));
+
+            // Make ReferenceTypeProperty have a value.
+            propertyMetadata = containerMetadata
+                .Properties[nameof(ModelWithBindRequiredAndRequiredAttribute.ReferenceTypeProperty)];
+            dto.Results[propertyMetadata] = new ModelBindingResult(
+                model: "value",
+                isModelSet: true,
+                key: "theModel." + nameof(ModelWithBindRequiredAndRequiredAttribute.ReferenceTypeProperty));
+
+            var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model);
+
+            // Act
+            testableBinder.ProcessDto(bindingContext, dto, modelValidationNode);
+
+            // Assert
+            Assert.False(modelStateDictionary.IsValid);
+
+            var entry = Assert.Single(
+                modelStateDictionary,
+                kvp => kvp.Key == "theModel." + nameof(ModelWithBindRequiredAndRequiredAttribute.ValueTypeProperty))
+                .Value;
+            var error = Assert.Single(entry.Errors);
+            Assert.Null(error.Exception);
+            Assert.Equal("Custom Message ValueTypeProperty", 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.
+        [Fact]
+        public void ProcessDto_ReferenceTypePropertyWithBindRequired_CustomValidationMessage()
+        {
+            // Arrange
+            var model = new ModelWithBindRequiredAndRequiredAttribute();
+            var containerMetadata = GetMetadataForType(model.GetType());
+
+            var bindingContext = CreateContext(containerMetadata, model);
+            var modelStateDictionary = bindingContext.ModelState;
+
+            var dto = new ComplexModelDto(containerMetadata, containerMetadata.Properties);
+            var testableBinder = new TestableMutableObjectModelBinder();
+
+            // Make ValueTypeProperty have a value.
+            var propertyMetadata = containerMetadata
+                .Properties[nameof(ModelWithBindRequiredAndRequiredAttribute.ValueTypeProperty)];
+            dto.Results[propertyMetadata] = new ModelBindingResult(
+                17,
+                isModelSet: true,
+                key: "theModel." + nameof(ModelWithBindRequiredAndRequiredAttribute.ValueTypeProperty));
+
+            // Make ReferenceTypeProperty not have a value.
+            propertyMetadata = containerMetadata
+                .Properties[nameof(ModelWithBindRequiredAndRequiredAttribute.ReferenceTypeProperty)];
+            dto.Results[propertyMetadata] = new ModelBindingResult(
+                model: null,
+                isModelSet: false,
+                key: "theModel." + nameof(ModelWithBindRequiredAndRequiredAttribute.ReferenceTypeProperty));
+
+            var modelValidationNode = new ModelValidationNode(string.Empty, containerMetadata, model);
+
+            // Act
+            testableBinder.ProcessDto(bindingContext, dto, modelValidationNode);
+
+            // Assert
+            Assert.False(modelStateDictionary.IsValid);
+
+            var entry = Assert.Single(
+                modelStateDictionary,
+                kvp => kvp.Key == "theModel." + nameof(ModelWithBindRequiredAndRequiredAttribute.ReferenceTypeProperty))
+                .Value;
+            var error = Assert.Single(entry.Errors);
+            Assert.Null(error.Exception);
+            Assert.Equal("Custom Message ReferenceTypeProperty", error.ErrorMessage);
+
+            // Model gets provided values.
+            Assert.Equal(17, model.ValueTypeProperty);
+            Assert.Null(model.ReferenceTypeProperty);
+        }
+
+
         [Fact]
         public void ProcessDto_Success()
         {
@@ -1241,12 +1323,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 isModelSet: false,
                 key: "foo");
 
-            var validatorProvider = bindingContext.OperationBindingContext.ValidatorProvider;
-            var validatorProviderContext = new ModelValidatorProviderContext(propertyMetadata);
-            validatorProvider.GetValidators(validatorProviderContext);
-
-            var requiredValidator = validatorProviderContext.Validators.FirstOrDefault(v => v.IsRequired);
-
             var testableBinder = new TestableMutableObjectModelBinder();
 
             // Act
@@ -1254,8 +1330,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 bindingContext,
                 modelExplorer,
                 propertyMetadata,
-                dtoResult,
+                dtoResult);
-                requiredValidator);
 
             // Assert
             var person = Assert.IsType<Person>(bindingContext.Model);
@@ -1287,8 +1362,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 bindingContext,
                 modelExplorer,
                 propertyMetadata,
-                dtoResult,
+                dtoResult);
-                requiredValidator: null);
 
             // Assert
             var person = Assert.IsType<Person>(bindingContext.Model);
@@ -1320,8 +1394,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 bindingContext,
                 modelExplorer,
                 propertyMetadata,
-                dtoResult,
+                dtoResult);
-                requiredValidator: null);
 
             // Assert
             var person = Assert.IsType<Person>(bindingContext.Model);
@@ -1352,8 +1425,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 bindingContext,
                 modelExplorer,
                 propertyMetadata,
-                dtoResult,
+                dtoResult);
-                requiredValidator: null);
 
             // Assert
             // If didn't throw, success!
@@ -1406,8 +1478,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 bindingContext,
                 modelExplorer,
                 propertyMetadata,
-                dtoResult,
+                dtoResult);
-                requiredValidator: null);
 
             // Assert
             Assert.Equal("Joe", propertAccessor(model));
@@ -1486,8 +1557,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 bindingContext,
                 modelExplorer,
                 propertyMetadata,
-                dtoResult,
+                dtoResult);
-                requiredValidator: null);
 
             // Assert
             Assert.Equal(collection, propertyAccessor(model));
@@ -1511,12 +1581,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 key: "foo",
                 isModelSet: true);
 
-            var validatorProvider = bindingContext.OperationBindingContext.ValidatorProvider;
-            var validatorProviderContext = new ModelValidatorProviderContext(propertyMetadata);
-            validatorProvider.GetValidators(validatorProviderContext);
-
-            var requiredValidator = validatorProviderContext.Validators.FirstOrDefault(v => v.IsRequired);
-
             var testableBinder = new TestableMutableObjectModelBinder();
 
             // Act
@@ -1524,8 +1588,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 bindingContext,
                 modelExplorer,
                 propertyMetadata,
-                dtoResult,
+                dtoResult);
-                requiredValidator);
 
             // Assert
             Assert.True(bindingContext.ModelState.IsValid);
@@ -1560,8 +1623,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 bindingContext,
                 modelExplorer,
                 propertyMetadata,
-                dtoResult,
+                dtoResult);
-                requiredValidator: null);
 
             // Assert
             Assert.Equal("Date of death can't be before date of birth." + Environment.NewLine
@@ -1569,8 +1631,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                          bindingContext.ModelState["foo"].Errors[0].Exception.Message);
         }
 
+        // This can only really be done by writing an invalid model binder and returning 'isModelSet: true' 
+        // with a null model for a value type.
         [Fact]
-        public void SetProperty_SettingNonNullableValueTypeToNull_RequiredValidatorNotPresent_RegistersValidationCallback()
+        public void SetProperty_SettingNonNullableValueTypeToNull_CapturesException()
         {
             // Arrange
             var model = new Person();
@@ -1583,9 +1647,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             var dtoResult = new ModelBindingResult(
                 model: null,
                 isModelSet: true,
-                key: "foo");
+                key: "foo.DateOfBirth");
-
-            var requiredValidator = GetRequiredValidator(bindingContext, propertyMetadata);
 
             var testableBinder = new TestableMutableObjectModelBinder();
 
@@ -1594,50 +1656,20 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 bindingContext,
                 modelExplorer,
                 propertyMetadata,
-                dtoResult,
+                dtoResult);
-                requiredValidator);
 
             // Assert
             Assert.False(bindingContext.ModelState.IsValid);
-        }
 
-        [Fact]
+            var entry = Assert.Single(bindingContext.ModelState, kvp => kvp.Key == "foo.DateOfBirth").Value;
-        public void SetProperty_SettingNonNullableValueTypeToNull_RequiredValidatorPresent_AddsModelError()
+            var error = Assert.Single(entry.Errors);
-        {
+            Assert.Equal(string.Empty, error.ErrorMessage);
-            // Arrange
+            Assert.IsType<NullReferenceException>(error.Exception);
-            var model = new Person();
-            var bindingContext = CreateContext(GetMetadataForType(model.GetType()), model);
-            bindingContext.ModelName = " foo";
-
-            var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider;
-            var modelExplorer = metadataProvider.GetModelExplorerForType(typeof(Person), model);
-            var propertyMetadata = bindingContext.ModelMetadata.Properties["ValueTypeRequired"];
-
-            var dtoResult = new ModelBindingResult(
-                model: null,
-                isModelSet: true,
-                key: "foo.ValueTypeRequired");
-
-            var requiredValidator = GetRequiredValidator(bindingContext, propertyMetadata);
-
-            var testableBinder = new TestableMutableObjectModelBinder();
-
-            // Act
-            testableBinder.SetProperty(
-                bindingContext,
-                modelExplorer,
-                propertyMetadata,
-                dtoResult,
-                requiredValidator);
-
-            // Assert
-            Assert.False(bindingContext.ModelState.IsValid);
-            Assert.Equal("Sample message", bindingContext.ModelState["foo.ValueTypeRequired"].Errors[0].ErrorMessage);
         }
 
         [Fact]
         [ReplaceCulture]
-        public void SetProperty_SettingNullableTypeToNull_RequiredValidatorNotPresent_PropertySetterThrows_AddsRequiredMessageString()
+        public void SetProperty_PropertySetterThrows_CapturesException()
         {
             // Arrange
             var model = new ModelWhosePropertySetterThrows();
@@ -1645,7 +1677,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             bindingContext.ModelName = "foo";
 
             var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider;
-            var modelExplorer = metadataProvider.GetModelExplorerForType(typeof(Person), model);
+            var modelExplorer = metadataProvider.GetModelExplorerForType(typeof(ModelWhosePropertySetterThrows), model);
             var propertyMetadata = bindingContext.ModelMetadata.Properties["NameNoAttribute"];
 
             var dtoResult = new ModelBindingResult(
@@ -1653,8 +1685,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 isModelSet: true,
                 key: "foo.NameNoAttribute");
 
-            var requiredValidator = GetRequiredValidator(bindingContext, propertyMetadata);
-
             var testableBinder = new TestableMutableObjectModelBinder();
 
             // Act
@@ -1662,8 +1692,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 bindingContext,
                 modelExplorer,
                 propertyMetadata,
-                dtoResult,
+                dtoResult);
-                requiredValidator);
 
             // Assert
             Assert.False(bindingContext.ModelState.IsValid);
@@ -1673,41 +1702,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                          bindingContext.ModelState["foo.NameNoAttribute"].Errors[0].Exception.Message);
         }
 
-        [Fact]
-        public void SetProperty_SettingNullableTypeToNull_RequiredValidatorPresent_PropertySetterThrows_AddsRequiredMessageString()
-        {
-            // Arrange
-            var model = new ModelWhosePropertySetterThrows();
-            var bindingContext = CreateContext(GetMetadataForType(model.GetType()), model);
-            bindingContext.ModelName = "foo";
-
-            var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider;
-            var modelExplorer = metadataProvider.GetModelExplorerForType(typeof(Person), model);
-            var propertyMetadata = bindingContext.ModelMetadata.Properties["Name"];
-
-            var dtoResult = new ModelBindingResult(
-                model: null,
-                isModelSet: true,
-                key: "foo.Name");
-
-            var requiredValidator = GetRequiredValidator(bindingContext, propertyMetadata);
-
-            var testableBinder = new TestableMutableObjectModelBinder();
-
-            // Act
-            testableBinder.SetProperty(
-                bindingContext,
-                modelExplorer,
-                propertyMetadata,
-                dtoResult,
-                requiredValidator);
-
-            // Assert
-            Assert.False(bindingContext.ModelState.IsValid);
-            var error = Assert.Single(bindingContext.ModelState["foo.Name"].Errors);
-            Assert.Equal("This message comes from the [Required] attribute.", error.ErrorMessage);
-        }
-
         private static ModelBindingContext CreateContext(ModelMetadata metadata, object model)
         {
             return new ModelBindingContext
@@ -1841,6 +1835,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             public string Optional { get; set; }
         }
 
+        [BindRequired]
+        private class ModelWithBindRequiredAndRequiredAttribute
+        {
+            [Range(5, 20)]
+            [Required(ErrorMessage = "Custom Message {0}")]
+            public int ValueTypeProperty { get; set; }
+
+            [StringLength(25)]
+            [Required(ErrorMessage = "Custom Message {0}")]
+            public string ReferenceTypeProperty { get; set; }
+        }
+
         private sealed class MyModelTestingCanUpdateProperty
         {
             public int ReadOnlyInt { get; private set; }
@@ -2030,15 +2036,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 ModelBindingContext bindingContext,
                 ModelExplorer modelExplorer,
                 ModelMetadata propertyMetadata,
-                ModelBindingResult dtoResult,
+                ModelBindingResult dtoResult)
-                IModelValidator requiredValidator)
             {
                 base.SetProperty(
                     bindingContext,
                     modelExplorer,
                     propertyMetadata,
-                    dtoResult,
+                    dtoResult);
-                    requiredValidator);
             }
         }
     }

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

@@ -295,9 +295,9 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
 
         // This model sets a value for 'Title', and the model binder won't trounce it.
         //
-        // There's a validation error because we validate the value in the request (not present).
+        // There's no validation error because we validate the value on the model.
         [Fact]
-        public async Task FromHeader_BindHeader_ToModel_NoValues_InitializedValue_ValidationError()
+        public async Task FromHeader_BindHeader_ToModel_NoValues_InitializedValue_NoValidationError()
         {
             // Arrange
             var server = TestHelper.CreateServer(_app, SiteName, _configureServices);
@@ -321,8 +321,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
             Assert.Equal("How to Make Soup", result.HeaderValue);
             Assert.Equal<string>(new[] { "Cooking" }, result.HeaderValues);
 
-            var error = Assert.Single(result.ModelStateErrors);
+            Assert.Empty(result.ModelStateErrors);
-            Assert.Equal("Title", error);
         }
 
         private class Result

test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/MvcTagHelpersWebSite.MvcTagHelper_Customer.Index.html

@@ -7,7 +7,7 @@
         <div>
             <label class="order" for="Number">Number</label>
             <input class="form-control input-validation-error" type="number" data-val="true" data-val-range="The field Number must be between 1 and 100." data-val-range-max="100" data-val-range-min="1" id="Number" name="Number" value="" />
-            <span class="field-validation-error" data-valmsg-for="Number" data-valmsg-replace="true">A value is required.</span>
+            <span class="field-validation-error" data-valmsg-for="Number" data-valmsg-replace="true">The value &#x27;&#x27; is invalid.</span>
         </div>
         <div>
             <label class="order" for="Name">Name</label>
@@ -33,8 +33,7 @@
             <input type="radio" value="Female" id="Gender" name="Gender" /> Female
             <span class="field-validation-valid" data-valmsg-for="Gender" data-valmsg-replace="true"></span>
         </div>
-        <div class="order validation-summary-errors" data-valmsg-summary="true"><ul><li>A value is required.</li>
+        <div class="order validation-summary-errors" data-valmsg-summary="true"><ul><li>The Password field is required.</li>
-<li>The Password field is required.</li>
 </ul></div>
         <div class="order validation-summary-errors"><ul><li style="display:none"></li>
 </ul></div>

test/Microsoft.AspNet.Mvc.FunctionalTests/compiler/resources/TagHelpersWebSite.Employee.Create.Invalid.html

@@ -55,7 +55,7 @@
             <div class="form-group">
                 <label class="control-label col-md-2" for="JoinDate">JoinDate</label>
                 <div class="col-md-10">
-                    <input class="form-control input-validation-error" type="date" data-val="true" data-val-required="The JoinDate field is required." id="JoinDate" name="JoinDate" value="0001-01-01" />
+                    <input class="form-control input-validation-error" type="date" data-val="true" data-val-required="The JoinDate field is required." id="JoinDate" name="JoinDate" value="" />
                     <span class="field-validation-error" data-valmsg-for="JoinDate" data-valmsg-replace="true">The JoinDate field is required.</span>
                 </div>
             </div>

test/WebSites/TagHelpersWebSite/Models/Employee.cs

@@ -21,7 +21,7 @@ namespace TagHelpersWebSite.Models
 
         [Required]
         [DataType(DataType.Date)]
-        public DateTimeOffset JoinDate { get; set; }
+        public DateTimeOffset? JoinDate { get; set; }
 
         [DataType(DataType.EmailAddress)]
         public string Email { get; set; }