From 079147af1bf6b91fddf971fff834c7e2fbd01486 Mon Sep 17 00:00:00 2001 From: Libby Date: Fri, 13 Nov 2026 03:02:15 +0000 Subject: [PATCH] IsWritable does not look at store paths: a link at a store path is the write's to refuse (is-writable-link-scope) The advisory query answers for the held record and the cache's judges in all four runtimes; the doc says so, and a consumer-side test per runtime drives a cache attached to a live link: the query answers true, the write is refused with the link error, nothing on disk changes. Co-Authored-By: Claude Sonnet 5.5 --- packages/cultcache-py/CHANGELOG.md | 1 + .../cultcache-py/src/cultcache_py/cache.py | 3 +- packages/cultcache-py/tests/test_cultcache.py | 26 ++++++++++++++++ packages/cultcache-rs/CHANGELOG.md | 4 +++ packages/cultcache-rs/src/lib.rs | 31 ++++++++++++++++++- packages/cultcache-ts/CHANGELOG.md | 4 +++ packages/cultcache-ts/src/cult-cache.ts | 3 +- packages/cultcache-ts/test/cult-cache.test.ts | 24 ++++++++++++++ src/GameCult.Caching/CultCache.cs | 3 +- .../StoreLinkRefusalTests.cs | 20 ++++++++++++ 10 files changed, 115 insertions(+), 4 deletions(-) diff --git a/packages/cultcache-py/CHANGELOG.md b/packages/cultcache-py/CHANGELOG.md index b185ba9f..e5a8b8e8 100644 --- a/packages/cultcache-py/CHANGELOG.md +++ b/packages/cultcache-py/CHANGELOG.md @@ -8,6 +8,7 @@ ### Changed +- `CultCache.is_writable` does not look at store paths: a link at a store path is not predicted, and the write refuses it with `StoreLinkedError`. Pinned by a test through a cache attached to a linked store. - `JsonLinesBackingStore` writes LF line endings on Windows. It wrote CRLF there, through `write_text` newline translation, before the commit_store_file change (#154, adbbb780). - `msgpack` is a hard dependency, as `pyproject.toml` declared it. The optional-import fallbacks and the `cultcache-py[msgpack]` extra they named (which never existed) are gone: a missing msgpack is an `ImportError` at import. - `JsonLinesBackingStore` writes through the same `commit_store_file` as the single-file store: a unique staged file, fsynced before the replace and removed when the replace fails, so a refused replace no longer leaves `.tmp` behind. It staged a fixed `.tmp` without an fsync. diff --git a/packages/cultcache-py/src/cultcache_py/cache.py b/packages/cultcache-py/src/cultcache_py/cache.py index 85b01d89..80a12706 100644 --- a/packages/cultcache-py/src/cultcache_py/cache.py +++ b/packages/cultcache-py/src/cultcache_py/cache.py @@ -636,7 +636,8 @@ class CultCache: a record held read-only (its payload carried slots past the declared ones, at any depth, and nothing consents to the loss, so writing it would drop data a newer writer stored), for a key a store carries foreign, and for a key held as another type's record this document does not own. Advisory (the write still decides): it answers at call time, and a key nothing holds is - writable.""" + writable. It does not look at store paths: a link at a store path is not predicted, and the write refuses it with + `StoreLinkedError`.""" self._assert_registered(document) try: self._refuse_at(document, key) diff --git a/packages/cultcache-py/tests/test_cultcache.py b/packages/cultcache-py/tests/test_cultcache.py index f83cfa96..6b95f638 100644 --- a/packages/cultcache-py/tests/test_cultcache.py +++ b/packages/cultcache-py/tests/test_cultcache.py @@ -1485,6 +1485,32 @@ class CultCacheTests(unittest.TestCase): self.assertEqual(sorted(p.name for p in Path(tmp).iterdir()), ["link.jsonl", "volume"]) self.assertEqual(sorted(p.name for p in volume.iterdir()), ["store.jsonl"]) + # The advisory query answers for the held record and the cache's judges; a link at the store path is the write's to refuse. + @unittest.skipIf(os.name != "posix", "symbolic links need POSIX here") + def test_is_writable_over_a_store_at_a_live_link_answers_true_while_the_cache_write_is_refused(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + volume = Path(tmp) / "volume" + volume.mkdir() + target = volume / "store.cc" + document = item_doc() + seed = CultCache.builder().register_document_type(document).add_generic_store(SingleFileMessagePackBackingStore(target)).build() + seed.put(document, "held", Item(name="Held", category="c", value=1)) + before = target.read_bytes() + link = Path(tmp) / "link.cc" + link.symlink_to(target) + cache = CultCache.builder().register_document_type(document).add_generic_store(SingleFileMessagePackBackingStore(link)).build() + cache.pull_all_backing_stores() + names_before = (sorted(p.name for p in Path(tmp).iterdir()), sorted(p.name for p in volume.iterdir())) + + self.assertTrue(cache.is_writable(document, "held")) + self.assertTrue(cache.is_writable(document, "fresh")) + with self.assertRaises(StoreLinkedError) as caught: + cache.put(document, "fresh", Item(name="Fresh", category="d", value=2)) + self.assertEqual(Path(caught.exception.path), link) + self.assertTrue(link.is_symlink()) + self.assertEqual(target.read_bytes(), before) + self.assertEqual((sorted(p.name for p in Path(tmp).iterdir()), sorted(p.name for p in volume.iterdir())), names_before) + def test_store_linked_error_is_an_os_error_carrying_the_path(self) -> None: error = StoreLinkedError(Path("some/store.cc")) self.assertIsInstance(error, OSError) diff --git a/packages/cultcache-rs/CHANGELOG.md b/packages/cultcache-rs/CHANGELOG.md index b76a30d5..3978e248 100644 --- a/packages/cultcache-rs/CHANGELOG.md +++ b/packages/cultcache-rs/CHANGELOG.md @@ -6,6 +6,10 @@ only in git history. ## Unreleased +### Changed + +- `CultCache::is_writable` does not look at store paths: a link at a store path is not predicted, and the write refuses it with `StoreLinkedError`. Pinned by a test through a cache attached to a linked store. + ### Breaking Semver: this is a breaking change in the 0.x line (0.3.x to 0.4.0). diff --git a/packages/cultcache-rs/src/lib.rs b/packages/cultcache-rs/src/lib.rs index d4d6a64e..90c09435 100644 --- a/packages/cultcache-rs/src/lib.rs +++ b/packages/cultcache-rs/src/lib.rs @@ -3309,7 +3309,8 @@ impl CultCache { /// Whether a write or removal of `(type_id, key)` would be admitted: false when the held payload carries slots past /// its registered type's declared slots, at any depth, or cannot be read by this build at all (the record is /// read-only), when the cache holds no image of its stores (see [`CultCacheLoadRefused`]), when a store carries a foreign record at the key, or when another registered type holds it. True for a - /// key nothing holds. An unregistered type is an error. Advisory: the write entries ask the same checks themselves. + /// key nothing holds. An unregistered type is an error. It does not look at store paths: a link at a store path is not + /// predicted, and the write refuses it with [`StoreLinkedError`]. Advisory: the write entries ask the same checks themselves. pub fn is_writable(&self, type_id: &str, key: &str) -> Result { if self.require_loaded().is_err() { return Ok(false); @@ -8225,6 +8226,34 @@ mod tests { Ok(()) } + // The advisory query is the held record's and the cache's judges; a link at the store path is the write's to refuse. + #[cfg(unix)] + #[test] + fn is_writable_over_a_store_at_a_live_link_answers_true_while_the_cache_write_is_refused() -> Result<()> { + let temp = tempfile::tempdir()?; + let target = temp.path().join("store.cc"); + store_with(&target, |cache| cache.put("held", &settings_one()).map(drop))?; + let before = fs::read(&target)?; + let link = temp.path().join("link.cc"); + std::os::unix::fs::symlink(&target, &link)?; + + let mut cache = CultCache::new(); + cache.register_registry(TestEntries)?; + cache.add_backing_store(SingleFileMessagePackBackingStore::new(&link), [Settings::TYPE])?; + cache.pull_all_backing_stores()?; + let names_before = dir_names(temp.path())?; + + assert!(cache.is_writable(Settings::TYPE, "held")?, "the query does not look at the store path"); + assert!(cache.is_writable(Settings::TYPE, "fresh")?, "the query does not look at the store path"); + let error = cache.put("fresh", &settings_one()).unwrap_err(); + let linked = error.downcast_ref::().unwrap_or_else(|| panic!("{error:#}")); + assert_eq!(linked.path, link); + assert!(fs::symlink_metadata(&link)?.file_type().is_symlink(), "the link is left as it was"); + assert_eq!(fs::read(&target)?, before); + assert_eq!(dir_names(temp.path())?, names_before, "the refused write left no lock or staging file"); + Ok(()) + } + #[cfg(unix)] #[test] fn a_write_at_a_live_directory_link_is_refused_as_a_link() -> Result<()> { diff --git a/packages/cultcache-ts/CHANGELOG.md b/packages/cultcache-ts/CHANGELOG.md index c1a91df6..5583b45c 100644 --- a/packages/cultcache-ts/CHANGELOG.md +++ b/packages/cultcache-ts/CHANGELOG.md @@ -5,6 +5,10 @@ All notable changes to `@gamecult/cultcache-ts` are recorded here, following ## [Unreleased] +### Changed + +- `CultCache.isWritable` does not look at store paths: a link at a store path is not predicted, and the write refuses it with `StoreLinkedError`. Pinned by a test through a cache attached to a linked store. + ### Breaking - A write at a single-file store path that is a symbolic link, live or dangling, is refused with the new `StoreLinkedError` (exported from the package entry; `error.path` names the path) before the lock and the replace, and writes nothing: `push`, `pushAll`, `delete` and a soft flush alike. Before, a write at a live link replaced the link with a regular file, a dangling link failed with `ENOENT` (or, for `pushAll`, was replaced), and a soft flush at a link skipped. A caller catching `ENOENT` around a write at a dangling link now catches `StoreLinkedError`; configure the link's target as the store path. Reads through a live link work. An `lstat` failure other than `ENOENT` (`ENOTDIR`, `EACCES`, `ELOOP`) is rethrown as it is. diff --git a/packages/cultcache-ts/src/cult-cache.ts b/packages/cultcache-ts/src/cult-cache.ts index a88bd66d..543356aa 100644 --- a/packages/cultcache-ts/src/cult-cache.ts +++ b/packages/cultcache-ts/src/cult-cache.ts @@ -615,7 +615,8 @@ export class CultCache { * another key, a record carrying unread slots nothing consents to). It answers the WRITE form. A removal asks the judge's * removal form and differs in two states: an owner's `delete` of a record held in another store than its own home succeeds * where this is false (a write would push to the wrong store), and `delete` of a global type at a key other than __global__ is - * the plain false of an unheld key where this is false. Live and advisory: the write or removal still decides. + * the plain false of an unheld key where this is false. It does not look at store paths: a link at a store path is not predicted, and + * the write refuses it with `StoreLinkedError`. Live and advisory: the write or removal still decides. */ isWritable(definition: AnyCultCacheDocumentDefinition, key: string): boolean { const registered = this.#requireDefinition(definition); diff --git a/packages/cultcache-ts/test/cult-cache.test.ts b/packages/cultcache-ts/test/cult-cache.test.ts index 0ea29c62..d3011ee2 100644 --- a/packages/cultcache-ts/test/cult-cache.test.ts +++ b/packages/cultcache-ts/test/cult-cache.test.ts @@ -2473,6 +2473,30 @@ test("SingleFileMessagePackBackingStore refuses a dangling symlink at its path a assert.deepEqual(await readdir(volume), []); }); +// The advisory query answers for the held record and the cache's judges; a link at the store path is the write's to refuse. +test("isWritable over a store at a live link answers true while the cache write is refused", { skip: process.platform === "win32" }, async () => { + const dir = await mkdtemp(join(tmpdir(), "cultcache-writable-link-")); + const volume = join(dir, "volume"); + await mkdir(volume); + const target = join(volume, "store.cc"); + const link = join(dir, "link.cc"); + const document = defineDocumentType({ type: "note", schema: z.object({ text: z.string() }) }); + await CultCache.builder().withDocumentType(document).withGenericStore(new SingleFileMessagePackBackingStore(target)).build() + .put(document, "held", { text: "held" }); + const before = await readFile(target); + await symlink(target, link); + const cache = CultCache.builder().withDocumentType(document).withGenericStore(new SingleFileMessagePackBackingStore(link)).build(); + await cache.pullAllBackingStores(); + const namesBefore = [(await readdir(dir)).sort(), (await readdir(volume)).sort()]; + + assert.equal(cache.isWritable(document, "held"), true); + assert.equal(cache.isWritable(document, "fresh"), true); + await assert.rejects(() => cache.put(document, "fresh", { text: "fresh" }), (error: unknown) => error instanceof StoreLinkedError && error.path === link); + assert.ok((await lstat(link)).isSymbolicLink()); + assert.deepEqual(await readFile(target), before); + assert.deepEqual([(await readdir(dir)).sort(), (await readdir(volume)).sort()], namesBefore); +}); + // The second look of the read, staged: nothing is at the path only when the read and the lstat both find nothing. A path // that changed between them (a link in a parent swapped for a file, a store that appeared) is never an empty store. test("readStoreBytes reports nothing at the path only when the lstat finds nothing too", async () => { diff --git a/src/GameCult.Caching/CultCache.cs b/src/GameCult.Caching/CultCache.cs index 3af8ba70..469fc7af 100644 --- a/src/GameCult.Caching/CultCache.cs +++ b/src/GameCult.Caching/CultCache.cs @@ -2676,7 +2676,8 @@ namespace GameCult.Caching /// when that store holds it unowned (undeclared drift that drops persisted slots, or a payload carrying slots past this build's, /// see ). For a key the cache does not hold, the removal check every store makes of it /// (false when a store lists it foreign). One write it does not predict: a Flatten of a variant whose base chain holds an unowned record is refused, though - /// the variant itself is writable. Live and advisory: the write still decides. + /// the variant itself is writable. It does not look at store paths: a link at a store path is not predicted, and the write refuses it + /// with . Live and advisory: the write still decides. /// public bool IsWritable(CultRecordKey key) { diff --git a/tests/GameCult.Caching.Tests/StoreLinkRefusalTests.cs b/tests/GameCult.Caching.Tests/StoreLinkRefusalTests.cs index 4444b176..64536b77 100644 --- a/tests/GameCult.Caching.Tests/StoreLinkRefusalTests.cs +++ b/tests/GameCult.Caching.Tests/StoreLinkRefusalTests.cs @@ -170,6 +170,26 @@ namespace GameCult.Caching.Tests Assert.That(IsALink(link), "the link was replaced"); } + [Test] + public void AnAdvisoryQueryOverACacheAtALiveLinkAnswersForTheRecordAndTheWriteIsStillRefused() + { + var path = PathOf("store.cc"); + var link = PathOf("link.cc"); + SeedSingle(path); + Link(link, path); + var before = File.ReadAllBytes(path); + using var cache = CultCacheMessagePack.Create(link, new CultCacheOpenOptions { Registry = Registry, FlushOnDispose = false, StoreFlushOnDispose = false }); + + Assert.That(cache.IsWritable(new CultRecordKey("seed")), Is.True, "the query does not look at the store path"); + Assert.That(cache.IsWritable(new CultRecordKey("fresh")), Is.True, "the query does not look at the store path"); + var refusal = Assert.Throws(() => + cache.Commit(batch => batch.Upsert(typeof(IdDeck), new IdDeck { Name = "t" }, new CultRecordKey("fresh"))))!; + + Assert.That(refusal.Path, Is.EqualTo(link)); + Assert.That(IsALink(link), "the link was replaced"); + Assert.That(File.ReadAllBytes(path), Is.EqualTo(before)); + } + // A batch that upserts into one store and removes a key the cache does not hold has the other store judge that removal // under its own lock (JudgeChecks). private CultCache OpenTwoStores(string homePath, string otherPath)