Merge pull request #117 from SixLabors/js/injectable-culture-converters

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,46 @@ 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;
+            DebugGuard.NotNull(culture, nameof(culture));
 
             Type type = typeof(T);
-            ICommandConverter converter = CommandDescriptor.GetConverter(type);
-            try
+            ICommandConverter converter = Array.Find(this.converters, x => x.Type.Equals(type));
+
+            if (converter != null)
             {
-                return (T)converter.ConvertFrom(culture, WebUtility.UrlDecode(value), type);
+                return ((ICommandConverter<T>)converter).ConvertFrom(
+                    this,
+                    culture,
+                    WebUtility.UrlDecode(value),
+                    type);
             }
-            catch
+
+            // This special case allows us to reuse the same converter for infinite enum types
+            // if one has not already been configured.
+            if (type.IsEnum)
             {
-                return default;
+                converter = Array.Find(this.converters, x => x.Type.Equals(typeof(Enum)));
+                if (converter != null)
+                {
+                    return (T)((ICommandConverter<object>)converter).ConvertFrom(
+                        this,
+                        culture,
+                        WebUtility.UrlDecode(value),
+                        type);
+                }
             }
-        }
-
-        /// <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>));
+            // We don't actually return here.
+            // The compiler just cannot see our exception.
+            ThrowNotSupported(type);
+            return default;
         }
 
-        /// <summary>
-        /// Adds a selection of default array type converters.
-        /// </summary>
-        private void AddArrayConverters()
-        {
-            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>));
-
-            this.AddConverter(typeof(int[]), typeof(ArrayConverter<int>));
-            this.AddConverter(typeof(uint[]), typeof(ArrayConverter<uint>));
-
-            this.AddConverter(typeof(long[]), typeof(ArrayConverter<long>));
-            this.AddConverter(typeof(ulong[]), typeof(ArrayConverter<ulong>));
-
-            this.AddConverter(typeof(decimal[]), typeof(ArrayConverter<decimal>));
-            this.AddConverter(typeof(float[]), typeof(ArrayConverter<float>));
-            this.AddConverter(typeof(double[]), typeof(ArrayConverter<double>));
-
-            this.AddConverter(typeof(string[]), typeof(ArrayConverter<string>));
-        }
-
-        /// <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

@@ -4,17 +4,51 @@
 using System;
 using System.Collections.Generic;
 using System.Globalization;
+using System.Linq;
+using System.Runtime.CompilerServices;
 
 namespace SixLabors.ImageSharp.Web.Commands.Converters
 {
     /// <summary>
-    /// Converts the value of an string to a generic array.
+    /// Converts the value of a string to a generic array.
     /// </summary>
     /// <typeparam name="T">The parameter type to convert to.</typeparam>
-    internal sealed class ArrayConverter<T> : ListConverter<T>
+    internal sealed class ArrayConverter<T> : ICommandConverter<T[]>
     {
         /// <inheritdoc/>
-        public override object ConvertFrom(CultureInfo culture, string value, Type propertyType)
-            => ((List<T>)base.ConvertFrom(culture, value, propertyType)).ToArray();
+        public Type Type => typeof(T[]);
+
+        /// <inheritdoc/>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public T[] ConvertFrom(
+            CommandParser parser,
+            CultureInfo culture,
+            string value,
+            Type propertyType)
+        {
+            if (string.IsNullOrWhiteSpace(value))
+            {
+                return Array.Empty<T>();
+            }
+
+            var result = new List<T>();
+            foreach (string pill in GetStringArray(value, culture))
+            {
+                T item = parser.ParseValue<T>(pill, culture);
+                if (item != null)
+                {
+                    result.Add(item);
+                }
+            }
+
+            return result.ToArray();
+        }
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private static string[] GetStringArray(string input, CultureInfo culture)
+        {
+            char separator = culture.TextInfo.ListSeparator[0];
+            return input.Split(separator).Select(s => s.Trim()).ToArray();
+        }
     }
 }

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

@@ -5,14 +5,15 @@
 using System.Collections.Generic;
 using System.Globalization;
 using System.Reflection;
+using System.Runtime.CompilerServices;
 using System.Text.RegularExpressions;
 
 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<Color>
     {
         /// <summary>
         /// The web color hexadecimal regex. Matches strings arranged
@@ -31,17 +32,21 @@ 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/>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public Color ConvertFrom(CommandParser parser, CultureInfo culture, string value, Type propertyType)
         {
             if (string.IsNullOrWhiteSpace(value))
             {
-                return default(Color);
+                return default;
             }
 
             // Numeric r,g,b - r,g,b,a
             char separator = culture.TextInfo.ListSeparator[0];
 
-            if (value.Contains(separator.ToString()))
+            if (value.IndexOf(separator) != -1)
             {
                 string[] components = value.Split(separator);
 
@@ -56,11 +61,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,19 +80,16 @@ 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>
-        /// Initializes color table mapping color constants.
-        /// </summary>
-        /// <returns>The <see cref="IDictionary{String, Color}"/>.</returns>
         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);
                 }