From 80eed16c0d1c00dfa899d7ccab4f3e57c3fbcb46 Mon Sep 17 00:00:00 2001 From: Libby Date: Thu, 12 Nov 2026 23:13:57 +0000 Subject: [PATCH] Soft flush: the reader's own answer decides (no second stat, protocol default included); pin the readable-empty store, spy the flag through Mesh and host, changelog the breaking PushAll Co-Authored-By: Claude Sonnet 5.5 --- packages/cultcache-py/CHANGELOG.md | 1 + .../src/cultcache_py/backing_store.py | 9 +-- .../cultcache-py/src/cultcache_py/stores.py | 23 ++++--- .../cultcache-py/tests/test_soft_push_all.py | 39 ++++++++++-- .../DirectoryMessagePackBackingStore.cs | 8 ++- .../BackingStoreTests.cs | 30 ++++++++++ .../DirectoryStoreDurabilityTests.cs | 18 ++++++ .../NetworkingTests.cs | 60 +++++++++++++++++++ unity/org.gamecult.cultlib/CHANGELOG.md | 1 + 9 files changed, 170 insertions(+), 19 deletions(-) diff --git a/packages/cultcache-py/CHANGELOG.md b/packages/cultcache-py/CHANGELOG.md index ea3f72fd..e44c2430 100644 --- a/packages/cultcache-py/CHANGELOG.md +++ b/packages/cultcache-py/CHANGELOG.md @@ -13,6 +13,7 @@ ### Breaking +- `push_all(envelopes, soft=False)` on the single-file and JSONL stores and on the `BackingStore` protocol takes the soft flag, as TypeScript, Rust and C# do: a soft push writes only where nothing is at the path; over a store the reader reads, empty included, it returns without writing, and over a file the reader cannot read it refuses (`StoreUnreadableError` for the single-file store). The `BackingStore` protocol's default `push_all` cannot tell an absent store from an empty one (a pull returns a list), so a soft push through it always skips a store whose pull returns; a store kind that can tell nothing-at-the-path overrides `push_all`. A store kind that overrides `push_all(envelopes)` without the keyword fails when a caller passes `soft`. - A record whose payload carries a slot past its reader's declared slots, at any depth (the document, a nested record, a list element, a union arm, a unit arm), is read, held read-only, and every write or removal of its key (`put`, `put_global`, `update`, `put_envelope`, `put_envelopes`, `delete`, `delete_global`) is refused with `SchemaConflictError` before any store is touched (`cultlib-gaps:ruling:extra-trailing-slots`). Reading dropped those slots silently and the next write shed them. `SchemaConflictError.unread_slots` is an `UnreadSlots(type_name, slot_count, declared_count, occurrences, depth0_only)` and the message names the type and both counts, never a slot value. A depth-0 mark is consented, and the record writable, only when the record is stored under an id the document lists in `compatible_schema_ids` and that id's catalog entry has slots past the document's, covering the mark's slot count (`cultlib-gaps:ruling:slot-mark-consent`); a nested mark, or a mark under the document's own id, keeps the record read-only whatever is declared. Missing trailing slots keep their declared defaults. An untouched marked record keeps its stored bytes through every other write. Added `CultCache.is_writable(document, key)` (advisory; the write still decides: it also answers False for a foreign-carried key, for a key held as a type the document does not own, and for a global document at a non-global key; `delete` asks the same judges, so removing a key held as another type is refused with `SchemaConflictError` unless the document owns that record's schema id (its own or one it lists compatible), in which case the removal removes that record, its file row and its lookups, as a write of the key would replace it; it returned silently and removed nothing), `DocumentDefinition.decode_payload_marked`, and `UnreadSlots`. A document with a custom `decode` (no slot codec) never marks. A store holding a variant record is still unreadable here. Implies a minor bump while the version is 0.x (0.3.0 to 0.4.0): code that wrote back a record from a newer writer now gets `SchemaConflictError`. - A payload is one MessagePack value: `DocumentDefinition.decode_payload` refuses bytes after it, by count, with `ValueError` ` payload has N bytes after its first MessagePack value (refused on read).` (never the bytes, never the depth refusal; a payload that ends early stays the decoder's), as the C# reference does (`tests/vectors/union-parity/bytes-after-outcomes.msgpack`). It raised `msgpack.ExtraData` (a `ValueError`, so only a caller naming `msgpack.ExtraData` is affected); this is a minor-version bump while pre-1.0. The refused set is unchanged. Cache loads, `get` and every definition built on the MessagePack decoder go through it; a user `payload_decoder` that raises `ExtraData` is refused the same way. Depth is judged first, so a value nested past 256 with a tail is the depth refusal, as in C#. - A cache holds one record per key whatever its type (`variants:ruling:cache-refuses-retype`). A `put`, `put_envelope`, `put_envelopes` or `put_global` of a second type under a key the cache holds, or under a key a store carries foreign, is refused with `SchemaConflictError` naming the key, even with no store, and changes nothing; retype a key by deleting it, then putting the new type. A write may replace another registered type's record only when the written type owns its schema id (declares it or lists it compatible) and both types share one home store. A load that reads one key from two stores, whatever the types, is refused with `CultCacheError` naming the key and both types, and admits nothing. A store that holds a record no registered type claims now loads and is carried: the cache does not serve it, the store keeps its bytes through every write, `delete` or `put` at its key is refused with `SchemaConflictError`, and registering a type that declares its schema id claims it at the next load. It raised `CultCacheError` "Unknown persisted document type". Two global types share the key `__global__`, so a second global type in one cache is now refused, and a store that already holds two global types at `__global__` (a JSONL store written by caches that never pulled could produce one) no longer loads: the pull raises `CultCacheError` and admits nothing. C# refuses a second persisted global on pull the same way (`cultcache-store-composition.md`, "Globals never invented"); repair such a file by deleting one of the two records with the runtime that wrote it. diff --git a/packages/cultcache-py/src/cultcache_py/backing_store.py b/packages/cultcache-py/src/cultcache_py/backing_store.py index 7a93bbd0..7b07e7be 100644 --- a/packages/cultcache-py/src/cultcache_py/backing_store.py +++ b/packages/cultcache-py/src/cultcache_py/backing_store.py @@ -68,11 +68,12 @@ class BackingStore(Protocol): ... def push_all(self, envelopes: list[CultCacheEnvelope], soft: bool = False) -> None: - existing = { - (envelope.type, envelope.key): envelope for envelope in self.pull_all() - } - if soft and existing: + # A pull that returns is a store the reader read, empty included: the protocol cannot say "absent", so a soft push + # through this default always skips. A store that can tell nothing-at-the-path overrides push_all. + pulled = self.pull_all() + if soft: return + existing = {(envelope.type, envelope.key): envelope for envelope in pulled} for envelope in envelopes: existing[(envelope.type, envelope.key)] = envelope self._replace_all(list(existing.values())) diff --git a/packages/cultcache-py/src/cultcache_py/stores.py b/packages/cultcache-py/src/cultcache_py/stores.py index 35b53af3..2be88200 100644 --- a/packages/cultcache-py/src/cultcache_py/stores.py +++ b/packages/cultcache-py/src/cultcache_py/stores.py @@ -76,10 +76,14 @@ class JsonLinesBackingStore: self._lock = threading.RLock() def pull_all(self) -> list[CultCacheEnvelope]: + return self._pull() or [] + + def _pull(self) -> list[CultCacheEnvelope] | None: + """The envelopes the reader reads, or None when nothing is at the path.""" with self._lock: text = _read_store(self.path, lambda path: path.read_text(encoding="utf-8")) if text is None: - return [] + return None envelopes: list[CultCacheEnvelope] = [] for line_number, line in enumerate(text.splitlines(), start=1): if not line.strip(): @@ -108,9 +112,10 @@ class JsonLinesBackingStore: def push_all(self, envelopes: list[CultCacheEnvelope], soft: bool = False) -> None: with self._lock: - existing = {(item.type, item.key): item for item in self.pull_all()} - if soft and self.path.exists(): + arrived = self._pull() + if soft and arrived is not None: return + existing = {(item.type, item.key): item for item in arrived or []} for envelope in envelopes: existing[(envelope.type, envelope.key)] = envelope self._replace_all(list(existing.values())) @@ -144,13 +149,17 @@ class SingleFileMessagePackBackingStore: self._format = STORE_FORMAT_VERSION def pull_all(self) -> list[CultCacheEnvelope]: + return self._pull() or [] + + def _pull(self) -> list[CultCacheEnvelope] | None: + """The envelopes the reader reads, or None when nothing is at the path.""" with self._lock: # An empty file is not a store (no writer leaves one), so it is decoded and refused. # The disk decides the header: a file that is gone is not marked. self._format = STORE_FORMAT_VERSION data = _read_store(self.path, Path.read_bytes) if data is None: - return [] + return None try: decoded = msgpack.unpackb(data, raw=False) snapshot = _decode_snapshot(decoded) @@ -169,10 +178,10 @@ class SingleFileMessagePackBackingStore: """A soft push writes only where nothing is at the path: a store the reader reads is left byte for byte, and a file it cannot read is refused by the same read, as in TS and Rust.""" with self._write_transaction(): - arrived = self.pull_all() - if soft and self.path.exists(): + arrived = self._pull() + if soft and arrived is not None: return - self._replace_all(list(envelopes), arrived) + self._replace_all(list(envelopes), arrived or []) def delete(self, type: str, key: str) -> None: with self._write_transaction(): diff --git a/packages/cultcache-py/tests/test_soft_push_all.py b/packages/cultcache-py/tests/test_soft_push_all.py index 4bf8d4c1..9a2c6e03 100644 --- a/packages/cultcache-py/tests/test_soft_push_all.py +++ b/packages/cultcache-py/tests/test_soft_push_all.py @@ -34,6 +34,20 @@ class SoftPushAllTests(unittest.TestCase): store.push_all([envelope("b")]) # a plain push still lands what the caller holds self.assertEqual(["a", "b"], sorted(item.key for item in make(path).pull_all())) + def test_over_a_readable_store_holding_no_records_it_leaves_the_file_byte_for_byte(self) -> None: + for make in (SingleFileMessagePackBackingStore, JsonLinesBackingStore): + with self.subTest(make.__name__): + path = self.dir / make.__name__ + store = make(path) + store.push_all([envelope("a")]) + store.delete("t", "a") + self.assertTrue(path.exists()) + self.assertEqual([], make(path).pull_all()) + before = path.read_bytes() + store.push_all([envelope("b")], soft=True) + self.assertEqual(before, path.read_bytes()) + self.assertEqual([], make(path).pull_all()) + def test_over_an_absent_path_it_writes(self) -> None: for make in (SingleFileMessagePackBackingStore, JsonLinesBackingStore): with self.subTest(make.__name__): @@ -55,7 +69,7 @@ class SoftPushAllTests(unittest.TestCase): JsonLinesBackingStore(self.path).push_all([envelope("a")], soft=True) self.assertEqual(junk, self.path.read_bytes()) - def test_the_protocol_default_push_all_has_the_same_three_branches(self) -> None: + def test_the_protocol_default_push_all_skips_what_its_pull_read_empty_included(self) -> None: class Memory(BackingStore): def __init__(self) -> None: self.items: list[CultCacheEnvelope] = [] @@ -67,13 +81,28 @@ class SoftPushAllTests(unittest.TestCase): self.items = list(envelopes) store = Memory() - store.push_all([envelope("a")], soft=True) # nothing there: writes + store.push_all([envelope("a")], soft=True) # the pull read an empty store: skips + self.assertEqual([], store.items) + store.push_all([envelope("a")]) # plain: lands + store.push_all([envelope("b")], soft=True) # a store with records: skips self.assertEqual(["a"], [item.key for item in store.items]) - store.push_all([envelope("b")], soft=True) # something there: skips - self.assertEqual(["a"], [item.key for item in store.items]) - store.push_all([envelope("b")]) # plain: lands + store.push_all([envelope("b")]) self.assertEqual(["a", "b"], sorted(item.key for item in store.items)) + def test_the_protocol_default_push_all_refuses_where_its_pull_refuses(self) -> None: + class Unreadable(BackingStore): + replaced = False + + def pull_all(self) -> list[CultCacheEnvelope]: + raise StoreUnreadableError(Path("x"), ValueError("junk")) + + def _replace_all(self, envelopes: list[CultCacheEnvelope]) -> None: + Unreadable.replaced = True + + with self.assertRaises(StoreUnreadableError): + Unreadable().push_all([envelope("a")], soft=True) + self.assertFalse(Unreadable.replaced) + if __name__ == "__main__": unittest.main() diff --git a/src/GameCult.Caching.MessagePack/DirectoryMessagePackBackingStore.cs b/src/GameCult.Caching.MessagePack/DirectoryMessagePackBackingStore.cs index d16907b6..5005b995 100644 --- a/src/GameCult.Caching.MessagePack/DirectoryMessagePackBackingStore.cs +++ b/src/GameCult.Caching.MessagePack/DirectoryMessagePackBackingStore.cs @@ -247,9 +247,11 @@ public sealed class DirectoryMessagePackBackingStore : CacheBackingStore Directory.CreateDirectory(_manifestFile.DirectoryName!); using var commitLease = AcquireCommitLease(wait: true, create: true); - var (manifest, catalogBytes) = ReadManifest(); - // A soft flush writes only where no manifest is; a manifest ReadManifest read stays as it is, its staged writes staged. - if (soft && File.Exists(_manifestFile.FullName)) + var manifestBytes = ReadManifestBytes(); + var (manifest, catalogBytes) = ParseManifest(manifestBytes); + // A soft flush writes only where no manifest is: the reader's answer (null is absent), not a second look at the path. + // A manifest it read stays as it is, its staged writes staged. + if (soft && manifestBytes != null) return; RefuseUndeclaredOnDisk(manifest, StagedKeys, Array.Empty(), Array.Empty(), Array.Empty()); if (IsDirty) diff --git a/tests/GameCult.Caching.Tests/BackingStoreTests.cs b/tests/GameCult.Caching.Tests/BackingStoreTests.cs index e6cd5dc8..8d5c3424 100644 --- a/tests/GameCult.Caching.Tests/BackingStoreTests.cs +++ b/tests/GameCult.Caching.Tests/BackingStoreTests.cs @@ -87,6 +87,36 @@ namespace GameCult.Caching.Tests } } + [Test] + public async Task SoftFlush_Over_Readable_Store_Holding_No_Records_Leaves_It() + { + var filePath = Path.Combine(Path.GetTempPath(), $"cultlib-tests-{Guid.NewGuid():N}.msgpack"); + try + { + var store = new SingleFileMessagePackBackingStore(filePath); + var cache = new CultCache(); + cache.AddBackingStore(store); + var first = await cache.AddAsync(new NamedTestEntry { Name = "first", Value = "one" }); + await cache.FlushAsync(); + cache.Remove(first.Key); + await cache.FlushAsync(); + var before = File.ReadAllBytes(filePath); + var empty = new CultCache(); + empty.AddBackingStore(new SingleFileMessagePackBackingStore(filePath)); + Assert.That(empty.AllEntries, Is.Empty, "the store is not readable-empty"); + + await cache.AddAsync(new NamedTestEntry { Name = "second", Value = "two" }); + await cache.FlushAsync(soft: true); + + Assert.That(File.ReadAllBytes(filePath), Is.EqualTo(before), "a soft flush wrote over a readable store that holds no records"); + Assert.That(store.IsDirty, Is.True); + } + finally + { + if (File.Exists(filePath)) File.Delete(filePath); + } + } + [Test] public async Task PlainPushAll_Over_Readable_Store_Writes_The_Staged_Records() { diff --git a/tests/GameCult.Caching.Tests/DirectoryStoreDurabilityTests.cs b/tests/GameCult.Caching.Tests/DirectoryStoreDurabilityTests.cs index 41af7c1b..942ae891 100644 --- a/tests/GameCult.Caching.Tests/DirectoryStoreDurabilityTests.cs +++ b/tests/GameCult.Caching.Tests/DirectoryStoreDurabilityTests.cs @@ -243,6 +243,24 @@ namespace GameCult.Caching.Tests Assert.That(reader.Get(second)?.Text, Is.EqualTo("two")); } + [Test] + public async Task SoftFlushOverReadableEmptyManifestLeavesIt() + { + using var cache = Open(out var store); + var key = await cache.UpsertAsync(typeof(Page), new Page { Name = "a", Text = "one" }); + cache.FlushAllBackingStores(); + cache.Remove(key); + cache.FlushAllBackingStores(); + var before = File.ReadAllBytes(_manifest); + using (var reader = Open(out _)) Assert.That(reader.AllEntries, Is.Empty, "the store is not readable-empty"); + await cache.UpsertAsync(typeof(Page), new Page { Name = "b", Text = "two" }); + + cache.FlushAllBackingStores(soft: true); + + Assert.That(File.ReadAllBytes(_manifest), Is.EqualTo(before), "a soft flush wrote over a readable store that holds no records"); + Assert.That(store.IsDirty, Is.True); + } + [Test] public async Task SoftFlushOverUnreadableManifestRefusesAndLeavesIt() { diff --git a/tests/GameCult.Networking.Tests/NetworkingTests.cs b/tests/GameCult.Networking.Tests/NetworkingTests.cs index 09764518..b566dca3 100644 --- a/tests/GameCult.Networking.Tests/NetworkingTests.cs +++ b/tests/GameCult.Networking.Tests/NetworkingTests.cs @@ -7377,6 +7377,66 @@ namespace GameCult.Networking.Tests Assert.That(developmentServer.LogSensitivePayloads, Is.True); } + // Records the flag each PushAll took, and holds nothing: a flush that decided soft itself and never asked the store records nothing. + private sealed class SoftFlagSpyStore : CacheBackingStore + { + public sealed class SpyHome { } + + public readonly List Flags = new(); + public void Stage() => IsDirty = true; + public override void PullAll() { } + public override void Push(CultStoredDocument entry) { } + public override void Delete(CultStoredDocument entry) { } + public override void PushAll(bool soft) => Flags.Add(soft); + public override CultCommitOutcome CommitBatch(CultCommitRequest request, bool wait) => CultCommitOutcome.Committed; + } + + [Test] + public async Task CultNetLocal_Host_FlushAsync_Hands_The_Store_The_Flag_It_Was_Given() + { + var filePath = Path.Combine(Path.GetTempPath(), $"cultnet-host-spy-{Guid.NewGuid():N}.msgpack"); + CultNetHost? host = null; + try + { + host = await CultNetLocal.CreateHostAsync(filePath, new CultNetHostOptions { StartServer = false }); + var spy = new SoftFlagSpyStore(); + host.Cache.AddBackingStore(spy, typeof(SoftFlagSpyStore.SpyHome)); + spy.Stage(); + + await host.FlushAsync(soft: true); + await host.FlushAsync(); + + Assert.That(spy.Flags, Is.EqualTo(new[] { true, false })); + } + finally + { + host?.Dispose(); + if (File.Exists(filePath)) File.Delete(filePath); + } + } + + [Test] + public async Task CultMeshNode_FlushAsync_Hands_The_Store_The_Flag_It_Was_Given() + { + var rootPath = Path.Combine(TestContext.CurrentContext.WorkDirectory, "cultmesh-soft-spy", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(rootPath); + var documents = new CultNetDocumentRegistry() + .Register(CultNetDocumentBinding.ForDocument()); + using var node = await CultMesh.CreateNodeAsync(Path.Combine(rootPath, "world.ccmp"), new CultMeshNodeOptions + { + StartServer = false, + DatabaseOptions = new CultNetDatabaseOptions { RuntimeId = "soft-spy-local", DocumentRegistry = documents } + }); + var spy = new SoftFlagSpyStore(); + node.Cache.AddBackingStore(spy, typeof(SoftFlagSpyStore.SpyHome)); + spy.Stage(); + + await node.FlushAsync(soft: true); + await node.FlushAsync(); + + Assert.That(spy.Flags, Is.EqualTo(new[] { true, false })); + } + [Test] public async Task CultNetLocal_Host_SoftFlush_Passes_The_Flag_To_The_Store() { diff --git a/unity/org.gamecult.cultlib/CHANGELOG.md b/unity/org.gamecult.cultlib/CHANGELOG.md index a4a2bc11..612aee04 100644 --- a/unity/org.gamecult.cultlib/CHANGELOG.md +++ b/unity/org.gamecult.cultlib/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to this package are documented in this file. ### Breaking +- `CacheBackingStore.PushAll()` is no longer abstract: it is a non-virtual convenience over the new abstract `PushAll(bool soft)`. A store kind that overrides `PushAll()` no longer compiles; override `PushAll(bool soft)` instead. A soft flush writes only where nothing is at the path: over a store the reader reads, empty included, it returns without writing and leaves the staged writes staged and dirty for a later plain flush; over a file the reader cannot read it refuses. `CultCache.FlushAsync(bool soft = false)`, `CultCache.FlushAllBackingStores(bool soft = false)`, `CultMeshNode.FlushAsync(soft)` and `CultNetHost.FlushAsync(soft)` pass the flag to the stores; `soft: true` was accepted and ignored before, so a caller that passed it now stops landing writes on an existing store. Semver: breaking for C# consumers with their own store kind. - A document payload with bytes after its first MessagePack value is refused on read and on write (`MessagePackSerializationException` naming the type and the count of bytes after the value, never the bytes); the tail is not walked, so a tail nested past the limit is refused as bytes after the value, not as depth. Before, the tail was ignored on read and dropped on the next write. A payload that ends early is still the deserializer's to refuse. A CDN manifest is a record payload and follows the same rule: the acceptance vectors `trailing_nil`, `trailing_reserved_code` and `trailing_truncated_array` in `contracts/cultmesh/content-acceptance-vectors.json` are now refusals. `CultMeshProviderSessionWire.DecodeCommandDocument` refuses a command record whose payload has bytes after its first value (`MessagePackSerializationException` with the count); before, the tail was ignored. Semver: breaking for C# consumers, a major bump. - A single-file store holding two records under one key is unreadable: the reader refuses it with `CultStoreUnreadableException` naming the key (keys compare by ordinal code units, no case folding or normalisation), so a cache, `ReadStoreSnapshot` and a flush refuse the file and leave it as it is. Before, the later record silently replaced the earlier. A directory store reads its manifest through the same reader and refuses a duplicate key the same way. Repair the duplicate with the runtime that wrote it before reading the store here. - A record whose payload carries a slot past the slots its reading type declares, at any depth (the record, a nested object, a list element, a union arm or a unit arm), is loaded read-only: it is served, and every staged write or removal of its key (`Upsert`, `UpsertVariant`, `Remove`, a batch, and `Flatten` of a variant) is refused with `CultSchemaConflictException` before anything is staged, and the store keeps the record byte for byte. Before, the extra slots were skipped on read and dropped on the next write. Declaring an id compatible consents only to depth-0 slots the schema comparison reports ignored under that id; any other extra slot keeps the record read-only whatever is declared, including under the reader's own id. A variant overriding a slot its type does not have is read-only too (it used to be shed on flush). Write such a record from the build that declares those slots. Missing trailing slots still default and stay writable.