Revert "FINERACT-2304: Fix auditing of failed batch request while enclosing t…"

This reverts commit 96eb744592.

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

@@ -53,8 +53,6 @@ import org.apache.fineract.batch.exception.BatchReferenceInvalidException;
 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;
@@ -96,8 +94,6 @@ public class BatchApiServiceImpl implements BatchApiService {
 
     private final RetryConfigurationAssembler retryConfigurationAssembler;
 
-    private final CommandSourceService commandSourceService;
-
     private EntityManager entityManager;
 
     /**
@@ -170,11 +166,9 @@ public class BatchApiServiceImpl implements BatchApiService {
         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;
         }
@@ -401,17 +395,4 @@ public class BatchApiServiceImpl implements BatchApiService {
     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.debug("Saving {} failed entries for batch audit with error: {}", commandSources.size(), errorMessage);
-                commandSourceService.saveFailedCommandSourcesNewTransaction(commandSources);
-            }
-        } 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,8 +34,7 @@ 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,11 +22,9 @@ import static org.apache.fineract.commands.domain.CommandProcessingResultType.UN
 
 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;
@@ -71,6 +69,12 @@ public class CommandSourceService {
         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 {
@@ -86,8 +90,8 @@ public class CommandSourceService {
     }
 
     @Transactional(propagation = Propagation.REQUIRES_NEW, isolation = Isolation.REPEATABLE_READ)
-    public void saveResultNewTransaction(@NonNull CommandSource commandSource) {
+    public CommandSource saveResultNewTransaction(@NonNull CommandSource commandSource) {
-        saveResult(commandSource);
+        return saveResult(commandSource);
     }
 
     @Transactional(propagation = Propagation.REQUIRED)
@@ -104,14 +108,6 @@ public class CommandSourceService {
         return errorHandler.handle(ErrorHandler.getMappable(t));
     }
 
-    @Transactional(propagation = Propagation.REQUIRES_NEW)
-    public void saveFailedCommandSourcesNewTransaction(final List<CommandSource> commandSources) {
-        commandSources.forEach(commandSource -> {
-            commandSource.setStatus(CommandProcessingResultType.ROLLBACK);
-            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,10 +132,6 @@ public class SynchronousCommandProcessingService implements CommandProcessingSer
                 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,11 +18,8 @@
  */
 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 {
@@ -35,8 +32,6 @@ public final class 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
      *
@@ -92,7 +87,6 @@ public final class BatchRequestContextHolder {
 
     public static void resetIsEnclosingTransaction() {
         isEnclosingTransaction.remove();
-        commandSources.get().clear();
     }
 
     /**
@@ -136,15 +130,4 @@ public final class BatchRequestContextHolder {
     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,7 +19,6 @@
 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;
@@ -42,10 +41,7 @@ import org.apache.fineract.batch.domain.BatchRequest;
 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;
@@ -78,9 +74,6 @@ class BatchApiServiceImplTest {
     @Mock
     private ErrorHandler errorHandler;
 
-    @Mock
-    private CommandSourceService commandSourceService;
-
     @Mock
     private RetryRegistry registry;
 
@@ -104,7 +97,7 @@ class BatchApiServiceImplTest {
     @BeforeEach
     void setUp() {
         batchApiService = new BatchApiServiceImpl(strategyProvider, resolutionHelper, transactionManager, errorHandler, List.of(),
-                batchPreprocessors, retryConfigurationAssembler, commandSourceService);
+                batchPreprocessors, retryConfigurationAssembler);
         batchApiService.setEntityManager(entityManager);
         request = new BatchRequest();
         request.setRequestId(1L);
@@ -234,32 +227,6 @@ class BatchApiServiceImplTest {
         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());
-    }
-
     private static final class RetryException extends RuntimeException {}
 
 }