From 481c572857f930709db88247812f5dddb0fccf91 Mon Sep 17 00:00:00 2001 From: DemchaAV Date: Sat, 30 May 2026 10:30:58 +0100 Subject: [PATCH] fix(engine): empty pageBackgrounds clears; row weights/children mismatch is IAE Two v1.6.5 stabilization fixes flagged by the extended audit (2026-05-30): 1. GraphCompose.document().pageBackgrounds(emptyList()) now actually clears any earlier pageBackground(color), matching the builder's Javadoc. The previous `if (!pageBackgrounds.isEmpty())` guard meant an explicit empty list was silently treated as "leave unchanged". 2. LayoutCompiler#distributeRowSlotWidths and NodeDefinitionSupport#measureRow now reject a weights list whose size does not match the row's children with a descriptive IllegalArgumentException, instead of walking off the end of the list with an IndexOutOfBoundsException. RowNode's canonical constructor already validated this at construction; the new engine guards are defence-in-depth for any path that bypasses the constructor (reflection-based deserialization, etc.). Test coverage: - PageBackgroundTest#pageBackgroundsWithEmptyListClearsEarlierPageBackgroundColor - RowBuilderTest#rowBuilderWeightsMismatchAtBuildFailsWithDescriptiveMessage - RowBuilderTest#rowNodeConstructorRejectsWeightsThatDoNotMatchChildren ./mvnw test -pl . -> 1019 tests, 0 failures, 0 errors (~45s). Part of v1.6.5 publish hygiene (PR-7.3). --- CHANGELOG.md | 20 ++++++++++ .../java/com/demcha/compose/GraphCompose.java | 4 +- .../document/layout/LayoutCompiler.java | 5 +++ .../layout/NodeDefinitionSupport.java | 5 +++ .../document/api/PageBackgroundTest.java | 23 +++++++++++ .../compose/document/dsl/RowBuilderTest.java | 40 +++++++++++++++++++ 6 files changed, 94 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 85f0fd53..4b996111 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,26 @@ follow semantic versioning; release dates are ISO 8601. (`y = (1 - yRatio - heightRatio) * pageHeight`); full-page and full-height column fills are unchanged. Adds top-/bottom-/mid-band regression tests. +- **`GraphCompose.document().pageBackgrounds(emptyList())` now actually + clears.** The builder's Javadoc promised that an explicit empty list + overrides any earlier `pageBackground(color)` on the same builder, but + the implementation skipped empty lists, so `pageBackground(LIGHT_GRAY)` + followed by `pageBackgrounds(List.of())` still emitted the grey + background. The guard is removed; the empty list is now the documented + clear. Adds a regression test. +- **`distributeRowSlotWidths` weights / children mismatch.** When a row + was constructed with a `weights` list whose size did not match the + number of children (only reachable by bypassing `RowBuilder` and + building a `RowNode` directly), the engine's row distribution code + walked off the end of the `weights` list with a raw + `IndexOutOfBoundsException`. Both row-distribution call sites + (`LayoutCompiler#distributeRowSlotWidths`, `NodeDefinitionSupport#measureRow`) + now reject the mismatch with an `IllegalArgumentException` whose + message names both sizes and the expected fix. `RowNode`'s canonical + constructor already validated this at construction time; the new + engine guards are defence-in-depth for any path that bypasses it + (e.g. reflection-based deserialization). Adds regression tests for + the canonical-constructor IAE and the `RowBuilder.build()` ISE. ### Build diff --git a/src/main/java/com/demcha/compose/GraphCompose.java b/src/main/java/com/demcha/compose/GraphCompose.java index eaa03d13..6bbfcdfb 100644 --- a/src/main/java/com/demcha/compose/GraphCompose.java +++ b/src/main/java/com/demcha/compose/GraphCompose.java @@ -384,9 +384,7 @@ public DocumentSession create() { // Explicit pageBackgrounds() call wins — even an empty // list is an intentional clear that should override any // earlier pageBackground(color) on the same builder. - if (!pageBackgrounds.isEmpty()) { - session.pageBackgrounds(pageBackgrounds); - } + 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 8b92cdd3..cdb857cf 100644 --- a/src/main/java/com/demcha/compose/document/layout/LayoutCompiler.java +++ b/src/main/java/com/demcha/compose/document/layout/LayoutCompiler.java @@ -746,6 +746,11 @@ 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."); + } 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 269f11db..96434d11 100644 --- a/src/main/java/com/demcha/compose/document/layout/NodeDefinitionSupport.java +++ b/src/main/java/com/demcha/compose/document/layout/NodeDefinitionSupport.java @@ -324,6 +324,11 @@ 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."); + } double total = 0.0; for (Double w : node.weights()) { total += w; diff --git a/src/test/java/com/demcha/compose/document/api/PageBackgroundTest.java b/src/test/java/com/demcha/compose/document/api/PageBackgroundTest.java index a7f7fd17..8c73a11e 100644 --- a/src/test/java/com/demcha/compose/document/api/PageBackgroundTest.java +++ b/src/test/java/com/demcha/compose/document/api/PageBackgroundTest.java @@ -386,6 +386,29 @@ void topBandAndBottomBandHelpersRenderAtCorrectEdges() { } } + @Test + void pageBackgroundsWithEmptyListClearsEarlierPageBackgroundColor() { + // Calling pageBackgrounds(emptyList()) on the builder must override + // an earlier pageBackground(color) on the same builder. The empty + // list is an intentional clear, not a "leave the earlier value + // unchanged" signal — the Javadoc on the builder promises that. + try (DocumentSession session = GraphCompose.document() + .pageSize(400, 300) + .margin(DocumentInsets.of(20)) + .pageBackground(DocumentColor.of(Color.LIGHT_GRAY)) + .pageBackgrounds(List.of()) + .create()) { + + session.add(new SpacerNode("Block", 200, 80, + DocumentInsets.zero(), DocumentInsets.zero())); + LayoutGraph graph = session.layoutGraph(); + + assertThat(graph.fragments()).noneMatch(this::isPageBackgroundFragment); + } catch (Exception e) { + throw new RuntimeException(e); + } + } + private boolean isPageBackgroundFragment(PlacedFragment fragment) { return fragment.payload() instanceof ShapeFragmentPayload payload && payload.fillColor() != null diff --git a/src/test/java/com/demcha/compose/document/dsl/RowBuilderTest.java b/src/test/java/com/demcha/compose/document/dsl/RowBuilderTest.java index e2cec3c7..08babe96 100644 --- a/src/test/java/com/demcha/compose/document/dsl/RowBuilderTest.java +++ b/src/test/java/com/demcha/compose/document/dsl/RowBuilderTest.java @@ -382,6 +382,46 @@ void spacingAndDeprecatedGapProduceTheSameRowNode() { assertThat(viaSpacing.gap()).isEqualTo(viaGap.gap()); } + @Test + void rowBuilderWeightsMismatchAtBuildFailsWithDescriptiveMessage() { + // Two weights paired with three children must be rejected at the + // build() call rather than turning into an IOOBE downstream. + assertThatThrownBy(() -> new RowBuilder() + .name("Row") + .weights(1.0, 2.0) + .addParagraph(p -> p.name("A").text("A").textStyle(DocumentTextStyle.DEFAULT)) + .addParagraph(p -> p.name("B").text("B").textStyle(DocumentTextStyle.DEFAULT)) + .addParagraph(p -> p.name("C").text("C").textStyle(DocumentTextStyle.DEFAULT)) + .build()) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("weights") + .hasMessageContaining("children"); + } + + @Test + void rowNodeConstructorRejectsWeightsThatDoNotMatchChildren() { + // Bypassing RowBuilder.validate() by constructing RowNode directly + // must still surface a domain-level IllegalArgumentException — no + // raw IndexOutOfBoundsException from the engine's distribution code. + SpacerNode a = new SpacerNode("A", 10, 10, DocumentInsets.zero(), DocumentInsets.zero()); + SpacerNode b = new SpacerNode("B", 10, 10, DocumentInsets.zero(), DocumentInsets.zero()); + + assertThatThrownBy(() -> new RowNode( + "Row", + List.of(a, b), + List.of(1.0, 2.0, 3.0), + 0.0, + DocumentInsets.zero(), + DocumentInsets.zero(), + null, + null, + null, + null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("weights size") + .hasMessageContaining("children size"); + } + private static PlacedNode findNodeBySemanticName(List nodes, String name) { for (PlacedNode node : nodes) { if (name.equals(node.semanticName())) {