Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 7 additions & 3 deletions src/main/java/com/demcha/compose/GraphCompose.java
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,11 @@ public DocumentBuilder pageBackground(java.awt.Color color) {
* {@link com.demcha.compose.document.api.DocumentSession#pageBackgrounds}
* for the full semantics.
*
* <p>Calling this method with an empty list is an <b>explicit clear</b>:
* 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.</p>
*
* @param fills ordered fills, or {@code null}/empty to clear
* @return this builder
*/
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -746,11 +746,7 @@ private static double[] distributeRowSlotWidths(List<DocumentNode> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
44 changes: 44 additions & 0 deletions src/main/java/com/demcha/compose/document/layout/RowSlots.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package com.demcha.compose.document.layout;

import java.util.List;

/**
* Shared validation helpers for row weight / children distribution code.
*
* <p>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.</p>
*
* <p>Package-private intentionally — engine surface, not public API.</p>
*
* @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<Double> 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.");
}
}
}
Original file line number Diff line number Diff line change
@@ -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}.
*
* <p>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.</p>
*/
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");
});
}
}