Relism/Flash5
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(data): actually invoke TxSynchronization.beforeCommit, and document the contract

Browse Source 
beforeCommit(boolean) has been part of the TxSynchronization API from the
start and was invoked by nothing, in either manager — anyone implementing it
got silence, the same failure class as the dropped afterCommit callbacks in
the previous commit. Left out of that one because fixing it is a design
decision rather than a restored behaviour; this is that decision, written
down.

It now runs immediately before the real commit, with the transaction still
active and its session/connection still bound, which is the whole reason to
have a hook on this side of the commit: it can still write through the same
resource and land in the same atomic unit. Skipped when the transaction is
already rollback-only, since there is no commit to precede.

Throwing from it vetoes the commit: the transaction rolls back, the surviving
callbacks hear ROLLED_BACK, and the exception propagates. Without that, a hook
running before the commit would be strictly less useful than one running
after. A commit that fails on its own now takes the same path instead of
completing silently with no callback at all, and a rollback that also fails is
attached as a suppressed exception rather than replacing the one that explains
the failure.

Documented on the interface itself and in flash-ext-data-core/docs/README.md:
which hook sits on which side of the commit, what each may still touch, what
throwing does, and the per-transaction scoping rule from the previous commit.

Tests: 6 more (3 per manager) — runs inside the transaction with the resource
still bound, receives the read-only flag, skipped on a rollback-only
transaction, and vetoes the commit when it throws. 37 across the two managers
now, all green, reactor verify passes the 80% Jacoco gate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Zakaria El Orche
2026-08-12 23:33:05 +00:00
co-authored by Claude Opus 5
parent 7299490d0d
commit 0194470c1f
7 changed files with 304 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
flash-extensions/flash-ext-data-hibernate/src/main/java/dev/relism/flash/ext/data/hibernate/HibernateTxManager.java
+18
View File
@@ -127,9 +127,27 @@ public class HibernateTxManager implements TxManager {
s.session().getTransaction().rollback();
outcome = TxOutcome.ROLLED_BACK;
} else {
// Still inside the transaction, session still bound: a beforeCommit callback can
// write through it and land in this same commit. Throwing from there vetoes the
// commit — see TxSynchronization.
ResourceRegistry.fireBeforeCommit(s.isReadOnly(), s.synchronizationBaseline());
s.session().getTransaction().commit();
outcome = TxOutcome.COMMITTED;
}
} catch (RuntimeException e) {
// A vetoing beforeCommit, or a commit that failed outright: either way nothing was
// committed, so roll back and let the remaining callbacks hear ROLLED_BACK rather than
// nothing at all. A rollback failure here is swallowed deliberately — it would mask
// the exception that actually explains what went wrong, which is the one propagating.
if (s.session().getTransaction().isActive()) {
try {
s.session().getTransaction().rollback();
} catch (RuntimeException suppressed) {
e.addSuppressed(suppressed);
}
}
outcome = TxOutcome.ROLLED_BACK;
throw e;
} finally {
// Order is load-bearing, and getting it wrong is silent: cleanupIfIdle() calls
// ResourceRegistry.cleanup(), which removes the very ThreadLocal list of
flash-extensions/flash-ext-data-hibernate/src/test/java/dev/relism/flash/ext/data/hibernate/HibernateTxManagerSynchronizationTest.java
+78 -15
View File
@@ -14,9 +14,9 @@ import java.util.List;
import static org.junit.jupiter.api.Assertions.*;
/**
* Transaction synchronization semantics — the {@code afterCommit}/{@code afterRollback} callbacks
* {@code Data#afterCommit} exposes, and the contract callers build on: "my callback runs once, on
* the outermost commit, after the session is gone".
* Transaction synchronization semantics — the {@code beforeCommit}/{@code afterCommit}/
* {@code afterRollback} callbacks {@code Data#afterCommit} exposes, and the contract callers build
* on: "my callback runs once, for my transaction, on the right side of the commit".
*
* <p>None of this was covered before, and the gap was not academic: {@link #afterCommit_fires_on_commit}
* failed against the original {@code commit()}, which fired synchronizations only after
@@ -52,11 +52,15 @@ class HibernateTxManagerSynchronizationTest {
private static final class Recorder implements TxSynchronization {
final List<String> calls = new ArrayList<>();
@Override public void beforeCommit(boolean readOnly) { calls.add("beforeCommit:" + readOnly); }
@Override public void afterCommit() { calls.add("afterCommit"); }
@Override public void afterRollback() { calls.add("afterRollback"); }
@Override public void afterCompletion(TxOutcome outcome) { calls.add("afterCompletion:" + outcome); }
}
private static final List<String> COMMITTED = List.of("beforeCommit:false", "afterCommit", "afterCompletion:COMMITTED");
private static final List<String> ROLLED_BACK = List.of("afterRollback", "afterCompletion:ROLLED_BACK");
@Test
void afterCommit_fires_on_commit() {
Recorder recorder = new Recorder();
@@ -65,7 +69,7 @@ class HibernateTxManagerSynchronizationTest {
manager.commit(tx);
assertEquals(List.of("afterCommit", "afterCompletion:COMMITTED"), recorder.calls);
assertEquals(COMMITTED, recorder.calls);
}
@Test
@@ -76,12 +80,12 @@ class HibernateTxManagerSynchronizationTest {
manager.rollback(tx);
assertEquals(List.of("afterRollback", "afterCompletion:ROLLED_BACK"), recorder.calls);
assertEquals(ROLLED_BACK, recorder.calls);
}
/** A commit() call on a tx already marked rollback-only really rolls back — the callbacks must say so. */
/** A commit() call on a tx already marked rollback-only really rolls back — and has no commit to precede. */
@Test
void commit_of_a_rollback_only_tx_fires_the_rollback_callbacks() {
void commit_of_a_rollback_only_tx_rolls_back_without_running_beforeCommit() {
Recorder recorder = new Recorder();
TxStatus tx = manager.begin(TxDefinition.DEFAULTS);
tx.markRollbackOnly();
@@ -89,13 +93,72 @@ class HibernateTxManagerSynchronizationTest {
manager.commit(tx);
assertEquals(List.of("afterRollback", "afterCompletion:ROLLED_BACK"), recorder.calls);
assertEquals(ROLLED_BACK, recorder.calls);
}
/** beforeCommit runs inside the transaction: session still bound, transaction still active. */
@Test
void beforeCommit_runs_while_the_transaction_is_still_active() {
List<Boolean> stillActive = new ArrayList<>();
List<Session> sessionSeen = new ArrayList<>();
TxStatus tx = manager.begin(TxDefinition.DEFAULTS);
Session session = tx.resource(Session.class);
ResourceRegistry.addSynchronization(new TxSynchronization() {
@Override
public void beforeCommit(boolean readOnly) {
stillActive.add(session.getTransaction().isActive());
// MANDATORY only succeeds while a transaction is bound — proof this runs inside it.
TxStatus joined = manager.begin(TxDefinition.DEFAULTS.withPropagation(TransactionPropagation.MANDATORY));
sessionSeen.add(joined.resource(Session.class));
manager.commit(joined);
}
});
manager.commit(tx);
assertEquals(List.of(true), stillActive, "the transaction must not have committed yet");
assertSame(session, sessionSeen.get(0), "the same session must still be bound, so writes land in this commit");
}
@Test
void beforeCommit_is_told_whether_the_transaction_is_read_only() {
Recorder readWrite = new Recorder();
TxStatus rw = manager.begin(TxDefinition.DEFAULTS);
ResourceRegistry.addSynchronization(readWrite);
manager.commit(rw);
Recorder readOnly = new Recorder();
TxStatus ro = manager.begin(TxDefinition.DEFAULTS.asReadOnly());
ResourceRegistry.addSynchronization(readOnly);
manager.commit(ro);
assertEquals("beforeCommit:false", readWrite.calls.get(0));
assertEquals("beforeCommit:true", readOnly.calls.get(0));
}
/** Throwing from beforeCommit is a veto: no commit, the rollback callbacks run, the exception propagates. */
@Test
void a_throwing_beforeCommit_vetoes_the_commit() {
Recorder recorder = new Recorder();
TxStatus tx = manager.begin(TxDefinition.DEFAULTS);
Session session = tx.resource(Session.class);
ResourceRegistry.addSynchronization(new TxSynchronization() {
@Override public void beforeCommit(boolean readOnly) { throw new IllegalStateException("veto"); }
});
ResourceRegistry.addSynchronization(recorder);
IllegalStateException thrown = assertThrows(IllegalStateException.class, () -> manager.commit(tx));
assertEquals("veto", thrown.getMessage());
assertFalse(session.getTransaction().isActive(), "the vetoed transaction must be rolled back, not left open");
assertEquals(ROLLED_BACK, recorder.calls, "the surviving callbacks must hear ROLLED_BACK, not silence");
}
/**
* The load-bearing ordering detail: callbacks run <em>after</em> the committed session is
* unbound, so a callback that opens its own transaction (a cache reload, an outbox drain)
* gets a fresh one instead of silently joining the transaction that just committed.
* The load-bearing ordering detail: post-completion callbacks run <em>after</em> the committed
* session is unbound, so a callback that opens its own transaction (a cache reload, an outbox
* drain) gets a fresh one instead of silently joining the transaction that just committed.
*/
@Test
void a_synchronization_may_open_its_own_transaction() {
@@ -133,7 +196,7 @@ class HibernateTxManagerSynchronizationTest {
assertEquals(List.of(), recorder.calls, "the joined commit did not commit anything yet");
manager.commit(outer);
assertEquals(List.of("afterCommit", "afterCompletion:COMMITTED"), recorder.calls);
assertEquals(COMMITTED, recorder.calls);
}
/** Each callback belongs to one transaction: a second transaction must not re-run the first's. */
@@ -169,11 +232,11 @@ class HibernateTxManagerSynchronizationTest {
ResourceRegistry.addSynchronization(innerSync);
manager.commit(inner);
assertEquals(List.of("afterCommit", "afterCompletion:COMMITTED"), innerSync.calls);
assertEquals(COMMITTED, innerSync.calls);
assertEquals(List.of(), outerSync.calls, "the outer transaction has not committed yet");
manager.commit(outer);
assertEquals(List.of("afterCommit", "afterCompletion:COMMITTED"), outerSync.calls);
assertEquals(COMMITTED, outerSync.calls);
}
/** A rolled-back inner REQUIRES_NEW must not fire the outer's callbacks either — same reason, opposite outcome. */
@@ -190,6 +253,6 @@ class HibernateTxManagerSynchronizationTest {
assertEquals(List.of(), outerSync.calls, "the outer transaction is still open");
manager.commit(outer);
assertEquals(List.of("afterCommit", "afterCompletion:COMMITTED"), outerSync.calls);
assertEquals(COMMITTED, outerSync.calls);
}
}