improve threading synchronization for date lookup cache

Data/Helpers/DataValueCache.cs

@@ -68,9 +68,14 @@ namespace Data.Helpers
 
             foreach (var id in ids)
             {
-                if (_cacheStates[key].ContainsKey(id))
+                var cacheInstance = _cacheStates[key];
+
+                lock (_cacheStates)
                 {
-                    continue;
+                    if (cacheInstance.ContainsKey(id))
+                    {
+                        continue;
+                    }
                 }
 
                 var state = new CacheState<TReturnType>
@@ -80,10 +85,12 @@ namespace Data.Helpers
                     ExpirationTime = expirationTime ?? TimeSpan.FromMinutes(DefaultExpireMinutes)
                 };
 
-                _cacheStates[key].Add(id, state);
+                lock (_cacheStates)
+                {
+                    cacheInstance.Add(id, state);
+                }
 
                 _autoRefresh = autoRefresh;
-                
 
                 if (!_autoRefresh || expirationTime == TimeSpan.MaxValue)
                 {
@@ -96,8 +103,8 @@ namespace Data.Helpers
             }
         }
 
-        public async Task<TReturnType> GetCacheItem(string keyName, CancellationToken cancellationToken = default) =>
+        public Task<TReturnType> GetCacheItem(string keyName, CancellationToken cancellationToken = default) =>
-            await GetCacheItem(keyName, null, cancellationToken);
+            GetCacheItem(keyName, null, cancellationToken);
 
         public async Task<TReturnType> GetCacheItem(string keyName, object id = null,
             CancellationToken cancellationToken = default)
@@ -107,7 +114,14 @@ namespace Data.Helpers
                 throw new ArgumentException("No cache found for key {key}", keyName);
             }
 
-            var state = id is null ? _cacheStates[keyName].Values.First() : _cacheStates[keyName][id];
+            var cacheInstance = _cacheStates[keyName];
+
+            CacheState<TReturnType> state;
+            
+            lock (_cacheStates)
+            {
+                state = id is null ? cacheInstance.Values.First() : _cacheStates[keyName][id];
+            }
 
             // when auto refresh is off we want to check the expiration and value
             // when auto refresh is on, we want to only check the value, because it'll be refreshed automatically

Data/Helpers/LookupCache.cs

@@ -8,107 +8,94 @@ using System.Threading;
 using System.Threading.Tasks;
 using ILogger = Microsoft.Extensions.Logging.ILogger;
 
-namespace Data.Helpers
+namespace Data.Helpers;
+
+public class LookupCache<T> : ILookupCache<T> where T : class, IUniqueId
 {
-    public class LookupCache<T> : ILookupCache<T> where T : class, IUniqueId
+    private readonly ILogger _logger;
+    private readonly IDatabaseContextFactory _contextFactory;
+    private Dictionary<long, T> _cachedItems;
+    private readonly SemaphoreSlim _onOperation = new(1, 1);
+
+    public LookupCache(ILogger<LookupCache<T>> logger, IDatabaseContextFactory contextFactory)
     {
-        private readonly ILogger _logger;
+        _logger = logger;
-        private readonly IDatabaseContextFactory _contextFactory;
+        _contextFactory = contextFactory;
-        private Dictionary<long, T> _cachedItems;
+    }
-        private readonly SemaphoreSlim _onOperation = new SemaphoreSlim(1, 1);
 
-        public LookupCache(ILogger<LookupCache<T>> logger, IDatabaseContextFactory contextFactory)
+    public async Task<T> AddAsync(T item)
+    {
+        await _onOperation.WaitAsync();
+        T existingItem = null;
+
+        if (_cachedItems.ContainsKey(item.Id))
         {
-            _logger = logger;
+            existingItem = _cachedItems[item.Id];
-            _contextFactory = contextFactory;
         }
 
-        public async Task<T> AddAsync(T item)
+        if (existingItem != null)
         {
-            await _onOperation.WaitAsync();
+            _logger.LogDebug("Cached item already added for {Type} {Id} {Value}", typeof(T).Name, item.Id,
-            T existingItem = null;
+                item.Value);
+            _onOperation.Release();
+            return existingItem;
+        }
 
-            if (_cachedItems.ContainsKey(item.Id))
+        try
+        {
+            _logger.LogDebug("Adding new {Type} with {Id} {Value}", typeof(T).Name, item.Id, item.Value);
+            await using var context = _contextFactory.CreateContext();
+            context.Set<T>().Add(item);
+            await context.SaveChangesAsync();
+            _cachedItems.Add(item.Id, item);
+            return item;
+        }
+        catch (Exception ex)
+        {
+            _logger.LogError(ex, "Could not add item to cache for {Type}", typeof(T).Name);
+            throw new Exception("Could not add item to cache");
+        }
+        finally
+        {
+            if (_onOperation.CurrentCount == 0)
             {
-                existingItem = _cachedItems[item.Id];
-            }
-
-            if (existingItem != null)
-            {
-                _logger.LogDebug("Cached item already added for {type} {id} {value}", typeof(T).Name, item.Id,
-                    item.Value);
                 _onOperation.Release();
-                return existingItem;
-            }
-
-            try
-            {
-                _logger.LogDebug("Adding new {type} with {id} {value}", typeof(T).Name, item.Id, item.Value);
-                await using var context = _contextFactory.CreateContext();
-                context.Set<T>().Add(item);
-                await context.SaveChangesAsync();
-                _cachedItems.Add(item.Id, item);
-                return item;
-            }
-            catch (Exception ex)
-            {
-                _logger.LogError(ex, "Could not add item to cache for {type}", typeof(T).Name);
-                throw new Exception("Could not add item to cache");
-            }
-            finally
-            {
-                if (_onOperation.CurrentCount == 0)
-                {
-                    _onOperation.Release();
-                }
-            }
-        }
-
-        public async Task<T> FirstAsync(Func<T, bool> query)
-        {
-            await _onOperation.WaitAsync();
-
-            try
-            {
-                var cachedResult = _cachedItems.Values.Where(query);
-
-                if (cachedResult.Any())
-                {
-                    return cachedResult.FirstOrDefault();
-                }
-            }
-
-            catch
-            {
-            }
-
-            finally
-            {
-                if (_onOperation.CurrentCount == 0)
-                {
-                    _onOperation.Release(1);
-                }
-            }
-
-            return null;
-        }
-
-        public IEnumerable<T> GetAll()
-        {
-            return _cachedItems.Values;
-        }
-
-        public async Task InitializeAsync()
-        {
-            try
-            {
-                await using var context = _contextFactory.CreateContext(false);
-                _cachedItems = await context.Set<T>().ToDictionaryAsync(item => item.Id);
-            }
-            catch (Exception ex)
-            {
-                _logger.LogError(ex, "Could not initialize caching for {cacheType}", typeof(T).Name);
             }
         }
     }
+
+    public async Task<T> FirstAsync(Func<T, bool> query)
+    {
+        try
+        {
+            await _onOperation.WaitAsync();
+            var cachedResult = _cachedItems.Values.Where(query);
+            return cachedResult.FirstOrDefault();
+        }
+        finally
+        {
+            if (_onOperation.CurrentCount == 0)
+            {
+                _onOperation.Release(1);
+            }
+        }
+    }
+
+    public IEnumerable<T> GetAll()
+    {
+        return _cachedItems.Values;
+    }
+
+    public async Task InitializeAsync()
+    {
+        try
+        {
+            await using var context = _contextFactory.CreateContext(false);
+            _cachedItems = await context.Set<T>().ToDictionaryAsync(item => item.Id);
+        }
+        catch (Exception ex)
+        {
+            _logger.LogError(ex, "Could not initialize caching for {CacheType}", typeof(T).Name);
+        }
+    }
 }