Merge pull request #253 from aether-framework/bugfix/145-146-149-151-153-154-155-156-157-160-codec-and-core-fixes

Refactor error handling and schema validation workflows

Author: Splatcrafter

aether-datafixers-codec/src/main/java/de/splatgames/aether/datafixers/codec/json/jackson/JacksonJsonOps.java

@@ -25,7 +25,9 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.node.ArrayNode;
+import com.fasterxml.jackson.databind.node.BigIntegerNode;
 import com.fasterxml.jackson.databind.node.BooleanNode;
+import com.fasterxml.jackson.databind.node.DecimalNode;
 import com.fasterxml.jackson.databind.node.DoubleNode;
 import com.fasterxml.jackson.databind.node.FloatNode;
 import com.fasterxml.jackson.databind.node.IntNode;
@@ -43,6 +45,8 @@
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
+import java.math.BigDecimal;
+import java.math.BigInteger;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Spliterator;
@@ -689,6 +693,12 @@ public JsonNode createNumeric(@NotNull final Number value) {
         if (value instanceof Byte) {
             return ShortNode.valueOf(value.byteValue());
         }
+        if (value instanceof BigDecimal bd) {
+            return DecimalNode.valueOf(bd);
+        }
+        if (value instanceof BigInteger bi) {
+            return BigIntegerNode.valueOf(bi);
+        }
         return DoubleNode.valueOf(value.doubleValue());
     }
 

aether-datafixers-codec/src/main/java/de/splatgames/aether/datafixers/codec/toml/jackson/JacksonTomlOps.java

@@ -935,6 +935,12 @@ public DataResult<JsonNode> mergeToList(@NotNull final JsonNode list,
             return DataResult.error("Not an array: " + list);
         }
         final ArrayNode result = list.isNull() ? this.nodeFactory.arrayNode() : ((ArrayNode) list).deepCopy();
+        // TOML requires homogeneous arrays — validate element type consistency
+        if (!result.isEmpty() && result.get(0).getNodeType() != value.getNodeType()) {
+            return DataResult.error(
+                    "TOML requires homogeneous arrays: expected "
+                            + result.get(0).getNodeType() + " but got " + value.getNodeType());
+        }
         result.add(value);
         return DataResult.success(result);
     }
@@ -1089,6 +1095,10 @@ public DataResult<JsonNode> mergeToMap(@NotNull final JsonNode map,
         if (!key.isTextual()) {
             return DataResult.error("Key is not a string: " + key);
         }
+        // TOML does not support null values
+        if (value.isNull()) {
+            return DataResult.error("TOML does not support null values for key: " + key.asText());
+        }
         final ObjectNode result = map.isNull() ? this.nodeFactory.objectNode() : ((ObjectNode) map).deepCopy();
         result.set(key.asText(), value);
         return DataResult.success(result);

aether-datafixers-codec/src/main/java/de/splatgames/aether/datafixers/codec/yaml/snakeyaml/SnakeYamlOps.java

@@ -828,7 +828,7 @@ public Object createMap(@NotNull final Stream<Pair<Object, Object>> entries) {
                 return; // Skip entries with null keys
             }
             final String key = keyObj.toString();
-            map.put(key, valueObj);
+            map.put(key, valueObj == null ? YamlNull.INSTANCE : valueObj);
         });
         return map;
     }
@@ -1254,9 +1254,21 @@ public <U> Object convertTo(@NotNull final DynamicOps<U> sourceOps,
      * @return a deep copy of the value, or the value itself if it is immutable;
      *         {@code null} if the input is {@code null}
      */
+    private static final int MAX_DEEP_COPY_DEPTH = 512;
+
     @Nullable
     @SuppressWarnings("unchecked")
     private Object deepCopy(@Nullable final Object value) {
+        return deepCopy(value, 0);
+    }
+
+    @Nullable
+    @SuppressWarnings("unchecked")
+    private Object deepCopy(@Nullable final Object value, final int depth) {
+        if (depth > MAX_DEEP_COPY_DEPTH) {
+            throw new IllegalStateException(
+                    "YAML structure too deeply nested (depth > " + MAX_DEEP_COPY_DEPTH + ")");
+        }
         if (value == null) {
             return null;
         }
@@ -1267,15 +1279,15 @@ private Object deepCopy(@Nullable final Object value) {
             final Map<String, Object> original = (Map<String, Object>) value;
             final Map<String, Object> copy = new LinkedHashMap<>();
             for (final Map.Entry<String, Object> entry : original.entrySet()) {
-                copy.put(entry.getKey(), deepCopy(entry.getValue()));
+                copy.put(entry.getKey(), deepCopy(entry.getValue(), depth + 1));
             }
             return copy;
         }
         if (value instanceof List) {
             final List<Object> original = (List<Object>) value;
             final List<Object> copy = new ArrayList<>();
             for (final Object element : original) {
-                copy.add(deepCopy(element));
+                copy.add(deepCopy(element, depth + 1));
             }
             return copy;
         }

aether-datafixers-codec/src/test/java/de/splatgames/aether/datafixers/codec/yaml/snakeyaml/SnakeYamlOpsTest.java

@@ -959,7 +959,7 @@ void createMapHandlesNullValues() {
 
             @SuppressWarnings("unchecked")
             final Map<String, Object> resultMap = (Map<String, Object>) map;
-            assertThat(resultMap.get("key")).isNull();
+            assertThat(resultMap.get("key")).isSameAs(SnakeYamlOps.NULL);
         }
     }
 }

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

@@ -68,7 +68,7 @@ private MigrationReportImpl(final BuilderImpl builder) {
         this.fromVersion = builder.fromVersion;
         this.toVersion = builder.toVersion;
         this.startTime = builder.startTime;
-        this.endTime = Instant.now();
+        this.endTime = builder.endTime;
         this.fixExecutions = List.copyOf(builder.fixExecutions);
         this.touchedTypes = Set.copyOf(builder.touchedTypes);
         this.warnings = List.copyOf(builder.warnings);
@@ -164,6 +164,7 @@ public static final class BuilderImpl implements MigrationReport.Builder {
         private DataVersion fromVersion;
         private DataVersion toVersion;
         private Instant startTime;
+        private Instant endTime;
         private final List<FixExecution> fixExecutions = new ArrayList<>();
         private final Set<TypeReference> touchedTypes = new LinkedHashSet<>();
         private final List<String> warnings = new ArrayList<>();
@@ -259,14 +260,22 @@ public Builder endFix(
             this.fixExecutions.add(execution);
 
             // Reset current fix tracking
+            this.resetFixState();
+
+            return this;
+        }
+
+        /**
+         * Resets fix-tracking state. Called after endFix() or on error paths
+         * to prevent stale state from corrupting subsequent fix records.
+         */
+        private void resetFixState() {
             this.currentFixName = null;
             this.currentFixFromVersion = null;
             this.currentFixToVersion = null;
             this.currentFixStartTime = null;
             this.currentRuleApplications.clear();
             this.currentFixBeforeSnapshot = null;
-
-            return this;
         }
 
         @Override
@@ -301,6 +310,7 @@ public MigrationReport build() {
                 );
             }
 
+            this.endTime = Instant.now();
             return new MigrationReportImpl(this);
         }
     }

aether-datafixers-core/src/main/java/de/splatgames/aether/datafixers/core/fix/DataFixRegistry.java

@@ -216,7 +216,7 @@ public boolean hasFixesInRange(
      *
      * <p>This method is idempotent - calling it multiple times has no effect after the first call.</p>
      */
-    public void freeze() {
+    public synchronized void freeze() {
         if (!this.frozen) {
             // Create an immutable deep copy
             final Map<TypeReference, NavigableMap<DataVersion, List<DataFix<?>>>> immutableCopy = new HashMap<>();

aether-datafixers-core/src/main/java/de/splatgames/aether/datafixers/core/fix/DataFixerImpl.java

@@ -187,8 +187,16 @@ public <T> Dynamic<T> update(
                     diagCtx.reportBuilder().endFix(fix, duration, afterSnapshot);
                 }
             } catch (final FixException e) {
+                if (diagCtx != null) {
+                    final Duration duration = Duration.between(fixStart, Instant.now());
+                    diagCtx.reportBuilder().endFix(fix, duration, null);
+                }
                 throw e; // Re-throw FixException as-is
             } catch (final Exception e) {
+                if (diagCtx != null) {
+                    final Duration duration = Duration.between(fixStart, Instant.now());
+                    diagCtx.reportBuilder().endFix(fix, duration, null);
+                }
                 throw new FixException(
                         "Fix '" + fix.name() + "' failed: " + e.getMessage(),
                         fix.name(),

aether-datafixers-core/src/main/java/de/splatgames/aether/datafixers/core/fix/SchemaDataFix.java

@@ -27,6 +27,7 @@
 import de.splatgames.aether.datafixers.api.TypeReference;
 import de.splatgames.aether.datafixers.api.diagnostic.DiagnosticContext;
 import de.splatgames.aether.datafixers.api.dynamic.Dynamic;
+import de.splatgames.aether.datafixers.api.exception.FixException;
 import de.splatgames.aether.datafixers.api.fix.DataFix;
 import de.splatgames.aether.datafixers.api.fix.DataFixerContext;
 import de.splatgames.aether.datafixers.api.rewrite.TypeRewriteRule;
@@ -170,6 +171,13 @@ protected SchemaDataFix(
         final Typed<?> typedIn = new Typed<>((Type<Object>) logical, input);
         final Typed<?> typedOut = rule.apply(typedIn);
 
+        if (!(typedOut.value() instanceof Dynamic)) {
+            throw new FixException(
+                    "Rule produced non-Dynamic output for type '" + type.getId()
+                            + "' in fix '" + this.name() + "': " + typedOut.value().getClass().getName()
+            );
+        }
+
         @SuppressWarnings("unchecked")
         final Dynamic<Object> result = (Dynamic<Object>) typedOut.value();