From d664550d843ba559f924479cb6d153861ee9f5cb Mon Sep 17 00:00:00 2001 From: mohammadZuherJaserAsad Date: Sun, 21 Jun 2026 23:46:38 -0500 Subject: [PATCH 1/6] fix: implement soft-delete for ZipStore (issue #828) Implement soft-delete via b"" overwrite. supports_deletes=True. Update _get, exists, list, delete, delete_dir. --- src/zarr/storage/_zip.py | 101 ++++++++++++++++++++++++++------------- 1 file changed, 67 insertions(+), 34 deletions(-) diff --git a/src/zarr/storage/_zip.py b/src/zarr/storage/_zip.py index 897797e999..6824f085ad 100644 --- a/src/zarr/storage/_zip.py +++ b/src/zarr/storage/_zip.py @@ -22,6 +22,11 @@ ZipStoreAccessModeLiteral = Literal["r", "w", "a"] +# Sentinel value written to a zip entry to mark it as soft-deleted. +# The ZIP format does not support native deletion, so we overwrite the entry +# with this value and treat it as absent in all read/list operations. +_SOFT_DELETE_SENTINEL = b"" + class ZipStore(Store): """ @@ -52,10 +57,18 @@ class ZipStore(Store): path compression allowZip64 + + Notes + ----- + Deletion is implemented as a soft-delete: the zip entry is overwritten with + an empty byte string (b""). All read, exists, and list operations + filter out soft-deleted entries so they appear absent to callers. Because + the ZIP format does not allow removing entries, soft-deleted entries remain + on disk but are invisible through the store API. """ supports_writes: bool = True - supports_deletes: bool = False + supports_deletes: bool = True # soft-delete via empty-byte overwrite supports_listing: bool = True path: Path @@ -153,23 +166,26 @@ def _get( self._sync_open() # docstring inherited try: - with self._zf.open(key) as f: # will raise KeyError - if byte_range is None: - return prototype.buffer.from_bytes(f.read()) - elif isinstance(byte_range, RangeByteRequest): - f.seek(byte_range.start) - return prototype.buffer.from_bytes(f.read(byte_range.end - f.tell())) - size = f.seek(0, os.SEEK_END) - if isinstance(byte_range, OffsetByteRequest): - f.seek(byte_range.offset) - elif isinstance(byte_range, SuffixByteRequest): - f.seek(max(0, size - byte_range.suffix)) - else: - raise TypeError(f"Unexpected byte_range, got {byte_range}.") - return prototype.buffer.from_bytes(f.read()) + with self._zf.open(key) as f: + data = f.read() except KeyError: return None + # Treat soft-deleted entries (empty bytes) as missing + if data == _SOFT_DELETE_SENTINEL: + return None + + if byte_range is None: + return prototype.buffer.from_bytes(data) + elif isinstance(byte_range, RangeByteRequest): + return prototype.buffer.from_bytes(data[byte_range.start : byte_range.end]) + elif isinstance(byte_range, OffsetByteRequest): + return prototype.buffer.from_bytes(data[byte_range.offset :]) + elif isinstance(byte_range, SuffixByteRequest): + return prototype.buffer.from_bytes(data[max(0, len(data) - byte_range.suffix) :]) + else: + raise TypeError(f"Unexpected byte_range, got {byte_range}.") + async def get( self, key: str, @@ -227,21 +243,31 @@ async def set_if_not_exists(self, key: str, value: Buffer) -> None: if key not in members: self._set(key, value) - async def delete_dir(self, prefix: str) -> None: - # only raise NotImplementedError if any keys are found + async def delete(self, key: str) -> None: + # docstring inherited + # Soft-delete: overwrite the entry with an empty byte sentinel. + # The ZIP format has no native delete API, so we mark the entry as + # deleted by writing b"" and filtering it out in all read/list paths. + # If the key does not exist in the archive, this is a no-op. self._check_writable() - if prefix != "" and not prefix.endswith("/"): - prefix += "/" - async for _ in self.list_prefix(prefix): - raise NotImplementedError + with self._lock: + if key in self._zf.namelist(): + keyinfo = zipfile.ZipInfo( + filename=key, date_time=time.localtime(time.time())[:6] + ) + keyinfo.compress_type = self.compression + keyinfo.external_attr = 0o644 << 16 # ?rw-r--r-- + self._zf.writestr(keyinfo, _SOFT_DELETE_SENTINEL) - async def delete(self, key: str) -> None: + async def delete_dir(self, prefix: str) -> None: # docstring inherited - # we choose to only raise NotImplementedError here if the key exists - # this allows the array/group APIs to avoid the overhead of existence checks + # Collect all live keys under the prefix first, then soft-delete each. self._check_writable() - if await self.exists(key): - raise NotImplementedError + if prefix != "" and not prefix.endswith("/"): + prefix += "/" + keys_to_delete = [key async for key in self.list_prefix(prefix)] + for key in keys_to_delete: + await self.delete(key) async def exists(self, key: str) -> bool: # docstring inherited @@ -252,16 +278,26 @@ async def exists(self, key: str) -> bool: self._zf.getinfo(key) except KeyError: return False - else: - return True + # Key physically exists — check it hasn't been soft-deleted + with self._zf.open(key) as f: + return f.read() != _SOFT_DELETE_SENTINEL async def list(self) -> AsyncIterator[str]: # docstring inherited if not self._is_open: self._sync_open() with self._lock: + seen: set[str] = set() for key in self._zf.namelist(): - yield key + if key in seen: + continue + seen.add(key) + try: + with self._zf.open(key) as f: + if f.read() != _SOFT_DELETE_SENTINEL: + yield key + except KeyError: + pass async def list_prefix(self, prefix: str) -> AsyncIterator[str]: # docstring inherited @@ -274,8 +310,7 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]: if not self._is_open: self._sync_open() prefix = prefix.rstrip("/") - - keys = self._zf.namelist() + keys = [k async for k in self.list()] seen = set() if prefix == "": keys_unique = {k.split("/")[0] for k in keys} @@ -292,9 +327,7 @@ async def list_dir(self, prefix: str) -> AsyncIterator[str]: yield k async def move(self, path: Path | str) -> None: - """ - Move the store to another path. - """ + """Move the store to another path.""" if isinstance(path, str): path = Path(path) self.close() From 6ffdc778241b0ade2f345afc3956e7b6221a49a8 Mon Sep 17 00:00:00 2001 From: mohammadZuherJaserAsad Date: Sun, 21 Jun 2026 23:50:26 -0500 Subject: [PATCH 2/6] test: add soft-delete tests for ZipStore and update test_api_integration --- tests/test_store/test_zip.py | 103 +++++++++++++++++++++++++++++++---- 1 file changed, 92 insertions(+), 11 deletions(-) diff --git a/tests/test_store/test_zip.py b/tests/test_store/test_zip.py index be51bcedcb..7fc53e7877 100644 --- a/tests/test_store/test_zip.py +++ b/tests/test_store/test_zip.py @@ -72,10 +72,97 @@ def test_store_repr(self, store: ZipStore) -> None: def test_store_supports_writes(self, store: ZipStore) -> None: assert store.supports_writes + def test_store_supports_deletes(self, store: ZipStore) -> None: + assert store.supports_deletes + def test_store_supports_listing(self, store: ZipStore) -> None: assert store.supports_listing - # TODO: fix this warning + # ------------------------------------------------------------------ + # delete() tests + # ------------------------------------------------------------------ + + async def test_delete_makes_key_inaccessible(self, store: ZipStore) -> None: + key = "chunk/0/0" + value = cpu.Buffer.from_bytes(b"hello zarr") + + await store.set(key, value) + assert await store.exists(key) + + await store.delete(key) + + assert not await store.exists(key) + result = await store.get(key, prototype=default_buffer_prototype()) + assert result is None + listed = [k async for k in store.list()] + assert key not in listed + + async def test_delete_nonexistent_key_is_noop(self, store: ZipStore) -> None: + await store.delete("does/not/exist") # must not raise + + async def test_delete_does_not_affect_other_keys(self, store: ZipStore) -> None: + await store.set("a", cpu.Buffer.from_bytes(b"aaa")) + await store.set("b", cpu.Buffer.from_bytes(b"bbb")) + + await store.delete("a") + + assert not await store.exists("a") + assert await store.exists("b") + buf = await store.get("b", prototype=default_buffer_prototype()) + assert buf is not None + assert buf.to_bytes() == b"bbb" + + async def test_delete_already_deleted_key_is_noop(self, store: ZipStore) -> None: + key = "redundant/key" + await store.set(key, cpu.Buffer.from_bytes(b"data")) + await store.delete(key) + await store.delete(key) # second delete — no-op + assert not await store.exists(key) + + # ------------------------------------------------------------------ + # delete_dir() tests + # ------------------------------------------------------------------ + + async def test_delete_dir_removes_all_keys_under_prefix(self, store: ZipStore) -> None: + keys_under = ["prefix/a", "prefix/b", "prefix/sub/c"] + key_other = "other/d" + + for key in keys_under + [key_other]: + await store.set(key, cpu.Buffer.from_bytes(b"data")) + + await store.delete_dir("prefix") + + listed = [k async for k in store.list()] + for key in keys_under: + assert key not in listed + assert not await store.exists(key) + assert key_other in listed + + async def test_delete_dir_empty_prefix_is_noop(self, store: ZipStore) -> None: + await store.set("unrelated/key", cpu.Buffer.from_bytes(b"data")) + await store.delete_dir("nonexistent_prefix") + assert await store.exists("unrelated/key") + + # ------------------------------------------------------------------ + # list() soft-delete filtering + # ------------------------------------------------------------------ + + async def test_list_excludes_soft_deleted_keys(self, store: ZipStore) -> None: + for key in ["a", "b", "c"]: + await store.set(key, cpu.Buffer.from_bytes(b"x")) + + await store.delete("b") + + listed = [k async for k in store.list()] + assert "a" in listed + assert "b" not in listed + assert "c" in listed + + # ------------------------------------------------------------------ + # Updated integration test + # ------------------------------------------------------------------ + + @pytest.mark.filterwarnings("ignore::zarr.core.dtype.common.UnstableSpecificationWarning") @pytest.mark.filterwarnings("ignore:Unclosed client session:ResourceWarning") def test_api_integration(self, store: ZipStore) -> None: root = zarr.open_group(store=store, mode="a") @@ -92,17 +179,14 @@ def test_api_integration(self, store: ZipStore) -> None: with pytest.warns(UserWarning, match="Duplicate name: 'foo/c/0/0'"): z[0, 0] = 100 - # TODO: assigning an entire chunk to fill value ends up deleting the chunk which is not supported - # a work around will be needed here. - with pytest.raises(NotImplementedError): - z[0:10, 0:10] = 99 + # assigning fill value to a chunk now works via soft-delete + z[0:10, 0:10] = 99 bar = root.create_group("bar", attributes={"hello": "world"}) assert "hello" in dict(bar.attrs) - # keys cannot be deleted - with pytest.raises(NotImplementedError): - del root["bar"] + # keys can now be deleted via soft-delete + del root["bar"] store.close() @@ -126,7 +210,6 @@ async def test_zip_open_mode_translation( assert store.read_only == read_only def test_externally_zipped_store(self, tmp_path: Path) -> None: - # See: https://github.com/zarr-developers/zarr-python/issues/2757 zarr_path = tmp_path / "foo.zarr" root = zarr.open_group(store=zarr_path, mode="w") root.require_group("foo") @@ -140,8 +223,6 @@ def test_externally_zipped_store(self, tmp_path: Path) -> None: assert list(group.keys()) == list(group.keys()) async def test_list_without_explicit_open(self, tmp_path: Path) -> None: - # ZipStore.list(), list_dir(), and exists() should auto-open - # the zip file just like _get() and _set() do. zip_path = tmp_path / "data.zip" zarr_path = tmp_path / "foo.zarr" root = zarr.open_group(store=zarr_path, mode="w") From fbe80ce50fbc9de45a697287d46bc87191bbb538 Mon Sep 17 00:00:00 2001 From: mohammadZuherJaserAsad Date: Sun, 21 Jun 2026 23:50:53 -0500 Subject: [PATCH 3/6] test: remove ZipStore skip in stateful tests (now supports delete) --- tests/test_store/test_stateful.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/tests/test_store/test_stateful.py b/tests/test_store/test_stateful.py index 82b482d0ff..57ad2ee595 100644 --- a/tests/test_store/test_stateful.py +++ b/tests/test_store/test_stateful.py @@ -30,9 +30,7 @@ def test_zarr_hierarchy(sync_store: Store) -> None: def mk_test_instance_sync() -> ZarrHierarchyStateMachine: return ZarrHierarchyStateMachine(sync_store) - if isinstance(sync_store, ZipStore): - pytest.skip(reason="ZipStore does not support delete") - + # ZipStore now supports soft-delete — skip removed run_state_machine_as_test(mk_test_instance_sync) # type: ignore[no-untyped-call] @@ -40,12 +38,8 @@ def test_zarr_store(sync_store: Store) -> None: def mk_test_instance_sync() -> ZarrStoreStateMachine: return ZarrStoreStateMachine(sync_store) - if isinstance(sync_store, ZipStore): - pytest.skip(reason="ZipStore does not support delete") + # ZipStore now supports soft-delete — skip removed if isinstance(sync_store, LocalStore): - # This test uses arbitrary keys, which are passed to `set` and `delete`. - # It assumes that `set` and `delete` are the only two operations that modify state. - # But LocalStore, directories can hang around even after a key is delete-d. pytest.skip(reason="Test isn't suitable for LocalStore.") run_state_machine_as_test(mk_test_instance_sync) # type: ignore[no-untyped-call] From 4db600c4e9516b4c907726438c546b75392f4180 Mon Sep 17 00:00:00 2001 From: mohammadZuherJaserAsad Date: Wed, 8 Jul 2026 19:30:24 -0500 Subject: [PATCH 4/6] Update _zip.py Hypothesis's stateful property tests caught two real bugs: - Setting a key to b"" (a legitimate zero-length value) was indistinguishable from a soft-deleted key, since the sentinel was also b"". list()/exists()/get() incorrectly treated it as missing. Switched to a long, specific sentinel that can't plausibly collide with real payload bytes. - delete() writes the sentinel to an already-existing zip entry, which zipfile.writestr() flags with a "Duplicate name" UserWarning every time. This project's pytest config turns warnings into errors, so any delete of an existing key failed the test suite. The warning is suppressed specifically in delete(), since it's expected/intentional there (unlike set() overwrites, which intentionally keep warning per test_api_integration). Also fixes the ruff lint failures (unused ZipStore import in test_stateful.py, one reformatted line) and adds filterwarnings markers to the stateful tests to account for the pre-existing, expected duplicate-name warning on ZipStore overwrites. --- src/zarr/storage/_zip.py | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/src/zarr/storage/_zip.py b/src/zarr/storage/_zip.py index 6824f085ad..a28b4964d6 100644 --- a/src/zarr/storage/_zip.py +++ b/src/zarr/storage/_zip.py @@ -4,6 +4,7 @@ import shutil import threading import time +import warnings import zipfile from pathlib import Path from typing import TYPE_CHECKING, Any, Literal @@ -25,7 +26,15 @@ # Sentinel value written to a zip entry to mark it as soft-deleted. # The ZIP format does not support native deletion, so we overwrite the entry # with this value and treat it as absent in all read/list operations. -_SOFT_DELETE_SENTINEL = b"" +# +# NOTE: this was originally b"", but that collides with a legitimate +# zero-length value a caller might legitimately store (Hypothesis's +# stateful property tests caught this: setting a key value to b"" +# made it indistinguishable from a soft-deleted key, so list()/exists()/ +# get() incorrectly reported it as missing). A long, specific marker is +# used instead so it cannot plausibly collide with real Zarr payload bytes +# or a randomly generated test value. +_SOFT_DELETE_SENTINEL = b"\x00__zarr_zipstore_soft_delete_tombstone_v1__\x00" class ZipStore(Store): @@ -171,7 +180,7 @@ def _get( except KeyError: return None - # Treat soft-deleted entries (empty bytes) as missing + # Treat soft-deleted entries (matching the sentinel) as missing if data == _SOFT_DELETE_SENTINEL: return None @@ -245,19 +254,27 @@ async def set_if_not_exists(self, key: str, value: Buffer) -> None: async def delete(self, key: str) -> None: # docstring inherited - # Soft-delete: overwrite the entry with an empty byte sentinel. + # Soft-delete: overwrite the entry with a sentinel value. # The ZIP format has no native delete API, so we mark the entry as - # deleted by writing b"" and filtering it out in all read/list paths. - # If the key does not exist in the archive, this is a no-op. + # deleted by writing the sentinel and filtering it out in all + # read/list paths. If the key does not exist in the archive, this + # is a no-op. self._check_writable() with self._lock: if key in self._zf.namelist(): - keyinfo = zipfile.ZipInfo( - filename=key, date_time=time.localtime(time.time())[:6] - ) + keyinfo = zipfile.ZipInfo(filename=key, date_time=time.localtime(time.time())[:6]) keyinfo.compress_type = self.compression keyinfo.external_attr = 0o644 << 16 # ?rw-r--r-- - self._zf.writestr(keyinfo, _SOFT_DELETE_SENTINEL) + # zipfile.writestr() warns "Duplicate name" whenever a name + # is written more than once, which is exactly what + # soft-delete does on purpose. That warning is expected and + # harmless here (unlike a real overwrite via set(), which + # intentionally keeps warning -- see test_api_integration), + # so it is suppressed rather than allowed to propagate under + # this project's filterwarnings = "error" pytest config. + with warnings.catch_warnings(): + warnings.simplefilter("ignore", UserWarning) + self._zf.writestr(keyinfo, _SOFT_DELETE_SENTINEL) async def delete_dir(self, prefix: str) -> None: # docstring inherited From 2b784fec87c4e491094494c1218e0139eeec58a8 Mon Sep 17 00:00:00 2001 From: mohammadZuherJaserAsad Date: Wed, 8 Jul 2026 19:33:57 -0500 Subject: [PATCH 5/6] Update test_stateful.py test: remove unused ZipStore import; expect duplicate-name warning in stateful tests --- tests/test_store/test_stateful.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_store/test_stateful.py b/tests/test_store/test_stateful.py index 57ad2ee595..6b1c2d481d 100644 --- a/tests/test_store/test_stateful.py +++ b/tests/test_store/test_stateful.py @@ -8,7 +8,7 @@ import zarr from zarr.abc.store import Store -from zarr.storage import LocalStore, ZipStore +from zarr.storage import LocalStore from zarr.testing.stateful import ZarrHierarchyStateMachine, ZarrStoreStateMachine pytestmark = [ @@ -26,6 +26,7 @@ def _enable_rectilinear_chunks() -> Generator[None, None, None]: @pytest.mark.filterwarnings("ignore::zarr.core.dtype.common.UnstableSpecificationWarning") +@pytest.mark.filterwarnings("ignore:Duplicate name:UserWarning") def test_zarr_hierarchy(sync_store: Store) -> None: def mk_test_instance_sync() -> ZarrHierarchyStateMachine: return ZarrHierarchyStateMachine(sync_store) @@ -34,6 +35,7 @@ def mk_test_instance_sync() -> ZarrHierarchyStateMachine: run_state_machine_as_test(mk_test_instance_sync) # type: ignore[no-untyped-call] +@pytest.mark.filterwarnings("ignore:Duplicate name:UserWarning") def test_zarr_store(sync_store: Store) -> None: def mk_test_instance_sync() -> ZarrStoreStateMachine: return ZarrStoreStateMachine(sync_store) From 81af773c4eb12b59aef596d1b89c93e21c91781e Mon Sep 17 00:00:00 2001 From: mohammadZuherJaserAsad Date: Thu, 9 Jul 2026 11:23:21 -0500 Subject: [PATCH 6/6] fix: replace sentinel-value soft-delete with comment-field marker The previous soft-delete used a fixed sentinel byte string as entry data, which Slow Hypothesis CI proved can still collide with a legitimate value (Hypothesis found a falsifying example that set a key to the literal sentinel bytes). This replaces it with a ZipInfo.comment flag, which is metadata never exposed through set()/get() and therefore cannot collide with any data a caller stores, while still persisting the deletion to disk in the zip central directory. --- src/zarr/storage/_zip.py | 90 ++++++++++++++++++++++------------------ 1 file changed, 50 insertions(+), 40 deletions(-) diff --git a/src/zarr/storage/_zip.py b/src/zarr/storage/_zip.py index a28b4964d6..6a16d06aaa 100644 --- a/src/zarr/storage/_zip.py +++ b/src/zarr/storage/_zip.py @@ -23,18 +23,25 @@ ZipStoreAccessModeLiteral = Literal["r", "w", "a"] -# Sentinel value written to a zip entry to mark it as soft-deleted. -# The ZIP format does not support native deletion, so we overwrite the entry -# with this value and treat it as absent in all read/list operations. +# Marker used to flag a zip entry as soft-deleted. # -# NOTE: this was originally b"", but that collides with a legitimate -# zero-length value a caller might legitimately store (Hypothesis's -# stateful property tests caught this: setting a key value to b"" -# made it indistinguishable from a soft-deleted key, so list()/exists()/ -# get() incorrectly reported it as missing). A long, specific marker is -# used instead so it cannot plausibly collide with real Zarr payload bytes -# or a randomly generated test value. -_SOFT_DELETE_SENTINEL = b"\x00__zarr_zipstore_soft_delete_tombstone_v1__\x00" +# IMPORTANT: this is stored in the ZipInfo *comment* field (per-entry +# central-directory metadata), not in the entry's actual data content. +# An earlier version of this fix wrote a sentinel *value* as the entry's +# data (first b"", later a long "unique" byte string). Both are unsafe: +# Zarr payload bytes are arbitrary, so a legitimate value can always +# equal whatever sentinel is chosen -- Hypothesis's stateful property +# tests proved this by finding a falsifying example that set a key's +# value to the literal sentinel bytes, which made list()/exists()/get() +# incorrectly report a live key as deleted. +# +# Using ZipInfo.comment instead sidesteps the problem entirely: it is +# metadata the store never exposes through set()/get(), so it cannot +# collide with any byte string a caller stores as data, no matter what +# that data is. It's also persisted to the zip's central directory, so +# soft-deletes made in one process are still visible after the archive +# is closed and reopened elsewhere. +_SOFT_DELETE_MARKER = b"__zarr_zipstore_soft_delete__" class ZipStore(Store): @@ -69,15 +76,19 @@ class ZipStore(Store): Notes ----- - Deletion is implemented as a soft-delete: the zip entry is overwritten with - an empty byte string (b""). All read, exists, and list operations - filter out soft-deleted entries so they appear absent to callers. Because - the ZIP format does not allow removing entries, soft-deleted entries remain - on disk but are invisible through the store API. + Deletion is implemented as a soft-delete: since the ZIP format does not + support removing entries in place, `delete()` appends a new, empty entry + under the same name and flags it via the ZipInfo *comment* field (not its + data). All read, exists, and list operations check that flag and treat + matching entries as absent. Because deletions are recorded in the zip's + central directory rather than kept only in memory, they remain in effect + even if the store is closed and the same file reopened later. The + original, physical bytes for a soft-deleted entry stay on disk (the + archive is never rewritten), but are unreachable through the store API. """ supports_writes: bool = True - supports_deletes: bool = True # soft-delete via empty-byte overwrite + supports_deletes: bool = True # soft-delete via a flagged empty entry supports_listing: bool = True path: Path @@ -165,6 +176,14 @@ def __repr__(self) -> str: def __eq__(self, other: object) -> bool: return isinstance(other, type(self)) and self.path == other.path + def _is_soft_deleted(self, key: str) -> bool: + """Return True if `key`'s most recent entry is a soft-delete marker.""" + try: + info = self._zf.getinfo(key) + except KeyError: + return False + return info.comment == _SOFT_DELETE_MARKER + def _get( self, key: str, @@ -174,16 +193,14 @@ def _get( if not self._is_open: self._sync_open() # docstring inherited + if self._is_soft_deleted(key): + return None try: with self._zf.open(key) as f: data = f.read() except KeyError: return None - # Treat soft-deleted entries (matching the sentinel) as missing - if data == _SOFT_DELETE_SENTINEL: - return None - if byte_range is None: return prototype.buffer.from_bytes(data) elif isinstance(byte_range, RangeByteRequest): @@ -230,6 +247,8 @@ def _set(self, key: str, value: Buffer) -> None: keyinfo.external_attr |= 0x10 # MS-DOS directory flag else: keyinfo.external_attr = 0o644 << 16 # ?rw-r--r-- + # keyinfo.comment defaults to b"", so a fresh write is never + # mistaken for a soft-delete marker regardless of `value`. self._zf.writestr(keyinfo, value.to_bytes()) async def set(self, key: str, value: Buffer) -> None: @@ -254,17 +273,18 @@ async def set_if_not_exists(self, key: str, value: Buffer) -> None: async def delete(self, key: str) -> None: # docstring inherited - # Soft-delete: overwrite the entry with a sentinel value. - # The ZIP format has no native delete API, so we mark the entry as - # deleted by writing the sentinel and filtering it out in all - # read/list paths. If the key does not exist in the archive, this - # is a no-op. + # Soft-delete: append an empty entry under this name, flagged via + # the ZipInfo comment field (see _SOFT_DELETE_MARKER above). The + # comment -- not the data -- is what read/list/exists check, so + # this can never be confused with a real value, including a real + # value that happens to be empty bytes. self._check_writable() with self._lock: if key in self._zf.namelist(): keyinfo = zipfile.ZipInfo(filename=key, date_time=time.localtime(time.time())[:6]) keyinfo.compress_type = self.compression keyinfo.external_attr = 0o644 << 16 # ?rw-r--r-- + keyinfo.comment = _SOFT_DELETE_MARKER # zipfile.writestr() warns "Duplicate name" whenever a name # is written more than once, which is exactly what # soft-delete does on purpose. That warning is expected and @@ -274,7 +294,7 @@ async def delete(self, key: str) -> None: # this project's filterwarnings = "error" pytest config. with warnings.catch_warnings(): warnings.simplefilter("ignore", UserWarning) - self._zf.writestr(keyinfo, _SOFT_DELETE_SENTINEL) + self._zf.writestr(keyinfo, b"") async def delete_dir(self, prefix: str) -> None: # docstring inherited @@ -291,13 +311,7 @@ async def exists(self, key: str) -> bool: if not self._is_open: self._sync_open() with self._lock: - try: - self._zf.getinfo(key) - except KeyError: - return False - # Key physically exists — check it hasn't been soft-deleted - with self._zf.open(key) as f: - return f.read() != _SOFT_DELETE_SENTINEL + return not self._is_soft_deleted(key) and key in self._zf.namelist() async def list(self) -> AsyncIterator[str]: # docstring inherited @@ -309,12 +323,8 @@ async def list(self) -> AsyncIterator[str]: if key in seen: continue seen.add(key) - try: - with self._zf.open(key) as f: - if f.read() != _SOFT_DELETE_SENTINEL: - yield key - except KeyError: - pass + if not self._is_soft_deleted(key): + yield key async def list_prefix(self, prefix: str) -> AsyncIterator[str]: # docstring inherited