From 05986a9f06b783094a1844743f8e7d0466d1a95a Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Tue, 22 Sep 2026 14:10:45 +0000 Subject: [PATCH] fix: a bare store's own absolute keys break once it is instance-wrapped #2 resolves every aget/asetitem key through wrapped_self's wrapper chain, but wrapped_self cannot tell a call reached through a wrapper from a direct call on the inner store. After wrap_kvs(bare, ...), bare.aget(abs_path) went through the wrapper's id_of_key again and read rootdir+rootdir+name. Keys the store already accepts are now taken verbatim. Also pin dol>=0.3.50, the first release with wrapped_self (an older installed dol made import fail). Co-Authored-By: Claude Opus 5 --- aiofiledol/__init__.py | 10 +++++ aiofiledol/tests/test_relative_path_keys.py | 44 +++++++++++++++++++++ pyproject.toml | 2 +- 3 files changed, 55 insertions(+), 1 deletion(-) diff --git a/aiofiledol/__init__.py b/aiofiledol/__init__.py index ea20f42..7ea1288 100644 --- a/aiofiledol/__init__.py +++ b/aiofiledol/__init__.py @@ -36,7 +36,17 @@ def _resolve_key(store, k): This REPLACES ``store._id_of_key(k)`` -- it walks the whole chain including the leaf, so the two must never be composed. For an un-wrapped store no layer defines ``_id_of_key`` and ``k`` is returned verbatim. + + A key the store itself already accepts (a path under its ``rootdir``) is also + taken verbatim. ``wrapped_self`` cannot tell whether a method was reached through + a wrapper or called on the inner store directly, so without this guard merely + WRAPPING an instance -- ``wrap_kvs(bare_store, ...)`` -- would make that instance's + own absolute-key ``aget``/``asetitem`` push the key through the wrapper's + ``id_of_key`` a second time (``rootdir + rootdir + name``). Relative keys never + match an absolute ``rootdir`` pattern, so they still get resolved. """ + if store.is_valid_key(k): + return k return inner_most_key(wrapped_self(store), k, default=k) diff --git a/aiofiledol/tests/test_relative_path_keys.py b/aiofiledol/tests/test_relative_path_keys.py index 5946d75..9ceb7f0 100644 --- a/aiofiledol/tests/test_relative_path_keys.py +++ b/aiofiledol/tests/test_relative_path_keys.py @@ -73,3 +73,47 @@ def test_bare_reader_still_takes_absolute_keys(tmp_path, monkeypatch): s = AioFileBytesReader(rootdir, max_levels=0) assert asyncio.run(s.aget(os.path.join(rootdir, KEY))) == REAL + + +def test_bare_store_keeps_absolute_keys_after_being_instance_wrapped( + tmp_path, monkeypatch +): + """Wrapping an instance must not change what that instance's own methods do. + + ``wrapped_self`` climbs from the bare store to any live wrapper of it, so the + bare store's absolute key used to be pushed through the wrapper's ``id_of_key`` + too, reading ``rootdir + rootdir + name``. + """ + from dol import wrap_kvs + + rootdir = _store_dir(tmp_path, "realstore", REAL) + monkeypatch.chdir(_store_dir(tmp_path, "elsewhere")) + + bare = AioFileBytesReader(rootdir, max_levels=0) + wrapped = wrap_kvs( + bare, + id_of_key=lambda k: rootdir + k, + key_of_id=lambda _id: _id[len(rootdir) :], + ) + + assert asyncio.run(wrapped.aget(KEY)) == REAL # through the wrapper + assert asyncio.run(bare.aget(os.path.join(rootdir, KEY))) == REAL # directly + + +def test_bare_persister_writes_where_told_after_being_instance_wrapped(tmp_path): + from dol import wrap_kvs + + from aiofiledol import AioFileBytesPersister + + rootdir = _store_dir(tmp_path, "realstore") + bare = AioFileBytesPersister(rootdir, max_levels=0) + _wrapped = wrap_kvs( + bare, + id_of_key=lambda k: rootdir + k, + key_of_id=lambda _id: _id[len(rootdir) :], + ) + + target = os.path.join(rootdir, KEY) + asyncio.run(bare.asetitem(target, REAL)) + with open(target, "rb") as fp: + assert fp.read() == REAL diff --git a/pyproject.toml b/pyproject.toml index b19876c..976a71e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -33,7 +33,7 @@ classifiers = [ "Framework :: AsyncIO", ] dependencies = [ - "dol", + "dol>=0.3.50", # wrapped_self (dol#18) first shipped in 0.3.50 "aiofile", ]