Merge pull request #254 from aether-framework/bugfix/129-131-139-142-143-144-159-168-169-178-mixed-core-fixes

Refactor migration handling for consistency and improved validation

Author: Splatcrafter

aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/optic/Prism.java

@@ -270,6 +270,8 @@ public S reverseGet(@NotNull final A value) {
      * @throws NullPointerException if {@code source} or {@code modifier} is {@code null}
      */
     @NotNull
+    // Safe cast: for monomorphic prisms (S == T), the source is returned unchanged
+    // when getOption returns empty, preserving the original type.
     @SuppressWarnings("unchecked")
     default T modify(@NotNull final S source,
                      @NotNull final Function<A, B> modifier) {
@@ -301,6 +303,8 @@ default T modify(@NotNull final S source,
      * @throws NullPointerException if {@code source} or {@code value} is {@code null}
      */
     @NotNull
+    // Safe cast: for monomorphic prisms (S == T), the source is returned unchanged
+    // when getOption returns empty, preserving the original type.
     @SuppressWarnings("unchecked")
     default T set(@NotNull final S source, @NotNull final B value) {
         Preconditions.checkNotNull(source, "source must not be null");

aether-datafixers-codec/src/main/java/de/splatgames/aether/datafixers/codec/xml/jackson/JacksonXmlOps.java

@@ -1556,6 +1556,23 @@ public <U> JsonNode convertTo(@NotNull final DynamicOps<U> sourceOps,
         return empty();
     }
 
+    /**
+     * Validates that the given node is suitable for XML serialization.
+     *
+     * <p>XML requires exactly one root element. This method checks that the given
+     * node is an object (map) node, which will become the root element when serialized.</p>
+     *
+     * @param node the node to validate, must not be {@code null}
+     * @throws IllegalArgumentException if the node is not suitable for XML serialization
+     */
+    public static void validateForSerialization(@NotNull final JsonNode node) {
+        Preconditions.checkNotNull(node, "node must not be null");
+        if (!node.isObject()) {
+            throw new IllegalArgumentException(
+                    "XML serialization requires an object node as root, but got: " + node.getNodeType());
+        }
+    }
+
     /**
      * Returns a string representation of this {@code DynamicOps} instance.
      *

aether-datafixers-core/src/main/java/de/splatgames/aether/datafixers/core/AetherDataFixer.java

@@ -181,6 +181,10 @@ public TaggedDynamic update(
         Preconditions.checkNotNull(input, "input must not be null");
         Preconditions.checkNotNull(fromVersion, "fromVersion must not be null");
         Preconditions.checkNotNull(toVersion, "toVersion must not be null");
+        Preconditions.checkArgument(
+                fromVersion.compareTo(toVersion) <= 0,
+                "fromVersion (%s) must be <= toVersion (%s)", fromVersion, toVersion
+        );
 
         @SuppressWarnings("unchecked") final Dynamic<Object> dyn = (Dynamic<Object>) input.value();
 

aether-datafixers-core/src/main/java/de/splatgames/aether/datafixers/core/bootstrap/DataFixerRuntimeFactory.java

@@ -29,6 +29,8 @@
 import de.splatgames.aether.datafixers.core.fix.DataFixerBuilder;
 import de.splatgames.aether.datafixers.core.schema.SimpleSchemaRegistry;
 import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Factory for creating fully configured {@link AetherDataFixer} instances.
@@ -67,6 +69,8 @@
  */
 public final class DataFixerRuntimeFactory {
 
+    private static final Logger LOG = LoggerFactory.getLogger(DataFixerRuntimeFactory.class);
+
     /**
      * Creates a fully configured data fixer from a bootstrap.
      *
@@ -85,6 +89,13 @@ public AetherDataFixer create(
 
         final SimpleSchemaRegistry schemas = new SimpleSchemaRegistry();
         bootstrap.registerSchemas(schemas);
+
+        if (schemas.stream().findAny().isEmpty()) {
+            LOG.warn("Bootstrap registered no schemas — DataFixer may not function correctly");
+        } else if (schemas.get(currentVersion) == null) {
+            LOG.warn("No schema registered for currentVersion: {}", currentVersion);
+        }
+
         schemas.freeze();
 
         final DataFixerBuilder builder = new DataFixerBuilder(currentVersion);

aether-datafixers-core/src/main/java/de/splatgames/aether/datafixers/core/codec/SimpleCodecRegistry.java

@@ -87,7 +87,7 @@ public boolean has(@NotNull final TypeReference ref) {
     }
 
     @Override
-    public void freeze() {
+    public synchronized void freeze() {
         if (!this.frozen) {
             this.codecs = Map.copyOf(this.codecs);
             this.frozen = true;

aether-datafixers-core/src/main/java/de/splatgames/aether/datafixers/core/diagnostic/DiagnosticRuleWrapper.java

@@ -93,14 +93,15 @@ public Optional<Typed<?>> rewrite(
             return this.delegate.rewrite(type, input);
         }
 
-        final Instant start = Instant.now();
+        final Instant timestamp = Instant.now();
+        final long startNano = System.nanoTime();
         final Optional<Typed<?>> result = this.delegate.rewrite(type, input);
-        final Duration duration = Duration.between(start, Instant.now());
+        final Duration duration = Duration.ofNanos(System.nanoTime() - startNano);
 
         final RuleApplication application = new RuleApplication(
                 this.delegate.toString(),
                 type.describe(),
-                start,
+                timestamp,
                 duration,
                 result.isPresent(),
                 null

aether-datafixers-core/src/main/java/de/splatgames/aether/datafixers/core/schema/SimpleSchemaRegistry.java

@@ -129,7 +129,7 @@ public Schema latest() {
     }
 
     @Override
-    public void freeze() {
+    public synchronized void freeze() {
         if (!this.frozen) {
             this.schemas = Collections.unmodifiableNavigableMap(new TreeMap<>(this.schemas));
             this.frozen = true;

aether-datafixers-core/src/main/java/de/splatgames/aether/datafixers/core/type/SimpleTypeRegistry.java

@@ -93,7 +93,7 @@ public Set<TypeReference> references() {
     }
 
     @Override
-    public void freeze() {
+    public synchronized void freeze() {
         if (!this.frozen) {
             this.types = Map.copyOf(this.types);
             this.frozen = true;

aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/analysis/MigrationAnalyzer.java

@@ -381,10 +381,8 @@ private MigrationStep analyzeStep(
         ).schemaDiff(diff)
                 .affectedTypes(affectedTypes);
 
-        // Add first fix if present (simplified - in reality there might be multiple)
-        if (!fixes.isEmpty()) {
-            stepBuilder.fix(fixes.get(0));
-        }
+        // Add all applicable fixes for this migration step
+        stepBuilder.fixes(fixes);
 
         return stepBuilder.build();
     }

aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/analysis/MigrationStep.java

@@ -31,6 +31,8 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
@@ -82,9 +84,9 @@ public final class MigrationStep {
     private final DataVersion targetVersion;
 
     /**
-     * The DataFix applied in this step, or {@code null} if no fix applies.
+     * The DataFixes applied in this step. Empty if no fixes apply.
      */
-    private final DataFix<?> fix;
+    private final List<DataFix<?>> fixes;
 
     /**
      * The schema diff between source and target versions, or {@code null} if not computed.
@@ -105,20 +107,20 @@ public final class MigrationStep {
      *
      * @param sourceVersion the source version, must not be {@code null}
      * @param targetVersion the target version, must not be {@code null}
-     * @param fix           the DataFix for this step, may be {@code null}
+     * @param fixes         the DataFixes for this step, must not be {@code null}
      * @param schemaDiff    the schema diff, may be {@code null}
      * @param affectedTypes the affected types, must not be {@code null}
      */
     private MigrationStep(
             @NotNull final DataVersion sourceVersion,
             @NotNull final DataVersion targetVersion,
-            @Nullable final DataFix<?> fix,
+            @NotNull final List<DataFix<?>> fixes,
             @Nullable final SchemaDiff schemaDiff,
             @NotNull final Set<TypeReference> affectedTypes
     ) {
         this.sourceVersion = Preconditions.checkNotNull(sourceVersion, "sourceVersion must not be null");
         this.targetVersion = Preconditions.checkNotNull(targetVersion, "targetVersion must not be null");
-        this.fix = fix;
+        this.fixes = List.copyOf(Preconditions.checkNotNull(fixes, "fixes must not be null"));
         this.schemaDiff = schemaDiff;
         this.affectedTypes = Set.copyOf(Preconditions.checkNotNull(affectedTypes, "affectedTypes must not be null"));
     }
@@ -143,7 +145,7 @@ public static MigrationStep withFix(
         Preconditions.checkNotNull(targetVersion, "targetVersion must not be null");
         Preconditions.checkNotNull(fix, "fix must not be null");
         Preconditions.checkNotNull(affectedTypes, "affectedTypes must not be null");
-        return new MigrationStep(sourceVersion, targetVersion, fix, null, affectedTypes);
+        return new MigrationStep(sourceVersion, targetVersion, List.of(fix), null, affectedTypes);
     }
 
     /**
@@ -165,7 +167,7 @@ public static MigrationStep withoutFix(
         Preconditions.checkNotNull(sourceVersion, "sourceVersion must not be null");
         Preconditions.checkNotNull(targetVersion, "targetVersion must not be null");
         Preconditions.checkNotNull(affectedTypes, "affectedTypes must not be null");
-        return new MigrationStep(sourceVersion, targetVersion, null, schemaDiff, affectedTypes);
+        return new MigrationStep(sourceVersion, targetVersion, List.of(), schemaDiff, affectedTypes);
     }
 
     /**
@@ -212,7 +214,17 @@ public DataVersion targetVersion() {
      */
     @NotNull
     public Optional<DataFix<?>> fix() {
-        return Optional.ofNullable(this.fix);
+        return this.fixes.isEmpty() ? Optional.empty() : Optional.of(this.fixes.get(0));
+    }
+
+    /**
+     * Returns all DataFixes applied in this step.
+     *
+     * @return an unmodifiable list of fixes, never {@code null}
+     */
+    @NotNull
+    public List<DataFix<?>> fixes() {
+        return this.fixes;
     }
 
     /**
@@ -241,7 +253,7 @@ public Set<TypeReference> affectedTypes() {
      * @return {@code true} if a fix is present
      */
     public boolean hasFix() {
-        return this.fix != null;
+        return !this.fixes.isEmpty();
     }
 
     /**
@@ -284,7 +296,7 @@ public boolean equals(final Object obj) {
         }
         return this.sourceVersion.equals(other.sourceVersion)
                 && this.targetVersion.equals(other.targetVersion)
-                && Objects.equals(this.fix, other.fix)
+                && Objects.equals(this.fixes, other.fixes)
                 && this.affectedTypes.equals(other.affectedTypes);
     }
 
@@ -298,7 +310,7 @@ public boolean equals(final Object obj) {
      */
     @Override
     public int hashCode() {
-        return Objects.hash(this.sourceVersion, this.targetVersion, this.fix, this.affectedTypes);
+        return Objects.hash(this.sourceVersion, this.targetVersion, this.fixes, this.affectedTypes);
     }
 
     /**
@@ -315,8 +327,8 @@ public String toString() {
         final StringBuilder sb = new StringBuilder();
         sb.append("MigrationStep[");
         sb.append(this.sourceVersion.getVersion()).append(" -> ").append(this.targetVersion.getVersion());
-        if (this.fix != null) {
-            sb.append(", fix=").append(this.fix.getClass().getSimpleName());
+        if (!this.fixes.isEmpty()) {
+            sb.append(", fixes=").append(this.fixes.size());
         }
         sb.append(", affected=").append(this.affectedTypes.size()).append(" types");
         sb.append("]");
@@ -355,9 +367,9 @@ public static final class Builder {
         private final DataVersion targetVersion;
 
         /**
-         * The optional DataFix; defaults to {@code null}.
+         * The list of DataFixes; defaults to empty.
          */
-        private DataFix<?> fix;
+        private final List<DataFix<?>> fixes = new ArrayList<>();
 
         /**
          * The optional schema diff; defaults to {@code null}.
@@ -392,7 +404,22 @@ private Builder(
          */
         @NotNull
         public Builder fix(@Nullable final DataFix<?> fix) {
-            this.fix = fix;
+            if (fix != null) {
+                this.fixes.add(fix);
+            }
+            return this;
+        }
+
+        /**
+         * Adds all fixes for this step.
+         *
+         * @param fixes the fixes to add, must not be {@code null}
+         * @return this builder for chaining
+         */
+        @NotNull
+        public Builder fixes(@NotNull final List<DataFix<?>> fixes) {
+            Preconditions.checkNotNull(fixes, "fixes must not be null");
+            this.fixes.addAll(fixes);
             return this;
         }
 
@@ -434,7 +461,7 @@ public MigrationStep build() {
             return new MigrationStep(
                     this.sourceVersion,
                     this.targetVersion,
-                    this.fix,
+                    this.fixes,
                     this.schemaDiff,
                     this.affectedTypes
             );