Skip to content

Commit a8d4887

Browse files
authored
Merge pull request #255 from aether-framework/bugfix/125-128-148-158-162-163-170-174-176-209-mixed-improvements
Refine documentation to improve clarity on key considerations
2 parents 80358d8 + 0fb3394 commit a8d4887

11 files changed

Lines changed: 74 additions & 9 deletions

File tree

aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/codec/Codecs.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,11 @@ public <T> DataResult<Pair<List<A>, T>> decode(@NotNull final DynamicOps<T> ops,
567567
* GsonOps.INSTANCE, jsonElement);
568568
* }</pre>
569569
*
570+
* <p><b>Null handling:</b> The inner codec must never decode to {@code null}.
571+
* If the inner codec successfully decodes but produces a null value, this codec
572+
* returns a {@link DataResult} error. To handle absent values, the inner codec
573+
* should fail (returning an error), which this codec maps to {@code Optional.empty()}.</p>
574+
*
570575
* @param codec the base codec for the optional value, must not be {@code null}
571576
* @param <A> the type of the optional value
572577
* @return a new codec for {@code Optional<A>}, never {@code null}

aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/dynamic/DynamicOps.java

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,18 @@ default T createMap(@NotNull final Map<T, T> map) {
391391
/**
392392
* Converts a value from another DynamicOps representation.
393393
*
394+
* <p><b>Cross-format edge cases:</b> Converting between different format representations
395+
* may lose information or fail for format-specific features:</p>
396+
* <ul>
397+
* <li><b>Null values:</b> TOML has no null representation; nulls may be lost or cause errors</li>
398+
* <li><b>Array types:</b> TOML requires homogeneous arrays; heterogeneous arrays will fail</li>
399+
* <li><b>XML attributes:</b> Attribute/element distinction is lost through Dynamic round-trips</li>
400+
* <li><b>Numeric precision:</b> BigDecimal handling varies between implementations</li>
401+
* <li><b>Key types:</b> SnakeYAML uses raw strings, Jackson uses TextNode for map keys</li>
402+
* </ul>
403+
* <p>For maximum compatibility, use data shapes that are valid across all target formats:
404+
* string/number/boolean primitives, homogeneous arrays, and non-null values.</p>
405+
*
394406
* @param ops the source ops
395407
* @param input the input value
396408
* @param <U> the source value type

aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/result/DataResult.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -960,7 +960,7 @@ public Optional<A> partialResult() {
960960
if (result.isSuccess()) {
961961
return new Error<>(this.message, result.result().orElse(null));
962962
}
963-
return new Error<>(this.message + "; " + result.error().orElse(""), result.partialResult().orElse(null));
963+
return new Error<>(this.message + "\n caused by: " + result.error().orElse(""), result.partialResult().orElse(null));
964964
}
965965
return (DataResult<B>) this;
966966
}

aether-datafixers-api/src/main/java/de/splatgames/aether/datafixers/api/schema/SchemaRegistry.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,9 @@
6767
* }</pre>
6868
*
6969
* <h2>Thread Safety</h2>
70-
* <p>Implementations should be thread-safe for concurrent access.</p>
70+
* <p>Implementations must be thread-safe for concurrent <b>reads</b> after
71+
* {@code freeze()} has been called. Registration methods are not required to be
72+
* thread-safe and should only be called during single-threaded initialization.</p>
7173
*
7274
* @author Erik Pförtner
7375
* @see Schema

aether-datafixers-cli/src/main/java/de/splatgames/aether/datafixers/cli/command/MigrateCommand.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,13 +378,17 @@ public class MigrateCommand implements Callable<Integer> {
378378
* <li>Full stack traces for errors</li>
379379
* </ul>
380380
*
381+
* <p><b>Security note:</b> Full stack traces may expose internal implementation
382+
* details, file paths, and class names. Do not expose verbose output to untrusted
383+
* users if the CLI is wrapped in a service or web interface.</p>
384+
*
381385
* <p>Default value: {@code false}</p>
382386
*
383387
* <p>CLI usage: {@code -v} or {@code --verbose}</p>
384388
*/
385389
@Option(
386390
names = {"-v", "--verbose"},
387-
description = "Enable verbose output."
391+
description = "Enable verbose output (includes stack traces; see security note in docs)."
388392
)
389393
private boolean verbose;
390394

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,12 @@
156156
* become child elements, but this can be customized with Jackson annotations or mapper
157157
* configuration.</p>
158158
*
159+
* <p><b>Important:</b> The {@code DynamicOps<JsonNode>} model treats all properties uniformly
160+
* as map entries. XML attribute information is <b>not preserved</b> through Dynamic round-trips.
161+
* Attributes become nested map entries indistinguishable from child elements, and re-serialized
162+
* XML may differ structurally from the original. For use cases requiring attribute fidelity,
163+
* process XML directly with Jackson's XML annotations rather than through the Dynamic API.</p>
164+
*
159165
* <h3>Arrays and Repeated Elements</h3>
160166
* <p>XML has no native array type. Arrays are typically represented as repeated elements
161167
* with the same name or wrapped in a container element. The {@link XmlMapper}'s

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1244,6 +1244,16 @@ public <U> Object convertTo(@NotNull final DynamicOps<U> sourceOps,
12441244
* are immutable in Java)</li>
12451245
* </ul>
12461246
*
1247+
* <p><b>Performance Note:</b> Unlike Jackson-based implementations which delegate to
1248+
* the optimized {@code JsonNode.deepCopy()}, this method performs a manual recursive
1249+
* copy using Java reflection-free iteration. For very large data structures, this may
1250+
* be measurably slower. Consider using Jackson-based implementations
1251+
* ({@link de.splatgames.aether.datafixers.codec.yaml.jackson.JacksonYamlOps}) for
1252+
* performance-sensitive use cases.</p>
1253+
*
1254+
* <ul>
1255+
* </ul>
1256+
*
12471257
* <p><b>Performance Note</b></p>
12481258
* <p>Deep copying has O(n) time and space complexity where n is the total number of
12491259
* elements in the structure. For large data structures, this can be significant.

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,9 @@
6161
* }</pre>
6262
*
6363
* <h2>Thread Safety</h2>
64-
* <p>This implementation is not thread-safe. For concurrent access, external
65-
* synchronization is required.</p>
64+
* <p>Thread-safe for concurrent reads after {@link #freeze()} is called.
65+
* Registration methods ({@link #register}) are not thread-safe and should
66+
* only be called during single-threaded initialization.</p>
6667
*
6768
* @author Erik Pförtner
6869
* @see SchemaRegistry

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -451,9 +451,13 @@ private void analyzeStepCoverage(
451451
* but no DataFix is registered to handle the type at this version,
452452
* a coverage gap is recorded.</p>
453453
*
454-
* <p><b>Note:</b> This implementation checks for the presence of any fix
455-
* for the type. A more sophisticated implementation could verify that
456-
* the fix actually handles all specific field changes.</p>
454+
* <p><b>Known Limitation:</b> This implementation only checks whether <i>any</i> fix
455+
* exists for the type at this version. It does <b>not</b> verify that the fix handles
456+
* all specific field changes (e.g., a fix may handle field 'armor' but not 'health').
457+
* This can produce false negatives where a gap exists at the field level but is not
458+
* reported because a type-level fix is present. Verifying field-level coverage would
459+
* require metadata in {@link DataFix} about which fields it modifies, which is not
460+
* currently part of the API.</p>
457461
*
458462
* @param type the type being analyzed, must not be {@code null}
459463
* @param sourceSchema the source schema, must not be {@code null}

aether-datafixers-schema-tools/src/main/java/de/splatgames/aether/datafixers/schematools/introspection/TypeIntrospector.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,12 @@ public static TypeStructure introspect(@NotNull final Type<?> type) {
128128
* <li>Primitive: Returns an empty list</li>
129129
* </ul>
130130
*
131+
* <p><b>Recursive behavior:</b> Fields are extracted recursively from nested types.
132+
* The result includes both parent fields and their nested children using dot-notation
133+
* paths. For example, a type with field "player" containing nested field "position"
134+
* with sub-field "x" produces entries: {@code ["player", "player.position",
135+
* "player.position.x"]}.</p>
136+
*
131137
* @param type the type to extract fields from, must not be {@code null}
132138
* @return a list of fields found in the type, never {@code null}
133139
* @throws NullPointerException if {@code type} is {@code null}

0 commit comments

Comments
 (0)