diff options
3 files changed, 89 insertions, 36 deletions
diff --git a/Jellyfin.Api/ModelBinders/CommaDelimitedArrayModelBinder.cs b/Jellyfin.Api/ModelBinders/CommaDelimitedArrayModelBinder.cs index 4f012cab2..e90f48d2f 100644 --- a/Jellyfin.Api/ModelBinders/CommaDelimitedArrayModelBinder.cs +++ b/Jellyfin.Api/ModelBinders/CommaDelimitedArrayModelBinder.cs @@ -1,7 +1,9 @@ using System; +using System.Collections.Generic; using System.ComponentModel; using System.Threading.Tasks; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Extensions.Logging; namespace Jellyfin.Api.ModelBinders { @@ -11,6 +13,17 @@ namespace Jellyfin.Api.ModelBinders /// </summary> public class CommaDelimitedArrayModelBinder : IModelBinder { + private readonly ILogger<CommaDelimitedArrayModelBinder> _logger; + + /// <summary> + /// Initializes a new instance of the <see cref="CommaDelimitedArrayModelBinder"/> class. + /// </summary> + /// <param name="logger">Instance of the <see cref="ILogger{CommaDelimitedArrayModelBinder}"/> interface.</param> + public CommaDelimitedArrayModelBinder(ILogger<CommaDelimitedArrayModelBinder> logger) + { + _logger = logger; + } + /// <inheritdoc/> public Task BindModelAsync(ModelBindingContext bindingContext) { @@ -20,16 +33,8 @@ namespace Jellyfin.Api.ModelBinders if (valueProviderResult.Length > 1) { - var result = Array.CreateInstance(elementType, valueProviderResult.Length); - - for (int i = 0; i < valueProviderResult.Length; i++) - { - var value = converter.ConvertFromString(valueProviderResult.Values[i].Trim()); - - result.SetValue(value, i); - } - - bindingContext.Result = ModelBindingResult.Success(result); + var typedValues = GetParsedResult(valueProviderResult.Values, elementType, converter); + bindingContext.Result = ModelBindingResult.Success(typedValues); } else { @@ -37,13 +42,8 @@ namespace Jellyfin.Api.ModelBinders if (value != null) { - var values = Array.ConvertAll( - value.Split(new[] { "," }, StringSplitOptions.RemoveEmptyEntries), - x => converter.ConvertFromString(x?.Trim())); - - var typedValues = Array.CreateInstance(elementType, values.Length); - values.CopyTo(typedValues, 0); - + var splitValues = value.Split(',', StringSplitOptions.RemoveEmptyEntries); + var typedValues = GetParsedResult(splitValues, elementType, converter); bindingContext.Result = ModelBindingResult.Success(typedValues); } else @@ -55,5 +55,36 @@ namespace Jellyfin.Api.ModelBinders return Task.CompletedTask; } + + private Array GetParsedResult(IReadOnlyList<string> values, Type elementType, TypeConverter converter) + { + var parsedValues = new object?[values.Count]; + var convertedCount = 0; + for (var i = 0; i < values.Count; i++) + { + try + { + parsedValues[i] = converter.ConvertFromString(values[i].Trim()); + convertedCount++; + } + catch (FormatException e) + { + _logger.LogWarning(e, "Error converting value."); + } + } + + var typedValues = Array.CreateInstance(elementType, convertedCount); + var typedValueIndex = 0; + for (var i = 0; i < parsedValues.Length; i++) + { + if (parsedValues[i] != null) + { + typedValues.SetValue(parsedValues[i], typedValueIndex); + typedValueIndex++; + } + } + + return typedValues; + } } } diff --git a/MediaBrowser.Common/Json/Converters/JsonCommaDelimitedArrayConverter.cs b/MediaBrowser.Common/Json/Converters/JsonCommaDelimitedArrayConverter.cs index bf7048c37..b24a49761 100644 --- a/MediaBrowser.Common/Json/Converters/JsonCommaDelimitedArrayConverter.cs +++ b/MediaBrowser.Common/Json/Converters/JsonCommaDelimitedArrayConverter.cs @@ -32,13 +32,34 @@ namespace MediaBrowser.Common.Json.Converters return Array.Empty<T>(); } - var entries = new T[stringEntries.Length]; + var parsedValues = new object[stringEntries.Length]; + var convertedCount = 0; for (var i = 0; i < stringEntries.Length; i++) { - entries[i] = (T)_typeConverter.ConvertFrom(stringEntries[i].Trim()); + try + { + parsedValues[i] = _typeConverter.ConvertFrom(stringEntries[i].Trim()); + convertedCount++; + } + catch (FormatException) + { + // TODO log when upgraded to .Net5 + // _logger.LogWarning(e, "Error converting value."); + } } - return entries; + var typedValues = new T[convertedCount]; + var typedValueIndex = 0; + for (var i = 0; i < stringEntries.Length; i++) + { + if (parsedValues[i] != null) + { + typedValues.SetValue(parsedValues[i], typedValueIndex); + typedValueIndex++; + } + } + + return typedValues; } return JsonSerializer.Deserialize<T[]>(ref reader, options); diff --git a/tests/Jellyfin.Api.Tests/ModelBinders/CommaDelimitedArrayModelBinderTests.cs b/tests/Jellyfin.Api.Tests/ModelBinders/CommaDelimitedArrayModelBinderTests.cs index 89c7d62f7..82c7f6427 100644 --- a/tests/Jellyfin.Api.Tests/ModelBinders/CommaDelimitedArrayModelBinderTests.cs +++ b/tests/Jellyfin.Api.Tests/ModelBinders/CommaDelimitedArrayModelBinderTests.cs @@ -5,6 +5,7 @@ using System.Threading.Tasks; using Jellyfin.Api.ModelBinders; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Primitives; using Moq; using Xunit; @@ -21,7 +22,7 @@ namespace Jellyfin.Api.Tests.ModelBinders var queryParamString = "lol,xd"; var queryParamType = typeof(string[]); - var modelBinder = new CommaDelimitedArrayModelBinder(); + var modelBinder = new CommaDelimitedArrayModelBinder(new NullLogger<CommaDelimitedArrayModelBinder>()); var valueProvider = new QueryStringValueProvider( new BindingSource(string.Empty, string.Empty, false, false), new QueryCollection(new Dictionary<string, StringValues> { { queryParamName, new StringValues(queryParamString) } }), @@ -46,7 +47,7 @@ namespace Jellyfin.Api.Tests.ModelBinders var queryParamString = "42,0"; var queryParamType = typeof(int[]); - var modelBinder = new CommaDelimitedArrayModelBinder(); + var modelBinder = new CommaDelimitedArrayModelBinder(new NullLogger<CommaDelimitedArrayModelBinder>()); var valueProvider = new QueryStringValueProvider( new BindingSource(string.Empty, string.Empty, false, false), new QueryCollection(new Dictionary<string, StringValues> { { queryParamName, new StringValues(queryParamString) } }), @@ -71,7 +72,7 @@ namespace Jellyfin.Api.Tests.ModelBinders var queryParamString = "How,Much"; var queryParamType = typeof(TestType[]); - var modelBinder = new CommaDelimitedArrayModelBinder(); + var modelBinder = new CommaDelimitedArrayModelBinder(new NullLogger<CommaDelimitedArrayModelBinder>()); var valueProvider = new QueryStringValueProvider( new BindingSource(string.Empty, string.Empty, false, false), new QueryCollection(new Dictionary<string, StringValues> { { queryParamName, new StringValues(queryParamString) } }), @@ -96,7 +97,7 @@ namespace Jellyfin.Api.Tests.ModelBinders var queryParamString = "How,,Much"; var queryParamType = typeof(TestType[]); - var modelBinder = new CommaDelimitedArrayModelBinder(); + var modelBinder = new CommaDelimitedArrayModelBinder(new NullLogger<CommaDelimitedArrayModelBinder>()); var valueProvider = new QueryStringValueProvider( new BindingSource(string.Empty, string.Empty, false, false), new QueryCollection(new Dictionary<string, StringValues> { { queryParamName, new StringValues(queryParamString) } }), @@ -122,7 +123,7 @@ namespace Jellyfin.Api.Tests.ModelBinders var queryParamString2 = "Much"; var queryParamType = typeof(TestType[]); - var modelBinder = new CommaDelimitedArrayModelBinder(); + var modelBinder = new CommaDelimitedArrayModelBinder(new NullLogger<CommaDelimitedArrayModelBinder>()); var valueProvider = new QueryStringValueProvider( new BindingSource(string.Empty, string.Empty, false, false), @@ -150,7 +151,7 @@ namespace Jellyfin.Api.Tests.ModelBinders var queryParamValues = Array.Empty<TestType>(); var queryParamType = typeof(TestType[]); - var modelBinder = new CommaDelimitedArrayModelBinder(); + var modelBinder = new CommaDelimitedArrayModelBinder(new NullLogger<CommaDelimitedArrayModelBinder>()); var valueProvider = new QueryStringValueProvider( new BindingSource(string.Empty, string.Empty, false, false), @@ -172,13 +173,13 @@ namespace Jellyfin.Api.Tests.ModelBinders } [Fact] - public async Task BindModelAsync_ThrowsIfCommaDelimitedEnumArrayQueryIsInvalid() + public async Task BindModelAsync_EnumArrayQuery_BindValidOnly() { var queryParamName = "test"; var queryParamString = "🔥,😢"; var queryParamType = typeof(TestType[]); - var modelBinder = new CommaDelimitedArrayModelBinder(); + var modelBinder = new CommaDelimitedArrayModelBinder(new NullLogger<CommaDelimitedArrayModelBinder>()); var valueProvider = new QueryStringValueProvider( new BindingSource(string.Empty, string.Empty, false, false), new QueryCollection(new Dictionary<string, StringValues> { { queryParamName, new StringValues(queryParamString) } }), @@ -189,20 +190,20 @@ namespace Jellyfin.Api.Tests.ModelBinders bindingContextMock.Setup(b => b.ModelType).Returns(queryParamType); bindingContextMock.SetupProperty(b => b.Result); - Func<Task> act = async () => await modelBinder.BindModelAsync(bindingContextMock.Object); - - await Assert.ThrowsAsync<FormatException>(act); + await modelBinder.BindModelAsync(bindingContextMock.Object); + Assert.True(bindingContextMock.Object.Result.IsModelSet); + Assert.Empty((TestType[])bindingContextMock.Object.Result.Model); } [Fact] - public async Task BindModelAsync_ThrowsIfCommaDelimitedEnumArrayQueryIsInvalid2() + public async Task BindModelAsync_EnumArrayQuery_BindValidOnly_2() { var queryParamName = "test"; var queryParamString1 = "How"; var queryParamString2 = "😱"; var queryParamType = typeof(TestType[]); - var modelBinder = new CommaDelimitedArrayModelBinder(); + var modelBinder = new CommaDelimitedArrayModelBinder(new NullLogger<CommaDelimitedArrayModelBinder>()); var valueProvider = new QueryStringValueProvider( new BindingSource(string.Empty, string.Empty, false, false), @@ -217,9 +218,9 @@ namespace Jellyfin.Api.Tests.ModelBinders bindingContextMock.Setup(b => b.ModelType).Returns(queryParamType); bindingContextMock.SetupProperty(b => b.Result); - Func<Task> act = async () => await modelBinder.BindModelAsync(bindingContextMock.Object); - - await Assert.ThrowsAsync<FormatException>(act); + await modelBinder.BindModelAsync(bindingContextMock.Object); + Assert.True(bindingContextMock.Object.Result.IsModelSet); + Assert.Single((TestType[])bindingContextMock.Object.Result.Model); } } } |