Skip to content

Commit b2f151b

Browse files
SplatcrafterSplatcrafter
authored
Merge pull request #252 from aether-framework/bugfix/166-167-171-173-177-179-180-181-182-183-184-186-testkit-and-schematools-fixes
Refactor and optimize validation logic for improved error handling
2 parents 1137efb + 2e37c91 commit b2f151b

11 files changed

Lines changed: 118 additions & 62 deletions

File tree

‎aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/ConventionChecker.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,11 @@ public static ValidationResult checkSchema(
143143
final Type<?> type = schema.types().get(ref);
144144
if (type != null) {
145145
checkFieldNames(type, typeName, schemaLocation, rules, result);
146+
} else {
147+
result.add(ValidationIssue.warning(
148+
CONVENTION_TYPE_NAME,
149+
"Type '" + typeName + "' is registered but resolves to null"
150+
).at(schemaLocation + "/" + typeName));
146151
}
147152
}
148153

‎aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/ConventionRules.java‎

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,8 @@ public final class ConventionRules {
8080
* </ul>
8181
*/
8282
public static final ConventionRules STRICT = builder()
83-
.typeNamePattern(Pattern.compile("^[a-z][a-z0-9_]*$"))
84-
.fieldNamePattern(Pattern.compile("^[a-z][a-z0-9_]*$"))
83+
.typeNamePattern(Pattern.compile("^[a-z_][a-z0-9_]*$"))
84+
.fieldNamePattern(Pattern.compile("^[a-z_][a-z0-9_]*$"))
8585
.schemaClassPrefix("Schema")
8686
.fixClassSuffix("Fix")
8787
.treatViolationsAsErrors(true)
@@ -249,11 +249,7 @@ public boolean isValidTypeName(@NotNull final String typeName) {
249249
}
250250

251251
// Check custom validator
252-
if (this.customTypeValidator != null && !this.customTypeValidator.test(typeName)) {
253-
return false;
254-
}
255-
256-
return true;
252+
return this.customTypeValidator == null || this.customTypeValidator.test(typeName);
257253
}
258254

259255
/**
@@ -275,11 +271,7 @@ public boolean isValidFieldName(@NotNull final String fieldName) {
275271
}
276272

277273
// Check custom validator
278-
if (this.customFieldValidator != null && !this.customFieldValidator.test(fieldName)) {
279-
return false;
280-
}
281-
282-
return true;
274+
return this.customFieldValidator == null || this.customFieldValidator.test(fieldName);
283275
}
284276

285277
/**
@@ -304,11 +296,7 @@ public boolean isValidSchemaClassName(@NotNull final String className) {
304296
}
305297

306298
// Check suffix
307-
if (this.schemaClassSuffix != null && !className.endsWith(this.schemaClassSuffix)) {
308-
return false;
309-
}
310-
311-
return true;
299+
return this.schemaClassSuffix == null || className.endsWith(this.schemaClassSuffix);
312300
}
313301

314302
/**
@@ -333,11 +321,7 @@ public boolean isValidFixClassName(@NotNull final String className) {
333321
}
334322

335323
// Check suffix
336-
if (this.fixClassSuffix != null && !className.endsWith(this.fixClassSuffix)) {
337-
return false;
338-
}
339-
340-
return true;
324+
return this.fixClassSuffix == null || className.endsWith(this.fixClassSuffix);
341325
}
342326

343327
/**

‎aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/SchemaValidator.java‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,14 @@ public static SchemaValidator forBootstrap(@NotNull final DataFixerBootstrap boo
190190

191191
// Bootstrap into capturing registries
192192
final SchemaRegistry schemaRegistry = new SimpleSchemaRegistry();
193-
final DataFixerBuilder fixerBuilder = new DataFixerBuilder(new DataVersion(Integer.MAX_VALUE));
194-
195193
bootstrap.registerSchemas(schemaRegistry);
194+
195+
final int maxVersion = schemaRegistry.stream()
196+
.mapToInt(s -> s.version().getVersion())
197+
.max()
198+
.orElse(0);
199+
200+
final DataFixerBuilder fixerBuilder = new DataFixerBuilder(new DataVersion(maxVersion));
196201
bootstrap.registerFixes(fixerBuilder);
197202

198203
return new SchemaValidator(null, schemaRegistry, fixerBuilder);
@@ -390,11 +395,11 @@ private ValidationResult validateFixCoverageInternal() {
390395
final int minVersion = schemas.stream()
391396
.map(s -> s.version().getVersion())
392397
.min(Comparator.naturalOrder())
393-
.orElse(0);
398+
.orElseThrow();
394399
final int maxVersion = schemas.stream()
395400
.map(s -> s.version().getVersion())
396401
.max(Comparator.naturalOrder())
397-
.orElse(0);
402+
.orElseThrow();
398403

399404
if (minVersion == maxVersion) {
400405
return ValidationResult.empty();

‎aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/validation/StructureValidator.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,16 @@ public static ValidationResult validate(
117117

118118
// Check parent chain
119119
if (schema.parent() != null) {
120+
// Check that parent exists in the registry if one is provided
121+
if (registry != null) {
122+
final int parentVersion = schema.parent().version().getVersion();
123+
if (registry.get(new de.splatgames.aether.datafixers.api.DataVersion(parentVersion)) == null) {
124+
result.add(ValidationIssue.error(STRUCTURE_MISSING_PARENT,
125+
"Parent schema v" + parentVersion + " not found in registry")
126+
.at(location)
127+
.withContext("parentVersion", parentVersion));
128+
}
129+
}
120130
validateParentChain(schema, registry, result, location);
121131
}
122132

‎aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/assertion/DataResultAssert.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ public DataResultAssert<A> hasValue(@NotNull final A expected) {
194194
@NotNull
195195
public DataResultAssert<A> hasValueSatisfying(@NotNull final Consumer<A> requirements) {
196196
this.isSuccess();
197-
requirements.accept(this.actual.result().orElse(null));
197+
requirements.accept(this.actual.result().orElseThrow());
198198
return this;
199199
}
200200

‎aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/assertion/DynamicAssert.java‎

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -616,11 +616,22 @@ public DynamicAssert<T> isNotEmpty() {
616616
public DynamicAssert<T> containsStringValues(@NotNull final String... expected) {
617617
isNotNull();
618618
this.isList();
619-
final List<String> actualValues = this.actual.asListStream()
619+
final List<Dynamic<T>> elements = this.actual.asListStream()
620620
.result()
621-
.map(s -> s.map(d -> d.asString().orElse(null)).collect(Collectors.toList()))
621+
.map(s -> s.collect(Collectors.toList()))
622622
.orElse(List.of());
623623

624+
final List<String> actualValues = new java.util.ArrayList<>();
625+
for (int i = 0; i < elements.size(); i++) {
626+
final Dynamic<T> element = elements.get(i);
627+
final String value = element.asString().orElse(null);
628+
if (value == null && !element.isString()) {
629+
failWithMessage("Expected%s element [%d] to be a string but was: %s",
630+
this.pathInfo(), i, this.describeElement(element));
631+
}
632+
actualValues.add(value);
633+
}
634+
624635
for (final String exp : expected) {
625636
if (!actualValues.contains(exp)) {
626637
failWithMessage("Expected%s to contain '%s' but values were: %s",
@@ -640,11 +651,22 @@ public DynamicAssert<T> containsStringValues(@NotNull final String... expected)
640651
public DynamicAssert<T> containsIntValues(final int... expected) {
641652
isNotNull();
642653
this.isList();
643-
final List<Integer> actualValues = this.actual.asListStream()
654+
final List<Dynamic<T>> elements = this.actual.asListStream()
644655
.result()
645-
.map(s -> s.map(d -> d.asInt().orElse(null)).collect(Collectors.toList()))
656+
.map(s -> s.collect(Collectors.toList()))
646657
.orElse(List.of());
647658

659+
final List<Integer> actualValues = new java.util.ArrayList<>();
660+
for (int i = 0; i < elements.size(); i++) {
661+
final Dynamic<T> element = elements.get(i);
662+
final Integer value = element.asInt().orElse(null);
663+
if (value == null && !element.isNumber()) {
664+
failWithMessage("Expected%s element [%d] to be an integer but was: %s",
665+
this.pathInfo(), i, this.describeElement(element));
666+
}
667+
actualValues.add(value);
668+
}
669+
648670
for (final int exp : expected) {
649671
if (!actualValues.contains(exp)) {
650672
failWithMessage("Expected%s to contain %d but values were: %s",
@@ -725,6 +747,15 @@ private String availableFields() {
725747
return fields.isEmpty() ? "(none)" : String.join(", ", fields);
726748
}
727749

750+
private String describeElement(final Dynamic<T> element) {
751+
if (element.isString()) return "string: " + element.asString().orElse("?");
752+
if (element.isNumber()) return "number: " + element.asNumber().orElse(null);
753+
if (element.isBoolean()) return "boolean: " + element.asBoolean().orElse(null);
754+
if (element.isMap()) return "map";
755+
if (element.isList()) return "list";
756+
return "unknown: " + element.value();
757+
}
758+
728759
private String fieldsOf(final Dynamic<T> d) {
729760
return d.asMapStream()
730761
.result()

‎aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/context/AssertingContext.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,11 +181,15 @@ private static String formatMessage(final String message, final Object[] args) {
181181
if (args == null || args.length == 0) {
182182
return message;
183183
}
184-
String result = message;
184+
final StringBuilder result = new StringBuilder(message);
185185
for (final Object arg : args) {
186-
result = result.replaceFirst("\\{}", String.valueOf(arg));
186+
final int idx = result.indexOf("{}");
187+
if (idx < 0) {
188+
break;
189+
}
190+
result.replace(idx, idx + 2, String.valueOf(arg));
187191
}
188-
return result;
192+
return result.toString();
189193
}
190194

191195
private enum Mode {

‎aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/context/RecordingContext.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -266,11 +266,15 @@ public String formattedMessage() {
266266
if (this.args == null || this.args.length == 0) {
267267
return this.message;
268268
}
269-
String result = this.message;
269+
final StringBuilder result = new StringBuilder(this.message);
270270
for (final Object arg : this.args) {
271-
result = result.replaceFirst("\\{}", String.valueOf(arg));
271+
final int idx = result.indexOf("{}");
272+
if (idx < 0) {
273+
break;
274+
}
275+
result.replace(idx, idx + 2, String.valueOf(arg));
272276
}
273-
return result;
277+
return result.toString();
274278
}
275279

276280
@Override

‎aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/DataFixTester.java‎

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -223,11 +223,7 @@ public DataFixTester<T> expectOutput(@NotNull final Dynamic<T> expected) {
223223
public Dynamic<T> apply() {
224224
this.validateConfiguration();
225225

226-
final DataFixerContext effectiveContext = this.useRecordingContext
227-
? new RecordingContext()
228-
: this.context;
229-
230-
return this.fix.apply(this.typeReference, this.input, effectiveContext);
226+
return this.fix.apply(this.typeReference, this.input, this.resolveContext());
231227
}
232228

233229
/**
@@ -241,19 +237,8 @@ public Dynamic<T> apply() {
241237
public DataFixVerification<T> verify() {
242238
this.validateConfiguration();
243239

244-
final RecordingContext recordingContext;
245-
final DataFixerContext effectiveContext;
246-
247-
if (this.useRecordingContext) {
248-
recordingContext = new RecordingContext();
249-
effectiveContext = recordingContext;
250-
} else if (this.context instanceof RecordingContext rc) {
251-
recordingContext = rc;
252-
effectiveContext = rc;
253-
} else {
254-
recordingContext = null;
255-
effectiveContext = this.context;
256-
}
240+
final DataFixerContext effectiveContext = this.resolveContext();
241+
final RecordingContext recordingContext = effectiveContext instanceof RecordingContext rc ? rc : null;
257242

258243
final Dynamic<T> result = this.fix.apply(this.typeReference, this.input, effectiveContext);
259244

@@ -272,6 +257,20 @@ public DataFixVerification<T> verify() {
272257
return new DataFixVerification<>(result, recordingContext, true);
273258
}
274259

260+
// ==================== Internal ====================
261+
262+
/**
263+
* Resolves the effective context, ensuring a single instance is used
264+
* across both apply() and verify().
265+
*/
266+
@NotNull
267+
private DataFixerContext resolveContext() {
268+
if (this.useRecordingContext) {
269+
return new RecordingContext();
270+
}
271+
return this.context;
272+
}
273+
275274
// ==================== Validation ====================
276275

277276
private void validateConfiguration() {

‎aether-datafixers-testkit/src/main/java/de/splatgames/aether/datafixers/testkit/harness/MigrationTester.java‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@
3131
import de.splatgames.aether.datafixers.core.fix.DataFixerBuilder;
3232
import org.jetbrains.annotations.NotNull;
3333

34+
import java.util.ArrayList;
35+
import java.util.List;
36+
import java.util.Map;
3437
import java.util.Objects;
3538
import java.util.function.Consumer;
3639

@@ -298,12 +301,10 @@ private void validateConfiguration() {
298301
*/
299302
public static final class FixerSetup {
300303

301-
private final DataFixerBuilder builder;
304+
private final List<Map.Entry<TypeReference, DataFix<?>>> pendingFixes = new ArrayList<>();
302305
private int maxVersion = 1;
303306

304307
FixerSetup() {
305-
// Start with a placeholder version, will be updated
306-
this.builder = new DataFixerBuilder(new DataVersion(Integer.MAX_VALUE));
307308
}
308309

309310
/**
@@ -317,7 +318,7 @@ public static final class FixerSetup {
317318
public FixerSetup addFix(@NotNull final TypeReference type, @NotNull final DataFix<?> fix) {
318319
Preconditions.checkNotNull(type, "type must not be null");
319320
Preconditions.checkNotNull(fix, "fix must not be null");
320-
this.builder.addFix(type, fix);
321+
this.pendingFixes.add(Map.entry(type, fix));
321322
this.maxVersion = Math.max(this.maxVersion, fix.toVersion().getVersion());
322323
return this;
323324
}
@@ -337,7 +338,11 @@ public FixerSetup addFix(@NotNull final String typeId, @NotNull final DataFix<?>
337338
}
338339

339340
DataFixer build() {
340-
return this.builder.build();
341+
final DataFixerBuilder actualBuilder = new DataFixerBuilder(new DataVersion(this.maxVersion));
342+
for (final Map.Entry<TypeReference, DataFix<?>> entry : this.pendingFixes) {
343+
actualBuilder.addFix(entry.getKey(), entry.getValue());
344+
}
345+
return actualBuilder.build();
341346
}
342347
}
343348
}

0 commit comments

Comments
 (0)