`CompositeModelBinder.TryBind()` should return `null` more often

- #2129
- do not propagate results with `!IsModelSet`, allowing empty prefix fallback
- adjust `ComplexModelDtoModelBinder` to at least fake-bind all properties
 - default values not consistently picked up otherwise

nit: correct 2 test names in `KeyValuePairModelBinderTest`

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

@@ -15,27 +15,29 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 return null;
             }
 
-            ModelBindingHelper.ValidateBindingContext(bindingContext,
-                                                      typeof(ComplexModelDto),
-                                                      allowNullModel: false);
+            ModelBindingHelper.ValidateBindingContext(bindingContext, typeof(ComplexModelDto), allowNullModel: false);
 
             var dto = (ComplexModelDto)bindingContext.Model;
             foreach (var propertyMetadata in dto.PropertyMetadata)
             {
                 var propertyModelName = ModelBindingHelper.CreatePropertyModelName(
-                        bindingContext.ModelName,
-                        propertyMetadata.BinderModelName ?? propertyMetadata.PropertyName);
+                    bindingContext.ModelName,
+                    propertyMetadata.BinderModelName ?? propertyMetadata.PropertyName);
 
                 var propertyBindingContext = ModelBindingContext.GetChildModelBindingContext(
                     bindingContext,
                     propertyModelName,
                     propertyMetadata);
 
-                // bind and propagate the values
-                // If we can't bind then leave the result missing (don't add a null).
                 var modelBindingResult =
                     await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(propertyBindingContext);
-                if (modelBindingResult != null)
+                if (modelBindingResult == null)
+                {
+                    // Could not bind. Add a result so MutableObjectModelBinder will check for [DefaultValue].
+                    dto.Results[propertyMetadata] =
+                        new ModelBindingResult(model: null, key: propertyModelName, isModelSet: false);
+                }
+                else
                 {
                     dto.Results[propertyMetadata] = modelBindingResult;
                 }

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

@@ -97,7 +97,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 var result = await binder.BindModelAsync(bindingContext);
                 if (result != null)
                 {
-                    return result;
+                    // Use returned ModelBindingResult if it either indicates the model was set or is related to a
+                    // ModelState entry. The second condition is necessary because the ModelState entry would never be
+                    // validated if caller fell back to the empty prefix, leading to an possibly-incorrect !IsValid.
+                    //
+                    // In most (hopefully all) cases, the ModelState entry exists because some binders add errors
+                    // before returning a result with !IsModelSet. Those binders often cannot run twice anyhow.
+                    if (result.IsModelSet || bindingContext.ModelState.ContainsKey(bindingContext.ModelName))
+                    {
+                        return result;
+                    }
+
+                    // Current binder should have been able to bind value but found nothing. Exit loop in a way that
+                    // tells caller to fall back to the empty prefix, if appropriate. Do not return result because it
+                    // means only "other binders are not applicable".
+                    break;
                 }
             }
 

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

@@ -410,7 +410,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 }
             }
 
-            // for each property that was bound, call the setter, recording exceptions as necessary
+            // 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;

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

@@ -112,10 +112,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
         }
 
         [Fact]
-        public async Task ModelBinder_ReturnsTrue_WithoutSettingValue()
+        public async Task ModelBinder_ReturnsNull_IfBinderMatchesButDoesNotSetModel()
         {
             // Arrange
-
             var bindingContext = new ModelBindingContext
             {
                 FallbackToEmptyPrefix = true,
@@ -135,7 +134,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
             var modelBinder = new Mock<IModelBinder>();
             modelBinder
                 .Setup(mb => mb.BindModelAsync(It.IsAny<ModelBindingContext>()))
-                .Returns(Task.FromResult(new ModelBindingResult(null, "someName", false)));
+                .Returns(Task.FromResult(new ModelBindingResult(model: null, key: "someName", isModelSet: false)));
 
             var composite = CreateCompositeBinder(modelBinder.Object);
 
@@ -143,10 +142,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
             var result = await composite.BindModelAsync(bindingContext);
 
             // Assert
-            Assert.NotNull(result);
-            Assert.False(result.IsModelSet);
-            Assert.Equal("someName", result.Key);
-            Assert.Null(result.Model);
+            Assert.Null(result);
         }
 
         [Fact]

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

@@ -14,7 +14,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
     public class KeyValuePairModelBinderTest
     {
         [Fact]
-        public async Task BindModel_MissingKey_ReturnsTrue_AndAddsModelValidationError()
+        public async Task BindModel_MissingKey_ReturnsResult_AndAddsModelValidationError()
         {
             // Arrange
             var valueProvider = new SimpleHttpValueProvider();
@@ -36,7 +36,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
         }
 
         [Fact]
-        public async Task BindModel_MissingValue_ReturnsTrue_AndAddsModelValidationError()
+        public async Task BindModel_MissingValue_ReturnsResult_AndAddsModelValidationError()
         {
             // Arrange
             var valueProvider = new SimpleHttpValueProvider();