diff --git a/fineract-core/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java b/fineract-core/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java index b49b450b13e..fbd9b792509 100644 --- a/fineract-core/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java +++ b/fineract-core/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java @@ -53,8 +53,6 @@ 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 @@ private List callInTransaction(Consumer 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; } @@ -401,17 +395,4 @@ 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 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); - } - } } diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandProcessingResultType.java b/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandProcessingResultType.java index 13718cd0383..17f70dfe4bc 100644 --- a/fineract-core/src/main/java/org/apache/fineract/commands/domain/CommandProcessingResultType.java +++ b/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"), // - ROLLBACK(6, "commandProcessingResultType.rollback"); + ERROR(5, "commandProcessingResultType.error"); private static final Map BY_ID = Arrays.stream(values()) .collect(Collectors.toMap(CommandProcessingResultType::getValue, v -> v)); diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java b/fineract-core/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java index c7c1cf4d4c1..75840388ddc 100644 --- a/fineract-core/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java +++ b/fineract-core/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java @@ -22,11 +22,9 @@ 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 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 { @@ -86,8 +90,8 @@ private CommandSource saveInitial(CommandWrapper wrapper, JsonCommand jsonComman } @Transactional(propagation = Propagation.REQUIRES_NEW, isolation = Isolation.REPEATABLE_READ) - public void saveResultNewTransaction(@NonNull CommandSource commandSource) { - saveResult(commandSource); + public CommandSource saveResultNewTransaction(@NonNull CommandSource commandSource) { + return saveResult(commandSource); } @Transactional(propagation = Propagation.REQUIRED) @@ -104,14 +108,6 @@ public ErrorInfo generateErrorInfo(Throwable t) { return errorHandler.handle(ErrorHandler.getMappable(t)); } - @Transactional(propagation = Propagation.REQUIRES_NEW) - public void saveFailedCommandSourcesNewTransaction(final List 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)); diff --git a/fineract-core/src/main/java/org/apache/fineract/commands/service/SynchronousCommandProcessingService.java b/fineract-core/src/main/java/org/apache/fineract/commands/service/SynchronousCommandProcessingService.java index e130e7ced4a..43647f426a7 100644 --- a/fineract-core/src/main/java/org/apache/fineract/commands/service/SynchronousCommandProcessingService.java +++ b/fineract-core/src/main/java/org/apache/fineract/commands/service/SynchronousCommandProcessingService.java @@ -132,10 +132,6 @@ 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); diff --git a/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/domain/BatchRequestContextHolder.java b/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/domain/BatchRequestContextHolder.java index f7df592114f..8965cb33294 100644 --- a/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/domain/BatchRequestContextHolder.java +++ b/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 @@ private BatchRequestContextHolder() {} private static final ThreadLocal isEnclosingTransaction = new ThreadLocal<>(); - private static final ThreadLocal> commandSources = ThreadLocal.withInitial(ArrayList::new); - /** * True if the batch attributes are set * @@ -92,7 +87,6 @@ public static void setIsEnclosingTransaction(boolean isEnclosingTransaction) { public static void resetIsEnclosingTransaction() { isEnclosingTransaction.remove(); - commandSources.get().clear(); } /** @@ -136,15 +130,4 @@ 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 getCommandSources() { - return new ArrayList<>(commandSources.get()); - } - } diff --git a/fineract-core/src/test/java/org/apache/fineract/batch/service/BatchApiServiceImplTest.java b/fineract-core/src/test/java/org/apache/fineract/batch/service/BatchApiServiceImplTest.java index 4c4d289463f..573aed1a916 100644 --- a/fineract-core/src/test/java/org/apache/fineract/batch/service/BatchApiServiceImplTest.java +++ b/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.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 @@ void testHandleBatchRequestsWithEnclosingTransactionReadOnly() { Mockito.verifyNoInteractions(entityManager); } - @Test - void testFailedCommandSourceEntriesAreSavedOnBatchFailure() { - final List 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 {} }