diff --git a/Tharga.Cache.Tests/AddCacheIdempotencyTests.cs b/Tharga.Cache.Tests/AddCacheIdempotencyTests.cs index 0e7b827..edd5df1 100644 --- a/Tharga.Cache.Tests/AddCacheIdempotencyTests.cs +++ b/Tharga.Cache.Tests/AddCacheIdempotencyTests.cs @@ -120,4 +120,27 @@ public void AddCache_CalledTwice_RegistersSingletonOnce() var cacheMonitorCount = services.Count(s => s.ServiceType == typeof(ICacheMonitor)); cacheMonitorCount.Should().Be(1); } + + [Fact] + public async Task AddCache_CalledTwice_SecondCallRegistrationHonoredAtRuntime() + { + //Arrange — first call registers one type, the resolved cache singleton + //must still see the second call's type registration (merged options). + //Regression for Eplicta 2026-04-14: singleton factories used to close + //over the local CacheOptions, so only the first call's `o` was bound. + var services = new ServiceCollection(); + services.AddLogging(); + + services.AddCache(o => o.RegisterType(t => t.DefaultFreshSpan = TimeSpan.FromMinutes(5))); + services.AddCache(o => o.RegisterType(t => t.DefaultFreshSpan = TimeSpan.FromMinutes(5))); + + var provider = services.BuildServiceProvider(); + var cache = provider.GetRequiredService(); + + //Act — second call's type, no explicit freshSpan. Must resolve via merged options. + var act = async () => await cache.GetAsync("test-key", () => Task.FromResult(42)); + + //Assert + await act.Should().NotThrowAsync(); + } } diff --git a/Tharga.Cache/CacheRegistrationExtensions.cs b/Tharga.Cache/CacheRegistrationExtensions.cs index 7fbba44..3b35c5b 100644 --- a/Tharga.Cache/CacheRegistrationExtensions.cs +++ b/Tharga.Cache/CacheRegistrationExtensions.cs @@ -34,15 +34,17 @@ public static void AddCache(this IServiceCollection serviceCollection, Action(s => s.GetService()); serviceCollection.TryAddSingleton(s => { + var opts = s.GetRequiredService>().Value; var cacheMonitor = s.GetService(); var loggerFactory = s.GetService(); var logger = loggerFactory?.CreateLogger(); - return new FetchQueue(cacheMonitor, o, logger); + return new FetchQueue(cacheMonitor, opts, logger); }); serviceCollection.TryAddSingleton(s => { + var opts = s.GetRequiredService>().Value; var persistLoader = s.GetService(); - var cacheMonitor = new CacheMonitor(persistLoader, o); + var cacheMonitor = new CacheMonitor(persistLoader, opts); return cacheMonitor; }); @@ -59,31 +61,35 @@ public static void AddCache(this IServiceCollection serviceCollection, Action(s => { + var opts = s.GetRequiredService>().Value; var cacheMonitor = s.GetService(); var persistLoader = s.GetService(); var fetchQueue = s.GetService(); - return new EternalCache(cacheMonitor, persistLoader, fetchQueue, o); + return new EternalCache(cacheMonitor, persistLoader, fetchQueue, opts); }); serviceCollection.TryAddSingleton(s => { + var opts = s.GetRequiredService>().Value; var cacheMonitor = s.GetService(); var persistLoader = s.GetService(); var fetchQueue = s.GetService(); - return new TimeToLiveCache(cacheMonitor, persistLoader, fetchQueue, o); + return new TimeToLiveCache(cacheMonitor, persistLoader, fetchQueue, opts); }); serviceCollection.TryAddSingleton(s => { + var opts = s.GetRequiredService>().Value; var cacheMonitor = s.GetService(); var persistLoader = s.GetService(); var fetchQueue = s.GetService(); - return new TimeToIdleCache(cacheMonitor, persistLoader, fetchQueue, o); + return new TimeToIdleCache(cacheMonitor, persistLoader, fetchQueue, opts); }); serviceCollection.TryAddScoped(s => { + var opts = s.GetRequiredService>().Value; var cacheMonitor = s.GetService(); var persistLoader = s.GetService(); var fetchQueue = s.GetService(); - return new EternalCache(cacheMonitor, persistLoader, fetchQueue, o); + return new EternalCache(cacheMonitor, persistLoader, fetchQueue, opts); }); serviceCollection.TryAddSingleton(); diff --git a/Tharga.Cache/Tharga.Cache.csproj b/Tharga.Cache/Tharga.Cache.csproj index 31d44c6..4526dd0 100644 --- a/Tharga.Cache/Tharga.Cache.csproj +++ b/Tharga.Cache/Tharga.Cache.csproj @@ -3,7 +3,7 @@ net8.0;net9.0;net10.0 enable - 1.0.0 + 1.1.0 Daniel Bohlin Thargelion AB Tharga Cache diff --git a/plan/feature.md b/plan/feature.md new file mode 100644 index 0000000..e371e58 --- /dev/null +++ b/plan/feature.md @@ -0,0 +1,46 @@ +# Feature: Fix AddCache singleton options capture + +## Goal + +`AddCache` is intended to support being called multiple times — its own `AppendPreviousRegistrations` helper plus the `RemoveAll>` + re-add pattern documents that intent. In practice the second call's registrations are silently lost because the singleton factories close over the local `CacheOptions o` instead of resolving `IOptions` from the service provider. The first call's `o` is the one bound to every singleton, regardless of what later calls register. + +Switch every affected factory to resolve `IOptions` from the SP. Singletons are constructed lazily on first resolution, after composition is complete, so they pick up the final merged options. + +## Source + +Eplicta request (2026-04-14, High) in `$DOC_ROOT/Tharga/Requests.md`. Reproduces in any app that combines its own `AddCache` with `AddQuilt4NetApplicationInsightsClient`, which calls `AddCache` transitively. Eplicta observes 23 production exceptions on `EnvironmentOption[]`: `InvalidOperationException: "No freshSpan provided and no DefaultFreshSpan configured for cache type ..."` even though the type *was* registered in the second call — just not in the `o` the first call's closure captured. + +## Scope + +In `Tharga.Cache/CacheRegistrationExtensions.cs`, the 6 factories that close over `o`: + +1. `IFetchQueue` (line 35) — uses `o.MaxConcurrentFetchCount` +2. `IManagedCacheMonitor` (line 42) — passes `o` to `CacheMonitor` +3. `IEternalCache` (line 60) +4. `ITimeToLiveCache` (line 67) +5. `ITimeToIdleCache` (line 74) +6. `IScopeCache` (line 81) — scoped, but same closure bug applies + +Note: the Eplicta request only enumerated the 4 cache types. The two infrastructure singletons (`IFetchQueue`, `IManagedCacheMonitor`) have the same closure-capture bug and need the same fix for consistency. Without this they would still see first-call options if `MaxConcurrentFetchCount` / `WatchDogInterval` ever change between calls. + +Out of scope: +- Public API changes +- `RegisterAllIPersistImplementations`, `InvokeAllPersistRegistrations` — these run eagerly during `AddCache` itself, so reading the local `o` is correct. +- `o.RegisterConfigurations(serviceCollection)` callbacks — eager, local `o` is correct. +- Tharga.Console 3.7.4 → 4.0.0 upgrade in `Tharga.Cache.Console` (deferred to a separate change). + +## Acceptance criteria + +- All 6 factories resolve `IOptions` from the SP. No factory closes over the local `o`. +- New regression test in `AddCacheIdempotencyTests.cs`: call `AddCache` twice with different `RegisterType<>` calls, build the SP, resolve a cache, and prove the **second** call's registrations are honored (e.g. `DefaultFreshSpan` set in the second call works at runtime). +- All existing tests in `Tharga.Cache.Tests` still pass. +- `Tharga.Cache.csproj` version bumped: `1.0.0` → `1.1.0` (minor; user-chosen). +- README updated only if it documents one-call-only registration (check `Tharga.Cache/README.md` for guidance about `AddCache` call count). + +## Done condition + +- Feature branch `feature/fix-addcache-options-capture` pushed +- PR opened from `feature/fix-addcache-options-capture` → `master` (GitHub Actions branching strategy) +- CI green +- User confirms the fix as done +- Eplicta workaround referenced in `Core/Eplicta.Aggregator/Program.cs` (per the Requests.md note) can be removed once the new `Tharga.Cache` package version is consumed — tracked as follow-up in `Requests.md` diff --git a/plan/plan.md b/plan/plan.md new file mode 100644 index 0000000..2a1cd0c --- /dev/null +++ b/plan/plan.md @@ -0,0 +1,41 @@ +# Plan: Fix AddCache singleton options capture + +## Steps + +- [x] **1. Write the regression test first (red)** + Added `AddCache_CalledTwice_SecondCallRegistrationHonoredAtRuntime` to `AddCacheIdempotencyTests.cs`. Calls `AddCache` twice with `RegisterType` then `RegisterType`, both with `DefaultFreshSpan`. Resolves `ITimeToLiveCache`, calls `GetAsync` with no explicit freshSpan, asserts it does not throw. + +- [x] **2. Confirmed test fails on master** + Failed with the exact Eplicta-reported exception: `InvalidOperationException: No freshSpan provided and no DefaultFreshSpan configured for cache type Int32` at `TimeCacheBase.GetDefaultFreshSpan[T]()`. Bug reproduced. + +- [x] **3. Fix the 6 factories in `CacheRegistrationExtensions.AddCache`** + All factories now call `var opts = s.GetRequiredService>().Value;` and pass `opts` instead of the captured `o`. Fixed: `IFetchQueue`, `IManagedCacheMonitor`, `IEternalCache`, `ITimeToLiveCache`, `ITimeToIdleCache`, `IScopeCache`. + +- [x] **4. Run the test suite** + - `Tharga.Cache.Tests`: 474/474 pass (regression test included, plus one re-run for a flaky time-critical concurrency test unrelated to this fix). + - `Tharga.Cache.Redis.Tests`: 2/2 pass. + - `Tharga.Cache.File.Tests`: 2 skipped (file-infra-dependent). + - `Tharga.Cache.MongoDB.Tests`: no tests discovered (integration-only project). + +- [x] **5. Check README for one-call guidance** + Grepped `README.md` and `Tharga.Cache/README.md` — no text suggesting `AddCache` must be called once. No README change needed. + +- [x] **6. Bump version `Tharga.Cache.csproj` 1.0.0 → 1.1.0** + Done. + +- [~] **7. Commit + push** + Conventional commit: `fix: resolve CacheOptions from DI in singleton factories so multi-call AddCache merges correctly`. Push branch. + +- [ ] **8. Open PR `feature/fix-addcache-options-capture` → `master`** + +- [ ] **9. Wait for CI green; hand to user** + Do not merge — PR is the review gate. User to confirm done, then per shared-instructions: archive `plan/feature.md` to `$DOC_ROOT/Tharga/plans/Toolkit/Cache/done/fix-addcache-options-capture.md`, delete `plan/`, mark the Eplicta request `Status: Done` with completion date and add the follow-up entry in `Requests.md` (Eplicta should bump Tharga.Cache to 1.1.0 and remove their workaround in `Core/Eplicta.Aggregator/Program.cs`). + +## Notes + +- Scope was expanded from the 4 caches Eplicta listed to all 6 factories. `IFetchQueue` (reads `MaxConcurrentFetchCount`) and `IManagedCacheMonitor` (passes `CacheOptions` to `CacheMonitor`) have the same closure-capture bug and were fixed for consistency. +- First-call-wins on type registrations preserved (`AppendPreviousRegistrations` uses `TryAdd`). Only the singleton-options binding changed — singletons now lazily resolve `IOptions` from the SP, picking up the merged view from the last `AddCache` call. + +## Last session + +Implementation complete on `feature/fix-addcache-options-capture`. All 6 factories rewritten, regression test added and passing, version bumped to 1.1.0. Pending: commit, push, open PR, wait for CI, user confirmation, then archive plan + close Eplicta request with follow-up.