Fix #4238 IFormFile model binder suppresses validation

This change no longer suppresses validation for IFormFile and
IFormFileCollection model values. This will allow the use of [Required] on an
IFormFile model, or a custom attribute for validating IFormFileCollection.

These types already have ValidateChildren = false, so we don't recurse
into them.

src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/FormCollectionModelBinder.cs

@@ -36,8 +36,7 @@ public async Task BindModelAsync(ModelBindingContext bindingContext)
             {
                 model = new EmptyFormCollection();
             }
-
-            bindingContext.ValidationState.Add(model, new ValidationStateEntry() { SuppressValidation = true });
+
             bindingContext.Result = ModelBindingResult.Success(bindingContext.ModelName, model);
         }
 

src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/FormFileModelBinder.cs

@@ -98,10 +98,11 @@ public async Task BindModelAsync(ModelBindingContext bindingContext)
                 }
             }
 
+            // We need to add a ValidationState entry because the modelName might be non-standard. Otherwise
+            // the entry we create in model state might not be marked as valid.
             bindingContext.ValidationState.Add(value, new ValidationStateEntry()
             {
                 Key = modelName,
-                SuppressValidation = true
             });
 
             bindingContext.ModelState.SetModelValue(

test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/FormCollectionModelBinderTest.cs

@@ -36,10 +36,7 @@ public async Task FormCollectionModelBinder_ValidType_BindSuccessful()
             Assert.NotEqual(default(ModelBindingResult), result);
             Assert.True(result.IsModelSet);
 
-            var entry = bindingContext.ValidationState[result.Model];
-            Assert.True(entry.SuppressValidation);
-            Assert.Null(entry.Key);
-            Assert.Null(entry.Metadata);
+            Assert.Empty(bindingContext.ValidationState);
 
             var form = Assert.IsAssignableFrom<IFormCollection>(result.Model);
             Assert.Equal(2, form.Count);

test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/FormFileModelBinderTest.cs

@@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders
     public class FormFileModelBinderTest
     {
         [Fact]
-        public async Task FormFileModelBinder_SuppressesValidation()
+        public async Task FormFileModelBinder_SingleFile_BindSuccessful()
         {
             // Arrange
             var formFiles = new FormFileCollection();
@@ -33,7 +33,7 @@ public async Task FormFileModelBinder_SuppressesValidation()
             Assert.True(result.IsModelSet);
 
             var entry = bindingContext.ValidationState[result.Model];
-            Assert.True(entry.SuppressValidation);
+            Assert.False(entry.SuppressValidation);
             Assert.Equal("file", entry.Key);
             Assert.Null(entry.Metadata);
         }
@@ -55,7 +55,7 @@ public async Task FormFileModelBinder_ExpectMultipleFiles_BindSuccessful()
             Assert.True(result.IsModelSet);
 
             var entry = bindingContext.ValidationState[result.Model];
-            Assert.True(entry.SuppressValidation);
+            Assert.False(entry.SuppressValidation);
             Assert.Equal("file", entry.Key);
             Assert.Null(entry.Metadata);
 

test/Microsoft.AspNetCore.Mvc.IntegrationTests/FormFileModelBindingIntegrationTest.cs

@@ -74,7 +74,7 @@ public async Task BindProperty_WithData_WithEmptyPrefix_GetsBound()
             var key = Assert.Single(modelState.Keys, k => k == "Address.File");
             Assert.Null(modelState[key].RawValue);
             Assert.Empty(modelState[key].Errors);
-            Assert.Equal(ModelValidationState.Skipped, modelState[key].ValidationState);
+            Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState);
         }
 
         private class ListContainer1
@@ -125,7 +125,7 @@ public async Task BindCollectionProperty_WithData_IsBound()
             var modelStateEntry = kvp.Value;
             Assert.NotNull(modelStateEntry);
             Assert.Empty(modelStateEntry.Errors);
-            Assert.Equal(ModelValidationState.Skipped, modelStateEntry.ValidationState);
+            Assert.Equal(ModelValidationState.Valid, modelStateEntry.ValidationState);
             Assert.Null(modelStateEntry.AttemptedValue);
             Assert.Null(modelStateEntry.RawValue);
         }
@@ -216,7 +216,7 @@ public async Task BindReadOnlyCollectionProperty_WithData_IsBound()
             var modelStateEntry = kvp.Value;
             Assert.NotNull(modelStateEntry);
             Assert.Empty(modelStateEntry.Errors);
-            Assert.Equal(ModelValidationState.Skipped, modelStateEntry.ValidationState);
+            Assert.Equal(ModelValidationState.Valid, modelStateEntry.ValidationState);
             Assert.Null(modelStateEntry.AttemptedValue);
             Assert.Null(modelStateEntry.RawValue);
         }
@@ -265,7 +265,7 @@ public async Task BindParameter_WithData_GetsBound()
             var entry = Assert.Single(modelState);
             Assert.Equal("CustomParameter", entry.Key);
             Assert.Empty(entry.Value.Errors);
-            Assert.Equal(ModelValidationState.Skipped, entry.Value.ValidationState);
+            Assert.Equal(ModelValidationState.Valid, entry.Value.ValidationState);
             Assert.Null(entry.Value.AttemptedValue);
             Assert.Null(entry.Value.RawValue);
         }

test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs

@@ -1000,7 +1000,7 @@ public async Task TryUpdateModelAsync_TopLevelFormFileCollection_IsBound()
             var modelStateEntry = kvp.Value;
             Assert.NotNull(modelStateEntry);
             Assert.Empty(modelStateEntry.Errors);
-            Assert.Equal(ModelValidationState.Skipped, modelStateEntry.ValidationState);
+            Assert.Equal(ModelValidationState.Valid, modelStateEntry.ValidationState);
             Assert.Null(modelStateEntry.AttemptedValue);
             Assert.Null(modelStateEntry.RawValue);
         }