diff --git a/src/GameCult.Caching/Contracts/cultcache-store-composition.md b/src/GameCult.Caching/Contracts/cultcache-store-composition.md index 251a1c1e..a70814d5 100644 --- a/src/GameCult.Caching/Contracts/cultcache-store-composition.md +++ b/src/GameCult.Caching/Contracts/cultcache-store-composition.md @@ -101,10 +101,12 @@ which route wrote it. already agree when publication starts. - An admission runs a record's `[CultName]` and `[CultIndex]` getters once per landing record, before the store is written and before any `Sequence` is - minted. A conditional commit on a store that holds variants is judged a second - time against the file it merges onto, which runs the getters again for the - records it lands and for any variant it re-resolves; that judgement is also - before the store is written. A getter that throws refuses the whole change + minted. A conditional commit on a clean file store is judged a second time + against the file it merges onto whenever the cache holds variants in any store, + the batch lands a variant, or the file holds one. That second read runs the + getters again for the records the batch lands, for any variant it re-resolves, + and for the records other writers put in the file (the arriving set); that + judgement is also before the store is written. A getter that throws refuses the whole change set, loads included: nothing is written, minted or left in memory. - Pulling all stores pulls every attached store even if a handler throws during one store's load, then rethrows. diff --git a/src/GameCult.Caching/CultCache.cs b/src/GameCult.Caching/CultCache.cs index 986ea974..e8ec8fd6 100644 --- a/src/GameCult.Caching/CultCache.cs +++ b/src/GameCult.Caching/CultCache.cs @@ -1903,6 +1903,29 @@ namespace GameCult.Caching change.Sequence)); } + // Disposing removes the journal. A dispose the cache refuses (from inside a journal, or inside another cache's hold) + // throws and leaves the registration live and disposable; it counts as disposed only once the removal succeeded. + private sealed class JournalRegistration : IDisposable + { + private readonly CultCache _cache; + private readonly Action>> _journal; + private bool _removed; + + public JournalRegistration(CultCache cache, Action>> journal) + { + _cache = cache; + _journal = journal; + } + + public void Dispose() + { + if (_removed) + return; + _cache.Held(() => _cache._journals.Remove(_journal)); + _removed = true; + } + } + private IDisposable Register(Observer observer) { lock (_observerGate) @@ -1941,7 +1964,7 @@ namespace GameCult.Caching _journals.Add(journal); return true; }); - return Disposable.Create(() => Held(() => _journals.Remove(journal))); + return new JournalRegistration(this, journal); } // Attaching reads the store; there is no interval in which it is attached but unread. @@ -2425,11 +2448,13 @@ namespace GameCult.Caching lock (_gate) result = body(); } - catch + catch (Exception exception) { _held = null; Publish(mine); - throw; + if (mine.Failures.Count == 0) + throw; + throw new AggregateException(mine.Failures.Prepend(exception)); } _held = null; diff --git a/tests/GameCult.Caching.Tests/CultCacheJournalTests.cs b/tests/GameCult.Caching.Tests/CultCacheJournalTests.cs index 96e04d5f..404a75cb 100644 --- a/tests/GameCult.Caching.Tests/CultCacheJournalTests.cs +++ b/tests/GameCult.Caching.Tests/CultCacheJournalTests.cs @@ -275,6 +275,107 @@ namespace GameCult.Caching.Tests Assert.Throws(() => Put(cache, "two"), "a throwing journal stays registered"); } + [Test] + public void ADisposeTheCacheRefusesLeavesTheJournalRegisteredAndALaterDisposeRemovesIt() + { + using var cache = Memory(); + var calls = 0; + IDisposable? registration = null; + registration = cache.AddJournal(_ => + { + calls++; + registration!.Dispose(); + }); + + var thrown = Assert.Throws(() => Put(cache, "one"))!; + Assert.That(thrown.Message, Does.Contain("journal"), "disposing from inside a journal is refused"); + + registration.Dispose(); + Put(cache, "two"); + + Assert.That(calls, Is.EqualTo(1), "the outside dispose removed it, though the first attempt threw"); + registration.Dispose(); + } + + [Test] + public void ALoadIsJournaledWithItsChanges() + { + var path = Path.Combine(_directory, "notes.cc"); + using (var writer = CultCacheMessagePack.Create(path, new CultCacheOpenOptions { Registry = Registry })) + { + Put(writer, "a"); + Put(writer, "b"); + } + + using var cache = Memory(); + var journaled = new List>>(); + using var journal = cache.AddJournal(changes => journaled.Add(changes)); + + cache.AddBackingStore(new SingleFileMessagePackBackingStore(path)); + + Assert.Multiple(() => + { + Assert.That(journaled, Has.Count.EqualTo(1), "one admission, one call"); + Assert.That(journaled[0].Select(change => change.Key).OrderBy(key => key.Value), + Is.EqualTo(new[] { KeyOf("a"), KeyOf("b") })); + }); + } + + // A store that admits a load and then fails: the hold's body throws after the journal has already failed. + private sealed class LoadThenFailStore : CacheBackingStore + { + public bool Armed; + + public override void PullAll() => Held(() => + { + if (!Armed) + return; + var record = new CultStoredDocument( + KeyOf("loaded"), "2026-01-01T00:00:00.0000000+00:00", Registry.GetRequired(typeof(JournalNote)), new JournalNote { Name = "loaded" }); + Loaded!(new[] { record }, Array.Empty()); + throw new InvalidOperationException("store failed after the load"); + }); + + public override void Push(CultStoredDocument entry) { } + public override void Delete(CultStoredDocument entry) { } + public override void PushAll() { } + public override CultCommitOutcome CommitBatch(CultCommitRequest request, bool wait) => CultCommitOutcome.Committed; + } + + [Test] + public void AHoldWhoseBodyThrowsSurfacesTheBodyFirstThenTheJournalFailures() + { + using var cache = Memory(); + var store = new LoadThenFailStore(); + cache.AddBackingStore(store); + using var a = cache.AddJournal(_ => throw new InvalidOperationException("journal a")); + using var b = cache.AddJournal(_ => throw new InvalidOperationException("journal b")); + store.Armed = true; + + var thrown = Assert.Throws(() => store.PullAll())!; + + Assert.Multiple(() => + { + Assert.That(thrown.InnerExceptions.Select(exception => exception.Message), + Is.EqualTo(new[] { "store failed after the load", "journal a", "journal b" })); + Assert.That(cache.Get(KeyOf("loaded")), Is.Not.Null, "the load landed"); + }); + } + + [Test] + public void AHoldWhoseBodyThrowsAloneRethrowsItAsItself() + { + using var cache = Memory(); + var store = new LoadThenFailStore(); + cache.AddBackingStore(store); + using var journal = cache.AddJournal(_ => { }); + store.Armed = true; + + var thrown = Assert.Throws(() => store.PullAll())!; + + Assert.That(thrown.Message, Is.EqualTo("store failed after the load")); + } + [Test] public void AJournalThatThrowsDuringALoadLeavesTheStoreAttachedAndTheLoadAdopted() {