Skip to content

Commit f868499

Browse files
authored
fix(engine): empty pageBackgrounds clears; row weights/children mismatch is IAE (#81)
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).
1 parent 966ccda commit f868499

6 files changed

Lines changed: 94 additions & 3 deletions

File tree

CHANGELOG.md

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,26 @@ follow semantic versioning; release dates are ISO 8601.
4848
(`y = (1 - yRatio - heightRatio) * pageHeight`); full-page and
4949
full-height column fills are unchanged. Adds top-/bottom-/mid-band
5050
regression tests.
51+
- **`GraphCompose.document().pageBackgrounds(emptyList())` now actually
52+
clears.** The builder's Javadoc promised that an explicit empty list
53+
overrides any earlier `pageBackground(color)` on the same builder, but
54+
the implementation skipped empty lists, so `pageBackground(LIGHT_GRAY)`
55+
followed by `pageBackgrounds(List.of())` still emitted the grey
56+
background. The guard is removed; the empty list is now the documented
57+
clear. Adds a regression test.
58+
- **`distributeRowSlotWidths` weights / children mismatch.** When a row
59+
was constructed with a `weights` list whose size did not match the
60+
number of children (only reachable by bypassing `RowBuilder` and
61+
building a `RowNode` directly), the engine's row distribution code
62+
walked off the end of the `weights` list with a raw
63+
`IndexOutOfBoundsException`. Both row-distribution call sites
64+
(`LayoutCompiler#distributeRowSlotWidths`, `NodeDefinitionSupport#measureRow`)
65+
now reject the mismatch with an `IllegalArgumentException` whose
66+
message names both sizes and the expected fix. `RowNode`'s canonical
67+
constructor already validated this at construction time; the new
68+
engine guards are defence-in-depth for any path that bypasses it
69+
(e.g. reflection-based deserialization). Adds regression tests for
70+
the canonical-constructor IAE and the `RowBuilder.build()` ISE.
5171

5272
### Build
5373

src/main/java/com/demcha/compose/GraphCompose.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -384,9 +384,7 @@ public DocumentSession create() {
384384
// Explicit pageBackgrounds() call wins — even an empty
385385
// list is an intentional clear that should override any
386386
// earlier pageBackground(color) on the same builder.
387-
if (!pageBackgrounds.isEmpty()) {
388-
session.pageBackgrounds(pageBackgrounds);
389-
}
387+
session.pageBackgrounds(pageBackgrounds);
390388
} else if (pageBackground != null) {
391389
session.pageBackground(pageBackground);
392390
}

src/main/java/com/demcha/compose/document/layout/LayoutCompiler.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -746,6 +746,11 @@ private static double[] distributeRowSlotWidths(List<DocumentNode> children,
746746
}
747747
return slots;
748748
}
749+
if (weights.size() != n) {
750+
throw new IllegalArgumentException(
751+
"Row weights size (" + weights.size() + ") must match children size (" + n
752+
+ "). Pass exactly " + n + " weight(s) or leave weights empty for an even split.");
753+
}
749754
double total = 0.0;
750755
for (double w : weights) {
751756
total += w;

src/main/java/com/demcha/compose/document/layout/NodeDefinitionSupport.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,11 @@ public static MeasureResult measureRow(RowNode node,
324324
slotWidths[i] = share;
325325
}
326326
} else {
327+
if (node.weights().size() != n) {
328+
throw new IllegalArgumentException(
329+
"Row weights size (" + node.weights().size() + ") must match children size (" + n
330+
+ "). Pass exactly " + n + " weight(s) or leave weights empty for an even split.");
331+
}
327332
double total = 0.0;
328333
for (Double w : node.weights()) {
329334
total += w;

src/test/java/com/demcha/compose/document/api/PageBackgroundTest.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,29 @@ void topBandAndBottomBandHelpersRenderAtCorrectEdges() {
386386
}
387387
}
388388

389+
@Test
390+
void pageBackgroundsWithEmptyListClearsEarlierPageBackgroundColor() {
391+
// Calling pageBackgrounds(emptyList()) on the builder must override
392+
// an earlier pageBackground(color) on the same builder. The empty
393+
// list is an intentional clear, not a "leave the earlier value
394+
// unchanged" signal — the Javadoc on the builder promises that.
395+
try (DocumentSession session = GraphCompose.document()
396+
.pageSize(400, 300)
397+
.margin(DocumentInsets.of(20))
398+
.pageBackground(DocumentColor.of(Color.LIGHT_GRAY))
399+
.pageBackgrounds(List.of())
400+
.create()) {
401+
402+
session.add(new SpacerNode("Block", 200, 80,
403+
DocumentInsets.zero(), DocumentInsets.zero()));
404+
LayoutGraph graph = session.layoutGraph();
405+
406+
assertThat(graph.fragments()).noneMatch(this::isPageBackgroundFragment);
407+
} catch (Exception e) {
408+
throw new RuntimeException(e);
409+
}
410+
}
411+
389412
private boolean isPageBackgroundFragment(PlacedFragment fragment) {
390413
return fragment.payload() instanceof ShapeFragmentPayload payload
391414
&& payload.fillColor() != null

src/test/java/com/demcha/compose/document/dsl/RowBuilderTest.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,46 @@ void spacingAndDeprecatedGapProduceTheSameRowNode() {
382382
assertThat(viaSpacing.gap()).isEqualTo(viaGap.gap());
383383
}
384384

385+
@Test
386+
void rowBuilderWeightsMismatchAtBuildFailsWithDescriptiveMessage() {
387+
// Two weights paired with three children must be rejected at the
388+
// build() call rather than turning into an IOOBE downstream.
389+
assertThatThrownBy(() -> new RowBuilder()
390+
.name("Row")
391+
.weights(1.0, 2.0)
392+
.addParagraph(p -> p.name("A").text("A").textStyle(DocumentTextStyle.DEFAULT))
393+
.addParagraph(p -> p.name("B").text("B").textStyle(DocumentTextStyle.DEFAULT))
394+
.addParagraph(p -> p.name("C").text("C").textStyle(DocumentTextStyle.DEFAULT))
395+
.build())
396+
.isInstanceOf(IllegalStateException.class)
397+
.hasMessageContaining("weights")
398+
.hasMessageContaining("children");
399+
}
400+
401+
@Test
402+
void rowNodeConstructorRejectsWeightsThatDoNotMatchChildren() {
403+
// Bypassing RowBuilder.validate() by constructing RowNode directly
404+
// must still surface a domain-level IllegalArgumentException — no
405+
// raw IndexOutOfBoundsException from the engine's distribution code.
406+
SpacerNode a = new SpacerNode("A", 10, 10, DocumentInsets.zero(), DocumentInsets.zero());
407+
SpacerNode b = new SpacerNode("B", 10, 10, DocumentInsets.zero(), DocumentInsets.zero());
408+
409+
assertThatThrownBy(() -> new RowNode(
410+
"Row",
411+
List.of(a, b),
412+
List.of(1.0, 2.0, 3.0),
413+
0.0,
414+
DocumentInsets.zero(),
415+
DocumentInsets.zero(),
416+
null,
417+
null,
418+
null,
419+
null))
420+
.isInstanceOf(IllegalArgumentException.class)
421+
.hasMessageContaining("weights size")
422+
.hasMessageContaining("children size");
423+
}
424+
385425
private static PlacedNode findNodeBySemanticName(List<PlacedNode> nodes, String name) {
386426
for (PlacedNode node : nodes) {
387427
if (name.equals(node.semanticName())) {

0 commit comments

Comments
 (0)