diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CompositeModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CompositeModelBinder.cs index d5e87ea7ed..58607e18ae 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/CompositeModelBinder.cs +++ b/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, diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/HeaderModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/HeaderModelBinder.cs index c63d224f92..60725aeaf7 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/HeaderModelBinder.cs +++ b/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); } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs index a23cfaf57c..9447deb58d 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/Binders/MutableObjectModelBinder.cs +++ b/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); } } diff --git a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingContext.cs b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingContext.cs index b8448eb8ef..ff987e841a 100644 --- a/src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingContext.cs +++ b/src/Microsoft.AspNet.Mvc.ModelBinding/ModelBindingContext.cs @@ -71,11 +71,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } set { + IsModelSet = true; + EnsureModelMetadata(); ModelMetadata.Model = value; } } + /// + /// Gets or sets a value indicating whether or not the 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 null value. + /// + public bool IsModelSet { get; set; } + /// /// Gets or sets the metadata for the model associated with this context. /// diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingFromHeaderTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingFromHeaderTest.cs index 437e72f10f..741bb9481d 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingFromHeaderTest.cs +++ b/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(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; } diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTests.cs index c66f456d94..a71b41e1b6 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTests.cs +++ b/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>>(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] diff --git a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CompositeModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CompositeModelBinderTest.cs index cd93e04736..4a69e3b988 100644 --- a/test/Microsoft.AspNet.Mvc.ModelBinding.Test/Binders/CompositeModelBinderTest.cs +++ b/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)), + ModelName = "someName", + ModelState = new ModelStateDictionary(), + ValueProvider = new SimpleHttpValueProvider + { + { "someOtherName", "dummyValue" } + }, + OperationBindingContext = new OperationBindingContext + { + ValidatorProvider = GetValidatorProvider() + } + }; + + var modelBinder = new Mock(); + modelBinder + .Setup(mb => mb.BindModelAsync(It.IsAny())) + .Callback(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)), + ModelName = "someName", + ModelState = new ModelStateDictionary(), + ValueProvider = new SimpleHttpValueProvider + { + { "someOtherName", "dummyValue" } + }, + OperationBindingContext = new OperationBindingContext + { + ValidatorProvider = GetValidatorProvider() + } + }; + + var modelBinder = new Mock(); + modelBinder + .Setup(mb => mb.BindModelAsync(It.IsAny())) + .Callback(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() { diff --git a/test/WebSites/ModelBindingWebSite/Controllers/FromHeader_BlogController.cs b/test/WebSites/ModelBindingWebSite/Controllers/FromHeader_BlogController.cs index 4eb67c6a15..eafaf0bc4d 100644 --- a/test/WebSites/ModelBindingWebSite/Controllers/FromHeader_BlogController.cs +++ b/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; }