diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CancellationTokenModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CancellationTokenModelBinder.cs index 716345c169..534bc53a20 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CancellationTokenModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CancellationTokenModelBinder.cs @@ -7,7 +7,7 @@ using System.Threading.Tasks; namespace Microsoft.AspNet.Mvc.ModelBinding { /// - /// Represents a model binder which can bind models of type . + /// implementation to bind models of type . /// public class CancellationTokenModelBinder : IModelBinder { @@ -18,7 +18,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { var model = bindingContext.OperationBindingContext.HttpContext.RequestAborted; var validationNode = - new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, model); + new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, model) + { + SuppressValidation = true, + }; + return Task.FromResult(new ModelBindingResult( model, bindingContext.ModelName, diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs index bb7993345f..06c093624c 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs @@ -62,6 +62,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } else { + if (valueProviderResult.RawValue == null) + { + // Value exists but is null. Handle similarly to fallback case above. This avoids a + // ModelBindingResult with IsModelSet = true but ValidationNode = null. + model = bindingContext.Model ?? CreateEmptyCollection(); + var validationNode = + new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, model); + + return new ModelBindingResult( + model, + bindingContext.ModelName, + isModelSet: true, + validationNode: validationNode); + } + result = await BindSimpleCollection( bindingContext, valueProviderResult.RawValue, @@ -83,7 +98,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding model, bindingContext.ModelName, isModelSet: true, - validationNode: result?.ValidationNode); + validationNode: result.ValidationNode); } /// @@ -135,11 +150,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding object rawValue, CultureInfo culture) { - if (rawValue == null) - { - return null; // nothing to do - } - var boundCollection = new List(); var metadataProvider = bindingContext.OperationBindingContext.MetadataProvider; diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs index 84eaa8f2d0..de3cbf4061 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormCollectionModelBinder.cs @@ -12,7 +12,7 @@ using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Mvc.ModelBinding { /// - /// Modelbinder to bind form values to . + /// implementation to bind form values to . /// public class FormCollectionModelBinder : IModelBinder { @@ -46,7 +46,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } var validationNode = - new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, model); + new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, model) + { + SuppressValidation = true, + }; + return new ModelBindingResult( model, bindingContext.ModelName, diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormFileModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormFileModelBinder.cs index d5f7edf1f3..de994ecbad 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormFileModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/FormFileModelBinder.cs @@ -15,7 +15,7 @@ using Microsoft.Net.Http.Headers; namespace Microsoft.AspNet.Mvc.ModelBinding { /// - /// Modelbinder to bind posted files to . + /// implementation to bind posted files to . /// public class FormFileModelBinder : IModelBinder { @@ -43,7 +43,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding if (value != null) { validationNode = - new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, value); + new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, value) + { + SuppressValidation = true, + }; var valueProviderResult = new ValueProviderResult(rawValue: value); bindingContext.ModelState.SetModelValue(bindingContext.ModelName, valueProviderResult); diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeConverterModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeConverterModelBinder.cs index af07c39073..bc3859e0e2 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeConverterModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/TypeConverterModelBinder.cs @@ -23,7 +23,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var valueProviderResult = await bindingContext.ValueProvider.GetValueAsync(bindingContext.ModelName); if (valueProviderResult == null) { - return null; // no entry + // no entry + return null; } object newModel; @@ -32,13 +33,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding { newModel = valueProviderResult.ConvertTo(bindingContext.ModelType); ModelBindingHelper.ReplaceEmptyStringWithNull(bindingContext.ModelMetadata, ref newModel); - var validationNode = new ModelValidationNode( - bindingContext.ModelName, - bindingContext.ModelMetadata, - newModel); var isModelSet = true; - // When converting newModel a null value may indicate a failed conversion for an otherwise required + // When converting newModel a null value may indicate a failed conversion for an otherwise required // model (can't set a ValueType to null). This detects if a null model value is acceptable given the // current bindingContext. If not, an error is logged. if (newModel == null && !AllowsNullValue(bindingContext.ModelType)) @@ -50,6 +47,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding isModelSet = false; } + // Include a ModelValidationNode if binding succeeded. + var validationNode = isModelSet ? + new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, newModel) : + null; + return new ModelBindingResult(newModel, bindingContext.ModelName, isModelSet, validationNode); } catch (Exception ex) diff --git a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/HttpRequestMessage/HttpRequestMessageModelBinder.cs b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/HttpRequestMessage/HttpRequestMessageModelBinder.cs index b9d754dddb..ef22afbae6 100644 --- a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/HttpRequestMessage/HttpRequestMessageModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/HttpRequestMessage/HttpRequestMessageModelBinder.cs @@ -7,14 +7,28 @@ using Microsoft.AspNet.Mvc.ModelBinding; namespace Microsoft.AspNet.Mvc.WebApiCompatShim { + /// + /// implementation to bind models of type . + /// public class HttpRequestMessageModelBinder : IModelBinder { + /// public Task BindModelAsync(ModelBindingContext bindingContext) { if (bindingContext.ModelType == typeof(HttpRequestMessage)) { var model = bindingContext.OperationBindingContext.HttpContext.GetHttpRequestMessage(); - return Task.FromResult(new ModelBindingResult(model, bindingContext.ModelName, isModelSet: true)); + var validationNode = + new ModelValidationNode(bindingContext.ModelName, bindingContext.ModelMetadata, model) + { + SuppressValidation = true, + }; + + return Task.FromResult(new ModelBindingResult( + model, + bindingContext.ModelName, + isModelSet: true, + validationNode: validationNode)); } return Task.FromResult(null); diff --git a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/WebApiCompatShimOptionsSetup.cs b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/WebApiCompatShimOptionsSetup.cs index 1fa40be83f..f900b52f0a 100644 --- a/src/Microsoft.AspNet.Mvc.WebApiCompatShim/WebApiCompatShimOptionsSetup.cs +++ b/src/Microsoft.AspNet.Mvc.WebApiCompatShim/WebApiCompatShimOptionsSetup.cs @@ -37,6 +37,7 @@ namespace Microsoft.AspNet.Mvc.WebApiCompatShim // Add a formatter to write out an HttpResponseMessage to the response options.OutputFormatters.Insert(0, new HttpResponseMessageOutputFormatter()); + options.ValidationExcludeFilters.Add(typeof(HttpRequestMessage)); options.ValidationExcludeFilters.Add(typeof(HttpResponseMessage)); } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CancellationTokenModelBinderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CancellationTokenModelBinderTests.cs index f69b2aa438..606804e1ae 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CancellationTokenModelBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CancellationTokenModelBinderTests.cs @@ -19,11 +19,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var binder = new CancellationTokenModelBinder(); // Act - var bound = await binder.BindModelAsync(bindingContext); + var result = await binder.BindModelAsync(bindingContext); // Assert - Assert.NotNull(bound); - Assert.Equal(bindingContext.OperationBindingContext.HttpContext.RequestAborted, bound.Model); + Assert.NotNull(result); + Assert.True(result.IsModelSet); + Assert.Equal(bindingContext.OperationBindingContext.HttpContext.RequestAborted, result.Model); + Assert.NotNull(result.ValidationNode); + Assert.True(result.ValidationNode.SuppressValidation); } [Theory] @@ -37,10 +40,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var binder = new CancellationTokenModelBinder(); // Act - var bound = await binder.BindModelAsync(bindingContext); + var result = await binder.BindModelAsync(bindingContext); // Assert - Assert.Null(bound); + Assert.Null(result); } private static ModelBindingContext GetBindingContext(Type modelType) diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs index 413b6687a4..6c62049eec 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs @@ -181,6 +181,33 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test Assert.True(modelState.IsValid); } + + [Fact] + public async Task BindModelAsync_SimpleCollectionWithNullValue_Succeeds() + { + // Arrange + var binder = new CollectionModelBinder(); + var valueProvider = new SimpleHttpValueProvider + { + { "someName", null }, + }; + var bindingContext = GetModelBindingContext(valueProvider, isReadOnly: false); + var modelState = bindingContext.ModelState; + + // Act + var result = await binder.BindModelAsync(bindingContext); + + // Assert + Assert.NotNull(result); + Assert.True(result.IsModelSet); + Assert.NotNull(result.Model); + Assert.NotNull(result.ValidationNode); + + var model = Assert.IsType>(result.Model); + Assert.Empty(model); + + Assert.True(modelState.IsValid); + } #endif [Fact] @@ -205,19 +232,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test Assert.Empty(boundCollection.Model); } - [Fact] - public async Task BindSimpleCollection_RawValueIsNull_ReturnsNull() - { - // Arrange - var binder = new CollectionModelBinder(); - - // Act - var boundCollection = await binder.BindSimpleCollection(bindingContext: null, rawValue: null, culture: null); - - // Assert - Assert.Null(boundCollection); - } - [Fact] public async Task CollectionModelBinder_DoesNotCreateCollection_IfIsTopLevelObjectAndIsFirstChanceBinding() { diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormCollectionModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormCollectionModelBinderTest.cs index 59946c1aab..9e6216a9e1 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormCollectionModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormCollectionModelBinderTest.cs @@ -34,6 +34,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Assert Assert.NotNull(result); + Assert.True(result.IsModelSet); + Assert.NotNull(result.ValidationNode); + Assert.True(result.ValidationNode.SuppressValidation); + var form = Assert.IsAssignableFrom(result.Model); Assert.Equal(2, form.Count); Assert.Equal("value1", form["field1"]); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormFileModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormFileModelBinderTest.cs index ba9c8552d2..29fb58d72c 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormFileModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormFileModelBinderTest.cs @@ -32,6 +32,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Assert Assert.NotNull(result); + Assert.True(result.IsModelSet); + Assert.NotNull(result.ValidationNode); + Assert.True(result.ValidationNode.SuppressValidation); + var files = Assert.IsAssignableFrom>(result.Model); Assert.Equal(2, files.Count); } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/TypeConverterModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/TypeConverterModelBinderTest.cs index 2a3206b938..b0a0a51abe 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/TypeConverterModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/TypeConverterModelBinderTest.cs @@ -90,7 +90,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test // Assert Assert.False(result.IsModelSet); - Assert.NotNull(result.ValidationNode); + Assert.Null(result.Model); + Assert.Null(result.ValidationNode); + var error = Assert.Single(bindingContext.ModelState["theModelName"].Errors); Assert.Equal(error.ErrorMessage, "The value '' is invalid.", StringComparer.Ordinal); Assert.Null(error.Exception); diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/FormFileModelBindingIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/FormFileModelBindingIntegrationTest.cs index f73456d4a7..89c21c0ed2 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/FormFileModelBindingIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/FormFileModelBindingIntegrationTest.cs @@ -73,7 +73,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var key = Assert.Single(modelState.Keys, k => k == "Address.File"); Assert.NotNull(modelState[key].Value); Assert.Empty(modelState[key].Errors); - Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState); + Assert.Equal(ModelValidationState.Skipped, modelState[key].ValidationState); } [Fact] @@ -121,7 +121,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var entry = Assert.Single(modelState); Assert.Equal("CustomParameter", entry.Key); Assert.Empty(entry.Value.Errors); - Assert.Equal(ModelValidationState.Valid, entry.Value.ValidationState); + Assert.Equal(ModelValidationState.Skipped, entry.Value.ValidationState); Assert.Null(entry.Value.Value.AttemptedValue); Assert.Equal(file, entry.Value.Value.RawValue); } diff --git a/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/HttpRequestMessage/HttpRequestMessageModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/HttpRequestMessage/HttpRequestMessageModelBinderTest.cs new file mode 100644 index 0000000000..47020514da --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.WebApiCompatShimTest/HttpRequestMessage/HttpRequestMessageModelBinderTest.cs @@ -0,0 +1,70 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Net.Http; +using System.Threading.Tasks; +using Microsoft.AspNet.Http.Internal; +using Microsoft.AspNet.Mvc.ModelBinding; +using Xunit; + +namespace Microsoft.AspNet.Mvc.WebApiCompatShim +{ + public class HttpRequestMessageModelBinderTest + { + [Fact] + public async Task BindModelAsync_ReturnsNotNull_ForHttpRequestMessageType() + { + // Arrange + var binder = new HttpRequestMessageModelBinder(); + var bindingContext = GetBindingContext(typeof(HttpRequestMessage)); + var expectedModel = bindingContext.OperationBindingContext.HttpContext.GetHttpRequestMessage(); + + // Act + var result = await binder.BindModelAsync(bindingContext); + + // Assert + Assert.NotNull(result); + Assert.True(result.IsModelSet); + Assert.Same(expectedModel, result.Model); + Assert.NotNull(result.ValidationNode); + Assert.True(result.ValidationNode.SuppressValidation); + } + + [Theory] + [InlineData(typeof(int))] + [InlineData(typeof(object))] + [InlineData(typeof(HttpRequestMessageModelBinderTest))] + public async Task BindModelAsync_ReturnsNull_ForNonHttpRequestMessageType(Type type) + { + // Arrange + var binder = new HttpRequestMessageModelBinder(); + var bindingContext = GetBindingContext(type); + + // Act + var result = await binder.BindModelAsync(bindingContext); + + // Assert + Assert.Null(result); + } + + private static ModelBindingContext GetBindingContext(Type modelType) + { + var metadataProvider = new EmptyModelMetadataProvider(); + ModelBindingContext bindingContext = new ModelBindingContext + { + ModelMetadata = metadataProvider.GetMetadataForType(modelType), + ModelName = "someName", + OperationBindingContext = new OperationBindingContext + { + HttpContext = new DefaultHttpContext(), + MetadataProvider = metadataProvider, + } + }; + + bindingContext.OperationBindingContext.HttpContext.Request.Method = "GET"; + + return bindingContext; + } + } +}