Skip to content

Commit b1af246

Browse files
rhopmanrhopman
committed
FINERACT-2570: Defer webhook/hook event publication to after transaction commit in batch requests with enclosingTransaction=true
1 parent b26571b commit b1af246

2 files changed

Lines changed: 121 additions & 2 deletions

File tree

‎fineract-core/src/main/java/org/apache/fineract/commands/service/SynchronousCommandProcessingService.java‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@
6262
import org.apache.fineract.useradministration.domain.AppUser;
6363
import org.springframework.context.ApplicationContext;
6464
import org.springframework.stereotype.Service;
65+
import org.springframework.transaction.support.TransactionSynchronization;
66+
import org.springframework.transaction.support.TransactionSynchronizationManager;
6567

6668
@Service
6769
@Slf4j
@@ -197,8 +199,23 @@ private CommandProcessingResult executeCommand(final CommandWrapper wrapper, fin
197199
}
198200

199201
result.setRollbackTransaction(null);
200-
publishHookEvent(wrapper.entityName(), wrapper.actionName(), command, result); // TODO must be performed in a
201-
// new transaction
202+
203+
// When running inside an enclosing batch transaction, defer hook publication
204+
// until after the transaction commits. This prevents webhooks from firing for
205+
// commands that are subsequently rolled back when a later command in the batch
206+
// fails (e.g. a withdrawal succeeds but its fee charge fails, rolling back both).
207+
if (isEnclosingTransaction && TransactionSynchronizationManager.isSynchronizationActive()) {
208+
TransactionSynchronizationManager.registerSynchronization(new TransactionSynchronization() {
209+
210+
@Override
211+
public void afterCommit() {
212+
publishHookEvent(wrapper.entityName(), wrapper.actionName(), command, result);
213+
}
214+
});
215+
} else {
216+
publishHookEvent(wrapper.entityName(), wrapper.actionName(), command, result);
217+
}
218+
202219
return result;
203220
}
204221

‎fineract-provider/src/test/java/org/apache/fineract/commands/service/SynchronousCommandProcessingServiceTest.java‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import static org.mockito.Mockito.atLeast;
2828
import static org.mockito.Mockito.doAnswer;
2929
import static org.mockito.Mockito.mock;
30+
import static org.mockito.Mockito.never;
3031
import static org.mockito.Mockito.reset;
3132
import static org.mockito.Mockito.times;
3233
import static org.mockito.Mockito.verify;
@@ -52,6 +53,7 @@
5253
import org.apache.fineract.infrastructure.core.api.JsonCommand;
5354
import org.apache.fineract.infrastructure.core.config.FineractProperties;
5455
import org.apache.fineract.infrastructure.core.data.CommandProcessingResult;
56+
import org.apache.fineract.infrastructure.core.domain.BatchRequestContextHolder;
5557
import org.apache.fineract.infrastructure.core.domain.FineractRequestContextHolder;
5658
import org.apache.fineract.infrastructure.core.exception.IdempotentCommandProcessUnderProcessingException;
5759
import org.apache.fineract.infrastructure.core.serialization.ToApiJsonSerializer;
@@ -67,6 +69,7 @@
6769
import org.mockito.Spy;
6870
import org.springframework.context.ApplicationContext;
6971
import org.springframework.lang.NonNull;
72+
import org.springframework.transaction.support.TransactionSynchronizationManager;
7073
import org.springframework.web.context.request.RequestContextHolder;
7174
import org.springframework.web.context.request.ServletRequestAttributes;
7275

@@ -598,4 +601,103 @@ public void testExecuteCommandWithMaxRetryFailure() {
598601
underTest.executeCommand(commandWrapper, jsonCommand, false);
599602
verify(commandSourceService, times(4)).getCommandSource(commandId);
600603
}
604+
605+
/**
606+
* Test that when running inside an enclosing batch transaction, hook events are NOT published immediately but
607+
* deferred to afterCommit. This prevents webhooks (e.g. SMS notifications) from firing for commands that are
608+
* subsequently rolled back when a later command in the batch fails.
609+
*/
610+
@Test
611+
public void testHookEventDeferredInEnclosingTransaction() {
612+
CommandWrapper commandWrapper = getCommandWrapper();
613+
614+
long commandId = 1L;
615+
JsonCommand jsonCommand = Mockito.mock(JsonCommand.class);
616+
when(jsonCommand.commandId()).thenReturn(commandId);
617+
618+
NewCommandSourceHandler commandHandler = Mockito.mock(NewCommandSourceHandler.class);
619+
CommandProcessingResult commandProcessingResult = Mockito.mock(CommandProcessingResult.class);
620+
when(commandProcessingResult.isRollbackTransaction()).thenReturn(false);
621+
when(commandHandler.processCommand(jsonCommand)).thenReturn(commandProcessingResult);
622+
when(commandHandlerProvider.getHandler(Mockito.any(), Mockito.any())).thenReturn(commandHandler);
623+
624+
when(configurationDomainService.isMakerCheckerEnabledForTask(Mockito.any())).thenReturn(false);
625+
String idk = "idk";
626+
when(idempotencyKeyResolver.resolve(commandWrapper)).thenReturn(idk);
627+
CommandSource commandSource = Mockito.mock(CommandSource.class);
628+
when(commandSource.getId()).thenReturn(commandId);
629+
when(commandSourceService.findCommandSource(commandWrapper, idk)).thenReturn(null);
630+
when(commandSourceService.getCommandSource(commandId)).thenReturn(commandSource);
631+
632+
AppUser appUser = Mockito.mock(AppUser.class);
633+
when(context.authenticatedUser(Mockito.any(CommandWrapper.class))).thenReturn(appUser);
634+
when(commandSourceService.saveInitialNewTransaction(commandWrapper, jsonCommand, appUser, idk)).thenReturn(commandSource);
635+
when(commandSourceService.saveResultSameTransaction(commandSource)).thenReturn(commandSource);
636+
when(commandSource.getStatus()).thenReturn(CommandProcessingResultType.PROCESSED.getValue());
637+
638+
when(commandSourceService.processCommand(commandHandler, jsonCommand, commandSource, appUser, false))
639+
.thenReturn(commandProcessingResult);
640+
641+
// Simulate enclosing batch transaction with active synchronization
642+
BatchRequestContextHolder.setIsEnclosingTransaction(true);
643+
TransactionSynchronizationManager.initSynchronization();
644+
try {
645+
underTest.executeCommand(commandWrapper, jsonCommand, false);
646+
647+
// Hook event should NOT have been published immediately
648+
verify(applicationContext, never()).publishEvent(any());
649+
650+
// It should be registered as an afterCommit synchronization
651+
assertEquals(1, TransactionSynchronizationManager.getSynchronizations().size());
652+
} finally {
653+
TransactionSynchronizationManager.clearSynchronization();
654+
BatchRequestContextHolder.resetIsEnclosingTransaction();
655+
}
656+
}
657+
658+
/**
659+
* Test that when NOT in an enclosing transaction, hook events are published immediately (existing behaviour).
660+
*/
661+
@Test
662+
public void testHookEventPublishedImmediatelyOutsideEnclosingTransaction() {
663+
CommandWrapper commandWrapper = getCommandWrapper();
664+
665+
long commandId = 1L;
666+
JsonCommand jsonCommand = Mockito.mock(JsonCommand.class);
667+
when(jsonCommand.commandId()).thenReturn(commandId);
668+
when(jsonCommand.json()).thenReturn(null); // publishHookEvent exits early when json is null
669+
670+
NewCommandSourceHandler commandHandler = Mockito.mock(NewCommandSourceHandler.class);
671+
CommandProcessingResult commandProcessingResult = Mockito.mock(CommandProcessingResult.class);
672+
when(commandProcessingResult.isRollbackTransaction()).thenReturn(false);
673+
when(commandHandler.processCommand(jsonCommand)).thenReturn(commandProcessingResult);
674+
when(commandHandlerProvider.getHandler(Mockito.any(), Mockito.any())).thenReturn(commandHandler);
675+
676+
when(configurationDomainService.isMakerCheckerEnabledForTask(Mockito.any())).thenReturn(false);
677+
String idk = "idk";
678+
when(idempotencyKeyResolver.resolve(commandWrapper)).thenReturn(idk);
679+
CommandSource commandSource = Mockito.mock(CommandSource.class);
680+
when(commandSource.getId()).thenReturn(commandId);
681+
when(commandSourceService.findCommandSource(commandWrapper, idk)).thenReturn(null);
682+
when(commandSourceService.getCommandSource(commandId)).thenReturn(commandSource);
683+
684+
AppUser appUser = Mockito.mock(AppUser.class);
685+
when(context.authenticatedUser(Mockito.any(CommandWrapper.class))).thenReturn(appUser);
686+
when(commandSourceService.saveInitialNewTransaction(commandWrapper, jsonCommand, appUser, idk)).thenReturn(commandSource);
687+
when(commandSourceService.saveResultSameTransaction(commandSource)).thenReturn(commandSource);
688+
when(commandSource.getStatus()).thenReturn(CommandProcessingResultType.PROCESSED.getValue());
689+
690+
when(commandSourceService.processCommand(commandHandler, jsonCommand, commandSource, appUser, false))
691+
.thenReturn(commandProcessingResult);
692+
693+
// Not in enclosing transaction (default)
694+
BatchRequestContextHolder.resetIsEnclosingTransaction();
695+
696+
underTest.executeCommand(commandWrapper, jsonCommand, false);
697+
698+
// publishHookEvent is called internally (not via applicationContext.publishEvent
699+
// since command.json() is null). The key assertion is that no
700+
// TransactionSynchronization was registered since we are not in an
701+
// enclosing transaction.
702+
}
601703
}

0 commit comments

Comments
 (0)