FINERACT-2304 Fix auditing of failed batch request while enclosing transaction got enabled

Author: oleksii-novikov-onix

fineract-core/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java

@@ -53,6 +53,8 @@
 import org.apache.fineract.batch.exception.ErrorInfo;
 import org.apache.fineract.batch.service.ResolutionHelper.BatchRequestNode;
 import org.apache.fineract.commands.configuration.RetryConfigurationAssembler;
+import org.apache.fineract.commands.domain.CommandSource;
+import org.apache.fineract.commands.service.CommandSourceService;
 import org.apache.fineract.infrastructure.core.domain.BatchRequestContextHolder;
 import org.apache.fineract.infrastructure.core.exception.ErrorHandler;
 import org.apache.fineract.infrastructure.core.filters.BatchCallHandler;
@@ -94,6 +96,8 @@ public class BatchApiServiceImpl implements BatchApiService {
 
     private final RetryConfigurationAssembler retryConfigurationAssembler;
 
+    private final CommandSourceService commandSourceService;
+
     private EntityManager entityManager;
 
     /**
@@ -166,9 +170,11 @@ private List<BatchResponse> callInTransaction(Consumer<TransactionTemplate> tran
         try {
             return retryingBatch.get();
         } catch (TransactionException | NonTransientDataAccessException ex) {
+            saveFailedCommandSourceEntries(ex);
             return buildErrorResponses(ex, responseList);
         } catch (BatchExecutionException ex) {
             log.error("Exception during the batch request processing", ex);
+            saveFailedCommandSourceEntries(ex.getCause());
             responseList.add(buildErrorResponse(ex.getCause(), ex.getRequest()));
             return responseList;
         }
@@ -395,4 +401,17 @@ private BatchResponse buildErrorResponse(Long requestId, Integer statusCode, Str
     public void setEntityManager(EntityManager entityManager) {
         this.entityManager = entityManager;
     }
+
+    private void saveFailedCommandSourceEntries(final Throwable ex) {
+        try {
+            final List<CommandSource> commandSources = BatchRequestContextHolder.getCommandSources();
+            if (!commandSources.isEmpty()) {
+                final String errorMessage = ex != null ? ex.getMessage() : "Batch processing failed";
+                log.info("Saving {} failed entries for batch audit with error: {}", commandSources.size(), errorMessage);
+                commandSourceService.saveFailedCommandSourcesNewTransaction(commandSources, errorMessage);
+            }
+        } catch (Exception e) {
+            log.error("Failed to save failed CommandSource entries for batch audit", e);
+        }
+    }
 }

fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandProcessingResultType.java

@@ -34,7 +34,8 @@ public enum CommandProcessingResultType {
     AWAITING_APPROVAL(2, "commandProcessingResultType.awaiting.approval"), //
     REJECTED(3, "commandProcessingResultType.rejected"), //
     UNDER_PROCESSING(4, "commandProcessingResultType.underProcessing"), //
-    ERROR(5, "commandProcessingResultType.error");
+    ERROR(5, "commandProcessingResultType.error"), //
+    ROLLBACK(6, "commandProcessingResultType.rollback");
 
     private static final Map<Integer, CommandProcessingResultType> BY_ID = Arrays.stream(values())
             .collect(Collectors.toMap(CommandProcessingResultType::getValue, v -> v));

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

@@ -22,9 +22,11 @@
 
 import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
+import java.util.List;
 import java.util.Set;
 import lombok.RequiredArgsConstructor;
 import org.apache.fineract.batch.exception.ErrorInfo;
+import org.apache.fineract.commands.domain.CommandProcessingResultType;
 import org.apache.fineract.commands.domain.CommandSource;
 import org.apache.fineract.commands.domain.CommandSourceRepository;
 import org.apache.fineract.commands.domain.CommandWrapper;
@@ -69,12 +71,6 @@ public CommandSource saveInitialNewTransaction(CommandWrapper wrapper, JsonComma
         return saveInitial(wrapper, jsonCommand, maker, idempotencyKey);
     }
 
-    @NonNull
-    @Transactional(propagation = Propagation.REQUIRED)
-    public CommandSource saveInitialSameTransaction(CommandWrapper wrapper, JsonCommand jsonCommand, AppUser maker, String idempotencyKey) {
-        return saveInitial(wrapper, jsonCommand, maker, idempotencyKey);
-    }
-
     @NonNull
     private CommandSource saveInitial(CommandWrapper wrapper, JsonCommand jsonCommand, AppUser maker, String idempotencyKey) {
         try {
@@ -90,8 +86,8 @@ private CommandSource saveInitial(CommandWrapper wrapper, JsonCommand jsonComman
     }
 
     @Transactional(propagation = Propagation.REQUIRES_NEW, isolation = Isolation.REPEATABLE_READ)
-    public CommandSource saveResultNewTransaction(@NonNull CommandSource commandSource) {
-        return saveResult(commandSource);
+    public void saveResultNewTransaction(@NonNull CommandSource commandSource) {
+        saveResult(commandSource);
     }
 
     @Transactional(propagation = Propagation.REQUIRED)
@@ -108,6 +104,16 @@ public ErrorInfo generateErrorInfo(Throwable t) {
         return errorHandler.handle(ErrorHandler.getMappable(t));
     }
 
+    @Transactional(propagation = Propagation.REQUIRES_NEW)
+    public void saveFailedCommandSourcesNewTransaction(final List<CommandSource> commandSources, final String errorMessage) {
+        commandSources.forEach(commandSource -> {
+            commandSource.setStatus(CommandProcessingResultType.ROLLBACK);
+            commandSource.setResult(errorMessage);
+            commandSource.setResultStatusCode(500);
+            commandSourceRepository.saveAndFlush(commandSource);
+        });
+    }
+
     @Transactional(propagation = Propagation.REQUIRES_NEW)
     public CommandSource getCommandSource(Long commandSourceId) {
         return commandSourceRepository.findById(commandSourceId).orElseThrow(() -> new CommandNotFoundException(commandSourceId));

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

@@ -132,6 +132,10 @@ public CommandProcessingResult executeCommand(final CommandWrapper wrapper, fina
                 storeCommandIdInContext(commandSource); // Store command id as a request attribute
             }
 
+            if (isEnclosingTransaction) {
+                BatchRequestContextHolder.addCommandSource(commandSource);
+            }
+
             setIdempotencyKeyStoreFlag(true);
 
             return executeCommand(wrapper, command, isApprovedByChecker, commandSource, user, isEnclosingTransaction);

fineract-core/src/main/java/org/apache/fineract/infrastructure/core/domain/BatchRequestContextHolder.java

@@ -18,8 +18,11 @@
  */
 package org.apache.fineract.infrastructure.core.domain;
 
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
+import org.apache.fineract.commands.domain.CommandSource;
 import org.springframework.transaction.TransactionStatus;
 
 public final class BatchRequestContextHolder {
@@ -32,6 +35,8 @@ private BatchRequestContextHolder() {}
 
     private static final ThreadLocal<Boolean> isEnclosingTransaction = new ThreadLocal<>();
 
+    private static final ThreadLocal<List<CommandSource>> commandSources = ThreadLocal.withInitial(ArrayList::new);
+
     /**
      * True if the batch attributes are set
      *
@@ -87,6 +92,7 @@ public static void setIsEnclosingTransaction(boolean isEnclosingTransaction) {
 
     public static void resetIsEnclosingTransaction() {
         isEnclosingTransaction.remove();
+        commandSources.get().clear();
     }
 
     /**
@@ -130,4 +136,15 @@ public static void setEnclosingTransaction(TransactionStatus enclosingTransactio
     public static void resetTransaction() {
         batchTransaction.set(Optional.empty());
     }
+
+    public static void addCommandSource(final CommandSource commandSource) {
+        if (isEnclosingTransaction() && commandSource != null) {
+            commandSources.get().add(commandSource);
+        }
+    }
+
+    public static List<CommandSource> getCommandSources() {
+        return new ArrayList<>(commandSources.get());
+    }
+
 }

fineract-core/src/test/java/org/apache/fineract/batch/service/BatchApiServiceImplTest.java

@@ -19,6 +19,7 @@
 package org.apache.fineract.batch.service;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyString;
@@ -41,7 +42,10 @@
 import org.apache.fineract.batch.domain.BatchResponse;
 import org.apache.fineract.batch.exception.ErrorInfo;
 import org.apache.fineract.commands.configuration.RetryConfigurationAssembler;
+import org.apache.fineract.commands.domain.CommandSource;
+import org.apache.fineract.commands.service.CommandSourceService;
 import org.apache.fineract.infrastructure.core.config.FineractProperties;
+import org.apache.fineract.infrastructure.core.domain.BatchRequestContextHolder;
 import org.apache.fineract.infrastructure.core.domain.FineractRequestContextHolder;
 import org.apache.fineract.infrastructure.core.exception.ErrorHandler;
 import org.apache.fineract.infrastructure.core.filters.BatchRequestPreprocessor;
@@ -74,6 +78,9 @@ class BatchApiServiceImplTest {
     @Mock
     private ErrorHandler errorHandler;
 
+    @Mock
+    private CommandSourceService commandSourceService;
+
     @Mock
     private RetryRegistry registry;
 
@@ -97,7 +104,7 @@ class BatchApiServiceImplTest {
     @BeforeEach
     void setUp() {
         batchApiService = new BatchApiServiceImpl(strategyProvider, resolutionHelper, transactionManager, errorHandler, List.of(),
-                batchPreprocessors, retryConfigurationAssembler);
+                batchPreprocessors, retryConfigurationAssembler, commandSourceService);
         batchApiService.setEntityManager(entityManager);
         request = new BatchRequest();
         request.setRequestId(1L);
@@ -227,6 +234,32 @@ void testHandleBatchRequestsWithEnclosingTransactionReadOnly() {
         Mockito.verifyNoInteractions(entityManager);
     }
 
+    @Test
+    void testFailedCommandSourceEntriesAreSavedOnBatchFailure() {
+        final List<BatchRequest> requestList = List.of(request);
+        when(strategyProvider.getCommandStrategy(any())).thenReturn(commandStrategy);
+        when(commandStrategy.execute(any(), any())).thenThrow(new RuntimeException("Test failure"));
+
+        final ErrorInfo errorInfo = mock(ErrorInfo.class);
+        when(errorInfo.getMessage()).thenReturn("Test failure");
+        when(errorInfo.getStatusCode()).thenReturn(500);
+        when(errorHandler.handle(any())).thenReturn(errorInfo);
+
+        when(transactionManager.getTransaction(any()))
+                .thenReturn(new DefaultTransactionStatus("txn_name", null, true, true, false, false, false, null));
+
+        final CommandSource mockCommandSource = mock(CommandSource.class);
+        BatchRequestContextHolder.setIsEnclosingTransaction(true);
+        BatchRequestContextHolder.addCommandSource(mockCommandSource);
+
+        final BatchResponse result = batchApiService.handleBatchRequestsWithEnclosingTransaction(requestList, uriInfo).getFirst();
+        assertNotNull(result);
+        assertEquals(500, result.getStatusCode());
+        assertTrue(result.getBody().contains("Test failure"));
+
+        verify(commandSourceService, times(1)).saveFailedCommandSourcesNewTransaction(any(), anyString());
+    }
+
     private static final class RetryException extends RuntimeException {}
 
 }