IsWritable does not look at store paths: a link at a store path is the write's to refuse (is-writable-link-scope)
All checks were successful
CultMesh portability and performance / ubuntu-latest reactive semantics and leases (pull_request) Successful in 26m44s
All checks were successful
CultMesh portability and performance / ubuntu-latest reactive semantics and leases (pull_request) Successful in 26m44s
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 <noreply@anthropic.com>
This commit is contained in:
parent
f501c7ee60
commit
079147af1b
10 changed files with 115 additions and 4 deletions
|
|
@ -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 `<path>.tmp` behind. It staged a fixed `<path>.tmp` without an fsync.
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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).
|
||||
|
|
|
|||
|
|
@ -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<bool> {
|
||||
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::<StoreLinkedError>().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<()> {
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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 () => {
|
||||
|
|
|
|||
|
|
@ -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 <see cref="CultStoredDocument.UnreadSlots"/>). 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 <see cref="CultStoreLinkedException"/>. Live and advisory: the write still decides.
|
||||
/// </summary>
|
||||
public bool IsWritable(CultRecordKey key)
|
||||
{
|
||||
|
|
|
|||
|
|
@ -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<CultStoreLinkedException>(() =>
|
||||
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)
|
||||
|
|
|
|||
Loading…
Reference in a new issue