From 829a5c90460f60c9f1a449e3ec676d4e65a33e0c Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Tue, 25 Aug 2015 09:50:31 -0700 Subject: [PATCH] Expand model types `GenericModelBinder` can handle - #2993 - use `ClosedGenericMatcher` to handle e.g. non-generic model types implementing requested interfaces - reduce `IsAssignableFrom()` use since created binders use explicit casts i.e. handle explicit implementations - also add more integration tests covering various collection key formats, some with validation errors - merge a few `[Fact]`s into `[Theory]`s --- .../ModelBinding/GenericModelBinder.cs | 49 +- .../ArrayModelBinderIntegrationTest.cs | 4 +- .../CollectionModelBinderIntegrationTest.cs | 776 +++++++++++++++--- .../DictionaryModelBinderIntegrationTest.cs | 421 +++++++++- .../GenericModelBinderIntegrationTest.cs | 1 - 5 files changed, 1117 insertions(+), 134 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs index f325a5f558..7c8dcc2dbc 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs @@ -4,8 +4,11 @@ using System; using System.Collections.Generic; using System.Diagnostics; +#if DNXCORE50 using System.Reflection; +#endif using System.Threading.Tasks; +using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Mvc.ModelBinding { @@ -45,8 +48,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var modelType = context.ModelType; return GetArrayBinder(modelType) ?? - GetCollectionBinder(modelType) ?? GetDictionaryBinder(modelType) ?? + GetCollectionBinder(modelType) ?? GetEnumerableBinder(context) ?? GetKeyValuePairBinder(modelType); } @@ -58,6 +61,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding var elementType = modelType.GetElementType(); return typeof(ArrayModelBinder<>).MakeGenericType(elementType); } + return null; } @@ -99,9 +103,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding else { // A non-null instance must be updated in-place. For that the instance must also implement - // ICollection. For example an IEnumerable property may have a List default value. - var closedCollectionType = typeof(ICollection<>).MakeGenericType(modelTypeArguments); - if (!closedCollectionType.IsAssignableFrom(context.Model.GetType())) + // ICollection. For example an IEnumerable property may have a List default value. Do not use + // IsAssignableFrom() because that does not handle explicit interface implementations and binders all + // perform explicit casts. + var closedGenericInterface = + ClosedGenericMatcher.ExtractGenericInterface(context.Model.GetType(), typeof(ICollection<>)); + if (closedGenericInterface == null) { return null; } @@ -112,11 +119,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding private static Type GetKeyValuePairBinder(Type modelType) { - var modelTypeInfo = modelType.GetTypeInfo(); - if (modelTypeInfo.IsGenericType && - modelTypeInfo.GetGenericTypeDefinition() == typeof(KeyValuePair<,>)) + Debug.Assert(modelType != null); + + // Since KeyValuePair is a value type, ExtractGenericInterface() succeeds only on an exact match. + var closedGenericType = ClosedGenericMatcher.ExtractGenericInterface(modelType, typeof(KeyValuePair<,>)); + if (closedGenericType != null) { - return typeof(KeyValuePairModelBinder<,>).MakeGenericType(modelTypeInfo.GenericTypeArguments); + return typeof(KeyValuePairModelBinder<,>).MakeGenericType(modelType.GenericTypeArguments); } return null; @@ -144,28 +153,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding Debug.Assert(supportedInterfaceType != null); Debug.Assert(modelType != null); - var modelTypeInfo = modelType.GetTypeInfo(); - if (!modelTypeInfo.IsGenericType || modelTypeInfo.IsGenericTypeDefinition) - { - // modelType is not a closed generic type. - return null; - } + var closedGenericInterface = + ClosedGenericMatcher.ExtractGenericInterface(modelType, supportedInterfaceType); - var modelTypeArguments = modelTypeInfo.GenericTypeArguments; - if (modelTypeArguments.Length != supportedInterfaceType.GetTypeInfo().GenericTypeParameters.Length) - { - // Wrong number of generic type arguments. - return null; - } - - var closedInstanceType = supportedInterfaceType.MakeGenericType(modelTypeArguments); - if (!closedInstanceType.IsAssignableFrom(modelType)) - { - // modelType is not compatible with supportedInterfaceType. - return null; - } - - return modelTypeArguments; + return closedGenericInterface?.GenericTypeArguments; } } } diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/ArrayModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/ArrayModelBinderIntegrationTest.cs index aaeac5bfd5..06b1b5fb9f 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/ArrayModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/ArrayModelBinderIntegrationTest.cs @@ -164,7 +164,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.True(modelBindingResult.IsModelSet); Assert.Empty(Assert.IsType(modelBindingResult.Model)); - Assert.Equal(0, modelState.Count); + Assert.Empty(modelState); Assert.Equal(0, modelState.ErrorCount); Assert.True(modelState.IsValid); } @@ -330,7 +330,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.True(modelBindingResult.IsModelSet); Assert.Empty(Assert.IsType(modelBindingResult.Model)); - Assert.Equal(0, modelState.Count); + Assert.Empty(modelState); Assert.Equal(0, modelState.ErrorCount); Assert.True(modelState.IsValid); } diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs index 2ba4de5549..67bf5482db 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs @@ -1,10 +1,11 @@ // 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.Collections; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.ComponentModel.DataAnnotations; -using System.IO; -using System.Text; using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Http.Internal; @@ -59,8 +60,10 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.Equal("11", entry.RawValue); } - [Fact] - public async Task CollectionModelBinder_BindsListOfSimpleType_WithExplicitPrefix_Success() + [Theory] + [InlineData("?prefix[0]=10&prefix[1]=11")] + [InlineData("?prefix.index=low&prefix.index=high&prefix[low]=10&prefix[high]=11")] + public async Task CollectionModelBinder_BindsListOfSimpleType_WithExplicitPrefix_Success(string queryString) { // Arrange var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); @@ -76,7 +79,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => { - request.QueryString = new QueryString("?prefix[0]=10&prefix[1]=11"); + request.QueryString = new QueryString(queryString); }); var modelState = new ModelStateDictionary(); @@ -94,18 +97,12 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.Equal(2, modelState.Count); Assert.Equal(0, modelState.ErrorCount); Assert.True(modelState.IsValid); - - var entry = Assert.Single(modelState, kvp => kvp.Key == "prefix[0]").Value; - Assert.Equal("10", entry.AttemptedValue); - Assert.Equal("10", entry.RawValue); - - entry = Assert.Single(modelState, kvp => kvp.Key == "prefix[1]").Value; - Assert.Equal("11", entry.AttemptedValue); - Assert.Equal("11", entry.RawValue); } - [Fact] - public async Task CollectionModelBinder_BindsCollectionOfSimpleType_EmptyPrefix_Success() + [Theory] + [InlineData("?[0]=10&[1]=11")] + [InlineData("?index=low&index=high&[high]=11&[low]=10")] + public async Task CollectionModelBinder_BindsCollectionOfSimpleType_EmptyPrefix_Success(string queryString) { // Arrange var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); @@ -117,7 +114,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => { - request.QueryString = new QueryString("?[0]=10&[1]=11"); + request.QueryString = new QueryString(queryString); }); var modelState = new ModelStateDictionary(); @@ -135,14 +132,6 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.Equal(2, modelState.Count); Assert.Equal(0, modelState.ErrorCount); Assert.True(modelState.IsValid); - - var entry = Assert.Single(modelState, kvp => kvp.Key == "[0]").Value; - Assert.Equal("10", entry.AttemptedValue); - Assert.Equal("10", entry.RawValue); - - entry = Assert.Single(modelState, kvp => kvp.Key == "[1]").Value; - Assert.Equal("11", entry.AttemptedValue); - Assert.Equal("11", entry.RawValue); } [Fact] @@ -171,7 +160,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.True(modelBindingResult.IsModelSet); Assert.Empty(Assert.IsType>(modelBindingResult.Model)); - Assert.Equal(0, modelState.Count); + Assert.Empty(modelState); Assert.Equal(0, modelState.ErrorCount); Assert.True(modelState.IsValid); } @@ -181,8 +170,12 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests public int Id { get; set; } } - [Fact] - public async Task CollectionModelBinder_BindsListOfComplexType_WithPrefix_Success() + [Theory] + [InlineData("?[0].Id=10&[1].Id=11")] + [InlineData("?index=low&index=high&[low].Id=10&[high].Id=11")] + [InlineData("?parameter[0].Id=10¶meter[1].Id=11")] + [InlineData("?parameter.index=low¶meter.index=high¶meter[low].Id=10¶meter[high].Id=11")] + public async Task CollectionModelBinder_BindsListOfComplexType_ImpliedPrefix_Success(string queryString) { // Arrange var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); @@ -194,7 +187,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => { - request.QueryString = new QueryString("?parameter[0].Id=10¶meter[1].Id=11"); + request.QueryString = new QueryString(queryString); }); var modelState = new ModelStateDictionary(); @@ -213,18 +206,12 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.Equal(2, modelState.Count); Assert.Equal(0, modelState.ErrorCount); Assert.True(modelState.IsValid); - - var entry = Assert.Single(modelState, kvp => kvp.Key == "parameter[0].Id").Value; - Assert.Equal("10", entry.AttemptedValue); - Assert.Equal("10", entry.RawValue); - - entry = Assert.Single(modelState, kvp => kvp.Key == "parameter[1].Id").Value; - Assert.Equal("11", entry.AttemptedValue); - Assert.Equal("11", entry.RawValue); } - [Fact] - public async Task CollectionModelBinder_BindsListOfComplexType_WithExplicitPrefix_Success() + [Theory] + [InlineData("?prefix[0].Id=10&prefix[1].Id=11")] + [InlineData("?prefix.index=low&prefix.index=high&prefix[high].Id=11&prefix[low].Id=10")] + public async Task CollectionModelBinder_BindsListOfComplexType_ExplicitPrefix_Success(string queryString) { // Arrange var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); @@ -240,7 +227,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => { - request.QueryString = new QueryString("?prefix[0].Id=10&prefix[1].Id=11"); + request.QueryString = new QueryString(queryString); }); var modelState = new ModelStateDictionary(); @@ -259,56 +246,6 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.Equal(2, modelState.Count); Assert.Equal(0, modelState.ErrorCount); Assert.True(modelState.IsValid); - - var entry = Assert.Single(modelState, kvp => kvp.Key == "prefix[0].Id").Value; - Assert.Equal("10", entry.AttemptedValue); - Assert.Equal("10", entry.RawValue); - - entry = Assert.Single(modelState, kvp => kvp.Key == "prefix[1].Id").Value; - Assert.Equal("11", entry.AttemptedValue); - Assert.Equal("11", entry.RawValue); - } - - [Fact] - public async Task CollectionModelBinder_BindsCollectionOfComplexType_EmptyPrefix_Success() - { - // Arrange - var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); - var parameter = new ParameterDescriptor() - { - Name = "parameter", - ParameterType = typeof(List) - }; - - var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => - { - request.QueryString = new QueryString("?[0].Id=10&[1].Id=11"); - }); - - var modelState = new ModelStateDictionary(); - - // Act - var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext); - - // Assert - Assert.NotNull(modelBindingResult); - Assert.True(modelBindingResult.IsModelSet); - - var model = Assert.IsType>(modelBindingResult.Model); - Assert.Equal(10, model[0].Id); - Assert.Equal(11, model[1].Id); - - Assert.Equal(2, modelState.Count); - Assert.Equal(0, modelState.ErrorCount); - Assert.True(modelState.IsValid); - - var entry = Assert.Single(modelState, kvp => kvp.Key == "[0].Id").Value; - Assert.Equal("10", entry.AttemptedValue); - Assert.Equal("10", entry.RawValue); - - entry = Assert.Single(modelState, kvp => kvp.Key == "[1].Id").Value; - Assert.Equal("11", entry.AttemptedValue); - Assert.Equal("11", entry.RawValue); } [Fact] @@ -337,7 +274,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.True(modelBindingResult.IsModelSet); Assert.Empty(Assert.IsType>(modelBindingResult.Model)); - Assert.Equal(0, modelState.Count); + Assert.Empty(modelState); Assert.Equal(0, modelState.ErrorCount); Assert.True(modelState.IsValid); } @@ -514,6 +451,103 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.Equal(ModelValidationState.Invalid, entry.ValidationState); } + [Fact] + public async Task CollectionModelBinder_BindsListOfSimpleType_WithIndex_Success() + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(List) + }; + + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.QueryString = + new QueryString("?parameter.index=low¶meter.index=high¶meter[low]=10¶meter[high]=11"); + }); + + var modelState = new ModelStateDictionary(); + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext); + + // Assert + Assert.NotNull(modelBindingResult); + Assert.True(modelBindingResult.IsModelSet); + + var model = Assert.IsType>(modelBindingResult.Model); + Assert.Equal(new List() { 10, 11 }, model); + + // "index" is not stored in ModelState. + Assert.Equal(2, modelState.Count); + Assert.Equal(0, modelState.ErrorCount); + Assert.True(modelState.IsValid); + + var entry = Assert.Single(modelState, kvp => kvp.Key == "parameter[low]").Value; + Assert.Equal("10", entry.AttemptedValue); + Assert.Equal("10", entry.RawValue); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + + entry = Assert.Single(modelState, kvp => kvp.Key == "parameter[high]").Value; + Assert.Equal("11", entry.AttemptedValue); + Assert.Equal("11", entry.RawValue); + Assert.Equal(ModelValidationState.Valid, entry.ValidationState); + } + + [Fact] + public async Task CollectionModelBinder_BindsCollectionOfComplexType_WithRequiredProperty_WithIndex_PartialData() + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(ICollection) + }; + + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.QueryString = new QueryString("?index=low&index=high&[high].Id=11&[low].Id=10"); + }); + + var modelState = new ModelStateDictionary(); + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext); + + // Assert + Assert.NotNull(modelBindingResult); + Assert.True(modelBindingResult.IsModelSet); + + var model = Assert.IsType>(modelBindingResult.Model); + Assert.Equal(10, model[0].Id); + Assert.Null(model[0].Name); + Assert.Equal(11, model[1].Id); + Assert.Null(model[1].Name); + + Assert.Equal(4, modelState.Count); + Assert.Equal(2, modelState.ErrorCount); + Assert.False(modelState.IsValid); + + var entry = Assert.Single(modelState, kvp => kvp.Key == "[low].Id").Value; + Assert.Equal("10", entry.AttemptedValue); + Assert.Equal("10", entry.RawValue); + + entry = Assert.Single(modelState, kvp => kvp.Key == "[high].Id").Value; + Assert.Equal("11", entry.AttemptedValue); + Assert.Equal("11", entry.RawValue); + + entry = Assert.Single(modelState, kvp => kvp.Key == "[low].Name").Value; + Assert.Null(entry.RawValue); + Assert.Equal(ModelValidationState.Invalid, entry.ValidationState); + + entry = Assert.Single(modelState, kvp => kvp.Key == "[high].Name").Value; + Assert.Null(entry.RawValue); + Assert.Equal(ModelValidationState.Invalid, entry.ValidationState); + } + [Fact] public async Task CollectionModelBinder_BindsListOfComplexType_WithRequiredProperty_NoData() { @@ -540,7 +574,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.True(modelBindingResult.IsModelSet); Assert.Empty(Assert.IsType>(modelBindingResult.Model)); - Assert.Equal(0, modelState.Count); + Assert.Empty(modelState); Assert.Equal(0, modelState.ErrorCount); Assert.True(modelState.IsValid); } @@ -649,13 +683,567 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests 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 kvp = Assert.Single(modelState); + Assert.Equal("Addresses[Key1].Street", kvp.Key); + var entry = kvp.Value; var error = Assert.Single(entry.Errors); Assert.Equal("The field Street must be a string with a maximum length of 3.", error.ErrorMessage); } + + [Theory] + [InlineData("?[0].Street=LongStreet")] + [InlineData("?index=low&[low].Street=LongStreet")] + [InlineData("?parameter[0].Street=LongStreet")] + [InlineData("?parameter.index=low¶meter[low].Street=LongStreet")] + public async Task CollectionModelBinder_BindsCollectionOfComplexType_ImpliedPrefix_FindsValidationErrors( + string queryString) + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(ICollection), + }; + + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.QueryString = new QueryString(queryString); + }); + + var modelState = new ModelStateDictionary(); + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext); + + // Assert + Assert.NotNull(modelBindingResult); + Assert.True(modelBindingResult.IsModelSet); + var model = Assert.IsType>(modelBindingResult.Model); + var address = Assert.Single(model); + Assert.Equal("LongStreet", address.Street); + + Assert.False(modelState.IsValid); + + var entry = Assert.Single(modelState).Value; + var error = Assert.Single(entry.Errors); + Assert.Equal("The field Street must be a string with a maximum length of 3.", error.ErrorMessage); + } + + // parameter type, form content, expected type + public static TheoryData, Type> CollectionTypeData + { + get + { + return new TheoryData, Type> + { + { + typeof(IEnumerable), + new Dictionary + { + { "[0]", new[] { "hello" } }, + { "[1]", new[] { "world" } }, + }, + typeof(List) + }, + { + typeof(ICollection), + new Dictionary + { + { "index", new[] { "low", "high" } }, + { "[low]", new[] { "hello" } }, + { "[high]", new[] { "world" } }, + }, + typeof(List) + }, + { + typeof(IList), + new Dictionary + { + { "[0]", new[] { "hello" } }, + { "[1]", new[] { "world" } }, + }, + typeof(List) + }, + { + typeof(List), + new Dictionary + { + { "index", new[] { "low", "high" } }, + { "[low]", new[] { "hello" } }, + { "[high]", new[] { "world" } }, + }, + typeof(List) + }, + { + typeof(ClosedGenericCollection), + new Dictionary + { + { "[0]", new[] { "hello" } }, + { "[1]", new[] { "world" } }, + }, + typeof(ClosedGenericCollection) + }, + { + typeof(ClosedGenericList), + new Dictionary + { + { "index", new[] { "low", "high" } }, + { "[low]", new[] { "hello" } }, + { "[high]", new[] { "world" } }, + }, + typeof(ClosedGenericList) + }, + { + typeof(ExplicitClosedGenericCollection), + new Dictionary + { + { "[0]", new[] { "hello" } }, + { "[1]", new[] { "world" } }, + }, + typeof(ExplicitClosedGenericCollection) + }, + { + typeof(ExplicitClosedGenericList), + new Dictionary + { + { "index", new[] { "low", "high" } }, + { "[low]", new[] { "hello" } }, + { "[high]", new[] { "world" } }, + }, + typeof(ExplicitClosedGenericList) + }, + { + typeof(ExplicitCollection), + new Dictionary + { + { "[0]", new[] { "hello" } }, + { "[1]", new[] { "world" } }, + }, + typeof(ExplicitCollection) + }, + { + typeof(ExplicitList), + new Dictionary + { + { "index", new[] { "low", "high" } }, + { "[low]", new[] { "hello" } }, + { "[high]", new[] { "world" } }, + }, + typeof(ExplicitList) + }, + { + typeof(IEnumerable), + new Dictionary + { + { string.Empty, new[] { "hello", "world" } }, + }, + typeof(List) + }, + { + typeof(ICollection), + new Dictionary + { + { "[]", new[] { "hello", "world" } }, + }, + typeof(List) + }, + { + typeof(IList), + new Dictionary + { + { string.Empty, new[] { "hello", "world" } }, + }, + typeof(List) + }, + { + typeof(List), + new Dictionary + { + { "[]", new[] { "hello", "world" } }, + }, + typeof(List) + }, + { + typeof(ClosedGenericCollection), + new Dictionary + { + { string.Empty, new[] { "hello", "world" } }, + }, + typeof(ClosedGenericCollection) + }, + { + typeof(ClosedGenericList), + new Dictionary + { + { "[]", new[] { "hello", "world" } }, + }, + typeof(ClosedGenericList) + }, + { + typeof(ExplicitClosedGenericCollection), + new Dictionary + { + { string.Empty, new[] { "hello", "world" } }, + }, + typeof(ExplicitClosedGenericCollection) + }, + { + typeof(ExplicitClosedGenericList), + new Dictionary + { + { "[]", new[] { "hello", "world" } }, + }, + typeof(ExplicitClosedGenericList) + }, + { + typeof(ExplicitCollection), + new Dictionary + { + { string.Empty, new[] { "hello", "world" } }, + }, + typeof(ExplicitCollection) + }, + { + typeof(ExplicitList), + new Dictionary + { + { "[]", new[] { "hello", "world" } }, + }, + typeof(ExplicitList) + }, + }; + } + } + + [Theory] + [MemberData(nameof(CollectionTypeData))] + public async Task CollectionModelBinder_BindsParameterToExpectedType( + Type parameterType, + IDictionary formContent, + Type expectedType) + { + // Arrange + var expectedCollection = new List { "hello", "world" }; + var parameter = new ParameterDescriptor + { + Name = "parameter", + ParameterType = parameterType, + }; + + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var modelState = new ModelStateDictionary(); + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.Form = new FormCollection(formContent); + }); + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext); + + // Assert + Assert.NotNull(modelBindingResult); + Assert.True(modelBindingResult.IsModelSet); + + Assert.IsType(expectedType, modelBindingResult.Model); + + var model = modelBindingResult.Model as IEnumerable; + Assert.NotNull(model); // Guard + Assert.Equal(expectedCollection, model); + + Assert.True(modelState.IsValid); + Assert.NotEmpty(modelState); + Assert.Equal(0, modelState.ErrorCount); + } + + private class ClosedGenericCollection : Collection + { + } + + private class ClosedGenericList : List + { + } + + private class ExplicitClosedGenericCollection : ICollection + { + private List _data = new List(); + + int ICollection.Count + { + get + { + throw new NotImplementedException(); + } + } + + bool ICollection.IsReadOnly + { + get + { + return false; + } + } + + void ICollection.Add(string item) + { + _data.Add(item); + } + + void ICollection.Clear() + { + _data.Clear(); + } + + bool ICollection.Contains(string item) + { + throw new NotImplementedException(); + } + + void ICollection.CopyTo(string[] array, int arrayIndex) + { + throw new NotImplementedException(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return ((IEnumerable)_data).GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return _data.GetEnumerator(); + } + + bool ICollection.Remove(string item) + { + throw new NotImplementedException(); + } + } + + private class ExplicitClosedGenericList : IList + { + private List _data = new List(); + + string IList.this[int index] + { + get + { + throw new NotImplementedException(); + } + + set + { + throw new NotImplementedException(); + } + } + + int ICollection.Count + { + get + { + throw new NotImplementedException(); + } + } + + bool ICollection.IsReadOnly + { + get + { + return false; + } + } + + void ICollection.Add(string item) + { + _data.Add(item); + } + + void ICollection.Clear() + { + _data.Clear(); + } + + bool ICollection.Contains(string item) + { + throw new NotImplementedException(); + } + + void ICollection.CopyTo(string[] array, int arrayIndex) + { + throw new NotImplementedException(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return ((IEnumerable)_data).GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return _data.GetEnumerator(); + } + + int IList.IndexOf(string item) + { + throw new NotImplementedException(); + } + + void IList.Insert(int index, string item) + { + throw new NotImplementedException(); + } + + bool ICollection.Remove(string item) + { + throw new NotImplementedException(); + } + + void IList.RemoveAt(int index) + { + throw new NotImplementedException(); + } + } + + private class ExplicitCollection : ICollection + { + private List _data = new List(); + + int ICollection.Count + { + get + { + throw new NotImplementedException(); + } + } + + bool ICollection.IsReadOnly + { + get + { + return false; + } + } + + void ICollection.Add(T item) + { + _data.Add(item); + } + + void ICollection.Clear() + { + _data.Clear(); + } + + bool ICollection.Contains(T item) + { + throw new NotImplementedException(); + } + + void ICollection.CopyTo(T[] array, int arrayIndex) + { + throw new NotImplementedException(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return ((IEnumerable)_data).GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return _data.GetEnumerator(); + } + + bool ICollection.Remove(T item) + { + throw new NotImplementedException(); + } + } + + private class ExplicitList : IList + { + private List _data = new List(); + + T IList.this[int index] + { + get + { + throw new NotImplementedException(); + } + + set + { + throw new NotImplementedException(); + } + } + + int ICollection.Count + { + get + { + throw new NotImplementedException(); + } + } + + bool ICollection.IsReadOnly + { + get + { + return false; + } + } + + void ICollection.Add(T item) + { + _data.Add(item); + } + + void ICollection.Clear() + { + _data.Clear(); + } + + bool ICollection.Contains(T item) + { + throw new NotImplementedException(); + } + + void ICollection.CopyTo(T[] array, int arrayIndex) + { + throw new NotImplementedException(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return ((IEnumerable)_data).GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return _data.GetEnumerator(); + } + + int IList.IndexOf(T item) + { + throw new NotImplementedException(); + } + + void IList.Insert(int index, T item) + { + throw new NotImplementedException(); + } + + bool ICollection.Remove(T item) + { + throw new NotImplementedException(); + } + + void IList.RemoveAt(int index) + { + throw new NotImplementedException(); + } + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/DictionaryModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/DictionaryModelBinderIntegrationTest.cs index 13fcc86b20..4d3ffe1141 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/DictionaryModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/DictionaryModelBinderIntegrationTest.cs @@ -1,7 +1,10 @@ // 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.Collections; using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Mvc.ModelBinding; @@ -91,9 +94,57 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.Equal("10", entry.RawValue); } + [Fact] + public async Task DictionaryModelBinder_BindsDictionaryOfSimpleType_WithIndex_Success() + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor() + { + Name = "parameter", + ParameterType = typeof(Dictionary) + }; + + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.QueryString = + new QueryString("?parameter.index=low¶meter[low].Key=key0¶meter[low].Value=10"); + }); + + var modelState = new ModelStateDictionary(); + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext); + + // Assert + Assert.NotNull(modelBindingResult); + Assert.True(modelBindingResult.IsModelSet); + + var model = Assert.IsType>(modelBindingResult.Model); + Assert.Equal(new Dictionary() { { "key0", 10 } }, model); + + // "index" is not stored in ModelState. + Assert.Equal(2, modelState.Count); + Assert.Equal(0, modelState.ErrorCount); + Assert.True(modelState.IsValid); + + var entry = Assert.Single(modelState, kvp => kvp.Key == "parameter[low].Key").Value; + Assert.Equal("key0", entry.AttemptedValue); + Assert.Equal("key0", entry.RawValue); + + // Key and Value are skipped if they have simple types. + Assert.Equal(ModelValidationState.Skipped, entry.ValidationState); + + entry = Assert.Single(modelState, kvp => kvp.Key == "parameter[low].Value").Value; + Assert.Equal("10", entry.AttemptedValue); + Assert.Equal("10", entry.RawValue); + Assert.Equal(ModelValidationState.Skipped, entry.ValidationState); + } + [Theory] [InlineData("?prefix[key0]=10")] [InlineData("?prefix[0].Key=key0&prefix[0].Value=10")] + [InlineData("?prefix.index=low&prefix[low].Key=key0&prefix[low].Value=10")] public async Task DictionaryModelBinder_BindsDictionaryOfSimpleType_WithExplicitPrefix_Success( string queryString) { @@ -134,6 +185,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests [Theory] [InlineData("?[key0]=10")] [InlineData("?[0].Key=key0&[0].Value=10")] + [InlineData("?index=low&[low].Key=key0&[low].Value=10")] public async Task DictionaryModelBinder_BindsDictionaryOfSimpleType_EmptyPrefix_Success(string queryString) { // Arrange @@ -201,6 +253,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests private class Person { + [Range(minimum: 0, maximum: 15, ErrorMessage = "You're out of range.")] public int Id { get; set; } public override bool Equals(object obj) @@ -222,9 +275,13 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests } [Theory] + [InlineData("?[key0].Id=10")] + [InlineData("?[0].Key=key0&[0].Value.Id=10")] + [InlineData("?index=low&[low].Key=key0&[low].Value.Id=10")] [InlineData("?parameter[key0].Id=10")] [InlineData("?parameter[0].Key=key0¶meter[0].Value.Id=10")] - public async Task DictionaryModelBinder_BindsDictionaryOfComplexType_WithPrefix_Success(string queryString) + [InlineData("?parameter.index=low¶meter[low].Key=key0¶meter[low].Value.Id=10")] + public async Task DictionaryModelBinder_BindsDictionaryOfComplexType_ImpliedPrefix_Success(string queryString) { // Arrange var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); @@ -259,7 +316,8 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests [Theory] [InlineData("?prefix[key0].Id=10")] [InlineData("?prefix[0].Key=key0&prefix[0].Value.Id=10")] - public async Task DictionaryModelBinder_BindsDictionaryOfComplexType_WithExplicitPrefix_Success( + [InlineData("?prefix.index=low&prefix[low].Key=key0&prefix[low].Value.Id=10")] + public async Task DictionaryModelBinder_BindsDictionaryOfComplexType_ExplicitPrefix_Success( string queryString) { // Arrange @@ -297,9 +355,14 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests } [Theory] - [InlineData("?[key0].Id=10")] - [InlineData("?[0].Key=key0&[0].Value.Id=10")] - public async Task DictionaryModelBinder_BindsDictionaryOfComplexType_EmptyPrefix_Success(string queryString) + [InlineData("?[key0].Id=100")] + [InlineData("?[0].Key=key0&[0].Value.Id=100")] + [InlineData("?index=low&[low].Key=key0&[low].Value.Id=100")] + [InlineData("?parameter[key0].Id=100")] + [InlineData("?parameter[0].Key=key0¶meter[0].Value.Id=100")] + [InlineData("?parameter.index=low¶meter[low].Key=key0¶meter[low].Value.Id=100")] + public async Task DictionaryModelBinder_BindsDictionaryOfComplexType_ImpliedPrefix_FindsValidationErrors( + string queryString) { // Arrange var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); @@ -324,11 +387,19 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.True(modelBindingResult.IsModelSet); var model = Assert.IsType>(modelBindingResult.Model); - Assert.Equal(new Dictionary { { "key0", new Person { Id = 10 } }, }, model); + Assert.Equal(new Dictionary { { "key0", new Person { Id = 100 } }, }, model); Assert.NotEmpty(modelState); - Assert.Equal(0, modelState.ErrorCount); - Assert.True(modelState.IsValid); + Assert.False(modelState.IsValid); + Assert.All(modelState, kvp => + { + Assert.NotEqual(ModelValidationState.Unvalidated, kvp.Value.ValidationState); + Assert.NotEqual(ModelValidationState.Skipped, kvp.Value.ValidationState); + }); + + var entry = Assert.Single(modelState, kvp => kvp.Value.ValidationState == ModelValidationState.Invalid); + var error = Assert.Single(entry.Value.Errors); + Assert.Equal("You're out of range.", error.ErrorMessage); } [Fact] @@ -363,5 +434,339 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests Assert.Equal(0, modelState.ErrorCount); Assert.True(modelState.IsValid); } + + // parameter type, query string, expected type + public static TheoryData DictionaryTypeData + { + get + { + return new TheoryData + { + { + typeof(IDictionary), + "?[key0]=hello&[key1]=world", + typeof(Dictionary) + }, + { + typeof(Dictionary), + "?[key0]=hello&[key1]=world", + typeof(Dictionary) + }, + { + typeof(ClosedGenericDictionary), + "?[key0]=hello&[key1]=world", + typeof(ClosedGenericDictionary) + }, + { + typeof(ClosedGenericKeyDictionary), + "?[key0]=hello&[key1]=world", + typeof(ClosedGenericKeyDictionary) + }, + { + typeof(ExplicitClosedGenericDictionary), + "?[key0]=hello&[key1]=world", + typeof(ExplicitClosedGenericDictionary) + }, + { + typeof(ExplicitDictionary), + "?[key0]=hello&[key1]=world", + typeof(ExplicitDictionary) + }, + { + typeof(IDictionary), + "?index=low&index=high&[low].Key=key0&[low].Value=hello&[high].Key=key1&[high].Value=world", + typeof(Dictionary) + }, + { + typeof(Dictionary), + "?[0].Key=key0&[0].Value=hello&[1].Key=key1&[1].Value=world", + typeof(Dictionary) + }, + { + typeof(ClosedGenericDictionary), + "?index=low&index=high&[low].Key=key0&[low].Value=hello&[high].Key=key1&[high].Value=world", + typeof(ClosedGenericDictionary) + }, + { + typeof(ClosedGenericKeyDictionary), + "?[0].Key=key0&[0].Value=hello&[1].Key=key1&[1].Value=world", + typeof(ClosedGenericKeyDictionary) + }, + { + typeof(ExplicitClosedGenericDictionary), + "?index=low&index=high&[low].Key=key0&[low].Value=hello&[high].Key=key1&[high].Value=world", + typeof(ExplicitClosedGenericDictionary) + }, + { + typeof(ExplicitDictionary), + "?[0].Key=key0&[0].Value=hello&[1].Key=key1&[1].Value=world", + typeof(ExplicitDictionary) + }, + }; + } + } + + [Theory] + [MemberData(nameof(DictionaryTypeData))] + public async Task DictionaryModelBinder_BindsParameterToExpectedType( + Type parameterType, + string queryString, + Type expectedType) + { + // Arrange + var expectedDictionary = new Dictionary + { + { "key0", "hello" }, + { "key1", "world" }, + }; + var parameter = new ParameterDescriptor + { + Name = "parameter", + ParameterType = parameterType, + }; + + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var modelState = new ModelStateDictionary(); + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.QueryString = new QueryString(queryString); + }); + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, modelState, operationContext); + + // Assert + Assert.NotNull(modelBindingResult); + Assert.True(modelBindingResult.IsModelSet); + + Assert.IsType(expectedType, modelBindingResult.Model); + + var model = modelBindingResult.Model as IDictionary; + Assert.NotNull(model); // Guard + Assert.Equal(expectedDictionary.Keys, model.Keys); + Assert.Equal(expectedDictionary.Values, model.Values); + + Assert.True(modelState.IsValid); + Assert.NotEmpty(modelState); + Assert.Equal(0, modelState.ErrorCount); + } + + private class ClosedGenericDictionary : Dictionary + { + } + + private class ClosedGenericKeyDictionary : Dictionary + { + } + + private class ExplicitClosedGenericDictionary : IDictionary + { + private IDictionary _data = new Dictionary(); + + string IDictionary.this[string key] + { + get + { + throw new NotImplementedException(); + } + + set + { + _data[key] = value; + } + } + + int ICollection>.Count + { + get + { + return _data.Count; + } + } + + bool ICollection>.IsReadOnly + { + get + { + return false; + } + } + + ICollection IDictionary.Keys + { + get + { + return _data.Keys; + } + } + + ICollection IDictionary.Values + { + get + { + return _data.Values; + } + } + + void ICollection>.Add(KeyValuePair item) + { + _data.Add(item); + } + + void IDictionary.Add(string key, string value) + { + throw new NotImplementedException(); + } + + void ICollection>.Clear() + { + _data.Clear(); + } + + bool ICollection>.Contains(KeyValuePair item) + { + throw new NotImplementedException(); + } + + bool IDictionary.ContainsKey(string key) + { + throw new NotImplementedException(); + } + + void ICollection>.CopyTo(KeyValuePair[] array, int arrayIndex) + { + throw new NotImplementedException(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + throw new NotImplementedException(); + } + + IEnumerator> IEnumerable>.GetEnumerator() + { + throw new NotImplementedException(); + } + + bool ICollection>.Remove(KeyValuePair item) + { + throw new NotImplementedException(); + } + + bool IDictionary.Remove(string key) + { + throw new NotImplementedException(); + } + + bool IDictionary.TryGetValue(string key, out string value) + { + throw new NotImplementedException(); + } + } + + private class ExplicitDictionary : IDictionary + { + private IDictionary _data = new Dictionary(); + + TValue IDictionary.this[TKey key] + { + get + { + throw new NotImplementedException(); + } + + set + { + _data[key] = value; + } + } + + int ICollection>.Count + { + get + { + return _data.Count; + } + } + + bool ICollection>.IsReadOnly + { + get + { + return false; + } + } + + ICollection IDictionary.Keys + { + get + { + return _data.Keys; + } + } + + ICollection IDictionary.Values + { + get + { + return _data.Values; + } + } + + void ICollection>.Add(KeyValuePair item) + { + _data.Add(item); + } + + void IDictionary.Add(TKey key, TValue value) + { + throw new NotImplementedException(); + } + + void ICollection>.Clear() + { + _data.Clear(); + } + + bool ICollection>.Contains(KeyValuePair item) + { + throw new NotImplementedException(); + } + + bool IDictionary.ContainsKey(TKey key) + { + throw new NotImplementedException(); + } + + void ICollection>.CopyTo(KeyValuePair[] array, int arrayIndex) + { + throw new NotImplementedException(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + throw new NotImplementedException(); + } + + IEnumerator> IEnumerable>.GetEnumerator() + { + throw new NotImplementedException(); + } + + bool ICollection>.Remove(KeyValuePair item) + { + throw new NotImplementedException(); + } + + bool IDictionary.Remove(TKey key) + { + throw new NotImplementedException(); + } + + bool IDictionary.TryGetValue(TKey key, out TValue value) + { + throw new NotImplementedException(); + } + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs index 3ef6aa75c9..e5daf90fbc 100644 --- a/test/Microsoft.AspNet.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/GenericModelBinderIntegrationTest.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Collections.ObjectModel; using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Mvc.ModelBinding;