Skip to content

Commit 31cacc7

Browse files
authored
chore(api): InvalidatingNodeRegistry ensureOpen + negative test + NodeRegistry @SInCE note (J2/J3/J4) (#121)
Three small carry-over items from the v1.6.7 senior review, bundled into one PR because they share a topic (the I3 auto-invalidating registry wrapper) and a CHANGELOG section. J2 — symmetric closed-session behaviour: InvalidatingNodeRegistry.register now calls ensureOpen() before super.register(). Previously the two registration entry points (session.registry().register and session.registerNodeDefinition) disagreed on what happens after session.close(): the dedicated method threw IllegalStateException, the registry().register path silently mutated and invalidated a closed-session cache. After J2 both throw the same exception type with the same message. ensureOpen() inside the constructor's default- definitions setup is safe because the `closed` flag is field- initialised to false before the constructor body runs. J3 — negative test: DocumentSessionTest.registryRegisterOnClosedSessionThrowsIllegalStateException pins the new contract. Pairs with the existing positive cache- invalidation test from I3 so the senior-bar "positive + negative" pattern (cf PR-7.3) is complete. J4 — NodeRegistry @SInCE note: Class-level Javadoc on NodeRegistry now explicitly calls out the v1.6.7 non-final relaxation. The class became non-final in v1.6.7 (Track I3) so DocumentSession could install the auto- invalidating subclass; the change was already binary-compatible (japicmp: semver PATCH). The Javadoc just makes the rationale discoverable without reading the CHANGELOG. Test plan: - DocumentSessionTest now has 26 tests (25 prev + 1 new negative). - ./mvnw verify -pl . -P japicmp — 1058 tests, 0 failures. japicmp vs v1.6.7 baseline: semver PATCH (compatible — no public signature changes; ensureOpen() is a behavioural strengthening on an existing public method, well-known semver-PATCH territory).
1 parent 06f467b commit 31cacc7

4 files changed

Lines changed: 68 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,29 @@ follow-ups carried over from the v1.6.7 senior review (see
1515
[ROADMAP.md](ROADMAP.md) and the private taskboard). No breaking
1616
changes are planned.
1717

18+
### Fixes
19+
20+
- The two `DocumentSession` registration entry points are now
21+
**fully** interchangeable, not just cache-equivalent.
22+
`session.registry().register(...)` now calls `ensureOpen()`
23+
before mutating, matching the behaviour of
24+
`session.registerNodeDefinition(...)`. Previously
25+
`registry().register(...)` on a closed session silently mutated
26+
the registry and invalidated a closed-session cache (harmless
27+
but semantically odd). After this change both paths throw
28+
`IllegalStateException` on a closed session. (Track J2 — carry-
29+
over polish from the v1.6.7 senior review.)
30+
31+
### Internal
32+
33+
- `NodeRegistry` Javadoc updated to call out the v1.6.7 non-final
34+
relaxation explicitly (Track J4). The class became non-final
35+
in v1.6.7 (Track I3) so `DocumentSession` could install the
36+
auto-invalidating subclass; the change was already binary-
37+
compatible (japicmp classified it as `semver PATCH`). The
38+
Javadoc just makes the rationale discoverable without reading
39+
the CHANGELOG.
40+
1841
### Public API
1942

2043
- `MarkdownInline.append(...)` (the inline-markdown adapter used by

src/main/java/com/demcha/compose/document/api/DocumentSession.java

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -913,18 +913,23 @@ private interface PdfRenderingBody<R> {
913913

914914
/**
915915
* Session-owned {@link NodeRegistry} subclass that funnels every
916-
* {@link #register(NodeDefinition)} call through
917-
* {@link DocumentSession#invalidate()}. Without this wrapper, callers
918-
* that mutated the registry directly via {@code session.registry().
919-
* register(...)} would leave the layout cache pointing at a stale
920-
* compile result — the cache invalidation only happened on the
921-
* dedicated {@link DocumentSession#registerNodeDefinition(NodeDefinition)}
922-
* path. Added in v1.6.7 (Track I3) so the two registration entry points
923-
* have identical caching semantics.
916+
* {@link #register(NodeDefinition)} call through both
917+
* {@link DocumentSession#ensureOpen()} and
918+
* {@link DocumentSession#invalidate()}. The two registration entry
919+
* points — {@code session.registry().register(...)} and
920+
* {@link DocumentSession#registerNodeDefinition(NodeDefinition)} —
921+
* are now fully interchangeable: both refuse to mutate a closed
922+
* session and both invalidate the cached compile.
923+
*
924+
* <p>Added in v1.6.7 (Track I3) as cache-invalidation only; the
925+
* {@code ensureOpen()} symmetry landed in v1.6.8 (Track J2) after
926+
* the senior review noted that the two entry points still
927+
* disagreed on closed-session behaviour.</p>
924928
*/
925929
private final class InvalidatingNodeRegistry extends NodeRegistry {
926930
@Override
927931
public <E extends DocumentNode> NodeRegistry register(NodeDefinition<E> definition) {
932+
ensureOpen();
928933
super.register(definition);
929934
invalidate();
930935
return this;

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@
1919
* get the same caching semantics as
2020
* {@code session.registerNodeDefinition(...)}.</p>
2121
*
22+
* <p><strong>Non-final since v1.6.7.</strong> The class lost its
23+
* {@code final} modifier in v1.6.7 (Track I3) so {@code DocumentSession}
24+
* could install the auto-invalidating subclass described above. The
25+
* change is binary-compatible (japicmp classifies it as {@code semver
26+
* PATCH}); standalone-registry callers see no behavioural change.</p>
27+
*
2228
* @since 1.6.0
2329
*/
2430
public class NodeRegistry {

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -760,6 +760,32 @@ public List<LayoutFragment> emitFragments(PreparedNode<BadgeNode> prepared, Frag
760760
}
761761
}
762762

763+
/**
764+
* Negative-path companion to the I3 positive test above. After
765+
* closing the session, mutating the registry through
766+
* {@code session.registry().register(...)} must throw
767+
* {@link IllegalStateException} — symmetric with
768+
* {@link DocumentSession#registerNodeDefinition(NodeDefinition)},
769+
* which has always thrown on a closed session. Added in v1.6.8
770+
* (Track J2/J3) after the v1.6.7 senior review flagged that the
771+
* two entry points still disagreed on closed-session behaviour.
772+
*/
773+
@Test
774+
void registryRegisterOnClosedSessionThrowsIllegalStateException() {
775+
DocumentSession session = GraphCompose.document()
776+
.pageSize(200, 160)
777+
.margin(DocumentInsets.of(10))
778+
.create();
779+
// Close via try-with-resources is the canonical path; close
780+
// explicitly here so the assertion runs against a closed
781+
// session.
782+
session.close();
783+
784+
assertThatThrownBy(() -> session.registry()
785+
.register(new BadgeNodeDefinition()))
786+
.isInstanceOf(IllegalStateException.class);
787+
}
788+
763789
@Test
764790
@DisabledIfSystemProperty(named = "no.poi", matches = "true",
765791
disabledReason = "Exercises DocxSemanticBackend; skipped under the no-poi profile that excludes poi-ooxml from the test classpath")

0 commit comments

Comments
 (0)