Address #1865 root cause

- revert most of previous fix attempt (leaving doc comment updates)
- change `MutableObjectModelBinder` to ignore exact match in value providers
 - had an incorrect assumption: don't want exact model name to match since
   this binder supports only complex objects
 - ignored `BinderModelName`, value provider filtering, et cetera
- reduces over-binding e.g. `[Required]` validation within missing properties

src/Microsoft.AspNet.Mvc.ModelBinding/Binders/ComplexModelDtoModelBinder.cs

@@ -31,7 +31,7 @@ public async Task<ModelBindingResult> BindModelAsync(ModelBindingContext binding
                     await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(propertyBindingContext);
                 if (modelBindingResult == null)
                 {
-                    // Could not bind. Add default result to let caller know to pick up default value (if any).
+                    // Could not bind. Add a result so MutableObjectModelBinder will check for [DefaultValue].
                     dto.Results[propertyMetadata] =
                         new ModelBindingResult(model: null, key: propertyModelName, isModelSet: false);
                 }

src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CompositeModelBinder.cs

@@ -36,26 +36,17 @@ public virtual async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBind
             var newBindingContext = CreateNewBindingContext(bindingContext, bindingContext.ModelName);
             var modelBindingResult = await TryBind(newBindingContext);
 
-            if (bindingContext.FallbackToEmptyPrefix && !string.IsNullOrEmpty(bindingContext.ModelName))
+            if (modelBindingResult == null &&
+                bindingContext.FallbackToEmptyPrefix &&
+                !string.IsNullOrEmpty(bindingContext.ModelName))
             {
-                var fallback = modelBindingResult == null;
-                if (!fallback && !modelBindingResult.IsModelSet && modelBindingResult.Model != null)
-                {
-                    // Special case the empty object (i.e. non-null but contains nothing from value providers)
-                    // MutableObjectModelBinder may return.
-                    fallback = true;
-                }
-
-                if (fallback)
-                {
-                    // Remove any validation errors from first attempt; these are likely Required errors.
-                    bindingContext.ModelState.Clear();
+                // Remove any validation errors from first attempt; these are likely Required errors.
+                bindingContext.ModelState.Clear();
 
-                    // Fall back to empty prefix.
-                    newBindingContext = CreateNewBindingContext(bindingContext,
-                                                                modelName: string.Empty);
-                    modelBindingResult = await TryBind(newBindingContext);
-                }
+                // Fall back to empty prefix.
+                newBindingContext = CreateNewBindingContext(bindingContext,
+                                                            modelName: string.Empty);
+                modelBindingResult = await TryBind(newBindingContext);
             }
 
             if (modelBindingResult == null)
@@ -66,11 +57,7 @@ public virtual async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBind
             bindingContext.OperationBindingContext.BodyBindingState =
                 newBindingContext.OperationBindingContext.BodyBindingState;
 
-            // By default, use the original ModelName even if we fell back to the empty prefix.
-            var key = bindingContext.ModelName;
-
-            var isModelSet = modelBindingResult.IsModelSet || modelBindingResult.Model != null;
-            if (isModelSet)
+            if (modelBindingResult.IsModelSet)
             {
                 // Update the model state key if we are bound using an empty prefix and it is a complex type.
                 // This is needed as validation uses the model state key to log errors. The client validation expects
@@ -93,11 +80,15 @@ public virtual async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBind
                     // In this case, for the model parameter the key would be SimpleType instead of model.SimpleType.
                     // (i.e here the prefix for the model key is empty).
                     // For the id parameter the key would be id.
-                    key = modelBindingResult.Key;
+                    return modelBindingResult;
                 }
             }
 
-            return new ModelBindingResult(modelBindingResult.Model, key, isModelSet);
+            // Fall through to update the ModelBindingResult's key.
+            return new ModelBindingResult(
+                modelBindingResult.Model,
+                bindingContext.ModelName,
+                modelBindingResult.IsModelSet);
         }
 
         private async Task<ModelBindingResult> TryBind(ModelBindingContext bindingContext)
@@ -109,9 +100,7 @@ private async Task<ModelBindingResult> TryBind(ModelBindingContext bindingContex
                 var result = await binder.BindModelAsync(bindingContext);
                 if (result != null)
                 {
-                    if (result.IsModelSet ||
-                        result.Model != null ||
-                        bindingContext.ModelState.ContainsKey(bindingContext.ModelName))
+                    if (result.IsModelSet || bindingContext.ModelState.ContainsKey(bindingContext.ModelName))
                     {
                         return result;
                     }

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

@@ -36,11 +36,11 @@ public virtual async Task<ModelBindingResult> BindModelAsync(ModelBindingContext
             var result = await CreateAndPopulateDto(bindingContext, mutableObjectBinderContext.PropertyMetadata);
 
             // post-processing, e.g. property setters and hooking up validation
-            var isModelSet = ProcessDto(bindingContext, (ComplexModelDto)result.Model);
+            ProcessDto(bindingContext, (ComplexModelDto)result.Model);
             return new ModelBindingResult(
                 bindingContext.Model,
                 bindingContext.ModelName,
-                isModelSet);
+                isModelSet: true);
         }
 
         protected virtual bool CanUpdateProperty(ModelMetadata propertyMetadata)
@@ -95,16 +95,7 @@ internal async Task<bool> CanCreateModel(MutableObjectBinderContext context)
                 return true;
             }
 
-            // 3. The model name is not prefixed and a value provider can directly provide a value for the model name.
-            //    The fact that it is not prefixed means that the containsPrefixAsync call checks for the exact
-            //    model name instead of doing a prefix match.
-            if (!bindingContext.ModelName.Contains(".") &&
-                await bindingContext.ValueProvider.ContainsPrefixAsync(bindingContext.ModelName))
-            {
-                return true;
-            }
-
-            // 4. Any of the model properties can be bound using a value provider.
+            // 3. Any of the model properties can be bound using a value provider.
             if (await CanValueBindAnyModelProperties(context))
             {
                 return true;
@@ -367,7 +358,7 @@ internal static PropertyValidationInfo GetPropertyValidationInfo(ModelBindingCon
             return validationInfo;
         }
 
-        internal bool ProcessDto(ModelBindingContext bindingContext, ComplexModelDto dto)
+        internal void ProcessDto(ModelBindingContext bindingContext, ComplexModelDto dto)
         {
             var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider;
             var modelExplorer = metadataProvider.GetModelExplorerForType(bindingContext.ModelType, bindingContext.Model);
@@ -409,15 +400,13 @@ internal bool ProcessDto(ModelBindingContext bindingContext, ComplexModelDto dto
                 }
             }
 
-            // for each property that was bound, call the setter, recording exceptions as necessary
-            bool isModelSet = false;
+            // For each property that ComplexModelDtoModelBinder attempted to bind, call the setter, recording
+            // exceptions as necessary.
             foreach (var entry in dto.Results)
             {
                 var dtoResult = entry.Value;
                 if (dtoResult != null)
                 {
-                    isModelSet |= dtoResult.IsModelSet;
-
                     var propertyMetadata = entry.Key;
                     IModelValidator requiredValidator;
                     validationInfo.RequiredValidators.TryGetValue(
@@ -427,8 +416,6 @@ internal bool ProcessDto(ModelBindingContext bindingContext, ComplexModelDto dto
                     SetProperty(bindingContext, modelExplorer, propertyMetadata, dtoResult, requiredValidator);
                 }
             }
-
-            return isModelSet;
         }
 
         protected virtual void SetProperty(

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

@@ -716,7 +716,7 @@ public async Task ModelBindingBindsEmptyStringsToByteArrays()
         }
 
         [Fact]
-        public async Task ModelBinding_LimitsErrorsToMaxErrorCount()
+        public async Task ModelBinding_LimitsErrorsToMaxErrorCount_DoesNotValidateMembersOfMissingProperties()
         {
             // Arrange
             var server = TestHelper.CreateServer(_app, SiteName);
@@ -728,15 +728,16 @@ public async Task ModelBinding_LimitsErrorsToMaxErrorCount()
 
             //Assert
             var json = JsonConvert.DeserializeObject<Dictionary<string, string>>(response);
+
             // 8 is the value of MaxModelValidationErrors for the application being tested.
             Assert.Equal(8, json.Count);
-            Assert.Equal("The Field1 field is required.", json["Field1.Field1"]);
-            Assert.Equal("The Field2 field is required.", json["Field1.Field2"]);
-            Assert.Equal("The Field3 field is required.", json["Field1.Field3"]);
-            Assert.Equal("The Field1 field is required.", json["Field2.Field1"]);
-            Assert.Equal("The Field2 field is required.", json["Field2.Field2"]);
-            Assert.Equal("The Field3 field is required.", json["Field2.Field3"]);
-            Assert.Equal("The Field1 field is required.", json["Field3.Field1"]);
+            Assert.Equal("The Field1 field is required.", json["Field1"]);
+            Assert.Equal("The Field2 field is required.", json["Field2"]);
+            Assert.Equal("The Field3 field is required.", json["Field3"]);
+            Assert.Equal("The Field4 field is required.", json["Field4"]);
+            Assert.Equal("The Field5 field is required.", json["Field5"]);
+            Assert.Equal("The Field6 field is required.", json["Field6"]);
+            Assert.Equal("The Field7 field is required.", json["Field7"]);
             Assert.Equal("The maximum number of allowed model errors has been reached.", json[""]);
         }
 
@@ -1932,7 +1933,7 @@ public async Task BindModelAsync_WithNestedCollection()
         }
 
         [Fact]
-        public async Task BindModelAsync_WithIncorrectlyFormattedNestedCollectionValue()
+        public async Task BindModelAsync_WithIncorrectlyFormattedNestedCollectionValue_BindsSingleNullEntry()
         {
             // Arrange
             var server = TestHelper.CreateServer(_app, SiteName);
@@ -1949,9 +1950,11 @@ public async Task BindModelAsync_WithIncorrectlyFormattedNestedCollectionValue()
 
             // Assert
             var result = await ReadValue<UserWithAddress>(response);
+
+            // Reviewers: Slightly odd behavior is specific to unusual error cases. Happens because
+            // MutableObjectModelBinder no longer incorrectly creates a model when value providers have no data.
             var address = Assert.Single(result.Addresses);
-            Assert.Null(address.AddressLines);
-            Assert.Null(address.ZipCode);
+            Assert.Null(address);
         }
 
         [Fact]
@@ -1993,7 +1996,7 @@ public async Task BindModelAsync_WithNestedCollectionContainingRecursiveRelation
         }
 
         [Fact]
-        public async Task BindModelAsync_WithNestedCollectionContainingRecursiveRelation_WithMalformedValue()
+        public async Task BindModelAsync_WithNestedCollectionContainingRecursiveRelation_WithMalformedValue_BindsSingleNullEntry()
         {
             // Arrange
             var server = TestHelper.CreateServer(_app, SiteName);
@@ -2010,9 +2013,11 @@ public async Task BindModelAsync_WithNestedCollectionContainingRecursiveRelation
 
             // Assert
             var result = await ReadValue<PeopleModel>(response);
+
+            // Reviewers: Slightly odd behavior is specific to unusual error cases. Happens because
+            // MutableObjectModelBinder no longer incorrectly creates a model when value providers have no data.
             var person = Assert.Single(result.People);
-            Assert.Null(person.Name);
-            Assert.Null(person.Parent);
+            Assert.Null(person);
         }
 
         [Theory]

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

@@ -383,7 +383,7 @@ public async Task BindModel_InitsInstance()
 
             // Assert
             Assert.NotNull(retValue);
-            Assert.False(retValue.IsModelSet); // MutableObjectModelBinder lets caller know DTO got nothing.
+            Assert.True(retValue.IsModelSet);
             var returnedPerson = Assert.IsType<Person>(retValue.Model);
             Assert.Same(model, returnedPerson);
             testableBinder.Verify();
@@ -436,7 +436,7 @@ public async Task BindModel_InitsInstance_ForEmptyModelName()
 
             // Assert
             Assert.NotNull(retValue);
-            Assert.False(retValue.IsModelSet); // MutableObjectModelBinder lets caller know DTO got nothing.
+            Assert.True(retValue.IsModelSet);
             var returnedPerson = Assert.IsType<Person>(retValue.Model);
             Assert.Same(model, returnedPerson);
             testableBinder.Verify();

test/WebSites/ModelBindingWebSite/Models/LargeModelWithValidation.cs

@@ -18,5 +18,17 @@ public class LargeModelWithValidation
 
         [Required]
         public ModelWithValidation Field4 { get; set; }
+
+        [Required]
+        public ModelWithValidation Field5 { get; set; }
+
+        [Required]
+        public ModelWithValidation Field6 { get; set; }
+
+        [Required]
+        public ModelWithValidation Field7 { get; set; }
+
+        [Required]
+        public ModelWithValidation Field8 { get; set; }
     }
 }