From 4cf612c1965ce33f9377fd9cc5a95668b59d34cf Mon Sep 17 00:00:00 2001 From: Konstantin Date: Wed, 30 Sep 2026 12:04:29 +0200 Subject: [PATCH] Support suffix reads (negative start) in MemoryStore and ReadOnlyZipStore ShardingIndexedCodec reads a shard index located at the end via store.get(keys, -indexSize), i.e. "the last N bytes". FilesystemStore, HttpStore and S3Store handle a negative start, but MemoryStore passed it straight to ByteBuffer.wrap and ReadOnlyZipStore to ZipArchiveInputStream.skip, which throws "Negative skip value". BufferedZipStore delegates to MemoryStore and was affected too. As a result, any partial read of a sharded array (index_location "end") from a zip archive failed, e.g. reading a boundary shard whose valid region is smaller than the shard shape. Convert a negative start into an absolute offset using the entry size, add a StoreTest case for suffix reads for all stores, and an end-to-end test reading a sub-region of a sharded array from a zip. Co-Authored-By: Claude Opus 5.5 --- .../dev/zarr/zarrjava/store/MemoryStore.java | 2 + .../zarr/zarrjava/store/ReadOnlyZipStore.java | 8 ++++ .../zarrjava/store/ReadOnlyZipStoreTest.java | 42 +++++++++++++++++++ .../dev/zarr/zarrjava/store/StoreTest.java | 20 +++++++++ 4 files changed, 72 insertions(+) diff --git a/src/main/java/dev/zarr/zarrjava/store/MemoryStore.java b/src/main/java/dev/zarr/zarrjava/store/MemoryStore.java index 78573459..ccc2bdc2 100644 --- a/src/main/java/dev/zarr/zarrjava/store/MemoryStore.java +++ b/src/main/java/dev/zarr/zarrjava/store/MemoryStore.java @@ -49,6 +49,7 @@ public ByteBuffer get(String[] keys, long start) { public ByteBuffer get(String[] keys, long start, long end) { byte[] bytes = map.get(resolveKeys(keys)); if (bytes == null) return null; + if (start < 0) start = Math.max(0, bytes.length + start); // suffix read: the last -start bytes if (end < 0) end = bytes.length; if (end > Integer.MAX_VALUE) throw new IllegalArgumentException("End index too large"); return ByteBuffer.wrap(bytes, (int) start, (int) (end - start)); @@ -101,6 +102,7 @@ public String toString() { public InputStream getInputStream(String[] keys, long start, long end) { byte[] bytes = map.get(resolveKeys(keys)); if (bytes == null) return null; + if (start < 0) start = Math.max(0, bytes.length + start); // suffix read: the last -start bytes if (end < 0) end = bytes.length; if (end > Integer.MAX_VALUE) throw new IllegalArgumentException("End index too large"); return new java.io.ByteArrayInputStream(bytes, (int) start, (int) (end - start)); diff --git a/src/main/java/dev/zarr/zarrjava/store/ReadOnlyZipStore.java b/src/main/java/dev/zarr/zarrjava/store/ReadOnlyZipStore.java index 22293989..403af36e 100644 --- a/src/main/java/dev/zarr/zarrjava/store/ReadOnlyZipStore.java +++ b/src/main/java/dev/zarr/zarrjava/store/ReadOnlyZipStore.java @@ -109,6 +109,10 @@ public ByteBuffer get(String[] keys, long start, long end) { if (!fileIndex.containsKey(key)) { return null; } + if (start < 0) { + // suffix read: the last -start bytes + start = Math.max(0, getSize(keys) + start); + } InputStream inputStream = underlyingStore.getInputStream(); if (inputStream == null) { @@ -226,6 +230,10 @@ public InputStream getInputStream(String[] keys, long start, long end) { if (!fileIndex.containsKey(key)) { return null; } + if (start < 0) { + // suffix read: the last -start bytes + start = Math.max(0, getSize(keys) + start); + } InputStream baseStream = underlyingStore.getInputStream(); diff --git a/src/test/java/dev/zarr/zarrjava/store/ReadOnlyZipStoreTest.java b/src/test/java/dev/zarr/zarrjava/store/ReadOnlyZipStoreTest.java index b1a28d82..ed42fe15 100644 --- a/src/test/java/dev/zarr/zarrjava/store/ReadOnlyZipStoreTest.java +++ b/src/test/java/dev/zarr/zarrjava/store/ReadOnlyZipStoreTest.java @@ -3,9 +3,13 @@ import dev.zarr.zarrjava.Utils; import dev.zarr.zarrjava.ZarrException; import dev.zarr.zarrjava.core.Group; +import dev.zarr.zarrjava.v3.Array; +import dev.zarr.zarrjava.v3.DataType; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import java.io.IOException; import java.nio.file.Path; @@ -123,4 +127,42 @@ public void testReadFromBufferedZipStore() throws ZarrException, IOException { assertIsTestGroupV3(Group.open(readOnlyZipStore.resolve()), true); } + + @ParameterizedTest + @ValueSource(strings = {"start", "end"}) + public void testPartialReadOfShardedArray(String indexLocation) throws ZarrException, IOException { + // partial shard reads fetch the shard index via a suffix read when it is located at the end + Path sourceDir = TESTOUTPUT.resolve("testShardedZipStore_" + indexLocation); + Path targetDir = TESTOUTPUT.resolve("testShardedZipStore_" + indexLocation + ".zip"); + Array writeArray = Array.create(new FilesystemStore(sourceDir).resolve(), Array.metadataBuilder() + .withShape(24, 24) + .withDataType(DataType.INT32) + .withChunkShape(16, 16) + .withCodecs(c -> c.withSharding(new int[]{8, 8}, c1 -> c1.withBytes("LITTLE"), indexLocation)) + .withFillValue(0) + .build()); + int[] data = new int[24 * 24]; + for (int i = 0; i < data.length; i++) { + data[i] = i; + } + ucar.ma2.Array expected = ucar.ma2.Array.factory(ucar.ma2.DataType.INT, new int[]{24, 24}, data); + writeArray.write(expected); + + Utils.zipFile(sourceDir, targetDir); + + Array[] readArrays = { + Array.open(new ReadOnlyZipStore(targetDir).resolve()), + Array.open(new BufferedZipStore(targetDir).resolve()) + }; + for (Array readArray : readArrays) { + Assertions.assertArrayEquals(data, (int[]) readArray.read().get1DJavaArray(ucar.ma2.DataType.INT)); + + ucar.ma2.Array partial = readArray.read(new long[]{3, 5}, new long[]{14, 17}); + for (int i = 0; i < 14; i++) { + for (int j = 0; j < 17; j++) { + Assertions.assertEquals(data[(i + 3) * 24 + (j + 5)], partial.getInt(i * 17 + j)); + } + } + } + } } diff --git a/src/test/java/dev/zarr/zarrjava/store/StoreTest.java b/src/test/java/dev/zarr/zarrjava/store/StoreTest.java index 4f1c80a1..12cda953 100644 --- a/src/test/java/dev/zarr/zarrjava/store/StoreTest.java +++ b/src/test/java/dev/zarr/zarrjava/store/StoreTest.java @@ -128,6 +128,26 @@ public void testGetWithStartEnd() { Assertions.assertArrayEquals(expectedBytes, actualBytes); } + @Test + public void testGetSuffix() { + // a negative start denotes a suffix read (the last -start bytes), as used for sharding indexes + StoreHandle storeHandle = storeHandleWithData(); + long size = storeHandle.getSize(); + if (size < 20) { + Assertions.fail("Store size is too small to test suffix reads"); + } + ByteBuffer buffer = storeHandle.read(-10); + Assertions.assertEquals(10, buffer.remaining()); + + ByteBuffer fullBuffer = storeHandle.read(); + byte[] expectedBytes = new byte[10]; + fullBuffer.position((int) (size - 10)); + fullBuffer.get(expectedBytes, 0, 10); + byte[] actualBytes = new byte[10]; + buffer.get(actualBytes, 0, 10); + Assertions.assertArrayEquals(expectedBytes, actualBytes); + } + @Test public abstract void testList() throws ZarrException, IOException;