diff --git a/src/BuildingBlocks/Caching/CacheKeys.cs b/src/BuildingBlocks/Caching/CacheKeys.cs index 20e732e4e1..1687704127 100644 --- a/src/BuildingBlocks/Caching/CacheKeys.cs +++ b/src/BuildingBlocks/Caching/CacheKeys.cs @@ -2,8 +2,9 @@ namespace FSH.Framework.Caching; /// /// Cache key conventions and tag constants used across the FullStackHero starter kit. -/// Keys should be tenant-scoped where applicable; tags enable bulk invalidation via -/// . +/// Tenant scoping is applied automatically by — +/// keys returned by these methods are the logical (un-prefixed) key. +/// Tags enable bulk invalidation via HybridCache's tag system. /// public static class CacheKeys { @@ -35,8 +36,15 @@ public static class Tags /// Key for the system-wide default theme. public const string DefaultTheme = "theme:default"; - /// Key for an idempotency replay entry, scoped by tenant. - public static string IdempotencyEntry(string tenantId, string key) => $"idem:t:{tenantId}:{key}"; + /// Logical key for an idempotency replay entry (no tenant prefix — applied by ITenantCacheService). + public static string IdempotencyEntry(string key) => $"idem:{key}"; + + /// + /// Fully-qualified idempotency key including the tenant segment, used when reading + /// directly from + /// to match the key written by (which uses prefix t:{tenantId}:). + /// + public static string IdempotencyEntryFull(string tenantId, string key) => $"t:{tenantId}:idem:{key}"; /// /// Key for the impersonation-grant revocation marker, indexed by JWT id. diff --git a/src/BuildingBlocks/Caching/Caching.csproj b/src/BuildingBlocks/Caching/Caching.csproj index e326c179eb..fd575afa55 100644 --- a/src/BuildingBlocks/Caching/Caching.csproj +++ b/src/BuildingBlocks/Caching/Caching.csproj @@ -1,4 +1,4 @@ - + FSH.Framework.Caching @@ -6,6 +6,12 @@ FullStackHero.Framework.Caching + + + <_Parameter1>Caching.Tests + + + @@ -17,6 +23,7 @@ + diff --git a/src/BuildingBlocks/Caching/Extensions.cs b/src/BuildingBlocks/Caching/Extensions.cs index 1dd28108ad..736621be15 100644 --- a/src/BuildingBlocks/Caching/Extensions.cs +++ b/src/BuildingBlocks/Caching/Extensions.cs @@ -1,3 +1,4 @@ +using Finbuckle.MultiTenant.Abstractions; using Microsoft.AspNetCore.DataProtection; using Microsoft.Extensions.Caching.Hybrid; using Microsoft.Extensions.Configuration; @@ -85,6 +86,21 @@ public static IServiceCollection AddHeroCaching(this IServiceCollection services // factory that builds the inner via the original descriptor and returns our wrapper. DecorateHybridCache(services); + // Register the tenant-scoped cache service. Scoped lifetime because the tenant context + // (and therefore the key prefix) is per-request. Module code must inject + // ITenantCacheService — injecting HybridCache directly is prohibited in module assemblies + // and enforced by architecture tests. + services.AddScoped(sp => + new TenantHybridCache( + sp.GetRequiredService(), + sp.GetRequiredService())); + + // Register the global (cross-tenant) cache service. Singleton lifetime because it + // carries no per-request state. Use this ONLY for data shared across all tenants, + // e.g. system defaults, global lookup tables, shared configuration. + services.AddSingleton(sp => + new GlobalHybridCache(sp.GetRequiredService())); + return services; } diff --git a/src/BuildingBlocks/Caching/GlobalHybridCache.cs b/src/BuildingBlocks/Caching/GlobalHybridCache.cs new file mode 100644 index 0000000000..781aa423a0 --- /dev/null +++ b/src/BuildingBlocks/Caching/GlobalHybridCache.cs @@ -0,0 +1,46 @@ +using Microsoft.Extensions.Caching.Hybrid; + +namespace FSH.Framework.Caching; + +/// +/// implementation that delegates directly to +/// without any tenant scoping. +/// Registered as Singleton — safe because no per-request state is involved. +/// +internal sealed class GlobalHybridCache : IGlobalCacheService +{ + private readonly HybridCache _cache; + + public GlobalHybridCache(HybridCache cache) + { + ArgumentNullException.ThrowIfNull(cache); + _cache = cache; + } + + /// + public ValueTask GetOrCreateAsync( + string key, + TState state, + Func> factory, + HybridCacheEntryOptions? options = null, + IEnumerable? tags = null, + CancellationToken cancellationToken = default) + => _cache.GetOrCreateAsync(key, state, factory, options, tags, cancellationToken); + + /// + public ValueTask SetAsync( + string key, + T value, + HybridCacheEntryOptions? options = null, + IEnumerable? tags = null, + CancellationToken cancellationToken = default) + => _cache.SetAsync(key, value, options, tags, cancellationToken); + + /// + public ValueTask RemoveAsync(string key, CancellationToken cancellationToken = default) + => _cache.RemoveAsync(key, cancellationToken); + + /// + public ValueTask RemoveByTagAsync(string tag, CancellationToken cancellationToken = default) + => _cache.RemoveByTagAsync(tag, cancellationToken); +} diff --git a/src/BuildingBlocks/Caching/IGlobalCacheService.cs b/src/BuildingBlocks/Caching/IGlobalCacheService.cs new file mode 100644 index 0000000000..a7df2671b1 --- /dev/null +++ b/src/BuildingBlocks/Caching/IGlobalCacheService.cs @@ -0,0 +1,38 @@ +using Microsoft.Extensions.Caching.Hybrid; + +namespace FSH.Framework.Caching; + +/// +/// Cross-tenant cache service for entries that are intentionally shared across all tenants. +/// Examples: system defaults, global configuration, shared lookup tables. +/// +/// +/// Use this service ONLY for data that is genuinely identical for every tenant. +/// For any per-tenant data use instead. +/// Registered as Singleton because there is no per-request tenant context involved. +/// +public interface IGlobalCacheService +{ + /// Gets or creates a cross-tenant cache entry. + ValueTask GetOrCreateAsync( + string key, + TState state, + Func> factory, + HybridCacheEntryOptions? options = null, + IEnumerable? tags = null, + CancellationToken cancellationToken = default); + + /// Sets a cross-tenant cache entry. + ValueTask SetAsync( + string key, + T value, + HybridCacheEntryOptions? options = null, + IEnumerable? tags = null, + CancellationToken cancellationToken = default); + + /// Removes a cross-tenant cache entry by key. + ValueTask RemoveAsync(string key, CancellationToken cancellationToken = default); + + /// Removes all cross-tenant entries tagged with the given tag. + ValueTask RemoveByTagAsync(string tag, CancellationToken cancellationToken = default); +} diff --git a/src/BuildingBlocks/Caching/ITenantCacheService.cs b/src/BuildingBlocks/Caching/ITenantCacheService.cs new file mode 100644 index 0000000000..86ccee7a50 --- /dev/null +++ b/src/BuildingBlocks/Caching/ITenantCacheService.cs @@ -0,0 +1,54 @@ +using Microsoft.Extensions.Caching.Hybrid; + +namespace FSH.Framework.Caching; + +/// +/// Tenant-scoped wrapper over . +/// All keys and tags are automatically prefixed with the current tenant identifier, +/// preventing cross-tenant cache collisions without requiring callers to manage +/// the prefix themselves. +/// +/// +/// Register as Scoped because the tenant context (and therefore the prefix) +/// changes per HTTP request. Inject in module +/// code — do NOT inject directly from business modules. +/// +public interface ITenantCacheService +{ + /// + /// Gets an existing cache entry or creates a new one using the provided factory, + /// automatically scoping the key and tags to the current tenant. + /// + ValueTask GetOrCreateAsync( + string key, + TState state, + Func> factory, + HybridCacheEntryOptions? options = null, + IEnumerable? tags = null, + CancellationToken cancellationToken = default); + + /// + /// Sets a cache entry for the given key, scoped to the current tenant. + /// + ValueTask SetAsync( + string key, + T value, + HybridCacheEntryOptions? options = null, + IEnumerable? tags = null, + CancellationToken cancellationToken = default); + + /// + /// Removes a cache entry by key, scoped to the current tenant. + /// + ValueTask RemoveAsync(string key, CancellationToken cancellationToken = default); + + /// + /// Removes all cache entries carrying the given tag, scoped to the current tenant. + /// + ValueTask RemoveByTagAsync(string tag, CancellationToken cancellationToken = default); + + /// + /// Removes all cache entries carrying any of the given tags, scoped to the current tenant. + /// + ValueTask RemoveByTagAsync(IEnumerable tags, CancellationToken cancellationToken = default); +} diff --git a/src/BuildingBlocks/Caching/TenantHybridCache.cs b/src/BuildingBlocks/Caching/TenantHybridCache.cs new file mode 100644 index 0000000000..9898f21a15 --- /dev/null +++ b/src/BuildingBlocks/Caching/TenantHybridCache.cs @@ -0,0 +1,137 @@ +using Finbuckle.MultiTenant; +using Finbuckle.MultiTenant.Abstractions; +using Microsoft.Extensions.Caching.Hybrid; + +namespace FSH.Framework.Caching; + +/// +/// implementation that wraps +/// and automatically scopes every key and tag with the active tenant identifier. +/// +/// +/// This class must be registered as Scoped so that each HTTP request gets +/// a fresh instance bound to the correct tenant context. The underlying +/// remains Singleton and is shared across tenants — +/// only the key/tag prefixing is tenant-specific. +/// +internal sealed class TenantHybridCache : ITenantCacheService +{ + private readonly HybridCache _cache; + private readonly IMultiTenantContextAccessor _tenantAccessor; + + public TenantHybridCache(HybridCache cache, IMultiTenantContextAccessor tenantAccessor) + { + ArgumentNullException.ThrowIfNull(cache); + ArgumentNullException.ThrowIfNull(tenantAccessor); + _cache = cache; + _tenantAccessor = tenantAccessor; + } + + /// + /// Internal factory for test projects (via InternalsVisibleTo). Prefer DI for production code. + /// + internal static ITenantCacheService Create(HybridCache cache, IMultiTenantContextAccessor accessor) + => new TenantHybridCache(cache, accessor); + + // ----------------------------------------------------------------------- + // Key/tag scoping helpers + // ----------------------------------------------------------------------- + + private string GetTenantId() + { + var tenantId = _tenantAccessor.MultiTenantContext?.TenantInfo?.Id; + if (string.IsNullOrEmpty(tenantId)) + { + throw new InvalidOperationException( + "No active tenant context. TenantHybridCache requires a resolved tenant. " + + "Ensure the request passes through the Finbuckle middleware before the cache is accessed."); + } + + return tenantId; + } + + /// Scopes a logical key to the current tenant: t:{tenantId}:{key}. + private string ScopeKey(string key) => $"t:{GetTenantId()}:{key}"; + + /// + /// Returns the full tag set for a cache entry: + /// + /// tenant:{tenantId} — whole-tenant purge tag (used by ). + /// t:{tenantId}:{callerTag} for every caller-supplied tag — scoped so + /// can look up the same + /// prefixed tag and actually find the stored entries. + /// + /// Without the per-tag prefix the SET and REMOVE paths would use different tag strings, + /// making all tag-based invalidation a silent no-op. + /// + private static IEnumerable ScopeTags(string tenantId, IEnumerable? callerTags) + { + // Whole-tenant purge tag — lets callers blow away an entire tenant's cache. + yield return CacheKeys.Tags.Tenant(tenantId); + + if (callerTags is null) yield break; + + // Per-tag scoped form — must match the lookup key built in RemoveByTagAsync. + foreach (var tag in callerTags) + yield return $"t:{tenantId}:{tag}"; + } + + // ----------------------------------------------------------------------- + // ITenantCacheService implementation + // ----------------------------------------------------------------------- + + /// + public ValueTask GetOrCreateAsync( + string key, + TState state, + Func> factory, + HybridCacheEntryOptions? options = null, + IEnumerable? tags = null, + CancellationToken cancellationToken = default) + { + var tenantId = GetTenantId(); + return _cache.GetOrCreateAsync( + ScopeKey(key), + state, + factory, + options, + ScopeTags(tenantId, tags), + cancellationToken); + } + + /// + public ValueTask SetAsync( + string key, + T value, + HybridCacheEntryOptions? options = null, + IEnumerable? tags = null, + CancellationToken cancellationToken = default) + { + var tenantId = GetTenantId(); + return _cache.SetAsync( + ScopeKey(key), + value, + options, + ScopeTags(tenantId, tags), + cancellationToken); + } + + /// + public ValueTask RemoveAsync(string key, CancellationToken cancellationToken = default) + => _cache.RemoveAsync(ScopeKey(key), cancellationToken); + + /// + public ValueTask RemoveByTagAsync(string tag, CancellationToken cancellationToken = default) + { + var tenantId = GetTenantId(); + // Scope the tag so only entries for this tenant are evicted. + return _cache.RemoveByTagAsync($"t:{tenantId}:{tag}", cancellationToken); + } + + /// + public ValueTask RemoveByTagAsync(IEnumerable tags, CancellationToken cancellationToken = default) + { + var tenantId = GetTenantId(); + return _cache.RemoveByTagAsync(tags.Select(t => $"t:{tenantId}:{t}"), cancellationToken); + } +} diff --git a/src/BuildingBlocks/Web/Idempotency/IdempotencyEndpointFilter.cs b/src/BuildingBlocks/Web/Idempotency/IdempotencyEndpointFilter.cs index 0a79d00b2f..95392f1a97 100644 --- a/src/BuildingBlocks/Web/Idempotency/IdempotencyEndpointFilter.cs +++ b/src/BuildingBlocks/Web/Idempotency/IdempotencyEndpointFilter.cs @@ -1,6 +1,7 @@ using System.Security.Cryptography; using System.Text; using System.Text.Json; +using Finbuckle.MultiTenant.Abstractions; using FSH.Framework.Caching; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; @@ -19,8 +20,15 @@ namespace FSH.Framework.Web.Idempotency; /// /// /// Uses directly for the probe read (bypassing -/// 's factory-mandatory API) and -/// for the write path so replays benefit from L1 and the regular tag invalidation story. +/// 's factory-mandatory API) and +/// for the write path so replays +/// benefit from L1 and the regular tag invalidation story. Tenant scoping is +/// applied automatically by — no manual key +/// prefixing required in the write path. +/// Both the probe-read key construction and the write path resolve the tenant +/// from so they agree even during +/// root-operator impersonation, where the JWT claim may differ from the +/// middleware-resolved tenant. /// Using HybridCache with DisableUnderlyingData as a "get-only probe" is a /// known anti-pattern tracked at dotnet/aspnetcore#57191. /// @@ -49,18 +57,22 @@ public sealed class IdempotencyEndpointFilter : IEndpointFilter } var distributedCache = httpContext.RequestServices.GetRequiredService(); - var hybridCache = httpContext.RequestServices.GetRequiredService(); + var tenantCache = httpContext.RequestServices.GetRequiredService(); var logger = httpContext.RequestServices.GetRequiredService>(); + var tenantAccessor = httpContext.RequestServices.GetRequiredService(); - // Include tenant context in cache key for isolation - var tenantId = httpContext.User.FindFirst("tenant")?.Value ?? "global"; - var cacheKey = CacheKeys.IdempotencyEntry(tenantId, idempotencyKey); - var tags = new[] { CacheKeys.Tags.Idempotency, CacheKeys.Tags.Tenant(tenantId) }; + // Resolve tenant from IMultiTenantContextAccessor — must match ITenantCacheService which + // also uses the accessor internally. Using the JWT claim would diverge from the write path + // during root-operator impersonation (claim = "root", resolved tenant = override target). + var tenantId = tenantAccessor.MultiTenantContext?.TenantInfo?.Id ?? "global"; + var logicalKey = CacheKeys.IdempotencyEntry(idempotencyKey); + var fullKey = CacheKeys.IdempotencyEntryFull(tenantId, idempotencyKey); + var tags = new[] { CacheKeys.Tags.Idempotency }; // tenant tag injected automatically by ITenantCacheService // Probe-only read: IDistributedCache has a real GetAsync that returns null on miss, // unlike HybridCache which requires a factory. We bypass L1 here because idempotency // replays are rare relative to first-calls and L1 warmth has little value. - var cachedBytes = await distributedCache.GetAsync(cacheKey, httpContext.RequestAborted).ConfigureAwait(false); + var cachedBytes = await distributedCache.GetAsync(fullKey, httpContext.RequestAborted).ConfigureAwait(false); if (cachedBytes is not null && cachedBytes.Length > 0) { var cached = JsonSerializer.Deserialize(cachedBytes, JsonOpts); @@ -105,7 +117,8 @@ public sealed class IdempotencyEndpointFilter : IEndpointFilter Expiration = options.DefaultTtl, LocalCacheExpiration = options.DefaultTtl < TimeSpan.FromMinutes(2) ? options.DefaultTtl : TimeSpan.FromMinutes(2), }; - await hybridCache.SetAsync(cacheKey, responseToCache, setOptions, tags, httpContext.RequestAborted).ConfigureAwait(false); + // ITenantCacheService.SetAsync prefixes the logical key and adds the tenant tag automatically. + await tenantCache.SetAsync(logicalKey, responseToCache, setOptions, tags, httpContext.RequestAborted).ConfigureAwait(false); } // Best-effort caching: idempotency replay is a convenience, not a correctness requirement catch (Exception ex) when (ex is not OperationCanceledException) diff --git a/src/Modules/Identity/Modules.Identity/Authorization/RolePermissionSyncer.cs b/src/Modules/Identity/Modules.Identity/Authorization/RolePermissionSyncer.cs index d1148c9045..3ab8be20cf 100644 --- a/src/Modules/Identity/Modules.Identity/Authorization/RolePermissionSyncer.cs +++ b/src/Modules/Identity/Modules.Identity/Authorization/RolePermissionSyncer.cs @@ -7,7 +7,6 @@ using Finbuckle.MultiTenant.Abstractions; using Microsoft.AspNetCore.Identity; using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.Caching.Hybrid; using Microsoft.Extensions.Logging; namespace FSH.Modules.Identity.Authorization; @@ -21,7 +20,7 @@ public sealed class RolePermissionSyncer( IdentityDbContext context, RoleManager roleManager, IMultiTenantContextAccessor tenantAccessor, - HybridCache cache, + IGlobalCacheService cache, TimeProvider timeProvider, ILogger logger) { diff --git a/src/Modules/Identity/Modules.Identity/Services/UserPermissionService.cs b/src/Modules/Identity/Modules.Identity/Services/UserPermissionService.cs index 48477c6f36..8bce7a97b4 100644 --- a/src/Modules/Identity/Modules.Identity/Services/UserPermissionService.cs +++ b/src/Modules/Identity/Modules.Identity/Services/UserPermissionService.cs @@ -15,7 +15,7 @@ internal sealed class UserPermissionService( UserManager userManager, RoleManager roleManager, IdentityDbContext db, - HybridCache cache) : IUserPermissionService + IGlobalCacheService cache) : IUserPermissionService { // Hoisted to avoid per-call allocations. Small payload (< 4 KB after base64), so compression // CPU beats the marginal network savings — disable it for this hot path. diff --git a/src/Modules/Multitenancy/Modules.Multitenancy/Services/TenantThemeService.cs b/src/Modules/Multitenancy/Modules.Multitenancy/Services/TenantThemeService.cs index e21fc9e4fe..041538f26d 100644 --- a/src/Modules/Multitenancy/Modules.Multitenancy/Services/TenantThemeService.cs +++ b/src/Modules/Multitenancy/Modules.Multitenancy/Services/TenantThemeService.cs @@ -26,7 +26,8 @@ public sealed class TenantThemeService : ITenantThemeService private static readonly string[] DefaultThemeTags = [CacheKeys.Tags.Themes]; - private readonly HybridCache _cache; + private readonly ITenantCacheService _cache; + private readonly IGlobalCacheService _globalCache; // for cross-tenant entries like DefaultTheme private readonly TenantDbContext _dbContext; private readonly IMultiTenantContextAccessor _tenantAccessor; private readonly IStorageService _storageService; @@ -34,7 +35,8 @@ public sealed class TenantThemeService : ITenantThemeService private readonly ICurrentUser _currentUser; public TenantThemeService( - HybridCache cache, + ITenantCacheService cache, + IGlobalCacheService globalCache, TenantDbContext dbContext, IMultiTenantContextAccessor tenantAccessor, IStorageService storageService, @@ -42,6 +44,7 @@ public TenantThemeService( ICurrentUser currentUser) { _cache = cache; + _globalCache = globalCache; _dbContext = dbContext; _tenantAccessor = tenantAccessor; _storageService = storageService; @@ -60,9 +63,9 @@ public Task GetThemeAsync(string tenantId, CancellationToken ct { ArgumentException.ThrowIfNullOrWhiteSpace(tenantId); - // Per-tenant tag array — one small alloc per call is unavoidable because the tag is - // parameterized by tenantId. Keeping the array allocation local (not LOH) and short-lived. - var tags = new[] { CacheKeys.Tags.Themes, CacheKeys.Tags.Tenant(tenantId) }; + // Tags are relative — TenantHybridCache automatically adds the tenant prefix. + // CacheKeys.Tags.Themes is enough; the per-tenant tag is injected structurally. + var tags = new[] { CacheKeys.Tags.Themes }; // Stateless factory via a static method group — no closure allocation even on L1 hits. var state = new TenantFactoryState(_dbContext, tenantId); @@ -77,7 +80,9 @@ public Task GetThemeAsync(string tenantId, CancellationToken ct public Task GetDefaultThemeAsync(CancellationToken ct = default) { - return _cache.GetOrCreateAsync( + // DefaultTheme is intentionally cross-tenant (same for all tenants). + // We use the underlying HybridCache (not the tenant-scoped wrapper) for this entry. + return _globalCache.GetOrCreateAsync( CacheKeys.DefaultTheme, _dbContext, LoadDefaultThemeAsync, @@ -246,8 +251,8 @@ public async Task SetAsDefaultThemeAsync(string tenantId, CancellationToken ct = entity.IsDefault = true; await _dbContext.SaveChangesAsync(ct).ConfigureAwait(false); - // Invalidate default theme cache - await _cache.RemoveAsync(CacheKeys.DefaultTheme, ct).ConfigureAwait(false); + // Invalidate default theme cache (global cache, not tenant-scoped) + await _globalCache.RemoveAsync(CacheKeys.DefaultTheme, ct).ConfigureAwait(false); if (_logger.IsEnabled(LogLevel.Information)) { @@ -257,9 +262,10 @@ public async Task SetAsDefaultThemeAsync(string tenantId, CancellationToken ct = public async Task InvalidateCacheAsync(string tenantId, CancellationToken ct = default) { - // Purge both the tenant-specific entry and anything tagged for this tenant. + // With TenantHybridCache the key is already prefixed, so removing the logical key + // purges the tenant-scoped entry. RemoveByTagAsync similarly purges only this tenant's data. await _cache.RemoveAsync(CacheKeys.TenantTheme(tenantId), ct).ConfigureAwait(false); - await _cache.RemoveByTagAsync(CacheKeys.Tags.Tenant(tenantId), ct).ConfigureAwait(false); + await _cache.RemoveByTagAsync(CacheKeys.Tags.Themes, ct).ConfigureAwait(false); } private static TenantThemeDto MapEntityToDto(TenantTheme entity) diff --git a/src/Tests/Architecture.Tests/Architecture.Tests.csproj b/src/Tests/Architecture.Tests/Architecture.Tests.csproj index ff69562c3d..1ff49d2e19 100644 --- a/src/Tests/Architecture.Tests/Architecture.Tests.csproj +++ b/src/Tests/Architecture.Tests/Architecture.Tests.csproj @@ -24,6 +24,7 @@ + diff --git a/src/Tests/Architecture.Tests/CachingArchitectureTests.cs b/src/Tests/Architecture.Tests/CachingArchitectureTests.cs new file mode 100644 index 0000000000..3a047b7021 --- /dev/null +++ b/src/Tests/Architecture.Tests/CachingArchitectureTests.cs @@ -0,0 +1,158 @@ +using FSH.Framework.Caching; +using Microsoft.Extensions.Caching.Hybrid; +using NetArchTest.Rules; +using Shouldly; +using System.Reflection; +using Xunit; + +namespace Architecture.Tests; + +/// +/// Architecture guardrails for the caching layer. +/// These tests prevent developers from accidentally injecting +/// directly into business module code, which would bypass the automatic per-tenant +/// key scoping provided by . +/// +/// The two-cache pattern: +/// +/// — per-tenant scoped, for all business data. +/// — cross-tenant singleton, for shared system defaults. +/// +/// Neither role requires module code to inject HybridCache directly. +/// +public sealed class CachingArchitectureTests +{ + /// + /// All FSH module assemblies discovered at test run time. + /// The list grows automatically as new modules are added to the test project. + /// + private static readonly Assembly[] ModuleAssemblies = + ModuleAssemblyDiscovery.GetModuleAssemblies(); + + // ------------------------------------------------------------------------- + // Rule 1 — No direct HybridCache dependency in any module class + // ------------------------------------------------------------------------- + + /// + /// Verifies that no class in any business module injects directly. + /// All module code must inject either (tenant-scoped data) + /// or (intentionally cross-tenant data). + /// + /// Note: HybridCacheEntryOptions (a pure data/options type from the same namespace) + /// is intentionally allowed — it carries no state and is safe to use as a method argument. + /// We check for the cache service class by its full name, not the namespace. + /// + /// + [Fact] + public void ModuleClasses_Should_Not_Depend_On_HybridCache_Directly() + { + // We check by fully-qualified class name rather than namespace because: + // - HybridCache (the abstract class) is what we want to ban from constructors. + // - HybridCacheEntryOptions (a data record in the same namespace) is legitimately + // used in module code as options arguments to ITenantCacheService / IGlobalCacheService. + const string hybridCacheClass = "Microsoft.Extensions.Caching.Hybrid.HybridCache"; + + foreach (var assembly in ModuleAssemblies) + { + var result = Types + .InAssembly(assembly) + .ShouldNot() + .HaveDependencyOn(hybridCacheClass) + .GetResult(); + + var failingTypes = result.FailingTypeNames ?? []; + + failingTypes.ShouldBeEmpty( + $"Module '{assembly.GetName().Name}' contains types that inject HybridCache directly. " + + $"Use ITenantCacheService for per-tenant data or IGlobalCacheService for " + + $"cross-tenant shared data. Violating types: {string.Join(", ", failingTypes)}"); + } + } + + // ------------------------------------------------------------------------- + // Rule 2 — ITenantCacheService is a public interface (DI contract) + // ------------------------------------------------------------------------- + + /// + /// Verifies that is a public interface so that + /// module assemblies can depend on it. If someone accidentally changes its + /// visibility, module code would break at DI resolution time rather than compile time. + /// + [Fact] + public void ITenantCacheService_Should_Be_A_Public_Interface() + { + var type = typeof(ITenantCacheService); + + type.IsInterface.ShouldBeTrue( + $"{type.FullName} must be an interface so modules can depend on the abstraction."); + + type.IsPublic.ShouldBeTrue( + $"{type.FullName} must be public so module assemblies can reference it."); + } + + // ------------------------------------------------------------------------- + // Rule 3 — IGlobalCacheService is a public interface (DI contract) + // ------------------------------------------------------------------------- + + /// + /// Verifies that is a public interface, enabling + /// modules that legitimately need cross-tenant caching (e.g. system defaults) + /// to depend on it explicitly and intentionally. + /// + [Fact] + public void IGlobalCacheService_Should_Be_A_Public_Interface() + { + var type = typeof(IGlobalCacheService); + + type.IsInterface.ShouldBeTrue( + $"{type.FullName} must be an interface so modules can depend on the abstraction."); + + type.IsPublic.ShouldBeTrue( + $"{type.FullName} must be public so module assemblies can reference it."); + } + + // ------------------------------------------------------------------------- + // Rule 4 — Implementations stay internal (prevent module bypass) + // ------------------------------------------------------------------------- + + /// + /// Verifies that the concrete implementations (TenantHybridCache and + /// GlobalHybridCache) are internal, preventing modules from + /// newing them up directly and bypassing the DI contract. + /// + [Fact] + public void CacheService_Implementations_Should_Be_Internal() + { + var cachingAssembly = typeof(ITenantCacheService).Assembly; + + foreach (var implName in new[] { "TenantHybridCache", "GlobalHybridCache" }) + { + var implType = cachingAssembly + .GetTypes() + .FirstOrDefault(t => t.Name == implName); + + implType.ShouldNotBeNull( + $"{implName} implementation must exist in the Caching assembly."); + + implType!.IsPublic.ShouldBeFalse( + $"{implName} should be internal so module code cannot instantiate it directly. " + + $"All consumption must go through the interface resolved via DI."); + } + } + + // ------------------------------------------------------------------------- + // Rule 5 — Guard: at least one module was scanned (prevents vacuous pass) + // ------------------------------------------------------------------------- + + /// + /// Prevents Rule 1 from silently passing with an empty assembly list, + /// which would make it vacuously true and miss real violations in new modules. + /// + [Fact] + public void CachingArchitectureTests_Should_HaveAtLeastOneModuleToScan() + { + ModuleAssemblies.ShouldNotBeEmpty( + "At least one module assembly must be loaded for the caching architecture " + + "rules to be meaningful. Check Architecture.Tests.csproj project references."); + } +} diff --git a/src/Tests/Caching.Tests/CacheKeysTests.cs b/src/Tests/Caching.Tests/CacheKeysTests.cs index 03d346e601..de71acd3ab 100644 --- a/src/Tests/Caching.Tests/CacheKeysTests.cs +++ b/src/Tests/Caching.Tests/CacheKeysTests.cs @@ -27,9 +27,15 @@ public void DefaultTheme_Should_BeConstant() } [Fact] - public void IdempotencyEntry_Should_ScopeByTenant() + public void IdempotencyEntry_Should_ReturnLogicalKey() { - CacheKeys.IdempotencyEntry("t1", "req-42").ShouldBe("idem:t:t1:req-42"); + CacheKeys.IdempotencyEntry("req-42").ShouldBe("idem:req-42"); + } + + [Fact] + public void IdempotencyEntryFull_Should_ScopeByTenant() + { + CacheKeys.IdempotencyEntryFull("t1", "req-42").ShouldBe("t:t1:idem:req-42"); } [Fact] diff --git a/src/Tests/Caching.Tests/Caching.Tests.csproj b/src/Tests/Caching.Tests/Caching.Tests.csproj index 63ad564cbd..8b13c5efb3 100644 --- a/src/Tests/Caching.Tests/Caching.Tests.csproj +++ b/src/Tests/Caching.Tests/Caching.Tests.csproj @@ -22,6 +22,8 @@ runtime; build; native; contentfiles; analyzers; buildtransitive + + diff --git a/src/Tests/Caching.Tests/HeroCachingRegistrationTests.cs b/src/Tests/Caching.Tests/HeroCachingRegistrationTests.cs index af3fd046c4..f502b8efcd 100644 --- a/src/Tests/Caching.Tests/HeroCachingRegistrationTests.cs +++ b/src/Tests/Caching.Tests/HeroCachingRegistrationTests.cs @@ -64,4 +64,37 @@ public void AddHeroCaching_Should_Throw_When_ConfigurationIsNull() Should.Throw(() => services.AddHeroCaching(null!)); } + + [Fact] + public void AddHeroCaching_Should_RegisterITenantCacheService() + { + // Arrange + var services = new ServiceCollection(); + var config = new ConfigurationBuilder().Build(); + + // Act + services.AddHeroCaching(config); + + // Assert — ITenantCacheService must be registered as Scoped (per-request tenant context) + var descriptor = services.FirstOrDefault(d => d.ServiceType == typeof(ITenantCacheService)); + descriptor.ShouldNotBeNull(); + descriptor.Lifetime.ShouldBe(ServiceLifetime.Scoped); + } + + [Fact] + public void AddHeroCaching_Should_RegisterIGlobalCacheService_AsSingleton() + { + // Arrange + var services = new ServiceCollection(); + var config = new ConfigurationBuilder().Build(); + + // Act + services.AddHeroCaching(config); + + // Assert — IGlobalCacheService must be registered as Singleton (no per-request state) + var descriptor = services.FirstOrDefault(d => d.ServiceType == typeof(IGlobalCacheService)); + descriptor.ShouldNotBeNull("IGlobalCacheService should be registered by AddHeroCaching"); + descriptor.Lifetime.ShouldBe(ServiceLifetime.Singleton, + "IGlobalCacheService carries no per-request state and should live as Singleton"); + } } diff --git a/src/Tests/Caching.Tests/TenantCacheServiceTests.cs b/src/Tests/Caching.Tests/TenantCacheServiceTests.cs new file mode 100644 index 0000000000..f21aeef86f --- /dev/null +++ b/src/Tests/Caching.Tests/TenantCacheServiceTests.cs @@ -0,0 +1,216 @@ +using Finbuckle.MultiTenant; +using Finbuckle.MultiTenant.Abstractions; +using FSH.Framework.Caching; +using FSH.Framework.Shared.Multitenancy; +using Microsoft.Extensions.Caching.Hybrid; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using NSubstitute; + +namespace Caching.Tests; + +/// +/// Verifies that automatically scopes every cache +/// key and tag with the current tenant's identifier, preventing cross-tenant data leaks. +/// +public sealed class TenantCacheServiceTests +{ + private static AppTenantInfo BuildTenant(string id) => + new(id, id, string.Empty, $"{id}@test.com", issuer: null); + + private static IMultiTenantContextAccessor BuildAccessor(AppTenantInfo tenant) + { + var accessor = Substitute.For(); + accessor.MultiTenantContext.Returns(new MultiTenantContext(tenant)); + return accessor; + } + + private static (ITenantCacheService sut, HybridCache innerCache) CreateSut(string tenantId) + { + var tenant = BuildTenant(tenantId); + var accessor = BuildAccessor(tenant); + + var services = new ServiceCollection(); + services.AddHeroCaching(new ConfigurationBuilder().Build()); + var provider = services.BuildServiceProvider(); + var innerCache = provider.GetRequiredService(); + + return (TenantHybridCache.Create(innerCache, accessor), innerCache); + } + + [Fact] + public async Task GetOrCreateAsync_DifferentTenants_ShouldNotShareEntries() + { + // Arrange — two caches sharing the same underlying HybridCache but with different tenants + var services = new ServiceCollection(); + services.AddHeroCaching(new ConfigurationBuilder().Build()); + var provider = services.BuildServiceProvider(); + var innerCache = provider.GetRequiredService(); + + var sutA = TenantHybridCache.Create(innerCache, BuildAccessor(BuildTenant("tenant-a"))); + var sutB = TenantHybridCache.Create(innerCache, BuildAccessor(BuildTenant("tenant-b"))); + + int factoryCallCount = 0; + + // Act — write through tenant A's cache + await sutA.GetOrCreateAsync("shared-key", "state", (_, _) => + { + factoryCallCount++; + return ValueTask.FromResult("value-for-a"); + }); + + // Act — read through tenant B's cache (same logical key, different tenant scope) + var resultForB = await sutB.GetOrCreateAsync("shared-key", "state", (_, _) => + { + factoryCallCount++; + return ValueTask.FromResult("value-for-b"); + }); + + // Assert — factory must be called TWICE: tenant isolation is working + factoryCallCount.ShouldBe(2); + resultForB.ShouldBe("value-for-b"); + } + + [Fact] + public async Task SetAsync_RemoveAsync_Should_ScopeKey_To_Tenant() + { + // Arrange + var (sut, _) = CreateSut("tenant-set"); + await sut.SetAsync("mykey", "myvalue"); + + // Act — remove the tenant-scoped entry + await sut.RemoveAsync("mykey"); + + // Re-read — factory should run again after eviction + int factoryCount = 0; + await sut.GetOrCreateAsync("mykey", "state", (_, _) => + { + factoryCount++; + return ValueTask.FromResult("fresh"); + }); + + // Assert — factory ran, confirming removal operated on the correct scoped key + factoryCount.ShouldBe(1); + } + + [Fact] + public async Task GetOrCreateAsync_ShouldThrow_When_TenantContextIsNull() + { + // Arrange — accessor returns null context (no tenant resolved) + var accessor = Substitute.For(); + accessor.MultiTenantContext.Returns((IMultiTenantContext?)null!); + + var services = new ServiceCollection(); + services.AddHeroCaching(new ConfigurationBuilder().Build()); + var innerCache = services.BuildServiceProvider().GetRequiredService(); + + var sut = TenantHybridCache.Create(innerCache, accessor); + + // Act & Assert + await Should.ThrowAsync( + () => sut.GetOrCreateAsync("key", "state", + (_, _) => ValueTask.FromResult("v")).AsTask()); + } + + [Fact] + public async Task GetOrCreateAsync_ShouldThrow_When_TenantIdIsEmpty() + { + // Arrange — tenant context present but TenantId is empty + var tenant = BuildTenant(string.Empty); + var accessor = BuildAccessor(tenant); + + var services = new ServiceCollection(); + services.AddHeroCaching(new ConfigurationBuilder().Build()); + var innerCache = services.BuildServiceProvider().GetRequiredService(); + + var sut = TenantHybridCache.Create(innerCache, accessor); + + // Act & Assert + await Should.ThrowAsync( + () => sut.GetOrCreateAsync("key", "state", + (_, _) => ValueTask.FromResult("v")).AsTask()); + } + + /// + /// Regression test for the tag-format mismatch bug: entries tagged with a logical + /// tag (e.g. "themes") must be evictable via + /// using the same logical tag string. Without the t:{tenantId}: prefix applied + /// consistently on both SET and REMOVE paths, this round-trip silently fails. + /// + [Fact] + public async Task RemoveByTagAsync_RoundTrip_Should_EvictTaggedEntry() + { + // Arrange + var (sut, _) = CreateSut("tenant-tag-rt"); + int factoryCallCount = 0; + + // Prime the cache — entry is stored with tag "my-tag" + await sut.GetOrCreateAsync( + "tagged-key", + "state", + (_, _) => + { + factoryCallCount++; + return ValueTask.FromResult("original-value"); + }, + tags: ["my-tag"]); + + factoryCallCount.ShouldBe(1, "factory should have run once to prime the cache"); + + // Act — evict by the same logical tag string + await sut.RemoveByTagAsync("my-tag"); + + // Re-read: entry must be gone, so factory runs again + var result = await sut.GetOrCreateAsync( + "tagged-key", + "state", + (_, _) => + { + factoryCallCount++; + return ValueTask.FromResult("refreshed-value"); + }, + tags: ["my-tag"]); + + // Assert + factoryCallCount.ShouldBe(2, "factory must run a second time — RemoveByTagAsync should have evicted the entry"); + result.ShouldBe("refreshed-value"); + } + + /// + /// Same round-trip guarantee for the multi-tag overload. + /// + [Fact] + public async Task RemoveByTagAsync_MultiTag_RoundTrip_Should_EvictTaggedEntry() + { + // Arrange + var (sut, _) = CreateSut("tenant-multitag-rt"); + int factoryCallCount = 0; + + await sut.GetOrCreateAsync( + "multi-tagged-key", + "state", + (_, _) => + { + factoryCallCount++; + return ValueTask.FromResult("initial"); + }, + tags: ["tag-a", "tag-b"]); + + factoryCallCount.ShouldBe(1); + + // Act — evict via the multi-tag overload using one of the stored tags + await sut.RemoveByTagAsync(["tag-a", "tag-b"]); + + // Assert — must miss + await sut.GetOrCreateAsync( + "multi-tagged-key", + "state", + (_, _) => + { + factoryCallCount++; + return ValueTask.FromResult("re-fetched"); + }); + + factoryCallCount.ShouldBe(2, "multi-tag removal must evict the entry"); + } +} diff --git a/src/Tests/Integration.Tests/Tests/Multitenancy/TenantActivationTests.cs b/src/Tests/Integration.Tests/Tests/Multitenancy/TenantActivationTests.cs index e4d4448b6d..b919d54daf 100644 --- a/src/Tests/Integration.Tests/Tests/Multitenancy/TenantActivationTests.cs +++ b/src/Tests/Integration.Tests/Tests/Multitenancy/TenantActivationTests.cs @@ -25,6 +25,7 @@ public async Task ChangeTenantActivation_Should_ReturnOk_When_DeactivatingTenant name = $"Deactivate Tenant {uniqueId}", connectionString = (string?)null, adminEmail = $"deact-{uniqueId}@tenant.com", + adminPassword = TestConstants.DefaultPassword, issuer = "deact.issuer" }); createResponse.StatusCode.ShouldBe(HttpStatusCode.Created); @@ -50,6 +51,7 @@ public async Task ChangeTenantActivation_Should_DeactivateAndReactivate_When_Ten name = $"Toggle Tenant {uniqueId}", connectionString = (string?)null, adminEmail = $"toggle-{uniqueId}@tenant.com", + adminPassword = TestConstants.DefaultPassword, issuer = "toggle.issuer" }); diff --git a/src/Tests/Integration.Tests/Tests/Multitenancy/TenantCreationTests.cs b/src/Tests/Integration.Tests/Tests/Multitenancy/TenantCreationTests.cs index 37b8bf8dad..5a47a1cce9 100644 --- a/src/Tests/Integration.Tests/Tests/Multitenancy/TenantCreationTests.cs +++ b/src/Tests/Integration.Tests/Tests/Multitenancy/TenantCreationTests.cs @@ -28,6 +28,7 @@ public async Task CreateTenant_Should_Return201WithId_When_DataIsValid() name = $"Test Tenant {uniqueId}", connectionString = (string?)null, adminEmail = $"admin-{uniqueId}@tenant.com", + adminPassword = TestConstants.DefaultPassword, issuer = "test.issuer" }); @@ -47,6 +48,7 @@ public async Task CreateTenant_Should_Reject_When_IdAlreadyExists() name = $"Dup Tenant {uniqueId}", connectionString = (string?)null, adminEmail = $"dupadmin-{uniqueId}@tenant.com", + adminPassword = TestConstants.DefaultPassword, issuer = "dup.issuer" }; @@ -70,6 +72,7 @@ public async Task CreateTenant_Should_Return401_When_NotAuthenticated() name = "No Auth Tenant", connectionString = (string?)null, adminEmail = "noauth@tenant.com", + adminPassword = TestConstants.DefaultPassword, issuer = "noauth.issuer" });