diff --git a/CHANGELOG.md b/CHANGELOG.md index 484b3c5a..49f576df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,20 @@ JitPack continue to resolve through the existing coordinates. land in `target/japicmp/`. JitPack repository is scoped to the `japicmp` profile, so downstream consumers do not inherit it. +### Engine internals (no behaviour change) + +- **`RowSlots` helper extracted** from `LayoutCompiler` and + `NodeDefinitionSupport`. The defence-in-depth `IllegalArgumentException` + guard added in v1.6.5 (PR-7.3) for the row weights / children size + mismatch lived as duplicated inline code at both engine call sites + with no direct test — a future refactor could have silently deleted + either copy. The validation now lives in + `com.demcha.compose.document.layout.RowSlots#validateWeightsMatchChildren` + (package-private), with `RowSlotsTest` driving it directly. Error + message is unchanged. `GraphCompose.DocumentBuilder#pageBackgrounds(...)` + Javadoc now spells out the empty-list-clears semantics in prose, not + only in the `@param` line. + ## v1.6.5 — 2026-05-30 ### Templates v2 diff --git a/src/main/java/com/demcha/compose/GraphCompose.java b/src/main/java/com/demcha/compose/GraphCompose.java index 6bbfcdfb..06a9b7bc 100644 --- a/src/main/java/com/demcha/compose/GraphCompose.java +++ b/src/main/java/com/demcha/compose/GraphCompose.java @@ -251,6 +251,11 @@ public DocumentBuilder pageBackground(java.awt.Color color) { * {@link com.demcha.compose.document.api.DocumentSession#pageBackgrounds} * for the full semantics. * + *

Calling this method with an empty list is an explicit clear: + * it overrides any earlier {@link #pageBackground(com.demcha.compose.document.style.DocumentColor)} + * call on the same builder and emits no page-background fragments. + * Passing {@code null} has the same effect.

+ * * @param fills ordered fills, or {@code null}/empty to clear * @return this builder */ @@ -381,9 +386,8 @@ public DocumentSession create() { markdown, guideLines); if (pageBackgrounds != null) { - // Explicit pageBackgrounds() call wins — even an empty - // list is an intentional clear that should override any - // earlier pageBackground(color) on the same builder. + // Explicit pageBackgrounds() call wins over a prior + // pageBackground(color). Empty list = clear; see builder Javadoc. session.pageBackgrounds(pageBackgrounds); } else if (pageBackground != null) { session.pageBackground(pageBackground); diff --git a/src/main/java/com/demcha/compose/document/layout/LayoutCompiler.java b/src/main/java/com/demcha/compose/document/layout/LayoutCompiler.java index cdb857cf..97c233f9 100644 --- a/src/main/java/com/demcha/compose/document/layout/LayoutCompiler.java +++ b/src/main/java/com/demcha/compose/document/layout/LayoutCompiler.java @@ -746,11 +746,7 @@ private static double[] distributeRowSlotWidths(List children, } return slots; } - if (weights.size() != n) { - throw new IllegalArgumentException( - "Row weights size (" + weights.size() + ") must match children size (" + n - + "). Pass exactly " + n + " weight(s) or leave weights empty for an even split."); - } + RowSlots.validateWeightsMatchChildren(weights, n); double total = 0.0; for (double w : weights) { total += w; diff --git a/src/main/java/com/demcha/compose/document/layout/NodeDefinitionSupport.java b/src/main/java/com/demcha/compose/document/layout/NodeDefinitionSupport.java index 96434d11..9ab92dfb 100644 --- a/src/main/java/com/demcha/compose/document/layout/NodeDefinitionSupport.java +++ b/src/main/java/com/demcha/compose/document/layout/NodeDefinitionSupport.java @@ -324,11 +324,7 @@ public static MeasureResult measureRow(RowNode node, slotWidths[i] = share; } } else { - if (node.weights().size() != n) { - throw new IllegalArgumentException( - "Row weights size (" + node.weights().size() + ") must match children size (" + n - + "). Pass exactly " + n + " weight(s) or leave weights empty for an even split."); - } + RowSlots.validateWeightsMatchChildren(node.weights(), n); double total = 0.0; for (Double w : node.weights()) { total += w; diff --git a/src/main/java/com/demcha/compose/document/layout/RowSlots.java b/src/main/java/com/demcha/compose/document/layout/RowSlots.java new file mode 100644 index 00000000..8b11eb20 --- /dev/null +++ b/src/main/java/com/demcha/compose/document/layout/RowSlots.java @@ -0,0 +1,44 @@ +package com.demcha.compose.document.layout; + +import java.util.List; + +/** + * Shared validation helpers for row weight / children distribution code. + * + *

Centralises the {@link IllegalArgumentException} contract used by both + * {@link LayoutCompiler#distributeRowSlotWidths(List, List, double, double) compile-phase} + * and {@link NodeDefinitionSupport#measureRow measure-phase} row distribution. + * The {@code RowNode} canonical constructor already rejects a mismatched + * weights list at construction time; these helpers are defence-in-depth + * for any path that bypasses the constructor (reflection-based + * deserialization, framework proxies, etc.) and arrives at the engine + * with an inconsistent {@code (weights, children)} pair.

+ * + *

Package-private intentionally — engine surface, not public API.

+ * + * @author Artem Demchyshyn + */ +final class RowSlots { + + private RowSlots() { + // Utility class, no instantiation. + } + + /** + * Asserts that an explicit {@code weights} list matches the row's + * children count. Callers must skip this check when {@code weights} + * is null or empty — the even-split fallback applies there instead. + * + * @param weights non-null, non-empty weights list + * @param childCount number of row children + * @throws IllegalArgumentException if {@code weights.size() != childCount} + */ + static void validateWeightsMatchChildren(List weights, int childCount) { + if (weights.size() != childCount) { + throw new IllegalArgumentException( + "Row weights size (" + weights.size() + ") must match children size (" + + childCount + "). Pass exactly " + childCount + + " weight(s) or leave weights empty for an even split."); + } + } +} diff --git a/src/test/java/com/demcha/compose/document/layout/RowSlotsTest.java b/src/test/java/com/demcha/compose/document/layout/RowSlotsTest.java new file mode 100644 index 00000000..70323897 --- /dev/null +++ b/src/test/java/com/demcha/compose/document/layout/RowSlotsTest.java @@ -0,0 +1,61 @@ +package com.demcha.compose.document.layout; + +import org.junit.jupiter.api.Test; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +/** + * Engine-level contract for the row weights / children-count guard + * shared by {@link LayoutCompiler} and {@link NodeDefinitionSupport}. + * + *

The {@code RowNode} canonical constructor already rejects the + * mismatched state via {@link RowBuilder}, so under normal authoring the + * helper is unreachable. The helper exists for defence-in-depth paths + * that bypass the constructor (reflection-based deserialization, etc.); + * these tests pin the contract so a future refactor cannot silently + * delete the guard at either call site.

+ */ +class RowSlotsTest { + + @Test + void matchingSizesPassWithoutThrowing() { + assertThatCode(() -> RowSlots.validateWeightsMatchChildren(List.of(1.0, 2.0, 3.0), 3)) + .doesNotThrowAnyException(); + } + + @Test + void moreWeightsThanChildrenIsRejectedWithBothSizesNamed() { + assertThatThrownBy(() -> RowSlots.validateWeightsMatchChildren(List.of(1.0, 2.0, 3.0), 2)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("weights") + .hasMessageContaining("children") + .hasMessageContaining("(3)") + .hasMessageContaining("(2)"); + } + + @Test + void fewerWeightsThanChildrenIsRejectedWithBothSizesNamed() { + assertThatThrownBy(() -> RowSlots.validateWeightsMatchChildren(List.of(1.0, 2.0), 5)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("(2)") + .hasMessageContaining("(5)"); + } + + @Test + void errorMessageHintsAtTheFix() { + // The senior-review bar for engine exception messages: name the + // values AND tell the caller how to fix it. Asserting the verb + // is enough — the exact wording is implementation detail. + assertThatThrownBy(() -> RowSlots.validateWeightsMatchChildren(List.of(1.0), 4)) + .isInstanceOf(IllegalArgumentException.class) + .extracting(Throwable::getMessage) + .satisfies(msg -> { + String s = (String) msg; + assertThat(s).containsAnyOf("Pass", "Provide", "Use"); + }); + } +}