Make the JsonConverters for delimited arrays more generic (#13396)

* Make the JsonConverters for delimited arrays more generic

Also adds some tests for serialization (with different types) as we didn't have any before.

* Ignore warnings

5 files changed, 33 insertions, 41 deletions

diff --git a/src/Jellyfin.Extensions/Json/Converters/JsonCommaDelimitedArrayConverter.cs b/src/Jellyfin.Extensions/Json/Converters/JsonCommaDelimitedCollectionConverter.cs
index ccbc296fd..b1946143d 100644
--- a/src/Jellyfin.Extensions/Json/Converters/JsonCommaDelimitedArrayConverter.cs
+++ b/src/Jellyfin.Extensions/Json/Converters/JsonCommaDelimitedCollectionConverter.cs
@@ -1,15 +1,15 @@
 namespace Jellyfin.Extensions.Json.Converters
 {
     /// <summary>
-    /// Convert comma delimited string to array of type.
+    /// Convert comma delimited string to collection of type.
     /// </summary>
     /// <typeparam name="T">Type to convert to.</typeparam>
-    public sealed class JsonCommaDelimitedArrayConverter<T> : JsonDelimitedArrayConverter<T>
+    public sealed class JsonCommaDelimitedCollectionConverter<T> : JsonDelimitedCollectionConverter<T>
     {
         /// <summary>
-        /// Initializes a new instance of the <see cref="JsonCommaDelimitedArrayConverter{T}"/> class.
+        /// Initializes a new instance of the <see cref="JsonCommaDelimitedCollectionConverter{T}"/> class.
         /// </summary>
-        public JsonCommaDelimitedArrayConverter() : base()
+        public JsonCommaDelimitedCollectionConverter() : base()
         {
         }
 
diff --git a/src/Jellyfin.Extensions/Json/Converters/JsonCommaDelimitedArrayConverterFactory.cs b/src/Jellyfin.Extensions/Json/Converters/JsonCommaDelimitedCollectionConverterFactory.cs
index a95e493db..daa79b2b5 100644
--- a/src/Jellyfin.Extensions/Json/Converters/JsonCommaDelimitedArrayConverterFactory.cs
+++ b/src/Jellyfin.Extensions/Json/Converters/JsonCommaDelimitedCollectionConverterFactory.cs
@@ -1,28 +1,31 @@
 using System;
+using System.Collections.Generic;
 using System.Text.Json;
 using System.Text.Json.Serialization;
 
 namespace Jellyfin.Extensions.Json.Converters
 {
     /// <summary>
-    /// Json comma delimited array converter factory.
+    /// Json comma delimited collection converter factory.
     /// </summary>
     /// <remarks>
     /// This must be applied as an attribute, adding to the JsonConverter list causes stack overflow.
     /// </remarks>
-    public class JsonCommaDelimitedArrayConverterFactory : JsonConverterFactory
+    public class JsonCommaDelimitedCollectionConverterFactory : JsonConverterFactory
     {
         /// <inheritdoc />
         public override bool CanConvert(Type typeToConvert)
         {
-            return true;
+            return typeToConvert.IsArray
+                || (typeToConvert.IsGenericType
+                    && (typeToConvert.GetGenericTypeDefinition().IsAssignableFrom(typeof(IReadOnlyCollection<>)) || typeToConvert.GetGenericTypeDefinition().IsAssignableFrom(typeof(IReadOnlyList<>))));
         }
 
         /// <inheritdoc />
         public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options)
         {
             var structType = typeToConvert.GetElementType() ?? typeToConvert.GenericTypeArguments[0];
-            return (JsonConverter?)Activator.CreateInstance(typeof(JsonCommaDelimitedArrayConverter<>).MakeGenericType(structType));
+            return (JsonConverter?)Activator.CreateInstance(typeof(JsonCommaDelimitedCollectionConverter<>).MakeGenericType(structType));
         }
     }
 }
diff --git a/src/Jellyfin.Extensions/Json/Converters/JsonDelimitedArrayConverter.cs b/src/Jellyfin.Extensions/Json/Converters/JsonDelimitedCollectionConverter.cs
index 7472f9c66..fe85d7f73 100644
--- a/src/Jellyfin.Extensions/Json/Converters/JsonDelimitedArrayConverter.cs
+++ b/src/Jellyfin.Extensions/Json/Converters/JsonDelimitedCollectionConverter.cs
@@ -10,14 +10,14 @@ namespace Jellyfin.Extensions.Json.Converters
     /// Convert delimited string to array of type.
     /// </summary>
     /// <typeparam name="T">Type to convert to.</typeparam>
-    public abstract class JsonDelimitedArrayConverter<T> : JsonConverter<T[]>
+    public abstract class JsonDelimitedCollectionConverter<T> : JsonConverter<IReadOnlyCollection<T>>
     {
         private readonly TypeConverter _typeConverter;
 
         /// <summary>
-        /// Initializes a new instance of the <see cref="JsonDelimitedArrayConverter{T}"/> class.
+        /// Initializes a new instance of the <see cref="JsonDelimitedCollectionConverter{T}"/> class.
         /// </summary>
-        protected JsonDelimitedArrayConverter()
+        protected JsonDelimitedCollectionConverter()
         {
             _typeConverter = TypeDescriptor.GetConverter(typeof(T));
         }
@@ -28,7 +28,7 @@ namespace Jellyfin.Extensions.Json.Converters
         protected virtual char Delimiter { get; }
 
         /// <inheritdoc />
-        public override T[]? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+        public override IReadOnlyCollection<T>? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
         {
             if (reader.TokenType == JsonTokenType.String)
             {
@@ -56,35 +56,21 @@ namespace Jellyfin.Extensions.Json.Converters
                     }
                 }
 
-                return typedValues.ToArray();
+                if (typeToConvert.IsArray)
+                {
+                    return typedValues.ToArray();
+                }
+
+                return typedValues;
             }
 
             return JsonSerializer.Deserialize<T[]>(ref reader, options);
         }
 
         /// <inheritdoc />
-        public override void Write(Utf8JsonWriter writer, T[]? value, JsonSerializerOptions options)
+        public override void Write(Utf8JsonWriter writer, IReadOnlyCollection<T>? value, JsonSerializerOptions options)
         {
-            if (value is not null)
-            {
-                writer.WriteStartArray();
-                if (value.Length > 0)
-                {
-                    foreach (var it in value)
-                    {
-                        if (it is not null)
-                        {
-                            writer.WriteStringValue(it.ToString());
-                        }
-                    }
-                }
-
-                writer.WriteEndArray();
-            }
-            else
-            {
-                writer.WriteNullValue();
-            }
+            JsonSerializer.Serialize(writer, value, options);
         }
     }
 }
diff --git a/src/Jellyfin.Extensions/Json/Converters/JsonPipeDelimitedArrayConverter.cs b/src/Jellyfin.Extensions/Json/Converters/JsonPipeDelimitedCollectionConverter.cs
index 55720ee4f..57378a360 100644
--- a/src/Jellyfin.Extensions/Json/Converters/JsonPipeDelimitedArrayConverter.cs
+++ b/src/Jellyfin.Extensions/Json/Converters/JsonPipeDelimitedCollectionConverter.cs
@@ -4,12 +4,12 @@ namespace Jellyfin.Extensions.Json.Converters
     /// Convert Pipe delimited string to array of type.
     /// </summary>
     /// <typeparam name="T">Type to convert to.</typeparam>
-    public sealed class JsonPipeDelimitedArrayConverter<T> : JsonDelimitedArrayConverter<T>
+    public sealed class JsonPipeDelimitedCollectionConverter<T> : JsonDelimitedCollectionConverter<T>
     {
         /// <summary>
-        /// Initializes a new instance of the <see cref="JsonPipeDelimitedArrayConverter{T}"/> class.
+        /// Initializes a new instance of the <see cref="JsonPipeDelimitedCollectionConverter{T}"/> class.
         /// </summary>
-        public JsonPipeDelimitedArrayConverter() : base()
+        public JsonPipeDelimitedCollectionConverter() : base()
         {
         }
 
diff --git a/src/Jellyfin.Extensions/Json/Converters/JsonPipeDelimitedArrayConverterFactory.cs b/src/Jellyfin.Extensions/Json/Converters/JsonPipeDelimitedCollectionConverterFactory.cs
index ae9e1f67a..f487fcaca 100644
--- a/src/Jellyfin.Extensions/Json/Converters/JsonPipeDelimitedArrayConverterFactory.cs
+++ b/src/Jellyfin.Extensions/Json/Converters/JsonPipeDelimitedCollectionConverterFactory.cs
@@ -1,28 +1,31 @@
 using System;
+using System.Collections.Generic;
 using System.Text.Json;
 using System.Text.Json.Serialization;
 
 namespace Jellyfin.Extensions.Json.Converters
 {
     /// <summary>
-    /// Json Pipe delimited array converter factory.
+    /// Json Pipe delimited collection converter factory.
     /// </summary>
     /// <remarks>
     /// This must be applied as an attribute, adding to the JsonConverter list causes stack overflow.
     /// </remarks>
-    public class JsonPipeDelimitedArrayConverterFactory : JsonConverterFactory
+    public class JsonPipeDelimitedCollectionConverterFactory : JsonConverterFactory
     {
         /// <inheritdoc />
         public override bool CanConvert(Type typeToConvert)
         {
-            return true;
+            return typeToConvert.IsArray
+                || (typeToConvert.IsGenericType
+                    && (typeToConvert.GetGenericTypeDefinition().IsAssignableFrom(typeof(IReadOnlyCollection<>)) || typeToConvert.GetGenericTypeDefinition().IsAssignableFrom(typeof(IReadOnlyList<>))));
         }
 
         /// <inheritdoc />
         public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options)
         {
             var structType = typeToConvert.GetElementType() ?? typeToConvert.GenericTypeArguments[0];
-            return (JsonConverter?)Activator.CreateInstance(typeof(JsonPipeDelimitedArrayConverter<>).MakeGenericType(structType));
+            return (JsonConverter?)Activator.CreateInstance(typeof(JsonPipeDelimitedCollectionConverter<>).MakeGenericType(structType));
         }
     }
 }