Skip to content

Commit 2e37c91

Browse files
SplatcrafterSplatcrafter
committed
Refactor validation and assertion logic to enhance error handling and message clarity across DynamicAssert, StructureValidator, and DataFixTester.
1 parent 0d088b1 commit 2e37c91

5 files changed

Lines changed: 79 additions & 30 deletions

File tree

‎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/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/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)