Issue #1695 - Create a pattern for a 'greedy' model binder.

See #1695 for a detailed explanation. This change builds support into the
system for the case that a model binder returns true without setting a
value for the Model.

In this case, validation will be skipped if it's a top-level object.

Note that explicitly setting null will still run validation.

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

@@ -54,7 +54,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
 
             // Only perform validation at the root of the object graph. ValidationNode will recursively walk the graph.
             // Ignore ComplexModelDto since it essentially wraps the primary object.
-            if (IsBindingAtRootOfObjectGraph(newBindingContext))
+            if (newBindingContext.IsModelSet && IsBindingAtRootOfObjectGraph(newBindingContext))
             {
                 // run validation and return the model
                 // If we fell back to an empty prefix above and are dealing with simple types,
@@ -79,7 +79,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
 
             bindingContext.OperationBindingContext.BodyBindingState =
                 newBindingContext.OperationBindingContext.BodyBindingState;
-            bindingContext.Model = newBindingContext.Model;
+
+            if (newBindingContext.IsModelSet)
+            {
+                bindingContext.Model = newBindingContext.Model;
+            }
+
             return true;
         }
 
@@ -115,6 +120,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
         {
             var newBindingContext = new ModelBindingContext
             {
+                IsModelSet = oldBindingContext.IsModelSet,
                 ModelMetadata = oldBindingContext.ModelMetadata,
                 ModelName = modelName,
                 ModelState = oldBindingContext.ModelState,

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

@@ -22,7 +22,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             if (bindingContext.ModelType == typeof(string))
             {
                 var value = request.Headers.Get(bindingContext.ModelName);
-                bindingContext.Model = value;
+                if (value != null)
+                {
+                    bindingContext.Model = value;
+                }
 
                 return Task.FromResult(true);
             }

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

@@ -263,7 +263,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
         {
             if (bindingContext.Model == null)
             {
-                bindingContext.ModelMetadata.Model = CreateModel(bindingContext);
+                bindingContext.Model = CreateModel(bindingContext);
             }
         }
 

src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingContext.cs

@@ -71,11 +71,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             }
             set
             {
+                IsModelSet = true;
+
                 EnsureModelMetadata();
                 ModelMetadata.Model = value;
             }
         }
 
+        /// <summary>
+        /// Gets or sets a value indicating whether or not the <see cref="Model"/> value has been set.
+        /// 
+        /// This property can be used to distinguish between a model binder which does not find a value and
+        /// the case where a model binder sets the <c>null</c> value.
+        /// </summary>
+        public bool IsModelSet { get; set; }
+
         /// <summary>
         /// Gets or sets the metadata for the model associated with this context.
         /// </summary>

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

@@ -71,7 +71,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
             Assert.Empty(result.ModelStateErrors);
         }
 
-        // The action that this test hits will echo back the model-state error
+        // There should be no model state error for a top-level object
         [Theory]
         [InlineData("transactionId1234", "1e331f25-0869-4c87-8a94-64e6e40cb5a0")]
         public async Task FromHeader_BindHeader_ToString_OnParameter_NoValues(string headerName, string headerValue)
@@ -96,9 +96,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
 
             Assert.Null(result.HeaderValue);
             Assert.Null(result.HeaderValues);
-            
-            var error = Assert.Single(result.ModelStateErrors);
-            Assert.Equal("transactionId", error);
+            Assert.Empty(result.ModelStateErrors);
         }
 
         // The action that this test hits will echo back the model-bound values
@@ -160,6 +158,35 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
             Assert.Empty(result.ModelStateErrors);
         }
 
+        // Title on the model has [Required] so it will have a validation error
+        // Tags does not, so no error.
+        [Fact]
+        public async Task FromHeader_BindHeader_ToModel_NoValues_ValidationError()
+        {
+            // Arrange
+            var server = TestServer.Create(_services, _app);
+            var client = server.CreateClient();
+
+            var request = new HttpRequestMessage(HttpMethod.Get, "http://localhost/Blog/BindToModel?author=Marvin");
+
+            // Intentionally not setting a title or tags
+
+            // Act
+            var response = await client.SendAsync(request);
+
+            // Assert
+            Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+
+            var body = await response.Content.ReadAsStringAsync();
+            var result = JsonConvert.DeserializeObject<Result>(body);
+
+            Assert.Null(result.HeaderValue);
+            Assert.Null(result.HeaderValues);
+
+            var error = Assert.Single(result.ModelStateErrors);
+            Assert.Equal("Title", error);
+        }
+
         private class Result
         {
             public string HeaderValue { get; set; }

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

@@ -1112,7 +1112,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
             var body = await response.Content.ReadAsStringAsync();
             var modelStateErrors = JsonConvert.DeserializeObject<IDictionary<string, IEnumerable<string>>>(body);
 
-            Assert.Equal(3, modelStateErrors.Count);
+            Assert.Equal(2, modelStateErrors.Count);
             Assert.Equal(new[] {
                     "The field Year must be between 1980 and 2034.",
                     "Year is invalid"
@@ -1120,9 +1120,6 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
 
             var vinError = Assert.Single(modelStateErrors["model.Vin"]);
             Assert.Equal("The Vin field is required.", vinError);
-
-            var trackingIdError = Assert.Single(modelStateErrors["X-TrackingId"]);
-            Assert.Equal("A value is required but was not present in the request.", trackingIdError);
         }
 
         [Fact]

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

@@ -119,6 +119,97 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
             Assert.True(bindingContext.ModelState.IsValid);
         }
 
+        [Fact]
+        public async Task ModelBinder_ReturnsTrue_WithoutSettingValue_SkipsValidation()
+        {
+            // Arrange
+            var validationCalled = false;
+
+            var bindingContext = new ModelBindingContext
+            {
+                FallbackToEmptyPrefix = true,
+                ModelMetadata = new EmptyModelMetadataProvider().GetMetadataForType(null, typeof(List<int>)),
+                ModelName = "someName",
+                ModelState = new ModelStateDictionary(),
+                ValueProvider = new SimpleHttpValueProvider
+                {
+                    { "someOtherName", "dummyValue" }
+                },
+                OperationBindingContext = new OperationBindingContext
+                {
+                    ValidatorProvider = GetValidatorProvider()
+                }
+            };
+
+            var modelBinder = new Mock<IModelBinder>();
+            modelBinder
+                .Setup(mb => mb.BindModelAsync(It.IsAny<ModelBindingContext>()))
+                .Callback<ModelBindingContext>(context =>
+                {
+                    context.ValidationNode.Validating += delegate { validationCalled = true; };
+                })
+                .Returns(Task.FromResult(true));
+
+            var composite = CreateCompositeBinder(modelBinder.Object);
+
+            // Act
+            var isBound = await composite.BindModelAsync(bindingContext);
+
+            // Assert
+            Assert.True(isBound);
+
+            Assert.Null(bindingContext.Model);
+            Assert.False(validationCalled);
+            Assert.False(bindingContext.IsModelSet);
+            Assert.True(bindingContext.ModelState.IsValid);
+        }
+
+        [Fact]
+        public async Task ModelBinder_ReturnsTrue_SetsNullValue_RunsValidation()
+        {
+            // Arrange
+            var validationCalled = false;
+
+            var bindingContext = new ModelBindingContext
+            {
+                FallbackToEmptyPrefix = true,
+                ModelMetadata = new EmptyModelMetadataProvider().GetMetadataForType(null, typeof(List<int>)),
+                ModelName = "someName",
+                ModelState = new ModelStateDictionary(),
+                ValueProvider = new SimpleHttpValueProvider
+                {
+                    { "someOtherName", "dummyValue" }
+                },
+                OperationBindingContext = new OperationBindingContext
+                {
+                    ValidatorProvider = GetValidatorProvider()
+                }
+            };
+
+            var modelBinder = new Mock<IModelBinder>();
+            modelBinder
+                .Setup(mb => mb.BindModelAsync(It.IsAny<ModelBindingContext>()))
+                .Callback<ModelBindingContext>(context =>
+                {
+                    context.Model = null;
+                    context.ValidationNode.Validating += delegate { validationCalled = true; };
+                })
+                .Returns(Task.FromResult(true));
+
+            var composite = CreateCompositeBinder(modelBinder.Object);
+
+            // Act
+            var isBound = await composite.BindModelAsync(bindingContext);
+
+            // Assert
+            Assert.True(isBound);
+
+            Assert.Null(bindingContext.Model);
+            Assert.True(validationCalled);
+            Assert.True(bindingContext.IsModelSet);
+            Assert.False(bindingContext.ModelState.IsValid);
+        }
+
         [Fact]
         public async Task BindModel_UnsuccessfulBind_BinderFails_ReturnsNull()
         {

test/WebSites/ModelBindingWebSite/Controllers/FromHeader_BlogController.cs

@@ -1,6 +1,7 @@
 // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
+using System.ComponentModel.DataAnnotations;
 using System.Linq;
 using Microsoft.AspNet.Mvc;
 
@@ -63,6 +64,7 @@ namespace ModelBindingWebSite.Controllers
 
         public class BlogPost
         {
+            [Required]
             [FromHeader]
             public string Title { get; set; }