diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/CollectionModelBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/CollectionModelBinder.cs index 932d34a032..53b4122c86 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/CollectionModelBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/CollectionModelBinder.cs @@ -7,9 +7,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Globalization; using System.Linq; -#if NETSTANDARD1_3 using System.Reflection; -#endif using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; @@ -45,7 +43,6 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } bindingContext.Result = ModelBindingResult.Success(bindingContext.ModelName, model); - return; } return; @@ -85,7 +82,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding if (valueProviderResult != ValueProviderResult.None) { - // If we did simple binding, then modelstate should be updated to reflect what we bound for ModelName. + // If we did simple binding, then modelstate should be updated to reflect what we bound for ModelName. // If we did complex binding, there will already be an entry for each index. bindingContext.ModelState.SetModelValue( bindingContext.ModelName, @@ -93,13 +90,20 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } bindingContext.Result = ModelBindingResult.Success(bindingContext.ModelName, model); - return; } /// public virtual bool CanCreateInstance(Type targetType) { - return CreateEmptyCollection(targetType) != null; + if (targetType.IsAssignableFrom(typeof(List))) + { + // Simple case such as ICollection, IEnumerable and IList. + return true; + } + + return targetType.GetTypeInfo().IsClass && + !targetType.GetTypeInfo().IsAbstract && + typeof(ICollection).IsAssignableFrom(targetType); } /// @@ -126,15 +130,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// An instance of . protected object CreateInstance(Type targetType) { - try - { - return Activator.CreateInstance(targetType); - } - catch (Exception) - { - // Details of exception are not important. - return null; - } + return Activator.CreateInstance(targetType); } // Used when the ValueProvider contains the collection to be bound as a single element, e.g. the raw value diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/DictionaryModelBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/DictionaryModelBinder.cs index e08a20be20..419b242d14 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/DictionaryModelBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/DictionaryModelBinder.cs @@ -113,10 +113,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding return collection.ToDictionary(kvp => kvp.Key, kvp => kvp.Value); } - var newCollection = CreateInstance(targetType); - CopyToModel(newCollection, collection); - - return newCollection; + return base.ConvertToCollectionType(targetType, collection); } /// @@ -128,7 +125,18 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding return new Dictionary(); } - return CreateInstance(targetType); + return base.CreateEmptyCollection(targetType); + } + + public override bool CanCreateInstance(Type targetType) + { + if (targetType.IsAssignableFrom(typeof(Dictionary))) + { + // Simple case such as IDictionary. + return true; + } + + return base.CanCreateInstance(targetType); } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/FormFileModelBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/FormFileModelBinder.cs index c405b5d250..7387fc6a7b 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/FormFileModelBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/FormFileModelBinder.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.Diagnostics; using System.Linq; #if NETSTANDARD1_3 @@ -12,7 +13,6 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; -using Microsoft.Net.Http.Headers; namespace Microsoft.AspNetCore.Mvc.ModelBinding { @@ -30,72 +30,111 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } // This method is optimized to use cached tasks when possible and avoid allocating - // using Task.FromResult. If you need to make changes of this nature, profile - // allocations afterwards and look for Task. + // using Task.FromResult or async state machines. - if (bindingContext.ModelType != typeof(IFormFile) && - !typeof(IEnumerable).IsAssignableFrom(bindingContext.ModelType)) + var modelType = bindingContext.ModelType; + if (modelType != typeof(IFormFile) && !typeof(IEnumerable).IsAssignableFrom(modelType)) { + // Not a type this model binder supports. Let other binders run. return TaskCache.CompletedTask; } - return BindModelCoreAsync(bindingContext); + var createFileCollection = modelType == typeof(IFormFileCollection) && + !bindingContext.ModelMetadata.IsReadOnly; + if (!createFileCollection && !ModelBindingHelper.CanGetCompatibleCollection(bindingContext)) + { + // Silently fail and stop other model binders running if unable to create an instance or use the + // current instance. + bindingContext.Result = ModelBindingResult.Failed(bindingContext.ModelName); + return TaskCache.CompletedTask; + } + + ICollection postedFiles; + if (createFileCollection) + { + postedFiles = new List(); + } + else + { + postedFiles = ModelBindingHelper.GetCompatibleCollection(bindingContext); + } + + return BindModelCoreAsync(bindingContext, postedFiles); } - private async Task BindModelCoreAsync(ModelBindingContext bindingContext) + private async Task BindModelCoreAsync(ModelBindingContext bindingContext, ICollection postedFiles) { - // If we're at the top level, then use the FieldName (paramter or property name). + Debug.Assert(postedFiles != null); + + // If we're at the top level, then use the FieldName (parameter or property name). // This handles the fact that there will be nothing in the ValueProviders for this parameter // and so we'll do the right thing even though we 'fell-back' to the empty prefix. var modelName = bindingContext.IsTopLevelObject ? bindingContext.BinderModelName ?? bindingContext.FieldName : bindingContext.ModelName; + await GetFormFilesAsync(modelName, bindingContext, postedFiles); + object value; if (bindingContext.ModelType == typeof(IFormFile)) { - var postedFiles = await GetFormFilesAsync(modelName, bindingContext); - value = postedFiles.FirstOrDefault(); - } - else if (typeof(IEnumerable).IsAssignableFrom(bindingContext.ModelType)) - { - var postedFiles = await GetFormFilesAsync(modelName, bindingContext); - value = ModelBindingHelper.ConvertValuesToCollectionType(bindingContext.ModelType, postedFiles); - } - else - { - // This binder does not support the requested type. - Debug.Fail("We shouldn't be called without a matching type."); - return; - } - - if (value == null) - { - bindingContext.Result = ModelBindingResult.Failed(bindingContext.ModelName); - return; - } - else - { - bindingContext.ValidationState.Add(value, new ValidationStateEntry() + if (postedFiles.Count == 0) { - Key = modelName, - SuppressValidation = true - }); + // Silently fail if the named file does not exist in the request. + bindingContext.Result = ModelBindingResult.Failed(bindingContext.ModelName); + return; + } - bindingContext.ModelState.SetModelValue( - modelName, - rawValue: null, - attemptedValue: null); - - bindingContext.Result = ModelBindingResult.Success(bindingContext.ModelName, value); - return; + value = postedFiles.First(); } + else + { + if (postedFiles.Count == 0 && !bindingContext.IsTopLevelObject) + { + // Silently fail if no files match. Will bind to an empty collection (treat empty as a success + // case and not reach here) if binding to a top-level object. + bindingContext.Result = ModelBindingResult.Failed(bindingContext.ModelName); + return; + } + + // Perform any final type mangling needed. + var modelType = bindingContext.ModelType; + if (modelType == typeof(IFormFile[])) + { + Debug.Assert(postedFiles is List); + value = ((List)postedFiles).ToArray(); + } + else if (modelType == typeof(IFormFileCollection)) + { + Debug.Assert(postedFiles is List); + value = new FileCollection((List)postedFiles); + } + else + { + value = postedFiles; + } + } + + bindingContext.ValidationState.Add(value, new ValidationStateEntry() + { + Key = modelName, + SuppressValidation = true + }); + + bindingContext.ModelState.SetModelValue( + modelName, + rawValue: null, + attemptedValue: null); + + bindingContext.Result = ModelBindingResult.Success(bindingContext.ModelName, value); } - private async Task> GetFormFilesAsync(string modelName, ModelBindingContext bindingContext) + private async Task GetFormFilesAsync( + string modelName, + ModelBindingContext bindingContext, + ICollection postedFiles) { var request = bindingContext.OperationBindingContext.HttpContext.Request; - var postedFiles = new List(); if (request.HasFormContentType) { var form = await request.ReadFormAsync(); @@ -114,8 +153,45 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } } } + } - return postedFiles; + private class FileCollection : ReadOnlyCollection, IFormFileCollection + { + public FileCollection(List list) + : base(list) + { + } + + public IFormFile this[string name] => GetFile(name); + + public IFormFile GetFile(string name) + { + for (var i = 0; i < Items.Count; i++) + { + var file = Items[i]; + if (string.Equals(name, file.Name, StringComparison.OrdinalIgnoreCase)) + { + return file; + } + } + + return null; + } + + public IReadOnlyList GetFiles(string name) + { + var files = new List(); + for (var i = 0; i < Items.Count; i++) + { + var file = Items[i]; + if (string.Equals(name, file.Name, StringComparison.OrdinalIgnoreCase)) + { + files.Add(file); + } + } + + return files; + } } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/HeaderModelBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/HeaderModelBinder.cs index 5b5d192c1e..72655a9595 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/HeaderModelBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/HeaderModelBinder.cs @@ -2,8 +2,6 @@ // 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; #if NETSTANDARD1_3 using System.Reflection; #endif @@ -28,8 +26,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } // This method is optimized to use cached tasks when possible and avoid allocating - // using Task.FromResult. If you need to make changes of this nature, profile - // allocations afterwards and look for Task. + // using Task.FromResult or async state machines. var allowedBindingSource = bindingContext.BindingSource; if (allowedBindingSource == null || @@ -41,34 +38,37 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } var request = bindingContext.OperationBindingContext.HttpContext.Request; - var modelMetadata = bindingContext.ModelMetadata; // Property name can be null if the model metadata represents a type (rather than a property or parameter). var headerName = bindingContext.FieldName; - object model = null; - if (bindingContext.ModelType == typeof(string)) + + object model; + if (ModelBindingHelper.CanGetCompatibleCollection(bindingContext)) { - string value = request.Headers[headerName]; - if (value != null) + if (bindingContext.ModelType == typeof(string)) { - model = value; + var value = request.Headers[headerName]; + model = (string)value; + } + else + { + var values = request.Headers.GetCommaSeparatedValues(headerName); + model = GetCompatibleCollection(bindingContext, values); } } - else if (typeof(IEnumerable).IsAssignableFrom(bindingContext.ModelType)) + else { - var values = request.Headers.GetCommaSeparatedValues(headerName); - if (values.Length > 0) - { - model = ModelBindingHelper.ConvertValuesToCollectionType( - bindingContext.ModelType, - values); - } + // An unsupported datatype or a new collection is needed (perhaps because target type is an array) but + // can't assign it to the property. + model = null; } if (model == null) { + // Silently fail if unable to create an instance or use the current instance. Also reach here in the + // typeof(string) case if the header does not exist in the request and in the + // typeof(IEnumerable) case if the header does not exist and this is not a top-level object. bindingContext.Result = ModelBindingResult.Failed(bindingContext.ModelName); - return TaskCache.CompletedTask; } else { @@ -78,8 +78,32 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding request.Headers[headerName]); bindingContext.Result = ModelBindingResult.Success(bindingContext.ModelName, model); - return TaskCache.CompletedTask; } + + return TaskCache.CompletedTask; + } + + private static object GetCompatibleCollection(ModelBindingContext bindingContext, string[] values) + { + // Almost-always success if IsTopLevelObject. + if (!bindingContext.IsTopLevelObject && values.Length == 0) + { + return null; + } + + if (bindingContext.ModelType.IsAssignableFrom(typeof(string[]))) + { + // Array we already have is compatible. + return values; + } + + var collection = ModelBindingHelper.GetCompatibleCollection(bindingContext, values.Length); + for (int i = 0; i < values.Length; i++) + { + collection.Add(values[i]); + } + + return collection; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBindingHelper.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBindingHelper.cs index 1ba5650f31..9e1a376389 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBindingHelper.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBindingHelper.cs @@ -748,46 +748,146 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding return (model is TModel) ? (TModel)model : default(TModel); } - public static object ConvertValuesToCollectionType(Type modelType, IList values) + /// + /// Gets an indication whether is likely to return a usable + /// non-null value. + /// + /// The element type of the required. + /// The . + /// + /// true if is likely to return a usable non-null + /// value; false otherwise. + /// + /// "Usable" in this context means the property can be set or its value reused. + public static bool CanGetCompatibleCollection(ModelBindingContext bindingContext) { - // There's a limited set of collection types we can support here. - // - // For the simple cases - choose a T[] or List if the destination type supports - // it. - // - // For more complex cases, if the destination type is a class and implements ICollection - // then activate it and add the values. - // - // Otherwise just give up. - if (typeof(List).IsAssignableFrom(modelType)) + var model = bindingContext.Model; + var modelType = bindingContext.ModelType; + var writeable = !bindingContext.ModelMetadata.IsReadOnly; + if (typeof(T).IsAssignableFrom(modelType)) { - return new List(values); + // Scalar case. Existing model is not relevant and property must always be set. Will use a List + // intermediate and set property to first element, if any. + return writeable; } - else if (typeof(T[]).IsAssignableFrom(modelType)) + + if (modelType == typeof(T[])) { - return values.ToArray(); + // Can't change the length of an existing array or replace it. Will use a List intermediate and set + // property to an array created from that. + return writeable; } - else if ( + + if (!typeof(IEnumerable).IsAssignableFrom(modelType)) + { + // Not a supported collection. + return false; + } + + var collection = model as ICollection; + if (collection != null && !collection.IsReadOnly) + { + // Can use the existing collection. + return true; + } + + // Most likely the model is null. + // Also covers the corner case where the model implements IEnumerable but not ICollection e.g. + // public IEnumerable Property { get; set; } = new T[0]; + if (modelType.IsAssignableFrom(typeof(List))) + { + return writeable; + } + + // Will we be able to activate an instance and use that? + return writeable && modelType.GetTypeInfo().IsClass && !modelType.GetTypeInfo().IsAbstract && - typeof(ICollection).IsAssignableFrom(modelType)) - { - var result = (ICollection)Activator.CreateInstance(modelType); - foreach (var value in values) - { - result.Add(value); - } + typeof(ICollection).IsAssignableFrom(modelType); + } - return result; - } - else if (typeof(IEnumerable).IsAssignableFrom(modelType)) + /// + /// Creates an instance compatible with 's + /// . + /// + /// The element type of the required. + /// The . + /// + /// An instance compatible with 's + /// . + /// + /// + /// Should not be called if returned false. + /// + public static ICollection GetCompatibleCollection(ModelBindingContext bindingContext) + { + return GetCompatibleCollection(bindingContext, capacity: null); + } + + /// + /// Creates an instance compatible with 's + /// . + /// + /// The element type of the required. + /// The . + /// + /// Capacity for use when creating a instance. Not used when creating another type. + /// + /// + /// An instance compatible with 's + /// . + /// + /// + /// Should not be called if returned false. + /// + public static ICollection GetCompatibleCollection(ModelBindingContext bindingContext, int capacity) + { + return GetCompatibleCollection(bindingContext, (int?)capacity); + } + + private static ICollection GetCompatibleCollection(ModelBindingContext bindingContext, int? capacity) + { + var model = bindingContext.Model; + var modelType = bindingContext.ModelType; + + // There's a limited set of collection types we can create here. + // + // For the simple cases: Choose List if the destination type supports it (at least as an intermediary). + // + // For more complex cases: If the destination type is a class that implements ICollection, then activate + // an instance and return that. + // + // Otherwise just give up. + if (typeof(T).IsAssignableFrom(modelType)) { - return values; + return CreateList(capacity); } - else + + if (modelType == typeof(T[])) { - return null; + return CreateList(capacity); } + + // Does collection exist and can it be reused? + var collection = model as ICollection; + if (collection != null && !collection.IsReadOnly) + { + collection.Clear(); + + return collection; + } + + if (modelType.IsAssignableFrom(typeof(List))) + { + return CreateList(capacity); + } + + return (ICollection)Activator.CreateInstance(modelType); + } + + private static List CreateList(int? capacity) + { + return capacity.HasValue ? new List(capacity.Value) : new List(); } /// @@ -860,7 +960,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding return type.GetTypeInfo().IsValueType ? Activator.CreateInstance(type) : null; } - if (value.GetType().IsAssignableFrom(type)) + if (type.IsAssignableFrom(value.GetType())) { return value; } @@ -917,7 +1017,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding private static object ConvertSimpleType(object value, Type destinationType, CultureInfo culture) { - if (value == null || value.GetType().IsAssignableFrom(destinationType)) + if (value == null || destinationType.IsAssignableFrom(value.GetType())) { return value; } diff --git a/src/Microsoft.AspNetCore.Mvc.Razor/RazorPageActivator.cs b/src/Microsoft.AspNetCore.Mvc.Razor/RazorPageActivator.cs index fbc72d27e4..c074a42f73 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor/RazorPageActivator.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor/RazorPageActivator.cs @@ -110,10 +110,16 @@ namespace Microsoft.AspNetCore.Mvc.Razor Func valueAccessor; if (typeof(ViewDataDictionary).IsAssignableFrom(property.PropertyType)) { + // Logic looks reversed in condition above but is OK. Support only properties of base + // ViewDataDictionary type and activationInfo.ViewDataDictionaryType. VDD will fail when + // assigning to the property (InvalidCastException) and that's fine. valueAccessor = context => context.ViewData; } - else if (typeof(IUrlHelper).IsAssignableFrom(property.PropertyType)) + else if (property.PropertyType == typeof(IUrlHelper)) { + // W.r.t. specificity of above condition: Users are much more likely to inject their own + // IUrlHelperFactory than to create a class implementing IUrlHelper (or a sub-interface) and inject + // that. But the second scenario is supported. (Note the class must implement ICanHasViewContext.) valueAccessor = context => { var serviceProvider = context.HttpContext.RequestServices; diff --git a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewComponents/DefaultViewComponentDescriptorProvider.cs b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewComponents/DefaultViewComponentDescriptorProvider.cs index 682d2e2495..ab54680b1a 100644 --- a/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewComponents/DefaultViewComponentDescriptorProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewComponents/DefaultViewComponentDescriptorProvider.cs @@ -115,13 +115,14 @@ namespace Microsoft.AspNetCore.Mvc.ViewComponents } else { + // Will invoke synchronously. Method must not return void, Task or Task. if (selectedMethod.ReturnType == typeof(void)) { throw new InvalidOperationException(Resources.FormatViewComponent_SyncMethod_ShouldReturnValue( SyncMethodName, componentName)); } - else if (selectedMethod.ReturnType.IsAssignableFrom(typeof(Task))) + else if (typeof(Task).IsAssignableFrom(selectedMethod.ReturnType)) { throw new InvalidOperationException(Resources.FormatViewComponent_SyncMethod_CannotReturnTask( SyncMethodName, diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs index c6d3c9796d..b234ae2cd6 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/CollectionModelBinderTest.cs @@ -317,8 +317,6 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { typeof(List), true }, { typeof(LinkedList), true }, { typeof(ISet), false }, - { typeof(ListWithInternalConstructor), false }, - { typeof(ListWithThrowingConstructor), false }, }; } } @@ -446,22 +444,5 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public string Name { get; set; } } - - private class ListWithInternalConstructor : List - { - internal ListWithInternalConstructor() - : base() - { - } - } - - private class ListWithThrowingConstructor : List - { - public ListWithThrowingConstructor() - : base() - { - throw new RankException("No, don't do this."); - } - } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/DictionaryModelBinderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/DictionaryModelBinderTest.cs index d9ad2333ec..c582e13c0d 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/DictionaryModelBinderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/DictionaryModelBinderTest.cs @@ -388,9 +388,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Test { typeof(IDictionary), true }, { typeof(Dictionary), true }, { typeof(SortedDictionary), true }, - { typeof(IList>), false }, - { typeof(DictionaryWithInternalConstructor), false }, - { typeof(DictionaryWithThrowingConstructor), false }, + { typeof(IList>), true }, + { typeof(ISet>), false }, }; } } @@ -554,22 +553,5 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Test return $"{{{ Id }, '{ Name }'}}"; } } - - private class DictionaryWithInternalConstructor : Dictionary - { - internal DictionaryWithInternalConstructor() - : base() - { - } - } - - private class DictionaryWithThrowingConstructor : Dictionary - { - public DictionaryWithThrowingConstructor() - : base() - { - throw new RankException("No, don't do this."); - } - } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/FormFileModelBinderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/FormFileModelBinderTest.cs index db9f39857a..13c818ce11 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/FormFileModelBinderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/FormFileModelBinderTest.cs @@ -8,7 +8,6 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; -using Microsoft.Net.Http.Headers; using Moq; using Xunit; @@ -43,9 +42,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public async Task FormFileModelBinder_ExpectMultipleFiles_BindSuccessful() { // Arrange - var formFiles = new FormFileCollection(); - formFiles.Add(GetMockFormFile("file", "file1.txt")); - formFiles.Add(GetMockFormFile("file", "file2.txt")); + var formFiles = GetTwoFiles(); var httpContext = GetMockHttpContext(GetMockFormCollection(formFiles)); var bindingContext = GetBindingContext(typeof(IEnumerable), httpContext); var binder = new FormFileModelBinder(); @@ -66,13 +63,38 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Assert.Equal(2, files.Count); } + [Theory] + [InlineData(typeof(IFormFile[]))] + [InlineData(typeof(ICollection))] + [InlineData(typeof(IList))] + [InlineData(typeof(IFormFileCollection))] + [InlineData(typeof(List))] + [InlineData(typeof(LinkedList))] + [InlineData(typeof(FileList))] + [InlineData(typeof(FormFileCollection))] + public async Task FormFileModelBinder_BindsFiles_ForCollectionsItCanCreate(Type destinationType) + { + // Arrange + var binder = new FormFileModelBinder(); + var formFiles = GetTwoFiles(); + var httpContext = GetMockHttpContext(GetMockFormCollection(formFiles)); + var bindingContext = GetBindingContext(destinationType, httpContext); + + // Act + var result = await binder.BindModelResultAsync(bindingContext); + + // Assert + Assert.NotEqual(default(ModelBindingResult), result); + Assert.True(result.IsModelSet); + Assert.IsAssignableFrom(destinationType, result.Model); + Assert.Equal(formFiles, result.Model as IEnumerable); + } + [Fact] public async Task FormFileModelBinder_ExpectSingleFile_BindFirstFile() { // Arrange - var formFiles = new FormFileCollection(); - formFiles.Add(GetMockFormFile("file", "file1.txt")); - formFiles.Add(GetMockFormFile("file", "file2.txt")); + var formFiles = GetTwoFiles(); var httpContext = GetMockHttpContext(GetMockFormCollection(formFiles)); var bindingContext = GetBindingContext(typeof(IFormFile), httpContext); var binder = new FormFileModelBinder(); @@ -86,8 +108,26 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Assert.Equal("file1.txt", file.FileName); } + [Theory] + [InlineData(typeof(string))] + [InlineData(typeof(IEnumerable))] + public async Task FormFileModelBinder_ReturnsNothing_ForUnsupportedDestinationTypes(Type destinationType) + { + // Arrange + var formFiles = GetTwoFiles(); + var httpContext = GetMockHttpContext(GetMockFormCollection(formFiles)); + var bindingContext = GetBindingContext(destinationType, httpContext); + var binder = new FormFileModelBinder(); + + // Act + var result = await binder.BindModelResultAsync(bindingContext); + + // Assert + Assert.Equal(default(ModelBindingResult), result); + } + [Fact] - public async Task FormFileModelBinder_ReturnsNothing_WhenNoFilePosted() + public async Task FormFileModelBinder_ReturnsFailedResult_WhenNoFilePosted() { // Arrange var formFiles = new FormFileCollection(); @@ -100,11 +140,12 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding // Assert Assert.NotEqual(default(ModelBindingResult), result); + Assert.False(result.IsModelSet); Assert.Null(result.Model); } [Fact] - public async Task FormFileModelBinder_ReturnsNothing_WhenNamesDontMatch() + public async Task FormFileModelBinder_ReturnsFailedResult_WhenNamesDoNotMatch() { // Arrange var formFiles = new FormFileCollection(); @@ -118,6 +159,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding // Assert Assert.NotEqual(default(ModelBindingResult), result); + Assert.False(result.IsModelSet); Assert.Null(result.Model); } @@ -151,7 +193,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } [Fact] - public async Task FormFileModelBinder_ReturnsNothing_WithEmptyContentDisposition() + public async Task FormFileModelBinder_ReturnsFailedResult_WithEmptyContentDisposition() { // Arrange var formFiles = new FormFileCollection(); @@ -165,11 +207,12 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding // Assert Assert.NotEqual(default(ModelBindingResult), result); + Assert.False(result.IsModelSet); Assert.Null(result.Model); } [Fact] - public async Task FormFileModelBinder_ReturnsNothing_WithNoFileNameAndZeroLength() + public async Task FormFileModelBinder_ReturnsFailedResult_WithNoFileNameAndZeroLength() { // Arrange var formFiles = new FormFileCollection(); @@ -183,15 +226,75 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding // Assert Assert.NotEqual(default(ModelBindingResult), result); + Assert.False(result.IsModelSet); Assert.Null(result.Model); } + [Fact] + public async Task FormFileModelBinder_ReturnsFailedResult_ForReadOnlyDestination() + { + // Arrange + var binder = new FormFileModelBinder(); + var formFiles = GetTwoFiles(); + var httpContext = GetMockHttpContext(GetMockFormCollection(formFiles)); + var bindingContext = GetBindingContextForReadOnlyArray(httpContext); + + // Act + var result = await binder.BindModelResultAsync(bindingContext); + + // Assert + Assert.NotEqual(default(ModelBindingResult), result); + Assert.False(result.IsModelSet); + Assert.Null(result.Model); + } + + [Fact] + public async Task FormFileModelBinder_ReturnsFailedResult_ForCollectionsItCannotCreate() + { + // Arrange + var binder = new FormFileModelBinder(); + var formFiles = GetTwoFiles(); + var httpContext = GetMockHttpContext(GetMockFormCollection(formFiles)); + var bindingContext = GetBindingContext(typeof(ISet), httpContext); + + // Act + var result = await binder.BindModelResultAsync(bindingContext); + + // Assert + Assert.NotEqual(default(ModelBindingResult), result); + Assert.False(result.IsModelSet); + Assert.Null(result.Model); + } + + private static DefaultModelBindingContext GetBindingContextForReadOnlyArray(HttpContext httpContext) + { + var metadataProvider = new TestModelMetadataProvider(); + metadataProvider + .ForProperty(nameof(ModelWithReadOnlyArray.ArrayProperty)) + .BindingDetails(bd => bd.BindingSource = BindingSource.Header); + var modelMetadata = metadataProvider.GetMetadataForProperty( + typeof(ModelWithReadOnlyArray), + nameof(ModelWithReadOnlyArray.ArrayProperty)); + + return GetBindingContext(metadataProvider, modelMetadata, httpContext); + } + private static DefaultModelBindingContext GetBindingContext(Type modelType, HttpContext httpContext) { var metadataProvider = new EmptyModelMetadataProvider(); + var metadata = metadataProvider.GetMetadataForType(modelType); + + return GetBindingContext(metadataProvider, metadata, httpContext); + } + + private static DefaultModelBindingContext GetBindingContext( + IModelMetadataProvider metadataProvider, + ModelMetadata metadata, + HttpContext httpContext) + { var bindingContext = new DefaultModelBindingContext { - ModelMetadata = metadataProvider.GetMetadataForType(modelType), + ModelMetadata = metadata, ModelName = "file", ModelState = new ModelStateDictionary(), OperationBindingContext = new OperationBindingContext @@ -218,6 +321,17 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding return httpContext.Object; } + private static FormFileCollection GetTwoFiles() + { + var formFiles = new FormFileCollection + { + GetMockFormFile("file", "file1.txt"), + GetMockFormFile("file", "file2.txt"), + }; + + return formFiles; + } + private static IFormCollection GetMockFormCollection(FormFileCollection formFiles) { var formCollection = new Mock(); @@ -233,5 +347,14 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding return formFile.Object; } + + private class ModelWithReadOnlyArray + { + public IFormFile[] ArrayProperty { get; } + } + + private class FileList : List + { + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/HeaderModelBinderTests.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/HeaderModelBinderTests.cs index 7925c67f12..5383e2be71 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/HeaderModelBinderTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/HeaderModelBinderTests.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.AspNetCore.Http.Internal; using Xunit; @@ -72,6 +73,34 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Test Assert.Equal(headerValue, result.Model); } + [Theory] + [InlineData(typeof(IEnumerable))] + [InlineData(typeof(ICollection))] + [InlineData(typeof(IList))] + [InlineData(typeof(List))] + [InlineData(typeof(LinkedList))] + [InlineData(typeof(StringList))] + public async Task HeaderBinder_BindsHeaders_ForCollectionsItCanCreate(Type destinationType) + { + // Arrange + var header = "Accept"; + var headerValue = "application/json,text/json"; + var binder = new HeaderModelBinder(); + var modelBindingContext = GetBindingContext(destinationType); + + modelBindingContext.FieldName = header; + modelBindingContext.OperationBindingContext.HttpContext.Request.Headers.Add(header, new[] { headerValue }); + + // Act + var result = await binder.BindModelResultAsync(modelBindingContext); + + // Assert + Assert.NotEqual(default(ModelBindingResult), result); + Assert.True(result.IsModelSet); + Assert.IsAssignableFrom(destinationType, result.Model); + Assert.Equal(headerValue.Split(','), result.Model as IEnumerable); + } + [Fact] public async Task HeaderBinder_ReturnsNothing_ForNullBindingSource() { @@ -116,11 +145,76 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Test Assert.Equal(default(ModelBindingResult), result); } + [Fact] + public async Task HeaderBinder_ReturnsFailedResult_ForReadOnlyDestination() + { + // Arrange + var header = "Accept"; + var headerValue = "application/json,text/json"; + var binder = new HeaderModelBinder(); + var modelBindingContext = GetBindingContextForReadOnlyArray(); + + modelBindingContext.FieldName = header; + modelBindingContext.OperationBindingContext.HttpContext.Request.Headers.Add(header, new[] { headerValue }); + + // Act + var result = await binder.BindModelResultAsync(modelBindingContext); + + // Assert + Assert.NotEqual(default(ModelBindingResult), result); + Assert.False(result.IsModelSet); + Assert.Equal("modelName", result.Key); + Assert.Null(result.Model); + } + + [Fact] + public async Task HeaderBinder_ReturnsFailedResult_ForCollectionsItCannotCreate() + { + // Arrange + var header = "Accept"; + var headerValue = "application/json,text/json"; + var binder = new HeaderModelBinder(); + var modelBindingContext = GetBindingContext(typeof(ISet)); + + modelBindingContext.FieldName = header; + modelBindingContext.OperationBindingContext.HttpContext.Request.Headers.Add(header, new[] { headerValue }); + + // Act + var result = await binder.BindModelResultAsync(modelBindingContext); + + // Assert + Assert.NotEqual(default(ModelBindingResult), result); + Assert.False(result.IsModelSet); + Assert.Equal("modelName", result.Key); + Assert.Null(result.Model); + } + private static DefaultModelBindingContext GetBindingContext(Type modelType) { var metadataProvider = new TestModelMetadataProvider(); metadataProvider.ForType(modelType).BindingDetails(d => d.BindingSource = BindingSource.Header); var modelMetadata = metadataProvider.GetMetadataForType(modelType); + + return GetBindingContext(metadataProvider, modelMetadata); + } + + private static DefaultModelBindingContext GetBindingContextForReadOnlyArray() + { + var metadataProvider = new TestModelMetadataProvider(); + metadataProvider + .ForProperty(nameof(ModelWithReadOnlyArray.ArrayProperty)) + .BindingDetails(bd => bd.BindingSource = BindingSource.Header); + var modelMetadata = metadataProvider.GetMetadataForProperty( + typeof(ModelWithReadOnlyArray), + nameof(ModelWithReadOnlyArray.ArrayProperty)); + + return GetBindingContext(metadataProvider, modelMetadata); + } + + private static DefaultModelBindingContext GetBindingContext( + IModelMetadataProvider metadataProvider, + ModelMetadata modelMetadata) + { var bindingContext = new DefaultModelBindingContext { ModelMetadata = modelMetadata, @@ -142,5 +236,14 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Test return bindingContext; } + + private class ModelWithReadOnlyArray + { + public string[] ArrayProperty { get; } + } + + private class StringList : List + { + } } } \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ModelBindingHelperTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ModelBindingHelperTest.cs index 24f1033e1c..84d93522c6 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ModelBindingHelperTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ModelBindingHelperTest.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Collections.ObjectModel; using System.ComponentModel.DataAnnotations; using System.Globalization; using System.Linq.Expressions; @@ -12,10 +13,8 @@ using Microsoft.AspNetCore.Mvc.DataAnnotations; using Microsoft.AspNetCore.Mvc.DataAnnotations.Internal; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.Internal; -using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.AspNetCore.Mvc.ModelBinding.Test; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; -using Microsoft.AspNetCore.Routing; using Moq; using Xunit; @@ -550,10 +549,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { "ExcludedProperty", "ExcludedPropertyValue" } }; - Func includePredicate = - (context, propertyName) => - string.Equals(propertyName, "IncludedProperty", StringComparison.OrdinalIgnoreCase) || - string.Equals(propertyName, "MyProperty", StringComparison.OrdinalIgnoreCase); + Func includePredicate = (context, propertyName) => + string.Equals(propertyName, "IncludedProperty", StringComparison.OrdinalIgnoreCase) || + string.Equals(propertyName, "MyProperty", StringComparison.OrdinalIgnoreCase); var valueProvider = new TestValueProvider(values); var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); @@ -658,8 +656,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding var binder = new StubModelBinder(); var model = new MyModel(); - Func includePredicate = - (context, propertyName) => true; + Func includePredicate = (context, propertyName) => true; // Act & Assert var exception = await Assert.ThrowsAsync( @@ -1018,7 +1015,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } [Fact] - public void ConvertToReturnsNullIfTrimmedValueIsEmptyString() + public void ConvertToReturnsNull_IfConvertingNullToArrayType() { // Arrange @@ -1244,21 +1241,6 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding () => ModelBindingHelper.ConvertTo("this-is-not-a-valid-value", destinationType)); } - [Fact] - public void ConvertToThrowsIfNoConverterExists() - { - // Arrange - var destinationType = typeof(MyClassWithoutConverter); - - // Act & Assert - var ex = Assert.Throws( - () => ModelBindingHelper.ConvertTo("x", destinationType)); - Assert.Equal("The parameter conversion from type 'System.String' to type " + - $"'{typeof(MyClassWithoutConverter).FullName}' " + - "failed because no type converter can convert between these types.", - ex.Message); - } - [Fact] public void ConvertToUsesProvidedCulture() { @@ -1308,43 +1290,80 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } } + // None of the types here have converters from MyClassWithoutConverter. [Theory] [InlineData(typeof(TimeSpan))] [InlineData(typeof(DateTime))] [InlineData(typeof(DateTimeOffset))] [InlineData(typeof(Guid))] [InlineData(typeof(IntEnum))] - public void ConvertTo_Throws_IfValueIsNotStringData(Type destinationType) + public void ConvertTo_Throws_IfValueIsNotConvertible(Type destinationType) { // Arrange + var expectedMessage = $"The parameter conversion from type '{typeof(MyClassWithoutConverter)}' to type " + + $"'{destinationType}' failed because no type converter can convert between these types."; - // Act + // Act & Assert var ex = Assert.Throws( () => ModelBindingHelper.ConvertTo(new MyClassWithoutConverter(), destinationType)); - - // Assert - var expectedMessage = string.Format("The parameter conversion from type '{0}' to type '{1}' " + - "failed because no type converter can convert between these types.", - typeof(MyClassWithoutConverter), destinationType); Assert.Equal(expectedMessage, ex.Message); } + // String does not have a converter to MyClassWithoutConverter. [Fact] public void ConvertTo_Throws_IfDestinationTypeIsNotConvertible() { // Arrange var value = "Hello world"; var destinationType = typeof(MyClassWithoutConverter); + var expectedMessage = $"The parameter conversion from type '{value.GetType()}' to type " + + $"'{typeof(MyClassWithoutConverter)}' failed because no type converter can convert between these types."; - // Act + // Act & Assert var ex = Assert.Throws( () => ModelBindingHelper.ConvertTo(value, destinationType)); + Assert.Equal(expectedMessage, ex.Message); + } + + // Happens very rarely in practice since conversion is almost-always from strings or string arrays. + [Theory] + [InlineData(typeof(MyClassWithoutConverter))] + [InlineData(typeof(MySubClassWithoutConverter))] + public void ConvertTo_ReturnsValue_IfCompatible(Type destinationType) + { + // Arrange + var value = new MySubClassWithoutConverter(); + + // Act + var result = ModelBindingHelper.ConvertTo(value, destinationType); // Assert - var expectedMessage = string.Format("The parameter conversion from type '{0}' to type '{1}' " + - "failed because no type converter can convert between these types.", - value.GetType(), typeof(MyClassWithoutConverter)); - Assert.Equal(expectedMessage, ex.Message); + Assert.Same(value, result); + } + + [Theory] + [InlineData(typeof(MyClassWithoutConverter[]))] + [InlineData(typeof(MySubClassWithoutConverter[]))] + public void ConvertTo_ReusesArrayElements_IfCompatible(Type destinationType) + { + // Arrange + var value = new MyClassWithoutConverter[] + { + new MySubClassWithoutConverter(), + new MySubClassWithoutConverter(), + new MySubClassWithoutConverter(), + }; + + // Act + var result = ModelBindingHelper.ConvertTo(value, destinationType); + + // Assert + Assert.IsType(destinationType, result); + Assert.Collection( + result as IEnumerable, + element => { Assert.Same(value[0], element); }, + element => { Assert.Same(value[1], element); }, + element => { Assert.Same(value[2], element); }); } [Theory] @@ -1367,10 +1386,246 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Assert.Equal(expected, outValue); } + [Theory] + [InlineData(typeof(int))] + [InlineData(typeof(int[]))] + [InlineData(typeof(IEnumerable))] + [InlineData(typeof(IReadOnlyCollection))] + [InlineData(typeof(IReadOnlyList))] + [InlineData(typeof(ICollection))] + [InlineData(typeof(IList))] + [InlineData(typeof(List))] + [InlineData(typeof(Collection))] + [InlineData(typeof(IntList))] + [InlineData(typeof(LinkedList))] + public void CanGetCompatibleCollection_ReturnsTrue(Type destinationType) + { + // Arrange + var bindingContext = GetBindingContext(destinationType); + + // Act + var result = ModelBindingHelper.CanGetCompatibleCollection(bindingContext); + + // Assert + Assert.True(result); + } + + [Theory] + [InlineData(typeof(int))] + [InlineData(typeof(int[]))] + [InlineData(typeof(IEnumerable))] + [InlineData(typeof(IReadOnlyCollection))] + [InlineData(typeof(IReadOnlyList))] + [InlineData(typeof(ICollection))] + [InlineData(typeof(IList))] + [InlineData(typeof(List))] + public void GetCompatibleCollection_ReturnsList(Type destinationType) + { + // Arrange + var bindingContext = GetBindingContext(destinationType); + + // Act + var result = ModelBindingHelper.GetCompatibleCollection(bindingContext); + + // Assert + Assert.IsType>(result); + } + + [Theory] + [InlineData(typeof(Collection))] + [InlineData(typeof(IntList))] + [InlineData(typeof(LinkedList))] + public void GetCompatibleCollection_ActivatesCollection(Type destinationType) + { + // Arrange + var bindingContext = GetBindingContext(destinationType); + + // Act + var result = ModelBindingHelper.GetCompatibleCollection(bindingContext); + + // Assert + Assert.IsType(destinationType, result); + } + + [Fact] + public void GetCompatibleCollection_SetsCapacity() + { + // Arrange + var bindingContext = GetBindingContext(typeof(IList)); + + // Act + var result = ModelBindingHelper.GetCompatibleCollection(bindingContext, capacity: 23); + + // Assert + var list = Assert.IsType>(result); + Assert.Equal(23, list.Capacity); + } + + [Theory] + [InlineData(nameof(ModelWithReadOnlyAndSpecialCaseProperties.ArrayProperty))] + [InlineData(nameof(ModelWithReadOnlyAndSpecialCaseProperties.ArrayPropertyWithValue))] + [InlineData(nameof(ModelWithReadOnlyAndSpecialCaseProperties.EnumerableProperty))] + [InlineData(nameof(ModelWithReadOnlyAndSpecialCaseProperties.EnumerablePropertyWithArrayValue))] + [InlineData(nameof(ModelWithReadOnlyAndSpecialCaseProperties.ListProperty))] + [InlineData(nameof(ModelWithReadOnlyAndSpecialCaseProperties.ScalarProperty))] + [InlineData(nameof(ModelWithReadOnlyAndSpecialCaseProperties.ScalarPropertyWithValue))] + public void CanGetCompatibleCollection_ReturnsFalse_IfReadOnly(string propertyName) + { + // Arrange + var bindingContext = GetBindingContextForProperty(propertyName); + + // Act + var result = ModelBindingHelper.CanGetCompatibleCollection(bindingContext); + + // Assert + Assert.False(result); + } + + [Theory] + [InlineData(nameof(ModelWithReadOnlyAndSpecialCaseProperties.EnumerablePropertyWithArrayValueAndSetter))] + [InlineData(nameof(ModelWithReadOnlyAndSpecialCaseProperties.EnumerablePropertyWithListValue))] + [InlineData(nameof(ModelWithReadOnlyAndSpecialCaseProperties.ListPropertyWithValue))] + public void CanGetCompatibleCollection_ReturnsTrue_IfCollection(string propertyName) + { + // Arrange + var bindingContext = GetBindingContextForProperty(propertyName); + + // Act + var result = ModelBindingHelper.CanGetCompatibleCollection(bindingContext); + + // Assert + Assert.True(result); + } + + [Theory] + [InlineData(nameof(ModelWithReadOnlyAndSpecialCaseProperties.EnumerablePropertyWithListValue))] + [InlineData(nameof(ModelWithReadOnlyAndSpecialCaseProperties.ListPropertyWithValue))] + public void GetCompatibleCollection_ReturnsExistingCollection(string propertyName) + { + // Arrange + var bindingContext = GetBindingContextForProperty(propertyName); + + // Act + var result = ModelBindingHelper.GetCompatibleCollection(bindingContext); + + // Assert + Assert.Same(bindingContext.Model, result); + var list = Assert.IsType>(result); + Assert.Empty(list); + } + + [Fact] + public void CanGetCompatibleCollection_ReturnsNewCollection() + { + // Arrange + var bindingContext = GetBindingContextForProperty( + nameof(ModelWithReadOnlyAndSpecialCaseProperties.EnumerablePropertyWithArrayValueAndSetter)); + + // Act + var result = ModelBindingHelper.GetCompatibleCollection(bindingContext); + + // Assert + Assert.NotSame(bindingContext.Model, result); + var list = Assert.IsType>(result); + Assert.Empty(list); + } + + [Theory] + [InlineData(typeof(Collection))] + [InlineData(typeof(List))] + [InlineData(typeof(MyModel))] + [InlineData(typeof(AbstractIntList))] + [InlineData(typeof(ISet))] + public void CanGetCompatibleCollection_ReturnsFalse(Type destinationType) + { + // Arrange + var bindingContext = GetBindingContext(destinationType); + + // Act + var result = ModelBindingHelper.CanGetCompatibleCollection(bindingContext); + + // Assert + Assert.False(result); + } + + private static DefaultModelBindingContext GetBindingContextForProperty(string propertyName) + { + var metadataProvider = TestModelMetadataProvider.CreateDefaultProvider(); + var modelMetadata = metadataProvider.GetMetadataForProperty( + typeof(ModelWithReadOnlyAndSpecialCaseProperties), + propertyName); + var bindingContext = GetBindingContext(metadataProvider, modelMetadata); + + var container = new ModelWithReadOnlyAndSpecialCaseProperties(); + bindingContext.Model = modelMetadata.PropertyGetter(container); + + return bindingContext; + } + + private static DefaultModelBindingContext GetBindingContext(Type modelType) + { + var metadataProvider = new EmptyModelMetadataProvider(); + var metadata = metadataProvider.GetMetadataForType(modelType); + + return GetBindingContext(metadataProvider, metadata); + } + + private static DefaultModelBindingContext GetBindingContext( + IModelMetadataProvider metadataProvider, + ModelMetadata metadata) + { + var bindingContext = new DefaultModelBindingContext + { + ModelMetadata = metadata, + OperationBindingContext = new OperationBindingContext + { + MetadataProvider = metadataProvider, + }, + }; + + return bindingContext; + } + + private class ModelWithReadOnlyAndSpecialCaseProperties + { + public int[] ArrayProperty { get; } + + public int[] ArrayPropertyWithValue { get; } = new int[4]; + + public IEnumerable EnumerableProperty { get; } + + public IEnumerable EnumerablePropertyWithArrayValue { get; } = new int[4]; + + // Special case: Value cannot be used but property can be set. + public IEnumerable EnumerablePropertyWithArrayValueAndSetter { get; set; } = new int[4]; + + public IEnumerable EnumerablePropertyWithListValue { get; } = new List { 23 }; + + public List ListProperty { get; } + + public List ListPropertyWithValue { get; } = new List { 23 }; + + public int ScalarProperty { get; } + + public int ScalarPropertyWithValue { get; } = 23; + } + private class MyClassWithoutConverter { } + private class MySubClassWithoutConverter : MyClassWithoutConverter + { + } + + private abstract class AbstractIntList : List + { + } + + private class IntList : List + { + } + private enum IntEnum { Value0 = 0, diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/FormFileModelBindingIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/FormFileModelBindingIntegrationTest.cs index ac72c3dab9..578c912213 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/FormFileModelBindingIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/FormFileModelBindingIntegrationTest.cs @@ -77,6 +77,150 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.Equal(ModelValidationState.Skipped, modelState[key].ValidationState); } + private class ListContainer1 + { + [ModelBinder(Name = "files")] + public List ListProperty { get; set; } + } + + [Fact] + public async Task BindCollectionProperty_WithData_IsBound() + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor + { + Name = "Parameter1", + BindingInfo = new BindingInfo(), + ParameterType = typeof(ListContainer1), + }; + + var data = "some data"; + var operationContext = ModelBindingTestHelper.GetOperationBindingContext( + request => UpdateRequest(request, data, "files")); + var modelState = operationContext.ActionContext.ModelState; + + // Act + var result = await argumentBinder.BindModelAsync(parameter, operationContext) ?? + default(ModelBindingResult); + + // Assert + Assert.True(result.IsModelSet); + + // Model + var boundContainer = Assert.IsType(result.Model); + Assert.NotNull(boundContainer); + Assert.NotNull(boundContainer.ListProperty); + var file = Assert.Single(boundContainer.ListProperty); + Assert.Equal("form-data; name=files; filename=text.txt", file.ContentDisposition); + using (var reader = new StreamReader(file.OpenReadStream())) + { + Assert.Equal(data, reader.ReadToEnd()); + } + + // ModelState + Assert.True(modelState.IsValid); + var kvp = Assert.Single(modelState); + Assert.Equal("files", kvp.Key); + var modelStateEntry = kvp.Value; + Assert.NotNull(modelStateEntry); + Assert.Empty(modelStateEntry.Errors); + Assert.Equal(ModelValidationState.Skipped, modelStateEntry.ValidationState); + Assert.Null(modelStateEntry.AttemptedValue); + Assert.Null(modelStateEntry.RawValue); + } + + [Fact] + public async Task BindCollectionProperty_NoData_IsNotBound() + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor + { + Name = "Parameter1", + BindingInfo = new BindingInfo(), + ParameterType = typeof(ListContainer1), + }; + + var operationContext = ModelBindingTestHelper.GetOperationBindingContext( + request => UpdateRequest(request, data: null, name: null)); + var modelState = operationContext.ActionContext.ModelState; + + // Act + var result = await argumentBinder.BindModelAsync(parameter, operationContext) ?? + default(ModelBindingResult); + + // Assert + Assert.True(result.IsModelSet); + + // Model (bound to an empty collection) + var boundContainer = Assert.IsType(result.Model); + Assert.NotNull(boundContainer); + Assert.Null(boundContainer.ListProperty); + + // ModelState + Assert.True(modelState.IsValid); + Assert.Empty(modelState); + } + + private class ListContainer2 + { + [ModelBinder(Name = "files")] + public List ListProperty { get; } = new List + { + new FormFile(new MemoryStream(), baseStreamOffset: 0, length: 0, name: "file", fileName: "file1"), + new FormFile(new MemoryStream(), baseStreamOffset: 0, length: 0, name: "file", fileName: "file2"), + new FormFile(new MemoryStream(), baseStreamOffset: 0, length: 0, name: "file", fileName: "file3"), + }; + } + + [Fact] + public async Task BindReadOnlyCollectionProperty_WithData_IsBound() + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor + { + Name = "Parameter1", + BindingInfo = new BindingInfo(), + ParameterType = typeof(ListContainer2), + }; + + var data = "some data"; + var operationContext = ModelBindingTestHelper.GetOperationBindingContext( + request => UpdateRequest(request, data, "files")); + var modelState = operationContext.ActionContext.ModelState; + + // Act + var result = await argumentBinder.BindModelAsync(parameter, operationContext) ?? + default(ModelBindingResult); + + // Assert + Assert.True(result.IsModelSet); + + // Model + var boundContainer = Assert.IsType(result.Model); + Assert.NotNull(boundContainer); + Assert.NotNull(boundContainer.ListProperty); + var file = Assert.Single(boundContainer.ListProperty); + Assert.Equal("form-data; name=files; filename=text.txt", file.ContentDisposition); + using (var reader = new StreamReader(file.OpenReadStream())) + { + Assert.Equal(data, reader.ReadToEnd()); + } + + // ModelState + Assert.True(modelState.IsValid); + var kvp = Assert.Single(modelState); + Assert.Equal("files", kvp.Key); + var modelStateEntry = kvp.Value; + Assert.NotNull(modelStateEntry); + Assert.Empty(modelStateEntry.Errors); + Assert.Equal(ModelValidationState.Skipped, modelStateEntry.ValidationState); + Assert.Null(modelStateEntry.AttemptedValue); + Assert.Null(modelStateEntry.RawValue); + } + [Fact] public async Task BindParameter_WithData_GetsBound() { @@ -149,7 +293,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var modelState = operationContext.ActionContext.ModelState; // Act - var modelBindingResult = await argumentBinder.BindModelAsync(parameter, operationContext) ?? default(ModelBindingResult); + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, operationContext) ?? + default(ModelBindingResult); // Assert Assert.Equal(default(ModelBindingResult), modelBindingResult); diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs index 07774a8dd3..7d9b9b2eb4 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/HeaderModelBinderIntegrationTest.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.Linq; using System.Threading.Tasks; @@ -81,11 +82,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests ParameterType = typeof(Person) }; - // Do not add any headers. - var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => { - request.Headers.Add("Header", new[] { "someValue" }); - }); - + var operationContext = ModelBindingTestHelper.GetOperationBindingContext( + request => request.Headers.Add("Header", new[] { "someValue" })); var modelState = operationContext.ActionContext.ModelState; // Act @@ -154,6 +152,102 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.Equal(new string[] { "someValue" }, entry.Value.RawValue); } + private class ListContainer1 + { + [FromHeader(Name = "Header")] + public List ListProperty { get; set; } + } + + [Fact] + public async Task BindCollectionPropertyFromHeader_WithData_IsBound() + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor + { + Name = "Parameter1", + BindingInfo = new BindingInfo(), + ParameterType = typeof(ListContainer1), + }; + + var operationContext = ModelBindingTestHelper.GetOperationBindingContext( + request => request.Headers.Add("Header", new[] { "someValue" })); + var modelState = operationContext.ActionContext.ModelState; + + // Act + var result = await argumentBinder.BindModelAsync(parameter, operationContext) ?? + default(ModelBindingResult); + + // Assert + Assert.True(result.IsModelSet); + + // Model + var boundContainer = Assert.IsType(result.Model); + Assert.NotNull(boundContainer); + Assert.NotNull(boundContainer.ListProperty); + var entry = Assert.Single(boundContainer.ListProperty); + Assert.Equal("someValue", entry); + + // ModelState + Assert.True(modelState.IsValid); + var kvp = Assert.Single(modelState); + Assert.Equal("Header", kvp.Key); + var modelStateEntry = kvp.Value; + Assert.NotNull(modelStateEntry); + Assert.Empty(modelStateEntry.Errors); + Assert.Equal(ModelValidationState.Valid, modelStateEntry.ValidationState); + Assert.Equal("someValue", modelStateEntry.AttemptedValue); + Assert.Equal(new[] { "someValue" }, modelStateEntry.RawValue); + } + + private class ListContainer2 + { + [FromHeader(Name = "Header")] + public List ListProperty { get; } = new List { "One", "Two", "Three" }; + } + + [Fact] + public async Task BindReadOnlyCollectionPropertyFromHeader_WithData_IsBound() + { + // Arrange + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(); + var parameter = new ParameterDescriptor + { + Name = "Parameter1", + BindingInfo = new BindingInfo(), + ParameterType = typeof(ListContainer2), + }; + + var operationContext = ModelBindingTestHelper.GetOperationBindingContext( + request => request.Headers.Add("Header", new[] { "someValue" })); + var modelState = operationContext.ActionContext.ModelState; + + // Act + var result = await argumentBinder.BindModelAsync(parameter, operationContext) ?? + default(ModelBindingResult); + + // Assert + Assert.True(result.IsModelSet); + + // Model + var boundContainer = Assert.IsType(result.Model); + Assert.NotNull(boundContainer); + Assert.NotNull(boundContainer.ListProperty); + var entry = Assert.Single(boundContainer.ListProperty); + Assert.Equal("someValue", entry); + + // ModelState + Assert.True(modelState.IsValid); + var kvp = Assert.Single(modelState); + Assert.Equal("Header", kvp.Key); + var modelStateEntry = kvp.Value; + Assert.NotNull(modelStateEntry); + Assert.Empty(modelStateEntry.Errors); + Assert.Equal(ModelValidationState.Valid, modelStateEntry.ValidationState); + Assert.Equal("someValue", modelStateEntry.AttemptedValue); + Assert.Equal(new[] { "someValue" }, modelStateEntry.RawValue); + } + [Theory] [InlineData(typeof(string[]), "value1, value2, value3")] [InlineData(typeof(string), "value")] diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs index f4b5d1f063..5959dd77c1 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/MutableObjectModelBinderIntegrationTest.cs @@ -517,17 +517,14 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var model = Assert.IsType(modelBindingResult.Model); Assert.NotNull(model.Customer); Assert.Equal("bill", model.Customer.Name); - Assert.Empty(model.Customer.Documents); + Assert.Null(model.Customer.Documents); - Assert.Equal(2, modelState.Count); Assert.Equal(0, modelState.ErrorCount); Assert.True(modelState.IsValid); - var entry = Assert.Single(modelState, e => e.Key == "parameter.Customer.Documents").Value; - Assert.Null(entry.AttemptedValue); // FormFile entries don't include the model. - Assert.Null(entry.RawValue); - - entry = Assert.Single(modelState, e => e.Key == "parameter.Customer.Name").Value; + var kvp = Assert.Single(modelState); + Assert.Equal("parameter.Customer.Name", kvp.Key); + var entry = kvp.Value; Assert.Equal("bill", entry.AttemptedValue); Assert.Equal("bill", entry.RawValue); } diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs index 2370425778..05cee32164 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/TryUpdateModelIntegrationTest.cs @@ -3,9 +3,14 @@ using System; using System.Collections.Generic; +using System.IO; +using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features.Internal; +using Microsoft.AspNetCore.Http.Internal; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Extensions.Primitives; using Xunit; namespace Microsoft.AspNetCore.Mvc.IntegrationTests @@ -959,6 +964,65 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.Empty(modelState); } + [Fact] + public async Task TryUpdateModelAsync_TopLevelFormFileCollection_IsBound() + { + // Arrange + var data = "some data"; + var operationContext = ModelBindingTestHelper.GetOperationBindingContext( + request => UpdateRequest(request, data, "files")); + var modelState = operationContext.ActionContext.ModelState; + var model = new List + { + new FormFile(new MemoryStream(), baseStreamOffset: 0, length: 0, name: "file", fileName: "file1"), + new FormFile(new MemoryStream(), baseStreamOffset: 0, length: 0, name: "file", fileName: "file2"), + new FormFile(new MemoryStream(), baseStreamOffset: 0, length: 0, name: "file", fileName: "file3"), + }; + + // Act + var result = await TryUpdateModel(model, prefix: "files", operationContext: operationContext); + + // Assert + Assert.True(result); + + // Model + var file = Assert.Single(model); + Assert.Equal("form-data; name=files; filename=text.txt", file.ContentDisposition); + using (var reader = new StreamReader(file.OpenReadStream())) + { + Assert.Equal(data, reader.ReadToEnd()); + } + + // ModelState + Assert.True(modelState.IsValid); + var kvp = Assert.Single(modelState); + Assert.Equal("files", kvp.Key); + var modelStateEntry = kvp.Value; + Assert.NotNull(modelStateEntry); + Assert.Empty(modelStateEntry.Errors); + Assert.Equal(ModelValidationState.Skipped, modelStateEntry.ValidationState); + Assert.Null(modelStateEntry.AttemptedValue); + Assert.Null(modelStateEntry.RawValue); + } + + private void UpdateRequest(HttpRequest request, string data, string name) + { + const string fileName = "text.txt"; + var fileCollection = new FormFileCollection(); + var formCollection = new FormCollection(new Dictionary(), fileCollection); + + request.Form = formCollection; + request.ContentType = "multipart/form-data; boundary=----WebKitFormBoundarymx2fSWqWSd0OxQqq"; + + request.Headers["Content-Disposition"] = $"form-data; name={name}; filename={fileName}"; + + var memoryStream = new MemoryStream(Encoding.UTF8.GetBytes(data)); + fileCollection.Add(new FormFile(memoryStream, 0, data.Length, name, fileName) + { + Headers = request.Headers + }); + } + private class CustomReadOnlyCollection : ICollection { private ICollection _original; diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageActivatorTest.cs b/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageActivatorTest.cs index f676cd75f1..6129984e76 100644 --- a/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageActivatorTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorPageActivatorTest.cs @@ -14,6 +14,7 @@ using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.AspNetCore.Mvc.Razor.Internal; using Microsoft.AspNetCore.Mvc.Rendering; +using Microsoft.AspNetCore.Mvc.Routing; using Microsoft.AspNetCore.Mvc.ViewEngines; using Microsoft.AspNetCore.Mvc.ViewFeatures; using Microsoft.AspNetCore.Mvc.ViewFeatures.Internal; @@ -217,6 +218,70 @@ namespace Microsoft.AspNetCore.Mvc.Razor Assert.IsType>(viewContext.ViewData); } + [Fact] + public void Activate_Throws_WhenViewDataPropertyHasIncorrectType() + { + // Arrange + var activator = new RazorPageActivator(new EmptyModelMetadataProvider()); + var instance = new HasIncorrectViewDataPropertyType(); + + var collection = new ServiceCollection(); + collection + .AddSingleton(new HtmlTestEncoder()) + .AddSingleton(new DiagnosticListener("Microsoft.AspNetCore.Mvc")); + var httpContext = new DefaultHttpContext + { + RequestServices = collection.BuildServiceProvider(), + }; + + var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + var viewContext = new ViewContext( + actionContext, + Mock.Of(), + new ViewDataDictionary(new EmptyModelMetadataProvider()), + Mock.Of(), + TextWriter.Null, + new HtmlHelperOptions()); + + // Act & Assert + Assert.Throws(() => activator.Activate(instance, viewContext)); + } + + [Fact] + public void Activate_CanGetUrlHelperFromDependencyInjection() + { + // Arrange + var activator = new RazorPageActivator(new EmptyModelMetadataProvider()); + var instance = new HasUnusualIUrlHelperProperty(); + + // IUrlHelperFactory should not be used. But set it up to match a real configuration. + var collection = new ServiceCollection(); + collection + .AddSingleton() + .AddSingleton(new HtmlTestEncoder()) + .AddSingleton(new DiagnosticListener("Microsoft.AspNetCore.Mvc")) + .AddSingleton(); + var httpContext = new DefaultHttpContext + { + RequestServices = collection.BuildServiceProvider(), + }; + + var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + var viewContext = new ViewContext( + actionContext, + Mock.Of(), + new ViewDataDictionary(new EmptyModelMetadataProvider()), + Mock.Of(), + TextWriter.Null, + new HtmlHelperOptions()); + + // Act + activator.Activate(instance, viewContext); + + // Assert + Assert.NotNull(instance.UrlHelper); + } + private abstract class TestPageBase : RazorPage { [RazorInject] @@ -258,6 +323,68 @@ namespace Microsoft.AspNetCore.Mvc.Razor } } + private class HasIncorrectViewDataPropertyType : RazorPage + { + [RazorInject] + public ViewDataDictionary MoreViewData { get; set; } + + public override Task ExecuteAsync() + { + throw new NotImplementedException(); + } + } + + private class HasUnusualIUrlHelperProperty : RazorPage + { + [RazorInject] + public IUrlHelperWrapper UrlHelper { get; set; } + + public override Task ExecuteAsync() + { + throw new NotImplementedException(); + } + } + + private class UrlHelperWrapper : IUrlHelperWrapper + { + public ActionContext ActionContext + { + get + { + throw new NotImplementedException(); + } + } + + public string Action(UrlActionContext actionContext) + { + throw new NotImplementedException(); + } + + public string Content(string contentPath) + { + throw new NotImplementedException(); + } + + public bool IsLocalUrl(string url) + { + throw new NotImplementedException(); + } + + public string Link(string routeName, object values) + { + throw new NotImplementedException(); + } + + public string RouteUrl(UrlRouteContext routeContext) + { + throw new NotImplementedException(); + } + } + + private interface IUrlHelperWrapper : IUrlHelper + { + } + private class MyService : ICanHasViewContext { public ViewContext ViewContext { get; private set; } diff --git a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewComponents/DefaultViewComponentDescriptorProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewComponents/DefaultViewComponentDescriptorProviderTest.cs index 35c15d6592..273de6c2ce 100644 --- a/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewComponents/DefaultViewComponentDescriptorProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/ViewComponents/DefaultViewComponentDescriptorProviderTest.cs @@ -93,11 +93,12 @@ namespace Microsoft.AspNetCore.Mvc.ViewComponents Assert.Equal(expected, ex.Message); } - [Fact] - public void GetViewComponents_ThrowsIfInvokeReturnsATask() + [Theory] + [InlineData(typeof(TaskReturningInvokeViewComponent))] + [InlineData(typeof(GenericTaskReturningInvokeViewComponent))] + public void GetViewComponents_ThrowsIfInvokeReturnsATask(Type type) { // Arrange - var type = typeof(TaskReturningInvokeViewComponent); var expected = $"Method 'Invoke' of view component '{type}' cannot return a Task."; var provider = CreateProvider(type); @@ -189,6 +190,11 @@ namespace Microsoft.AspNetCore.Mvc.ViewComponents public Task Invoke() => Task.FromResult(0); } + public class GenericTaskReturningInvokeViewComponent + { + public Task Invoke() => Task.FromResult(0); + } + public class VoidReturningInvokeViewComponent { public void Invoke(int x)