rhopman commented on code in PR #5719:
URL: https://github.com/apache/fineract/pull/5719#discussion_r3026402437


##########
fineract-provider/src/test/java/org/apache/fineract/commands/service/SynchronousCommandProcessingServiceTest.java:
##########
@@ -598,4 +601,103 @@ public void testExecuteCommandWithMaxRetryFailure() {
         underTest.executeCommand(commandWrapper, jsonCommand, false);
         verify(commandSourceService, times(4)).getCommandSource(commandId);
     }
+
+    /**
+     * Test that when running inside an enclosing batch transaction, hook 
events are NOT published immediately but
+     * deferred to afterCommit. This prevents webhooks (e.g. SMS 
notifications) from firing for commands that are
+     * subsequently rolled back when a later command in the batch fails.
+     */
+    @Test
+    public void testHookEventDeferredInEnclosingTransaction() {
+        CommandWrapper commandWrapper = getCommandWrapper();
+
+        long commandId = 1L;
+        JsonCommand jsonCommand = Mockito.mock(JsonCommand.class);
+        when(jsonCommand.commandId()).thenReturn(commandId);
+
+        NewCommandSourceHandler commandHandler = 
Mockito.mock(NewCommandSourceHandler.class);
+        CommandProcessingResult commandProcessingResult = 
Mockito.mock(CommandProcessingResult.class);
+        
when(commandProcessingResult.isRollbackTransaction()).thenReturn(false);
+        
when(commandHandler.processCommand(jsonCommand)).thenReturn(commandProcessingResult);
+        when(commandHandlerProvider.getHandler(Mockito.any(), 
Mockito.any())).thenReturn(commandHandler);
+
+        
when(configurationDomainService.isMakerCheckerEnabledForTask(Mockito.any())).thenReturn(false);
+        String idk = "idk";
+        when(idempotencyKeyResolver.resolve(commandWrapper)).thenReturn(idk);
+        CommandSource commandSource = Mockito.mock(CommandSource.class);
+        when(commandSource.getId()).thenReturn(commandId);
+        when(commandSourceService.findCommandSource(commandWrapper, 
idk)).thenReturn(null);
+        
when(commandSourceService.getCommandSource(commandId)).thenReturn(commandSource);
+
+        AppUser appUser = Mockito.mock(AppUser.class);
+        
when(context.authenticatedUser(Mockito.any(CommandWrapper.class))).thenReturn(appUser);
+        when(commandSourceService.saveInitialNewTransaction(commandWrapper, 
jsonCommand, appUser, idk)).thenReturn(commandSource);
+        
when(commandSourceService.saveResultSameTransaction(commandSource)).thenReturn(commandSource);
+        
when(commandSource.getStatus()).thenReturn(CommandProcessingResultType.PROCESSED.getValue());
+
+        when(commandSourceService.processCommand(commandHandler, jsonCommand, 
commandSource, appUser, false))
+                .thenReturn(commandProcessingResult);
+
+        // Simulate enclosing batch transaction with active synchronization
+        BatchRequestContextHolder.setIsEnclosingTransaction(true);
+        TransactionSynchronizationManager.initSynchronization();
+        try {
+            underTest.executeCommand(commandWrapper, jsonCommand, false);
+
+            // Hook event should NOT have been published immediately
+            verify(applicationContext, never()).publishEvent(any());
+
+            // It should be registered as an afterCommit synchronization
+            assertEquals(1, 
TransactionSynchronizationManager.getSynchronizations().size());
+        } finally {
+            TransactionSynchronizationManager.clearSynchronization();
+            BatchRequestContextHolder.resetIsEnclosingTransaction();
+        }
+    }
+
+    /**
+     * Test that when NOT in an enclosing transaction, hook events are 
published immediately (existing behaviour).
+     */
+    @Test
+    public void testHookEventPublishedImmediatelyOutsideEnclosingTransaction() 
{
+        CommandWrapper commandWrapper = getCommandWrapper();
+
+        long commandId = 1L;
+        JsonCommand jsonCommand = Mockito.mock(JsonCommand.class);
+        when(jsonCommand.commandId()).thenReturn(commandId);
+        when(jsonCommand.json()).thenReturn(null); // publishHookEvent exits 
early when json is null

Review Comment:
   Good suggestions @DeathGun44. I added the assertion and renamed the test.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to