From 24d284d522a0ebafd2b91cbcb575d013946e3614 Mon Sep 17 00:00:00 2001 From: Konstantin Date: Wed, 26 Aug 2026 18:24:18 +0200 Subject: [PATCH 1/6] List group members level by level instead of enumerating all keys Group.list() called storeHandle.list(), which is fully recursive and returns every key below the group, including all chunk keys. Listing a group with 1M chunks on S3 cost ~1000 paginated requests to find a handful of zarr.json files. Walk the hierarchy with listChildren() instead, one level at a time, and never descend into arrays. The implementation moves to core.Group, so v2 and v3 share it, and adds members() for the immediate children only. Also: - S3Store.listChildren() used a single listObjectsV2 call, silently truncating groups with more than 1000 children. Use the paginator. - FilesystemStore.get() threw instead of returning null when a path component is a file rather than a directory. Probing a plain file next to a group's nodes is normal now, and such a key holds no data. Co-Authored-By: Claude Opus 5 (1M context) --- .../java/dev/zarr/zarrjava/core/Group.java | 99 ++++- .../zarr/zarrjava/store/FilesystemStore.java | 19 + .../java/dev/zarr/zarrjava/store/S3Store.java | 22 +- src/main/java/dev/zarr/zarrjava/v2/Group.java | 23 -- src/main/java/dev/zarr/zarrjava/v3/Group.java | 21 -- .../java/dev/zarr/zarrjava/GroupListTest.java | 355 ++++++++++++++++++ .../dev/zarr/zarrjava/store/S3StoreTest.java | 19 + 7 files changed, 505 insertions(+), 53 deletions(-) create mode 100644 src/test/java/dev/zarr/zarrjava/GroupListTest.java diff --git a/src/main/java/dev/zarr/zarrjava/core/Group.java b/src/main/java/dev/zarr/zarrjava/core/Group.java index 7b425a37..9d3a73c5 100644 --- a/src/main/java/dev/zarr/zarrjava/core/Group.java +++ b/src/main/java/dev/zarr/zarrjava/core/Group.java @@ -3,16 +3,32 @@ import dev.zarr.zarrjava.ZarrException; import dev.zarr.zarrjava.store.FilesystemStore; import dev.zarr.zarrjava.store.StoreHandle; +import dev.zarr.zarrjava.utils.Utils; import javax.annotation.Nonnull; import javax.annotation.Nullable; import java.io.IOException; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.AbstractMap; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; import java.util.stream.Stream; public abstract class Group extends AbstractNode { + /** + * Keys that hold metadata of the group itself and never point at a child node. + */ + private static final Set METADATA_KEYS = Collections.unmodifiableSet( + new HashSet<>(Arrays.asList(ZARR_JSON, ZARRAY, ZATTRS, ZGROUP))); + protected Group(@Nonnull StoreHandle storeHandle) { super(storeHandle); } @@ -70,7 +86,41 @@ public Node get(String key) throws ZarrException, IOException { return get(new String[]{key}); } - public abstract Stream list(); + /** + * Lists the immediate children (arrays and subgroups) of this group. + *

+ * This costs a single listing request on the underlying store, plus one metadata read per + * child. Keys that do not hold a Zarr node are skipped. + * + * @return a stream of the direct children of this group + * @throws UnsupportedOperationException if the underlying store does not support listing + */ + public Stream members() { + return childKeys(new String[0]).parallelStream() + .map(this::openChild) + .filter(Objects::nonNull) + .collect(Collectors.toList()) + .stream(); + } + + public Node[] membersAsArray() { + try (Stream nodeStream = members()) { + return nodeStream.toArray(Node[]::new); + } + } + + /** + * Lists all descendants (arrays and groups) of this group, at any depth. + *

+ * The group hierarchy is walked one level at a time, so only group keys are listed and chunk + * keys are never enumerated. Descending into an array is not necessary and does not happen. + * + * @return a stream of all descendants of this group, excluding the group itself + * @throws UnsupportedOperationException if the underlying store does not support listing + */ + public Stream list() { + return listDescendants(new String[0]); + } public Node[] listAsArray() { try (Stream nodeStream = list()) { @@ -78,5 +128,52 @@ public Node[] listAsArray() { } } + private Stream listDescendants(String[] prefix) { + List> children = childKeys(prefix).parallelStream() + .map(key -> new AbstractMap.SimpleEntry(key, openChild(key))) + .collect(Collectors.toList()); + + return children.stream().flatMap(child -> { + Node node = child.getValue(); + if (node == null) { + // Not a node itself, but it may still contain nodes further down. + return listDescendants(child.getKey()); + } + if (node instanceof Group) { + return Stream.concat(Stream.of(node), listDescendants(child.getKey())); + } + return Stream.of(node); + }); + } + + /** + * Lists the keys directly below {@code prefix} that may hold a child node, relative to this + * group. + */ + private List childKeys(String[] prefix) { + try (Stream children = storeHandle.resolve(prefix).listChildren()) { + return children + .filter(name -> !METADATA_KEYS.contains(name)) + .map(name -> Utils.concatArrays(prefix, new String[]{name})) + .collect(Collectors.toList()); + } + } + + /** + * Opens the node at {@code key}, or returns null if there is no node there. + */ + @Nullable + private Node openChild(String[] key) { + try { + return get(key); + } catch (IOException e) { + throw new RuntimeException( + "Failed to read node metadata for key '" + String.join("/", key) + "': " + e.getMessage(), e); + } catch (ZarrException e) { + throw new RuntimeException( + "Failed to parse node metadata for key '" + String.join("/", key) + "': " + e.getMessage(), e); + } + } + public abstract GroupMetadata metadata(); } diff --git a/src/main/java/dev/zarr/zarrjava/store/FilesystemStore.java b/src/main/java/dev/zarr/zarrjava/store/FilesystemStore.java index 3644121c..5954edc6 100644 --- a/src/main/java/dev/zarr/zarrjava/store/FilesystemStore.java +++ b/src/main/java/dev/zarr/zarrjava/store/FilesystemStore.java @@ -44,6 +44,16 @@ public boolean exists(String[] keys) { return Files.isRegularFile(resolveKeys(keys)); } + /** + * Whether there is no file at the given keys. Reading such a key does not always fail with a + * {@link NoSuchFileException}: if one of the path components is a file rather than a + * directory, the filesystem reports "Not a directory" instead. Either way the key holds no + * data, so {@link #get} has to return null for it. + */ + private boolean isMissing(String[] keys) { + return !Files.isRegularFile(resolveKeys(keys)); + } + @Nullable @Override public ByteBuffer get(String[] keys) { @@ -52,6 +62,9 @@ public ByteBuffer get(String[] keys) { } catch (NoSuchFileException e) { return null; } catch (IOException e) { + if (isMissing(keys)) { + return null; + } throw StoreException.readFailed(this.toString(), keys, e); } } @@ -75,6 +88,9 @@ public ByteBuffer get(String[] keys, long start) { } catch (NoSuchFileException e) { return null; } catch (IOException e) { + if (isMissing(keys)) { + return null; + } throw StoreException.readFailed(this.toString(), keys, e); } } @@ -97,6 +113,9 @@ public ByteBuffer get(String[] keys, long start, long end) { } catch (NoSuchFileException e) { return null; } catch (IOException e) { + if (isMissing(keys)) { + return null; + } throw StoreException.readFailed(this.toString(), keys, e); } } diff --git a/src/main/java/dev/zarr/zarrjava/store/S3Store.java b/src/main/java/dev/zarr/zarrjava/store/S3Store.java index 5eda1b8a..486d55a4 100644 --- a/src/main/java/dev/zarr/zarrjava/store/S3Store.java +++ b/src/main/java/dev/zarr/zarrjava/store/S3Store.java @@ -3,6 +3,8 @@ import java.io.IOException; import java.io.InputStream; import java.nio.ByteBuffer; +import java.util.ArrayList; +import java.util.List; import java.util.stream.Stream; import javax.annotation.Nonnull; @@ -12,7 +14,6 @@ import software.amazon.awssdk.core.ResponseInputStream; import software.amazon.awssdk.core.sync.RequestBody; import software.amazon.awssdk.services.s3.S3Client; -import software.amazon.awssdk.services.s3.model.CommonPrefix; import software.amazon.awssdk.services.s3.model.DeleteObjectRequest; import software.amazon.awssdk.services.s3.model.GetObjectRequest; import software.amazon.awssdk.services.s3.model.GetObjectResponse; @@ -184,15 +185,20 @@ public Stream listChildren(String[] keys) { .delimiter("/") .build(); - ListObjectsV2Response res = s3client.listObjectsV2(req); - - // Combine CommonPrefixes (folders) and Contents (files) - Stream folders = res.commonPrefixes().stream().map(CommonPrefix::prefix); final String finalFullPrefix = fullPrefix; - Stream files = res.contents().stream().map(S3Object::key) - .filter(key -> !key.equals(finalFullPrefix)); + // Combine CommonPrefixes (folders) and Contents (files) across all pages. A single + // listObjectsV2 call returns at most 1000 entries, which would silently truncate the + // children of a large group. + List children = new ArrayList<>(); + for (ListObjectsV2Response res : s3client.listObjectsV2Paginator(req)) { + res.commonPrefixes().forEach(commonPrefix -> children.add(commonPrefix.prefix())); + res.contents().stream() + .map(S3Object::key) + .filter(key -> !key.equals(finalFullPrefix)) + .forEach(children::add); + } - return Stream.concat(folders, files) + return children.stream() .map(k -> keyToRelativeArray(k, finalFullPrefix)[0]); } diff --git a/src/main/java/dev/zarr/zarrjava/v2/Group.java b/src/main/java/dev/zarr/zarrjava/v2/Group.java index 5c946c26..8bc68b4f 100644 --- a/src/main/java/dev/zarr/zarrjava/v2/Group.java +++ b/src/main/java/dev/zarr/zarrjava/v2/Group.java @@ -16,10 +16,7 @@ import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Arrays; -import java.util.Objects; import java.util.function.Function; -import java.util.stream.Stream; import static dev.zarr.zarrjava.v2.Node.makeObjectMapper; import static dev.zarr.zarrjava.v2.Node.makeObjectWriter; @@ -182,26 +179,6 @@ public Node get(String[] key) throws ZarrException, IOException { } } - @Override - public Stream list() { - return storeHandle.list().map(key -> { - if (key.length <= 1) return null; // exclude root from list - String fileName = key[key.length - 1]; - StoreHandle parent = storeHandle.resolve(Arrays.copyOf(key, key.length - 1)); - try { - if (fileName.equals(ZARRAY)) { - return Array.open(parent); - } - if (fileName.equals(ZGROUP)) { - return (dev.zarr.zarrjava.core.Node) Group.open(parent); - } - } catch (Exception e) { - throw new RuntimeException(e); - } - return null; - }).filter(Objects::nonNull); - } - /** * Creates a new subgroup with default metadata at the specified key. * diff --git a/src/main/java/dev/zarr/zarrjava/v3/Group.java b/src/main/java/dev/zarr/zarrjava/v3/Group.java index 8b1a81bb..d9aa5bfb 100644 --- a/src/main/java/dev/zarr/zarrjava/v3/Group.java +++ b/src/main/java/dev/zarr/zarrjava/v3/Group.java @@ -15,9 +15,7 @@ import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.Paths; -import java.util.Arrays; import java.util.function.Function; -import java.util.stream.Stream; import static dev.zarr.zarrjava.v3.Node.makeObjectMapper; import static dev.zarr.zarrjava.v3.Node.makeObjectWriter; @@ -192,25 +190,6 @@ public Node get(String[] key) throws ZarrException, IOException { } } - @Override - public Stream list() { - Stream metadataKeys = storeHandle.list() - .filter(key -> key[key.length - 1].equals(ZARR_JSON)) - .filter(key -> key.length > 1); // exclude root from list - return metadataKeys.map(key -> { - try { - return get(Arrays.copyOf(key, key.length - 1)); - } catch (IOException e) { - throw new RuntimeException( - "Failed to read node metadata for key '" + String.join("/", key) + "': " + e.getMessage(), e); - } catch (ZarrException e) { - throw new RuntimeException( - "Failed to parse node metadata for key '" + String.join("/", key) + "': " + e.getMessage(), e); - } - }); - } - - /** * Creates a new subgroup with the provided metadata at the specified key. * diff --git a/src/test/java/dev/zarr/zarrjava/GroupListTest.java b/src/test/java/dev/zarr/zarrjava/GroupListTest.java new file mode 100644 index 00000000..ee31af66 --- /dev/null +++ b/src/test/java/dev/zarr/zarrjava/GroupListTest.java @@ -0,0 +1,355 @@ +package dev.zarr.zarrjava; + +import dev.zarr.zarrjava.core.AbstractNode; +import dev.zarr.zarrjava.core.Group; +import dev.zarr.zarrjava.core.Node; +import dev.zarr.zarrjava.store.FilesystemStore; +import dev.zarr.zarrjava.store.MemoryStore; +import dev.zarr.zarrjava.store.Store; +import dev.zarr.zarrjava.store.StoreHandle; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import java.io.IOException; +import java.io.InputStream; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** + * Tests that {@link Group#list()} walks the hierarchy level by level instead of enumerating every + * key below the group, and that it still returns the same nodes as before. + */ +public class GroupListTest { + + /** + * A {@link MemoryStore} that counts how often it is asked to list or read, so that tests can + * assert on the number of store operations a group traversal costs. + */ + static final class CountingStore implements Store, Store.ListableStore { + + private final MemoryStore delegate = new MemoryStore(); + final AtomicInteger listCalls = new AtomicInteger(); + final AtomicInteger listChildrenCalls = new AtomicInteger(); + final AtomicInteger readCalls = new AtomicInteger(); + + void resetCounters() { + listCalls.set(0); + listChildrenCalls.set(0); + readCalls.set(0); + } + + @Override + public Stream list(String[] prefix) { + listCalls.incrementAndGet(); + return delegate.list(prefix); + } + + @Override + public Stream listChildren(String[] prefix) { + listChildrenCalls.incrementAndGet(); + return delegate.listChildren(prefix); + } + + @Override + public boolean exists(String[] keys) { + readCalls.incrementAndGet(); + return delegate.exists(keys); + } + + @Nullable + @Override + public ByteBuffer get(String[] keys) { + readCalls.incrementAndGet(); + return delegate.get(keys); + } + + @Nullable + @Override + public ByteBuffer get(String[] keys, long start) { + readCalls.incrementAndGet(); + return delegate.get(keys, start); + } + + @Nullable + @Override + public ByteBuffer get(String[] keys, long start, long end) { + readCalls.incrementAndGet(); + return delegate.get(keys, start, end); + } + + @Override + public void set(String[] keys, ByteBuffer bytes) { + delegate.set(keys, bytes); + } + + @Override + public void delete(String[] keys) { + delegate.delete(keys); + } + + @Nonnull + @Override + public StoreHandle resolve(String... keys) { + return new StoreHandle(this, keys); + } + + @Override + public InputStream getInputStream(String[] keys, long start, long end) { + readCalls.incrementAndGet(); + return delegate.getInputStream(keys, start, end); + } + + @Override + public long getSize(String[] keys) { + return delegate.getSize(keys); + } + + @Override + public String toString() { + return ""; + } + } + + static byte[] testData(int size) { + byte[] data = new byte[size]; + for (int i = 0; i < size; i++) { + data[i] = (byte) i; + } + return data; + } + + /** + * Writes a v3 hierarchy: + * 

+     * /            group
+     * /arr         array (chunked)
+     * /sub         group
+     * /sub/nested  array
+     * /sub/deep    group
+     * /sub/deep/deepArray array
+     *
+ */ + static dev.zarr.zarrjava.v3.Group writeTreeV3(StoreHandle storeHandle, int chunkSize) + throws IOException, ZarrException { + dev.zarr.zarrjava.v3.Group root = dev.zarr.zarrjava.v3.Group.create(storeHandle); + dev.zarr.zarrjava.v3.Array array = root.createArray("arr", b -> b + .withShape(64, 64) + .withDataType(dev.zarr.zarrjava.v3.DataType.UINT8) + .withChunkShape(chunkSize, chunkSize)); + array.write(ucar.ma2.Array.factory(ucar.ma2.DataType.BYTE, new int[]{64, 64}, testData(64 * 64))); + + dev.zarr.zarrjava.v3.Group sub = root.createGroup("sub"); + sub.createArray("nested", b -> b + .withShape(8, 8) + .withDataType(dev.zarr.zarrjava.v3.DataType.UINT8) + .withChunkShape(8, 8)); + dev.zarr.zarrjava.v3.Group deep = sub.createGroup("deep"); + deep.createArray("deepArray", b -> b + .withShape(8, 8) + .withDataType(dev.zarr.zarrjava.v3.DataType.UINT8) + .withChunkShape(8, 8)); + return root; + } + + static dev.zarr.zarrjava.v2.Group writeTreeV2(StoreHandle storeHandle) throws IOException, ZarrException { + dev.zarr.zarrjava.v2.Group root = dev.zarr.zarrjava.v2.Group.create(storeHandle); + dev.zarr.zarrjava.v2.Array array = root.createArray("arr", b -> b + .withShape(64, 64) + .withDataType(dev.zarr.zarrjava.v2.DataType.UINT8) + .withChunks(8, 8)); + array.write(ucar.ma2.Array.factory(ucar.ma2.DataType.BYTE, new int[]{64, 64}, testData(64 * 64))); + + dev.zarr.zarrjava.v2.Group sub = root.createGroup("sub"); + sub.createArray("nested", b -> b + .withShape(8, 8) + .withDataType(dev.zarr.zarrjava.v2.DataType.UINT8) + .withChunks(8, 8)); + dev.zarr.zarrjava.v2.Group deep = sub.createGroup("deep"); + deep.createArray("deepArray", b -> b + .withShape(8, 8) + .withDataType(dev.zarr.zarrjava.v2.DataType.UINT8) + .withChunks(8, 8)); + return root; + } + + static Set pathsOf(Stream nodes) { + return nodes + .map(node -> String.join("/", ((AbstractNode) node).storeHandle.keys)) + .collect(Collectors.toSet()); + } + + private static final Set EXPECTED_DESCENDANTS = new HashSet<>(Arrays.asList( + "arr", + "sub", + "sub/nested", + "sub/deep", + "sub/deep/deepArray" + )); + + @Test + public void testListReturnsAllDescendantsV3() throws IOException, ZarrException { + CountingStore store = new CountingStore(); + Group root = writeTreeV3(store.resolve(), 8); + + Assertions.assertEquals(EXPECTED_DESCENDANTS, pathsOf(root.list())); + } + + @Test + public void testListReturnsAllDescendantsV2() throws IOException, ZarrException { + CountingStore store = new CountingStore(); + Group root = writeTreeV2(store.resolve()); + + Assertions.assertEquals(EXPECTED_DESCENDANTS, pathsOf(root.list())); + } + + @Test + public void testListDoesNotEnumerateKeys() throws IOException, ZarrException { + CountingStore store = new CountingStore(); + Group root = writeTreeV3(store.resolve(), 8); + + store.resetCounters(); + Assertions.assertEquals(5, root.listAsArray().length); + + Assertions.assertEquals(0, store.listCalls.get(), + "list() must not use the recursive store listing"); + // One listing per group in the tree: the root, sub and sub/deep. + Assertions.assertEquals(3, store.listChildrenCalls.get()); + } + + /** + * The whole point of the level-wise walk: the cost of listing a group must not grow with the + * number of chunks the arrays below it have. + */ + @ParameterizedTest + @CsvSource({"64", "32", "8", "2"}) + public void testListCostIsIndependentOfChunkCount(int chunkSize) throws IOException, ZarrException { + CountingStore store = new CountingStore(); + Group root = writeTreeV3(store.resolve(), chunkSize); + + store.resetCounters(); + Assertions.assertEquals(EXPECTED_DESCENDANTS, pathsOf(root.list())); + + Assertions.assertEquals(0, store.listCalls.get()); + Assertions.assertEquals(3, store.listChildrenCalls.get()); + // One metadata read per node found, none for chunks. + Assertions.assertEquals(5, store.readCalls.get()); + } + + @Test + public void testMembersReturnsOnlyDirectChildren() throws IOException, ZarrException { + CountingStore store = new CountingStore(); + Group root = writeTreeV3(store.resolve(), 8); + + store.resetCounters(); + Assertions.assertEquals(new HashSet<>(Arrays.asList("arr", "sub")), pathsOf(root.members())); + Assertions.assertEquals(1, store.listChildrenCalls.get()); + Assertions.assertEquals(0, store.listCalls.get()); + + Group sub = (Group) root.get("sub"); + Assertions.assertNotNull(sub); + Assertions.assertEquals(new HashSet<>(Arrays.asList("sub/nested", "sub/deep")), pathsOf(sub.members())); + } + + /** + * A directory that is not a node itself may still contain nodes further down. The previous + * implementation found those because it walked all keys, so keep finding them. + */ + @Test + public void testListFindsNodesBelowNonNodeDirectories() throws IOException, ZarrException { + CountingStore store = new CountingStore(); + dev.zarr.zarrjava.v3.Group root = dev.zarr.zarrjava.v3.Group.create(store.resolve()); + dev.zarr.zarrjava.v3.Group.create(store.resolve("plain", "inner")); + + Assertions.assertEquals(new HashSet<>(Arrays.asList("plain/inner")), pathsOf(root.list())); + } + + /** + * A group directory can contain plain files next to its nodes. Probing those must not fail, + * even on a filesystem, where reading through a file rather than a directory reports + * "Not a directory" instead of "No such file". + */ + @Test + public void testListSkipsPlainFilesInGroup(@TempDir Path tempDir) throws IOException, ZarrException { + FilesystemStore store = new FilesystemStore(tempDir); + dev.zarr.zarrjava.v3.Group root = dev.zarr.zarrjava.v3.Group.create(store.resolve()); + root.createGroup("sub"); + Files.write(tempDir.resolve("properties.json"), "{}".getBytes(StandardCharsets.UTF_8)); + + Assertions.assertEquals(new HashSet<>(Arrays.asList("sub")), pathsOf(root.list())); + Assertions.assertNull(store.get(new String[]{"properties.json", "zarr.json"})); + } + + @Test + public void testListOnNonListableStoreThrows() throws IOException, ZarrException { + MemoryStore memoryStore = new MemoryStore(); + dev.zarr.zarrjava.v3.Group.create(memoryStore.resolve()); + + Store nonListable = new Store() { + @Override + public boolean exists(String[] keys) { + return memoryStore.exists(keys); + } + + @Nullable + @Override + public ByteBuffer get(String[] keys) { + return memoryStore.get(keys); + } + + @Nullable + @Override + public ByteBuffer get(String[] keys, long start) { + return memoryStore.get(keys, start); + } + + @Nullable + @Override + public ByteBuffer get(String[] keys, long start, long end) { + return memoryStore.get(keys, start, end); + } + + @Override + public void set(String[] keys, ByteBuffer bytes) { + memoryStore.set(keys, bytes); + } + + @Override + public void delete(String[] keys) { + memoryStore.delete(keys); + } + + @Nonnull + @Override + public StoreHandle resolve(String... keys) { + return new StoreHandle(this, keys); + } + + @Override + public InputStream getInputStream(String[] keys, long start, long end) { + return memoryStore.getInputStream(keys, start, end); + } + + @Override + public long getSize(String[] keys) { + return memoryStore.getSize(keys); + } + }; + + Group group = dev.zarr.zarrjava.v3.Group.open(nonListable.resolve()); + Assertions.assertThrows(UnsupportedOperationException.class, group::list); + Assertions.assertThrows(UnsupportedOperationException.class, group::members); + } +} diff --git a/src/test/java/dev/zarr/zarrjava/store/S3StoreTest.java b/src/test/java/dev/zarr/zarrjava/store/S3StoreTest.java index 8d4c69a7..c90cd722 100644 --- a/src/test/java/dev/zarr/zarrjava/store/S3StoreTest.java +++ b/src/test/java/dev/zarr/zarrjava/store/S3StoreTest.java @@ -18,6 +18,9 @@ import java.io.InputStream; import java.net.URI; import java.nio.ByteBuffer; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.IntStream; /** * Tests for S3Store @@ -109,4 +112,20 @@ StoreHandle storeHandleWithoutData() { Store storeWithArrays() { return new S3Store(s3Client, bucketName, "storeWithArrays"); } + + /** + * A single ListObjectsV2 response holds at most 1000 entries, so listing the children of a + * group with more than 1000 of them has to follow the continuation token. + */ + @Test + void testListChildrenIsPaginated() { + int childCount = 1001; + S3Store store = new S3Store(s3Client, bucketName, "manyChildren"); + IntStream.range(0, childCount).parallel().forEach(i -> + store.resolve("child" + i, "data").set(ByteBuffer.allocate(1))); + + List children = store.listChildren(new String[0]).collect(Collectors.toList()); + Assertions.assertEquals(childCount, children.size()); + Assertions.assertTrue(children.contains("child" + (childCount - 1))); + } } From 73cba817db4ddcd3bcbd9b0b480a067bca4f4ae7 Mon Sep 17 00:00:00 2001 From: konstibob <44369572+konstibob@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:10:17 +0200 Subject: [PATCH 2/6] Update src/main/java/dev/zarr/zarrjava/core/Group.java Co-authored-by: Norman Rzepka --- src/main/java/dev/zarr/zarrjava/core/Group.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/dev/zarr/zarrjava/core/Group.java b/src/main/java/dev/zarr/zarrjava/core/Group.java index 9d3a73c5..eb122da2 100644 --- a/src/main/java/dev/zarr/zarrjava/core/Group.java +++ b/src/main/java/dev/zarr/zarrjava/core/Group.java @@ -110,7 +110,7 @@ public Node[] membersAsArray() { } /** - * Lists all descendants (arrays and groups) of this group, at any depth. + * Recursively lists all descendants (arrays and groups) of this group, at any depth. *

* The group hierarchy is walked one level at a time, so only group keys are listed and chunk * keys are never enumerated. Descending into an array is not necessary and does not happen. From 4f7b58de9ce0ba7d3a889bc065fbecaee79ab5a6 Mon Sep 17 00:00:00 2001 From: konstibob <44369572+konstibob@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:12:03 +0200 Subject: [PATCH 3/6] Update src/main/java/dev/zarr/zarrjava/core/Group.java Co-authored-by: Norman Rzepka --- src/main/java/dev/zarr/zarrjava/core/Group.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/dev/zarr/zarrjava/core/Group.java b/src/main/java/dev/zarr/zarrjava/core/Group.java index eb122da2..21547261 100644 --- a/src/main/java/dev/zarr/zarrjava/core/Group.java +++ b/src/main/java/dev/zarr/zarrjava/core/Group.java @@ -163,7 +163,7 @@ private List childKeys(String[] prefix) { * Opens the node at {@code key}, or returns null if there is no node there. */ @Nullable - private Node openChild(String[] key) { + private Node openDescendant(String[] key) { try { return get(key); } catch (IOException e) { From 6eff02de467ffd847a521f66b5039aeda5755002 Mon Sep 17 00:00:00 2001 From: konstibob <44369572+konstibob@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:12:13 +0200 Subject: [PATCH 4/6] Update src/main/java/dev/zarr/zarrjava/core/Group.java Co-authored-by: Norman Rzepka --- src/main/java/dev/zarr/zarrjava/core/Group.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/dev/zarr/zarrjava/core/Group.java b/src/main/java/dev/zarr/zarrjava/core/Group.java index 21547261..b0309291 100644 --- a/src/main/java/dev/zarr/zarrjava/core/Group.java +++ b/src/main/java/dev/zarr/zarrjava/core/Group.java @@ -150,7 +150,7 @@ private Stream listDescendants(String[] prefix) { * Lists the keys directly below {@code prefix} that may hold a child node, relative to this * group. */ - private List childKeys(String[] prefix) { + private List descendantKeys(String[] prefix) { try (Stream children = storeHandle.resolve(prefix).listChildren()) { return children .filter(name -> !METADATA_KEYS.contains(name)) From 7a6d494faea2f207a2da842f31cf5bfff71344fb Mon Sep 17 00:00:00 2001 From: Konstantin Date: Thu, 24 Sep 2026 11:05:52 +0200 Subject: [PATCH 5/6] Fix call sites after childKeys/openChild rename The renames in the previous two commits only touched the method declarations, leaving members() and listDescendants() calling names that no longer exist, so the module did not compile. Co-Authored-By: Claude Opus 5 (1M context) --- src/main/java/dev/zarr/zarrjava/core/Group.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/dev/zarr/zarrjava/core/Group.java b/src/main/java/dev/zarr/zarrjava/core/Group.java index b0309291..550d0fc8 100644 --- a/src/main/java/dev/zarr/zarrjava/core/Group.java +++ b/src/main/java/dev/zarr/zarrjava/core/Group.java @@ -96,8 +96,8 @@ public Node get(String key) throws ZarrException, IOException { * @throws UnsupportedOperationException if the underlying store does not support listing */ public Stream members() { - return childKeys(new String[0]).parallelStream() - .map(this::openChild) + return descendantKeys(new String[0]).parallelStream() + .map(this::openDescendant) .filter(Objects::nonNull) .collect(Collectors.toList()) .stream(); @@ -129,8 +129,8 @@ public Node[] listAsArray() { } private Stream listDescendants(String[] prefix) { - List> children = childKeys(prefix).parallelStream() - .map(key -> new AbstractMap.SimpleEntry(key, openChild(key))) + List> children = descendantKeys(prefix).parallelStream() + .map(key -> new AbstractMap.SimpleEntry(key, openDescendant(key))) .collect(Collectors.toList()); return children.stream().flatMap(child -> { From 28ebf40cdd1b4b9d67a743b95f72c929d1e7cafd Mon Sep 17 00:00:00 2001 From: Konstantin Date: Thu, 24 Sep 2026 11:06:10 +0200 Subject: [PATCH 6/6] Move metadata key set into the version-specific Group classes core.Group filtered a single set containing both v2 and v3 metadata keys, so each version also skipped the other version's keys. Replace it with an abstract metadataKeys() that v2 answers with .zgroup/.zattrs/.zarray and v3 with zarr.json. Co-Authored-By: Claude Opus 5 (1M context) --- src/main/java/dev/zarr/zarrjava/core/Group.java | 17 +++++++---------- src/main/java/dev/zarr/zarrjava/v2/Group.java | 16 ++++++++++++++++ src/main/java/dev/zarr/zarrjava/v3/Group.java | 12 ++++++++++++ 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/src/main/java/dev/zarr/zarrjava/core/Group.java b/src/main/java/dev/zarr/zarrjava/core/Group.java index 550d0fc8..50537a31 100644 --- a/src/main/java/dev/zarr/zarrjava/core/Group.java +++ b/src/main/java/dev/zarr/zarrjava/core/Group.java @@ -11,9 +11,6 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.AbstractMap; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; @@ -23,12 +20,6 @@ public abstract class Group extends AbstractNode { - /** - * Keys that hold metadata of the group itself and never point at a child node. - */ - private static final Set METADATA_KEYS = Collections.unmodifiableSet( - new HashSet<>(Arrays.asList(ZARR_JSON, ZARRAY, ZATTRS, ZGROUP))); - protected Group(@Nonnull StoreHandle storeHandle) { super(storeHandle); } @@ -86,6 +77,11 @@ public Node get(String key) throws ZarrException, IOException { return get(new String[]{key}); } + /** + * Keys that hold metadata of the group itself and never point at a child node. + */ + protected abstract Set metadataKeys(); + /** * Lists the immediate children (arrays and subgroups) of this group. *

@@ -151,9 +147,10 @@ private Stream listDescendants(String[] prefix) { * group. */ private List descendantKeys(String[] prefix) { + Set metadataKeys = metadataKeys(); try (Stream children = storeHandle.resolve(prefix).listChildren()) { return children - .filter(name -> !METADATA_KEYS.contains(name)) + .filter(name -> !metadataKeys.contains(name)) .map(name -> Utils.concatArrays(prefix, new String[]{name})) .collect(Collectors.toList()); } diff --git a/src/main/java/dev/zarr/zarrjava/v2/Group.java b/src/main/java/dev/zarr/zarrjava/v2/Group.java index 8bc68b4f..e1881101 100644 --- a/src/main/java/dev/zarr/zarrjava/v2/Group.java +++ b/src/main/java/dev/zarr/zarrjava/v2/Group.java @@ -16,12 +16,23 @@ import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; import java.util.function.Function; import static dev.zarr.zarrjava.v2.Node.makeObjectMapper; import static dev.zarr.zarrjava.v2.Node.makeObjectWriter; public class Group extends dev.zarr.zarrjava.core.Group implements Node { + + /** + * Keys that hold metadata of a v2 group itself and never point at a child node. + */ + private static final Set METADATA_KEYS = Collections.unmodifiableSet( + new HashSet<>(Arrays.asList(ZGROUP, ZATTRS, ZARRAY))); + public GroupMetadata metadata; protected Group(@Nonnull StoreHandle storeHandle, @Nonnull GroupMetadata groupMetadata) { @@ -270,6 +281,11 @@ public String toString() { return String.format("", storeHandle); } + @Override + protected Set metadataKeys() { + return METADATA_KEYS; + } + @Override public GroupMetadata metadata() { return metadata; diff --git a/src/main/java/dev/zarr/zarrjava/v3/Group.java b/src/main/java/dev/zarr/zarrjava/v3/Group.java index d9aa5bfb..6d3736ad 100644 --- a/src/main/java/dev/zarr/zarrjava/v3/Group.java +++ b/src/main/java/dev/zarr/zarrjava/v3/Group.java @@ -15,6 +15,8 @@ import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Collections; +import java.util.Set; import java.util.function.Function; import static dev.zarr.zarrjava.v3.Node.makeObjectMapper; @@ -23,6 +25,11 @@ public class Group extends dev.zarr.zarrjava.core.Group implements Node { + /** + * Keys that hold metadata of a v3 group itself and never point at a child node. + */ + private static final Set METADATA_KEYS = Collections.singleton(ZARR_JSON); + public GroupMetadata metadata; protected Group(@Nonnull StoreHandle storeHandle, @Nonnull GroupMetadata groupMetadata) throws IOException { @@ -290,6 +297,11 @@ public String toString() { return String.format("", storeHandle); } + @Override + protected Set metadataKeys() { + return METADATA_KEYS; + } + @Override public GroupMetadata metadata() { return metadata;