diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs index ed804246b3..a8b71f9e84 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs @@ -81,24 +81,21 @@ namespace Microsoft.AspNet.Mvc operationContext); var modelBindingResult = await operationContext.ModelBinder.BindModelAsync(modelBindingContext); - if (modelBindingResult != null && modelBindingResult.IsModelSet) + if (modelBindingResult != null && + modelBindingResult.IsModelSet && + modelBindingResult.ValidationNode != null) { - var key = modelBindingResult.Key; var modelExplorer = new ModelExplorer( _modelMetadataProvider, metadata, modelBindingResult.Model); + var validationContext = new ModelValidationContext( + modelBindingContext.BindingSource, + operationContext.ValidatorProvider, + modelState, + modelExplorer); - if (modelBindingResult.ValidationNode != null) - { - var validationContext = new ModelValidationContext( - modelBindingContext.BindingSource, - operationContext.ValidatorProvider, - modelState, - modelExplorer); - - _validator.Validate(validationContext, modelBindingResult.ValidationNode); - } + _validator.Validate(validationContext, modelBindingResult.ValidationNode); } return modelBindingResult; diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ComplexModelDtoModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ComplexModelDtoModelBinder.cs index 00ef73c64f..9314d5a591 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ComplexModelDtoModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ComplexModelDtoModelBinder.cs @@ -32,7 +32,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding await bindingContext.OperationBindingContext.ModelBinder.BindModelAsync(propertyBindingContext); if (modelBindingResult == null) { - // Could not bind. Add a result so MutableObjectModelBinder will check for [DefaultValue]. + // Could not bind. Let MutableObjectModelBinder know explicitly. dto.Results[propertyMetadata] = new ModelBindingResult(model: null, key: propertyModelName, isModelSet: false); } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BinderTypeBasedModelBinderModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BinderTypeBasedModelBinderModelBinderTest.cs index ea175c58de..3310ae6a2c 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BinderTypeBasedModelBinderModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BinderTypeBasedModelBinderModelBinderTest.cs @@ -15,7 +15,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test public class BinderTypeBasedModelBinderModelBinderTest { [Fact] - public async Task BindModel_ReturnsFalseIfNoBinderTypeIsSet() + public async Task BindModel_ReturnsNull_IfNoBinderTypeIsSet() { // Arrange var bindingContext = GetBindingContext(typeof(Person)); @@ -30,10 +30,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test } [Fact] - public async Task BindModel_ReturnsTrueEvenIfSelectedBinderReturnsFalse() + public async Task BindModel_ReturnsNotNull_EvenIfSelectedBinderReturnsNull() { // Arrange - var bindingContext = GetBindingContext(typeof(Person), binderType: typeof(FalseModelBinder)); + var bindingContext = GetBindingContext(typeof(Person), binderType: typeof(NullModelBinder)); var binder = new BinderTypeBasedModelBinder(); @@ -48,7 +48,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test public async Task BindModel_CallsBindAsync_OnProvidedModelBinder() { // Arrange - var bindingContext = GetBindingContext(typeof(Person), binderType: typeof(TrueModelBinder)); + var bindingContext = GetBindingContext(typeof(Person), binderType: typeof(NotNullModelBinder)); var model = new Person(); var serviceProvider = new ServiceCollection() @@ -121,7 +121,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test public int Age { get; set; } } - private class FalseModelBinder : IModelBinder + private class NullModelBinder : IModelBinder { public Task BindModelAsync(ModelBindingContext bindingContext) { @@ -129,11 +129,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test } } - private class TrueModelBinder : IModelBinder + private class NotNullModelBinder : IModelBinder { private readonly object _model; - public TrueModelBinder() + public NotNullModelBinder() { _model = new Person(); } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BindingSourceModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BindingSourceModelBinderTest.cs index 8f15ce3c3e..6bd764325f 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BindingSourceModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/BindingSourceModelBinderTest.cs @@ -27,20 +27,20 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // Act & Assert var exception = Assert.Throws( - () => new TestableBindingSourceModelBinder(bindingSource)); + () => new TestableBindingSourceModelBinder(bindingSource, isModelSet: false)); Assert.Equal(expected, exception.Message); } [Fact] - public async Task BindingSourceModelBinder_ReturnsFalse_WithNoSource() + public async Task BindingSourceModelBinder_ReturnsNull_WithNoSource() { // Arrange var context = new ModelBindingContext(); context.ModelMetadata = new EmptyModelMetadataProvider().GetMetadataForType(typeof(string)); - var binder = new TestableBindingSourceModelBinder(BindingSource.Body); + var binder = new TestableBindingSourceModelBinder(BindingSource.Body, isModelSet: false); - // Act + // Act var result = await binder.BindModelAsync(context); // Assert @@ -49,7 +49,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding } [Fact] - public async Task BindingSourceModelBinder_ReturnsFalse_NonMatchingSource() + public async Task BindingSourceModelBinder_ReturnsNull_NonMatchingSource() { // Arrange var provider = new TestModelMetadataProvider(); @@ -58,9 +58,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var context = new ModelBindingContext(); context.ModelMetadata = provider.GetMetadataForType(typeof(string)); - var binder = new TestableBindingSourceModelBinder(BindingSource.Body); + var binder = new TestableBindingSourceModelBinder(BindingSource.Body, isModelSet: false); - // Act + // Act var result = await binder.BindModelAsync(context); // Assert @@ -68,8 +68,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding Assert.False(binder.WasBindModelCoreCalled); } - [Fact] - public async Task BindingSourceModelBinder_ReturnsTrue_MatchingSource() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task BindingSourceModelBinder_ReturnsNonNull_MatchingSource(bool isModelSet) { // Arrange var provider = new TestModelMetadataProvider(); @@ -82,30 +84,35 @@ namespace Microsoft.AspNet.Mvc.ModelBinding BinderModelName = modelMetadata.BinderModelName }; - var binder = new TestableBindingSourceModelBinder(BindingSource.Body); + var binder = new TestableBindingSourceModelBinder(BindingSource.Body, isModelSet); - // Act + // Act var result = await binder.BindModelAsync(context); // Assert Assert.NotNull(result); - Assert.True(result.IsModelSet); + Assert.Equal(isModelSet, result.IsModelSet); + Assert.Null(result.Model); Assert.True(binder.WasBindModelCoreCalled); } private class TestableBindingSourceModelBinder : BindingSourceModelBinder { - public bool WasBindModelCoreCalled { get; private set; } + bool _isModelSet; - public TestableBindingSourceModelBinder(BindingSource source) + public TestableBindingSourceModelBinder(BindingSource source, bool isModelSet) : base(source) { + _isModelSet = isModelSet; } + public bool WasBindModelCoreCalled { get; private set; } + protected override Task BindModelCoreAsync([NotNull] ModelBindingContext bindingContext) { WasBindModelCoreCalled = true; - return Task.FromResult(new ModelBindingResult(null, bindingContext.ModelName, true)); + return Task.FromResult( + new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: _isModelSet)); } } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/ByteArrayModelBinderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/ByteArrayModelBinderTests.cs index 789dc10605..970b9f9069 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/ByteArrayModelBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/ByteArrayModelBinderTests.cs @@ -3,7 +3,6 @@ #if DNX451 using System; -using System.Linq; using System.Threading.Tasks; using Microsoft.AspNet.Testing; using Xunit; @@ -81,7 +80,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test } [Fact] - public async Task BindModelReturnsFalseWhenValueNotFound() + public async Task BindModel_ReturnsNull_WhenValueNotFound() { // Arrange var valueProvider = new SimpleHttpValueProvider() @@ -100,7 +99,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test } [Fact] - public async Task ByteArrayModelBinderReturnsFalseForOtherTypes() + public async Task BindModel_ReturnsNull_ForOtherTypes() { // Arrange var bindingContext = GetBindingContext(null, typeof(int[])); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CancellationTokenModelBinderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CancellationTokenModelBinderTests.cs index 075c133597..f69b2aa438 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CancellationTokenModelBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CancellationTokenModelBinderTests.cs @@ -12,7 +12,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test public class CancellationTokenModelBinderTests { [Fact] - public async Task CancellationTokenModelBinder_ReturnsTrue_ForCancellationTokenType() + public async Task CancellationTokenModelBinder_ReturnsNotNull_ForCancellationTokenType() { // Arrange var bindingContext = GetBindingContext(typeof(CancellationToken)); @@ -30,7 +30,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test [InlineData(typeof(int))] [InlineData(typeof(object))] [InlineData(typeof(CancellationTokenModelBinderTests))] - public async Task CancellationTokenModelBinder_ReturnsFalse_ForNonCancellationTokenType(Type t) + public async Task CancellationTokenModelBinder_ReturnsNull_ForNonCancellationTokenType(Type t) { // Arrange var bindingContext = GetBindingContext(t); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CompositeModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CompositeModelBinderTest.cs index 1927e48dc2..1195aa75a7 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CompositeModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CompositeModelBinderTest.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.ComponentModel.DataAnnotations; -using System.Linq; using System.Threading.Tasks; using Microsoft.AspNet.Mvc.ModelBinding.Validation; using Moq; @@ -268,10 +267,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test } [Fact] - public async Task ModelBinder_ReturnsTrue_SetsNullValue_SetsModelStateKey() + public async Task ModelBinder_ReturnsNotNull_SetsNullValue_SetsModelStateKey() { // Arrange - var bindingContext = new ModelBindingContext { FallbackToEmptyPrefix = true, @@ -291,7 +289,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var modelBinder = new Mock(); modelBinder .Setup(mb => mb.BindModelAsync(It.IsAny())) - .Returns(Task.FromResult(new ModelBindingResult(null, "someName", true))); + .Returns(Task.FromResult(new ModelBindingResult(model: null, key: "someName", isModelSet: true))); var composite = CreateCompositeBinder(modelBinder.Object); @@ -455,7 +453,6 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var result = await binder.BindModelAsync(bindingContext); // Assert - // The result is null because of issue #2473 Assert.Null(result); } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormValueProviderFactoryTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormValueProviderFactoryTests.cs index 1697f16e9d..de41590e31 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormValueProviderFactoryTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/FormValueProviderFactoryTests.cs @@ -33,8 +33,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test [Theory] [InlineData("application/x-www-form-urlencoded")] [InlineData("application/x-www-form-urlencoded;charset=utf-8")] - [InlineData("multipart/form-data")] - [InlineData("multipart/form-data;charset=utf-8")] + [InlineData("multipart/form-data; boundary=----WebKitFormBoundarymx2fSWqWSd0OxQqq")] + [InlineData("multipart/form-data; boundary=----WebKitFormBoundarymx2fSWqWSd0OxQqq; charset=utf-8")] public void GetValueProvider_ReturnsValueProviderInstanceWithInvariantCulture(string contentType) { // Arrange diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/HeaderModelBinderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/HeaderModelBinderTests.cs index 457d6073c1..3706737c70 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/HeaderModelBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/HeaderModelBinderTests.cs @@ -17,7 +17,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test [InlineData(typeof(int))] [InlineData(typeof(int[]))] [InlineData(typeof(BindingSource))] - public async Task BindModelAsync_ReturnsTrue_ForAllTypes(Type type) + public async Task BindModelAsync_ReturnsNotNull_ForAllTypes(Type type) { // Arrange var binder = new HeaderModelBinder(); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/KeyValuePairModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/KeyValuePairModelBinderTest.cs index b430df1ae1..d3dbd30d0e 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/KeyValuePairModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/KeyValuePairModelBinderTest.cs @@ -115,38 +115,27 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test Assert.Empty(validationNode.ChildNodes); } - [Fact] - public async Task TryBindStrongModel_BinderExists_BinderReturnsCorrectlyTypedObject_ReturnsTrue() - { - // Arrange - var bindingContext = GetBindingContext(new SimpleHttpValueProvider()); - var binder = new KeyValuePairModelBinder(); - var modelValidationNodeList = new List(); - - // Act - var result = await binder.TryBindStrongModel(bindingContext, "key", modelValidationNodeList); - - // Assert - Assert.True(result.IsModelSet); - Assert.Equal(42, result.Model); - Assert.Empty(bindingContext.ModelState); - } - - [Fact] - public async Task TryBindStrongModel_BinderExists_BinderReturnsIncorrectlyTypedObject_ReturnsTrue() + [Theory] + [InlineData(null, false)] + [InlineData(null, true)] + [InlineData(42, false)] + [InlineData(42, true)] + public async Task TryBindStrongModel_InnerBinderReturnsNotNull_ReturnsInnerBinderResult( + object model, + bool isModelSet) { // Arrange + var innerResult = new ModelBindingResult(model, key: string.Empty, isModelSet: isModelSet); var innerBinder = new Mock(); innerBinder .Setup(o => o.BindModelAsync(It.IsAny())) - .Returns((ModelBindingContext mbc) => + .Returns((ModelBindingContext context) => { - Assert.Equal("someName.key", mbc.ModelName); - return Task.FromResult(new ModelBindingResult(null, string.Empty, true)); + Assert.Equal("someName.key", context.ModelName); + return Task.FromResult(innerResult); }); var bindingContext = GetBindingContext(new SimpleHttpValueProvider(), innerBinder.Object); - var binder = new KeyValuePairModelBinder(); var modelValidationNodeList = new List(); @@ -154,8 +143,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test var result = await binder.TryBindStrongModel(bindingContext, "key", modelValidationNodeList); // Assert - Assert.True(result.IsModelSet); - Assert.Null(result.Model); + Assert.Same(innerResult, result); Assert.Empty(bindingContext.ModelState); } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/TypeConverterModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/TypeConverterModelBinderTest.cs index 2b967f8404..3c6e139c41 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/TypeConverterModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/TypeConverterModelBinderTest.cs @@ -15,7 +15,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test [InlineData(typeof(object))] [InlineData(typeof(Calendar))] [InlineData(typeof(TestClass))] - public async Task BindModel_ReturnsFalse_IfTypeCannotBeConverted(Type destinationType) + public async Task BindModel_ReturnsNull_IfTypeCannotBeConverted(Type destinationType) { // Arrange var bindingContext = GetBindingContext(destinationType); @@ -42,7 +42,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test [InlineData(typeof(DateTimeOffset))] [InlineData(typeof(double))] [InlineData(typeof(DayOfWeek))] - public async Task BindModel_ReturnsTrue_IfTypeCanBeConverted(Type destinationType) + public async Task BindModel_ReturnsNotNull_IfTypeCanBeConverted(Type destinationType) { if (TestPlatformHelper.IsMono && destinationType == typeof(DateTimeOffset)) @@ -92,7 +92,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test } [Fact] - public async Task BindModel_NullValueProviderResult_ReturnsFalse() + public async Task BindModel_NullValueProviderResult_ReturnsNull() { // Arrange var bindingContext = GetBindingContext(typeof(int)); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/TypeMatchModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/TypeMatchModelBinderTest.cs index b466ce0a71..0497e8ea9a 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/TypeMatchModelBinderTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/TypeMatchModelBinderTest.cs @@ -1,9 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. See License.txt in the project root for license information. using System; -using System.Collections.Generic; using System.Threading.Tasks; -using Microsoft.Framework.Internal; using Xunit; namespace Microsoft.AspNet.Mvc.ModelBinding.Test @@ -11,7 +9,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test public class TypeMatchModelBinderTest { [Fact] - public async Task BindModel_InvalidValueProviderResult_ReturnsFalse() + public async Task BindModel_InvalidValueProviderResult_ReturnsNull() { // Arrange var bindingContext = GetBindingContext(); @@ -33,7 +31,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test } [Fact] - public async Task BindModel_ValidValueProviderResult_ReturnsTrue() + public async Task BindModel_ValidValueProviderResult_ReturnsNotNull() { // Arrange var bindingContext = GetBindingContext(); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs index 2c557abc18..06bcd6682f 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs @@ -17,7 +17,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test public class ControllerActionArgumentBinderTests { [Fact] - public async Task BindActionArgumentsAsync_DoesNotAddActionArguments_IfBinderReturnsFalse() + public async Task BindActionArgumentsAsync_DoesNotAddActionArguments_IfBinderReturnsNull() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -91,7 +91,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test } [Fact] - public async Task BindActionArgumentsAsync_AddsActionArguments_IfBinderReturnsTrue() + public async Task BindActionArgumentsAsync_AddsActionArguments_IfBinderReturnsNotNull() { // Arrange Func method = foo => 1; @@ -114,7 +114,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test { context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(string)); }) - .Returns(Task.FromResult(result: new ModelBindingResult(value, "", true))); + .Returns(Task.FromResult(result: new ModelBindingResult(value, key: string.Empty, isModelSet: true))); var actionContext = new ActionContext( new DefaultHttpContext(), @@ -342,8 +342,10 @@ namespace Microsoft.AspNet.Mvc.Core.Test Assert.Null(controller.UntouchedProperty); } - [Fact] - public async Task BindActionArgumentsAsync_DoesNotSetNullValues_ForNonNullableProperties() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task BindActionArgumentsAsync_DoesNotSetNullValues_ForNonNullableProperties(bool isModelSet) { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -361,7 +363,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test binder .Setup(b => b.BindModelAsync(It.IsAny())) .Returns(Task.FromResult( - result: new ModelBindingResult(model: null, key: string.Empty, isModelSet: true))); + result: new ModelBindingResult(model: null, key: string.Empty, isModelSet: isModelSet))); var actionBindingContext = new ActionBindingContext() { ModelBinder = binder.Object, diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ModelBindingHelperTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ModelBindingHelperTest.cs index 6ce2d7ddd4..4b67017478 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ModelBindingHelperTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ModelBindingHelperTest.cs @@ -19,7 +19,7 @@ namespace Microsoft.AspNet.Mvc.Test public class ModelBindingHelperTest { [Fact] - public async Task TryUpdateModel_ReturnsFalse_IfBinderReturnsFalse() + public async Task TryUpdateModel_ReturnsFalse_IfBinderReturnsNull() { // Arrange var metadataProvider = new EmptyModelMetadataProvider(); @@ -28,7 +28,7 @@ namespace Microsoft.AspNet.Mvc.Test binder.Setup(b => b.BindModelAsync(It.IsAny())) .Returns(Task.FromResult(null)); var model = new MyModel(); - + // Act var result = await ModelBindingHelper.TryUpdateModelAsync( model, @@ -130,7 +130,7 @@ namespace Microsoft.AspNet.Mvc.Test } [Fact] - public async Task TryUpdateModel_UsingIncludePredicateOverload_ReturnsFalse_IfBinderReturnsFalse() + public async Task TryUpdateModel_UsingIncludePredicateOverload_ReturnsFalse_IfBinderReturnsNull() { // Arrange var metadataProvider = new EmptyModelMetadataProvider(); @@ -219,7 +219,7 @@ namespace Microsoft.AspNet.Mvc.Test } [Fact] - public async Task TryUpdateModel_UsingIncludeExpressionOverload_ReturnsFalse_IfBinderReturnsFalse() + public async Task TryUpdateModel_UsingIncludeExpressionOverload_ReturnsFalse_IfBinderReturnsNull() { // Arrange var metadataProvider = new EmptyModelMetadataProvider(); @@ -475,7 +475,7 @@ namespace Microsoft.AspNet.Mvc.Test } [Fact] - public async Task TryUpdateModelNonGeneric_PredicateOverload_ReturnsFalse_IfBinderReturnsFalse() + public async Task TryUpdateModelNonGeneric_PredicateOverload_ReturnsFalse_IfBinderReturnsNull() { // Arrange var metadataProvider = new EmptyModelMetadataProvider(); @@ -568,7 +568,7 @@ namespace Microsoft.AspNet.Mvc.Test } [Fact] - public async Task TryUpdateModelNonGeneric_ModelTypeOverload_ReturnsFalse_IfBinderReturnsFalse() + public async Task TryUpdateModelNonGeneric_ModelTypeOverload_ReturnsFalse_IfBinderReturnsNull() { // Arrange var metadataProvider = new EmptyModelMetadataProvider(); @@ -774,7 +774,7 @@ namespace Microsoft.AspNet.Mvc.Test dictionary["product.Category.Name"] = new ModelState { ValidationState = ModelValidationState.Valid }; dictionary["product.Order[0].Name"] = new ModelState { ValidationState = ModelValidationState.Invalid }; dictionary.AddModelError("product.Order[0].Name", "Order name invalid."); - dictionary["product.Order[0].Address.Street"] = + dictionary["product.Order[0].Address.Street"] = new ModelState { ValidationState = ModelValidationState.Invalid }; dictionary.AddModelError("product.Order[0].Address.Street", "Street invalid."); dictionary["product.Order[1].Name"] = new ModelState { ValidationState = ModelValidationState.Valid }; diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/BinderTypeBasedModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/BinderTypeBasedModelBinderIntegrationTest.cs index f38f7a749a..037e5fd059 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/BinderTypeBasedModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/BinderTypeBasedModelBinderIntegrationTest.cs @@ -1,7 +1,6 @@ // 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.Globalization; using System.Threading.Tasks; using Microsoft.AspNet.Mvc.ModelBinding; @@ -12,11 +11,9 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests { public class BinderTypeBasedModelBinderIntegrationTest { - // The NullModelBinder and NullModelNotSetModelBinder return a non null ModelBindingResult but a null model. - [Theory(Skip = "ModelBindingResult should be non null if a model binder returns a non null resul #2473.")] - [InlineData(typeof(NullModelBinder), true)] + [Fact] [InlineData(typeof(NullModelNotSetModelBinder), false)] - public async Task BindParameter_WithModelBinderType_NoData(Type modelBinderType, bool isModelSet) + public async Task BindParameter_WithModelBinderType_NullData_ReturnsNull() { // Arrange var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); @@ -25,14 +22,14 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Name = "Parameter1", BindingInfo = new BindingInfo() { - BinderType = modelBinderType + BinderType = typeof(NullModelBinder) }, ParameterType = typeof(string) }; // No data is passed. - var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => { }); + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(); var modelState = new ModelStateDictionary(); // Act @@ -42,27 +39,53 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests // ModelBindingResult Assert.NotNull(modelBindingResult); + Assert.True(modelBindingResult.IsModelSet); Assert.Null(modelBindingResult.Model); - Assert.Equal(isModelSet, modelBindingResult.IsModelSet); - // ModelState + // ModelState (not set unless inner binder sets it) Assert.True(modelState.IsValid); - var key = Assert.Single(modelState.Keys); - Assert.Equal("CustomParameter", key); - Assert.Equal(0, modelState.ErrorCount); - Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState); - Assert.Null(modelState[key].Value); // value is only set if the custom model binder sets it. + Assert.Empty(modelState); + } + + [Fact] + public async Task BindParameter_WithModelBinderType_NoData_ReturnsNull() + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor() + { + Name = "Parameter1", + BindingInfo = new BindingInfo() + { + BinderType = typeof(NullModelNotSetModelBinder) + }, + + ParameterType = typeof(string) + }; + + // No data is passed. + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(); + var modelState = new ModelStateDictionary(); + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext); + + // Assert + + // ModelBindingResult + Assert.Null(modelBindingResult); + + // ModelState (not set unless inner binder sets it) + Assert.True(modelState.IsValid); + Assert.Empty(modelState); } private class Person2 { } - // Since the NullResultModelBinder returns a null ModelBindingResult, it acts - // as a non greedy model binder, however since it is applied using a BinderTypeBasedModelBinder, - // which wraps this model binder and behaves as a greed model binder, we get a non null result. - [Fact(Skip = "ModelBindingResult should be non null if a model binder returns a non null resul #2473.")] - public async Task BindParameter_WithModelBinderType_NonGreedy_NoData() + [Fact] + public async Task BindParameter_WithModelBinderType_NonGreedy_NoData_ReturnsNull() { // Arrange var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); @@ -78,7 +101,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests }; // No data is passed. - var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => { }); + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(); var modelState = new ModelStateDictionary(); // Act @@ -87,14 +110,14 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests // Assert // ModelBindingResult - Assert.NotNull(modelBindingResult); + Assert.Null(modelBindingResult); // ModelState Assert.True(modelState.IsValid); Assert.Empty(modelState.Keys); } - - // ModelBinderAttribute can be used without specifing the binder type. + + // ModelBinderAttribute can be used without specifying the binder type. // In such cases BinderTypeBasedModelBinder acts like a non greedy binder where // it returns a null ModelBindingResult allowing other ModelBinders to run. [Fact] @@ -114,7 +137,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests }; // No data is passed. - var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => { }); + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(); var modelState = new ModelStateDictionary(); // Act @@ -149,7 +172,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests ParameterType = typeof(Person2) }; - var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => { }); + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(); var modelState = new ModelStateDictionary(); // Act @@ -193,7 +216,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests ParameterType = typeof(Person) }; - var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => { }); + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(); var modelState = new ModelStateDictionary(); // Act @@ -234,7 +257,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests ParameterType = typeof(Person) }; - var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => { }); + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(); var modelState = new ModelStateDictionary(); // Act diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/FormCollectionModelBindingIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/FormCollectionModelBindingIntegrationTest.cs index d5b7e7afeb..40cc235f06 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/FormCollectionModelBindingIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/FormCollectionModelBindingIntegrationTest.cs @@ -128,8 +128,8 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.Empty(modelState.Keys); } - [Fact(Skip = "FormCollection should not return null modelBindingResult for a type that matches. #2456")] - public async Task BindParameter_NoData_DoesNotGetBound() + [Fact] + public async Task BindParameter_NoData_BindsWithEmptyCollection() { // Arrange var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); @@ -158,11 +158,16 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests // ModelBindingResult Assert.NotNull(modelBindingResult); - Assert.Null(modelBindingResult.Model); + var collection = Assert.IsType(modelBindingResult.Model); // ModelState Assert.True(modelState.IsValid); Assert.Empty(modelState.Keys); + + // FormCollection + Assert.Empty(collection); + Assert.Empty(collection.Files); + Assert.Empty(collection.Keys); } private void UpdateRequest(HttpRequest request, string data, string name) @@ -172,7 +177,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var memoryStream = new MemoryStream(Encoding.UTF8.GetBytes(data)); request.Form = formCollection; - request.ContentType = "multipart/form-data"; + request.ContentType = "multipart/form-data; boundary=----WebKitFormBoundarymx2fSWqWSd0OxQqq"; request.Headers["Content-Disposition"] = "form-data; name=" + name + "; filename=text.txt"; fileCollection.Add(new FormFile(memoryStream, 0, data.Length) { diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/FormFileModelBindingIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/FormFileModelBindingIntegrationTest.cs index 561ebe0c1b..e5059d91bd 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/FormFileModelBindingIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/FormFileModelBindingIntegrationTest.cs @@ -125,7 +125,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.Empty(modelState.Keys); } - [Fact(Skip = "FormFile Should not return null modelBindingResult for a type that matches. #2456")] + [Fact] public async Task BindParameter_NoData_DoesNotGetBound() { // Arrange @@ -142,10 +142,8 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests }; // No data is passed. - var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => - { - request.ContentType = "multipart/form-data"; - }); + var operationContext = ModelBindingTestHelper.GetOperationBindingContext( + request => UpdateRequest(request, data: null, name: null)); var modelState = new ModelStateDictionary(); @@ -154,9 +152,8 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests // Assert - // ModelBindingResult - Assert.NotNull(modelBindingResult); // Fails due to bug #2456 - Assert.Null(modelBindingResult.Model); + // ModelBindingResult; expect null because binding failed. + Assert.Null(modelBindingResult); // ModelState Assert.True(modelState.IsValid); @@ -167,11 +164,19 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests { var fileCollection = new FormFileCollection(); var formCollection = new FormCollection(new Dictionary(), fileCollection); - var memoryStream = new MemoryStream(Encoding.UTF8.GetBytes(data)); request.Form = formCollection; - request.ContentType = "multipart/form-data"; + request.ContentType = "multipart/form-data; boundary=----WebKitFormBoundarymx2fSWqWSd0OxQqq"; + + if (string.IsNullOrEmpty(data) || string.IsNullOrEmpty(name)) + { + // Leave the submission empty. + return; + } + request.Headers["Content-Disposition"] = "form-data; name=" + name + "; filename=text.txt"; + + var memoryStream = new MemoryStream(Encoding.UTF8.GetBytes(data)); fileCollection.Add(new FormFile(memoryStream, 0, data.Length) { Headers = request.Headers diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs index 616564892f..3d92e4a5e0 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs @@ -41,7 +41,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests }; // Do not add any headers. - var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => { }); + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(); var modelState = new ModelStateDictionary(); // Act diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs index 6d93979e37..e21abd1e8e 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs @@ -397,7 +397,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests // We don't provide enough data in this test for the 'Person' model to be created. So even though there is // a [FromServices], it won't be used. - [Fact(Skip = "Extra entries in model state #2646.")] + [Fact] public async Task MutableObjectModelBinder_BindsNestedPOCO_WithServicesModelBinder_WithPrefix_NoData() { // Arrange @@ -2043,7 +2043,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var memoryStream = new MemoryStream(Encoding.UTF8.GetBytes(content)); request.Form = formCollection; - request.ContentType = "multipart/form-data"; + request.ContentType = "multipart/form-data; boundary=----WebKitFormBoundarymx2fSWqWSd0OxQqq"; request.Headers["Content-Disposition"] = "form-data; name=" + name + "; filename=text.txt"; fileCollection.Add(new FormFile(memoryStream, 0, memoryStream.Length)