diff --git a/aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/ConventionChecker.java b/aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/ConventionChecker.java index 5c72dbc..681e5a8 100644 --- a/aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/ConventionChecker.java +++ b/aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/ConventionChecker.java @@ -143,6 +143,11 @@ public static ValidationResult checkSchema( final Type type = schema.types().get(ref); if (type != null) { checkFieldNames(type, typeName, schemaLocation, rules, result); + } else { + result.add(ValidationIssue.warning( + CONVENTION_TYPE_NAME, + "Type '" + typeName + "' is registered but resolves to null" + ).at(schemaLocation + "/" + typeName)); } } diff --git a/aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/ConventionRules.java b/aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/ConventionRules.java index ea575a1..8cce83a 100644 --- a/aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/ConventionRules.java +++ b/aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/ConventionRules.java @@ -80,8 +80,8 @@ public final class ConventionRules { * */ public static final ConventionRules STRICT = builder() - .typeNamePattern(Pattern.compile("^[a-z][a-z0-9_]*$")) - .fieldNamePattern(Pattern.compile("^[a-z][a-z0-9_]*$")) + .typeNamePattern(Pattern.compile("^[a-z_][a-z0-9_]*$")) + .fieldNamePattern(Pattern.compile("^[a-z_][a-z0-9_]*$")) .schemaClassPrefix("Schema") .fixClassSuffix("Fix") .treatViolationsAsErrors(true) @@ -249,11 +249,7 @@ public boolean isValidTypeName(@NotNull final String typeName) { } // Check custom validator - if (this.customTypeValidator != null && !this.customTypeValidator.test(typeName)) { - return false; - } - - return true; + return this.customTypeValidator == null || this.customTypeValidator.test(typeName); } /** @@ -275,11 +271,7 @@ public boolean isValidFieldName(@NotNull final String fieldName) { } // Check custom validator - if (this.customFieldValidator != null && !this.customFieldValidator.test(fieldName)) { - return false; - } - - return true; + return this.customFieldValidator == null || this.customFieldValidator.test(fieldName); } /** @@ -304,11 +296,7 @@ public boolean isValidSchemaClassName(@NotNull final String className) { } // Check suffix - if (this.schemaClassSuffix != null && !className.endsWith(this.schemaClassSuffix)) { - return false; - } - - return true; + return this.schemaClassSuffix == null || className.endsWith(this.schemaClassSuffix); } /** @@ -333,11 +321,7 @@ public boolean isValidFixClassName(@NotNull final String className) { } // Check suffix - if (this.fixClassSuffix != null && !className.endsWith(this.fixClassSuffix)) { - return false; - } - - return true; + return this.fixClassSuffix == null || className.endsWith(this.fixClassSuffix); } /** diff --git a/aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/SchemaValidator.java b/aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/SchemaValidator.java index cfb8b0c..50623c7 100644 --- a/aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/SchemaValidator.java +++ b/aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/SchemaValidator.java @@ -190,9 +190,14 @@ public static SchemaValidator forBootstrap(@NotNull final DataFixerBootstrap boo // Bootstrap into capturing registries final SchemaRegistry schemaRegistry = new SimpleSchemaRegistry(); - final DataFixerBuilder fixerBuilder = new DataFixerBuilder(new DataVersion(Integer.MAX_VALUE)); - bootstrap.registerSchemas(schemaRegistry); + + final int maxVersion = schemaRegistry.stream() + .mapToInt(s -> s.version().getVersion()) + .max() + .orElse(0); + + final DataFixerBuilder fixerBuilder = new DataFixerBuilder(new DataVersion(maxVersion)); bootstrap.registerFixes(fixerBuilder); return new SchemaValidator(null, schemaRegistry, fixerBuilder); @@ -390,11 +395,11 @@ private ValidationResult validateFixCoverageInternal() { final int minVersion = schemas.stream() .map(s -> s.version().getVersion()) .min(Comparator.naturalOrder()) - .orElse(0); + .orElseThrow(); final int maxVersion = schemas.stream() .map(s -> s.version().getVersion()) .max(Comparator.naturalOrder()) - .orElse(0); + .orElseThrow(); if (minVersion == maxVersion) { return ValidationResult.empty(); diff --git a/aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/StructureValidator.java b/aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/StructureValidator.java index 03164c6..ca20e10 100644 --- a/aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/StructureValidator.java +++ b/aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/StructureValidator.java @@ -117,6 +117,16 @@ public static ValidationResult validate( // Check parent chain if (schema.parent() != null) { + // Check that parent exists in the registry if one is provided + if (registry != null) { + final int parentVersion = schema.parent().version().getVersion(); + if (registry.get(new de.splatgames.aether.datafixers.api.DataVersion(parentVersion)) == null) { + result.add(ValidationIssue.error(STRUCTURE_MISSING_PARENT, + "Parent schema v" + parentVersion + " not found in registry") + .at(location) + .withContext("parentVersion", parentVersion)); + } + } validateParentChain(schema, registry, result, location); } diff --git a/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/assertion/DataResultAssert.java b/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/assertion/DataResultAssert.java index e4d611e..6379aec 100644 --- a/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/assertion/DataResultAssert.java +++ b/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/assertion/DataResultAssert.java @@ -194,7 +194,7 @@ public DataResultAssert hasValue(@NotNull final A expected) { @NotNull public DataResultAssert hasValueSatisfying(@NotNull final Consumer requirements) { this.isSuccess(); - requirements.accept(this.actual.result().orElse(null)); + requirements.accept(this.actual.result().orElseThrow()); return this; } diff --git a/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/assertion/DynamicAssert.java b/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/assertion/DynamicAssert.java index c3784b9..0e5adcf 100644 --- a/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/assertion/DynamicAssert.java +++ b/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/assertion/DynamicAssert.java @@ -616,11 +616,22 @@ public DynamicAssert isNotEmpty() { public DynamicAssert containsStringValues(@NotNull final String... expected) { isNotNull(); this.isList(); - final List actualValues = this.actual.asListStream() + final List> elements = this.actual.asListStream() .result() - .map(s -> s.map(d -> d.asString().orElse(null)).collect(Collectors.toList())) + .map(s -> s.collect(Collectors.toList())) .orElse(List.of()); + final List actualValues = new java.util.ArrayList<>(); + for (int i = 0; i < elements.size(); i++) { + final Dynamic element = elements.get(i); + final String value = element.asString().orElse(null); + if (value == null && !element.isString()) { + failWithMessage("Expected%s element [%d] to be a string but was: %s", + this.pathInfo(), i, this.describeElement(element)); + } + actualValues.add(value); + } + for (final String exp : expected) { if (!actualValues.contains(exp)) { failWithMessage("Expected%s to contain '%s' but values were: %s", @@ -640,11 +651,22 @@ public DynamicAssert containsStringValues(@NotNull final String... expected) public DynamicAssert containsIntValues(final int... expected) { isNotNull(); this.isList(); - final List actualValues = this.actual.asListStream() + final List> elements = this.actual.asListStream() .result() - .map(s -> s.map(d -> d.asInt().orElse(null)).collect(Collectors.toList())) + .map(s -> s.collect(Collectors.toList())) .orElse(List.of()); + final List actualValues = new java.util.ArrayList<>(); + for (int i = 0; i < elements.size(); i++) { + final Dynamic element = elements.get(i); + final Integer value = element.asInt().orElse(null); + if (value == null && !element.isNumber()) { + failWithMessage("Expected%s element [%d] to be an integer but was: %s", + this.pathInfo(), i, this.describeElement(element)); + } + actualValues.add(value); + } + for (final int exp : expected) { if (!actualValues.contains(exp)) { failWithMessage("Expected%s to contain %d but values were: %s", @@ -725,6 +747,15 @@ private String availableFields() { return fields.isEmpty() ? "(none)" : String.join(", ", fields); } + private String describeElement(final Dynamic element) { + if (element.isString()) return "string: " + element.asString().orElse("?"); + if (element.isNumber()) return "number: " + element.asNumber().orElse(null); + if (element.isBoolean()) return "boolean: " + element.asBoolean().orElse(null); + if (element.isMap()) return "map"; + if (element.isList()) return "list"; + return "unknown: " + element.value(); + } + private String fieldsOf(final Dynamic d) { return d.asMapStream() .result() diff --git a/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/context/AssertingContext.java b/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/context/AssertingContext.java index 2a5f860..77ef557 100644 --- a/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/context/AssertingContext.java +++ b/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/context/AssertingContext.java @@ -181,11 +181,15 @@ private static String formatMessage(final String message, final Object[] args) { if (args == null || args.length == 0) { return message; } - String result = message; + final StringBuilder result = new StringBuilder(message); for (final Object arg : args) { - result = result.replaceFirst("\\{}", String.valueOf(arg)); + final int idx = result.indexOf("{}"); + if (idx < 0) { + break; + } + result.replace(idx, idx + 2, String.valueOf(arg)); } - return result; + return result.toString(); } private enum Mode { diff --git a/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/context/RecordingContext.java b/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/context/RecordingContext.java index 2f3e8b4..f2b2914 100644 --- a/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/context/RecordingContext.java +++ b/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/context/RecordingContext.java @@ -266,11 +266,15 @@ public String formattedMessage() { if (this.args == null || this.args.length == 0) { return this.message; } - String result = this.message; + final StringBuilder result = new StringBuilder(this.message); for (final Object arg : this.args) { - result = result.replaceFirst("\\{}", String.valueOf(arg)); + final int idx = result.indexOf("{}"); + if (idx < 0) { + break; + } + result.replace(idx, idx + 2, String.valueOf(arg)); } - return result; + return result.toString(); } @Override diff --git a/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/DataFixTester.java b/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/DataFixTester.java index 4b86255..4523c0c 100644 --- a/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/DataFixTester.java +++ b/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/DataFixTester.java @@ -223,11 +223,7 @@ public DataFixTester expectOutput(@NotNull final Dynamic expected) { public Dynamic apply() { this.validateConfiguration(); - final DataFixerContext effectiveContext = this.useRecordingContext - ? new RecordingContext() - : this.context; - - return this.fix.apply(this.typeReference, this.input, effectiveContext); + return this.fix.apply(this.typeReference, this.input, this.resolveContext()); } /** @@ -241,19 +237,8 @@ public Dynamic apply() { public DataFixVerification verify() { this.validateConfiguration(); - final RecordingContext recordingContext; - final DataFixerContext effectiveContext; - - if (this.useRecordingContext) { - recordingContext = new RecordingContext(); - effectiveContext = recordingContext; - } else if (this.context instanceof RecordingContext rc) { - recordingContext = rc; - effectiveContext = rc; - } else { - recordingContext = null; - effectiveContext = this.context; - } + final DataFixerContext effectiveContext = this.resolveContext(); + final RecordingContext recordingContext = effectiveContext instanceof RecordingContext rc ? rc : null; final Dynamic result = this.fix.apply(this.typeReference, this.input, effectiveContext); @@ -272,6 +257,20 @@ public DataFixVerification verify() { return new DataFixVerification<>(result, recordingContext, true); } + // ==================== Internal ==================== + + /** + * Resolves the effective context, ensuring a single instance is used + * across both apply() and verify(). + */ + @NotNull + private DataFixerContext resolveContext() { + if (this.useRecordingContext) { + return new RecordingContext(); + } + return this.context; + } + // ==================== Validation ==================== private void validateConfiguration() { diff --git a/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/MigrationTester.java b/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/MigrationTester.java index 63140a6..3863bff 100644 --- a/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/MigrationTester.java +++ b/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/MigrationTester.java @@ -31,6 +31,9 @@ import de.splatgames.aether.datafixers.core.fix.DataFixerBuilder; import org.jetbrains.annotations.NotNull; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.function.Consumer; @@ -298,12 +301,10 @@ private void validateConfiguration() { */ public static final class FixerSetup { - private final DataFixerBuilder builder; + private final List>> pendingFixes = new ArrayList<>(); private int maxVersion = 1; FixerSetup() { - // Start with a placeholder version, will be updated - this.builder = new DataFixerBuilder(new DataVersion(Integer.MAX_VALUE)); } /** @@ -317,7 +318,7 @@ public static final class FixerSetup { public FixerSetup addFix(@NotNull final TypeReference type, @NotNull final DataFix fix) { Preconditions.checkNotNull(type, "type must not be null"); Preconditions.checkNotNull(fix, "fix must not be null"); - this.builder.addFix(type, fix); + this.pendingFixes.add(Map.entry(type, fix)); this.maxVersion = Math.max(this.maxVersion, fix.toVersion().getVersion()); return this; } @@ -337,7 +338,11 @@ public FixerSetup addFix(@NotNull final String typeId, @NotNull final DataFix } DataFixer build() { - return this.builder.build(); + final DataFixerBuilder actualBuilder = new DataFixerBuilder(new DataVersion(this.maxVersion)); + for (final Map.Entry> entry : this.pendingFixes) { + actualBuilder.addFix(entry.getKey(), entry.getValue()); + } + return actualBuilder.build(); } } } diff --git a/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/SchemaTester.java b/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/SchemaTester.java index 33b3343..a96cfdb 100644 --- a/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/SchemaTester.java +++ b/aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/SchemaTester.java @@ -292,6 +292,10 @@ public SchemaTester hasParent() { */ @NotNull public SchemaTester hasNoParent() { + if (this.expectedParent != null) { + throw new IllegalStateException( + "Contradictory configuration: hasNoParent() called after inheritsFrom()"); + } this.expectHasParent = false; return this; } @@ -306,6 +310,10 @@ public SchemaTester hasNoParent() { @NotNull public SchemaTester inheritsFrom(@NotNull final Schema parent) { Preconditions.checkNotNull(parent, "parent must not be null"); + if (Boolean.FALSE.equals(this.expectHasParent)) { + throw new IllegalStateException( + "Contradictory configuration: inheritsFrom() called after hasNoParent()"); + } this.expectedParent = parent; this.expectHasParent = true; return this; @@ -369,7 +377,8 @@ public SchemaTester verify() { // Validate parent existence if (this.expectHasParent != null) { - final boolean hasParent = this.schema.parent() != null; + final Schema actualParentRef = this.schema.parent(); + final boolean hasParent = actualParentRef != null; if (this.expectHasParent && !hasParent) { throw new AssertionError(String.format( "Schema v%d has no parent, but one was expected", @@ -380,7 +389,7 @@ public SchemaTester verify() { throw new AssertionError(String.format( "Schema v%d has a parent (v%d), but none was expected", this.schema.version().getVersion(), - this.schema.parent().version().getVersion() + actualParentRef.version().getVersion() )); } }