🛡️ Add concurrency checks to UnitValueAbbreviationLookup (#1160)

Refs #919 

Use `ConcurrentDictionary` for lookups.
Use `IReadOnlyList` and treat list instances as immutable.
Reduce `ToList()` allocations.
Use `Lazy` to compute all abbreviations.

Author: angularsen

UnitsNet.Tests/UnitAbbreviationsCacheTests.cs

@@ -2,9 +2,8 @@
 // Copyright 2013 Andreas Gullberg Larsen ([email protected]). Maintained at https://github.com/angularsen/UnitsNet.
 
 using System;
-using System.Collections.Generic;
 using System.Globalization;
-using System.Linq;
+using UnitsNet.Tests.CustomQuantities;
 using UnitsNet.Units;
 using Xunit;
 using Xunit.Abstractions;
@@ -274,6 +273,33 @@ public void MapUnitToDefaultAbbreviation_GivenCustomAbbreviation_SetsAbbreviatio
             Assert.Equal("1 m^2", Area.FromSquareMeters(1).ToString(newZealandCulture));
         }
 
+        [Fact]
+        public void MapUnitToAbbreviation_DoesNotInsertDuplicates()
+        {
+            var cache = new UnitAbbreviationsCache();
+
+            cache.MapUnitToAbbreviation(HowMuchUnit.Some, "soome");
+            cache.MapUnitToAbbreviation(HowMuchUnit.Some, "soome");
+            cache.MapUnitToAbbreviation(HowMuchUnit.Some, "soome");
+
+            Assert.Equal("soome", cache.GetDefaultAbbreviation(HowMuchUnit.Some));
+            Assert.Equal(new[] { "soome" }, cache.GetUnitAbbreviations(HowMuchUnit.Some));
+            Assert.Equal(new[] { "soome" }, cache.GetAllUnitAbbreviationsForQuantity(typeof(HowMuchUnit)));
+        }
+
+        [Fact]
+        public void MapUnitToDefaultAbbreviation_Twice_SetsNewDefaultAndKeepsOrderOfExistingAbbreviations()
+        {
+            var cache = new UnitAbbreviationsCache();
+
+            cache.MapUnitToAbbreviation(HowMuchUnit.Some, "soome");
+            cache.MapUnitToDefaultAbbreviation(HowMuchUnit.Some, "1st default");
+            cache.MapUnitToDefaultAbbreviation(HowMuchUnit.Some, "2nd default");
+
+            Assert.Equal("2nd default", cache.GetDefaultAbbreviation(HowMuchUnit.Some));
+            Assert.Equal(new[] { "2nd default", "1st default", "soome" }, cache.GetUnitAbbreviations(HowMuchUnit.Some));
+        }
+
         /// <summary>
         ///     Convenience method to the proper culture parameter type.
         /// </summary>

UnitsNet/CustomCode/UnitAbbreviationsCache.cs

@@ -54,7 +54,7 @@ static UnitAbbreviationsCache()
 
         private void LoadGeneratedAbbreviations()
         {
-            foreach(var quantity in Quantity.GetQuantityTypes())
+            foreach (Type quantity in Quantity.GetQuantityTypes())
             {
                 var mapGeneratedLocalizationsMethod = quantity.GetMethod(nameof(Length.MapGeneratedLocalizations), BindingFlags.NonPublic | BindingFlags.Static);
                 mapGeneratedLocalizationsMethod?.Invoke(null, new object[]{this});
@@ -272,7 +272,7 @@ public string[] GetUnitAbbreviations(Type unitType, int unitValue, IFormatProvid
         /// <param name="unitEnumType">Enum type for unit.</param>
         /// <param name="formatProvider">The format provider to use for lookup. Defaults to <see cref="CultureInfo.CurrentCulture" /> if null.</param>
         /// <returns>Unit abbreviations associated with unit.</returns>
-        public string[] GetAllUnitAbbreviationsForQuantity(Type unitEnumType, IFormatProvider? formatProvider = null)
+        public IReadOnlyList<string> GetAllUnitAbbreviationsForQuantity(Type unitEnumType, IFormatProvider? formatProvider = null)
         {
             formatProvider ??= CultureInfo.CurrentCulture;
 
@@ -292,7 +292,7 @@ internal bool TryGetUnitValueAbbreviationLookup(Type unitType, IFormatProvider?
 
             formatProvider ??= CultureInfo.CurrentCulture;
 
-            if (!_lookupsForCulture.TryGetValue(formatProvider, out var quantitiesForProvider))
+            if (!_lookupsForCulture.TryGetValue(formatProvider, out UnitTypeToLookup? quantitiesForProvider))
             {
                 return !Equals(formatProvider, FallbackCulture) &&
                        TryGetUnitValueAbbreviationLookup(unitType, FallbackCulture, out unitToAbbreviations);

UnitsNet/CustomCode/UnitValueAbbreviationLookup.cs

@@ -2,78 +2,93 @@
 // Copyright 2013 Andreas Gullberg Larsen ([email protected]). Maintained at https://github.com/angularsen/UnitsNet.
 
 using System;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Linq;
 
-using UnitToAbbreviationMap = System.Collections.Generic.Dictionary<int, System.Collections.Generic.List<string>>;
-using AbbreviationToUnitMap = System.Collections.Generic.Dictionary<string, System.Collections.Generic.List<int>>;
-
 namespace UnitsNet
 {
+    internal class UnitToAbbreviationMap : ConcurrentDictionary<int, IReadOnlyList<string>> { }
+    internal class AbbreviationToUnitMap : ConcurrentDictionary<string, IReadOnlyList<int>> { }
+
     internal class UnitValueAbbreviationLookup
     {
         private readonly UnitToAbbreviationMap _unitToAbbreviationMap = new();
         private readonly AbbreviationToUnitMap _abbreviationToUnitMap = new();
         private readonly AbbreviationToUnitMap _lowerCaseAbbreviationToUnitMap = new();
+        private Lazy<string[]> _allUnitAbbreviationsLazy;
+        private readonly object _syncRoot = new();
 
-        internal string[] GetAllUnitAbbreviationsForQuantity()
+        public UnitValueAbbreviationLookup()
         {
-            return _unitToAbbreviationMap.Values.SelectMany(abbreviations => abbreviations).Distinct().ToArray();
+            _allUnitAbbreviationsLazy = new Lazy<string[]>(ComputeAllUnitAbbreviationsValue);
+        }
+
+        internal IReadOnlyList<string> GetAllUnitAbbreviationsForQuantity()
+        {
+            return _allUnitAbbreviationsLazy.Value;
         }
 
-        internal List<string> GetAbbreviationsForUnit<TUnitType>(TUnitType unit) where TUnitType : Enum
+        internal IReadOnlyList<string> GetAbbreviationsForUnit<TUnitType>(TUnitType unit) where TUnitType : Enum
         {
             return GetAbbreviationsForUnit(Convert.ToInt32(unit));
         }
 
-        internal List<string> GetAbbreviationsForUnit(int unit)
+        internal IReadOnlyList<string> GetAbbreviationsForUnit(int unit)
         {
             if (!_unitToAbbreviationMap.TryGetValue(unit, out var abbreviations))
                 return new List<string>(0);
 
-            return abbreviations.Distinct().ToList();
+            return abbreviations.ToList();
         }
 
-        internal List<int> GetUnitsForAbbreviation(string abbreviation, bool ignoreCase)
+        internal IReadOnlyList<int> GetUnitsForAbbreviation(string abbreviation, bool ignoreCase)
         {
             var lowerCaseAbbreviation = abbreviation.ToLower();
             var key = ignoreCase ? lowerCaseAbbreviation : abbreviation;
             var map = ignoreCase ? _lowerCaseAbbreviationToUnitMap : _abbreviationToUnitMap;
 
-            if (!map.TryGetValue(key, out List<int> units))
+            if (!map.TryGetValue(key, out IReadOnlyList<int> units))
                 return new List<int>(0);
 
-            return units.Distinct().ToList();
+            return units.ToList();
         }
 
         internal void Add(int unit, string abbreviation, bool setAsDefault = false, bool allowAbbreviationLookup = true)
         {
-            var lowerCaseAbbreviation = abbreviation.ToLower();
-
-            if (!_unitToAbbreviationMap.TryGetValue(unit, out var abbreviationsForUnit))
-                abbreviationsForUnit = _unitToAbbreviationMap[unit] = new List<string>();
-
-            if (allowAbbreviationLookup)
+            // Restrict concurrency on writes.
+            // By using ConcurrencyDictionary and immutable IReadOnlyList instances, we don't need to lock on reads.
+            lock (_syncRoot)
             {
-                if (!_abbreviationToUnitMap.TryGetValue(abbreviation, out var unitsForAbbreviation))
-                    _abbreviationToUnitMap[abbreviation] = unitsForAbbreviation = new List<int>();
-
-                if (!_lowerCaseAbbreviationToUnitMap.TryGetValue(lowerCaseAbbreviation, out var unitsForLowerCaseAbbreviation))
-                    _lowerCaseAbbreviationToUnitMap[lowerCaseAbbreviation] = unitsForLowerCaseAbbreviation = new List<int>();
-
-                unitsForLowerCaseAbbreviation.Remove(unit);
-                unitsForLowerCaseAbbreviation.Add(unit);
-
-                unitsForAbbreviation.Remove(unit);
-                unitsForAbbreviation.Add(unit);
+                var lowerCaseAbbreviation = abbreviation.ToLower();
+
+                if (allowAbbreviationLookup)
+                {
+                    _abbreviationToUnitMap.AddOrUpdate(abbreviation,
+                        addValueFactory: _ => new List<int> { unit },
+                        updateValueFactory: (_, existing) => existing.Append(unit).Distinct().ToList());
+
+                    _lowerCaseAbbreviationToUnitMap.AddOrUpdate(lowerCaseAbbreviation,
+                        addValueFactory: _ => new List<int> { unit },
+                        updateValueFactory: (_, existing) => existing.Append(unit).Distinct().ToList());
+                }
+
+                _unitToAbbreviationMap.AddOrUpdate(unit,
+                    addValueFactory: _ => new List<string> { abbreviation },
+                    updateValueFactory: (_, existing) =>
+                    {
+                        return setAsDefault
+                            ? existing.Where(x => x != abbreviation).Prepend(abbreviation).ToList()
+                            : existing.Where(x => x != abbreviation).Append(abbreviation).ToList();
+                    });
+
+                _allUnitAbbreviationsLazy = new Lazy<string[]>(ComputeAllUnitAbbreviationsValue);
             }
+        }
 
-            abbreviationsForUnit.Remove(abbreviation);
-
-            if (setAsDefault)
-                abbreviationsForUnit.Insert(0, abbreviation);
-            else
-                abbreviationsForUnit.Add(abbreviation);
+        private string[] ComputeAllUnitAbbreviationsValue()
+        {
+            return _unitToAbbreviationMap.Values.SelectMany(abbreviations => abbreviations).Distinct().ToArray();
         }
     }
 }