From 15fcc89708305381bc3a8f6956ed69fe3b39dbd9 Mon Sep 17 00:00:00 2001 From: asim Date: Fri, 14 Feb 2025 10:22:37 +0530 Subject: [PATCH 1/2] fix: implement soft delete by replacing key with empty byte literal Signed-off-by: asim --- src/zarr/storage/_zip.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/zarr/storage/_zip.py b/src/zarr/storage/_zip.py index bbfe6c67aa..3f61b2892f 100644 --- a/src/zarr/storage/_zip.py +++ b/src/zarr/storage/_zip.py @@ -241,11 +241,12 @@ async def delete_dir(self, prefix: str) -> None: async def delete(self, key: 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 + # If key is present it is replaced by an empty byte literal as a way of representing it as deleted self._check_writable() if await self.exists(key): - raise NotImplementedError + with self._lock: + empty_buffer = Buffer.from_bytes(b"") + self._set(key, empty_buffer) async def exists(self, key: str) -> bool: # docstring inherited From f1fae845a9796bf7202d67bcfd3a3c5124f524f5 Mon Sep 17 00:00:00 2001 From: asim Date: Sat, 15 Feb 2025 15:50:16 +0530 Subject: [PATCH 2/2] Implement soft delete for the zip store Signed-off-by: asim --- src/zarr/storage/_zip.py | 11 +++++++++-- src/zarr/testing/store.py | 11 ++++++++++- tests/test_store/test_zip.py | 4 ++-- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/zarr/storage/_zip.py b/src/zarr/storage/_zip.py index 3f61b2892f..ab155a5d8b 100644 --- a/src/zarr/storage/_zip.py +++ b/src/zarr/storage/_zip.py @@ -244,9 +244,16 @@ async def delete(self, key: str) -> None: # If key is present it is replaced by an empty byte literal as a way of representing it as deleted self._check_writable() if await self.exists(key): + keyinfo = zipfile.ZipInfo(filename=key, date_time=time.localtime(time.time())[:6]) + keyinfo.compress_type = self.compression + if keyinfo.filename[-1] == os.sep: + keyinfo.external_attr = 0o40775 << 16 # drwxrwxr-x + keyinfo.external_attr |= 0x10 + else: + keyinfo.external_attr = 0o644 << 16 # ?rw-r--r-- + with self._lock: - empty_buffer = Buffer.from_bytes(b"") - self._set(key, empty_buffer) + self._zf.writestr(keyinfo, b"") async def exists(self, key: str) -> bool: # docstring inherited diff --git a/src/zarr/testing/store.py b/src/zarr/testing/store.py index 112f6261e9..e5dead6f5a 100644 --- a/src/zarr/testing/store.py +++ b/src/zarr/testing/store.py @@ -5,7 +5,7 @@ from abc import abstractmethod from typing import TYPE_CHECKING, Generic, TypeVar -from zarr.storage import WrapperStore +from zarr.storage import WrapperStore, ZipStore if TYPE_CHECKING: from typing import Any @@ -340,6 +340,15 @@ async def test_delete(self, store: S) -> None: await store.delete("foo/zarr.json") assert not await store.exists("foo/zarr.json") + async def test_delete_zip_store(self, store: S) -> None: + if not isinstance(store, ZipStore): + pytest.skip("store is not a ZipStore") + await store.set("foo/zarr.json", self.buffer_cls.from_bytes(b"bar")) + assert await store.exists("foo/zarr.json") + await store.delete("foo/zarr.json") + result = await store.get("foo/zarr.json", default_buffer_prototype()) + assert result == self.buffer_cls.from_bytes(b"") + async def test_delete_dir(self, store: S) -> None: if not store.supports_deletes: pytest.skip("store does not support deletes") diff --git a/tests/test_store/test_zip.py b/tests/test_store/test_zip.py index 839656108b..ce601549ac 100644 --- a/tests/test_store/test_zip.py +++ b/tests/test_store/test_zip.py @@ -83,8 +83,8 @@ def test_api_integration(self, store: ZipStore) -> None: # 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 + + z[0:10, 0:10] = 99 bar = root.create_group("bar", attributes={"hello": "world"}) assert "hello" in dict(bar.attrs)