diff --git a/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/dynamic/TaggedDynamic.java b/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/dynamic/TaggedDynamic.java index fe308ed..49277a9 100644 --- a/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/dynamic/TaggedDynamic.java +++ b/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/dynamic/TaggedDynamic.java @@ -179,4 +179,68 @@ public TypeReference type() { public Dynamic value() { return this.value; } + + /** + * Indicates whether some other object is "equal to" this one. + * + *

Two {@code TaggedDynamic} instances are considered equal if they have the same type reference and the same + * dynamic value. + * This means that both the type and the data must match for two tagged dynamics to be considered equal.

+ * + *

Example

+ *
{@code
+     * TaggedDynamic tagged1 = new TaggedDynamic(typeRef, dynamicValue);
+     * TaggedDynamic tagged2 = new TaggedDynamic(typeRef, dynamicValue);
+     *
+     * // tagged1 and tagged2 are equal because they have the same type and value
+     * boolean isEqual = tagged1.equals(tagged2); // true
+     * }
+ * + * @param obj the reference object with which to compare + * @return {@code true} if this object is the same as the obj argument; {@code false} otherwise + */ + @Override + public boolean equals(final Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + + final TaggedDynamic that = (TaggedDynamic) obj; + return this.type.equals(that.type) && this.value.equals(that.value); + } + + /** + * Returns a hash code value for the object. + * + *

The hash code is computed based on both the type reference and the dynamic value. This ensures that equal + * {@code TaggedDynamic} instances will have the same hash code, which is important for correct behavior in + * hash-based collections.

+ * + * @return a hash code value for this object + */ + @Override + public int hashCode() { + int result = this.type.hashCode(); + result = 31 * result + this.value.hashCode(); + return result; + } + + /** + * Returns a string representation of the object. + * + *

The string representation includes the type reference and the dynamic value, providing a human-readable + * summary of the tagged dynamic's contents. This can be useful for debugging and logging purposes.

+ * + * @return a string representation of the object + */ + @Override + public String toString() { + return "TaggedDynamic{" + + "type=" + this.type + + ", value=" + this.value + + '}'; + } } diff --git a/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/fix/Fixes.java b/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/fix/Fixes.java index f811ed3..b5b8f14 100644 --- a/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/fix/Fixes.java +++ b/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/fix/Fixes.java @@ -121,7 +121,7 @@ private Fixes() { * @param rewrite the function that transforms the typed value; must not be {@code null} * @return a type rewrite rule that applies the transformation to matching types; never {@code null} * @throws NullPointerException if any parameter is {@code null} - * @see #fixTypeEverywhere(String, Type, Function) + * @see #fixTypeEverywhere(String, DynamicOps, Type, Function) */ @NotNull public static TypeRewriteRule fixTypeEverywhereTyped(@NotNull final String name, @@ -156,22 +156,44 @@ public String toString() { /** * Creates a rule that transforms the dynamic representation of a type. * - * @param name the fix name - * @param type the type to transform - * @param rewrite the dynamic transformation function - * @return a type rewrite rule + *

This method creates a {@link TypeRewriteRule} that matches values of the specified + * type and transforms them by encoding to {@link Dynamic}, applying the rewrite function, and decoding back. The + * rule only matches types with the same {@link de.splatgames.aether.datafixers.api.TypeReference} as the target + * type.

+ * + *

Example

+ *
{@code
+     * TypeRewriteRule rule = Fixes.fixTypeEverywhere(
+     *     "addTimestamp",
+     *     GsonOps.INSTANCE,
+     *     playerType,
+     *     dynamic -> dynamic.set("timestamp", dynamic.createLong(0L))
+     * );
+     * }
+ * + * @param the dynamic type + * @param name the fix name; must not be {@code null} + * @param ops the dynamic ops for encoding/decoding; must not be {@code null} + * @param type the type to transform; must not be {@code null} + * @param rewrite the dynamic transformation function; must not be {@code null} + * @return a type rewrite rule; never {@code null} + * @throws NullPointerException if any parameter is {@code null} + * @see #fixTypeEverywhereTyped(String, Type, Type, Function) */ @NotNull - public static TypeRewriteRule fixTypeEverywhere(@NotNull final String name, - @NotNull final Type type, - @NotNull final Function, Dynamic> rewrite) { + public static TypeRewriteRule fixTypeEverywhere(@NotNull final String name, + @NotNull final DynamicOps ops, + @NotNull final Type type, + @NotNull final Function, Dynamic> rewrite) { Preconditions.checkNotNull(name, "name must not be null"); + Preconditions.checkNotNull(ops, "ops must not be null"); Preconditions.checkNotNull(type, "type must not be null"); Preconditions.checkNotNull(rewrite, "rewrite must not be null"); return new TypeRewriteRule() { @NotNull @Override + @SuppressWarnings({"unchecked", "rawtypes"}) public Optional> rewrite(@NotNull final Type inputType, @NotNull final Typed input) { Preconditions.checkNotNull(inputType, "inputType must not be null"); @@ -180,9 +202,11 @@ public Optional> rewrite(@NotNull final Type inputType, return Optional.empty(); } - // We need to get DynamicOps from somewhere - this requires the caller to provide it - // For now, we'll use a simplified approach - return Optional.of(input); + final DataResult> encodeResult = input.encode(ops); + return encodeResult.flatMap(dynamic -> { + final Dynamic transformed = rewrite.apply(dynamic); + return ((Type) type).read(transformed); + }).map(newValue -> new Typed<>((Type) type, newValue)).result(); } @Override diff --git a/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/optic/Finder.java b/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/optic/Finder.java index e212560..8d0f26b 100644 --- a/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/optic/Finder.java +++ b/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/optic/Finder.java @@ -179,8 +179,10 @@ public Dynamic set(@NotNull final Dynamic root, * Creates a finder that navigates to an element by index in a list/array structure. * *

The returned finder extracts and modifies the element at the specified - * index position. If the index is out of bounds (negative or beyond the list size), {@link #get} returns - * {@code null} and {@link #set} returns the root unchanged.

+ * index position. A negative {@code index} is rejected immediately by throwing + * {@link IllegalArgumentException} at construction time. If the index is non-negative + * but beyond the list size, {@link #get} returns {@code null} and {@link #set} returns + * the root unchanged.

* *

Example

*
{@code
@@ -195,16 +197,21 @@ public Dynamic set(@NotNull final Dynamic root,
      * Dynamic updated = firstScoreFinder.set(data, data.createInt(100));
      * // updated: {"scores": [100, 92, 78]}
      *
-     * // Out of bounds access
+     * // Out-of-range (positive) index — get returns null, set returns root unchanged
      * Finder outOfBounds = scoresFinder.then(Finder.index(10));
      * Dynamic missing = outOfBounds.get(data);  // null
+     *
+     * // Negative index — throws IllegalArgumentException immediately
+     * Finder.index(-1);  // throws IllegalArgumentException
      * }
* - * @param index the zero-based index of the element to focus on + * @param index the zero-based index of the element to focus on; must not be negative + * @throws IllegalArgumentException if {@code index} is negative * @return a finder that navigates to the element at the specified index, never {@code null} */ @NotNull static Finder index(final int index) { + Preconditions.checkArgument(index >= 0, "index must not be negative, got: %s", index); return new Finder<>() { @NotNull @Override @@ -232,7 +239,7 @@ public String id() { return root; } final var list = listResult.result().orElseThrow().toList(); - if (index < 0 || index >= list.size()) { + if (index >= list.size()) { return root; } final List> newList = new ArrayList<>(); diff --git a/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/optic/Iso.java b/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/optic/Iso.java index 92b1b80..e314bab 100644 --- a/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/optic/Iso.java +++ b/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/optic/Iso.java @@ -384,13 +384,6 @@ public A modify(@NotNull final B source, Preconditions.checkNotNull(modifier, "modifier must not be null"); return from(modifier.apply(to(source))); } - - @NotNull - @Override - public Optic compose(@NotNull final Optic other) { - Preconditions.checkNotNull(other, "other must not be null"); - throw new UnsupportedOperationException("Compose on reversed iso"); - } }; } diff --git a/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/rewrite/Rules.java b/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/rewrite/Rules.java index dbc608e..1c47e68 100644 --- a/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/rewrite/Rules.java +++ b/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/rewrite/Rules.java @@ -1568,9 +1568,13 @@ public static TypeRewriteRule flattenField(@NotNull final DynamicOps ops, Dynamic result = typedDynamic.remove(fieldName); final var entries = entriesResult.result().orElse(java.util.stream.Stream.empty()).toList(); for (final var entry : entries) { - final String key = entry.first().asString().result().orElse(null); + final Dynamic keyDynamic = entry.first(); + final Dynamic value = entry.second(); + if (keyDynamic == null || value == null) { + continue; + } + final String key = keyDynamic.asString().result().orElse(null); if (key != null) { - final Dynamic value = entry.second(); result = result.set(key, value); } } diff --git a/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/schema/Schema.java b/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/schema/Schema.java index 1d1861d..596a7ad 100644 --- a/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/schema/Schema.java +++ b/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/schema/Schema.java @@ -94,7 +94,8 @@ public class Schema { private final DataVersion version; private final Schema parent; - private TypeRegistry types; + private volatile TypeRegistry types; + private TypeRegistry buildingTypes; /** * Creates a new schema for the specified version with the given types. @@ -161,10 +162,17 @@ public Schema parent() { */ @NotNull public TypeRegistry types() { - if (this.types == null) { - this.types = this.buildTypes(); + TypeRegistry result = this.types; + if (result == null) { + synchronized (this) { + result = this.types; + if (result == null) { + result = this.buildTypes(); + this.types = result; + } + } } - return this.types; + return result; } /** @@ -178,18 +186,22 @@ public TypeRegistry types() { @NotNull private TypeRegistry buildTypes() { final TypeRegistry registry = this.createTypeRegistry(); - this.types = registry; + this.buildingTypes = registry; // Inherit types from parent if present if (this.parent != null) { - // Copy types from parent - this.parent.types(); - // Parent types are already registered in parent's registry - // For now, we don't copy - subclass must re-register all types it needs + final TypeRegistry parentTypes = this.parent.types(); + for (final TypeReference ref : parentTypes.references()) { + final Type parentType = parentTypes.get(ref); + if (parentType != null) { + registry.register(parentType); + } + } } // Let subclass register types this.registerTypes(); + this.buildingTypes = null; return registry; } @@ -232,8 +244,9 @@ protected void registerTypes() { */ protected final void registerType(@NotNull final Type type) { Preconditions.checkNotNull(type, "type must not be null"); - Preconditions.checkState(this.types != null, "Cannot register types before types() is called"); - this.types.register(type); + final TypeRegistry registry = this.buildingTypes; + Preconditions.checkState(registry != null, "Cannot register types outside of registerTypes()"); + registry.register(type); } /** @@ -267,13 +280,14 @@ protected final void registerType(@NotNull final TypeReference reference, @NotNull final TypeTemplate template) { Preconditions.checkNotNull(reference, "reference must not be null"); Preconditions.checkNotNull(template, "template must not be null"); - Preconditions.checkState(this.types != null, "Cannot register types before types() is called"); + final TypeRegistry registry = this.buildingTypes; + Preconditions.checkState(registry != null, "Cannot register types outside of registerTypes()"); // Apply the template with an empty family to get the concrete type final Type templateType = template.apply(TypeFamily.empty()); // Wrap the template type with the reference - this.types.register(new TemplateBasedType<>(reference, templateType)); + registry.register(new TemplateBasedType<>(reference, templateType)); } /** diff --git a/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/type/Typed.java b/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/type/Typed.java index 6215f4b..a589532 100644 --- a/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/type/Typed.java +++ b/aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/type/Typed.java @@ -384,7 +384,8 @@ public DataResult> updateDynamic(@NotNull final DynamicOps ops, * @param ops the dynamic operations for encoding, must not be {@code null} * @param finder the finder that locates the desired sub-value, must not be {@code null} * @param the underlying data format type - * @return a {@link DataResult} containing the found dynamic value or {@code null} if not found, never {@code null} + * @return a {@link DataResult} containing the found dynamic value on success, or an error result if the path + * was not found; never {@code null} * @throws NullPointerException if {@code ops} or {@code finder} is {@code null} * @see #updateAt(DynamicOps, Finder, Function) */ @@ -393,12 +394,12 @@ public DataResult> getAt(@NotNull final DynamicOps ops, @NotNull final Finder finder) { Preconditions.checkNotNull(ops, "ops must not be null"); Preconditions.checkNotNull(finder, "finder must not be null"); - return encode(ops).map(dynamic -> { + return encode(ops).flatMap(dynamic -> { final Dynamic found = finder.get(dynamic); if (found == null) { - return null; + return DataResult.error("Path not found: " + finder); } - return found.convert(ops); + return DataResult.success(found.convert(ops)); }); } diff --git a/aether-datafixers-api/src/test/java/de/splatgames/aether/datafixers/api/optic/FinderTest.java b/aether-datafixers-api/src/test/java/de/splatgames/aether/datafixers/api/optic/FinderTest.java index aa2ee0b..9e32dee 100644 --- a/aether-datafixers-api/src/test/java/de/splatgames/aether/datafixers/api/optic/FinderTest.java +++ b/aether-datafixers-api/src/test/java/de/splatgames/aether/datafixers/api/optic/FinderTest.java @@ -32,6 +32,7 @@ import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; /** * Unit tests for {@link Finder}. @@ -167,6 +168,14 @@ void idReturnsDescriptiveIdentifier() { assertThat(finder.id()).isEqualTo("index[3]"); } + + @Test + @DisplayName("index() rejects negative index") + void indexRejectsNegativeIndex() { + assertThatThrownBy(() -> Finder.index(-1)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("must not be negative"); + } } @Nested diff --git a/aether-datafixers-api/src/test/java/de/splatgames/aether/datafixers/api/optic/IsoTest.java b/aether-datafixers-api/src/test/java/de/splatgames/aether/datafixers/api/optic/IsoTest.java index 55ef085..e89fe31 100644 --- a/aether-datafixers-api/src/test/java/de/splatgames/aether/datafixers/api/optic/IsoTest.java +++ b/aether-datafixers-api/src/test/java/de/splatgames/aether/datafixers/api/optic/IsoTest.java @@ -196,6 +196,26 @@ void reverseAddsReverseToId() { assertThat(unwrapIso.reverse().id()).isEqualTo("wrapper.unwrap.reverse"); } + @Test + @DisplayName("reverse() supports compose()") + void reverseSupportsCompose() { + // reversed: String -> Wrapper + final Iso, Wrapper> reversed = unwrapIso.reverse(); + + // another iso: Wrapper -> String (extract value and uppercase) + final Iso, Wrapper, String, String> extractUpper = Iso.of( + "wrapper.upper", + w -> w.value().toUpperCase(), + s -> new Wrapper<>(s.toLowerCase()) + ); + + // compose: String -> Wrapper -> String (uppercased) + final Iso composed = reversed.compose(extractUpper); + + assertThat(composed.to("hello")).isEqualTo("HELLO"); + assertThat(composed.from("HELLO")).isEqualTo("hello"); + } + @Test @DisplayName("double reverse returns to original") void doubleReverseReturnsToOriginal() { diff --git a/aether-datafixers-functional-tests/src/test/java/de/splatgames/aether/datafixers/functional/chaos/RandomFailuresChaosIT.java b/aether-datafixers-functional-tests/src/test/java/de/splatgames/aether/datafixers/functional/chaos/RandomFailuresChaosIT.java index bf84494..49d84d2 100644 --- a/aether-datafixers-functional-tests/src/test/java/de/splatgames/aether/datafixers/functional/chaos/RandomFailuresChaosIT.java +++ b/aether-datafixers-functional-tests/src/test/java/de/splatgames/aether/datafixers/functional/chaos/RandomFailuresChaosIT.java @@ -29,6 +29,7 @@ import de.splatgames.aether.datafixers.api.dynamic.Dynamic; import de.splatgames.aether.datafixers.api.fix.DataFix; import de.splatgames.aether.datafixers.api.fix.DataFixer; +import de.splatgames.aether.datafixers.api.exception.FixException; import de.splatgames.aether.datafixers.api.fix.DataFixerContext; import de.splatgames.aether.datafixers.codec.json.gson.GsonOps; import de.splatgames.aether.datafixers.core.fix.DataFixerBuilder; @@ -104,8 +105,8 @@ void handlesSporadicFixFailures() { assertThat(result.get("processed").asBoolean().result()).contains(true); successCount.incrementAndGet(); - } catch (ChaosInjector.ChaosException e) { - // Expected chaos failure + } catch (FixException e) { + // Expected: DataFixerImpl wraps ChaosException in FixException failureCount.incrementAndGet(); } } @@ -142,8 +143,9 @@ void propagatesFixExceptionsCorrectly() { CHAOS_TYPE, input, new DataVersion(1), new DataVersion(2) )) - .isInstanceOf(ChaosInjector.ChaosException.class) - .hasMessageContaining("Chaos-injected failure"); + .isInstanceOf(FixException.class) + .hasMessageContaining("Chaos-injected failure") + .hasCauseInstanceOf(ChaosInjector.ChaosException.class); } @Test @@ -170,7 +172,8 @@ void failureInChainStopsSubsequentFixes() { assertThatThrownBy(() -> fixer.update( CHAOS_TYPE, input, new DataVersion(1), new DataVersion(4) - )).isInstanceOf(ChaosInjector.ChaosException.class); + )).isInstanceOf(FixException.class) + .hasCauseInstanceOf(ChaosInjector.ChaosException.class); // Fix 1 should have run assertThat(fix1Count.get()).isEqualTo(1); @@ -225,7 +228,7 @@ void isolatesFailuresBetweenThreads() throws Exception { totalSuccess.incrementAndGet(); threadsWithSuccess.add(threadId); - } catch (ChaosInjector.ChaosException e) { + } catch (FixException e) { totalFailure.incrementAndGet(); threadsWithFailure.add(threadId); } @@ -300,7 +303,7 @@ void fixerRemainsFunctionalAfterFailures() throws Exception { try { fixer.update(CHAOS_TYPE, input, new DataVersion(1), new DataVersion(2)); phase1Success.incrementAndGet(); - } catch (ChaosInjector.ChaosException e) { + } catch (FixException e) { phase1Failure.incrementAndGet(); } } @@ -336,7 +339,7 @@ void fixerRemainsFunctionalAfterFailures() throws Exception { try { fixer.update(CHAOS_TYPE, input, new DataVersion(1), new DataVersion(2)); phase3Success.incrementAndGet(); - } catch (ChaosInjector.ChaosException e) { + } catch (FixException e) { phase3Failure.incrementAndGet(); } } diff --git a/docs/migration/v0.5-to-v1.0.md b/docs/migration/v0.5-to-v1.0.md index c1199e3..71eb722 100644 --- a/docs/migration/v0.5-to-v1.0.md +++ b/docs/migration/v0.5-to-v1.0.md @@ -6,12 +6,13 @@ This guide helps you migrate your project from Aether Datafixers v0.5.x to v1.0. Version 1.0.0 introduces breaking changes to improve the codec package organization and API clarity. This guide will help you update your codebase with minimal effort. -| Change Type | Component | Impact | -|-----------------|-------------------------|---------------------------------------------------| -| **Breaking** | Codec package structure | Import statements must be updated | -| **Breaking** | `JacksonOps` rename | Class renamed to `JacksonJsonOps` | -| **Breaking** | `TestData.jackson()` | Method renamed to `TestData.jacksonJson()` | -| **Deprecation** | Rules traversal methods | Single-argument overloads deprecated (still work) | +| Change Type | Component | Impact | +|-----------------|-----------------------------|---------------------------------------------------| +| **Breaking** | Codec package structure | Import statements must be updated | +| **Breaking** | `JacksonOps` rename | Class renamed to `JacksonJsonOps` | +| **Breaking** | `TestData.jackson()` | Method renamed to `TestData.jacksonJson()` | +| **Breaking** | `Fixes.fixTypeEverywhere()` | Now requires a `DynamicOps` parameter | +| **Deprecation** | Rules traversal methods | Single-argument overloads deprecated (still work) | --- @@ -80,6 +81,31 @@ Dynamic testData = TestData.jacksonJson() **Rationale:** Consistency with `jacksonYaml()`, `jacksonToml()`, and `jacksonXml()` methods. +### 4. `Fixes.fixTypeEverywhere()` Signature Change + +The `Fixes.fixTypeEverywhere()` method now requires a `DynamicOps` parameter. In v0.5.x, this method was a **silent no-op** — it accepted a `Function, Dynamic>` but never applied it, returning the input unchanged. The fix adds the missing `DynamicOps` parameter needed to encode/decode the data during transformation. + +**Before (v0.5.x):** +```java +TypeRewriteRule rule = Fixes.fixTypeEverywhere( + "addTimestamp", + playerType, + dynamic -> dynamic.set("timestamp", dynamic.createLong(0L)) +); +``` + +**After (v1.0.0):** +```java +TypeRewriteRule rule = Fixes.fixTypeEverywhere( + "addTimestamp", + GsonOps.INSTANCE, + playerType, + dynamic -> dynamic.set("timestamp", dynamic.createLong(0L)) +); +``` + +**Rationale:** The previous implementation could not function without `DynamicOps` — it silently returned input unchanged, which is a data corruption risk. Adding the parameter fixes the method to actually apply the transformation. + --- ## Deprecations (Non-Breaking) @@ -332,6 +358,7 @@ The method was renamed. - [ ] Update `codec.jackson.JacksonOps` imports to `codec.json.jackson.JacksonJsonOps` - [ ] Replace `JacksonOps` class references with `JacksonJsonOps` - [ ] Replace `TestData.jackson()` with `TestData.jacksonJson()` +- [ ] Add `DynamicOps` parameter to `Fixes.fixTypeEverywhere()` calls - [ ] (Optional) Update deprecated `Rules` methods to include `DynamicOps` - [ ] Rebuild and verify no compilation errors - [ ] Run test suite