This is an automated email from the ASF dual-hosted git repository.
adamsaghy pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/fineract.git
The following commit(s) were added to refs/heads/develop by this push:
new 095e0818e4 Revert "FINERACT-2304: Fix auditing of failed batch request
while enclosing t…"
095e0818e4 is described below
commit 095e0818e4bf5e99a7f7697d7c53e3106a5e64fb
Author: Adam Saghy <[email protected]>
AuthorDate: Tue Jul 8 09:17:12 2025 +0200
Revert "FINERACT-2304: Fix auditing of failed batch request while enclosing
t…"
This reverts commit 96eb744592867eed0332498c4743c0c2be0e4c8a.
---
.../batch/service/BatchApiServiceImpl.java | 19 ------------
.../domain/CommandProcessingResultType.java | 3 +-
.../commands/service/CommandSourceService.java | 20 +++++--------
.../SynchronousCommandProcessingService.java | 4 ---
.../core/domain/BatchRequestContextHolder.java | 17 -----------
.../batch/service/BatchApiServiceImplTest.java | 35 +---------------------
6 files changed, 10 insertions(+), 88 deletions(-)
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 b49b450b13..fbd9b79250 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.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);
- }
- }
}
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 13718cd038..17f70dfe4b 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<Integer, CommandProcessingResultType> 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 c7c1cf4d4c..75840388dd 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 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)
{
- saveResult(commandSource);
+ public CommandSource saveResultNewTransaction(@NonNull CommandSource
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));
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 e130e7ced4..43647f426a 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 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);
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 f7df592114..8965cb3329 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 @@ 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());
- }
-
}
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 4c4d289463..573aed1a91 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.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 {}
}
