diff --git a/Mvc.sln b/Mvc.sln index 82f2d7b3b8..3e4c137962 100644 --- a/Mvc.sln +++ b/Mvc.sln @@ -1,7 +1,7 @@  Microsoft Visual Studio Solution File, Format Version 12.00 # Visual Studio 14 -VisualStudioVersion = 14.0.22810.0 +VisualStudioVersion = 14.0.22808.1 MinimumVisualStudioVersion = 10.0.40219.1 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "samples", "samples", "{DAAE4C74-D06F-4874-A166-33305D2643CE}" EndProject @@ -166,6 +166,8 @@ Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Microsoft.AspNet.Mvc.ApiExp EndProject Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Microsoft.AspNet.Mvc.ApiExplorer.Test", "test\Microsoft.AspNet.Mvc.ApiExplorer.Test\Microsoft.AspNet.Mvc.ApiExplorer.Test.xproj", "{4C2AD8AB-8AC0-46C4-80C6-C5577C7255F6}" EndProject +Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Microsoft.AspNet.Mvc.Abstractions.Test", "test\Microsoft.AspNet.Mvc.Abstractions.Test\Microsoft.AspNet.Mvc.Abstractions.Test.xproj", "{DA000953-7532-4DF5-8DB9-8143DF98D999}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -996,6 +998,18 @@ Global {4C2AD8AB-8AC0-46C4-80C6-C5577C7255F6}.Release|Mixed Platforms.Build.0 = Release|Any CPU {4C2AD8AB-8AC0-46C4-80C6-C5577C7255F6}.Release|x86.ActiveCfg = Release|Any CPU {4C2AD8AB-8AC0-46C4-80C6-C5577C7255F6}.Release|x86.Build.0 = Release|Any CPU + {DA000953-7532-4DF5-8DB9-8143DF98D999}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {DA000953-7532-4DF5-8DB9-8143DF98D999}.Debug|Any CPU.Build.0 = Debug|Any CPU + {DA000953-7532-4DF5-8DB9-8143DF98D999}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU + {DA000953-7532-4DF5-8DB9-8143DF98D999}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU + {DA000953-7532-4DF5-8DB9-8143DF98D999}.Debug|x86.ActiveCfg = Debug|Any CPU + {DA000953-7532-4DF5-8DB9-8143DF98D999}.Debug|x86.Build.0 = Debug|Any CPU + {DA000953-7532-4DF5-8DB9-8143DF98D999}.Release|Any CPU.ActiveCfg = Release|Any CPU + {DA000953-7532-4DF5-8DB9-8143DF98D999}.Release|Any CPU.Build.0 = Release|Any CPU + {DA000953-7532-4DF5-8DB9-8143DF98D999}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU + {DA000953-7532-4DF5-8DB9-8143DF98D999}.Release|Mixed Platforms.Build.0 = Release|Any CPU + {DA000953-7532-4DF5-8DB9-8143DF98D999}.Release|x86.ActiveCfg = Release|Any CPU + {DA000953-7532-4DF5-8DB9-8143DF98D999}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -1077,5 +1091,6 @@ Global {1154203C-7579-4525-906E-BC55268421C1} = {32285FA4-6B46-4D6B-A840-2B13E4C8B58E} {A2B72833-5D70-4C42-AE85-E0319926FB8A} = {32285FA4-6B46-4D6B-A840-2B13E4C8B58E} {4C2AD8AB-8AC0-46C4-80C6-C5577C7255F6} = {3BA657BF-28B1-42DA-B5B0-1C4601FCF7B1} + {DA000953-7532-4DF5-8DB9-8143DF98D999} = {3BA657BF-28B1-42DA-B5B0-1C4601FCF7B1} EndGlobalSection EndGlobal diff --git a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelStateDictionary.cs b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelStateDictionary.cs index c0a822a7fc..abca9b58a5 100644 --- a/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelStateDictionary.cs +++ b/src/Microsoft.AspNet.Mvc.Abstractions/ModelBinding/ModelStateDictionary.cs @@ -281,7 +281,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding /// state errors; otherwise. public ModelValidationState GetFieldValidationState([NotNull] string key) { - var entries = FindKeysWithPrefix(this, key); + var entries = FindKeysWithPrefix(key); if (!entries.Any()) { return ModelValidationState.Unvalidated; @@ -378,7 +378,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding // If key is null or empty, clear all entries in the dictionary // else just clear the ones that have key as prefix var entries = (string.IsNullOrEmpty(key)) ? - _innerDictionary : FindKeysWithPrefix(this, key); + _innerDictionary : FindKeysWithPrefix(key); foreach (var entry in entries) { @@ -502,17 +502,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding return GetEnumerator(); } - private static IEnumerable> FindKeysWithPrefix( - [NotNull] IDictionary dictionary, - [NotNull] string prefix) + public IEnumerable> FindKeysWithPrefix([NotNull] string prefix) { - TValue exactMatchValue; - if (dictionary.TryGetValue(prefix, out exactMatchValue)) + ModelState exactMatchValue; + if (_innerDictionary.TryGetValue(prefix, out exactMatchValue)) { - yield return new KeyValuePair(prefix, exactMatchValue); + yield return new KeyValuePair(prefix, exactMatchValue); } - foreach (var entry in dictionary) + foreach (var entry in _innerDictionary) { var key = entry.Key; @@ -521,19 +519,30 @@ namespace Microsoft.AspNet.Mvc.ModelBinding continue; } - if (key.StartsWith("[", StringComparison.OrdinalIgnoreCase)) - { - key = key.Substring(key.IndexOf('.') + 1); - if (string.Equals(prefix, key, StringComparison.Ordinal)) - { - yield return entry; - continue; - } - } - if (!key.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) { - continue; + + if (key.StartsWith("[", StringComparison.OrdinalIgnoreCase)) + { + var subKey = key.Substring(key.IndexOf('.') + 1); + + if (!subKey.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + if (string.Equals(prefix, subKey, StringComparison.Ordinal)) + { + yield return entry; + continue; + } + + key = subKey; + } + else + { + continue; + } } // Everything is prefixed by the empty string diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/DefaultObjectValidator.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/DefaultObjectValidator.cs index f2b534625b..571b17db05 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/DefaultObjectValidator.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/Validation/DefaultObjectValidator.cs @@ -66,12 +66,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation var currentValidationNode = validationContext.ValidationNode; if (currentValidationNode.SuppressValidation) { - // Short circuit if the node is marked to be suppressed - var validationState = modelState.GetFieldValidationState(modelKey); - if (validationState == ModelValidationState.Unvalidated) - { - modelValidationContext.ModelState.MarkFieldSkipped(modelKey); - } + // Short circuit if the node is marked to be suppressed. + // If there are any sub entries which were model bound, they need to be marked as skipped, + // Otherwise they will remain as unvalidated and the model state would be Invalid. + MarkChildNodesAsSkipped(modelKey, modelExplorer.Metadata, validationContext); // For validation purposes this model is valid. return true; @@ -103,7 +101,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation if (IsTypeExcludedFromValidation(_excludeFilters, modelType)) { var result = ShallowValidate(modelKey, modelExplorer, validationContext, validators); - MarkPropertiesAsSkipped(modelKey, modelExplorer.Metadata, validationContext); + + // If there are any sub entries which were model bound, they need to be marked as skipped, + // Otherwise they will remain as unvalidated and the model state would be Invalid. + MarkChildNodesAsSkipped(modelKey, modelExplorer.Metadata, validationContext); return result; } @@ -127,7 +128,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation return isValid; } - private void MarkPropertiesAsSkipped(string currentModelKey, ModelMetadata metadata, ValidationContext validationContext) + private void MarkChildNodesAsSkipped(string currentModelKey, ModelMetadata metadata, ValidationContext validationContext) { var modelState = validationContext.ModelValidationContext.ModelState; var fieldValidationState = modelState.GetFieldValidationState(currentModelKey); @@ -140,15 +141,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation return; } - foreach (var childMetadata in metadata.Properties) + // At this point we just want to mark all sub-entries present in the model state as skipped. + var entries = modelState.FindKeysWithPrefix(currentModelKey); + foreach (var entry in entries) { - var childKey = ModelNames.CreatePropertyModelName(currentModelKey, childMetadata.PropertyName); - var validationState = modelState.GetFieldValidationState(childKey); - - if (validationState == ModelValidationState.Unvalidated) - { - validationContext.ModelValidationContext.ModelState.MarkFieldSkipped(childKey); - } + entry.Value.ValidationState = ModelValidationState.Skipped; } } diff --git a/test/Microsoft.AspNet.Mvc.Abstractions.Test/ModelBinding/ModelStateDictionaryTest.cs b/test/Microsoft.AspNet.Mvc.Abstractions.Test/ModelBinding/ModelStateDictionaryTest.cs index 27e0b394ee..ea5d76fc7a 100644 --- a/test/Microsoft.AspNet.Mvc.Abstractions.Test/ModelBinding/ModelStateDictionaryTest.cs +++ b/test/Microsoft.AspNet.Mvc.Abstractions.Test/ModelBinding/ModelStateDictionaryTest.cs @@ -293,6 +293,45 @@ namespace Microsoft.AspNet.Mvc.ModelBinding Assert.Equal(ModelValidationState.Valid, validationState); } + [Theory] + [InlineData("[0].foo.bar")] + [InlineData("[0].foo.bar[0]")] + public void GetFieldValidationState_IndexedPrefix_ReturnsInvalidIfKeyChildContainsErrors(string key) + { + // Arrange + var msd = new ModelStateDictionary(); + msd.AddModelError(key, "error text"); + + // Act + var validationState = msd.GetFieldValidationState("[0].foo"); + + // Assert + Assert.Equal(ModelValidationState.Invalid, validationState); + } + + [Theory] + [InlineData("[0].foo.bar")] + [InlineData("[0].foo.bar[0]")] + public void GetFieldValidationState_IndexedPrefix_ReturnsValidIfModelStateDoesNotContainErrors(string key) + { + // Arrange + var validState = new ModelState + { + Value = new ValueProviderResult(null, null, null), + ValidationState = ModelValidationState.Valid + }; + var msd = new ModelStateDictionary + { + { key, validState } + }; + + // Act + var validationState = msd.GetFieldValidationState("[0].foo"); + + // Assert + Assert.Equal(ModelValidationState.Valid, validationState); + } + [Fact] public void IsValidPropertyReturnsFalseIfErrors() { diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Validation/DefaultObjectValidatorTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Validation/DefaultObjectValidatorTests.cs index 33e1525209..15eab7b499 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Validation/DefaultObjectValidatorTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/Validation/DefaultObjectValidatorTests.cs @@ -360,6 +360,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation .Validate(validationContext, topLevelValidationNode); // Assert + Assert.Equal(1, validationContext.ModelState.Count); Assert.Contains("Street", validationContext.ModelState.Keys); var streetState = validationContext.ModelState["Street"]; Assert.Equal(2, streetState.Errors.Count); @@ -427,7 +428,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation validator.Validate(validationContext, topLevelValidationNode); // Assert - Assert.Equal(new[] { "key1", "user.Password", "", "user.ConfirmPassword" }, + Assert.Equal(new[] { "key1", "", "user.ConfirmPassword" }, validationContext.ModelState.Keys.ToArray()); var modelState = validationContext.ModelState["user.ConfirmPassword"]; Assert.Empty(modelState.Errors); @@ -438,7 +439,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation } [Fact] - public void ForExcludedNonModelBoundType_Properties_NotMarkedAsSkiped() + public void ForExcludedNonModelBoundTypes_NoEntryInModelState() { // Arrange var user = new User() @@ -468,11 +469,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation validator.Validate(validationContext, topLevelValidationNode); // Assert - Assert.False(validationContext.ModelState.ContainsKey("user.Password")); - Assert.False(validationContext.ModelState.ContainsKey("user.ConfirmPassword")); - var modelState = validationContext.ModelState["user"]; - Assert.Empty(modelState.Errors); - Assert.Equal(modelState.ValidationState, ModelValidationState.Valid); + Assert.True(validationContext.ModelState.IsValid); + Assert.Empty(validationContext.ModelState); } [Fact] @@ -511,13 +509,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation validator.Validate(validationContext, topLevelValidationNode); // Assert - var modelState = validationContext.ModelState["user.Password"]; - Assert.Empty(modelState.Errors); - Assert.Equal(modelState.ValidationState, ModelValidationState.Skipped); + var entry = Assert.Single(validationContext.ModelState); + Assert.Equal("user.Password", entry.Key); + Assert.Empty(entry.Value.Errors); + Assert.Equal(entry.Value.ValidationState, ModelValidationState.Skipped); + } - modelState = validationContext.ModelState["user.ConfirmPassword"]; - Assert.Empty(modelState.Errors); - Assert.Equal(modelState.ValidationState, ModelValidationState.Skipped); + private class Person2 + { + public Address Address { get; set; } } [Fact] @@ -525,22 +525,28 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation { // Arrange var testValidationContext = GetModelValidationContext( - new TestServiceProvider(), - typeof(TestServiceProvider)); - + new Person2() + { + Address = new Address { Street = "GreaterThan5Characters" } + }, + typeof(Person2)); var validationContext = testValidationContext.ModelValidationContext; + + // Create an entry like a model binder would. + validationContext.ModelState.Add("person.Address", new ModelState()); + var validator = new DefaultObjectValidator( testValidationContext.ExcludeFilters, testValidationContext.ModelMetadataProvider); var modelExplorer = testValidationContext.ModelValidationContext.ModelExplorer; var topLevelValidationNode = new ModelValidationNode( - "serviceProvider", + "person", modelExplorer.Metadata, modelExplorer.Model); - var propertyExplorer = modelExplorer.GetExplorerForProperty("TestService"); + var propertyExplorer = modelExplorer.GetExplorerForProperty("Address"); var childNode = new ModelValidationNode( - "serviceProvider.TestService", + "person.Address", propertyExplorer.Metadata, propertyExplorer.Model) { @@ -554,8 +560,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation // Assert Assert.True(validationContext.ModelState.IsValid); - var modelState = validationContext.ModelState["serviceProvider.TestService"]; - Assert.Empty(modelState.Errors); + Assert.Equal(1, validationContext.ModelState.Count); + var modelState = validationContext.ModelState["person.Address"]; Assert.Equal(modelState.ValidationState, ModelValidationState.Skipped); } @@ -603,7 +609,14 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation // Assert Assert.True(validationContext.ModelState.IsValid); - var modelState = validationContext.ModelState["items"]; + Assert.Equal(3, validationContext.ModelState.Count); + var modelState = validationContext.ModelState["items[0]"]; + Assert.Equal(modelState.ValidationState, ModelValidationState.Valid); + + modelState = validationContext.ModelState["items[1]"]; + Assert.Equal(modelState.ValidationState, ModelValidationState.Valid); + + modelState = validationContext.ModelState["items[2]"]; Assert.Equal(modelState.ValidationState, ModelValidationState.Valid); } @@ -653,16 +666,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation // Assert Assert.True(validationContext.ModelState.IsValid); - var modelState = validationContext.ModelState["items"]; - Assert.Equal(modelState.ValidationState, ModelValidationState.Valid); - modelState = validationContext.ModelState["items[0].Key"]; - Assert.Equal(modelState.ValidationState, ModelValidationState.Skipped); + Assert.Equal(4, validationContext.ModelState.Count); + var modelState = validationContext.ModelState["items[0].Key"]; + Assert.Equal(ModelValidationState.Skipped, modelState.ValidationState); modelState = validationContext.ModelState["items[0].Value"]; - Assert.Equal(modelState.ValidationState, ModelValidationState.Skipped); + Assert.Equal(ModelValidationState.Skipped, modelState.ValidationState); modelState = validationContext.ModelState["items[1].Key"]; - Assert.Equal(modelState.ValidationState, ModelValidationState.Skipped); + Assert.Equal(ModelValidationState.Skipped, modelState.ValidationState); modelState = validationContext.ModelState["items[1].Value"]; - Assert.Equal(modelState.ValidationState, ModelValidationState.Skipped); + Assert.Equal(ModelValidationState.Skipped, modelState.ValidationState); } [Fact] @@ -690,8 +702,9 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation // Assert Assert.True(validationContext.ModelState.IsValid); - var key = Assert.Single(validationContext.ModelState.Keys); - Assert.Equal("person", key); + + // Since Person is not IValidatable and we do not look at its properties, the state is empty. + Assert.Empty(validationContext.ModelState.Keys); } [Fact] diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/InputObjectValidationTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/InputObjectValidationTests.cs index e1bab79caf..e7a2e35054 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/InputObjectValidationTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/InputObjectValidationTests.cs @@ -171,7 +171,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests } [Fact] - public async Task CheckIfExcludedField_IsValidatedForNonBodyBoundModels() + public async Task CheckIfExcludedField_IsNotValidatedForNonBodyBoundModels() { // Arrange var server = TestHelper.CreateServer(_app, SiteName, _configureServices); @@ -185,7 +185,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests //Assert Assert.Equal(HttpStatusCode.OK, response.StatusCode); - Assert.Equal("The Name field is required.", await response.Content.ReadAsStringAsync()); + Assert.Equal("xyz", await response.Content.ReadAsStringAsync()); } private class ErrorCollection diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs index e781c6d91a..91c55f95c5 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs @@ -11,7 +11,9 @@ using System.Reflection; using System.Text; using System.Threading.Tasks; using Microsoft.AspNet.Builder; +using Microsoft.AspNet.Mvc.ModelBinding; using Microsoft.Framework.DependencyInjection; +using ModelBindingWebSite.Controllers; using ModelBindingWebSite.Models; using ModelBindingWebSite.ViewModels; using Newtonsoft.Json; @@ -33,34 +35,12 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests var client = server.CreateClient(); // Act - var response = await client.GetAsync("http://localhost/Validation/DoNotValidateParameter"); + var response = await client.GetStringAsync("http://localhost/Validation/DoNotValidateParameter"); // Assert - var stringValue = await response.Content.ReadAsStringAsync(); - var isModelStateValid = JsonConvert.DeserializeObject(stringValue); - Assert.True(isModelStateValid); - } - - [Fact] - public async Task TypeBasedExclusion_ForBodyAndNonBodyBoundModels() - { - // Arrange - var server = TestHelper.CreateServer(_app, SiteName, _configureServices); - var client = server.CreateClient(); - - // Make sure the body object gets created with an invalid zip. - var input = "{\"OfficeAddress.Zip\":\"45\"}"; - var content = new StringContent(input, Encoding.UTF8, "application/json"); - - // Act - // Make sure the non body based object gets created with an invalid zip. - var response = await client.PostAsync( - "http://localhost/Validation/SkipValidation?ShippingAddresses[0].Zip=45&HomeAddress.Zip=46", content); - - // Assert - var stringValue = await response.Content.ReadAsStringAsync(); - var isModelStateValid = JsonConvert.DeserializeObject(stringValue); - Assert.True(isModelStateValid); + var modelState = JsonConvert.DeserializeObject(response); + Assert.Empty(modelState); + Assert.True(modelState.IsValid); } [Fact] @@ -76,8 +56,8 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests // Assert var stringValue = await response.Content.ReadAsStringAsync(); - var isModelStateValid = JsonConvert.DeserializeObject(stringValue); - Assert.True(isModelStateValid); + var json = JsonConvert.DeserializeObject(stringValue); + Assert.True(json.IsValid); } [Theory] diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs index 23f7c529aa..458de1dfe0 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/BodyValidationIntegrationTests.cs @@ -45,7 +45,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var operationContext = ModelBindingTestHelper.GetOperationBindingContext( request => { - request.Body = new MemoryStream(Encoding.UTF8.GetBytes("{ \"Id\":1234 }")); + request.Body = new MemoryStream(Encoding.UTF8.GetBytes(string.Empty)); request.ContentType = "application/json"; }); @@ -67,7 +67,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests } [Fact] - public async Task FromBodyOnActionParameter_EmptyBody_AddsModelStateError() + public async Task FromBodyOnActionParameter_EmptyBody_BindsToNullValue() { // Arrange var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); @@ -85,7 +85,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var operationContext = ModelBindingTestHelper.GetOperationBindingContext( request => { - request.Body = new MemoryStream(Encoding.UTF8.GetBytes("{ \"Id\":1234 }")); + request.Body = new MemoryStream(Encoding.UTF8.GetBytes(string.Empty)); request.ContentType = "application/json"; }); @@ -98,13 +98,9 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests // Assert Assert.NotNull(modelBindingResult); Assert.True(modelBindingResult.IsModelSet); - var boundPerson = Assert.IsType(modelBindingResult.Model); - Assert.NotNull(boundPerson); - var key = Assert.Single(modelState.Keys); - Assert.Equal("Address", key); - Assert.False(modelState.IsValid); - var error = Assert.Single(modelState[key].Errors); - Assert.Equal("The Address field is required.",error.ErrorMessage); + Assert.Null(modelBindingResult.Model); + Assert.Empty(modelState.Keys); + Assert.True(modelState.IsValid); } private class Person4 diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs index 167cea94eb..30a25e1dd0 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; using System.IO; using System.Text; using System.Threading.Tasks; @@ -597,5 +598,60 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.Equal("Street2", entry.Value.AttemptedValue); Assert.Equal("Street2", entry.Value.RawValue); } + + private class Person5 + { + public IList Addresses { get; set; } + } + + private class Address5 + { + public int Zip { get; set; } + + [StringLength(3)] + public string Street { get; set; } + } + + [Fact] + public async Task CollectionModelBinder_UsesCustomIndexes_AddsErrorsWithCorrectKeys() + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(Person5) + }; + + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + var formCollection = new FormCollection(new Dictionary() + { + { "Addresses.index", new [] { "Key1" } }, + { "Addresses[Key1].Street", new [] { "Street1" } }, + }); + + request.Form = formCollection; + request.ContentType = "application/x-www-form-urlencoded"; + }); + + var modelState = new ModelStateDictionary(); + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext); + + // Assert + Assert.NotNull(modelBindingResult); + Assert.True(modelBindingResult.IsModelSet); + Assert.IsType(modelBindingResult.Model); + + Assert.Equal(1, modelState.Count); + Assert.Equal(1, modelState.ErrorCount); + Assert.False(modelState.IsValid); + + var entry = Assert.Single(modelState, kvp => kvp.Key == "Addresses[Key1].Street").Value; + var error = Assert.Single(entry.Errors); + Assert.Equal("The field Street must be a string with a maximum length of 3.", error.ErrorMessage); + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/ModelBindingTestHelper.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/ModelBindingTestHelper.cs index 76f2d124e0..65ec766e19 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/ModelBindingTestHelper.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/ModelBindingTestHelper.cs @@ -49,21 +49,21 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests }; } - public static DefaultControllerActionArgumentBinder GetArgumentBinder() + public static DefaultControllerActionArgumentBinder GetArgumentBinder(MvcOptions options = null) { var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); return new DefaultControllerActionArgumentBinder( metadataProvider, - GetObjectValidator()); + GetObjectValidator(options)); } - public static IObjectModelValidator GetObjectValidator() + public static IObjectModelValidator GetObjectValidator(MvcOptions options = null) { - var options = new TestMvcOptions(); - options.Options.MaxModelValidationErrors = 5; + options = options ?? new TestMvcOptions().Options; + options.MaxModelValidationErrors = 5; var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); return new DefaultObjectValidator( - options.Options.ValidationExcludeFilters, + options.ValidationExcludeFilters, metadataProvider); } diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs index c8f3d1407d..6d93979e37 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs @@ -273,7 +273,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests public IScopedInstance BindingContext { get; set; } } - [Fact(Skip = "FromServices should not have an entry in model state #2464.")] + [Fact] public async Task MutableObjectModelBinder_BindsNestedPOCO_WithServicesModelBinder_WithPrefix_Success() { // Arrange @@ -313,7 +313,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.Equal("bill", entry.Value.RawValue); } - [Fact(Skip = "FromServices should not have an entry in model state #2464.")] + [Fact] public async Task MutableObjectModelBinder_BindsNestedPOCO_WithServicesModelBinder_WithEmptyPrefix_Success() { // Arrange @@ -355,7 +355,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 = "FromServices should not have an entry in model state #2464.")] + [Fact] public async Task MutableObjectModelBinder_BindsNestedPOCO_WithServicesModelBinder_WithPrefix_PartialData() { // Arrange @@ -427,7 +427,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var model = Assert.IsType(modelBindingResult.Model); Assert.Null(model.Customer); - Assert.Equal(0, modelState.Count); // Fails due to #2464 + Assert.Equal(0, modelState.Count); Assert.Equal(0, modelState.ErrorCount); Assert.True(modelState.IsValid); } diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs index e37516da6b..7cfb29dcd7 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs @@ -60,15 +60,10 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests // ModelState Assert.True(modelState.IsValid); - - Assert.Equal(1, modelState.Keys.Count); - var key = Assert.Single(modelState.Keys, k => k == "CustomParameter.Address.OutputFormatter"); - Assert.Equal(ModelValidationState.Skipped, modelState[key].ValidationState); - Assert.Null(modelState[key].Value); - Assert.Empty(modelState[key].Errors); + Assert.Empty(modelState.Keys); } - [Fact(Skip = "Should be no entry for model bound using services. #2464")] + [Fact] public async Task BindPropertyFromService_WithData_WithEmptyPrefix_GetsBound() { // Arrange @@ -99,13 +94,12 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests // ModelState Assert.True(modelState.IsValid); - Assert.Equal(1, modelState.Keys.Count); - var key = Assert.Single(modelState.Keys, k => k == "Address"); - Assert.Null(modelState[key].Value); // For non user bound models there should be no value. - Assert.Empty(modelState[key].Errors); + + // For non user bound models there should be no entry in model state. + Assert.Empty(modelState); } - [Fact(Skip = "#2464 ModelState should not have entry for non request bound models.")] + [Fact] public async Task BindParameterFromService_WithData_GetsBound() { // Arrange diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/ValidationIntegrationTests.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/ValidationIntegrationTests.cs index b84112dcef..6b832ca281 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/ValidationIntegrationTests.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/ValidationIntegrationTests.cs @@ -3,7 +3,9 @@ using System.Collections.Generic; using System.ComponentModel.DataAnnotations; +using System.IO; using System.Linq; +using System.Text; using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Mvc.ModelBinding; @@ -1005,6 +1007,82 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.False(modelState.IsValid); } + private class Order11 + { + public IEnumerable
ShippingAddresses { get; set; } + + public Address HomeAddress { get; set; } + + [FromBody] + public Address OfficeAddress { get; set; } + } + + private class Address + { + public int Street { get; set; } + public string State { get; set; } + + [Range(10000, 99999)] + public int Zip { get; set; } + + public Country Country { get; set; } + } + + private class Country + { + public string Name { get; set; } + } + [Fact] + public async Task TypeBasedExclusion_ForBodyAndNonBodyBoundModels() + { + // Arrange + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(Order11) + }; + + MvcOptions testOptions = null; + var input = "{\"OfficeAddress.Zip\":\"45\"}"; + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.QueryString = + new QueryString("?HomeAddress.Country.Name=US&ShippingAddresses[0].Zip=45&HomeAddress.Zip=46"); + request.Body = new MemoryStream(Encoding.UTF8.GetBytes(input)); + request.ContentType = "application/json"; + }, + options => { + + options.ValidationExcludeFilters.Add(typeof(Address)); + testOptions = options; + }); + + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(testOptions); + var modelState = new ModelStateDictionary(); + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext); + + Assert.Equal(3, modelState.Count); + Assert.Equal(0, modelState.ErrorCount); + Assert.True(modelState.IsValid); + + var entry = Assert.Single(modelState, e => e.Key == "HomeAddress.Country.Name").Value; + Assert.Equal("US", entry.Value.AttemptedValue); + Assert.Equal("US", entry.Value.RawValue); + Assert.Equal(ModelValidationState.Skipped, entry.ValidationState); + + entry = Assert.Single(modelState, e => e.Key == "ShippingAddresses[0].Zip").Value; + Assert.Equal("45", entry.Value.AttemptedValue); + Assert.Equal("45", entry.Value.RawValue); + Assert.Equal(ModelValidationState.Skipped, entry.ValidationState); + + entry = Assert.Single(modelState, e => e.Key == "HomeAddress.Zip").Value; + Assert.Equal("46", entry.Value.AttemptedValue); + Assert.Equal("46", entry.Value.RawValue); + Assert.Equal(ModelValidationState.Skipped, entry.ValidationState); + } + private static void AssertRequiredError(string key, ModelError error) { Assert.Equal(string.Format("The {0} field is required.", key), error.ErrorMessage); diff --git a/test/WebSites/FormatterWebSite/Controllers/ValidationController.cs b/test/WebSites/FormatterWebSite/Controllers/ValidationController.cs index 6994af1378..452b440f06 100644 --- a/test/WebSites/FormatterWebSite/Controllers/ValidationController.cs +++ b/test/WebSites/FormatterWebSite/Controllers/ValidationController.cs @@ -46,7 +46,6 @@ namespace FormatterWebSite [HttpPost] public string GetDeveloperAlias(Developer developer) { - // Since validation exclusion is currently only effective in case of body bound models. if (ModelState.IsValid) { return developer.Alias; diff --git a/test/WebSites/ModelBindingWebSite/Controllers/ValidationController.cs b/test/WebSites/ModelBindingWebSite/Controllers/ValidationController.cs index db32d0bbd1..99f6b4904e 100644 --- a/test/WebSites/ModelBindingWebSite/Controllers/ValidationController.cs +++ b/test/WebSites/ModelBindingWebSite/Controllers/ValidationController.cs @@ -14,22 +14,60 @@ namespace ModelBindingWebSite.Controllers [FromServices] public ITestService ControllerService { get; set; } - public bool SkipValidation(Resident resident) + public object AvoidRecursive(SelfishPerson selfishPerson) { - return ModelState.IsValid; + return new SerializableModelStateDictionary(ModelState); } - public bool AvoidRecursive(SelfishPerson selfishPerson) + public object DoNotValidateParameter([FromServices] ITestService service) { - return ModelState.IsValid; - } - - public bool DoNotValidateParameter([FromServices] ITestService service) - { - return ModelState.IsValid; + return ModelState; } } + public class SerializableModelStateDictionary : Dictionary + { + public bool IsValid { get; set; } + + public int ErrorCount { get; set; } + + public SerializableModelStateDictionary(ModelStateDictionary modelState) + { + var errorCount = 0; + foreach (var keyModelStatePair in modelState) + { + var key = keyModelStatePair.Key; + var value = keyModelStatePair.Value; + errorCount += value.Errors.Count; + var entry = new Entry() + { + Errors = value.Errors, + RawValue = value.Value.RawValue, + AttemptedValue = value.Value.AttemptedValue, + ValidationState = value.ValidationState + }; + + Add(key, entry); + } + + IsValid = modelState.IsValid; + ErrorCount = errorCount; + } + } + + public class Entry + { + public ModelValidationState ValidationState { get; set; } + + public ModelErrorCollection Errors { get; set; } + + public object RawValue { get; set; } + + public string AttemptedValue { get; set; } + + } + + public class SelfishPerson { public string Name { get; set; }