From 264e59ee7a62c6c66545752c7ecf246080efcd0d Mon Sep 17 00:00:00 2001 From: Daniel Bohlin Date: Sun, 16 Aug 2026 13:21:21 +0200 Subject: [PATCH 1/8] chore(deps): bump Tharga.Blazor and migrate tests to Microsoft.Testing.Platform for xunit.v3 4.0.0 --- .github/workflows/build.yml | 4 +- .gitignore | 3 + .../Tharga.Cache.Blazor.csproj | 2 +- .../Tharga.Cache.File.Tests.csproj | 17 +---- .../Tharga.Cache.MongoDB.Tests.csproj | 17 +---- .../Tharga.Cache.Redis.Tests.csproj | 17 +---- Tharga.Cache.Tests/Tharga.Cache.Tests.csproj | 17 +---- global.json | 5 ++ plan/feature.md | 66 +++++++++++++++++++ plan/plan.md | 53 +++++++++++++++ 10 files changed, 143 insertions(+), 58 deletions(-) create mode 100644 global.json create mode 100644 plan/feature.md create mode 100644 plan/plan.md diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d8a32cd..6f61ee9 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -105,7 +105,9 @@ jobs: - name: Test with coverage # Skip Integration (needs Mongo/Redis) and TimeCritical (wall-clock asserts) tests on shared CI runners. - run: dotnet test -c Release --no-build --verbosity normal --filter "(Category!=Integration)&(Category!=TimeCritical)" --collect:"XPlat Code Coverage" --results-directory ./coverage + # Microsoft.Testing.Platform (see global.json) — xunit.v3 4.x dropped the VSTest bridge, so the + # VSTest --filter expression and --collect:"XPlat Code Coverage" no longer apply here. + run: dotnet test -c Release --no-build --filter-not-trait "Category=Integration" --filter-not-trait "Category=TimeCritical" --coverage --coverage-output-format cobertura --results-directory ./coverage - name: Upload coverage to Codecov uses: codecov/codecov-action@v5 diff --git a/.gitignore b/.gitignore index 82e7f0e..7cbae9a 100644 --- a/.gitignore +++ b/.gitignore @@ -149,6 +149,9 @@ coverage*.json coverage*.xml coverage*.info +# Microsoft.Testing.Platform coverage output directory (--results-directory ./coverage) +coverage/ + # Visual Studio code coverage results *.coverage *.coveragexml diff --git a/Tharga.Cache.Blazor/Tharga.Cache.Blazor.csproj b/Tharga.Cache.Blazor/Tharga.Cache.Blazor.csproj index 32e14f6..b99bdf0 100644 --- a/Tharga.Cache.Blazor/Tharga.Cache.Blazor.csproj +++ b/Tharga.Cache.Blazor/Tharga.Cache.Blazor.csproj @@ -37,7 +37,7 @@ - + diff --git a/Tharga.Cache.File.Tests/Tharga.Cache.File.Tests.csproj b/Tharga.Cache.File.Tests/Tharga.Cache.File.Tests.csproj index 652a6b7..f3413e5 100644 --- a/Tharga.Cache.File.Tests/Tharga.Cache.File.Tests.csproj +++ b/Tharga.Cache.File.Tests/Tharga.Cache.File.Tests.csproj @@ -2,27 +2,16 @@ net10.0 + Exe enable - - - all - runtime; build; native; contentfiles; analyzers; buildtransitive - - - all - runtime; build; native; contentfiles; analyzers; buildtransitive - - - all - runtime; build; native; contentfiles; analyzers; buildtransitive - - + + diff --git a/Tharga.Cache.MongoDB.Tests/Tharga.Cache.MongoDB.Tests.csproj b/Tharga.Cache.MongoDB.Tests/Tharga.Cache.MongoDB.Tests.csproj index bbb27c9..7703b09 100644 --- a/Tharga.Cache.MongoDB.Tests/Tharga.Cache.MongoDB.Tests.csproj +++ b/Tharga.Cache.MongoDB.Tests/Tharga.Cache.MongoDB.Tests.csproj @@ -2,27 +2,16 @@ net10.0 + Exe enable - - - all - runtime; build; native; contentfiles; analyzers; buildtransitive - - - all - runtime; build; native; contentfiles; analyzers; buildtransitive - - - all - runtime; build; native; contentfiles; analyzers; buildtransitive - - + + diff --git a/Tharga.Cache.Redis.Tests/Tharga.Cache.Redis.Tests.csproj b/Tharga.Cache.Redis.Tests/Tharga.Cache.Redis.Tests.csproj index 37fbf75..78a7aba 100644 --- a/Tharga.Cache.Redis.Tests/Tharga.Cache.Redis.Tests.csproj +++ b/Tharga.Cache.Redis.Tests/Tharga.Cache.Redis.Tests.csproj @@ -2,6 +2,7 @@ net10.0 + Exe enable $(NoWarn);xUnit1051 @@ -10,21 +11,9 @@ - - - all - runtime; build; native; contentfiles; analyzers; buildtransitive - - - all - runtime; build; native; contentfiles; analyzers; buildtransitive - - - all - runtime; build; native; contentfiles; analyzers; buildtransitive - - + + diff --git a/Tharga.Cache.Tests/Tharga.Cache.Tests.csproj b/Tharga.Cache.Tests/Tharga.Cache.Tests.csproj index c244ecf..2fcce76 100644 --- a/Tharga.Cache.Tests/Tharga.Cache.Tests.csproj +++ b/Tharga.Cache.Tests/Tharga.Cache.Tests.csproj @@ -2,6 +2,7 @@ net10.0 + Exe enable $(NoWarn);xUnit1051 @@ -10,21 +11,9 @@ - - - all - runtime; build; native; contentfiles; analyzers; buildtransitive - - - all - runtime; build; native; contentfiles; analyzers; buildtransitive - - - all - runtime; build; native; contentfiles; analyzers; buildtransitive - - + + diff --git a/global.json b/global.json new file mode 100644 index 0000000..3140116 --- /dev/null +++ b/global.json @@ -0,0 +1,5 @@ +{ + "test": { + "runner": "Microsoft.Testing.Platform" + } +} diff --git a/plan/feature.md b/plan/feature.md new file mode 100644 index 0000000..b2bc18a --- /dev/null +++ b/plan/feature.md @@ -0,0 +1,66 @@ +# Fix: registration race (gh#55) and FluentAssertions dependency leak (gh#56) + +## Goal + +Close two consumer-reported defects in the published `Tharga.Cache` packages: + +1. **gh#55** — `AddCache` is not safe to call concurrently. A process-wide `static` + dictionary is read-and-merged without synchronization, so two hosts built at the + same time in one process can throw + `ArgumentException: Destination array is not long enough`. +2. **gh#56** — `Tharga.Cache` ships **FluentAssertions** as a public NuGet dependency, + propagating the Xceed Community License obligation to every consumer and causing + `NU1605` downgrade errors for consumers pinning an older version. + +## Scope + +- `Tharga.Cache/CacheRegistrationExtensions.cs` — remove the process-wide static + registration state; merge previous registrations from the `IServiceCollection` + instead. +- `Tharga.Cache/Tharga.Cache.csproj` — remove the unused FluentAssertions reference. +- Tests covering both. +- NuGet package updates across the whole solution, up front (per Feature Workflow). + +Out of scope: backlog item 9 (`FetchDataThrottleTests` flakiness). It is a known +pre-existing failure on unmodified master and is not caused by, nor fixed by, this +work — but it does mean the suite needs repeat runs to be trusted. + +## Approach + +### gh#55 — scope the merge state to the container + +The static dictionary is not merely unsynchronized, it is **process-wide state behind +a per-container API**: one host's type registrations leak into an unrelated host's +merged options. The tests already work around this by calling an `internal` +`ResetRegistrations()` from their constructor and `Dispose`. + +`AddCache` already replaces `IOptions` on every call, so the previous +call's `CacheOptions` is available as the existing descriptor's `ImplementationInstance` +before it is removed. Merging from there removes the shared state entirely — no lock, +no cross-host leak, and `ResetRegistrations` and its test hooks can go. + +Conflict semantics are preserved for the case that matters: with two calls registering +the same type, the later call still wins in the merged options, as today. + +### gh#56 — remove the reference + +No source file under `Tharga.Cache/` references FluentAssertions. All five test +projects already declare their own reference, so removal costs nothing. The dependency +flows into all six published packages because the siblings `ProjectReference` the core. + +## Acceptance criteria + +- [ ] Building N `ServiceCollection`s concurrently through `AddCache` neither throws + nor bleeds registrations between them, proven by a test. +- [ ] No `static` mutable registration state remains in `CacheRegistrationExtensions`. +- [ ] `ResetRegistrations` is gone, and `AddCacheIdempotencyTests` passes without it. +- [ ] `Tharga.Cache.csproj` declares no FluentAssertions reference; the packed nuspec + lists only `Microsoft.Extensions.Hosting.Abstractions`. +- [ ] Existing `AddCache` idempotency behaviour is unchanged. +- [ ] Solution builds Release and the full suite passes (allowing for the known + `FetchDataThrottleTests` flakiness, which must be no worse than the master baseline). + +## Done condition + +Both issues closed with evidence, records swept (backlog, central requests file, +GitHub issues), and the PR merged to master. diff --git a/plan/plan.md b/plan/plan.md new file mode 100644 index 0000000..27e3a98 --- /dev/null +++ b/plan/plan.md @@ -0,0 +1,53 @@ +# Plan + +## Steps + +- [~] 1. **NuGet updates up front (whole solution).** + Available: `Tharga.Blazor` 2.3.0 → 2.3.1 (patch); `xunit.v3` 3.2.2 → 4.0.0 and + `xunit.runner.visualstudio` 3.1.5 → 4.0.0 (major) across the four test projects. + Apply, then verify build + full suite before any fix code is written. + Note: `dotnet outdated` is not installed on this machine; used + `dotnet list package --outdated` instead. + +- [ ] 1b. **Microsoft.Testing.Platform migration** (forced by the xunit v4 major). + xunit.v3 4.0.0 drops the VSTest bridge on the .NET 10 SDK — `dotnet test` fails + with *"Testing with VSTest target is no longer supported"* until the project opts + into MTP. User chose (2026-08-15) to do the full migration in this PR rather than + defer it. Requires: `global.json` runner opt-in, dropping the VSTest-only + packages, translating the CI `--filter` trait expression to xunit's MTP filter + options, and replacing `--collect:"XPlat Code Coverage"` with an MTP coverage + route that still emits cobertura into `./coverage` for Codecov. + +- [ ] 2. **Baseline the suite on the branch.** Backlog item 9 records + `FetchDataThrottleTests` as failing most runs on unmodified master. Establish + how it behaves here so a later failure is not misread as caused by the fixes. + +- [ ] 3. **gh#56 — remove the FluentAssertions reference** from + `Tharga.Cache/Tharga.Cache.csproj`. Verify the packed nuspec no longer lists it. + +- [ ] 4. **gh#55 — write the failing test first.** Concurrent `AddCache` across many + independent `ServiceCollection`s: assert no throw, and assert no registration + bleeds from one collection into another. + +- [ ] 5. **gh#55 — implement.** Replace the static dictionary with a merge that reads + the previous `CacheOptions` off the existing `IOptions` descriptor. + Delete `ResetRegistrations` and the test hooks that call it. + +- [ ] 6. **Full verification.** Release build + full suite, repeated runs to separate + real regressions from the known throttle-test flakiness. + +- [ ] 7. **Documentation review.** Check `README.md` and the `docs/` site for anything + describing `AddCache` registration semantics or the package's dependencies. Land + as a separate `docs:` commit if anything changes. + +- [ ] 8. **Close the records** (in this PR): backlog file, central requests file, + and both GitHub issues with evidence. + +- [ ] 9. **Close out.** Archive `plan/feature.md` to the Plan directory `done/`, + `git rm -r plan`, final `fix:` commit, push, open PR. + +## Notes + +_(updated as work proceeds)_ + +- **Step 1 in progress** — sweep run, updates identified, not yet applied. From 8f9b29234f14f975e44039fd1486ce9433d086a2 Mon Sep 17 00:00:00 2001 From: Daniel Bohlin Date: Sun, 16 Aug 2026 13:25:59 +0200 Subject: [PATCH 2/8] fix: stop shipping FluentAssertions as a public dependency (#56) --- Tharga.Cache/Tharga.Cache.csproj | 1 - 1 file changed, 1 deletion(-) diff --git a/Tharga.Cache/Tharga.Cache.csproj b/Tharga.Cache/Tharga.Cache.csproj index cd368ee..23355f5 100644 --- a/Tharga.Cache/Tharga.Cache.csproj +++ b/Tharga.Cache/Tharga.Cache.csproj @@ -53,7 +53,6 @@ - all From 3456f25449cea48c076db7bce608ca05fef0d98d Mon Sep 17 00:00:00 2001 From: Daniel Bohlin Date: Sun, 16 Aug 2026 13:26:04 +0200 Subject: [PATCH 3/8] fix: scope AddCache registration merge to the service collection (#55) --- .../AddCacheConcurrencyTests.cs | 96 +++++++++++++++++++ .../AddCacheIdempotencyTests.cs | 12 +-- Tharga.Cache/CacheRegistrationExtensions.cs | 33 ++++--- 3 files changed, 113 insertions(+), 28 deletions(-) create mode 100644 Tharga.Cache.Tests/AddCacheConcurrencyTests.cs diff --git a/Tharga.Cache.Tests/AddCacheConcurrencyTests.cs b/Tharga.Cache.Tests/AddCacheConcurrencyTests.cs new file mode 100644 index 0000000..617626c --- /dev/null +++ b/Tharga.Cache.Tests/AddCacheConcurrencyTests.cs @@ -0,0 +1,96 @@ +using FluentAssertions; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Tharga.Cache.Persist; +using Xunit; + +namespace Tharga.Cache.Tests; + +public class AddCacheConcurrencyTests +{ + private const int HostCount = 64; + + private sealed record Marker; + + // Nesting a generic in itself yields as many distinct cache types as needed without + // declaring one class per host. + private static Type MarkerType(int depth) + { + var type = typeof(object); + for (var i = 0; i < depth; i++) + { + type = typeof(Marker<>).MakeGenericType(type); + } + + return type; + } + + private static void RegisterMarker(CacheOptions options, int depth) + { + typeof(CacheOptions) + .GetMethod(nameof(CacheOptions.RegisterType))! + .MakeGenericMethod(MarkerType(depth), typeof(IMemory)) + .Invoke(options, [null]); + } + + private static IServiceCollection BuildHost(int depth) + { + var services = new ServiceCollection(); + services.AddLogging(); + services.AddCache(o => RegisterMarker(o, depth)); + return services; + } + + private static IReadOnlyDictionary RegisteredTypes(IServiceCollection services) + { + var options = services + .Last(x => x.ServiceType == typeof(IOptions)) + .ImplementationInstance as IOptions; + + return options!.Value.GetRegistered(); + } + + [Fact] + public void AddCache_CalledConcurrentlyOnIndependentCollections_DoesNotThrow() + { + //Arrange + var depths = Enumerable.Range(1, HostCount).ToArray(); + + //Act + var act = () => Parallel.ForEach(depths, depth => BuildHost(depth)); + + //Assert + act.Should().NotThrow(); + } + + [Fact] + public void AddCache_OnIndependentCollections_DoesNotShareRegistrations() + { + //Arrange + var first = BuildHost(1); + + //Act + var second = BuildHost(2); + + //Assert + RegisteredTypes(first).Keys.Should().BeEquivalentTo([MarkerType(1)]); + RegisteredTypes(second).Keys.Should().BeEquivalentTo([MarkerType(2)]); + } + + [Fact] + public void AddCache_CalledConcurrently_EachCollectionKeepsOnlyItsOwnType() + { + //Arrange + var depths = Enumerable.Range(1, HostCount).ToArray(); + var hosts = new IServiceCollection[HostCount]; + + //Act + Parallel.ForEach(depths, depth => hosts[depth - 1] = BuildHost(depth)); + + //Assert + foreach (var depth in depths) + { + RegisteredTypes(hosts[depth - 1]).Keys.Should().BeEquivalentTo([MarkerType(depth)]); + } + } +} diff --git a/Tharga.Cache.Tests/AddCacheIdempotencyTests.cs b/Tharga.Cache.Tests/AddCacheIdempotencyTests.cs index edd5df1..0b7121b 100644 --- a/Tharga.Cache.Tests/AddCacheIdempotencyTests.cs +++ b/Tharga.Cache.Tests/AddCacheIdempotencyTests.cs @@ -5,18 +5,8 @@ namespace Tharga.Cache.Tests; -public class AddCacheIdempotencyTests : IDisposable +public class AddCacheIdempotencyTests { - public AddCacheIdempotencyTests() - { - CacheRegistrationExtensions.ResetRegistrations(); - } - - public void Dispose() - { - CacheRegistrationExtensions.ResetRegistrations(); - } - [Fact] public void AddCache_CalledTwice_WithDifferentTypes_DoesNotThrow() { diff --git a/Tharga.Cache/CacheRegistrationExtensions.cs b/Tharga.Cache/CacheRegistrationExtensions.cs index 97a242e..4bb62aa 100644 --- a/Tharga.Cache/CacheRegistrationExtensions.cs +++ b/Tharga.Cache/CacheRegistrationExtensions.cs @@ -10,13 +10,6 @@ namespace Tharga.Cache; public static class CacheRegistrationExtensions { - private static readonly Dictionary _configuredPersistTypes = new(); - - internal static void ResetRegistrations() - { - _configuredPersistTypes.Clear(); - } - public static void AddCache(this IServiceCollection serviceCollection, Action options = null) { var o = new CacheOptions @@ -25,7 +18,7 @@ public static void AddCache(this IServiceCollection serviceCollection, Action on each call so it carries the merged type registrations. serviceCollection.RemoveAll>(); @@ -104,17 +97,23 @@ public static void AddCache(this IServiceCollection serviceCollection, Action - /// If AddCache is called several times, this method merges all registrations so they can be used in the end. - /// First registration wins — duplicate types are silently skipped. + /// If AddCache is called several times on the same service collection, this method merges all registrations + /// so they can be used in the end. The type registered by this call wins — duplicates from earlier calls are + /// silently skipped. /// - private static void AppendPreviousRegistrations(CacheOptions o) + /// + /// The accumulated registrations are read back from the service collection rather than from process-wide + /// state, so hosts built concurrently in one process neither race nor inherit each other's registrations. + /// + private static void AppendPreviousRegistrations(IServiceCollection serviceCollection, CacheOptions o) { - var previouslyRegisteredTypes = _configuredPersistTypes.ToArray(); - foreach (var item in o.GetRegistered()) - { - _configuredPersistTypes.TryAdd(item.Key, item.Value); - } - foreach (var previouslyRegisteredType in previouslyRegisteredTypes) + var previous = serviceCollection + .LastOrDefault(x => x.ServiceType == typeof(IOptions))? + .ImplementationInstance as IOptions; + + if (previous == null) return; + + foreach (var previouslyRegisteredType in previous.Value.GetRegistered()) { o.TryAddType(previouslyRegisteredType.Key, previouslyRegisteredType.Value); } From d62d4ba277b4e1b41308459cf87081eea234bfdb Mon Sep 17 00:00:00 2001 From: Daniel Bohlin Date: Sun, 16 Aug 2026 13:26:46 +0200 Subject: [PATCH 4/8] docs: note that AddCache registration merging is scoped per service collection --- docs/articles/getting-started.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/articles/getting-started.md b/docs/articles/getting-started.md index 5696f34..14af694 100644 --- a/docs/articles/getting-started.md +++ b/docs/articles/getting-started.md @@ -16,6 +16,8 @@ builder.Services.AddCache(); `AddCache` is idempotent — calling it more than once (for example when several libraries each register cache types) merges the registrations instead of throwing. +The merge is scoped to the service collection it is called on, so it is safe to build several hosts concurrently in one process — parallel integration tests each constructing a `WebApplicationFactory`, or a multi-tenant host spinning up isolated containers. Registrations made on one service collection never appear in another. + ## The get-or-load pattern Inject one of the four cache interfaces and call `GetAsync` with a key and a fetch delegate. The first call runs the delegate and stores the result; subsequent calls within the fresh span return the cached value without invoking the delegate. From 49d9cbfc77d8f8d572d8e4025cbf959afe2558cc Mon Sep 17 00:00:00 2001 From: Daniel Bohlin Date: Sun, 16 Aug 2026 13:27:28 +0200 Subject: [PATCH 5/8] chore: update feature plan --- plan/plan.md | 80 ++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 69 insertions(+), 11 deletions(-) diff --git a/plan/plan.md b/plan/plan.md index 27e3a98..a70366c 100644 --- a/plan/plan.md +++ b/plan/plan.md @@ -2,14 +2,14 @@ ## Steps -- [~] 1. **NuGet updates up front (whole solution).** +- [x] 1. **NuGet updates up front (whole solution).** Available: `Tharga.Blazor` 2.3.0 → 2.3.1 (patch); `xunit.v3` 3.2.2 → 4.0.0 and `xunit.runner.visualstudio` 3.1.5 → 4.0.0 (major) across the four test projects. Apply, then verify build + full suite before any fix code is written. Note: `dotnet outdated` is not installed on this machine; used `dotnet list package --outdated` instead. -- [ ] 1b. **Microsoft.Testing.Platform migration** (forced by the xunit v4 major). +- [x] 1b. **Microsoft.Testing.Platform migration** (forced by the xunit v4 major). xunit.v3 4.0.0 drops the VSTest bridge on the .NET 10 SDK — `dotnet test` fails with *"Testing with VSTest target is no longer supported"* until the project opts into MTP. User chose (2026-08-15) to do the full migration in this PR rather than @@ -18,36 +18,94 @@ options, and replacing `--collect:"XPlat Code Coverage"` with an MTP coverage route that still emits cobertura into `./coverage` for Codecov. -- [ ] 2. **Baseline the suite on the branch.** Backlog item 9 records +- [x] 2. **Baseline the suite on the branch.** Backlog item 9 records `FetchDataThrottleTests` as failing most runs on unmodified master. Establish how it behaves here so a later failure is not misread as caused by the fixes. -- [ ] 3. **gh#56 — remove the FluentAssertions reference** from +- [x] 3. **gh#56 — remove the FluentAssertions reference** from `Tharga.Cache/Tharga.Cache.csproj`. Verify the packed nuspec no longer lists it. -- [ ] 4. **gh#55 — write the failing test first.** Concurrent `AddCache` across many +- [x] 4. **gh#55 — write the failing test first.** Concurrent `AddCache` across many independent `ServiceCollection`s: assert no throw, and assert no registration bleeds from one collection into another. -- [ ] 5. **gh#55 — implement.** Replace the static dictionary with a merge that reads +- [x] 5. **gh#55 — implement.** Replace the static dictionary with a merge that reads the previous `CacheOptions` off the existing `IOptions` descriptor. Delete `ResetRegistrations` and the test hooks that call it. -- [ ] 6. **Full verification.** Release build + full suite, repeated runs to separate +- [x] 6. **Full verification.** Release build + full suite, repeated runs to separate real regressions from the known throttle-test flakiness. -- [ ] 7. **Documentation review.** Check `README.md` and the `docs/` site for anything +- [x] 7. **Documentation review.** Check `README.md` and the `docs/` site for anything describing `AddCache` registration semantics or the package's dependencies. Land as a separate `docs:` commit if anything changes. -- [ ] 8. **Close the records** (in this PR): backlog file, central requests file, +- [~] 8. **User testing.** Push the branch; user tests from origin. Do NOT open the PR + until the user confirms the feature is done. + +- [ ] 9. **Close the records** (in this PR): backlog file, central requests file, and both GitHub issues with evidence. -- [ ] 9. **Close out.** Archive `plan/feature.md` to the Plan directory `done/`, +- [ ] 10. **Close out.** Archive `plan/feature.md` to the Plan directory `done/`, `git rm -r plan`, final `fix:` commit, push, open PR. ## Notes _(updated as work proceeds)_ -- **Step 1 in progress** — sweep run, updates identified, not yet applied. +- **Steps 1, 1b, 2 done** (commit `264e59e`). `Tharga.Blazor` → 2.3.1. All four test + projects moved to `xunit.v3` 4.0.0 under Microsoft.Testing.Platform: + - `global.json` selects the MTP runner (`test.runner`). + - Dropped `Microsoft.NET.Test.Sdk`, `xunit.runner.visualstudio`, `coverlet.collector` + and `coverlet.msbuild` — all VSTest-only under this model. Added + `Microsoft.Testing.Extensions.CodeCoverage` for cobertura output. + - Test projects now need `Exe`; xunit v3 4.x refuses to + build a library test project. + - CI test step rewritten: `--filter "(Category!=Integration)&(Category!=TimeCritical)"` + → `--filter-not-trait "Category=Integration" --filter-not-trait "Category=TimeCritical"`; + `--collect:"XPlat Code Coverage"` → `--coverage --coverage-output-format cobertura`. + Verified locally: four cobertura files land in `./coverage` for Codecov. + - `coverage/` added to `.gitignore`. +- **Baseline (step 2):** unfiltered run = 486 tests, 483 passed, 1 failed — the failure + is the known-flaky `FetchDataThrottleTests` from backlog item 9 + ("Expected monitorGetEventCount to be 10, but found 9"), i.e. the documented master + behaviour, not a migration regression. CI-filtered run = 464 tests, **all pass**, + because the filter excludes the TimeCritical throttle tests. + +- **Step 3 done** (commit `8f9b292`). Verified by packing the core project and reading + the nuspec out of the `.nupkg`: the `net10.0` dependency group now lists only + `Microsoft.Extensions.Hosting.Abstractions`. Confirmed no source file under + `Tharga.Cache/` referenced FluentAssertions, and all test projects already declare + their own reference, so nothing else moved. + +- **Steps 4 and 5 done** (commit `3456f25`). + - New `AddCacheConcurrencyTests` (3 tests, 64 concurrent hosts). Against the old code + all three failed, reproducing the reporter's exact exception — + `ArgumentException: Destination array is not long enough` at `Dictionary.CopyTo` ← + `Enumerable.ToArray` — plus an `InvalidOperationException` about concurrent + mutation, plus the cross-collection leak (host 2's options contained host 1's type). + - Distinct cache types per host come from nesting a private generic marker in itself, + so N hosts need no N declared classes. + - `AppendPreviousRegistrations` now takes the `IServiceCollection` and reads the prior + `CacheOptions` off the existing `IOptions` descriptor's + `ImplementationInstance`, before the `RemoveAll` that replaces it. Falls through + harmlessly when the descriptor is absent or registered by other means. + - `_configuredPersistTypes` and `ResetRegistrations` deleted; `AddCacheIdempotencyTests` + no longer needs `IDisposable` or the reset hooks and still passes unchanged. + +- **Step 6 verification.** + - CI-gate command (the one the workflow runs) green **3 out of 3**: 467 tests, + 465 passed, 2 skipped. + - Full unfiltered suite: 489 tests, 3 failed — all three in `FetchDataThrottleTests`, + all carrying `[Trait("Category", "TimeCritical")]` and therefore excluded from CI. + Run in isolation that class passes 2 out of 2, confirming these are the documented + parallel-load flakiness of backlog item 9 rather than a regression. Worth noting: + the new concurrency tests deliberately saturate the CPU with 64 parallel host + builds, which makes that pre-existing flakiness easier to hit in a local full-suite + run. It does not affect CI, but it strengthens the case for backlog item 9. + +- **Step 7 done** (commit `d62d4ba`). `docs/articles/getting-started.md` already stated + that `AddCache` is idempotent; added that the merge is scoped per service collection + and that concurrent host construction is safe, naming the reporter's + `WebApplicationFactory` scenario. `README.md` documents registration but not + idempotency, so it needed no change; there is no CHANGELOG in this repo. From 5a738711b61e227b9258afb009d205c893fe44cc Mon Sep 17 00:00:00 2001 From: Daniel Bohlin Date: Sun, 16 Aug 2026 13:40:36 +0200 Subject: [PATCH 6/8] chore(deps): bump Microsoft.Testing.Extensions.CodeCoverage to 18.10.0 --- Tharga.Cache.File.Tests/Tharga.Cache.File.Tests.csproj | 2 +- Tharga.Cache.MongoDB.Tests/Tharga.Cache.MongoDB.Tests.csproj | 2 +- Tharga.Cache.Redis.Tests/Tharga.Cache.Redis.Tests.csproj | 2 +- Tharga.Cache.Tests/Tharga.Cache.Tests.csproj | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Tharga.Cache.File.Tests/Tharga.Cache.File.Tests.csproj b/Tharga.Cache.File.Tests/Tharga.Cache.File.Tests.csproj index f3413e5..edf4b36 100644 --- a/Tharga.Cache.File.Tests/Tharga.Cache.File.Tests.csproj +++ b/Tharga.Cache.File.Tests/Tharga.Cache.File.Tests.csproj @@ -10,7 +10,7 @@ - + diff --git a/Tharga.Cache.MongoDB.Tests/Tharga.Cache.MongoDB.Tests.csproj b/Tharga.Cache.MongoDB.Tests/Tharga.Cache.MongoDB.Tests.csproj index 7703b09..878e8dd 100644 --- a/Tharga.Cache.MongoDB.Tests/Tharga.Cache.MongoDB.Tests.csproj +++ b/Tharga.Cache.MongoDB.Tests/Tharga.Cache.MongoDB.Tests.csproj @@ -10,7 +10,7 @@ - + diff --git a/Tharga.Cache.Redis.Tests/Tharga.Cache.Redis.Tests.csproj b/Tharga.Cache.Redis.Tests/Tharga.Cache.Redis.Tests.csproj index 78a7aba..1c14811 100644 --- a/Tharga.Cache.Redis.Tests/Tharga.Cache.Redis.Tests.csproj +++ b/Tharga.Cache.Redis.Tests/Tharga.Cache.Redis.Tests.csproj @@ -12,7 +12,7 @@ - + diff --git a/Tharga.Cache.Tests/Tharga.Cache.Tests.csproj b/Tharga.Cache.Tests/Tharga.Cache.Tests.csproj index 2fcce76..e3d1edf 100644 --- a/Tharga.Cache.Tests/Tharga.Cache.Tests.csproj +++ b/Tharga.Cache.Tests/Tharga.Cache.Tests.csproj @@ -12,7 +12,7 @@ - + From 21d82c4b04ef0517f1a2edd5e9c4d3e945154ccf Mon Sep 17 00:00:00 2001 From: Daniel Bohlin Date: Sun, 16 Aug 2026 13:43:58 +0200 Subject: [PATCH 7/8] test: assert the actual AddCache precedence instead of a non-null cache --- Tharga.Cache.Tests/AddCacheIdempotencyTests.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Tharga.Cache.Tests/AddCacheIdempotencyTests.cs b/Tharga.Cache.Tests/AddCacheIdempotencyTests.cs index 0b7121b..a4b80dc 100644 --- a/Tharga.Cache.Tests/AddCacheIdempotencyTests.cs +++ b/Tharga.Cache.Tests/AddCacheIdempotencyTests.cs @@ -1,5 +1,6 @@ using FluentAssertions; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; using Tharga.Cache.Persist; using Xunit; @@ -44,7 +45,7 @@ public void AddCache_CalledTwice_WithSameType_DoesNotThrow() } [Fact] - public void AddCache_CalledTwice_WithSameType_FirstRegistrationWins() + public void AddCache_CalledTwice_WithSameType_LatestRegistrationWins() { //Arrange var services = new ServiceCollection(); @@ -56,8 +57,8 @@ public void AddCache_CalledTwice_WithSameType_FirstRegistrationWins() //Assert var provider = services.BuildServiceProvider(); - var cache = provider.GetRequiredService(); - cache.Should().NotBeNull(); + var options = provider.GetRequiredService>().Value; + options.GetRegistered()[typeof(string)].DefaultFreshSpan.Should().Be(TimeSpan.FromMinutes(99)); } [Fact] From b42aeaf6fc841a0c037958668973290d143c80cb Mon Sep 17 00:00:00 2001 From: Daniel Bohlin Date: Sun, 16 Aug 2026 13:43:59 +0200 Subject: [PATCH 8/8] fix: issue-55-56-registration-race-and-fluentassertions complete --- plan/feature.md | 66 ---------------------------- plan/plan.md | 111 ------------------------------------------------ 2 files changed, 177 deletions(-) delete mode 100644 plan/feature.md delete mode 100644 plan/plan.md diff --git a/plan/feature.md b/plan/feature.md deleted file mode 100644 index b2bc18a..0000000 --- a/plan/feature.md +++ /dev/null @@ -1,66 +0,0 @@ -# Fix: registration race (gh#55) and FluentAssertions dependency leak (gh#56) - -## Goal - -Close two consumer-reported defects in the published `Tharga.Cache` packages: - -1. **gh#55** — `AddCache` is not safe to call concurrently. A process-wide `static` - dictionary is read-and-merged without synchronization, so two hosts built at the - same time in one process can throw - `ArgumentException: Destination array is not long enough`. -2. **gh#56** — `Tharga.Cache` ships **FluentAssertions** as a public NuGet dependency, - propagating the Xceed Community License obligation to every consumer and causing - `NU1605` downgrade errors for consumers pinning an older version. - -## Scope - -- `Tharga.Cache/CacheRegistrationExtensions.cs` — remove the process-wide static - registration state; merge previous registrations from the `IServiceCollection` - instead. -- `Tharga.Cache/Tharga.Cache.csproj` — remove the unused FluentAssertions reference. -- Tests covering both. -- NuGet package updates across the whole solution, up front (per Feature Workflow). - -Out of scope: backlog item 9 (`FetchDataThrottleTests` flakiness). It is a known -pre-existing failure on unmodified master and is not caused by, nor fixed by, this -work — but it does mean the suite needs repeat runs to be trusted. - -## Approach - -### gh#55 — scope the merge state to the container - -The static dictionary is not merely unsynchronized, it is **process-wide state behind -a per-container API**: one host's type registrations leak into an unrelated host's -merged options. The tests already work around this by calling an `internal` -`ResetRegistrations()` from their constructor and `Dispose`. - -`AddCache` already replaces `IOptions` on every call, so the previous -call's `CacheOptions` is available as the existing descriptor's `ImplementationInstance` -before it is removed. Merging from there removes the shared state entirely — no lock, -no cross-host leak, and `ResetRegistrations` and its test hooks can go. - -Conflict semantics are preserved for the case that matters: with two calls registering -the same type, the later call still wins in the merged options, as today. - -### gh#56 — remove the reference - -No source file under `Tharga.Cache/` references FluentAssertions. All five test -projects already declare their own reference, so removal costs nothing. The dependency -flows into all six published packages because the siblings `ProjectReference` the core. - -## Acceptance criteria - -- [ ] Building N `ServiceCollection`s concurrently through `AddCache` neither throws - nor bleeds registrations between them, proven by a test. -- [ ] No `static` mutable registration state remains in `CacheRegistrationExtensions`. -- [ ] `ResetRegistrations` is gone, and `AddCacheIdempotencyTests` passes without it. -- [ ] `Tharga.Cache.csproj` declares no FluentAssertions reference; the packed nuspec - lists only `Microsoft.Extensions.Hosting.Abstractions`. -- [ ] Existing `AddCache` idempotency behaviour is unchanged. -- [ ] Solution builds Release and the full suite passes (allowing for the known - `FetchDataThrottleTests` flakiness, which must be no worse than the master baseline). - -## Done condition - -Both issues closed with evidence, records swept (backlog, central requests file, -GitHub issues), and the PR merged to master. diff --git a/plan/plan.md b/plan/plan.md deleted file mode 100644 index a70366c..0000000 --- a/plan/plan.md +++ /dev/null @@ -1,111 +0,0 @@ -# Plan - -## Steps - -- [x] 1. **NuGet updates up front (whole solution).** - Available: `Tharga.Blazor` 2.3.0 → 2.3.1 (patch); `xunit.v3` 3.2.2 → 4.0.0 and - `xunit.runner.visualstudio` 3.1.5 → 4.0.0 (major) across the four test projects. - Apply, then verify build + full suite before any fix code is written. - Note: `dotnet outdated` is not installed on this machine; used - `dotnet list package --outdated` instead. - -- [x] 1b. **Microsoft.Testing.Platform migration** (forced by the xunit v4 major). - xunit.v3 4.0.0 drops the VSTest bridge on the .NET 10 SDK — `dotnet test` fails - with *"Testing with VSTest target is no longer supported"* until the project opts - into MTP. User chose (2026-08-15) to do the full migration in this PR rather than - defer it. Requires: `global.json` runner opt-in, dropping the VSTest-only - packages, translating the CI `--filter` trait expression to xunit's MTP filter - options, and replacing `--collect:"XPlat Code Coverage"` with an MTP coverage - route that still emits cobertura into `./coverage` for Codecov. - -- [x] 2. **Baseline the suite on the branch.** Backlog item 9 records - `FetchDataThrottleTests` as failing most runs on unmodified master. Establish - how it behaves here so a later failure is not misread as caused by the fixes. - -- [x] 3. **gh#56 — remove the FluentAssertions reference** from - `Tharga.Cache/Tharga.Cache.csproj`. Verify the packed nuspec no longer lists it. - -- [x] 4. **gh#55 — write the failing test first.** Concurrent `AddCache` across many - independent `ServiceCollection`s: assert no throw, and assert no registration - bleeds from one collection into another. - -- [x] 5. **gh#55 — implement.** Replace the static dictionary with a merge that reads - the previous `CacheOptions` off the existing `IOptions` descriptor. - Delete `ResetRegistrations` and the test hooks that call it. - -- [x] 6. **Full verification.** Release build + full suite, repeated runs to separate - real regressions from the known throttle-test flakiness. - -- [x] 7. **Documentation review.** Check `README.md` and the `docs/` site for anything - describing `AddCache` registration semantics or the package's dependencies. Land - as a separate `docs:` commit if anything changes. - -- [~] 8. **User testing.** Push the branch; user tests from origin. Do NOT open the PR - until the user confirms the feature is done. - -- [ ] 9. **Close the records** (in this PR): backlog file, central requests file, - and both GitHub issues with evidence. - -- [ ] 10. **Close out.** Archive `plan/feature.md` to the Plan directory `done/`, - `git rm -r plan`, final `fix:` commit, push, open PR. - -## Notes - -_(updated as work proceeds)_ - -- **Steps 1, 1b, 2 done** (commit `264e59e`). `Tharga.Blazor` → 2.3.1. All four test - projects moved to `xunit.v3` 4.0.0 under Microsoft.Testing.Platform: - - `global.json` selects the MTP runner (`test.runner`). - - Dropped `Microsoft.NET.Test.Sdk`, `xunit.runner.visualstudio`, `coverlet.collector` - and `coverlet.msbuild` — all VSTest-only under this model. Added - `Microsoft.Testing.Extensions.CodeCoverage` for cobertura output. - - Test projects now need `Exe`; xunit v3 4.x refuses to - build a library test project. - - CI test step rewritten: `--filter "(Category!=Integration)&(Category!=TimeCritical)"` - → `--filter-not-trait "Category=Integration" --filter-not-trait "Category=TimeCritical"`; - `--collect:"XPlat Code Coverage"` → `--coverage --coverage-output-format cobertura`. - Verified locally: four cobertura files land in `./coverage` for Codecov. - - `coverage/` added to `.gitignore`. -- **Baseline (step 2):** unfiltered run = 486 tests, 483 passed, 1 failed — the failure - is the known-flaky `FetchDataThrottleTests` from backlog item 9 - ("Expected monitorGetEventCount to be 10, but found 9"), i.e. the documented master - behaviour, not a migration regression. CI-filtered run = 464 tests, **all pass**, - because the filter excludes the TimeCritical throttle tests. - -- **Step 3 done** (commit `8f9b292`). Verified by packing the core project and reading - the nuspec out of the `.nupkg`: the `net10.0` dependency group now lists only - `Microsoft.Extensions.Hosting.Abstractions`. Confirmed no source file under - `Tharga.Cache/` referenced FluentAssertions, and all test projects already declare - their own reference, so nothing else moved. - -- **Steps 4 and 5 done** (commit `3456f25`). - - New `AddCacheConcurrencyTests` (3 tests, 64 concurrent hosts). Against the old code - all three failed, reproducing the reporter's exact exception — - `ArgumentException: Destination array is not long enough` at `Dictionary.CopyTo` ← - `Enumerable.ToArray` — plus an `InvalidOperationException` about concurrent - mutation, plus the cross-collection leak (host 2's options contained host 1's type). - - Distinct cache types per host come from nesting a private generic marker in itself, - so N hosts need no N declared classes. - - `AppendPreviousRegistrations` now takes the `IServiceCollection` and reads the prior - `CacheOptions` off the existing `IOptions` descriptor's - `ImplementationInstance`, before the `RemoveAll` that replaces it. Falls through - harmlessly when the descriptor is absent or registered by other means. - - `_configuredPersistTypes` and `ResetRegistrations` deleted; `AddCacheIdempotencyTests` - no longer needs `IDisposable` or the reset hooks and still passes unchanged. - -- **Step 6 verification.** - - CI-gate command (the one the workflow runs) green **3 out of 3**: 467 tests, - 465 passed, 2 skipped. - - Full unfiltered suite: 489 tests, 3 failed — all three in `FetchDataThrottleTests`, - all carrying `[Trait("Category", "TimeCritical")]` and therefore excluded from CI. - Run in isolation that class passes 2 out of 2, confirming these are the documented - parallel-load flakiness of backlog item 9 rather than a regression. Worth noting: - the new concurrency tests deliberately saturate the CPU with 64 parallel host - builds, which makes that pre-existing flakiness easier to hit in a local full-suite - run. It does not affect CI, but it strengthens the case for backlog item 9. - -- **Step 7 done** (commit `d62d4ba`). `docs/articles/getting-started.md` already stated - that `AddCache` is idempotent; added that the merge is scoped per service collection - and that concurrent host construction is safe, naming the reporter's - `WebApplicationFactory` scenario. `README.md` documents registration but not - idempotency, so it needed no change; there is no CHANGELOG in this repo.