diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ArrayModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ArrayModelBinder.cs
index f7df7ae430..4843261a2a 100644
--- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ArrayModelBinder.cs
+++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ArrayModelBinder.cs
@@ -1,7 +1,9 @@
// 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.Generic;
+using System.Diagnostics;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.Framework.Internal;
@@ -25,15 +27,27 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
return base.BindModelAsync(bindingContext);
}
- protected override object CreateEmptyCollection()
+ ///
+ public override bool CanCreateInstance(Type targetType)
{
+ return targetType == typeof(TElement[]);
+ }
+
+ ///
+ protected override object CreateEmptyCollection(Type targetType)
+ {
+ Debug.Assert(targetType == typeof(TElement[]), "GenericModelBinder only creates this binder for arrays.");
+
return new TElement[0];
}
///
- protected override object GetModel(IEnumerable newCollection)
+ protected override object ConvertToCollectionType(Type targetType, IEnumerable collection)
{
- return newCollection?.ToArray();
+ Debug.Assert(targetType == typeof(TElement[]), "GenericModelBinder only creates this binder for arrays.");
+
+ // If non-null, collection is a List, never already a TElement[].
+ return collection?.ToArray();
}
///
diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs
index f207b25c4d..bb7993345f 100644
--- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs
+++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs
@@ -7,6 +7,9 @@ using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
using System.Linq;
+#if DNXCORE50
+using System.Reflection;
+#endif
using System.Threading.Tasks;
using Microsoft.Framework.Internal;
@@ -16,22 +19,24 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
/// implementation for binding collection values.
///
/// Type of elements in the collection.
- public class CollectionModelBinder : IModelBinder
+ public class CollectionModelBinder : ICollectionModelBinder
{
///
public virtual async Task BindModelAsync([NotNull] ModelBindingContext bindingContext)
{
ModelBindingHelper.ValidateBindingContext(bindingContext);
- object model;
-
+ var model = bindingContext.Model;
if (!await bindingContext.ValueProvider.ContainsPrefixAsync(bindingContext.ModelName))
{
// If this is the fallback case and we failed to find data for a top-level model, then generate a
- // default 'empty' model and return it.
+ // default 'empty' model (or use existing Model) and return it.
if (!bindingContext.IsFirstChanceBinding && bindingContext.IsTopLevelObject)
{
- model = CreateEmptyCollection();
+ if (model == null)
+ {
+ model = CreateEmptyCollection(bindingContext.ModelType);
+ }
var validationNode = new ModelValidationNode(
bindingContext.ModelName,
@@ -50,12 +55,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
var valueProviderResult = await bindingContext.ValueProvider.GetValueAsync(bindingContext.ModelName);
- IEnumerable boundCollection;
CollectionResult result;
if (valueProviderResult == null)
{
result = await BindComplexCollection(bindingContext);
- boundCollection = result.Model;
}
else
{
@@ -63,13 +66,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
bindingContext,
valueProviderResult.RawValue,
valueProviderResult.Culture);
- boundCollection = result.Model;
}
- model = bindingContext.Model;
+ var boundCollection = result.Model;
if (model == null)
{
- model = GetModel(boundCollection);
+ model = ConvertToCollectionType(bindingContext.ModelType, boundCollection);
}
else
{
@@ -84,14 +86,50 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
validationNode: result?.ValidationNode);
}
- // Called when we're creating a default 'empty' model for a top level bind.
- protected virtual object CreateEmptyCollection()
+ ///
+ public virtual bool CanCreateInstance(Type targetType)
{
- return new List();
+ return CreateEmptyCollection(targetType) != null;
+ }
+
+ ///
+ /// Create an assignable to .
+ ///
+ /// of the model.
+ /// An assignable to .
+ /// Called when creating a default 'empty' model for a top level bind.
+ protected virtual object CreateEmptyCollection(Type targetType)
+ {
+ if (targetType.IsAssignableFrom(typeof(List)))
+ {
+ // Simple case such as ICollection, IEnumerable and IList.
+ return new List();
+ }
+
+ return CreateInstance(targetType);
+ }
+
+ ///
+ /// Create an instance of .
+ ///
+ /// of the model.
+ /// An instance of .
+ protected object CreateInstance(Type targetType)
+ {
+ try
+ {
+ return Activator.CreateInstance(targetType);
+ }
+ catch (Exception)
+ {
+ // Details of exception are not important.
+ return null;
+ }
}
// Used when the ValueProvider contains the collection to be bound as a single element, e.g. the raw value
// is [ "1", "2" ] and needs to be converted to an int[].
+ // Internal for testing.
internal async Task BindSimpleCollection(
ModelBindingContext bindingContext,
object rawValue,
@@ -156,6 +194,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
return await BindComplexCollectionFromIndexes(bindingContext, indexNames);
}
+ // Internal for testing.
internal async Task BindComplexCollectionFromIndexes(
ModelBindingContext bindingContext,
IEnumerable indexNames)
@@ -219,6 +258,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
};
}
+ // Internal for testing.
internal class CollectionResult
{
public ModelValidationNode ValidationNode { get; set; }
@@ -227,23 +267,37 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
}
///
- /// Gets an assignable to the collection property.
+ /// Gets an assignable to that contains members from
+ /// .
///
- ///
+ /// of the model.
+ ///
/// Collection of values retrieved from value providers. Or null if nothing was bound.
///
///
- /// assignable to the collection property. Or null if nothing was bound.
+ /// An assignable to . Or null if nothing was bound.
///
///
/// Extensibility point that allows the bound collection to be manipulated or transformed before being
/// returned from the binder.
///
- protected virtual object GetModel(IEnumerable newCollection)
+ protected virtual object ConvertToCollectionType(Type targetType, IEnumerable collection)
{
- // Depends on fact BindSimpleCollection() and BindComplexCollection() always return a List
- // instance or null. In addition GenericModelBinder confirms a List is assignable to the
- // property prior to instantiating this binder and subclass binders do not call this method.
+ if (collection == null)
+ {
+ return null;
+ }
+
+ if (targetType.IsAssignableFrom(typeof(List)))
+ {
+ // Depends on fact BindSimpleCollection() and BindComplexCollection() always return a List
+ // instance or null.
+ return collection;
+ }
+
+ var newCollection = CreateInstance(targetType);
+ CopyToModel(newCollection, collection);
+
return newCollection;
}
@@ -254,11 +308,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
///
/// Collection of values retrieved from value providers. Or null if nothing was bound.
///
- /// Called only in TryUpdateModelAsync(collection, ...) scenarios.
protected virtual void CopyToModel([NotNull] object target, IEnumerable sourceCollection)
{
var targetCollection = target as ICollection;
- Debug.Assert(targetCollection != null); // This binder is instantiated only for ICollection model types.
+ Debug.Assert(targetCollection != null, "This binder is instantiated only for ICollection model types.");
if (sourceCollection != null && targetCollection != null && !targetCollection.IsReadOnly)
{
@@ -270,7 +323,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
}
}
- internal static object[] RawValueToObjectArray(object rawValue)
+ private static object[] RawValueToObjectArray(object rawValue)
{
// precondition: rawValue is not null
diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/DictionaryModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/DictionaryModelBinder.cs
index 26c3dab010..3990f3255f 100644
--- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/DictionaryModelBinder.cs
+++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/DictionaryModelBinder.cs
@@ -1,9 +1,13 @@
// 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.Generic;
using System.Diagnostics;
using System.Linq;
+#if DNXCORE50
+using System.Reflection;
+#endif
using System.Threading.Tasks;
using Microsoft.Framework.Internal;
@@ -27,7 +31,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
}
Debug.Assert(result.Model != null);
- var model = (Dictionary)result.Model;
+ var model = (IDictionary)result.Model;
if (model.Count != 0)
{
// ICollection> approach was successful.
@@ -80,15 +84,37 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
}
///
- protected override object GetModel(IEnumerable> newCollection)
+ protected override object ConvertToCollectionType(
+ Type targetType,
+ IEnumerable> collection)
{
- return newCollection?.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
+ if (collection == null)
+ {
+ return null;
+ }
+
+ if (targetType.IsAssignableFrom(typeof(Dictionary)))
+ {
+ // Collection is a List>, never already a Dictionary.
+ return collection.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
+ }
+
+ var newCollection = CreateInstance(targetType);
+ CopyToModel(newCollection, collection);
+
+ return newCollection;
}
///
- protected override object CreateEmptyCollection()
+ protected override object CreateEmptyCollection(Type targetType)
{
- return new Dictionary();
+ if (targetType.IsAssignableFrom(typeof(Dictionary)))
+ {
+ // Simple case such as IDictionary.
+ return new Dictionary();
+ }
+
+ return CreateInstance(targetType);
}
private static TKey ConvertFromString(string keyString)
diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs
index 83820ea56f..f325a5f558 100644
--- a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs
+++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/GenericModelBinder.cs
@@ -13,12 +13,21 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
{
public async Task BindModelAsync(ModelBindingContext bindingContext)
{
- var binderType = ResolveBinderType(bindingContext.ModelType);
+ var binderType = ResolveBinderType(bindingContext);
if (binderType != null)
{
var binder = (IModelBinder)Activator.CreateInstance(binderType);
- var result = await binder.BindModelAsync(bindingContext);
+ var collectionBinder = binder as ICollectionModelBinder;
+ if (collectionBinder != null &&
+ bindingContext.Model == null &&
+ !collectionBinder.CanCreateInstance(bindingContext.ModelType))
+ {
+ // Able to resolve a binder type but need a new model instance and that binder cannot create it.
+ return null;
+ }
+
+ var result = await binder.BindModelAsync(bindingContext);
var modelBindingResult = result != null ?
result :
new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false);
@@ -31,12 +40,15 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
return null;
}
- private static Type ResolveBinderType(Type modelType)
+ private static Type ResolveBinderType(ModelBindingContext context)
{
+ var modelType = context.ModelType;
+
return GetArrayBinder(modelType) ??
- GetCollectionBinder(modelType) ??
- GetDictionaryBinder(modelType) ??
- GetKeyValuePairBinder(modelType);
+ GetCollectionBinder(modelType) ??
+ GetDictionaryBinder(modelType) ??
+ GetEnumerableBinder(context) ??
+ GetKeyValuePairBinder(modelType);
}
private static Type GetArrayBinder(Type modelType)
@@ -52,19 +64,50 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
private static Type GetCollectionBinder(Type modelType)
{
return GetGenericBinderType(
- typeof(ICollection<>),
- typeof(List<>),
- typeof(CollectionModelBinder<>),
- modelType);
+ typeof(ICollection<>),
+ typeof(CollectionModelBinder<>),
+ modelType);
}
private static Type GetDictionaryBinder(Type modelType)
{
return GetGenericBinderType(
- typeof(IDictionary<,>),
- typeof(Dictionary<,>),
- typeof(DictionaryModelBinder<,>),
- modelType);
+ typeof(IDictionary<,>),
+ typeof(DictionaryModelBinder<,>),
+ modelType);
+ }
+
+ private static Type GetEnumerableBinder(ModelBindingContext context)
+ {
+ var modelTypeArguments = GetGenericBinderTypeArgs(typeof(IEnumerable<>), context.ModelType);
+ if (modelTypeArguments == null)
+ {
+ return null;
+ }
+
+ if (context.Model == null)
+ {
+ // GetCollectionBinder has already confirmed modelType is not compatible with ICollection. Can a
+ // List (the default CollectionModelBinder type) instance be used instead of that exact type?
+ // Likely this will succeed only if the property type is exactly IEnumerable.
+ var closedListType = typeof(List<>).MakeGenericType(modelTypeArguments);
+ if (!context.ModelType.IsAssignableFrom(closedListType))
+ {
+ return null;
+ }
+ }
+ 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()))
+ {
+ return null;
+ }
+ }
+
+ return typeof(CollectionModelBinder<>).MakeGenericType(modelTypeArguments);
}
private static Type GetKeyValuePairBinder(Type modelType)
@@ -79,52 +122,46 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
return null;
}
- ///
- /// Example: GetGenericBinderType(typeof(IList<T>), typeof(List<T>),
- /// typeof(ListBinder<T>), ...) means that the ListBinder<T> type can update models that
- /// implement , and if for some reason the existing model instance is not updatable the
- /// binder will create a object and bind to that instead. This method will return
- /// ListBinder<T> or null, depending on whether the type and updatability checks succeed.
- ///
- private static Type GetGenericBinderType(Type supportedInterfaceType,
- Type newInstanceType,
- Type openBinderType,
- Type modelType)
+ // Example: GetGenericBinderType(typeof(IList), typeof(ListBinder), ...) means that the ListBinder
+ // type can update models that implement IList. This method will return
+ // ListBinder or null, depending on whether the type checks succeed.
+ private static Type GetGenericBinderType(Type supportedInterfaceType, Type openBinderType, Type modelType)
{
- Debug.Assert(supportedInterfaceType != null);
Debug.Assert(openBinderType != null);
- Debug.Assert(modelType != null);
var modelTypeArguments = GetGenericBinderTypeArgs(supportedInterfaceType, modelType);
-
if (modelTypeArguments == null)
{
return null;
}
- var closedNewInstanceType = newInstanceType.MakeGenericType(modelTypeArguments);
- if (!modelType.GetTypeInfo().IsAssignableFrom(closedNewInstanceType.GetTypeInfo()))
- {
- return null;
- }
-
return openBinderType.MakeGenericType(modelTypeArguments);
}
// Get the generic arguments for the binder, based on the model type. Or null if not compatible.
private static Type[] GetGenericBinderTypeArgs(Type supportedInterfaceType, Type modelType)
{
+ Debug.Assert(supportedInterfaceType != null);
+ Debug.Assert(modelType != null);
+
var modelTypeInfo = modelType.GetTypeInfo();
if (!modelTypeInfo.IsGenericType || modelTypeInfo.IsGenericTypeDefinition)
{
- // not a closed generic type
+ // modelType is not a closed generic type.
return null;
}
var modelTypeArguments = modelTypeInfo.GenericTypeArguments;
if (modelTypeArguments.Length != supportedInterfaceType.GetTypeInfo().GenericTypeParameters.Length)
{
- // wrong number of generic type arguments
+ // 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;
}
diff --git a/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ICollectionModelBinder.cs b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ICollectionModelBinder.cs
new file mode 100644
index 0000000000..13c4be6091
--- /dev/null
+++ b/src/Microsoft.AspNet.Mvc.Core/ModelBinding/ICollectionModelBinder.cs
@@ -0,0 +1,27 @@
+// 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;
+
+namespace Microsoft.AspNet.Mvc.ModelBinding
+{
+ ///
+ /// Interface for model binding collections.
+ ///
+ public interface ICollectionModelBinder : IModelBinder
+ {
+ ///
+ /// Gets an indication whether or not this implementation can create
+ /// an assignable to .
+ ///
+ /// of the model.
+ ///
+ /// true if this implementation can create an
+ /// assignable to ; false otherwise.
+ ///
+ ///
+ /// A true return value is necessary for successful model binding if model is initially null.
+ ///
+ bool CanCreateInstance(Type targetType);
+ }
+}
diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs
index 8b80636b7c..413b6687a4 100644
--- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs
+++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs
@@ -1,8 +1,9 @@
// 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.
-#if DNX451
+using System;
using System.Collections.Generic;
+#if DNX451
using System.Globalization;
using System.Linq;
#endif
@@ -275,6 +276,44 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
Assert.Same(result.ValidationNode.ModelMetadata, context.ModelMetadata);
}
+ // Setup like CollectionModelBinder_CreatesEmptyCollection_IfIsTopLevelObjectAndNotIsFirstChanceBinding except
+ // Model already has a value.
+ [Fact]
+ public async Task CollectionModelBinder_DoesNotCreateEmptyCollection_IfModelNonNull()
+ {
+ // Arrange
+ var binder = new CollectionModelBinder();
+
+ var context = CreateContext();
+ context.IsTopLevelObject = true;
+
+ var list = new List();
+ context.Model = list;
+
+ // Lack of prefix and non-empty model name both ignored.
+ context.ModelName = "modelName";
+
+ var metadataProvider = context.OperationBindingContext.MetadataProvider;
+ context.ModelMetadata = metadataProvider.GetMetadataForType(typeof(List));
+
+ context.ValueProvider = new TestValueProvider(new Dictionary());
+
+ // Act
+ var result = await binder.BindModelAsync(context);
+
+ // Assert
+ Assert.NotNull(result);
+
+ Assert.Same(list, result.Model);
+ Assert.Empty(list);
+ Assert.Equal("modelName", result.Key);
+ Assert.True(result.IsModelSet);
+
+ Assert.Same(result.ValidationNode.Model, result.Model);
+ Assert.Same(result.ValidationNode.Key, result.Key);
+ Assert.Same(result.ValidationNode.ModelMetadata, context.ModelMetadata);
+ }
+
[Theory]
[InlineData("")]
[InlineData("param")]
@@ -300,6 +339,39 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
Assert.Null(result);
}
+ // Model type -> can create instance.
+ public static TheoryData CanCreateInstanceData
+ {
+ get
+ {
+ return new TheoryData
+ {
+ { typeof(IEnumerable), true },
+ { typeof(ICollection), true },
+ { typeof(IList), true },
+ { typeof(List), true },
+ { typeof(LinkedList), true },
+ { typeof(ISet), false },
+ { typeof(ListWithInternalConstructor), false },
+ { typeof(ListWithThrowingConstructor), false },
+ };
+ }
+ }
+
+ [Theory]
+ [MemberData(nameof(CanCreateInstanceData))]
+ public void CanCreateInstance_ReturnsExpectedValue(Type modelType, bool expectedResult)
+ {
+ // Arrange
+ var binder = new CollectionModelBinder();
+
+ // Act
+ var result = binder.CanCreateInstance(modelType);
+
+ // Assert
+ Assert.Equal(expectedResult, result);
+ }
+
#if DNX451
[Fact]
public async Task BindSimpleCollection_SubBindingSucceeds()
@@ -335,7 +407,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
var bindingContext = new ModelBindingContext
{
- ModelMetadata = metadataProvider.GetMetadataForType(typeof(int)),
+ ModelMetadata = metadataProvider.GetMetadataForType(typeof(IList)),
ModelName = "someName",
ValueProvider = valueProvider,
OperationBindingContext = new OperationBindingContext
@@ -387,5 +459,29 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
{
public List ListProperty { get; set; }
}
+
+ private class ModelWithSimpleProperties
+ {
+ public int Id { get; set; }
+
+ public string Name { get; set; }
+ }
+
+ private class ListWithInternalConstructor : List
+ {
+ internal ListWithInternalConstructor()
+ : base()
+ {
+ }
+ }
+
+ private class ListWithThrowingConstructor : List
+ {
+ public ListWithThrowingConstructor()
+ : base()
+ {
+ throw new ApplicationException("No, don't do this.");
+ }
+ }
}
}
diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/DictionaryModelBinderTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/DictionaryModelBinderTest.cs
index f7d13df7ce..ec67e53eff 100644
--- a/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/DictionaryModelBinderTest.cs
+++ b/test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/DictionaryModelBinderTest.cs
@@ -112,8 +112,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
var metadataProvider = context.OperationBindingContext.MetadataProvider;
context.ModelMetadata = metadataProvider.GetMetadataForProperty(
- typeof(ModelWithDictionaryProperty),
- nameof(ModelWithDictionaryProperty.DictionaryProperty));
+ typeof(ModelWithDictionaryProperties),
+ nameof(ModelWithDictionaryProperties.DictionaryProperty));
// Act
var result = await binder.BindModelAsync(context);
@@ -150,8 +150,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
var metadataProvider = context.OperationBindingContext.MetadataProvider;
context.ModelMetadata = metadataProvider.GetMetadataForProperty(
- typeof(ModelWithDictionaryProperty),
- nameof(ModelWithDictionaryProperty.DictionaryProperty));
+ typeof(ModelWithDictionaryProperties),
+ nameof(ModelWithDictionaryProperties.DictionaryProperty));
// Act
var result = await binder.BindModelAsync(context);
@@ -202,8 +202,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
var metadataProvider = context.OperationBindingContext.MetadataProvider;
context.ModelMetadata = metadataProvider.GetMetadataForProperty(
- typeof(ModelWithDictionaryProperty),
- nameof(ModelWithDictionaryProperty.DictionaryProperty));
+ typeof(ModelWithDictionaryProperties),
+ nameof(ModelWithDictionaryProperties.DictionaryWithValueTypesProperty));
// Act
var result = await binder.BindModelAsync(context);
@@ -244,8 +244,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
var metadataProvider = context.OperationBindingContext.MetadataProvider;
context.ModelMetadata = metadataProvider.GetMetadataForProperty(
- typeof(ModelWithDictionaryProperty),
- nameof(ModelWithDictionaryProperty.DictionaryProperty));
+ typeof(ModelWithDictionaryProperties),
+ nameof(ModelWithDictionaryProperties.DictionaryWithComplexValuesProperty));
// Act
var result = await binder.BindModelAsync(context);
@@ -261,6 +261,41 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
Assert.Equal(dictionary, resultDictionary);
}
+ [Theory]
+ [MemberData(nameof(StringToStringData))]
+ public async Task BindModel_FallsBackToBindingValues_WithCustomDictionary(
+ string modelName,
+ string keyFormat,
+ IDictionary dictionary)
+ {
+ // Arrange
+ var expectedDictionary = new SortedDictionary(dictionary);
+ var binder = new DictionaryModelBinder();
+ var context = CreateContext();
+ context.ModelName = modelName;
+ context.OperationBindingContext.ModelBinder = CreateCompositeBinder();
+ context.OperationBindingContext.ValueProvider = CreateEnumerableValueProvider(keyFormat, dictionary);
+ context.ValueProvider = context.OperationBindingContext.ValueProvider;
+
+ var metadataProvider = context.OperationBindingContext.MetadataProvider;
+ context.ModelMetadata = metadataProvider.GetMetadataForProperty(
+ typeof(ModelWithDictionaryProperties),
+ nameof(ModelWithDictionaryProperties.CustomDictionaryProperty));
+
+ // Act
+ var result = await binder.BindModelAsync(context);
+
+ // Assert
+ Assert.NotNull(result);
+ Assert.False(result.IsFatalError);
+ Assert.True(result.IsModelSet);
+ Assert.Equal(modelName, result.Key);
+ Assert.NotNull(result.ValidationNode);
+
+ var resultDictionary = Assert.IsAssignableFrom>(result.Model);
+ Assert.Equal(expectedDictionary, resultDictionary);
+ }
+
[Fact]
public async Task DictionaryModelBinder_DoesNotCreateCollection_IfIsTopLevelObjectAndIsFirstChanceBinding()
{
@@ -332,8 +367,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
var metadataProvider = context.OperationBindingContext.MetadataProvider;
context.ModelMetadata = metadataProvider.GetMetadataForProperty(
- typeof(ModelWithDictionaryProperty),
- nameof(ModelWithDictionaryProperty.DictionaryProperty));
+ typeof(ModelWithDictionaryProperties),
+ nameof(ModelWithDictionaryProperties.DictionaryProperty));
context.ValueProvider = new TestValueProvider(new Dictionary());
@@ -344,6 +379,39 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
Assert.Null(result);
}
+ // Model type -> can create instance.
+ public static TheoryData CanCreateInstanceData
+ {
+ get
+ {
+ return new TheoryData
+ {
+ { typeof(IEnumerable>), true },
+ { typeof(ICollection>), true },
+ { typeof(IDictionary), true },
+ { typeof(Dictionary), true },
+ { typeof(SortedDictionary), true },
+ { typeof(IList>), false },
+ { typeof(DictionaryWithInternalConstructor), false },
+ { typeof(DictionaryWithThrowingConstructor), false },
+ };
+ }
+ }
+
+ [Theory]
+ [MemberData(nameof(CanCreateInstanceData))]
+ public void CanCreateInstance_ReturnsExpectedValue(Type modelType, bool expectedResult)
+ {
+ // Arrange
+ var binder = new DictionaryModelBinder();
+
+ // Act
+ var result = binder.CanCreateInstance(modelType);
+
+ // Assert
+ Assert.Equal(expectedResult, result);
+ }
+
private static ModelBindingContext CreateContext()
{
var modelBindingContext = new ModelBindingContext()
@@ -401,7 +469,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
private static ModelBindingContext GetModelBindingContext(bool isReadOnly)
{
var metadataProvider = new TestModelMetadataProvider();
- metadataProvider.ForType>().BindingDetails(bd => bd.IsReadOnly = isReadOnly);
+ metadataProvider.ForType>().BindingDetails(bd => bd.IsReadOnly = isReadOnly);
var valueProvider = new SimpleHttpValueProvider
{
{ "someName[0]", new KeyValuePair(42, "forty-two") },
@@ -441,9 +509,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
return mockKvpBinder.Object;
}
- private class ModelWithDictionaryProperty
+ private class ModelWithDictionaryProperties
{
+ // A Dictionary instance cannot be assigned to this property.
+ public SortedDictionary CustomDictionaryProperty { get; set; }
+
public Dictionary DictionaryProperty { get; set; }
+
+ public Dictionary DictionaryWithComplexValuesProperty { get; set; }
+
+ public Dictionary DictionaryWithValueTypesProperty { get; set; }
}
private class ModelWithProperties
@@ -471,6 +546,23 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
return $"{{{ Id }, '{ Name }'}}";
}
}
+
+ private class DictionaryWithInternalConstructor : Dictionary
+ {
+ internal DictionaryWithInternalConstructor()
+ : base()
+ {
+ }
+ }
+
+ private class DictionaryWithThrowingConstructor : Dictionary
+ {
+ public DictionaryWithThrowingConstructor()
+ : base()
+ {
+ throw new ApplicationException("No, don't do this.");
+ }
+ }
}
}
#endif
diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingFromQueryTest.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingFromQueryTest.cs
index 3955eebb3c..dd1496b377 100644
--- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingFromQueryTest.cs
+++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingFromQueryTest.cs
@@ -92,9 +92,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
var server = TestHelper.CreateServer(_app, SiteName, _configureServices);
var client = server.CreateClient();
- var url =
- "http://localhost/FromQueryAttribute_Company/CreateDepartment?TestEmployees[0].EmployeeId=1234";
-
+ var url = "http://localhost/FromQueryAttribute_Company/CreateDepartment?TestEmployees[0].EmployeeId=1234";
// Act
var response = await client.GetAsync(url);
@@ -118,7 +116,6 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
var url =
"http://localhost/FromQueryAttribute_Company/ValidateDepartment?TestEmployees[0].Department=contoso";
-
// Act
var response = await client.GetAsync(url);
diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/ActionParametersIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/ActionParametersIntegrationTest.cs
index 6d9af63990..a691f6fca7 100644
--- a/test/Microsoft.AspNet.Mvc.IntegrationTests/ActionParametersIntegrationTest.cs
+++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/ActionParametersIntegrationTest.cs
@@ -77,7 +77,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
public CustomReadOnlyCollection Address { get; set; }
}
- [Fact(Skip = "Concrete Collection types don't work with GenericModelBinder #2793")]
+ [Fact]
public async Task ActionParameter_ReadOnlyCollectionModel_EmptyPrefix_DoesNotGetBound()
{
// Arrange
@@ -106,12 +106,17 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
Assert.NotNull(boundModel);
Assert.NotNull(boundModel.Address);
- // Arrays should not be updated.
- Assert.Equal(0, boundModel.Address.Count());
+ // Read-only collection should not be updated.
+ Assert.Empty(boundModel.Address);
- // ModelState
+ // ModelState (data is valid but is not copied into Address).
Assert.True(modelState.IsValid);
- Assert.Empty(modelState.Keys);
+ var entry = Assert.Single(modelState);
+ Assert.Equal("Address[0].Street", entry.Key);
+ var state = entry.Value;
+ Assert.NotNull(state);
+ Assert.Equal(ModelValidationState.Valid, state.ValidationState);
+ Assert.Equal("SomeStreet", state.Value.RawValue);
}
private class Person4
@@ -252,7 +257,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState);
}
- [Fact(Skip = "Concrete Collection types don't work with GenericModelBinder #2793")]
+ [Fact]
public async Task ActionParameter_ReadOnlyCollectionModel_WithPrefix_DoesNotGetBound()
{
// Arrange
@@ -285,12 +290,17 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
Assert.NotNull(boundModel);
Assert.NotNull(boundModel.Address);
- // Arrays should not be updated.
- Assert.Equal(0, boundModel.Address.Count());
+ // Read-only collection should not be updated.
+ Assert.Empty(boundModel.Address);
- // ModelState
+ // ModelState (data is valid but is not copied into Address).
Assert.True(modelState.IsValid);
- Assert.Empty(modelState.Keys);
+ var entry = Assert.Single(modelState);
+ Assert.Equal("prefix.Address[0].Street", entry.Key);
+ var state = entry.Value;
+ Assert.NotNull(state);
+ Assert.Equal(ModelValidationState.Valid, state.ValidationState);
+ Assert.Equal("SomeStreet", state.Value.RawValue);
}
[Fact]
@@ -380,11 +390,12 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
Assert.Empty(modelState.Keys);
}
- private class CustomReadOnlyCollection : ICollection, IReadOnlyCollection
+ private class CustomReadOnlyCollection : ICollection
{
private ICollection _original;
- public CustomReadOnlyCollection() : this(new List())
+ public CustomReadOnlyCollection()
+ : this(new List())
{
}
diff --git a/test/Microsoft.AspNet.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs b/test/Microsoft.AspNet.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs
index 2b5882c895..782cac6fea 100644
--- a/test/Microsoft.AspNet.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs
+++ b/test/Microsoft.AspNet.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs
@@ -173,7 +173,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
public CustomReadOnlyCollection Address { get; set; }
}
- [Fact(Skip = "Concrete Collection types don't work with GenericModelBinder #2793")]
+ [Fact(Skip = "Validation incorrect for collections when using TryUpdateModel, #2941")]
public async Task TryUpdateModel_ReadOnlyCollectionModel_EmptyPrefix_DoesNotGetBound()
{
// Arrange
@@ -184,6 +184,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
var modelState = new ModelStateDictionary();
var model = new Person6();
+
// Act
var result = await TryUpdateModel(model, string.Empty, operationContext, modelState);
@@ -193,12 +194,17 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
// Model
Assert.NotNull(model.Address);
- // Arrays should not be updated.
- Assert.Equal(0, model.Address.Count());
+ // Read-only collection should not be updated.
+ Assert.Empty(model.Address);
- // ModelState
+ // ModelState (data is valid but is not copied into Address).
Assert.True(modelState.IsValid);
- Assert.Empty(modelState.Keys);
+ var entry = Assert.Single(modelState);
+ Assert.Equal("Address[0].Street", entry.Key);
+ var state = entry.Value;
+ Assert.NotNull(state);
+ Assert.Equal(ModelValidationState.Valid, state.ValidationState);
+ Assert.Equal("SomeStreet", state.Value.RawValue);
}
private class Person4
@@ -409,7 +415,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState);
}
- [Fact(Skip = "Concrete Collection types don't work with GenericModelBinder #2793")]
+ [Fact(Skip = "Validation incorrect for collections when using TryUpdateModel, #2941")]
public async Task TryUpdateModel_ReadOnlyCollectionModel_WithPrefix_DoesNotGetBound()
{
// Arrange
@@ -420,6 +426,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
var modelState = new ModelStateDictionary();
var model = new Person6();
+
// Act
var result = await TryUpdateModel(model, "prefix", operationContext, modelState);
@@ -429,12 +436,17 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
// Model
Assert.NotNull(model.Address);
- // Arrays should not be updated.
- Assert.Equal(0, model.Address.Count());
+ // Read-only collection should not be updated.
+ Assert.Empty(model.Address);
- // ModelState
+ // ModelState (data is valid but is not copied into Address).
Assert.True(modelState.IsValid);
- Assert.Empty(modelState.Keys);
+ var entry = Assert.Single(modelState);
+ Assert.Equal("prefix.Address[0].Street", entry.Key);
+ var state = entry.Value;
+ Assert.NotNull(state);
+ Assert.Equal(ModelValidationState.Valid, state.ValidationState);
+ Assert.Equal("SomeStreet", state.Value.RawValue);
}
[Fact]
@@ -499,11 +511,12 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
Assert.Empty(modelState.Keys);
}
- private class CustomReadOnlyCollection : ICollection, IReadOnlyCollection
+ private class CustomReadOnlyCollection : ICollection
{
private ICollection _original;
- public CustomReadOnlyCollection() : this(new List())
+ public CustomReadOnlyCollection()
+ : this(new List())
{
}