Ralph Hopman created FINERACT-2570:
--------------------------------------
Summary: Defer webhook/hook event publication to after transaction
commit in batch requests with enclosingTransaction=true
Key: FINERACT-2570
URL: https://issues.apache.org/jira/browse/FINERACT-2570
Project: Apache Fineract
Issue Type: Bug
Affects Versions: 1.14.0
Reporter: Ralph Hopman
Assignee: Ralph Hopman
When a batch request uses {{{}enclosingTransaction: true{}}}, Fineract fires
hook events (webhooks) _inside_ the database transaction, before the batch has
fully committed. If a later request in the batch fails, the entire batch is
rolled back, but the webhook for the earlier (now rolled-back) request has
already been dispatched.
This causes downstream systems that rely on webhooks (e.g. SMS notifications)
to act on events that never actually persisted.
h2. Root cause
In {{{}SynchronousCommandProcessingService.executeCommand(){}}}, the call to
{{publishHookEvent()}} happens synchronously at the end of each individual
command execution, while the enclosing database transaction is still open.
There is an existing TODO comment acknowledging this:
{code:java}
publishHookEvent(wrapper.entityName(), wrapper.actionName(), command, result);
// TODO must be performed in a new transaction
{code}
h2. Proposed solution
When running inside an enclosing batch transaction
({{{}BatchRequestContextHolder.isEnclosingTransaction() == true{}}}), defer the
hook event publication by registering it as a
{{TransactionSynchronization.afterCommit()}} callback instead of publishing
immediately.
* If the batch commits successfully, {{afterCommit()}} fires and all hook
events are published.
* If the batch rolls back, {{afterCommit()}} is never called. No spurious
webhooks are dispatched.
* When _not_ in an enclosing transaction (the common single-request case),
behavior is unchanged. Hooks fire immediately as before.
* Error hooks ({{{}publishHookErrorEvent{}}}) remain immediate. The command
failed, and error webhooks should still fire regardless of batch outcome.
h2. Tests
Two new unit tests should be added to
{{{}SynchronousCommandProcessingServiceTest{}}}:
* Verify that when {{BatchRequestContextHolder.isEnclosingTransaction()}} is
true and transaction synchronization is active,
{{applicationContext.publishEvent()}} is not called immediately and a
{{TransactionSynchronization}} callback is registered instead.
* Verify that existing non-batch behavior is preserved (hooks fire
immediately).
h2. Impact
* Fixes false-positive webhook notifications (SMS, email) for transactions
that are subsequently rolled back within a batch.
* No impact on non-batch (single request) command processing (behavior is
identical to today).
* No impact on error hook events (they continue to fire immediately).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
