Make converters injectable and respect culture

Author: JimBobSquarePants

src/ImageSharp.Web/Commands/CommandDescriptor.cs

@@ -4,7 +4,9 @@
 using System;
 using System.Collections.Generic;
 using System.Globalization;
+using System.Linq;
 using System.Net;
+using System.Runtime.CompilerServices;
 using SixLabors.ImageSharp.Web.Commands.Converters;
 
 namespace SixLabors.ImageSharp.Web.Commands
@@ -14,44 +16,18 @@ namespace SixLabors.ImageSharp.Web.Commands
     /// </summary>
     public sealed class CommandParser
     {
-        /// <summary>
-        /// A new instance of the <see cref="CommandParser"/> class with lazy initialization.
-        /// </summary>
-        private static readonly Lazy<CommandParser> Lazy = new Lazy<CommandParser>(() => new CommandParser());
+        private readonly ICommandConverter[] converters;
 
         /// <summary>
-        /// Prevents a default instance of the <see cref="CommandParser"/> class from being created.
+        /// Initializes a new instance of the <see cref="CommandParser"/> class.
         /// </summary>
-        private CommandParser()
+        /// <param name="converters">The collection of command converters.</param>
+        public CommandParser(IEnumerable<ICommandConverter> converters)
         {
-            this.AddSimpleConverters();
-            this.AddListConverters();
-            this.AddArrayConverters();
-            this.AddColorConverters();
+            Guard.NotNull(converters, nameof(converters));
+            this.converters = converters.ToArray();
         }
 
-        /// <summary>
-        /// Gets the current <see cref="CommandParser"/> instance.
-        /// </summary>
-        public static CommandParser Instance => Lazy.Value;
-
-        /// <summary>
-        /// Adds a command converter to the parser.
-        /// </summary>
-        /// <param name="type">The <see cref="Type"/> to add a converter for. </param>
-        /// <param name="converterType">The type of <see cref="CommandConverter"/> to add.</param>
-        public void AddConverter(Type type, Type converterType) => CommandDescriptor.AddConverter(type, converterType);
-
-        /// <summary>
-        /// Parses the given string value converting it to the given using the invariant culture.
-        /// </summary>
-        /// <param name="value">The string value to parse.</param>
-        /// <typeparam name="T">
-        /// The <see cref="Type"/> to convert the string to.
-        /// </typeparam>
-        /// <returns>The converted instance or the default.</returns>
-        public T ParseValue<T>(string value) => this.ParseValue<T>(value, CultureInfo.InvariantCulture);
-
         /// <summary>
         /// Parses the given string value converting it to the given type.
         /// </summary>
@@ -61,99 +37,38 @@ private CommandParser()
         /// The <see cref="Type"/> to convert the string to.
         /// </typeparam>
         /// <returns>The converted instance or the default.</returns>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public T ParseValue<T>(string value, CultureInfo culture)
-        {
-            culture ??= CultureInfo.InvariantCulture;
-
-            Type type = typeof(T);
-            ICommandConverter converter = CommandDescriptor.GetConverter(type);
-            try
-            {
-                return (T)converter.ConvertFrom(culture, WebUtility.UrlDecode(value), type);
-            }
-            catch
-            {
-                return default;
-            }
-        }
-
-        /// <summary>
-        /// Add the generic converters.
-        /// </summary>
-        private void AddSimpleConverters()
-        {
-            this.AddConverter(TypeConstants.Sbyte, typeof(IntegralNumberConverter<sbyte>));
-            this.AddConverter(TypeConstants.Byte, typeof(IntegralNumberConverter<byte>));
-
-            this.AddConverter(TypeConstants.Short, typeof(IntegralNumberConverter<short>));
-            this.AddConverter(TypeConstants.UShort, typeof(IntegralNumberConverter<ushort>));
-
-            this.AddConverter(TypeConstants.Int, typeof(IntegralNumberConverter<int>));
-            this.AddConverter(TypeConstants.UInt, typeof(IntegralNumberConverter<uint>));
-
-            this.AddConverter(TypeConstants.Long, typeof(IntegralNumberConverter<long>));
-            this.AddConverter(TypeConstants.ULong, typeof(IntegralNumberConverter<ulong>));
-
-            this.AddConverter(typeof(decimal), typeof(SimpleCommandConverter<decimal>));
-            this.AddConverter(typeof(float), typeof(SimpleCommandConverter<float>));
-
-            this.AddConverter(typeof(double), typeof(SimpleCommandConverter<double>));
-            this.AddConverter(typeof(string), typeof(SimpleCommandConverter<string>));
-
-            this.AddConverter(typeof(bool), typeof(SimpleCommandConverter<bool>));
-        }
-
-        /// <summary>
-        /// Adds a selection of default list type converters.
-        /// </summary>
-        private void AddListConverters()
-        {
-            this.AddConverter(typeof(List<sbyte>), typeof(ListConverter<sbyte>));
-            this.AddConverter(typeof(List<byte>), typeof(ListConverter<byte>));
-
-            this.AddConverter(typeof(List<short>), typeof(ListConverter<short>));
-            this.AddConverter(typeof(List<ushort>), typeof(ListConverter<ushort>));
-
-            this.AddConverter(typeof(List<int>), typeof(ListConverter<int>));
-            this.AddConverter(typeof(List<uint>), typeof(ListConverter<uint>));
-
-            this.AddConverter(typeof(List<long>), typeof(ListConverter<long>));
-            this.AddConverter(typeof(List<ulong>), typeof(ListConverter<ulong>));
-
-            this.AddConverter(typeof(List<decimal>), typeof(ListConverter<decimal>));
-            this.AddConverter(typeof(List<float>), typeof(ListConverter<float>));
-            this.AddConverter(typeof(List<double>), typeof(ListConverter<double>));
-
-            this.AddConverter(typeof(List<string>), typeof(ListConverter<string>));
-        }
+            => (T)this.ParseValue(typeof(T), value, culture);
 
         /// <summary>
-        /// Adds a selection of default array type converters.
+        /// Parses the given string value converting it to the given type.
         /// </summary>
-        private void AddArrayConverters()
+        /// <param name="type"> The type to convert the string to.</param>
+        /// <param name="value">The string value to parse.</param>
+        /// <param name="culture">The <see cref="CultureInfo"/> to use as the current culture.</param>
+        /// <returns>The converted instance or the default value.</returns>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public object ParseValue(Type type, string value, CultureInfo culture)
         {
-            this.AddConverter(typeof(sbyte[]), typeof(ArrayConverter<sbyte>));
-            this.AddConverter(typeof(byte[]), typeof(ArrayConverter<byte>));
-
-            this.AddConverter(typeof(short[]), typeof(ArrayConverter<short>));
-            this.AddConverter(typeof(ushort[]), typeof(ArrayConverter<ushort>));
+            DebugGuard.NotNull(type, nameof(type));
+            DebugGuard.NotNull(culture, nameof(culture));
 
-            this.AddConverter(typeof(int[]), typeof(ArrayConverter<int>));
-            this.AddConverter(typeof(uint[]), typeof(ArrayConverter<uint>));
+            // This allows us to reuse the same converter for infinite enum types.
+            Type matchType = type.IsEnum ? typeof(Enum) : type;
 
-            this.AddConverter(typeof(long[]), typeof(ArrayConverter<long>));
-            this.AddConverter(typeof(ulong[]), typeof(ArrayConverter<ulong>));
+            ICommandConverter converter = Array.Find(this.converters, x => x.Type.Equals(matchType));
 
-            this.AddConverter(typeof(decimal[]), typeof(ArrayConverter<decimal>));
-            this.AddConverter(typeof(float[]), typeof(ArrayConverter<float>));
-            this.AddConverter(typeof(double[]), typeof(ArrayConverter<double>));
+            if (converter is null)
+            {
+                ThrowNotSupported(type);
+            }
 
-            this.AddConverter(typeof(string[]), typeof(ArrayConverter<string>));
+            return converter.ConvertFrom(this, culture, WebUtility.UrlDecode(value), type);
         }
 
-        /// <summary>
-        /// Adds the default color converters.
-        /// </summary>
-        private void AddColorConverters() => this.AddConverter(TypeConstants.Color, typeof(ColorConverter));
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        private static void ThrowNotSupported(Type type)
+            => throw new NotSupportedException($"Cannot convert to {type.FullName}.");
     }
 }

src/ImageSharp.Web/Commands/CommandParser.cs

@@ -14,7 +14,14 @@ namespace SixLabors.ImageSharp.Web.Commands.Converters
     internal sealed class ArrayConverter<T> : ListConverter<T>
     {
         /// <inheritdoc/>
-        public override object ConvertFrom(CultureInfo culture, string value, Type propertyType)
-            => ((List<T>)base.ConvertFrom(culture, value, propertyType)).ToArray();
+        public override Type Type => typeof(T[]);
+
+        /// <inheritdoc/>
+        public override object ConvertFrom(
+            CommandParser parser,
+            CultureInfo culture,
+            string value,
+            Type propertyType)
+            => ((List<T>)base.ConvertFrom(parser, culture, value, propertyType)).ToArray();
     }
 }

src/ImageSharp.Web/Commands/Converters/ArrayConverter{T}.cs

@@ -12,7 +12,7 @@ namespace SixLabors.ImageSharp.Web.Commands.Converters
     /// <summary>
     /// Allows the conversion of strings into rgba32 pixel colors.
     /// </summary>
-    internal class ColorConverter : CommandConverter
+    internal class ColorConverter : ICommandConverter
     {
         /// <summary>
         /// The web color hexadecimal regex. Matches strings arranged
@@ -31,7 +31,10 @@ internal class ColorConverter : CommandConverter
         private static readonly Lazy<IDictionary<string, Color>> ColorConstantsTable = new Lazy<IDictionary<string, Color>>(InitializeColorConstantsTable);
 
         /// <inheritdoc/>
-        public override object ConvertFrom(CultureInfo culture, string value, Type propertyType)
+        public Type Type => typeof(Color);
+
+        /// <inheritdoc/>
+        public object ConvertFrom(CommandParser parser, CultureInfo culture, string value, Type propertyType)
         {
             if (string.IsNullOrWhiteSpace(value))
             {
@@ -56,11 +59,14 @@ public override object ConvertFrom(CultureInfo culture, string value, Type prope
 
                 if (convert)
                 {
-                    List<byte> rgba = CommandParser.Instance.ParseValue<List<byte>>(value);
+                    List<byte> rgba = parser.ParseValue<List<byte>>(value, culture);
 
-                    return rgba.Count == 4
-                        ? Color.FromRgba(rgba[0], rgba[1], rgba[2], rgba[3])
-                        : Color.FromRgb(rgba[0], rgba[1], rgba[2]);
+                    return rgba.Count switch
+                    {
+                        4 => Color.FromRgba(rgba[0], rgba[1], rgba[2], rgba[3]),
+                        3 => Color.FromRgb(rgba[0], rgba[1], rgba[2]),
+                        _ => default,
+                    };
                 }
             }
 
@@ -72,7 +78,7 @@ public override object ConvertFrom(CultureInfo culture, string value, Type prope
 
             // Named colors
             IDictionary<string, Color> table = ColorConstantsTable.Value;
-            return table.ContainsKey(value) ? table[value] : base.ConvertFrom(culture, value, propertyType);
+            return table.ContainsKey(value) ? table[value] : default;
         }
 
         /// <summary>
@@ -82,9 +88,10 @@ public override object ConvertFrom(CultureInfo culture, string value, Type prope
         private static IDictionary<string, Color> InitializeColorConstantsTable()
         {
             IDictionary<string, Color> table = new Dictionary<string, Color>(StringComparer.OrdinalIgnoreCase);
-            foreach (FieldInfo field in TypeConstants.Color.GetFields(BindingFlags.Public | BindingFlags.Static))
+
+            foreach (FieldInfo field in typeof(Color).GetFields(BindingFlags.Public | BindingFlags.Static))
             {
-                if (field.FieldType == TypeConstants.Color)
+                if (field.FieldType == typeof(Color))
                 {
                     table[field.Name] = (Color)field.GetValue(null);
                 }