From 8697775172a830de78a5760a03dca2d9f99246a1 Mon Sep 17 00:00:00 2001 From: Konstantin Date: Thu, 27 Aug 2026 15:08:57 +0200 Subject: [PATCH 1/4] Ignore v3 metadata members declaring must_understand: false A Zarr v3 writer may add members to a metadata document that a reader does not know about. When such a member is a JSON object carrying "must_understand": false, the specification requires a reader to ignore it; any other unknown member has to be rejected, because it may change how the array is to be interpreted. zarr-java rejected every unknown member instead, because the ObjectMapper in v3.Node was built with Jackson's default FAIL_ON_UNKNOWN_PROPERTIES. Reading a zarr.json written by a newer implementation therefore failed with an UnrecognizedPropertyException on a member that was explicitly marked as safe to skip. Unknown members are now collected through a @JsonAnySetter creator parameter and validated in ExtraFields, which mirrors zarr-python 3.1.6 (zarr/core/metadata/v3.py): - a JSON object with "must_understand": false is ignored, - anything else is rejected with a ZarrException, - an extra field colliding with a member the document defines itself is rejected. Ignored members are kept on the metadata and written back out by a @JsonAnyGetter, so that another implementation's extension survives a metadata rewrite. ArrayMetadataBuilder.fromArrayMetadata and Group.setAttributes carry them over, which covers resize, setAttributes and updateAttributes. must_understand deliberately does not apply to codec, chunk grid or chunk key encoding names: a codec cannot be skipped and still leave the chunk bytes decodable, so an unknown name there still fails. Verified against zarr-python 3.1.6 in both directions: a document it writes with an extra field now opens, and a document zarr-java rewrites is read back by zarr-python with the field intact in its own extra_fields. Co-Authored-By: Claude Opus 5 (1M context) --- .../dev/zarr/zarrjava/v3/ArrayMetadata.java | 59 +++- .../zarrjava/v3/ArrayMetadataBuilder.java | 16 +- .../dev/zarr/zarrjava/v3/ExtraFields.java | 104 ++++++ src/main/java/dev/zarr/zarrjava/v3/Group.java | 3 +- .../dev/zarr/zarrjava/v3/GroupMetadata.java | 40 ++- .../dev/zarr/zarrjava/ExtraFieldsTest.java | 321 ++++++++++++++++++ 6 files changed, 538 insertions(+), 5 deletions(-) create mode 100644 src/main/java/dev/zarr/zarrjava/v3/ExtraFields.java create mode 100644 src/test/java/dev/zarr/zarrjava/ExtraFieldsTest.java diff --git a/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadata.java b/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadata.java index 58d57335..dbabf86c 100644 --- a/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadata.java +++ b/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadata.java @@ -1,5 +1,7 @@ package dev.zarr.zarrjava.v3; +import com.fasterxml.jackson.annotation.JsonAnyGetter; +import com.fasterxml.jackson.annotation.JsonAnySetter; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonProperty; @@ -48,6 +50,14 @@ public final class ArrayMetadata extends dev.zarr.zarrjava.core.ArrayMetadata { @JsonProperty("storage_transformers") public final Map[] storageTransformers; + /** + * Members of the metadata document that zarr-java does not know about. The Zarr v3 specification + * requires that these are ignored when they declare {@code "must_understand": false}, and that + * they are rejected otherwise. They are kept here so that rewriting the metadata does not drop + * extensions written by another implementation. + */ + private final Map extraFields; + @JsonIgnore public CoreArrayMetadata coreArrayMetadata; @@ -65,6 +75,39 @@ public ArrayMetadata( ); } + public ArrayMetadata( + long[] shape, DataType dataType, ChunkGrid chunkGrid, ChunkKeyEncoding chunkKeyEncoding, + Object fillValue, + @Nonnull Codec[] codecs, + @Nullable String[] dimensionNames, + @Nullable Attributes attributes, + @Nullable Map[] storageTransformers, + @Nullable Map extraFields + ) throws ZarrException { + this(ZARR_FORMAT, NODE_TYPE, shape, dataType, chunkGrid, chunkKeyEncoding, fillValue, codecs, + dimensionNames, + attributes, storageTransformers, extraFields + ); + } + + public ArrayMetadata( + int zarrFormat, + String nodeType, + long[] shape, + DataType dataType, + ChunkGrid chunkGrid, + ChunkKeyEncoding chunkKeyEncoding, + Object fillValue, + @Nonnull Codec[] codecs, + @Nullable String[] dimensionNames, + @Nullable Attributes attributes, + @Nullable Map[] storageTransformers + ) throws ZarrException { + this(zarrFormat, nodeType, shape, dataType, chunkGrid, chunkKeyEncoding, fillValue, codecs, + dimensionNames, attributes, storageTransformers, null + ); + } + @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) public ArrayMetadata( @JsonProperty(value = "zarr_format", required = true) int zarrFormat, @@ -77,9 +120,11 @@ public ArrayMetadata( @Nonnull @JsonProperty(value = "codecs") Codec[] codecs, @Nullable @JsonProperty(value = "dimension_names") String[] dimensionNames, @Nullable @JsonProperty(value = "attributes") Attributes attributes, - @Nullable @JsonProperty(value = "storage_transformers") Map[] storageTransformers + @Nullable @JsonProperty(value = "storage_transformers") Map[] storageTransformers, + @Nullable @JsonAnySetter Map extraFields ) throws ZarrException { super(shape, fillValue, dataType); + this.extraFields = ExtraFields.validated(extraFields, ExtraFields.ARRAY_METADATA_KEYS); if (zarrFormat != this.zarrFormat) { throw new ZarrException( "Expected zarr format '" + this.zarrFormat + "', got '" + zarrFormat + "'."); @@ -129,6 +174,18 @@ public ArrayMetadata( this.storageTransformers = storageTransformers; } + /** + * The members of the metadata document that zarr-java does not know about, but that declared + * {@code "must_understand": false} and could therefore be ignored. They are written back out + * unchanged, so that extensions written by another implementation survive a metadata rewrite. + * + * @return the extra fields, never {@code null} + */ + @JsonAnyGetter + public Map extraFields() { + return extraFields; + } + public static Optional getShardingIndexedCodec(Codec[] codecs) { return Arrays.stream(codecs).filter(codec -> codec instanceof ShardingIndexedCodec).findFirst(); } diff --git a/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadataBuilder.java b/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadataBuilder.java index 5d5f4941..fd6b5c37 100644 --- a/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadataBuilder.java +++ b/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadataBuilder.java @@ -31,6 +31,7 @@ public class ArrayMetadataBuilder { Attributes attributes = new Attributes(); Map[] storageTransformers = new HashMap[]{}; String[] dimensionNames = null; + Map extraFields = null; protected ArrayMetadataBuilder() { } @@ -49,6 +50,7 @@ protected static ArrayMetadataBuilder fromArrayMetadata(ArrayMetadata arrayMetad builder.codecs = arrayMetadata.codecs; builder.dimensionNames = arrayMetadata.dimensionNames; builder.storageTransformers = arrayMetadata.storageTransformers; + builder.extraFields = arrayMetadata.extraFields(); if (withAttributes) { builder.attributes = arrayMetadata.attributes; } @@ -155,6 +157,17 @@ public ArrayMetadataBuilder withStorageTransformers(Map[] storag return this; } + /** + * Sets members of the metadata document that zarr-java itself does not interpret. Every value + * needs to be a map carrying {@code "must_understand": false}, otherwise {@link #build()} fails. + * + * @param extraFields the extra fields to write into the metadata document + */ + public ArrayMetadataBuilder withExtraFields(Map extraFields) { + this.extraFields = extraFields; + return this; + } + public ArrayMetadata build() throws ZarrException { if (shape == null) { throw new ZarrException("Shape needs to be provided. Please call `.withShape`."); @@ -172,7 +185,8 @@ public ArrayMetadata build() throws ZarrException { return new ArrayMetadata(shape, dataType, chunkGrid, chunkKeyEncoding, fillValue, codecs, dimensionNames, attributes, - storageTransformers + storageTransformers, + extraFields ); } } diff --git a/src/main/java/dev/zarr/zarrjava/v3/ExtraFields.java b/src/main/java/dev/zarr/zarrjava/v3/ExtraFields.java new file mode 100644 index 00000000..6c1b8433 --- /dev/null +++ b/src/main/java/dev/zarr/zarrjava/v3/ExtraFields.java @@ -0,0 +1,104 @@ +package dev.zarr.zarrjava.v3; + +import dev.zarr.zarrjava.ZarrException; + +import javax.annotation.Nullable; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; + +/** + * Handling of unknown ("extra") members of a Zarr v3 metadata document. + * + *

The Zarr v3 specification allows a writer to add members that a reader may not know about. Such + * a member has to be a JSON object carrying {@code "must_understand": false}, which declares that a + * reader that does not know the member may safely ignore it. Any other unknown member has to be + * rejected, because it may change how the array is to be interpreted. + * + *

These semantics match zarr-python 3.1.6, see {@code zarr/core/metadata/v3.py}. + */ +final class ExtraFields { + + static final String MUST_UNDERSTAND = "must_understand"; + + /** + * Members of a v3 array metadata document that zarr-java knows about. Anything else read from a + * {@code zarr.json} array document is an extra field. + */ + static final Set ARRAY_METADATA_KEYS = unmodifiableSetOf( + "zarr_format", "node_type", "shape", "data_type", "chunk_grid", "chunk_key_encoding", + "fill_value", "codecs", "attributes", "dimension_names", "storage_transformers" + ); + + /** + * Members of a v3 group metadata document that zarr-java knows about. Anything else read from a + * {@code zarr.json} group document is an extra field. + */ + static final Set GROUP_METADATA_KEYS = unmodifiableSetOf( + "zarr_format", "node_type", "attributes", "consolidated_metadata" + ); + + private ExtraFields() { + } + + private static Set unmodifiableSetOf(String... keys) { + return Collections.unmodifiableSet(new HashSet<>(Arrays.asList(keys))); + } + + /** + * Whether an unknown metadata member may be ignored, i.e. whether it is a JSON object with a + * {@code must_understand} member that is set to {@code false}. + */ + static boolean isIgnorable(@Nullable Object value) { + return value instanceof Map + && Boolean.FALSE.equals(((Map) value).get(MUST_UNDERSTAND)); + } + + /** + * Validates unknown members of a metadata document and returns them so that they can be written + * back out unchanged. + * + * @param extraFields the unknown members, may be {@code null} + * @param reservedKeys the members that the metadata document defines itself, either + * {@link #ARRAY_METADATA_KEYS} or {@link #GROUP_METADATA_KEYS} + * @return the extra fields, never {@code null} + * @throws ZarrException if a member collides with a reserved key, or if a member may not be + * ignored because it is not a JSON object carrying + * {@code "must_understand": false} + */ + static Map validated( + @Nullable Map extraFields, Set reservedKeys + ) throws ZarrException { + if (extraFields == null || extraFields.isEmpty()) { + return Collections.emptyMap(); + } + final List reservedCollisions = new ArrayList<>(); + final List notIgnorable = new ArrayList<>(); + for (Map.Entry entry : extraFields.entrySet()) { + if (reservedKeys.contains(entry.getKey())) { + reservedCollisions.add(entry.getKey()); + } else if (!isIgnorable(entry.getValue())) { + notIgnorable.add(entry.getKey()); + } + } + if (!reservedCollisions.isEmpty()) { + Collections.sort(reservedCollisions); + throw new ZarrException( + "Invalid extra fields. The following keys: " + reservedCollisions + " are invalid " + + "because they collide with keys reserved for use by the metadata document."); + } + if (!notIgnorable.isEmpty()) { + Collections.sort(notIgnorable); + throw new ZarrException( + "Got a Zarr v3 metadata document with the following disallowed extra fields: " + + notIgnorable + ". Extra fields are not allowed unless they are a JSON object " + + "with a \"" + MUST_UNDERSTAND + "\" key which is assigned the value `false`."); + } + return Collections.unmodifiableMap(new LinkedHashMap<>(extraFields)); + } +} diff --git a/src/main/java/dev/zarr/zarrjava/v3/Group.java b/src/main/java/dev/zarr/zarrjava/v3/Group.java index 8b1a81bb..6a3547f7 100644 --- a/src/main/java/dev/zarr/zarrjava/v3/Group.java +++ b/src/main/java/dev/zarr/zarrjava/v3/Group.java @@ -302,7 +302,8 @@ public Group updateAttributes(Function attributeMapper) * @throws IOException if the metadata cannot be serialized */ public Group setAttributes(Attributes newAttributes) throws ZarrException, IOException { - GroupMetadata newGroupMetadata = new GroupMetadata(newAttributes); + GroupMetadata newGroupMetadata = + new GroupMetadata(newAttributes, metadata.extraFields()); return writeMetadata(newGroupMetadata); } diff --git a/src/main/java/dev/zarr/zarrjava/v3/GroupMetadata.java b/src/main/java/dev/zarr/zarrjava/v3/GroupMetadata.java index 56414a2e..6283bc6e 100644 --- a/src/main/java/dev/zarr/zarrjava/v3/GroupMetadata.java +++ b/src/main/java/dev/zarr/zarrjava/v3/GroupMetadata.java @@ -1,5 +1,7 @@ package dev.zarr.zarrjava.v3; +import com.fasterxml.jackson.annotation.JsonAnyGetter; +import com.fasterxml.jackson.annotation.JsonAnySetter; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import dev.zarr.zarrjava.ZarrException; @@ -7,6 +9,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import java.util.Map; public final class GroupMetadata extends dev.zarr.zarrjava.core.GroupMetadata { @@ -22,15 +25,35 @@ public final class GroupMetadata extends dev.zarr.zarrjava.core.GroupMetadata { @Nullable public final Attributes attributes; + /** + * Members of the metadata document that zarr-java does not know about. The Zarr v3 specification + * requires that these are ignored when they declare {@code "must_understand": false}, and that + * they are rejected otherwise. They are kept here so that rewriting the metadata does not drop + * extensions written by another implementation. + */ + private final Map extraFields; + public GroupMetadata(@Nullable Attributes attributes) throws ZarrException { - this(ZARR_FORMAT, NODE_TYPE, attributes); + this(ZARR_FORMAT, NODE_TYPE, attributes, null); + } + + public GroupMetadata( + @Nullable Attributes attributes, @Nullable Map extraFields + ) throws ZarrException { + this(ZARR_FORMAT, NODE_TYPE, attributes, extraFields); + } + + public GroupMetadata(int zarrFormat, String nodeType, @Nullable Attributes attributes) + throws ZarrException { + this(zarrFormat, nodeType, attributes, null); } @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) public GroupMetadata( @JsonProperty(value = "zarr_format", required = true) int zarrFormat, @JsonProperty(value = "node_type", required = true) String nodeType, - @Nullable @JsonProperty(value = "attributes") Attributes attributes + @Nullable @JsonProperty(value = "attributes") Attributes attributes, + @Nullable @JsonAnySetter Map extraFields ) throws ZarrException { if (zarrFormat != this.zarrFormat) { throw new ZarrException( @@ -41,6 +64,7 @@ public GroupMetadata( "Expected node type '" + this.nodeType + "', got '" + nodeType + "'."); } this.attributes = attributes; + this.extraFields = ExtraFields.validated(extraFields, ExtraFields.GROUP_METADATA_KEYS); } public static GroupMetadata defaultValue() { @@ -53,6 +77,18 @@ public static GroupMetadata defaultValue() { } } + /** + * The members of the metadata document that zarr-java does not know about, but that declared + * {@code "must_understand": false} and could therefore be ignored. They are written back out + * unchanged, so that extensions written by another implementation survive a metadata rewrite. + * + * @return the extra fields, never {@code null} + */ + @JsonAnyGetter + public Map extraFields() { + return extraFields; + } + @Override public @Nonnull Attributes attributes() throws ZarrException { if (attributes == null) { diff --git a/src/test/java/dev/zarr/zarrjava/ExtraFieldsTest.java b/src/test/java/dev/zarr/zarrjava/ExtraFieldsTest.java new file mode 100644 index 00000000..5d42dca2 --- /dev/null +++ b/src/test/java/dev/zarr/zarrjava/ExtraFieldsTest.java @@ -0,0 +1,321 @@ +package dev.zarr.zarrjava; + +import com.fasterxml.jackson.databind.ObjectMapper; +import dev.zarr.zarrjava.core.Attributes; +import dev.zarr.zarrjava.store.FilesystemStore; +import dev.zarr.zarrjava.store.StoreHandle; +import dev.zarr.zarrjava.v3.Array; +import dev.zarr.zarrjava.v3.ArrayMetadata; +import dev.zarr.zarrjava.v3.DataType; +import dev.zarr.zarrjava.v3.Group; +import dev.zarr.zarrjava.v3.GroupMetadata; +import dev.zarr.zarrjava.v3.Node; +import org.junit.jupiter.api.Assertions; +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.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.Map; + +/** + * Unknown members of a Zarr v3 metadata document. + * + *

The specification requires a reader to ignore an unknown member that declares + * {@code "must_understand": false}, and to reject any other unknown member. These tests pin that + * behaviour, including that an ignored member survives a metadata rewrite. The semantics match + * zarr-python 3.1.6, see {@code zarr/core/metadata/v3.py}. + */ +public class ExtraFieldsTest extends ZarrTest { + + /** + * A minimal but valid v3 array metadata document. {@code %s} is a placeholder for additional + * members, so that a test can inject an extra field. + */ + private static final String ARRAY_METADATA_TEMPLATE = "{" + + "\"zarr_format\":3," + + "\"node_type\":\"array\"," + + "\"shape\":[4]," + + "\"data_type\":\"uint8\"," + + "\"chunk_grid\":{\"name\":\"regular\",\"configuration\":{\"chunk_shape\":[2]}}," + + "\"chunk_key_encoding\":{\"name\":\"default\"}," + + "\"fill_value\":0," + + "\"codecs\":[{\"name\":\"bytes\",\"configuration\":{\"endian\":\"little\"}}]" + + "%s}"; + + private static final String GROUP_METADATA_TEMPLATE = + "{\"zarr_format\":3,\"node_type\":\"group\",\"attributes\":{}%s}"; + + private static String arrayMetadata(String extraMembers) { + return String.format(ARRAY_METADATA_TEMPLATE, extraMembers); + } + + private static String groupMetadata(String extraMembers) { + return String.format(GROUP_METADATA_TEMPLATE, extraMembers); + } + + private static ObjectMapper objectMapper() { + return Node.makeObjectMapper(); + } + + // --------------------------------------------------------------------------------------------- + // (a) an ignorable extra field is accepted and round-trips + // --------------------------------------------------------------------------------------------- + + @Test + public void testArrayAcceptsIgnorableExtraField() throws Exception { + ArrayMetadata metadata = objectMapper().readValue( + arrayMetadata(",\"some_future_field\":{\"must_understand\":false,\"detail\":42}"), + ArrayMetadata.class); + + Assertions.assertEquals(1, metadata.extraFields().size(), + "the unknown member should have been captured as an extra field"); + Object extra = metadata.extraFields().get("some_future_field"); + Assertions.assertTrue(extra instanceof Map); + Assertions.assertEquals(Boolean.FALSE, ((Map) extra).get("must_understand")); + Assertions.assertEquals(42, ((Map) extra).get("detail")); + } + + @Test + public void testArrayExtraFieldIsWrittenBackOut() throws Exception { + ObjectMapper objectMapper = objectMapper(); + ArrayMetadata metadata = objectMapper.readValue( + arrayMetadata(",\"some_future_field\":{\"must_understand\":false,\"detail\":42}"), + ArrayMetadata.class); + + String serialized = objectMapper.writeValueAsString(metadata); + + // The extra field has to be written back as a top-level member, not nested under a property + // named after the Java field, and not dropped. + Map reparsed = objectMapper.readValue(serialized, Map.class); + Assertions.assertTrue(reparsed.containsKey("some_future_field"), + "extra field was dropped on write, serialized document: " + serialized); + Assertions.assertFalse(reparsed.containsKey("extraFields"), + "extra fields leaked as their own property, serialized document: " + serialized); + Assertions.assertEquals(Boolean.FALSE, + ((Map) reparsed.get("some_future_field")).get("must_understand")); + Assertions.assertEquals(42, ((Map) reparsed.get("some_future_field")).get("detail")); + + // And it survives a second parse, i.e. the round-trip is stable. + ArrayMetadata roundTripped = objectMapper.readValue(serialized, ArrayMetadata.class); + Assertions.assertEquals(metadata.extraFields(), roundTripped.extraFields()); + } + + @Test + public void testGroupAcceptsAndRoundTripsIgnorableExtraField() throws Exception { + ObjectMapper objectMapper = objectMapper(); + GroupMetadata metadata = objectMapper.readValue( + groupMetadata(",\"some_future_field\":{\"must_understand\":false}"), + GroupMetadata.class); + + Assertions.assertEquals(1, metadata.extraFields().size()); + + Map reparsed = objectMapper.readValue( + objectMapper.writeValueAsString(metadata), Map.class); + Assertions.assertTrue(reparsed.containsKey("some_future_field")); + } + + /** + * An extra field has to survive a metadata rewrite, otherwise updating an array's attributes + * would silently discard another implementation's extension. + */ + @Test + public void testArrayExtraFieldSurvivesMetadataRewrite() throws Exception { + StoreHandle storeHandle = + new FilesystemStore(TESTOUTPUT).resolve("extraFields", "arrayRewrite"); + Path zarrJson = storeHandle.resolve("zarr.json").toPath(); + + // Write a document with an extra field directly, then open it through the public API. + storeHandle.resolve("zarr.json").set(ByteBuffer.wrap( + arrayMetadata(",\"some_future_field\":{\"must_understand\":false,\"detail\":42}") + .getBytes(StandardCharsets.UTF_8))); + + Array array = Array.open(storeHandle); + Assertions.assertEquals(1, array.metadata().extraFields().size()); + + Attributes attributes = new Attributes(); + attributes.put("answer", 7); + Array updated = array.setAttributes(attributes); + + Assertions.assertEquals(1, updated.metadata().extraFields().size(), + "the extra field was dropped when the metadata was rewritten"); + + // Also assert against the bytes actually on disk. + String onDisk = new String( + java.nio.file.Files.readAllBytes(zarrJson), StandardCharsets.UTF_8); + Assertions.assertTrue(onDisk.contains("some_future_field"), + "the extra field is missing from the rewritten document: " + onDisk); + } + + @Test + public void testGroupExtraFieldSurvivesMetadataRewrite() throws Exception { + StoreHandle storeHandle = + new FilesystemStore(TESTOUTPUT).resolve("extraFields", "groupRewrite"); + storeHandle.resolve("zarr.json").set(ByteBuffer.wrap( + groupMetadata(",\"some_future_field\":{\"must_understand\":false}") + .getBytes(StandardCharsets.UTF_8))); + + Group group = Group.open(storeHandle); + Assertions.assertEquals(1, group.metadata().extraFields().size()); + + Attributes attributes = new Attributes(); + attributes.put("answer", 7); + Group updated = group.setAttributes(attributes); + + Assertions.assertEquals(1, updated.metadata().extraFields().size(), + "the extra field was dropped when the group metadata was rewritten"); + } + + // --------------------------------------------------------------------------------------------- + // (b) (c) (d) every other unknown member is rejected + // --------------------------------------------------------------------------------------------- + + /** + * @param extraMember an unknown member that the reader may not ignore: a scalar carries no + * {@code must_understand} declaration at all, an object may omit it or set it + * to {@code true}, and {@code must_understand} has to be exactly {@code false} + * rather than a truthy or stringly-typed stand-in. + */ + @ParameterizedTest + @ValueSource(strings = { + ",\"some_future_field\":5", + ",\"some_future_field\":\"a string\"", + ",\"some_future_field\":null", + ",\"some_future_field\":[1,2,3]", + ",\"some_future_field\":{}", + ",\"some_future_field\":{\"detail\":42}", + ",\"some_future_field\":{\"must_understand\":true}", + ",\"some_future_field\":{\"must_understand\":\"false\"}", + ",\"some_future_field\":{\"must_understand\":0}", + ",\"some_future_field\":{\"must_understand\":null}", + }) + public void testArrayRejectsNonIgnorableExtraField(String extraMember) { + Exception exception = Assertions.assertThrows(Exception.class, + () -> objectMapper().readValue(arrayMetadata(extraMember), ArrayMetadata.class)); + + assertCausedByZarrExceptionMentioning(exception, "some_future_field"); + } + + @ParameterizedTest + @ValueSource(strings = { + ",\"some_future_field\":5", + ",\"some_future_field\":{\"detail\":42}", + ",\"some_future_field\":{\"must_understand\":true}", + }) + public void testGroupRejectsNonIgnorableExtraField(String extraMember) { + Exception exception = Assertions.assertThrows(Exception.class, + () -> objectMapper().readValue(groupMetadata(extraMember), GroupMetadata.class)); + + assertCausedByZarrExceptionMentioning(exception, "some_future_field"); + } + + /** + * A disallowed extra field has to be rejected when opening an array, not silently accepted. + */ + @Test + public void testOpenRejectsNonIgnorableExtraField() throws Exception { + StoreHandle storeHandle = + new FilesystemStore(TESTOUTPUT).resolve("extraFields", "rejectedOnOpen"); + storeHandle.resolve("zarr.json").set(ByteBuffer.wrap( + arrayMetadata(",\"some_future_field\":{\"must_understand\":true}") + .getBytes(StandardCharsets.UTF_8))); + + Exception exception = + Assertions.assertThrows(Exception.class, () -> Array.open(storeHandle)); + assertCausedByZarrExceptionMentioning(exception, "some_future_field"); + } + + /** + * Constructing metadata with an extra field that collides with a member the metadata document + * defines itself has to fail, mirroring zarr-python's {@code parse_extra_fields}. + */ + @Test + public void testExtraFieldCollidingWithReservedKeyIsRejected() { + Map ignorable = new HashMap<>(); + ignorable.put("must_understand", false); + Map extraFields = new HashMap<>(); + extraFields.put("shape", ignorable); + + ZarrException exception = Assertions.assertThrows(ZarrException.class, + () -> Array.metadataBuilder() + .withShape(4) + .withDataType(DataType.UINT8) + .withChunkShape(2) + .withExtraFields(extraFields) + .build()); + + Assertions.assertTrue(exception.getMessage().contains("shape"), exception.getMessage()); + Assertions.assertTrue(exception.getMessage().contains("collide"), exception.getMessage()); + } + + // --------------------------------------------------------------------------------------------- + // (e) must_understand does not apply to codecs + // --------------------------------------------------------------------------------------------- + + /** + * An unknown codec name always has to fail: a codec cannot be skipped and still leave the chunk + * bytes decodable, so there is no {@code must_understand} escape hatch for it. + */ + @Test + public void testUnknownCodecNameThrows() { + Assertions.assertThrows(Exception.class, () -> objectMapper().readValue( + arrayMetadata("").replace("\"name\":\"bytes\"", "\"name\":\"not_a_real_codec\""), + ArrayMetadata.class)); + } + + /** + * Not even declaring {@code must_understand: false} inside a codec object makes an unknown codec + * skippable. + */ + @Test + public void testUnknownCodecNameThrowsEvenWithMustUnderstandFalse() { + Assertions.assertThrows(Exception.class, () -> objectMapper().readValue( + arrayMetadata("").replace( + "\"codecs\":[{\"name\":\"bytes\",\"configuration\":{\"endian\":\"little\"}}]", + "\"codecs\":[{\"name\":\"not_a_real_codec\",\"must_understand\":false}]"), + ArrayMetadata.class)); + } + + /** + * An unknown chunk grid name also has to fail, for the same reason. + */ + @Test + public void testUnknownChunkGridNameThrows() { + Assertions.assertThrows(Exception.class, () -> objectMapper().readValue( + arrayMetadata("").replace("\"name\":\"regular\"", "\"name\":\"not_a_real_grid\""), + ArrayMetadata.class)); + } + + /** + * An unknown chunk key encoding name also has to fail. + */ + @Test + public void testUnknownChunkKeyEncodingNameThrows() { + Assertions.assertThrows(Exception.class, () -> objectMapper().readValue( + arrayMetadata("").replace("\"name\":\"default\"", "\"name\":\"not_a_real_encoding\""), + ArrayMetadata.class)); + } + + // --------------------------------------------------------------------------------------------- + // helpers + // --------------------------------------------------------------------------------------------- + + /** + * Jackson wraps an exception thrown from a creator, so the {@link ZarrException} carrying the + * explanation shows up as a cause rather than as the thrown exception itself. + */ + private static void assertCausedByZarrExceptionMentioning(Throwable thrown, String needle) { + for (Throwable cause = thrown; cause != null; cause = cause.getCause()) { + if (cause instanceof ZarrException && cause.getMessage() != null + && cause.getMessage().contains(needle)) { + return; + } + } + Assertions.fail("expected a ZarrException mentioning '" + needle + "', but got: " + thrown, + thrown); + } +} From 6e5063ec8b0e4ec510d71becef9be611eac44abb Mon Sep 17 00:00:00 2001 From: konstibob <44369572+konstibob@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:08:21 +0200 Subject: [PATCH 2/4] removed chunkEncoding from ArrayMetadata --- src/main/java/dev/zarr/zarrjava/v3/ArrayMetadataBuilder.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadataBuilder.java b/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadataBuilder.java index fd6b5c37..64fd1331 100644 --- a/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadataBuilder.java +++ b/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadataBuilder.java @@ -31,7 +31,6 @@ public class ArrayMetadataBuilder { Attributes attributes = new Attributes(); Map[] storageTransformers = new HashMap[]{}; String[] dimensionNames = null; - Map extraFields = null; protected ArrayMetadataBuilder() { } From 62c606d28e38e1ff68bcda381ef663c42b4a3c16 Mon Sep 17 00:00:00 2001 From: Konstantin Date: Tue, 8 Sep 2026 13:57:36 +0200 Subject: [PATCH 3/4] Make the ExtraFields key sets private The reserved-key sets were package-private only because callers had to pass one into validated(). That made the contents of the sets part of the v3 package's surface, and it allowed a caller to validate an array document against the group key set, which the compiler could not catch. validated() is now private and reached through validatedArrayFields and validatedGroupFields, so no other class names a key set. ARRAY_METADATA_KEYS, GROUP_METADATA_KEYS, MUST_UNDERSTAND and isIgnorable are private as well; the class itself stays package-private. No behaviour change; ExtraFieldsTest passes unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- .../dev/zarr/zarrjava/v3/ArrayMetadata.java | 2 +- .../dev/zarr/zarrjava/v3/ExtraFields.java | 34 ++++++++++++++++--- .../dev/zarr/zarrjava/v3/GroupMetadata.java | 2 +- 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadata.java b/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadata.java index dbabf86c..cb880cf0 100644 --- a/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadata.java +++ b/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadata.java @@ -124,7 +124,7 @@ public ArrayMetadata( @Nullable @JsonAnySetter Map extraFields ) throws ZarrException { super(shape, fillValue, dataType); - this.extraFields = ExtraFields.validated(extraFields, ExtraFields.ARRAY_METADATA_KEYS); + this.extraFields = ExtraFields.validatedArrayFields(extraFields); if (zarrFormat != this.zarrFormat) { throw new ZarrException( "Expected zarr format '" + this.zarrFormat + "', got '" + zarrFormat + "'."); diff --git a/src/main/java/dev/zarr/zarrjava/v3/ExtraFields.java b/src/main/java/dev/zarr/zarrjava/v3/ExtraFields.java index 6c1b8433..6b353381 100644 --- a/src/main/java/dev/zarr/zarrjava/v3/ExtraFields.java +++ b/src/main/java/dev/zarr/zarrjava/v3/ExtraFields.java @@ -24,13 +24,13 @@ */ final class ExtraFields { - static final String MUST_UNDERSTAND = "must_understand"; + private static final String MUST_UNDERSTAND = "must_understand"; /** * Members of a v3 array metadata document that zarr-java knows about. Anything else read from a * {@code zarr.json} array document is an extra field. */ - static final Set ARRAY_METADATA_KEYS = unmodifiableSetOf( + private static final Set ARRAY_METADATA_KEYS = unmodifiableSetOf( "zarr_format", "node_type", "shape", "data_type", "chunk_grid", "chunk_key_encoding", "fill_value", "codecs", "attributes", "dimension_names", "storage_transformers" ); @@ -39,7 +39,7 @@ final class ExtraFields { * Members of a v3 group metadata document that zarr-java knows about. Anything else read from a * {@code zarr.json} group document is an extra field. */ - static final Set GROUP_METADATA_KEYS = unmodifiableSetOf( + private static final Set GROUP_METADATA_KEYS = unmodifiableSetOf( "zarr_format", "node_type", "attributes", "consolidated_metadata" ); @@ -50,11 +50,35 @@ private static Set unmodifiableSetOf(String... keys) { return Collections.unmodifiableSet(new HashSet<>(Arrays.asList(keys))); } + /** + * Validates the unknown members of a v3 array metadata document. + * + * @param extraFields the unknown members, may be {@code null} + * @return the extra fields, never {@code null} + * @throws ZarrException if a member may not be ignored, see {@link #validated} + */ + static Map validatedArrayFields(@Nullable Map extraFields) + throws ZarrException { + return validated(extraFields, ARRAY_METADATA_KEYS); + } + + /** + * Validates the unknown members of a v3 group metadata document. + * + * @param extraFields the unknown members, may be {@code null} + * @return the extra fields, never {@code null} + * @throws ZarrException if a member may not be ignored, see {@link #validated} + */ + static Map validatedGroupFields(@Nullable Map extraFields) + throws ZarrException { + return validated(extraFields, GROUP_METADATA_KEYS); + } + /** * Whether an unknown metadata member may be ignored, i.e. whether it is a JSON object with a * {@code must_understand} member that is set to {@code false}. */ - static boolean isIgnorable(@Nullable Object value) { + private static boolean isIgnorable(@Nullable Object value) { return value instanceof Map && Boolean.FALSE.equals(((Map) value).get(MUST_UNDERSTAND)); } @@ -71,7 +95,7 @@ static boolean isIgnorable(@Nullable Object value) { * ignored because it is not a JSON object carrying * {@code "must_understand": false} */ - static Map validated( + private static Map validated( @Nullable Map extraFields, Set reservedKeys ) throws ZarrException { if (extraFields == null || extraFields.isEmpty()) { diff --git a/src/main/java/dev/zarr/zarrjava/v3/GroupMetadata.java b/src/main/java/dev/zarr/zarrjava/v3/GroupMetadata.java index 6283bc6e..0f3592bb 100644 --- a/src/main/java/dev/zarr/zarrjava/v3/GroupMetadata.java +++ b/src/main/java/dev/zarr/zarrjava/v3/GroupMetadata.java @@ -64,7 +64,7 @@ public GroupMetadata( "Expected node type '" + this.nodeType + "', got '" + nodeType + "'."); } this.attributes = attributes; - this.extraFields = ExtraFields.validated(extraFields, ExtraFields.GROUP_METADATA_KEYS); + this.extraFields = ExtraFields.validatedGroupFields(extraFields); } public static GroupMetadata defaultValue() { From 4ba922b202fc014d476888352944b42745cef431 Mon Sep 17 00:00:00 2001 From: Konstantin Date: Tue, 8 Sep 2026 15:30:33 +0200 Subject: [PATCH 4/4] added removed line by accident --- src/main/java/dev/zarr/zarrjava/v3/ArrayMetadataBuilder.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadataBuilder.java b/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadataBuilder.java index 64fd1331..fd6b5c37 100644 --- a/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadataBuilder.java +++ b/src/main/java/dev/zarr/zarrjava/v3/ArrayMetadataBuilder.java @@ -31,6 +31,7 @@ public class ArrayMetadataBuilder { Attributes attributes = new Attributes(); Map[] storageTransformers = new HashMap[]{}; String[] dimensionNames = null; + Map extraFields = null; protected ArrayMetadataBuilder() { }