17. Tenant-Isolated AppSettings Caching

17.1 Purpose and scope

This chapter closes the known issue tracked in Chapter 16 §16.7: a shared-DB cross-tenant AppSettings caching leak. It fixes tenant isolation for every AppSettings-related cache key/invalidation in the BackEnd repo — no package definition, entitlement rule, or subscription logic was touched, and Shumoul.Framework.MultiTenancy.Api was not changed or re-released (the bug and its fix are entirely inside Shumoul.Saas.Api).

17.2 Root cause

AppSetting rows are correctly tenant-scoped at the database/query level — ApplicationDbContext's blanket IsMultiTenant() convention (OnModelCreating, applied to every entity type except a short, explicit exclusion list for cross-tenant entities) covers AppSetting like almost everything else, and a live SQL check during this phase confirmed real, correct, per-tenant TenantId values on every row of the shared dbo.AppSettings table. The database was never the leak.

The leak was entirely in the caching layer, which sits in front of that correctly-scoped data:

• CacheKeys.GetSettingCacheKey<T>() (Shumoul.Application/Constants/CacheKeys.cs) built every group-level AppSettings cache key as $"Setting:{typeof(T).Name}" — the settings type name alone, with no tenant identity anywhere in the string. The same pattern repeated (type name, or type name plus a non-tenant scope id such as an employee/device/printer Guid) across every sibling cache-key method in that file.
• IEasyCachingProvider (EasyCaching in-memory in dev, and any distributed provider in other environments) and IDistributedCache/ICacheService are both registered as process-wide singletons (services.AddEasyCaching(...), services.AddDistributedMemoryCache(), services.TryAdd(ServiceDescriptor.Singleton<ICacheService, CacheService>()) in ServiceCollectionExtensions.cs) — by design, since ASP.NET Core hosts one process for every tenant request routed to it by subdomain.
• Put together: a tenant-unaware key string, cached in a process-wide store, meant any two tenants served by the same host process — not only shared-DB tenants, isolated-DB tenants too — could read or invalidate each other's cached AppSettings object. Shared-DB tenants simply surfaced it first, because the prior phase's verification pass happened to process several of them back-to-back in one process.
• A second, compounding defect was found in the same code path: AppSettingService.GetAppSetting<T>(bool refreshCache = false) accepted a refreshCache parameter but the live implementation never used it — a stale, uncommitted-looking leftover from an older commented-out implementation that did honor it. Every caller that explicitly asked to bypass the cache (most notably InternalOnboardingSettingsPatchService.ApplyGroupAsync<T>, which calls GetAppSetting<T>(refreshCache: true) specifically to avoid working from a stale object before merging a patch) silently got the cached value anyway — which, combined with the tenant-unaware key, is precisely how a second tenant's onboarding or settings-patch apply could pick up a completely different tenant's settings object as its starting point.
• No DI lifetime defect was found. AppSettingService/TenantSettingService/etc. are all Transient (ITransientService), and the two cache abstractions are correctly Singleton — a singleton is the right lifetime for a cache wrapper that holds no tenant state of its own. The bug was entirely in the content of the keys those already-correctly-scoped services built at each call site, never in a service's registration lifetime.

17.3 Fix — a single centralized, tenant-aware key builder

New service, Shumoul.Application.Interfaces.Shared.IAppSettingCacheKeyBuilder / Shumoul.Infrastructure.Services.AppSettingCacheKeyBuilder (ITransientService, depends on Finbuckle's scoped ITenantInfo — the same tenant accessor InternalOnboardingSettingsPatchService already used):

BuildGroupKey<T>()                          → AppSettings:{TenantId}:{GroupName}
BuildSingleKey(settingKey)                  → AppSettings:{TenantId}:single:{Key}
BuildScopedGroupKey<T>(scopeKind, scopeId)   → AppSettings:{TenantId}:{GroupName}:{scopeKind}:{scopeId}

• {TenantId} is ITenantInfo.Id — the exact same ambient tenant identity Finbuckle already resolves per request and stamps onto every IsMultiTenant()-configured entity. No new tenant-identification mechanism was introduced.
• If no tenant is resolved (a background/host context with no HTTP tenant resolution), the builder falls back to an explicit sentinel scope (__no-tenant__) — logged as a warning — rather than silently sharing a key any real tenant's id could also produce.
• BuildScopedGroupKey deliberately shares its prefix with BuildGroupKey for the same tenant+group (AppSettings:{TenantId}:{GroupName}:{scopeKind}:{scopeId} starts with AppSettings:{TenantId}:{GroupName}), so invalidating the base group also invalidates any per-user/device/ printer combined view built on top of it, for that same tenant only — see §17.4.
• All key construction is centralized here; no service builds an AppSettings cache key string by hand anymore.

17.3.1 Callers updated

Service	What changed
AppSettingService	Every CacheKeys.GetSettingCacheKey* call replaced with the builder; GetAppSetting<T>'s refreshCache parameter now actually bypasses the cached read (still writes the fresh value back to cache)
TenantSettingService	Same two fixes — this class is a near-duplicate of AppSettingService and had the identical bug
CashierUserSettingService	Per-employee combined settings now keyed AppSettings:{TenantId}:{GroupName}:user:{employeeId}
DeviceSettingService	Per-device combined settings now keyed AppSettings:{TenantId}:{GroupName}:device:{deviceId}
PrinterSettingService	Per-printer combined settings now keyed AppSettings:{TenantId}:{GroupName}:printer:{printerId}

InternalOnboardingSettingsPatchService itself needed no changes — it already calls GetAppSetting<T>(refreshCache: true) correctly; the defect was entirely in that flag being ignored inside AppSettingService.

17.4 Invalidation

• SetAppSetting<T> invalidates _easyCaching.RemoveByPrefixAsync(BuildGroupKey<T>()) — tenant-scoped by construction, so setting Tenant A's group can never remove Tenant B's cache entry for the same group.
• Because BuildScopedGroupKey shares the group key as a literal prefix, that same prefix removal also clears any already-cached per-user/device/printer combined view for that tenant+group — closing a small pre-existing correctness gap where changing a group's base values could leave a stale combined object cached for a specific employee/device/printer.
• Per-user/device/printer Set calls (SetCashierUserSetting, DeviceSettingService.Set, PrinterSettingService.Set) invalidate only their own single scoped key — changing one employee's override never touches another employee's cache entry or the tenant's shared base-group entry.
• No cache is ever cleared globally across tenants as part of normal Get/Set flow — only ever scoped to the current tenant (and, within that tenant, to the affected group).

17.5 Tenant identity rule

ITenantInfo.Id (Finbuckle) is used — the same identity already stamped on every other IsMultiTenant() entity and already used by InternalOnboardingSettingsPatchService. Not the tenant's database name (shared-DB tenants share one), not a user id, not a subscription or package id — none of those identify the tenant correctly or consistently across isolated-DB and shared-DB modes.

17.6 Isolated-DB tenants

Isolated-DB tenants were also exposed to this bug — the leak was never limited to shared-DB tenants, only first observed there. An isolated-DB tenant's own database was never at risk (a completely separate connection/database), but the process-wide cache singleton doesn't know or care which tenant uses which database mode — two isolated-DB tenants served by the same host process could collide exactly like two shared-DB tenants. The fix embeds TenantId in every key unconditionally, so isolated-DB tenants are protected by the exact same mechanism as shared-DB tenants, with no special-casing required.

17.7 Tests

Shumoul.Application.Tests/ServicesTests/:

• AppSettingCacheKeyBuilderTests.cs (11 tests) — pure unit tests on the key builder: TenantId present in group/single/scoped keys, different tenants never produce the same key for the same group/key/scope-id (including the deliberately adversarial case of two tenants sharing the same employee/device Guid), same tenant+group is stable across calls (required for cache hits to work at all), scoped keys share their group's prefix for the same tenant only, and the no-tenant sentinel never equals a real tenant id.
• AppSettingServiceTenantIsolationTests.cs (4 tests) — the real regression/reproduction suite: two AppSettingService instances (one per tenant) wired against one real, shared, non-mocked EasyCaching in-memory IEasyCachingProvider — the same process-wide singleton the real bug lived in — each with its own mocked ITenantInfo and its own mocked per-tenant repository rows. Covers: read A then B then A again then B again with no cross-contamination; Set on A never changes or invalidates B's cached value; the refreshCache: true fix actually bypasses a stale cached value; a no-tenant-context caller never reads a real tenant's cached entry.

Before the AppSettingService.GetAppSetting<T> fix, GetAppSetting_RefreshCacheTrue_BypassesAStaleCachedValue failed with a NullReferenceException (the ternary refreshCache ? default : await ...GetAsync<T>(...) produced a null CacheValue<T> since EasyCaching's CacheValue<T> is a reference type, not a struct) — the method was restructured to an explicit if (!refreshCache) { ... } guard instead, which also reads more plainly as "skip the cache read entirely when asked to."

17.8 Manual live verification

Dev host started clean, tenants 171994 (Finance Starter) and 793084 (Finance Advanced) — the same shared-DB pair from Chapter 16 §16.7 — through the real GET/POST api/v1/AppSettings/{Get,Set}CashierApps endpoints, using the harmless CashierAppsSettings.Enable_Tips boolean (never a financial/production-sensitive value):

1. GET for tenant A and B — both false (their real, independent starting values).
2. POST tenant A's own full settings payload with Enable_Tips = true (every other field preserved unchanged from A's own GET, including A's own CashAccountId/BankAccountId — never copied from B).
3. GET A → true. GET B (immediately after, same process) → still false. GET A again → still true. GET B again → still false.
4. Tenant A's value reverted back to false to leave no residual change on the shared dev database.
5. Read-only SQL confirmed zero GeneralLedgerJournalEntries/PaymentVouchers/ReceiptVouchers/ StockItemTransactions/AccountOpeningBalances/AppNotifications rows were created for either tenant during this pass, and no WhatsApp/SMS/email notification was triggered (SetCashierApps performs no notification dispatch).

This is a live, in-process reproduction of exactly the failure mode Chapter 16 described — proving the fix on the same shared physical database and the same tenant pair that originally surfaced it.

17.9 Performance

• Repeated reads for the same tenant+group still hit the cache exactly as before — the fix only changed the key string, not the read/write/expiry logic (CacheSettings.AppSettingExpireInMinutes etc. unchanged).
• Different tenants now use provably different keys, so no tenant's read ever forces another tenant's re-fetch, and no tenant's write ever forces another tenant's cache to cold-start.
• No global/all-tenant cache clear was introduced anywhere; invalidation stays scoped to the acting tenant (and, for group-level Set, to that tenant's own affected group and its own scoped children — §17.4).

17.10 Build and test verification (this pass)

Repo	Command	Result
BackEnd	dotnet build (Infrastructure, then full solution)	0 errors, 0 new warnings
BackEnd	dotnet test (targeted: new + onboarding/operational-defaults suites)	15 new tests pass; 32 pre-existing onboarding/operational-defaults tests unaffected
BackEnd	dotnet test (full solution)	830 total (baseline 815 + 15 new), 35 failed — identical to the documented pre-existing baseline (OAuthServiceTests/RegisterClientTests EF InMemory-transaction limitation, DateOnlyJsonConverterTests), zero new failures

Shumoul.Framework.MultiTenancy.Api package: unchanged, still 1.0.124 — this bug and its fix are entirely inside Shumoul.Saas.Api; no MultiTenancyApi source was touched, so no version bump or package republish was needed.

17.11 What not to do (guardrails for future settings work)

• Never build an AppSettings cache key from the settings type name alone (typeof(T).Name) — always go through IAppSettingCacheKeyBuilder.
• Never introduce a static/singleton field holding a tenant id, a tenant's settings object, or any other tenant-specific state. AppSettingCacheKeyBuilder itself is stateless — it only ever reads the current scoped ITenantInfo at call time and builds a string; it stores nothing between calls.
• Never call RemoveByPrefixAsync/RemoveAsync/global-clear across all tenants as part of a normal Get or Set flow. A genuine cross-tenant admin cache-clear operation, if ever needed, must be a separate, explicitly-named, root-admin-gated action — never a side effect of an ordinary tenant request.
• If a new per-scope settings service is added (mirroring CashierUserSettingService/ DeviceSettingService/PrinterSettingService), use IAppSettingCacheKeyBuilder.BuildScopedGroupKey with a clearly named scopeKind string — never a bare Guid-only key, even though scope-id collision across tenants is astronomically unlikely with sequential GUIDs; the tenant segment is the actual isolation guarantee, not the scope id's uniqueness.

17.12 Troubleshooting future cross-tenant setting leakage

Symptom	Likely cause	Check
A tenant sees another tenant's settings value, or a just-changed value doesn't take effect for the same tenant	A cache key built without going through IAppSettingCacheKeyBuilder	Grep for CacheKeys.Get in Shumoul.Infrastructure/Services/ and for any _easyCaching/_cache call not routed through _cacheKeyBuilder
A settings value looks stale immediately after an Apply/Patch flow that explicitly requested a fresh read	refreshCache: true not actually bypassing the cache	Confirm GetAppSetting<T> still uses the if (!refreshCache) { ... } guard (§17.3), not a ternary against default(CacheValue<T>)
Changing a base settings group doesn't affect an already-cached per-user/device/printer combined view	A scoped key not built via BuildScopedGroupKey (so it doesn't share the group's invalidation prefix)	Confirm the scoped service uses _cacheKeyBuilder.BuildScopedGroupKey<T>(...), not a hand-built string

17.13 Remaining gaps

• GetSettingValueAsync<T>(string key)/SetSettingValueAsync<T>(string key, T value) look up an AppSetting row by Key alone, with no GroupName filter — if two different settings groups ever defined a property with the same name, this ad-hoc lookup could resolve the wrong group's row. This is a pre-existing ambiguity in the data query, not a tenant-isolation defect (still correctly scoped to the current tenant at the DB level). Fixed in the very next phase — see Chapter 18 — AppSettings Group-Aware Key Lookup.
• No Redis-specific concern was found or needs tracking: RemoveByPrefixAsync is an EasyCaching abstraction that behaves the same way regardless of the underlying provider (in-memory today; the same tenant-scoped prefix works identically if a Redis provider is configured later).
• No production/staging deployment approval is needed beyond the standard build/test/deploy pipeline — this is a pure BackEnd code fix with no schema, package, or configuration change.
• The unrelated, pre-existing uncommitted secret-redaction diff in Shumoul.Framework.MultiTenancy.Host/appsettings.Development.json is still sitting untouched in the MultiTenancyApi working tree, as in the prior phase — not part of this fix, left for a separate, explicit decision.
• No frontend messaging change is needed — this was a pure backend caching defect with no user-facing error message or contract change.