Handle broader range of collection types in model binding

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,29 @@ public override Task<ModelBindingResult> BindModelAsync([NotNull] ModelBindingCo
             return base.BindModelAsync(bindingContext);
         }
 
-        protected override object CreateEmptyCollection()
+        /// <inheritdoc />
+        public override bool CanCreateInstance(Type targetType)
+        {
+            Debug.Assert(targetType == typeof(TElement[]), "GenericModelBinder only creates this binder for arrays.");
+
+            return true;
+        }
+
+        /// <inheritdoc />
+        protected override object CreateEmptyCollection(Type targetType)
         {
+            Debug.Assert(targetType == typeof(TElement[]), "GenericModelBinder only creates this binder for arrays.");
+
             return new TElement[0];
         }
 
         /// <inheritdoc />
-        protected override object GetModel(IEnumerable<TElement> newCollection)
+        protected override object ConvertToCollectionType(Type targetType, IEnumerable<TElement> collection)
         {
-            return newCollection?.ToArray();
+            Debug.Assert(targetType == typeof(TElement[]), "GenericModelBinder only creates this binder for arrays.");
+
+            // If non-null, collection is a List<TElement>, never already a TElement[].
+            return collection?.ToArray();
         }
 
         /// <inheritdoc />

src/Microsoft.AspNet.Mvc.Core/ModelBinding/CollectionModelBinder.cs

@@ -7,6 +7,9 @@
 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
     /// <see cref="IModelBinder"/> implementation for binding collection values.
     /// </summary>
     /// <typeparam name="TElement">Type of elements in the collection.</typeparam>
-    public class CollectionModelBinder<TElement> : IModelBinder
+    public class CollectionModelBinder<TElement> : ICollectionModelBinder
     {
         /// <inheritdoc />
         public virtual async Task<ModelBindingResult> 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,26 +55,23 @@ public virtual async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBind
 
             var valueProviderResult = await bindingContext.ValueProvider.GetValueAsync(bindingContext.ModelName);
 
-            IEnumerable<TElement> boundCollection;
             CollectionResult result;
             if (valueProviderResult == null)
             {
                 result = await BindComplexCollection(bindingContext);
-                boundCollection = result.Model;
             }
             else
             {
                 result = await BindSimpleCollection(
                     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 @@ public virtual async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBind
                 validationNode: result?.ValidationNode);
         }
 
-        // Called when we're creating a default 'empty' model for a top level bind.
-        protected virtual object CreateEmptyCollection()
+        /// <inheritdoc />
+        public virtual bool CanCreateInstance(Type targetType)
+        {
+            return CreateEmptyCollection(targetType) != null;
+        }
+
+        /// <summary>
+        /// Create an <see cref="object"/> assignable to <paramref name="targetType"/>.
+        /// </summary>
+        /// <param name="targetType"><see cref="Type"/> of the model.</param>
+        /// <returns>An <see cref="object"/> assignable to <paramref name="targetType"/>.</returns>
+        /// <remarks>Called when creating a default 'empty' model for a top level bind.</remarks>
+        protected virtual object CreateEmptyCollection(Type targetType)
+        {
+            if (targetType.IsAssignableFrom(typeof(List<TElement>)))
+            {
+                // Simple case such as ICollection<TElement>, IEnumerable<TElement> and IList<TElement>.
+                return new List<TElement>();
+            }
+
+            return CreateInstance(targetType);
+        }
+
+        /// <summary>
+        /// Create an instance of <paramref name="targetType"/>.
+        /// </summary>
+        /// <param name="targetType"><see cref="Type"/> of the model.</param>
+        /// <returns>An instance of <paramref name="targetType"/>.</returns>
+        protected object CreateInstance(Type targetType)
         {
-            return new List<TElement>();
+            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<CollectionResult> BindSimpleCollection(
             ModelBindingContext bindingContext,
             object rawValue,
@@ -156,6 +194,7 @@ private async Task<CollectionResult> BindComplexCollection(ModelBindingContext b
             return await BindComplexCollectionFromIndexes(bindingContext, indexNames);
         }
 
+        // Internal for testing.
         internal async Task<CollectionResult> BindComplexCollectionFromIndexes(
             ModelBindingContext bindingContext,
             IEnumerable<string> indexNames)
@@ -219,6 +258,7 @@ internal async Task<CollectionResult> BindComplexCollectionFromIndexes(
             };
         }
 
+        // Internal for testing.
         internal class CollectionResult
         {
             public ModelValidationNode ValidationNode { get; set; }
@@ -227,23 +267,37 @@ internal class CollectionResult
         }
 
         /// <summary>
-        /// Gets an <see cref="object"/> assignable to the collection property.
+        /// Gets an <see cref="object"/> assignable to <paramref name="targetType"/> that contains members from
+        /// <paramref name="collection"/>.
         /// </summary>
-        /// <param name="newCollection">
+        /// <param name="targetType"><see cref="Type"/> of the model.</param>
+        /// <param name="collection">
         /// Collection of values retrieved from value providers. Or <c>null</c> if nothing was bound.
         /// </param>
         /// <returns>
-        /// <see cref="object"/> assignable to the collection property. Or <c>null</c> if nothing was bound.
+        /// An <see cref="object"/> assignable to <paramref name="targetType"/>. Or <c>null</c> if nothing was bound.
         /// </returns>
         /// <remarks>
         /// Extensibility point that allows the bound collection to be manipulated or transformed before being
         /// returned from the binder.
         /// </remarks>
-        protected virtual object GetModel(IEnumerable<TElement> newCollection)
+        protected virtual object ConvertToCollectionType(Type targetType, IEnumerable<TElement> collection)
         {
-            // Depends on fact BindSimpleCollection() and BindComplexCollection() always return a List<TElement>
-            // instance or null. In addition GenericModelBinder confirms a List<TElement> 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<TElement>)))
+            {
+                // Depends on fact BindSimpleCollection() and BindComplexCollection() always return a List<TElement>
+                // instance or null.
+                return collection;
+            }
+
+            var newCollection = CreateInstance(targetType);
+            CopyToModel(newCollection, collection);
+
             return newCollection;
         }
 
@@ -254,11 +308,10 @@ protected virtual object GetModel(IEnumerable<TElement> newCollection)
         /// <param name="sourceCollection">
         /// Collection of values retrieved from value providers. Or <c>null</c> if nothing was bound.
         /// </param>
-        /// <remarks>Called only in TryUpdateModelAsync(collection, ...) scenarios.</remarks>
         protected virtual void CopyToModel([NotNull] object target, IEnumerable<TElement> sourceCollection)
         {
             var targetCollection = target as ICollection<TElement>;
-            Debug.Assert(targetCollection != null); // This binder is instantiated only for ICollection model types.
+            Debug.Assert(targetCollection != null, "This binder is instantiated only for ICollection<T> model types.");
 
             if (sourceCollection != null && targetCollection != null && !targetCollection.IsReadOnly)
             {
@@ -270,7 +323,7 @@ protected virtual void CopyToModel([NotNull] object target, IEnumerable<TElement
             }
         }
 
-        internal static object[] RawValueToObjectArray(object rawValue)
+        private static object[] RawValueToObjectArray(object rawValue)
         {
             // precondition: rawValue is not null
 

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 @@ public override async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBin
             }
 
             Debug.Assert(result.Model != null);
-            var model = (Dictionary<TKey, TValue>)result.Model;
+            var model = (IDictionary<TKey, TValue>)result.Model;
             if (model.Count != 0)
             {
                 // ICollection<KeyValuePair<TKey, TValue>> approach was successful.
@@ -80,15 +84,37 @@ public override async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBin
         }
 
         /// <inheritdoc />
-        protected override object GetModel(IEnumerable<KeyValuePair<TKey, TValue>> newCollection)
+        protected override object ConvertToCollectionType(
+            Type targetType,
+            IEnumerable<KeyValuePair<TKey, TValue>> collection)
         {
-            return newCollection?.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
+            if (collection == null)
+            {
+                return null;
+            }
+
+            if (targetType.IsAssignableFrom(typeof(Dictionary<TKey, TValue>)))
+            {
+                // Collection is a List<KeyValuePair<TKey, TValue>>, never already a Dictionary<TKey, TValue>.
+                return collection.ToDictionary(kvp => kvp.Key, kvp => kvp.Value);
+            }
+
+            var newCollection = CreateInstance(targetType);
+            CopyToModel(newCollection, collection);
+
+            return newCollection;
         }
 
         /// <inheritdoc />
-        protected override object CreateEmptyCollection()
+        protected override object CreateEmptyCollection(Type targetType)
         {
-            return new Dictionary<TKey, TValue>();
+            if (targetType.IsAssignableFrom(typeof(Dictionary<TKey, TValue>)))
+            {
+                // Simple case such as IDictionary<TKey, TValue>.
+                return new Dictionary<TKey, TValue>();
+            }
+
+            return CreateInstance(targetType);
         }
 
         private static TKey ConvertFromString(string keyString)