From 71b4b49182192b39bead21c41f51d37ac6a5e590 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Apr 2026 07:05:43 +0000 Subject: [PATCH 1/3] feat(core): add unified ICacheStore abstraction Introduces SimpleModule.Core.Caching with an async-first ICacheStore interface so modules no longer call IMemoryCache directly. The default implementation, MemoryCacheStore, wraps IMemoryCache and adds two capabilities the raw memory cache lacks: * stampede-safe GetOrCreateAsync via per-key SemaphoreSlim * RemoveByPrefixAsync via tracked-key set + post-eviction cleanup CacheResult distinguishes a miss from a cached null so negative caching (used by SettingsService) keeps working. CacheEntryOptions is implementation-agnostic so a future Redis-backed store can drop in without touching call sites. WithPrefix returns a scoped decorator for module/tenant key namespacing. AddSimpleModuleCaching is wired into AddSimpleModuleInfrastructure so every module gets ICacheStore registered automatically; SettingsService and PublicMenuService are migrated as the proof point. Adds 16 unit tests in SimpleModule.Core.Tests covering hit/miss/negative cache, stampede coalescing, expiration, prefix removal, and the prefixed-store decorator. --- .../Caching/CacheEntryOptions.cs | 41 +++ .../SimpleModule.Core/Caching/CacheKey.cs | 19 ++ .../SimpleModule.Core/Caching/CacheResult.cs | 24 ++ .../Caching/CacheStoreExtensions.cs | 40 +++ .../CachingServiceCollectionExtensions.cs | 24 ++ .../SimpleModule.Core/Caching/ICacheStore.cs | 55 ++++ .../Caching/MemoryCacheStore.cs | 172 ++++++++++++ .../Caching/PrefixedCacheStore.cs | 48 ++++ .../SimpleModuleHostExtensions.cs | 4 + .../Services/PublicMenuService.cs | 70 ++--- .../SimpleModule.Settings/SettingsService.cs | 19 +- .../Unit/PublicMenuServiceTests.cs | 10 +- .../Unit/SettingsServiceTests.cs | 6 +- .../Caching/MemoryCacheStoreTests.cs | 248 ++++++++++++++++++ 14 files changed, 736 insertions(+), 44 deletions(-) create mode 100644 framework/SimpleModule.Core/Caching/CacheEntryOptions.cs create mode 100644 framework/SimpleModule.Core/Caching/CacheKey.cs create mode 100644 framework/SimpleModule.Core/Caching/CacheResult.cs create mode 100644 framework/SimpleModule.Core/Caching/CacheStoreExtensions.cs create mode 100644 framework/SimpleModule.Core/Caching/CachingServiceCollectionExtensions.cs create mode 100644 framework/SimpleModule.Core/Caching/ICacheStore.cs create mode 100644 framework/SimpleModule.Core/Caching/MemoryCacheStore.cs create mode 100644 framework/SimpleModule.Core/Caching/PrefixedCacheStore.cs create mode 100644 tests/SimpleModule.Core.Tests/Caching/MemoryCacheStoreTests.cs diff --git a/framework/SimpleModule.Core/Caching/CacheEntryOptions.cs b/framework/SimpleModule.Core/Caching/CacheEntryOptions.cs new file mode 100644 index 00000000..82850d52 --- /dev/null +++ b/framework/SimpleModule.Core/Caching/CacheEntryOptions.cs @@ -0,0 +1,41 @@ +namespace SimpleModule.Core.Caching; + +/// +/// Implementation-agnostic options describing how an entry should be retained in the cache. +/// +public sealed class CacheEntryOptions +{ + /// + /// Lifetime relative to the time the entry is written. Mutually exclusive with + /// . + /// + public TimeSpan? AbsoluteExpirationRelativeToNow { get; init; } + + /// + /// An absolute point in time at which the entry expires. Mutually exclusive with + /// . + /// + public DateTimeOffset? AbsoluteExpiration { get; init; } + + /// + /// Sliding expiration window. The entry is evicted if it is not accessed within this window. + /// + public TimeSpan? SlidingExpiration { get; init; } + + /// + /// Optional size hint, used by stores that enforce a size limit. + /// + public long? Size { get; init; } + + /// + /// Creates options that expire after the supplied duration. + /// + public static CacheEntryOptions Expires(TimeSpan duration) => + new() { AbsoluteExpirationRelativeToNow = duration }; + + /// + /// Creates options with a sliding expiration window. + /// + public static CacheEntryOptions Sliding(TimeSpan window) => + new() { SlidingExpiration = window }; +} diff --git a/framework/SimpleModule.Core/Caching/CacheKey.cs b/framework/SimpleModule.Core/Caching/CacheKey.cs new file mode 100644 index 00000000..32d5f022 --- /dev/null +++ b/framework/SimpleModule.Core/Caching/CacheKey.cs @@ -0,0 +1,19 @@ +namespace SimpleModule.Core.Caching; + +/// +/// Helpers for composing consistent cache keys. +/// +public static class CacheKey +{ + /// + /// Joins the supplied parts with :, skipping null or empty segments. + /// + /// + /// CacheKey.Compose("settings", scope.ToString(), userId, key); + /// + public static string Compose(params string?[] parts) + { + ArgumentNullException.ThrowIfNull(parts); + return string.Join(':', parts.Where(p => !string.IsNullOrEmpty(p))); + } +} diff --git a/framework/SimpleModule.Core/Caching/CacheResult.cs b/framework/SimpleModule.Core/Caching/CacheResult.cs new file mode 100644 index 00000000..4fc81380 --- /dev/null +++ b/framework/SimpleModule.Core/Caching/CacheResult.cs @@ -0,0 +1,24 @@ +namespace SimpleModule.Core.Caching; + +/// +/// Result of a cache lookup. Distinguishes a miss from a hit that contains a +/// value (negative caching). +/// +/// The cached value type. +public readonly record struct CacheResult(bool Hit, T? Value); + +/// +/// Non-generic helpers for constructing values. +/// +public static class CacheResult +{ + /// + /// Creates a miss result for type . + /// + public static CacheResult Miss() => default; + + /// + /// Creates a hit result with the supplied value (which may be ). + /// + public static CacheResult Hit(T? value) => new(true, value); +} diff --git a/framework/SimpleModule.Core/Caching/CacheStoreExtensions.cs b/framework/SimpleModule.Core/Caching/CacheStoreExtensions.cs new file mode 100644 index 00000000..c9302126 --- /dev/null +++ b/framework/SimpleModule.Core/Caching/CacheStoreExtensions.cs @@ -0,0 +1,40 @@ +namespace SimpleModule.Core.Caching; + +/// +/// Convenience extensions over . +/// +public static class CacheStoreExtensions +{ + /// + /// Returns a view over the store where every key is automatically prefixed with + /// (joined with :). Useful for module- or tenant-scoped + /// cache namespacing without forcing every call site to remember the prefix. + /// + public static ICacheStore WithPrefix(this ICacheStore store, string prefix) + { + ArgumentNullException.ThrowIfNull(store); + return new PrefixedCacheStore(store, prefix); + } + + /// + /// Synchronous-style helper for the common pattern var v = await cache.GetOrCreateAsync(...) + /// where the factory is itself synchronous. + /// + public static ValueTask GetOrCreateAsync( + this ICacheStore store, + string key, + Func factory, + CacheEntryOptions? options = null, + CancellationToken cancellationToken = default + ) + { + ArgumentNullException.ThrowIfNull(store); + ArgumentNullException.ThrowIfNull(factory); + return store.GetOrCreateAsync( + key, + _ => new ValueTask(factory()), + options, + cancellationToken + ); + } +} diff --git a/framework/SimpleModule.Core/Caching/CachingServiceCollectionExtensions.cs b/framework/SimpleModule.Core/Caching/CachingServiceCollectionExtensions.cs new file mode 100644 index 00000000..b692085c --- /dev/null +++ b/framework/SimpleModule.Core/Caching/CachingServiceCollectionExtensions.cs @@ -0,0 +1,24 @@ +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; + +namespace SimpleModule.Core.Caching; + +/// +/// DI registration for the unified SimpleModule caching abstraction. +/// +public static class CachingServiceCollectionExtensions +{ + /// + /// Registers with the default in-process + /// implementation, along with the underlying + /// . Safe to call + /// multiple times — registrations are added with TryAdd. + /// + public static IServiceCollection AddSimpleModuleCaching(this IServiceCollection services) + { + ArgumentNullException.ThrowIfNull(services); + services.AddMemoryCache(); + services.TryAddSingleton(); + return services; + } +} diff --git a/framework/SimpleModule.Core/Caching/ICacheStore.cs b/framework/SimpleModule.Core/Caching/ICacheStore.cs new file mode 100644 index 00000000..8b7141a7 --- /dev/null +++ b/framework/SimpleModule.Core/Caching/ICacheStore.cs @@ -0,0 +1,55 @@ +namespace SimpleModule.Core.Caching; + +/// +/// Unified caching abstraction used across SimpleModule modules. +/// +/// +/// The default registration is an in-process MemoryCacheStore backed by +/// . The interface is intentionally +/// async-first so that distributed implementations (Redis, etc.) can be plugged in without +/// changing call sites. +/// +public interface ICacheStore +{ + /// + /// Looks up an entry. Returns a that distinguishes a miss from a + /// hit containing a value (negative caching). + /// + ValueTask> TryGetAsync( + string key, + CancellationToken cancellationToken = default + ); + + /// + /// Writes an entry, replacing any existing value for . + /// + ValueTask SetAsync( + string key, + T? value, + CacheEntryOptions? options = null, + CancellationToken cancellationToken = default + ); + + /// + /// Returns the cached value for , invoking + /// to populate the cache on a miss. Implementations must guard against cache stampedes — + /// concurrent callers for the same key see invoked at most once. + /// + ValueTask GetOrCreateAsync( + string key, + Func> factory, + CacheEntryOptions? options = null, + CancellationToken cancellationToken = default + ); + + /// + /// Removes a single entry. No-op if the key is absent. + /// + ValueTask RemoveAsync(string key, CancellationToken cancellationToken = default); + + /// + /// Removes every entry whose key starts with . Useful for + /// invalidating a logical group (e.g., all entries for a user, tenant, or module). + /// + ValueTask RemoveByPrefixAsync(string prefix, CancellationToken cancellationToken = default); +} diff --git a/framework/SimpleModule.Core/Caching/MemoryCacheStore.cs b/framework/SimpleModule.Core/Caching/MemoryCacheStore.cs new file mode 100644 index 00000000..0d78ff1e --- /dev/null +++ b/framework/SimpleModule.Core/Caching/MemoryCacheStore.cs @@ -0,0 +1,172 @@ +using System.Collections.Concurrent; +using Microsoft.Extensions.Caching.Memory; + +namespace SimpleModule.Core.Caching; + +/// +/// In-process implementation backed by +/// . Adds two capabilities on top of the raw memory cache: +/// stampede-safe via per-key locking, and +/// via a tracked key set. +/// +public sealed class MemoryCacheStore : ICacheStore, IDisposable +{ + private readonly IMemoryCache _cache; + private readonly ConcurrentDictionary _trackedKeys = new(StringComparer.Ordinal); + private readonly ConcurrentDictionary _keyLocks = new( + StringComparer.Ordinal + ); + + public MemoryCacheStore(IMemoryCache cache) + { + ArgumentNullException.ThrowIfNull(cache); + _cache = cache; + } + + public ValueTask> TryGetAsync( + string key, + CancellationToken cancellationToken = default + ) + { + ArgumentException.ThrowIfNullOrEmpty(key); + cancellationToken.ThrowIfCancellationRequested(); + + if (_cache.TryGetValue(key, out var raw)) + { + return ValueTask.FromResult(CacheResult.Hit((T?)raw)); + } + + return ValueTask.FromResult(CacheResult.Miss()); + } + + public ValueTask SetAsync( + string key, + T? value, + CacheEntryOptions? options = null, + CancellationToken cancellationToken = default + ) + { + ArgumentException.ThrowIfNullOrEmpty(key); + cancellationToken.ThrowIfCancellationRequested(); + + SetCore(key, value, options); + return ValueTask.CompletedTask; + } + + public async ValueTask GetOrCreateAsync( + string key, + Func> factory, + CacheEntryOptions? options = null, + CancellationToken cancellationToken = default + ) + { + ArgumentException.ThrowIfNullOrEmpty(key); + ArgumentNullException.ThrowIfNull(factory); + + if (_cache.TryGetValue(key, out var existing)) + { + return (T?)existing; + } + + var gate = _keyLocks.GetOrAdd(key, static _ => new SemaphoreSlim(1, 1)); + await gate.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + if (_cache.TryGetValue(key, out existing)) + { + return (T?)existing; + } + + var value = await factory(cancellationToken).ConfigureAwait(false); + SetCore(key, value, options); + return value; + } + finally + { + gate.Release(); + } + } + + public ValueTask RemoveAsync(string key, CancellationToken cancellationToken = default) + { + ArgumentException.ThrowIfNullOrEmpty(key); + cancellationToken.ThrowIfCancellationRequested(); + + _cache.Remove(key); + _trackedKeys.TryRemove(key, out _); + return ValueTask.CompletedTask; + } + + public ValueTask RemoveByPrefixAsync( + string prefix, + CancellationToken cancellationToken = default + ) + { + ArgumentException.ThrowIfNullOrEmpty(prefix); + cancellationToken.ThrowIfCancellationRequested(); + + foreach (var key in _trackedKeys.Keys) + { + if (key.StartsWith(prefix, StringComparison.Ordinal)) + { + _cache.Remove(key); + _trackedKeys.TryRemove(key, out _); + } + } + + return ValueTask.CompletedTask; + } + + private void SetCore(string key, T? value, CacheEntryOptions? options) + { + using var entry = _cache.CreateEntry(key); + entry.Value = value; + + if (options is not null) + { + if (options.AbsoluteExpirationRelativeToNow is { } relative) + { + entry.AbsoluteExpirationRelativeToNow = relative; + } + + if (options.AbsoluteExpiration is { } absolute) + { + entry.AbsoluteExpiration = absolute; + } + + if (options.SlidingExpiration is { } sliding) + { + entry.SlidingExpiration = sliding; + } + + if (options.Size is { } size) + { + entry.Size = size; + } + } + + // Track the key so RemoveByPrefixAsync can find it. The eviction callback below + // also removes from the tracking set when the entry naturally expires or is evicted + // by memory pressure, so the set stays bounded. + _trackedKeys[key] = 0; + entry.RegisterPostEvictionCallback( + static (evictedKey, _, _, state) => + { + if (state is MemoryCacheStore self && evictedKey is string s) + { + self._trackedKeys.TryRemove(s, out _); + } + }, + this + ); + } + + public void Dispose() + { + foreach (var gate in _keyLocks.Values) + { + gate.Dispose(); + } + _keyLocks.Clear(); + } +} diff --git a/framework/SimpleModule.Core/Caching/PrefixedCacheStore.cs b/framework/SimpleModule.Core/Caching/PrefixedCacheStore.cs new file mode 100644 index 00000000..5a10dd51 --- /dev/null +++ b/framework/SimpleModule.Core/Caching/PrefixedCacheStore.cs @@ -0,0 +1,48 @@ +namespace SimpleModule.Core.Caching; + +/// +/// Decorator that scopes every key with a fixed prefix before forwarding to the inner store. +/// Created via . +/// +internal sealed class PrefixedCacheStore : ICacheStore +{ + private readonly ICacheStore _inner; + private readonly string _prefix; + + public PrefixedCacheStore(ICacheStore inner, string prefix) + { + ArgumentNullException.ThrowIfNull(inner); + ArgumentException.ThrowIfNullOrEmpty(prefix); + _inner = inner; + _prefix = prefix.EndsWith(':') ? prefix : prefix + ':'; + } + + private string Scope(string key) => _prefix + key; + + public ValueTask> TryGetAsync( + string key, + CancellationToken cancellationToken = default + ) => _inner.TryGetAsync(Scope(key), cancellationToken); + + public ValueTask SetAsync( + string key, + T? value, + CacheEntryOptions? options = null, + CancellationToken cancellationToken = default + ) => _inner.SetAsync(Scope(key), value, options, cancellationToken); + + public ValueTask GetOrCreateAsync( + string key, + Func> factory, + CacheEntryOptions? options = null, + CancellationToken cancellationToken = default + ) => _inner.GetOrCreateAsync(Scope(key), factory, options, cancellationToken); + + public ValueTask RemoveAsync(string key, CancellationToken cancellationToken = default) => + _inner.RemoveAsync(Scope(key), cancellationToken); + + public ValueTask RemoveByPrefixAsync( + string prefix, + CancellationToken cancellationToken = default + ) => _inner.RemoveByPrefixAsync(_prefix + prefix, cancellationToken); +} diff --git a/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.cs b/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.cs index f1ca2614..0802cc09 100644 --- a/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.cs +++ b/framework/SimpleModule.Hosting/SimpleModuleHostExtensions.cs @@ -8,6 +8,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Hosting; +using SimpleModule.Core.Caching; using SimpleModule.Core.Constants; using SimpleModule.Core.Events; using SimpleModule.Core.Exceptions; @@ -70,6 +71,9 @@ public static WebApplicationBuilder AddSimpleModuleInfrastructure( builder.Services.AddSingleton(); + // Unified caching abstraction (ICacheStore) shared across all modules. + builder.Services.AddSimpleModuleCaching(); + builder.Services.AddSingleton(); builder.Services.AddHostedService(); builder.Services.AddScoped(); diff --git a/modules/Settings/src/SimpleModule.Settings/Services/PublicMenuService.cs b/modules/Settings/src/SimpleModule.Settings/Services/PublicMenuService.cs index 25cf35d4..6a0a2531 100644 --- a/modules/Settings/src/SimpleModule.Settings/Services/PublicMenuService.cs +++ b/modules/Settings/src/SimpleModule.Settings/Services/PublicMenuService.cs @@ -1,6 +1,6 @@ using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Options; +using SimpleModule.Core.Caching; using SimpleModule.Core.Menu; using SimpleModule.Settings.Contracts; using SimpleModule.Settings.Entities; @@ -9,7 +9,7 @@ namespace SimpleModule.Settings.Services; public sealed class PublicMenuService( SettingsDbContext db, - IMemoryCache cache, + ICacheStore cache, IOptions moduleOptions ) : IPublicMenuProvider { @@ -18,20 +18,19 @@ IOptions moduleOptions public async Task> GetMenuTreeAsync() { - if ( - cache.TryGetValue(MenuTreeCacheKey, out IReadOnlyList? cached) - && cached is not null - ) - return cached; - - var entities = await db - .PublicMenuItems.Where(e => e.IsVisible) - .OrderBy(e => e.SortOrder) - .ToListAsync(); - - var tree = BuildPublicTree(entities, parentId: null); - cache.Set(MenuTreeCacheKey, tree, moduleOptions.Value.CacheDuration); - return tree; + var result = await cache.GetOrCreateAsync>( + MenuTreeCacheKey, + async ct => + { + var entities = await db + .PublicMenuItems.Where(e => e.IsVisible) + .OrderBy(e => e.SortOrder) + .ToListAsync(ct); + return BuildPublicTree(entities, parentId: null); + }, + CacheEntryOptions.Expires(moduleOptions.Value.CacheDuration) + ); + return result ?? []; } [System.Diagnostics.CodeAnalysis.SuppressMessage( @@ -41,16 +40,17 @@ public async Task> GetMenuTreeAsync() )] public async Task GetHomePageUrlAsync() { - if (cache.TryGetValue(HomePageCacheKey, out string? cached)) - return cached; - - var entity = await db - .PublicMenuItems.Where(e => e.IsVisible && e.IsHomePage) - .FirstOrDefaultAsync(); - - var url = entity is not null ? (entity.Url ?? entity.PageRoute) : null; - cache.Set(HomePageCacheKey, url, moduleOptions.Value.CacheDuration); - return url; + return await cache.GetOrCreateAsync( + HomePageCacheKey, + async ct => + { + var entity = await db + .PublicMenuItems.Where(e => e.IsVisible && e.IsHomePage) + .FirstOrDefaultAsync(ct); + return entity is not null ? (entity.Url ?? entity.PageRoute) : null; + }, + CacheEntryOptions.Expires(moduleOptions.Value.CacheDuration) + ); } public async Task> GetAllAsync() @@ -98,7 +98,7 @@ await db db.PublicMenuItems.Add(entity); await db.SaveChangesAsync(); - InvalidateCache(); + await InvalidateCache(); return entity; } @@ -122,7 +122,7 @@ await db entity.UpdatedAt = DateTimeOffset.UtcNow; await db.SaveChangesAsync(); - InvalidateCache(); + await InvalidateCache(); return entity; } @@ -134,7 +134,7 @@ public async Task DeleteAsync(int id) db.PublicMenuItems.Remove(entity); await db.SaveChangesAsync(); - InvalidateCache(); + await InvalidateCache(); return true; } @@ -152,7 +152,7 @@ public async Task ReorderAsync(ReorderMenuItemsRequest request) } await db.SaveChangesAsync(); - InvalidateCache(); + await InvalidateCache(); } public async Task SetHomePageAsync(int id) @@ -167,14 +167,14 @@ public async Task SetHomePageAsync(int id) await db.SaveChangesAsync(); } - InvalidateCache(); + await InvalidateCache(); } public async Task ClearHomePageAsync() { await ClearAllHomePageFlags(); await db.SaveChangesAsync(); - InvalidateCache(); + await InvalidateCache(); } private async Task ClearAllHomePageFlags() @@ -253,9 +253,9 @@ private static List BuildDtoTree( .ToList(); } - private void InvalidateCache() + private async ValueTask InvalidateCache() { - cache.Remove(MenuTreeCacheKey); - cache.Remove(HomePageCacheKey); + await cache.RemoveAsync(MenuTreeCacheKey); + await cache.RemoveAsync(HomePageCacheKey); } } diff --git a/modules/Settings/src/SimpleModule.Settings/SettingsService.cs b/modules/Settings/src/SimpleModule.Settings/SettingsService.cs index c6f64d9c..aea5c27f 100644 --- a/modules/Settings/src/SimpleModule.Settings/SettingsService.cs +++ b/modules/Settings/src/SimpleModule.Settings/SettingsService.cs @@ -1,8 +1,8 @@ using System.Text.Json; using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using SimpleModule.Core.Caching; using SimpleModule.Core.Settings; using SimpleModule.Settings.Contracts; using SimpleModule.Settings.Entities; @@ -12,7 +12,7 @@ namespace SimpleModule.Settings; public sealed partial class SettingsService( SettingsDbContext db, ISettingsDefinitionRegistry definitions, - IMemoryCache cache, + ICacheStore cache, IOptions moduleOptions, ILogger logger ) : ISettingsContracts @@ -25,8 +25,9 @@ ILogger logger { var cacheKey = BuildCacheKey(key, scope, userId); - if (cache.TryGetValue(cacheKey, out string? cached)) - return cached; + var hit = await cache.TryGetAsync(cacheKey); + if (hit.Hit) + return hit.Value; var entity = await db .Settings.AsNoTracking() @@ -36,7 +37,11 @@ ILogger logger && (scope == SettingScope.User ? s.UserId == userId : s.UserId == null) ); - cache.Set(cacheKey, entity?.Value, moduleOptions.Value.CacheDuration); + await cache.SetAsync( + cacheKey, + entity?.Value, + CacheEntryOptions.Expires(moduleOptions.Value.CacheDuration) + ); return entity?.Value; } @@ -104,7 +109,7 @@ public async Task SetSettingAsync( } await db.SaveChangesAsync(); - cache.Remove(BuildCacheKey(key, scope, userId)); + await cache.RemoveAsync(BuildCacheKey(key, scope, userId)); LogSettingUpdated(key, scope); } @@ -120,7 +125,7 @@ public async Task DeleteSettingAsync(string key, SettingScope scope, string? use { db.Settings.Remove(entity); await db.SaveChangesAsync(); - cache.Remove(BuildCacheKey(key, scope, userId)); + await cache.RemoveAsync(BuildCacheKey(key, scope, userId)); LogSettingDeleted(key, scope); } } diff --git a/modules/Settings/tests/SimpleModule.Settings.Tests/Unit/PublicMenuServiceTests.cs b/modules/Settings/tests/SimpleModule.Settings.Tests/Unit/PublicMenuServiceTests.cs index 38386bf7..0a165016 100644 --- a/modules/Settings/tests/SimpleModule.Settings.Tests/Unit/PublicMenuServiceTests.cs +++ b/modules/Settings/tests/SimpleModule.Settings.Tests/Unit/PublicMenuServiceTests.cs @@ -2,6 +2,7 @@ using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Options; +using SimpleModule.Core.Caching; using SimpleModule.Database; using SimpleModule.Settings; using SimpleModule.Settings.Contracts; @@ -15,6 +16,7 @@ public sealed class PublicMenuServiceTests : IDisposable private readonly SettingsDbContext _db; private readonly PublicMenuService _service; private readonly MemoryCache _cache; + private readonly MemoryCacheStore _cacheStore; public PublicMenuServiceTests() { @@ -28,7 +30,12 @@ public PublicMenuServiceTests() _db.Database.EnsureCreated(); _cache = new MemoryCache(new MemoryCacheOptions()); - _service = new PublicMenuService(_db, _cache, Options.Create(new SettingsModuleOptions())); + _cacheStore = new MemoryCacheStore(_cache); + _service = new PublicMenuService( + _db, + _cacheStore, + Options.Create(new SettingsModuleOptions()) + ); } [Fact] @@ -218,6 +225,7 @@ public async Task SetHomePageAsync_ClearsPreviousHomePage() public void Dispose() { + _cacheStore.Dispose(); _cache.Dispose(); _db.Dispose(); GC.SuppressFinalize(this); diff --git a/modules/Settings/tests/SimpleModule.Settings.Tests/Unit/SettingsServiceTests.cs b/modules/Settings/tests/SimpleModule.Settings.Tests/Unit/SettingsServiceTests.cs index 00cfede9..24328241 100644 --- a/modules/Settings/tests/SimpleModule.Settings.Tests/Unit/SettingsServiceTests.cs +++ b/modules/Settings/tests/SimpleModule.Settings.Tests/Unit/SettingsServiceTests.cs @@ -3,6 +3,7 @@ using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; +using SimpleModule.Core.Caching; using SimpleModule.Core.Settings; using SimpleModule.Database; using SimpleModule.Settings; @@ -13,6 +14,7 @@ public sealed class SettingsServiceTests : IDisposable { private readonly SettingsDbContext _db; private readonly MemoryCache _cache; + private readonly MemoryCacheStore _cacheStore; private readonly SettingsService _service; public SettingsServiceTests() @@ -38,10 +40,11 @@ public SettingsServiceTests() ]); _cache = new MemoryCache(new MemoryCacheOptions()); + _cacheStore = new MemoryCacheStore(_cache); _service = new SettingsService( _db, registry, - _cache, + _cacheStore, Options.Create(new SettingsModuleOptions()), NullLogger.Instance ); @@ -139,6 +142,7 @@ public async Task GetSettingAsync_Bool_NoDbValue_ReturnsFalse() public void Dispose() { + _cacheStore.Dispose(); _cache.Dispose(); _db.Dispose(); GC.SuppressFinalize(this); diff --git a/tests/SimpleModule.Core.Tests/Caching/MemoryCacheStoreTests.cs b/tests/SimpleModule.Core.Tests/Caching/MemoryCacheStoreTests.cs new file mode 100644 index 00000000..f8a04eb2 --- /dev/null +++ b/tests/SimpleModule.Core.Tests/Caching/MemoryCacheStoreTests.cs @@ -0,0 +1,248 @@ +using FluentAssertions; +using Microsoft.Extensions.Caching.Memory; +using SimpleModule.Core.Caching; + +namespace SimpleModule.Core.Tests.Caching; + +public sealed class MemoryCacheStoreTests : IDisposable +{ + private readonly MemoryCache _memoryCache = new(new MemoryCacheOptions()); + private readonly MemoryCacheStore _store; + + public MemoryCacheStoreTests() + { + _store = new MemoryCacheStore(_memoryCache); + } + + [Fact] + public async Task TryGetAsync_ReturnsMiss_WhenKeyAbsent() + { + var result = await _store.TryGetAsync("missing"); + + result.Hit.Should().BeFalse(); + result.Value.Should().BeNull(); + } + + [Fact] + public async Task SetAsync_ThenTryGetAsync_RoundTripsValue() + { + await _store.SetAsync("k1", "value"); + + var result = await _store.TryGetAsync("k1"); + + result.Hit.Should().BeTrue(); + result.Value.Should().Be("value"); + } + + [Fact] + public async Task SetAsync_AllowsCachingNullForNegativeCaching() + { + await _store.SetAsync("k1", null); + + var result = await _store.TryGetAsync("k1"); + + result.Hit.Should().BeTrue("a cached null should be a hit, not a miss"); + result.Value.Should().BeNull(); + } + + [Fact] + public async Task SetAsync_OverwritesExistingValue() + { + await _store.SetAsync("k1", "first"); + await _store.SetAsync("k1", "second"); + + var result = await _store.TryGetAsync("k1"); + + result.Value.Should().Be("second"); + } + + [Fact] + public async Task RemoveAsync_DeletesEntry() + { + await _store.SetAsync("k1", "value"); + + await _store.RemoveAsync("k1"); + + var result = await _store.TryGetAsync("k1"); + result.Hit.Should().BeFalse(); + } + + [Fact] + public async Task RemoveAsync_IsNoOp_WhenKeyAbsent() + { + var act = async () => await _store.RemoveAsync("ghost"); + + await act.Should().NotThrowAsync(); + } + + [Fact] + public async Task GetOrCreateAsync_InvokesFactory_OnMiss() + { + var calls = 0; + + var value = await _store.GetOrCreateAsync( + "k1", + _ => + { + calls++; + return new ValueTask("created"); + } + ); + + value.Should().Be("created"); + calls.Should().Be(1); + } + + [Fact] + public async Task GetOrCreateAsync_SkipsFactory_OnHit() + { + await _store.SetAsync("k1", "existing"); + var calls = 0; + + var value = await _store.GetOrCreateAsync( + "k1", + _ => + { + calls++; + return new ValueTask("created"); + } + ); + + value.Should().Be("existing"); + calls.Should().Be(0); + } + + [Fact] + public async Task GetOrCreateAsync_PreventsStampede_UnderConcurrentCallers() + { + var factoryCalls = 0; + var gate = new TaskCompletionSource(); + + async ValueTask Factory(CancellationToken _) + { + Interlocked.Increment(ref factoryCalls); + await gate.Task; + return "value"; + } + + var t1 = _store.GetOrCreateAsync("stampede", Factory).AsTask(); + var t2 = _store.GetOrCreateAsync("stampede", Factory).AsTask(); + var t3 = _store.GetOrCreateAsync("stampede", Factory).AsTask(); + + // Let the first factory release. + gate.SetResult(); + var results = await Task.WhenAll(t1, t2, t3); + + factoryCalls + .Should() + .Be(1, "concurrent GetOrCreateAsync calls for the same key must coalesce"); + results.Should().AllBeEquivalentTo("value"); + } + + [Fact] + public async Task GetOrCreateAsync_RespectsExpirationOptions() + { + await _store.GetOrCreateAsync( + "k1", + _ => new ValueTask("v"), + CacheEntryOptions.Expires(TimeSpan.FromMilliseconds(50)) + ); + + await Task.Delay(150); + + var result = await _store.TryGetAsync("k1"); + result.Hit.Should().BeFalse(); + } + + [Fact] + public async Task RemoveByPrefixAsync_RemovesAllMatchingKeys() + { + await _store.SetAsync("user:1:profile", "p1"); + await _store.SetAsync("user:1:settings", "s1"); + await _store.SetAsync("user:2:profile", "p2"); + await _store.SetAsync("system:bootstrap", "x"); + + await _store.RemoveByPrefixAsync("user:1:"); + + (await _store.TryGetAsync("user:1:profile")).Hit.Should().BeFalse(); + (await _store.TryGetAsync("user:1:settings")).Hit.Should().BeFalse(); + (await _store.TryGetAsync("user:2:profile")).Hit.Should().BeTrue(); + (await _store.TryGetAsync("system:bootstrap")).Hit.Should().BeTrue(); + } + + [Fact] + public async Task RemoveByPrefixAsync_RemovesEverything_WithEmptyKey() + { + await _store.SetAsync("a", "1"); + await _store.SetAsync("b", "2"); + + await _store.RemoveByPrefixAsync("a"); + + (await _store.TryGetAsync("a")).Hit.Should().BeFalse(); + (await _store.TryGetAsync("b")).Hit.Should().BeTrue(); + } + + [Fact] + public async Task TryGetAsync_ThrowsOnNullOrEmptyKey() + { + var act1 = async () => await _store.TryGetAsync(null!); + var act2 = async () => await _store.TryGetAsync(string.Empty); + + await act1.Should().ThrowAsync(); + await act2.Should().ThrowAsync(); + } + + [Fact] + public async Task WithPrefix_ScopesAllOperations() + { + var scoped = _store.WithPrefix("tenant-a"); + + await scoped.SetAsync("user", "alice"); + + // Scoped store sees the value. + var hit = await scoped.TryGetAsync("user"); + hit.Hit.Should().BeTrue(); + hit.Value.Should().Be("alice"); + + // Underlying store sees the prefixed key. + var underlying = await _store.TryGetAsync("tenant-a:user"); + underlying.Hit.Should().BeTrue(); + underlying.Value.Should().Be("alice"); + + // The unscoped key does not exist. + (await _store.TryGetAsync("user")) + .Hit.Should() + .BeFalse(); + } + + [Fact] + public async Task WithPrefix_RemoveByPrefix_ScopesPrefix() + { + var scoped = _store.WithPrefix("tenant-a"); + await scoped.SetAsync("user:1", "alice"); + await scoped.SetAsync("user:2", "bob"); + await _store.SetAsync("user:1", "global"); // unscoped, must survive + + await scoped.RemoveByPrefixAsync("user"); + + (await scoped.TryGetAsync("user:1")).Hit.Should().BeFalse(); + (await scoped.TryGetAsync("user:2")).Hit.Should().BeFalse(); + (await _store.TryGetAsync("user:1")).Hit.Should().BeTrue(); + } + + [Fact] + public void CacheKey_Compose_JoinsPartsAndSkipsEmpty() + { + CacheKey.Compose("a", "b", "c").Should().Be("a:b:c"); + CacheKey.Compose("a", null, "c").Should().Be("a:c"); + CacheKey.Compose("a", string.Empty, "c").Should().Be("a:c"); + CacheKey.Compose("only").Should().Be("only"); + } + + public void Dispose() + { + _store.Dispose(); + _memoryCache.Dispose(); + GC.SuppressFinalize(this); + } +} From 1c02fe5276ce8df49b9337cf8bb4b35676a36b1f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Apr 2026 07:54:45 +0000 Subject: [PATCH 2/3] refactor: migrate all remaining cache users to ICacheStore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces direct IMemoryCache usage with the unified ICacheStore abstraction across every module that caches state in-process: * FeatureFlagService — two cache layers (per-flag + all-flags aggregate) now use GetOrCreateAsync with stampede protection, and the invalidation path is async * PermissionClaimsTransformation — permission set lookup per user + role combination * HostNameTenantResolver — hostname-to-tenant lookup * NuGetMarketplaceService — search and package detail caches, replacing the IMemoryCache entry-callback style with CacheEntryOptions * InstalledPackageDetector — installed NuGet id set * LocaleResolutionMiddleware — explicit user-setting cache plus Accept-Language cache; factored the header fallback into an async method so the awaited TryGet/Set flow stays clean Drops the per-module services.AddMemoryCache() calls from FeatureFlags, Localization, Marketplace, Settings, and Tenants — AddSimpleModuleCaching() is wired into the host infrastructure and already registers IMemoryCache and ICacheStore globally. Test projects (FeatureFlags, Localization) construct a MemoryCacheStore(MemoryCache) pair so they still exercise the real implementation. Verified: all 7 affected test projects pass — 216 Core, 27 Settings, 14 FeatureFlags, 26 Localization, 15 Permissions, 22 Tenants, 19 Marketplace (339 tests total). --- .../FeatureFlagService.cs | 138 +++++++++--------- .../FeatureFlagsModule.cs | 1 - .../Unit/FeatureFlagServiceTests.cs | 16 +- .../LocalizationModule.cs | 1 - .../Middleware/LocaleResolutionMiddleware.cs | 31 ++-- .../Unit/LocaleResolutionMiddlewareTests.cs | 25 +++- .../InstalledPackageDetector.cs | 23 ++- .../MarketplaceModule.cs | 1 - .../NuGetMarketplaceService.cs | 26 ++-- .../PermissionClaimsTransformation.cs | 36 +++-- .../SimpleModule.Settings/SettingsModule.cs | 1 - .../Resolvers/HostNameTenantResolver.cs | 20 ++- .../src/SimpleModule.Tenants/TenantsModule.cs | 1 - 13 files changed, 172 insertions(+), 148 deletions(-) diff --git a/modules/FeatureFlags/src/SimpleModule.FeatureFlags/FeatureFlagService.cs b/modules/FeatureFlags/src/SimpleModule.FeatureFlags/FeatureFlagService.cs index 9a8f1599..7933a3ba 100644 --- a/modules/FeatureFlags/src/SimpleModule.FeatureFlags/FeatureFlagService.cs +++ b/modules/FeatureFlags/src/SimpleModule.FeatureFlags/FeatureFlagService.cs @@ -1,7 +1,7 @@ using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using SimpleModule.Core.Caching; using SimpleModule.Core.Entities; using SimpleModule.Core.FeatureFlags; using SimpleModule.FeatureFlags.Contracts; @@ -12,7 +12,7 @@ namespace SimpleModule.FeatureFlags; public sealed partial class FeatureFlagService( FeatureFlagsDbContext db, IFeatureFlagRegistry registry, - IMemoryCache cache, + ICacheStore cache, ILogger logger, IServiceProvider serviceProvider ) : IFeatureFlagContracts, IFeatureFlagService @@ -125,7 +125,7 @@ UpdateFeatureFlagRequest request } await db.SaveChangesAsync(); - InvalidateCache(flagName); + await InvalidateCacheAsync(flagName); LogFlagToggled(logger, flagName, request.IsEnabled); @@ -177,7 +177,7 @@ SetOverrideRequest request } await db.SaveChangesAsync(); - InvalidateCache(flagName); + await InvalidateCacheAsync(flagName); return new FeatureFlagOverride { @@ -196,78 +196,86 @@ public async Task DeleteOverrideAsync(int overrideId) { db.FeatureFlagOverrides.Remove(entity); await db.SaveChangesAsync(); - InvalidateCache(entity.FlagName); + await InvalidateCacheAsync(entity.FlagName); } } private async Task> GetAllFlagDataAsync() { - if ( - cache.TryGetValue(AllFlagDataCacheKey, out Dictionary? cached) - && cached is not null - ) - { - return cached; - } - - var definitions = registry.GetAllDefinitions(); - var flagNames = definitions.Select(d => d.Name).ToList(); - - var dbFlags = await db - .FeatureFlags.AsNoTracking() - .Where(f => flagNames.Contains(f.Name)) - .ToDictionaryAsync(f => f.Name); - - var dbOverrides = await db - .FeatureFlagOverrides.AsNoTracking() - .Where(o => flagNames.Contains(o.FlagName)) - .ToListAsync(); - - var overridesByFlag = dbOverrides - .GroupBy(o => o.FlagName) - .ToDictionary(g => g.Key, g => g.ToList()); - - var allData = new Dictionary(flagNames.Count, StringComparer.Ordinal); - foreach (var def in definitions) - { - var isEnabled = dbFlags.TryGetValue(def.Name, out var entity) - ? entity.IsEnabled - : def.DefaultEnabled; - - var flagOverrides = overridesByFlag.GetValueOrDefault(def.Name, []); - var data = BuildFlagData(isEnabled, flagOverrides); - - allData[def.Name] = data; - cache.Set($"ff:data:{def.Name}", data, CacheDuration); - } + var result = await cache.GetOrCreateAsync>( + AllFlagDataCacheKey, + async ct => + { + var definitions = registry.GetAllDefinitions(); + var flagNames = definitions.Select(d => d.Name).ToList(); + + var dbFlags = await db + .FeatureFlags.AsNoTracking() + .Where(f => flagNames.Contains(f.Name)) + .ToDictionaryAsync(f => f.Name, ct); + + var dbOverrides = await db + .FeatureFlagOverrides.AsNoTracking() + .Where(o => flagNames.Contains(o.FlagName)) + .ToListAsync(ct); + + var overridesByFlag = dbOverrides + .GroupBy(o => o.FlagName) + .ToDictionary(g => g.Key, g => g.ToList()); + + var allData = new Dictionary( + flagNames.Count, + StringComparer.Ordinal + ); + foreach (var def in definitions) + { + var isEnabled = dbFlags.TryGetValue(def.Name, out var entity) + ? entity.IsEnabled + : def.DefaultEnabled; + + var flagOverrides = overridesByFlag.GetValueOrDefault(def.Name, []); + var data = BuildFlagData(isEnabled, flagOverrides); + + allData[def.Name] = data; + await cache.SetAsync( + $"ff:data:{def.Name}", + data, + CacheEntryOptions.Expires(CacheDuration), + ct + ); + } - cache.Set(AllFlagDataCacheKey, allData, CacheDuration); - return allData; + return allData; + }, + CacheEntryOptions.Expires(CacheDuration) + ); + return result ?? []; } private async Task GetFlagDataAsync(string flagName) { var cacheKey = $"ff:data:{flagName}"; - if (cache.TryGetValue(cacheKey, out FlagData? cached) && cached is not null) - { - return cached; - } - - var flag = await db - .FeatureFlags.AsNoTracking() - .FirstOrDefaultAsync(f => f.Name == flagName); + var result = await cache.GetOrCreateAsync( + cacheKey, + async ct => + { + var flag = await db + .FeatureFlags.AsNoTracking() + .FirstOrDefaultAsync(f => f.Name == flagName, ct); - var isEnabled = - flag?.IsEnabled ?? registry.GetDefinition(flagName)?.DefaultEnabled ?? false; + var isEnabled = + flag?.IsEnabled ?? registry.GetDefinition(flagName)?.DefaultEnabled ?? false; - var overrides = await db - .FeatureFlagOverrides.AsNoTracking() - .Where(o => o.FlagName == flagName) - .ToListAsync(); + var overrides = await db + .FeatureFlagOverrides.AsNoTracking() + .Where(o => o.FlagName == flagName) + .ToListAsync(ct); - var data = BuildFlagData(isEnabled, overrides); - cache.Set(cacheKey, data, CacheDuration); - return data; + return BuildFlagData(isEnabled, overrides); + }, + CacheEntryOptions.Expires(CacheDuration) + ); + return result ?? BuildFlagData(false, []); } private static FlagData BuildFlagData(bool isEnabled, List overrides) @@ -338,10 +346,10 @@ private static FeatureFlag ToDto(FeatureFlagDefinition? def, FeatureFlagEntity? }; } - private void InvalidateCache(string flagName) + private async ValueTask InvalidateCacheAsync(string flagName) { - cache.Remove($"ff:data:{flagName}"); - cache.Remove(AllFlagDataCacheKey); + await cache.RemoveAsync($"ff:data:{flagName}"); + await cache.RemoveAsync(AllFlagDataCacheKey); } [LoggerMessage( diff --git a/modules/FeatureFlags/src/SimpleModule.FeatureFlags/FeatureFlagsModule.cs b/modules/FeatureFlags/src/SimpleModule.FeatureFlags/FeatureFlagsModule.cs index ad86fa63..3b10fd4e 100644 --- a/modules/FeatureFlags/src/SimpleModule.FeatureFlags/FeatureFlagsModule.cs +++ b/modules/FeatureFlags/src/SimpleModule.FeatureFlags/FeatureFlagsModule.cs @@ -17,7 +17,6 @@ public class FeatureFlagsModule : IModule { public void ConfigureServices(IServiceCollection services, IConfiguration configuration) { - services.AddMemoryCache(); services.AddModuleDbContext( configuration, FeatureFlagsConstants.ModuleName diff --git a/modules/FeatureFlags/tests/SimpleModule.FeatureFlags.Tests/Unit/FeatureFlagServiceTests.cs b/modules/FeatureFlags/tests/SimpleModule.FeatureFlags.Tests/Unit/FeatureFlagServiceTests.cs index 4e9d0b11..42c87f14 100644 --- a/modules/FeatureFlags/tests/SimpleModule.FeatureFlags.Tests/Unit/FeatureFlagServiceTests.cs +++ b/modules/FeatureFlags/tests/SimpleModule.FeatureFlags.Tests/Unit/FeatureFlagServiceTests.cs @@ -4,6 +4,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; +using SimpleModule.Core.Caching; using SimpleModule.Core.FeatureFlags; using SimpleModule.Database; using SimpleModule.FeatureFlags; @@ -18,7 +19,9 @@ public sealed class FeatureFlagServiceTests : IDisposable private readonly FeatureFlagService _sut; private readonly IFeatureFlagRegistry _registry; private readonly MemoryCache _cache; + private readonly MemoryCacheStore _cacheStore; private readonly List _freshCaches = []; + private readonly List _freshCacheStores = []; public FeatureFlagServiceTests() { @@ -51,10 +54,11 @@ public FeatureFlagServiceTests() _registry = builder.Build(); _cache = new MemoryCache(Options.Create(new MemoryCacheOptions())); + _cacheStore = new MemoryCacheStore(_cache); _sut = new FeatureFlagService( _db, _registry, - _cache, + _cacheStore, NullLogger.Instance, new ServiceCollection().BuildServiceProvider() ); @@ -62,11 +66,17 @@ public FeatureFlagServiceTests() public void Dispose() { + foreach (var s in _freshCacheStores) + { + s.Dispose(); + } + foreach (var c in _freshCaches) { c.Dispose(); } + _cacheStore.Dispose(); _cache.Dispose(); _db.Dispose(); } @@ -76,10 +86,12 @@ private FeatureFlagService CreateFreshService() // Create a new cache to bypass cached results var freshCache = new MemoryCache(Options.Create(new MemoryCacheOptions())); _freshCaches.Add(freshCache); + var freshStore = new MemoryCacheStore(freshCache); + _freshCacheStores.Add(freshStore); return new FeatureFlagService( _db, _registry, - freshCache, + freshStore, NullLogger.Instance, new ServiceCollection().BuildServiceProvider() ); diff --git a/modules/Localization/src/SimpleModule.Localization/LocalizationModule.cs b/modules/Localization/src/SimpleModule.Localization/LocalizationModule.cs index 506d236e..d95c8c82 100644 --- a/modules/Localization/src/SimpleModule.Localization/LocalizationModule.cs +++ b/modules/Localization/src/SimpleModule.Localization/LocalizationModule.cs @@ -13,7 +13,6 @@ public sealed class LocalizationModule : IModule { public void ConfigureServices(IServiceCollection services, IConfiguration configuration) { - services.AddMemoryCache(); services.AddSingleton(); services.AddLocalization(); services.AddSingleton(); diff --git a/modules/Localization/src/SimpleModule.Localization/Middleware/LocaleResolutionMiddleware.cs b/modules/Localization/src/SimpleModule.Localization/Middleware/LocaleResolutionMiddleware.cs index c443932b..fd1d0d54 100644 --- a/modules/Localization/src/SimpleModule.Localization/Middleware/LocaleResolutionMiddleware.cs +++ b/modules/Localization/src/SimpleModule.Localization/Middleware/LocaleResolutionMiddleware.cs @@ -1,9 +1,9 @@ using System.Globalization; using System.Security.Claims; using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using SimpleModule.Core.Caching; using SimpleModule.Core.Inertia; using SimpleModule.Core.Settings; using SimpleModule.Localization.Contracts; @@ -16,11 +16,14 @@ public sealed class LocaleResolutionMiddleware( RequestDelegate next, IConfiguration configuration, TranslationLoader loader, - IMemoryCache cache + ICacheStore cache ) { - private static readonly TimeSpan UserLocaleCacheDuration = TimeSpan.FromMinutes(5); - private static readonly TimeSpan AcceptLanguageCacheDuration = TimeSpan.FromMinutes(30); + private static readonly CacheEntryOptions UserLocaleCacheOptions = CacheEntryOptions.Expires( + TimeSpan.FromMinutes(5) + ); + private static readonly CacheEntryOptions AcceptLanguageCacheOptions = + CacheEntryOptions.Expires(TimeSpan.FromMinutes(30)); public async Task InvokeAsync(HttpContext context) { @@ -62,9 +65,10 @@ private async Task ResolveLocaleAsync(HttpContext context) // This avoids cross-browser cache pollution where one browser's Accept-Language // leaks to another browser for the same user. var cacheKey = UserLocaleKey(userId); - if (cache.TryGetValue(cacheKey, out string? cachedLocale) && cachedLocale is not null) + var cachedHit = await cache.TryGetAsync(cacheKey); + if (cachedHit.Hit && !string.IsNullOrEmpty(cachedHit.Value)) { - return cachedLocale; + return cachedHit.Value; } var settings = context.RequestServices.GetService(); @@ -77,17 +81,17 @@ private async Task ResolveLocaleAsync(HttpContext context) ); if (!string.IsNullOrEmpty(userLocale)) { - cache.Set(cacheKey, userLocale, UserLocaleCacheDuration); + await cache.SetAsync(cacheKey, userLocale, UserLocaleCacheOptions); return userLocale; } } } // No explicit user setting — resolve from Accept-Language header or default - return ResolveFromAcceptLanguage(context); + return await ResolveFromAcceptLanguageAsync(context); } - private string ResolveFromAcceptLanguage(HttpContext context) + private async Task ResolveFromAcceptLanguageAsync(HttpContext context) { var rawHeader = context.Request.Headers.AcceptLanguage.ToString(); if (string.IsNullOrEmpty(rawHeader)) @@ -96,9 +100,10 @@ private string ResolveFromAcceptLanguage(HttpContext context) } var cacheKey = AcceptLanguageKey(rawHeader); - if (cache.TryGetValue(cacheKey, out string? cached) && cached is not null) + var cachedHit = await cache.TryGetAsync(cacheKey); + if (cachedHit.Hit && !string.IsNullOrEmpty(cachedHit.Value)) { - return cached; + return cachedHit.Value; } var acceptLanguageHeaders = context.Request.GetTypedHeaders().AcceptLanguage; @@ -111,14 +116,14 @@ private string ResolveFromAcceptLanguage(HttpContext context) if (supportedLocales.Contains(tag)) { - cache.Set(cacheKey, tag, AcceptLanguageCacheDuration); + await cache.SetAsync(cacheKey, tag, AcceptLanguageCacheOptions); return tag; } var twoLetter = tag.Split('-')[0]; if (supportedLocales.Contains(twoLetter)) { - cache.Set(cacheKey, twoLetter, AcceptLanguageCacheDuration); + await cache.SetAsync(cacheKey, twoLetter, AcceptLanguageCacheOptions); return twoLetter; } } diff --git a/modules/Localization/tests/SimpleModule.Localization.Tests/Unit/LocaleResolutionMiddlewareTests.cs b/modules/Localization/tests/SimpleModule.Localization.Tests/Unit/LocaleResolutionMiddlewareTests.cs index f2118b77..5f3eeb5b 100644 --- a/modules/Localization/tests/SimpleModule.Localization.Tests/Unit/LocaleResolutionMiddlewareTests.cs +++ b/modules/Localization/tests/SimpleModule.Localization.Tests/Unit/LocaleResolutionMiddlewareTests.cs @@ -5,6 +5,7 @@ using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using SimpleModule.Core.Caching; using SimpleModule.Core.Inertia; using SimpleModule.Core.Settings; using SimpleModule.Localization.Middleware; @@ -17,6 +18,7 @@ public sealed class LocaleResolutionMiddlewareTests : IDisposable { private readonly TranslationLoader _loader; private readonly MemoryCache _cache = new(new MemoryCacheOptions()); + private readonly MemoryCacheStore _cacheStore; public LocaleResolutionMiddlewareTests() { @@ -29,6 +31,7 @@ public LocaleResolutionMiddlewareTests() "es", new Dictionary { ["common.save"] = "Guardar" } ); + _cacheStore = new MemoryCacheStore(_cache); } [Fact] @@ -41,7 +44,7 @@ public async Task Invoke_AuthenticatedUserWithLanguageSetting_UsesUserLocale() var middleware = CreateMiddleware( CaptureLocale(v => capturedLocale = v), CreateConfiguration(null), - _cache + _cacheStore ); await middleware.InvokeAsync(context); @@ -64,7 +67,7 @@ public async Task Invoke_AnonymousWithAcceptLanguageHeader_UsesHeaderLocale() var middleware = CreateMiddleware( CaptureLocale(v => capturedLocale = v), CreateConfiguration(null), - _cache + _cacheStore ); await middleware.InvokeAsync(context); @@ -82,7 +85,7 @@ public async Task Invoke_NoHeaderNoSetting_UsesConfigDefault() var middleware = CreateMiddleware( CaptureLocale(v => capturedLocale = v), CreateConfiguration("es"), - _cache + _cacheStore ); await middleware.InvokeAsync(context); @@ -100,7 +103,7 @@ public async Task Invoke_NoHeaderNoSettingNoConfig_FallsBackToEn() var middleware = CreateMiddleware( CaptureLocale(v => capturedLocale = v), CreateConfiguration(null), - _cache + _cacheStore ); await middleware.InvokeAsync(context); @@ -114,11 +117,12 @@ public async Task Invoke_CachesExplicitUserSetting() var callCount = 0; var settings = new FakeSettingsContracts("es", onGet: () => callCount++); using var localCache = new MemoryCache(new MemoryCacheOptions()); + using var localCacheStore = new MemoryCacheStore(localCache); var middleware = CreateMiddleware( _ => Task.CompletedTask, CreateConfiguration(null), - localCache + localCacheStore ); var context1 = CreateHttpContext(settings, userId: "user-1"); @@ -140,11 +144,12 @@ public async Task Invoke_DoesNotCacheFallbackPerUser() var callCount = 0; var settings = new FakeSettingsContracts(null, onGet: () => callCount++); using var localCache = new MemoryCache(new MemoryCacheOptions()); + using var localCacheStore = new MemoryCacheStore(localCache); var middleware = CreateMiddleware( _ => Task.CompletedTask, CreateConfiguration(null), - localCache + localCacheStore ); var context1 = CreateHttpContext(settings, userId: "user-2"); @@ -160,7 +165,7 @@ public async Task Invoke_DoesNotCacheFallbackPerUser() private LocaleResolutionMiddleware CreateMiddleware( RequestDelegate next, IConfiguration config, - IMemoryCache cache + ICacheStore cache ) { return new LocaleResolutionMiddleware(next, config, _loader, cache); @@ -253,5 +258,9 @@ public Task> GetSettingsAsync(SettingsFilter? filter = null } } - public void Dispose() => _cache.Dispose(); + public void Dispose() + { + _cacheStore.Dispose(); + _cache.Dispose(); + } } diff --git a/modules/Marketplace/src/SimpleModule.Marketplace/InstalledPackageDetector.cs b/modules/Marketplace/src/SimpleModule.Marketplace/InstalledPackageDetector.cs index ce2a5abf..2d191883 100644 --- a/modules/Marketplace/src/SimpleModule.Marketplace/InstalledPackageDetector.cs +++ b/modules/Marketplace/src/SimpleModule.Marketplace/InstalledPackageDetector.cs @@ -1,31 +1,30 @@ using System.Xml; using System.Xml.Linq; using Microsoft.AspNetCore.Hosting; -using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging; +using SimpleModule.Core.Caching; namespace SimpleModule.Marketplace; public partial class InstalledPackageDetector( IWebHostEnvironment environment, - IMemoryCache cache, + ICacheStore cache, ILogger logger ) { private const string CacheKey = "Marketplace:InstalledPackages"; + private static readonly CacheEntryOptions CacheOptions = CacheEntryOptions.Expires( + TimeSpan.FromMinutes(1) + ); - public Task> GetInstalledPackageIdsAsync() + public async Task> GetInstalledPackageIdsAsync() { - return Task.FromResult( - cache.GetOrCreate( - CacheKey, - entry => - { - entry.AbsoluteExpirationRelativeToNow = TimeSpan.FromMinutes(1); - return ReadInstalledPackages(); - } - ) ?? [] + var result = await cache.GetOrCreateAsync>( + CacheKey, + _ => new ValueTask?>(ReadInstalledPackages()), + CacheOptions ); + return result ?? []; } private HashSet ReadInstalledPackages() diff --git a/modules/Marketplace/src/SimpleModule.Marketplace/MarketplaceModule.cs b/modules/Marketplace/src/SimpleModule.Marketplace/MarketplaceModule.cs index e87b9acf..dbceeee0 100644 --- a/modules/Marketplace/src/SimpleModule.Marketplace/MarketplaceModule.cs +++ b/modules/Marketplace/src/SimpleModule.Marketplace/MarketplaceModule.cs @@ -26,7 +26,6 @@ public void ConfigureServices(IServiceCollection services, IConfiguration config } ); - services.AddMemoryCache(); services.AddSingleton(); } diff --git a/modules/Marketplace/src/SimpleModule.Marketplace/NuGetMarketplaceService.cs b/modules/Marketplace/src/SimpleModule.Marketplace/NuGetMarketplaceService.cs index dbda4f35..bcccfe60 100644 --- a/modules/Marketplace/src/SimpleModule.Marketplace/NuGetMarketplaceService.cs +++ b/modules/Marketplace/src/SimpleModule.Marketplace/NuGetMarketplaceService.cs @@ -1,8 +1,8 @@ using System.Diagnostics.CodeAnalysis; using System.Net.Http.Json; using System.Text.Json.Serialization; -using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Options; +using SimpleModule.Core.Caching; using SimpleModule.Marketplace.Contracts; namespace SimpleModule.Marketplace; @@ -11,7 +11,7 @@ public class NuGetMarketplaceService( IHttpClientFactory httpClientFactory, IOptions options, InstalledPackageDetector installedPackageDetector, - IMemoryCache cache + ICacheStore cache ) : IMarketplaceContracts { public async Task SearchPackagesAsync(MarketplaceSearchRequest request) @@ -20,13 +20,10 @@ public async Task SearchPackagesAsync(MarketplaceSearch var cached = await cache.GetOrCreateAsync( cacheKey, - async entry => - { - entry.AbsoluteExpirationRelativeToNow = TimeSpan.FromMinutes( - options.Value.SearchCacheDurationMinutes - ); - return await FetchAllPackagesAsync(request.Query); - } + async _ => await FetchAllPackagesAsync(request.Query), + CacheEntryOptions.Expires( + TimeSpan.FromMinutes(options.Value.SearchCacheDurationMinutes) + ) ); var result = cached ?? new MarketplaceSearchResult(); @@ -60,13 +57,10 @@ .. packages.OrderByDescending(p => p.TotalDownloads), return await cache.GetOrCreateAsync( cacheKey, - async entry => - { - entry.AbsoluteExpirationRelativeToNow = TimeSpan.FromMinutes( - options.Value.DetailCacheDurationMinutes - ); - return await FetchPackageDetailsAsync(packageId); - } + async _ => await FetchPackageDetailsAsync(packageId), + CacheEntryOptions.Expires( + TimeSpan.FromMinutes(options.Value.DetailCacheDurationMinutes) + ) ); } diff --git a/modules/Permissions/src/SimpleModule.Permissions/PermissionClaimsTransformation.cs b/modules/Permissions/src/SimpleModule.Permissions/PermissionClaimsTransformation.cs index e7504bb1..f7fed4ed 100644 --- a/modules/Permissions/src/SimpleModule.Permissions/PermissionClaimsTransformation.cs +++ b/modules/Permissions/src/SimpleModule.Permissions/PermissionClaimsTransformation.cs @@ -1,6 +1,6 @@ using System.Security.Claims; using Microsoft.AspNetCore.Authentication; -using Microsoft.Extensions.Caching.Memory; +using SimpleModule.Core.Caching; using SimpleModule.Core.Extensions; using SimpleModule.Permissions.Contracts; using SimpleModule.Users.Contracts; @@ -10,10 +10,12 @@ namespace SimpleModule.Permissions; public sealed class PermissionClaimsTransformation( IPermissionContracts permissionContracts, IUserContracts userContracts, - IMemoryCache cache + ICacheStore cache ) : IClaimsTransformation { - private static readonly TimeSpan CacheDuration = TimeSpan.FromMinutes(5); + private static readonly CacheEntryOptions CacheOptions = CacheEntryOptions.Expires( + TimeSpan.FromMinutes(5) + ); public async Task TransformAsync(ClaimsPrincipal principal) { @@ -33,20 +35,22 @@ public async Task TransformAsync(ClaimsPrincipal principal) var rolesKey = string.Join(',', roles.Order()); var cacheKey = $"permissions:{userId}:{rolesKey}"; - if (!cache.TryGetValue(cacheKey, out IReadOnlySet? permissions)) - { - var roleIdMap = - roles.Count > 0 - ? await userContracts.GetRoleIdsByNamesAsync(roles) - : new Dictionary(); - - permissions = await permissionContracts.GetAllPermissionsForUserAsync( - UserId.From(userId), - roleIdMap.Values.Select(id => RoleId.From(id)) - ); + var permissions = await cache.GetOrCreateAsync>( + cacheKey, + async _ => + { + var roleIdMap = + roles.Count > 0 + ? await userContracts.GetRoleIdsByNamesAsync(roles) + : new Dictionary(); - cache.Set(cacheKey, permissions, CacheDuration); - } + return await permissionContracts.GetAllPermissionsForUserAsync( + UserId.From(userId), + roleIdMap.Values.Select(id => RoleId.From(id)) + ); + }, + CacheOptions + ); var identity = new ClaimsIdentity(); foreach (var permission in permissions!) diff --git a/modules/Settings/src/SimpleModule.Settings/SettingsModule.cs b/modules/Settings/src/SimpleModule.Settings/SettingsModule.cs index e8401c2c..949a19c7 100644 --- a/modules/Settings/src/SimpleModule.Settings/SettingsModule.cs +++ b/modules/Settings/src/SimpleModule.Settings/SettingsModule.cs @@ -18,7 +18,6 @@ public class SettingsModule : IModule { public void ConfigureServices(IServiceCollection services, IConfiguration configuration) { - services.AddMemoryCache(); services.AddModuleDbContext(configuration, SettingsConstants.ModuleName); services.AddScoped(); services.AddScoped(sp => sp.GetRequiredService()); diff --git a/modules/Tenants/src/SimpleModule.Tenants/Resolvers/HostNameTenantResolver.cs b/modules/Tenants/src/SimpleModule.Tenants/Resolvers/HostNameTenantResolver.cs index 861d0f92..ab82e928 100644 --- a/modules/Tenants/src/SimpleModule.Tenants/Resolvers/HostNameTenantResolver.cs +++ b/modules/Tenants/src/SimpleModule.Tenants/Resolvers/HostNameTenantResolver.cs @@ -1,15 +1,14 @@ using Microsoft.AspNetCore.Http; using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.Caching.Memory; +using SimpleModule.Core.Caching; namespace SimpleModule.Tenants.Resolvers; -public sealed class HostNameTenantResolver(TenantsDbContext db, IMemoryCache cache) +public sealed class HostNameTenantResolver(TenantsDbContext db, ICacheStore cache) { - private static readonly MemoryCacheEntryOptions CacheOptions = new() - { - AbsoluteExpirationRelativeToNow = TimeSpan.FromMinutes(5), - }; + private static readonly CacheEntryOptions CacheOptions = CacheEntryOptions.Expires( + TimeSpan.FromMinutes(5) + ); public async Task ResolveAsync(HttpContext context) { @@ -22,18 +21,17 @@ public sealed class HostNameTenantResolver(TenantsDbContext db, IMemoryCache cac var cacheKey = $"tenant:host:{host}"; return await cache.GetOrCreateAsync( cacheKey, - async entry => + async ct => { - entry.SetOptions(CacheOptions); - var tenantHost = await db .TenantHosts.AsNoTracking() .Where(h => h.HostName == host && h.IsActive) .Select(h => (int?)h.TenantId.Value) - .FirstOrDefaultAsync(); + .FirstOrDefaultAsync(ct); return tenantHost?.ToString(System.Globalization.CultureInfo.InvariantCulture); - } + }, + CacheOptions ); } } diff --git a/modules/Tenants/src/SimpleModule.Tenants/TenantsModule.cs b/modules/Tenants/src/SimpleModule.Tenants/TenantsModule.cs index 18076a10..ab40eee6 100644 --- a/modules/Tenants/src/SimpleModule.Tenants/TenantsModule.cs +++ b/modules/Tenants/src/SimpleModule.Tenants/TenantsModule.cs @@ -17,7 +17,6 @@ public class TenantsModule : IModule { public void ConfigureServices(IServiceCollection services, IConfiguration configuration) { - services.AddMemoryCache(); services.AddModuleDbContext(configuration, TenantsConstants.ModuleName); services.AddScoped(); services.AddScoped(); From 84e5592227b0723020bc45677c1dc2b1f1c06d0f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 9 Apr 2026 18:38:53 +0000 Subject: [PATCH 3/3] fix(caching): reclaim idle locks, wire workers, tighten call sites MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review cleanup from /simplify: MemoryCacheStore * _keyLocks no longer leaks forever. GetOrCreateAsync reclaims the semaphore in its finally block when CurrentCount == 1 (no waiters), RemoveAsync / RemoveByPrefixAsync do the same via a shared TryReclaimIdleLock helper, and the post-eviction callback also drops idle locks. A GetOrCreateAsync caller still holding the gate is never disposed out from under it — the reclaim only runs when the semaphore is observably idle. * Added two regression tests: one asserts no lock entries survive uncontended GetOrCreateAsync across many keys, one asserts RemoveAsync during an in-flight factory neither crashes nor strands the lock after the factory completes. SimpleModuleWorkerExtensions * Worker hosts now call AddSimpleModuleCaching alongside the event bus and EF interceptor registrations. Without this, any worker module that resolves ICacheStore (Permissions, Settings, FeatureFlags, etc.) would fail at runtime. PermissionClaimsTransformation * Dropped the `permissions!` null-forgiving. GetOrCreateAsync's nullable return is coalesced to an empty set so the foreach can never NRE if the cache ever yields null. FeatureFlagService * Consolidated the three inlined "ff:data:{name}" string literals behind a FlagDataCacheKey helper plus a FlagDataKeyPrefix constant. Changing the format is now a one-liner. SettingsService * BuildCacheKey now uses CacheKey.Compose — the null-userId branch was the exact case the helper was introduced to collapse. --- .../Caching/MemoryCacheStore.cs | 48 +++++++++++++++-- .../SimpleModuleWorkerExtensions.cs | 2 + .../FeatureFlagService.cs | 10 ++-- .../PermissionClaimsTransformation.cs | 33 ++++++------ .../SimpleModule.Settings/SettingsService.cs | 2 +- .../Caching/MemoryCacheStoreTests.cs | 52 +++++++++++++++++++ 6 files changed, 121 insertions(+), 26 deletions(-) diff --git a/framework/SimpleModule.Core/Caching/MemoryCacheStore.cs b/framework/SimpleModule.Core/Caching/MemoryCacheStore.cs index 0d78ff1e..e32b6956 100644 --- a/framework/SimpleModule.Core/Caching/MemoryCacheStore.cs +++ b/framework/SimpleModule.Core/Caching/MemoryCacheStore.cs @@ -84,6 +84,14 @@ public ValueTask SetAsync( finally { gate.Release(); + // Reclaim the lock entry once no other caller is waiting on it. + // CurrentCount == 1 means the semaphore is fully released and idle; any + // concurrent waiter would hold it below 1. This bounds the dictionary to + // keys currently being populated rather than every key ever populated. + if (gate.CurrentCount == 1 && _keyLocks.TryRemove(key, out var removed)) + { + removed.Dispose(); + } } } @@ -94,6 +102,7 @@ public ValueTask RemoveAsync(string key, CancellationToken cancellationToken = d _cache.Remove(key); _trackedKeys.TryRemove(key, out _); + TryReclaimIdleLock(key); return ValueTask.CompletedTask; } @@ -111,12 +120,31 @@ public ValueTask RemoveByPrefixAsync( { _cache.Remove(key); _trackedKeys.TryRemove(key, out _); + TryReclaimIdleLock(key); } } return ValueTask.CompletedTask; } + /// + /// Removes and disposes a per-key semaphore only when it is observably idle + /// (no waiters, fully released). If a caller + /// is still holding the gate, the lock is left in place and that caller's + /// own finally block will reclaim it after release. + /// + private void TryReclaimIdleLock(string key) + { + if ( + _keyLocks.TryGetValue(key, out var gate) + && gate.CurrentCount == 1 + && _keyLocks.TryRemove(key, out var removed) + ) + { + removed.Dispose(); + } + } + private void SetCore(string key, T? value, CacheEntryOptions? options) { using var entry = _cache.CreateEntry(key); @@ -145,16 +173,26 @@ private void SetCore(string key, T? value, CacheEntryOptions? options) } } - // Track the key so RemoveByPrefixAsync can find it. The eviction callback below - // also removes from the tracking set when the entry naturally expires or is evicted - // by memory pressure, so the set stays bounded. + // Track the key so RemoveByPrefixAsync can find it. The eviction callback + // releases both tracking entries and any idle per-key lock when the entry + // naturally expires or is evicted by memory pressure, so both sets stay bounded. _trackedKeys[key] = 0; entry.RegisterPostEvictionCallback( static (evictedKey, _, _, state) => { - if (state is MemoryCacheStore self && evictedKey is string s) + if (state is not MemoryCacheStore self || evictedKey is not string s) + { + return; + } + + self._trackedKeys.TryRemove(s, out _); + if ( + self._keyLocks.TryGetValue(s, out var gate) + && gate.CurrentCount == 1 + && self._keyLocks.TryRemove(s, out var removed) + ) { - self._trackedKeys.TryRemove(s, out _); + removed.Dispose(); } }, this diff --git a/framework/SimpleModule.Hosting/SimpleModuleWorkerExtensions.cs b/framework/SimpleModule.Hosting/SimpleModuleWorkerExtensions.cs index f2f9b7d2..ccfe8908 100644 --- a/framework/SimpleModule.Hosting/SimpleModuleWorkerExtensions.cs +++ b/framework/SimpleModule.Hosting/SimpleModuleWorkerExtensions.cs @@ -3,6 +3,7 @@ using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; +using SimpleModule.Core.Caching; using SimpleModule.Core.Events; using SimpleModule.Database.Interceptors; @@ -30,6 +31,7 @@ public static HostApplicationBuilder AddSimpleModuleWorker(this HostApplicationB builder.Configuration["BackgroundJobs:WorkerMode"] = "Consumer"; // Core infrastructure that the worker needs: + builder.Services.AddSimpleModuleCaching(); builder.Services.AddSingleton(); builder.Services.AddHostedService(); builder.Services.AddScoped(); diff --git a/modules/FeatureFlags/src/SimpleModule.FeatureFlags/FeatureFlagService.cs b/modules/FeatureFlags/src/SimpleModule.FeatureFlags/FeatureFlagService.cs index 7933a3ba..8a6ca460 100644 --- a/modules/FeatureFlags/src/SimpleModule.FeatureFlags/FeatureFlagService.cs +++ b/modules/FeatureFlags/src/SimpleModule.FeatureFlags/FeatureFlagService.cs @@ -22,6 +22,9 @@ IServiceProvider serviceProvider ); private static readonly TimeSpan CacheDuration = TimeSpan.FromSeconds(30); private const string AllFlagDataCacheKey = "ff:all-data"; + private const string FlagDataKeyPrefix = "ff:data:"; + + private static string FlagDataCacheKey(string flagName) => FlagDataKeyPrefix + flagName; private sealed record FlagData( bool IsEnabled, @@ -238,7 +241,7 @@ private async Task> GetAllFlagDataAsync() allData[def.Name] = data; await cache.SetAsync( - $"ff:data:{def.Name}", + FlagDataCacheKey(def.Name), data, CacheEntryOptions.Expires(CacheDuration), ct @@ -254,9 +257,8 @@ await cache.SetAsync( private async Task GetFlagDataAsync(string flagName) { - var cacheKey = $"ff:data:{flagName}"; var result = await cache.GetOrCreateAsync( - cacheKey, + FlagDataCacheKey(flagName), async ct => { var flag = await db @@ -348,7 +350,7 @@ private static FeatureFlag ToDto(FeatureFlagDefinition? def, FeatureFlagEntity? private async ValueTask InvalidateCacheAsync(string flagName) { - await cache.RemoveAsync($"ff:data:{flagName}"); + await cache.RemoveAsync(FlagDataCacheKey(flagName)); await cache.RemoveAsync(AllFlagDataCacheKey); } diff --git a/modules/Permissions/src/SimpleModule.Permissions/PermissionClaimsTransformation.cs b/modules/Permissions/src/SimpleModule.Permissions/PermissionClaimsTransformation.cs index f7fed4ed..511bac71 100644 --- a/modules/Permissions/src/SimpleModule.Permissions/PermissionClaimsTransformation.cs +++ b/modules/Permissions/src/SimpleModule.Permissions/PermissionClaimsTransformation.cs @@ -35,25 +35,26 @@ public async Task TransformAsync(ClaimsPrincipal principal) var rolesKey = string.Join(',', roles.Order()); var cacheKey = $"permissions:{userId}:{rolesKey}"; - var permissions = await cache.GetOrCreateAsync>( - cacheKey, - async _ => - { - var roleIdMap = - roles.Count > 0 - ? await userContracts.GetRoleIdsByNamesAsync(roles) - : new Dictionary(); + var permissions = + await cache.GetOrCreateAsync>( + cacheKey, + async _ => + { + var roleIdMap = + roles.Count > 0 + ? await userContracts.GetRoleIdsByNamesAsync(roles) + : new Dictionary(); - return await permissionContracts.GetAllPermissionsForUserAsync( - UserId.From(userId), - roleIdMap.Values.Select(id => RoleId.From(id)) - ); - }, - CacheOptions - ); + return await permissionContracts.GetAllPermissionsForUserAsync( + UserId.From(userId), + roleIdMap.Values.Select(id => RoleId.From(id)) + ); + }, + CacheOptions + ) ?? new HashSet(); var identity = new ClaimsIdentity(); - foreach (var permission in permissions!) + foreach (var permission in permissions) { identity.AddClaim(new Claim("permission", permission)); } diff --git a/modules/Settings/src/SimpleModule.Settings/SettingsService.cs b/modules/Settings/src/SimpleModule.Settings/SettingsService.cs index aea5c27f..3b656fdb 100644 --- a/modules/Settings/src/SimpleModule.Settings/SettingsService.cs +++ b/modules/Settings/src/SimpleModule.Settings/SettingsService.cs @@ -179,5 +179,5 @@ public async Task> GetSettingsAsync(SettingsFilter? filter private partial void LogDeserializationError(string key, string type, string error); private static string BuildCacheKey(string key, SettingScope scope, string? userId) => - userId is not null ? $"setting:{scope}:{userId}:{key}" : $"setting:{scope}:{key}"; + CacheKey.Compose("setting", scope.ToString(), userId, key); } diff --git a/tests/SimpleModule.Core.Tests/Caching/MemoryCacheStoreTests.cs b/tests/SimpleModule.Core.Tests/Caching/MemoryCacheStoreTests.cs index f8a04eb2..f88b7440 100644 --- a/tests/SimpleModule.Core.Tests/Caching/MemoryCacheStoreTests.cs +++ b/tests/SimpleModule.Core.Tests/Caching/MemoryCacheStoreTests.cs @@ -1,3 +1,5 @@ +using System.Collections.Concurrent; +using System.Reflection; using FluentAssertions; using Microsoft.Extensions.Caching.Memory; using SimpleModule.Core.Caching; @@ -230,6 +232,56 @@ public async Task WithPrefix_RemoveByPrefix_ScopesPrefix() (await _store.TryGetAsync("user:1")).Hit.Should().BeTrue(); } + [Fact] + public async Task GetOrCreateAsync_DoesNotLeakPerKeyLocks() + { + // Populate many distinct keys. After each uncontended call the per-key + // semaphore should be released; the lock dictionary must not grow unbounded. + for (var i = 0; i < 50; i++) + { + await _store.GetOrCreateAsync($"leak:{i}", _ => new ValueTask(i)); + } + + GetKeyLocks(_store).Should().BeEmpty(); + } + + [Fact] + public async Task RemoveAsync_DuringInFlightFactory_DoesNotCrash() + { + // RemoveAsync must not dispose a semaphore that a concurrent GetOrCreateAsync + // caller is still holding. The in-flight caller's own finally block reclaims + // the lock after it releases the gate. + var factoryGate = new TaskCompletionSource(); + var held = _store + .GetOrCreateAsync( + "contended", + async _ => + { + await factoryGate.Task; + return "value"; + } + ) + .AsTask(); + + await Task.Yield(); + await _store.RemoveAsync("contended"); + + factoryGate.SetResult(); + var result = await held; + + result.Should().Be("value"); + GetKeyLocks(_store).Should().NotContainKey("contended"); + } + + private static ConcurrentDictionary GetKeyLocks(MemoryCacheStore store) + { + var field = typeof(MemoryCacheStore).GetField( + "_keyLocks", + BindingFlags.NonPublic | BindingFlags.Instance + )!; + return (ConcurrentDictionary)field.GetValue(store)!; + } + [Fact] public void CacheKey_Compose_JoinsPartsAndSkipsEmpty() {