CompositeValueProvider.GetKeysFromPrefixAsync throws null if provider is not an

IEnumerableValueProvider

Fixes #1419

src/Microsoft.AspNet.Mvc.ModelBinding/ValueProviders/CompositeValueProvider.cs

@@ -9,18 +9,30 @@ using System.Threading.Tasks;
 
 namespace Microsoft.AspNet.Mvc.ModelBinding
 {
+    /// <summary>
+    /// Represents a <see cref="IValueProvider"/> whose values come from a collection of <see cref="IValueProvider"/>s.
+    /// </summary>
     public class CompositeValueProvider : Collection<IValueProvider>, IEnumerableValueProvider, IMetadataAwareValueProvider
     {
+        /// <summary>
+        /// Initializes a new instance of <see cref="CompositeValueProvider"/>.
+        /// </summary>
         public CompositeValueProvider()
             : base()
         {
         }
 
+        /// <summary>
+        /// Initializes a new instance of <see cref="CompositeValueProvider"/>.
+        /// </summary>
+        /// <param name="valueProviders">The sequence of <see cref="IValueProvider"/> to add to this instance of
+        /// <see cref="CompositeValueProvider"/>.</param>
         public CompositeValueProvider(IEnumerable<IValueProvider> valueProviders)
             : base(valueProviders.ToList())
         {
         }
 
+        /// <inheritdoc />
         public virtual async Task<bool> ContainsPrefixAsync(string prefix)
         {
             for (var i = 0; i < Count; i++)
@@ -33,6 +45,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             return false;
         }
 
+        /// <inheritdoc />
         public virtual async Task<ValueProviderResult> GetValueAsync(string key)
         {
             // Performance-sensitive
@@ -50,36 +63,37 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
             return null;
         }
 
+        /// <inheritdoc />
         public virtual async Task<IDictionary<string, string>> GetKeysFromPrefixAsync(string prefix)
         {
             foreach (var valueProvider in this)
             {
-                var result = await GetKeysFromPrefixFromProvider(valueProvider, prefix);
+                var enumeratedProvider = valueProvider as IEnumerableValueProvider;
-                if (result != null && result.Count > 0)
+                if (enumeratedProvider != null)
                 {
-                    return result;
+                    var result = await enumeratedProvider.GetKeysFromPrefixAsync(prefix);
+                    if (result != null && result.Count > 0)
+                    {
+                        return result;
+                    }
                 }
             }
             return new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
         }
 
-        private static Task<IDictionary<string, string>> GetKeysFromPrefixFromProvider(IValueProvider provider,
+        /// <inheritdoc />
-                                                                                       string prefix)
-        {
-            var enumeratedProvider = provider as IEnumerableValueProvider;
-            return (enumeratedProvider != null) ? enumeratedProvider.GetKeysFromPrefixAsync(prefix) : null;
-        }
-
         protected override void InsertItem(int index, [NotNull] IValueProvider item)
         {
             base.InsertItem(index, item);
         }
 
+        /// <inheritdoc />
         protected override void SetItem(int index, [NotNull] IValueProvider item)
         {
             base.SetItem(index, item);
         }
 
+        /// <inheritdoc />
         public IValueProvider Filter(IValueProviderMetadata valueBinderMetadata)
         {
             var filteredValueProviders = new List<IValueProvider>();

test/Microsoft.AspNet.Mvc.ModelBinding.Test/ValueProviders/CompositeValueProviderTests.cs

@@ -5,10 +5,11 @@
 
 using System;
 using System.Collections.Generic;
+using System.Threading.Tasks;
 using Moq;
 using Xunit;
 
-namespace Microsoft.AspNet.Mvc.ModelBinding.Test
+namespace Microsoft.AspNet.Mvc.ModelBinding
 {
     public class CompositeValueProviderTests
     {
@@ -21,6 +22,46 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
             }
         }
 
+        [Fact]
+        public async Task GetKeysFromPrefixAsync_ReturnsResultFromFirstValueProviderThatReturnsValues()
+        {
+            // Arrange
+            var provider1 = Mock.Of<IValueProvider>();
+            var dictionary =  new Dictionary<string, string>(StringComparer.Ordinal)
+            {
+                { "prefix-test", "some-value" },
+            };
+            var provider2 = new Mock<IEnumerableValueProvider>();
+            provider2.Setup(p => p.GetKeysFromPrefixAsync("prefix"))
+                     .Returns(Task.FromResult<IDictionary<string, string>>(dictionary))
+                     .Verifiable();
+            var provider = new CompositeValueProvider(new[] { provider1, provider2.Object });
+
+            // Act
+            var values = await provider.GetKeysFromPrefixAsync("prefix");
+
+            // Assert
+            var result = Assert.Single(values);
+            Assert.Equal("prefix-test", result.Key);
+            Assert.Equal("some-value", result.Value);
+            provider2.Verify();
+        }
+
+        [Fact]
+        public async Task GetKeysFromPrefixAsync_ReturnsEmptyDictionaryIfNoValueProviderReturnsValues()
+        {
+            // Arrange
+            var provider1 = Mock.Of<IValueProvider>();
+            var provider2 = Mock.Of<IValueProvider>();
+            var provider = new CompositeValueProvider(new[] { provider1, provider2 });
+
+            // Act
+            var values = await provider.GetKeysFromPrefixAsync("prefix");
+
+            // Assert
+            Assert.Empty(values);
+        }
+
         [Theory]
         [MemberData(nameof(RegisteredAsMetadataClasses))]
         public void FilterReturnsItself_ForAnyClassRegisteredAsGenericParam(IValueProviderMetadata metadata)