diff --git a/fineract-core/src/main/java/org/apache/fineract/accounting/journalentry/domain/JournalEntry.java b/fineract-core/src/main/java/org/apache/fineract/accounting/journalentry/domain/JournalEntry.java index 1feaf76e8af..35d56be852c 100644 --- a/fineract-core/src/main/java/org/apache/fineract/accounting/journalentry/domain/JournalEntry.java +++ b/fineract-core/src/main/java/org/apache/fineract/accounting/journalentry/domain/JournalEntry.java @@ -107,7 +107,7 @@ protected JournalEntry() { // } - public JournalEntry(final Office office, final PaymentDetail paymentDetail, final GLAccount glAccount, final String currencyCode, + protected JournalEntry(final Office office, final PaymentDetail paymentDetail, final GLAccount glAccount, final String currencyCode, final String transactionId, final boolean manualEntry, final LocalDate transactionDate, final Integer type, final BigDecimal amount, final String description, final Integer entityType, final Long entityId, final String referenceNumber, final Long loanTransactionId, final Long savingsTransactionId, final Long clientTransactionId, final Long shareTransactionId) { 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 66eb003ee08..a7d799adab0 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 @@ -145,6 +145,8 @@ private List callInTransaction(Consumer tran } catch (RuntimeException ex) { status.setRollbackOnly(); return buildErrorResponses(ex, responseList); + } finally { + BatchRequestContextHolder.setEnclosingTransaction(Optional.empty()); } }); } catch (TransactionException | NonTransientDataAccessException ex) { @@ -329,7 +331,7 @@ private BatchResponse buildErrorResponse(Throwable ex, BatchRequest request) { String body = null; Set
headers = new HashSet<>(); if (ex != null) { - ErrorInfo errorInfo = errorHandler.handle(errorHandler.getMappable(ex)); + ErrorInfo errorInfo = errorHandler.handle(ErrorHandler.getMappable(ex)); statusCode = errorInfo.getStatusCode(); body = errorInfo.getMessage(); headers = Optional.ofNullable(errorInfo.getHeaders()).orElse(new HashSet<>()); @@ -358,7 +360,7 @@ private List buildErrorResponses(Throwable ex, @NotNull List headers = new HashSet<>(); if (ex != null) { - ErrorInfo errorInfo = errorHandler.handle(errorHandler.getMappable(ex)); + ErrorInfo errorInfo = errorHandler.handle(ErrorHandler.getMappable(ex)); statusCode = errorInfo.getStatusCode(); body = errorInfo.getMessage(); headers = Optional.ofNullable(errorInfo.getHeaders()).orElse(new HashSet<>()); 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 dd4602fcd38..193041eafd9 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 @@ -62,9 +62,6 @@ public CommandSource saveInitialSameTransaction(CommandWrapper wrapper, JsonComm @NotNull private CommandSource saveInitial(CommandWrapper wrapper, JsonCommand jsonCommand, AppUser maker, String idempotencyKey) { CommandSource initialCommandSource = getInitialCommandSource(wrapper, jsonCommand, maker, idempotencyKey); - if (initialCommandSource.getCommandJson() == null) { - initialCommandSource.setCommandJson("{}"); - } return commandSourceRepository.saveAndFlush(initialCommandSource); } @@ -84,19 +81,22 @@ private CommandSource saveResult(@NotNull CommandSource commandSource) { } public ErrorInfo generateErrorInfo(Throwable t) { - return errorHandler.handle(errorHandler.getMappable(t)); + return errorHandler.handle(ErrorHandler.getMappable(t)); } + @Transactional(propagation = Propagation.REQUIRES_NEW) public CommandSource getCommandSource(Long commandSourceId) { return commandSourceRepository.findById(commandSourceId).orElseThrow(() -> new CommandNotFoundException(commandSourceId)); } + @Transactional(propagation = Propagation.REQUIRED) public CommandSource findCommandSource(CommandWrapper wrapper, String idempotencyKey) { return commandSourceRepository.findByActionNameAndEntityNameAndIdempotencyKey(wrapper.actionName(), wrapper.entityName(), idempotencyKey); } - private CommandSource getInitialCommandSource(CommandWrapper wrapper, JsonCommand jsonCommand, AppUser maker, String idempotencyKey) { + @Transactional(propagation = Propagation.REQUIRED) + public CommandSource getInitialCommandSource(CommandWrapper wrapper, JsonCommand jsonCommand, AppUser maker, String idempotencyKey) { CommandSource commandSourceResult; if (jsonCommand.commandId() != null) { commandSourceResult = commandSourceRepository.findById(jsonCommand.commandId()) @@ -105,6 +105,9 @@ private CommandSource getInitialCommandSource(CommandWrapper wrapper, JsonComman } else { commandSourceResult = CommandSource.fullEntryFrom(wrapper, jsonCommand, maker, idempotencyKey, UNDER_PROCESSING.getValue()); } + if (commandSourceResult.getCommandJson() == null) { + commandSourceResult.setCommandJson("{}"); + } return commandSourceResult; } } 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 fa6a94f75a6..72e9c70e91b 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 @@ -106,9 +106,12 @@ public CommandProcessingResult executeCommand(final CommandWrapper wrapper, fina boolean sameTransaction = BatchRequestContextHolder.getEnclosingTransaction().isPresent(); if (commandSource == null) { AppUser user = context.authenticatedUser(wrapper); - commandSource = sameTransaction ? commandSourceService.saveInitialSameTransaction(wrapper, command, user, idempotencyKey) - : commandSourceService.saveInitialNewTransaction(wrapper, command, user, idempotencyKey); - storeCommandIdInContext(commandSource); // Store command id as a request attribute + if (sameTransaction) { + commandSource = commandSourceService.getInitialCommandSource(wrapper, command, user, idempotencyKey); + } else { + commandSource = commandSourceService.saveInitialNewTransaction(wrapper, command, user, idempotencyKey); + storeCommandIdInContext(commandSource); // Store command id as a request attribute + } } setIdempotencyKeyStoreFlag(true); @@ -116,19 +119,21 @@ public CommandProcessingResult executeCommand(final CommandWrapper wrapper, fina try { result = findCommandHandler(wrapper).processCommand(command); } catch (Throwable t) { // NOSONAR - ErrorInfo errorInfo = commandSourceService.generateErrorInfo(t); + RuntimeException mappable = ErrorHandler.getMappable(t); + ErrorInfo errorInfo = commandSourceService.generateErrorInfo(mappable); commandSource.setResultStatusCode(errorInfo.getStatusCode()); commandSource.setResult(errorInfo.getMessage()); commandSource.setStatus(ERROR); - commandSource = sameTransaction ? commandSourceService.saveResultSameTransaction(commandSource) - : commandSourceService.saveResultNewTransaction(commandSource); - publishHookErrorEvent(wrapper, command, errorInfo); - throw t; + if (!sameTransaction) { // TODO: temporary solution + commandSource = commandSourceService.saveResultNewTransaction(commandSource); + } + publishHookErrorEvent(wrapper, command, errorInfo); // TODO must be performed in a new transaction + throw mappable; } commandSource.updateForAudit(result); - commandSource.setResult(toApiJsonSerializer.serializeResult(result)); commandSource.setResultStatusCode(SC_OK); + commandSource.setResult(toApiJsonSerializer.serializeResult(result)); commandSource.setStatus(PROCESSED); boolean isRollback = !isApprovedByChecker && (result.isRollbackTransaction() @@ -139,6 +144,9 @@ public CommandProcessingResult executeCommand(final CommandWrapper wrapper, fina } commandSource = commandSourceService.saveResultSameTransaction(commandSource); + if (sameTransaction) { + storeCommandIdInContext(commandSource); // Store command id as a request attribute + } if (isRollback) { /* @@ -147,14 +155,14 @@ public CommandProcessingResult executeCommand(final CommandWrapper wrapper, fina * when checker approves the transaction */ commandSource.setTransactionId(command.getTransactionId()); - // TODO: this should be removed together with lines 133-135 + // TODO: this should be removed together with lines 147-149 commandSource.setCommandJson(command.json()); // Set back CommandSource json data throw new RollbackTransactionAsCommandIsNotApprovedByCheckerException(commandSource); } result.setRollbackTransaction(null); - publishHookEvent(wrapper.entityName(), wrapper.actionName(), command, result); - + publishHookEvent(wrapper.entityName(), wrapper.actionName(), command, result); // TODO must be performed in a + // new transaction return result; } @@ -211,7 +219,7 @@ private CommandProcessingResult fallbackExecuteCommand(Exception e) { if (e instanceof RollbackTransactionAsCommandIsNotApprovedByCheckerException ex) { return logCommand(ex.getCommandSourceResult()); } - throw errorHandler.getMappable(e); + throw ErrorHandler.getMappable(e); } private NewCommandSourceHandler findCommandHandler(final CommandWrapper wrapper) { diff --git a/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/data/ApiGlobalErrorResponse.java b/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/data/ApiGlobalErrorResponse.java index 259f8dfd42c..f744f168687 100644 --- a/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/data/ApiGlobalErrorResponse.java +++ b/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/data/ApiGlobalErrorResponse.java @@ -117,7 +117,8 @@ public static ApiGlobalErrorResponse locked(String type, String identifier) { if (identifier != null) { details += " [" + identifier + ']'; } - String msg = "The server is currently unable to handle the request due to concurrent modification" + details + ", please try again"; + String msg = "The server is currently unable to handle the request due to concurrent modification " + details + + ", please try again"; return create(SC_LOCKED, "error.msg.platform.service." + type + ".conflict", msg, msg); } diff --git a/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/exception/ErrorHandler.java b/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/exception/ErrorHandler.java index 207a5283e6b..4ce0e98a85d 100644 --- a/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/exception/ErrorHandler.java +++ b/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/exception/ErrorHandler.java @@ -26,7 +26,9 @@ import jakarta.ws.rs.core.MultivaluedMap; import jakarta.ws.rs.core.Response; import jakarta.ws.rs.ext.ExceptionMapper; +import java.sql.SQLException; import java.text.ParseException; +import java.util.Arrays; import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -35,6 +37,7 @@ import lombok.extern.slf4j.Slf4j; import net.fortuna.ical4j.validate.ValidationException; import org.apache.commons.collections4.SetUtils; +import org.apache.commons.lang3.exception.ExceptionUtils; import org.apache.fineract.batch.domain.Header; import org.apache.fineract.batch.exception.ErrorInfo; import org.apache.fineract.infrastructure.core.data.ApiParameterError; @@ -45,6 +48,8 @@ import org.springframework.context.ApplicationContext; import org.springframework.core.NestedRuntimeException; import org.springframework.dao.NonTransientDataAccessException; +import org.springframework.dao.PessimisticLockingFailureException; +import org.springframework.lang.Nullable; import org.springframework.stereotype.Component; /** @@ -60,14 +65,55 @@ @AllArgsConstructor public final class ErrorHandler { + private static final Gson JSON_HELPER = GoogleGsonSerializerHelper.createGsonBuilder(true).create(); + + private enum PessimisticLockingFailureCode { + + ROLLBACK("40"), // Transaction rollback + DEADLOCK("60"), // Oracle: deadlock + HY00("HY", "Lock wait timeout exceeded"), // MySql deadlock HY00 + ; + + private final String code; + private final String msg; + + PessimisticLockingFailureCode(String code, String msg) { + this.code = code; + this.msg = msg; + } + + PessimisticLockingFailureCode(String code) { + this(code, null); + } + + private static Throwable match(Throwable t) { + Throwable rootCause = ExceptionUtils.getRootCause(t); + return rootCause instanceof SQLException sqle && Arrays.stream(values()).anyMatch(e -> e.matches(sqle)) ? rootCause : null; + } + + private boolean matches(SQLException ex) { + return code.equals(getSqlClassCode(ex)) && (msg == null || ex.getMessage().contains(msg)); + } + + @Nullable + private static String getSqlClassCode(SQLException ex) { + String sqlState = ex.getSQLState(); + if (sqlState == null) { + SQLException nestedEx = ex.getNextException(); + if (nestedEx != null) { + sqlState = nestedEx.getSQLState(); + } + } + return sqlState != null && sqlState.length() > 2 ? sqlState.substring(0, 2) : sqlState; + } + } + @Autowired private final ApplicationContext ctx; @Autowired private final DefaultExceptionMapper defaultExceptionMapper; - private static final Gson JSON_HELPER = GoogleGsonSerializerHelper.createGsonBuilder(true).create(); - @NotNull public ExceptionMapper findMostSpecificExceptionHandler(T exception) { Class clazz = exception.getClass(); @@ -120,8 +166,13 @@ public static RuntimeException getMappable(@NotNull Throwable t, String msgCode, String msg = defaultMsg == null ? t.getMessage() : defaultMsg; String codePfx = "error.msg" + (param == null ? "" : ("." + param)); Object[] args = defaultMsgArgs == null ? new Object[] { t } : defaultMsgArgs; + + Throwable cause; + if ((cause = PessimisticLockingFailureCode.match(t)) != null) { + return new PessimisticLockingFailureException(msg, cause); // deadlock + } if (t instanceof NestedRuntimeException nre) { - Throwable cause = nre.getMostSpecificCause(); + cause = nre.getMostSpecificCause(); msg = defaultMsg == null ? cause.getMessage() : defaultMsg; if (nre instanceof NonTransientDataAccessException) { msgCode = msgCode == null ? codePfx + ".data.integrity.issue" : msgCode; @@ -150,7 +201,7 @@ public static RuntimeException getMappable(@NotNull Throwable t, String msgCode, return new RuntimeException(msg, t); } - private Set createSet(T[] array) { + private static Set createSet(T[] array) { if (array == null) { return Set.of(); } else { diff --git a/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/exceptionmapper/ConcurrencyFailureExceptionMapper.java b/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/exceptionmapper/ConcurrencyFailureExceptionMapper.java index 665a3ef3387..902fb9e92d2 100644 --- a/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/exceptionmapper/ConcurrencyFailureExceptionMapper.java +++ b/fineract-core/src/main/java/org/apache/fineract/infrastructure/core/exceptionmapper/ConcurrencyFailureExceptionMapper.java @@ -44,11 +44,14 @@ public class ConcurrencyFailureExceptionMapper implements FineractExceptionMappe @Override public Response toResponse(final ConcurrencyFailureException exception) { log.warn("Exception: {}, Message: {}", exception.getClass().getName(), exception.getMessage()); - String type = "unknown"; - String identifier = "unknown"; + String type; + String identifier; if (exception instanceof ObjectOptimisticLockingFailureException olex) { type = olex.getPersistentClassName(); identifier = olex.getIdentifier() == null ? null : String.valueOf(olex.getIdentifier()); + } else { + type = "lock"; + identifier = null; } final ApiGlobalErrorResponse dataIntegrityError = ApiGlobalErrorResponse.locked(type, identifier); return Response.status(SC_LOCKED).entity(dataIntegrityError).type(MediaType.APPLICATION_JSON).build(); diff --git a/fineract-provider/src/main/resources/application.properties b/fineract-provider/src/main/resources/application.properties index 30ce3762095..fbbe8662e7d 100644 --- a/fineract-provider/src/main/resources/application.properties +++ b/fineract-provider/src/main/resources/application.properties @@ -320,7 +320,7 @@ resilience4j.retry.instances.executeCommand.max-attempts=${FINERACT_COMMAND_PROC resilience4j.retry.instances.executeCommand.wait-duration=${FINERACT_COMMAND_PROCESSING_RETRY_WAIT_DURATION:1s} resilience4j.retry.instances.executeCommand.enable-exponential-backoff=${FINERACT_COMMAND_PROCESSING_RETRY_ENABLE_EXPONENTIAL_BACKOFF:true} resilience4j.retry.instances.executeCommand.exponential-backoff-multiplier=${FINERACT_COMMAND_PROCESSING_RETRY_EXPONENTIAL_BACKOFF_MULTIPLIER:2} -resilience4j.retry.instances.executeCommand.retryExceptions=${FINERACT_COMMAND_PROCESSING_RETRY_EXCEPTIONS:org.springframework.dao.CannotAcquireLockException,org.springframework.orm.ObjectOptimisticLockingFailureException,org.eclipse.persistence.exceptions.OptimisticLockException,org.apache.fineract.infrastructure.core.exception.IdempotentCommandProcessUnderProcessingException} +resilience4j.retry.instances.executeCommand.retryExceptions=${FINERACT_COMMAND_PROCESSING_RETRY_EXCEPTIONS:org.springframework.dao.ConcurrencyFailureException,org.eclipse.persistence.exceptions.OptimisticLockException,org.apache.fineract.infrastructure.core.exception.IdempotentCommandProcessUnderProcessingException} resilience4j.retry.instances.processJobDetailForExecution.max-attempts=${FINERACT_PROCESS_JOB_DETAIL_RETRY_MAX_ATTEMPTS:3} resilience4j.retry.instances.processJobDetailForExecution.wait-duration=${FINERACT_PROCESS_JOB_DETAIL_RETRY_WAIT_DURATION:1s} @@ -331,10 +331,10 @@ resilience4j.retry.instances.recalculateInterest.max-attempts=${FINERACT_PROCESS resilience4j.retry.instances.recalculateInterest.wait-duration=${FINERACT_PROCESS_RECALCULATE_INTEREST_RETRY_WAIT_DURATION:1s} resilience4j.retry.instances.recalculateInterest.enable-exponential-backoff=${FINERACT_PROCESS_RECALCULATE_INTEREST_RETRY_ENABLE_EXPONENTIAL_BACKOFF:true} resilience4j.retry.instances.recalculateInterest.exponential-backoff-multiplier=${FINERACT_PROCESS_RECALCULATE_INTEREST_RETRY_EXPONENTIAL_BACKOFF_MULTIPLIER:2} -resilience4j.retry.instances.recalculateInterest.retryExceptions=${FINERACT_PROCESS_RECALCULATE_INTEREST_RETRY_EXCEPTIONS:org.springframework.dao.CannotAcquireLockException,org.springframework.orm.ObjectOptimisticLockingFailureException,org.eclipse.persistence.exceptions.OptimisticLockException} +resilience4j.retry.instances.recalculateInterest.retryExceptions=${FINERACT_PROCESS_RECALCULATE_INTEREST_RETRY_EXCEPTIONS:org.springframework.dao.ConcurrencyFailureException,org.eclipse.persistence.exceptions.OptimisticLockException} resilience4j.retry.instances.postInterest.max-attempts=${FINERACT_PROCESS_POST_INTEREST_RETRY_MAX_ATTEMPTS:3} resilience4j.retry.instances.postInterest.wait-duration=${FINERACT_PROCESS_POST_INTEREST_RETRY_WAIT_DURATION:1s} resilience4j.retry.instances.postInterest.enable-exponential-backoff=${FINERACT_PROCESS_POST_INTEREST_RETRY_ENABLE_EXPONENTIAL_BACKOFF:true} resilience4j.retry.instances.postInterest.exponential-backoff-multiplier=${FINERACT_PROCESS_POST_INTEREST_RETRY_EXPONENTIAL_BACKOFF_MULTIPLIER:2} -resilience4j.retry.instances.postInterest.retryExceptions=${FINERACT_PROCESS_POST_INTEREST_RETRY_EXCEPTIONS:org.springframework.dao.CannotAcquireLockException,org.springframework.orm.ObjectOptimisticLockingFailureException,org.eclipse.persistence.exceptions.OptimisticLockException} +resilience4j.retry.instances.postInterest.retryExceptions=${FINERACT_PROCESS_POST_INTEREST_RETRY_EXCEPTIONS:org.springframework.dao.ConcurrencyFailureException,org.eclipse.persistence.exceptions.OptimisticLockException} diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/BatchApiTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/BatchApiTest.java index 9bb890740be..b040ea45ff3 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/BatchApiTest.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/BatchApiTest.java @@ -179,15 +179,13 @@ public void shouldReturnOkStatusForCreateClientCommand() { */ @Test public void shouldRollBackAllTransactionsOnFailure() { - // Create first client request final BatchRequest br1 = BatchHelper.createClientRequest(4713L, "TestExtId11"); // Create second client request final BatchRequest br2 = BatchHelper.createClientRequest(4714L, "TestExtId12"); - // Create third client request, having same externalID as second client, - // hence cause of error + // Create third client request, having same externalID as second client, hence cause of error final BatchRequest br3 = BatchHelper.createClientRequest(4715L, "TestExtId11"); final List batchRequests = new ArrayList<>(); @@ -200,14 +198,13 @@ public void shouldRollBackAllTransactionsOnFailure() { final List response = BatchHelper.postBatchRequestsWithEnclosingTransaction(this.requestSpec, this.responseSpec, jsonifiedRequest); - // Verifies that none of the client in BatchRequest is created on the - // server - BatchHelper.verifyClientCreatedOnServer(this.requestSpec, this.responseSpec, "TestExtId11"); - BatchHelper.verifyClientCreatedOnServer(this.requestSpec, this.responseSpec, "TestExtId12"); + // Verifies that none of the client in BatchRequest is created on the server + BatchHelper.verifyClientNotCreatedOnServer(this.requestSpec, this.responseSpec, "TestExtId11"); + BatchHelper.verifyClientNotCreatedOnServer(this.requestSpec, this.responseSpec, "TestExtId12"); // Asserts that all the transactions have been successfully rolled back Assertions.assertEquals(1, response.size()); - Assertions.assertEquals(SC_FORBIDDEN, response.get(0).getStatusCode(), "Verify Status code 500"); + Assertions.assertEquals(SC_FORBIDDEN, response.get(0).getStatusCode(), "Verify Status code 403"); } /** diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/SavingsAccountTransactionTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/SavingsAccountTransactionTest.java index 6523a6ebacf..b161ab00187 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/SavingsAccountTransactionTest.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/SavingsAccountTransactionTest.java @@ -27,6 +27,7 @@ import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import com.fasterxml.jackson.core.JsonProcessingException; @@ -48,6 +49,7 @@ import java.util.UUID; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.Future; import java.util.stream.Collectors; import org.apache.fineract.batch.domain.BatchRequest; import org.apache.fineract.batch.domain.BatchResponse; @@ -77,7 +79,6 @@ public class SavingsAccountTransactionTest { private static final Logger log = LoggerFactory.getLogger(SavingsAccountTransactionTest.class); public static final String ACCOUNT_TYPE_INDIVIDUAL = "INDIVIDUAL"; - public static final String DEFAULT_DATE_FORMAT = "dd MMMM yyyy"; final String startDateString = "03 June 2023"; final String depositDateString = "05 June 2023"; final String withdrawDateString = "10 June 2023"; @@ -193,16 +194,17 @@ public void testConcurrentSavingsBatchTransactions() { String transactionDate = SavingsAccountHelper.TRANSACTION_DATE; String transactionAmount = "10"; ExecutorService executor = Executors.newFixedThreadPool(30); - for (int i = 0; i < 10; i++) { + ArrayList> results = new ArrayList<>(); + for (int i = 0; i < 5; i++) { log.info("Starting concurrent transaction number {}", i); SavingsTransactionData transactionData = SavingsTransactionData.builder().transactionDate(transactionDate) .transactionAmount(transactionAmount).paymentTypeId(PAYMENT_TYPE_ID).note("note_" + i).build(); Runnable workerWithTransaction = new TransactionExecutor(batchWithTransactionHelper, savingsId, transactionData, true, datatableName, columnNames); - executor.execute(workerWithTransaction); + results.add(executor.submit(workerWithTransaction)); Runnable workerWithoutTransaction = new TransactionExecutor(batchWithoutTransactionHelper, savingsId, transactionData, false, datatableName, columnNames); - executor.execute(workerWithoutTransaction); + results.add(executor.submit(workerWithoutTransaction)); } executor.shutdown(); @@ -211,6 +213,64 @@ public void testConcurrentSavingsBatchTransactions() { } this.datatableHelper.deleteDatatable(datatableName); + try { + for (Future result : results) { + assertNull(result.get()); + } + } catch (Exception e) { + throw new RuntimeException(e); + } + log.info("\nFinished all threads"); + } + + // @Test + public void testDeadlockSavingsBatchTransactions() { + final Integer clientID = ClientHelper.createClient(requestSpec, responseSpec); + ClientHelper.verifyClientCreatedOnServer(requestSpec, responseSpec, clientID); + + final Integer savingsProductId = createSavingsProductDailyPosting(); + assertNotNull(savingsProductId); + + final Integer savingsId1 = savingsAccountHelper.applyForSavingsApplication(clientID, savingsProductId, ACCOUNT_TYPE_INDIVIDUAL); + savingsAccountHelper.approveSavings(savingsId1); + savingsAccountHelper.activateSavings(savingsId1); + + final Integer savingsId2 = savingsAccountHelper.applyForSavingsApplication(clientID, savingsProductId, ACCOUNT_TYPE_INDIVIDUAL); + savingsAccountHelper.approveSavings(savingsId2); + savingsAccountHelper.activateSavings(savingsId2); + + SavingsAccountHelper batchWithTransactionHelper = new SavingsAccountHelper(requestSpec, concurrentResponseSpec); + String transactionDate = SavingsAccountHelper.TRANSACTION_DATE; + String transactionAmount = "10"; + + ExecutorService executor = Executors.newFixedThreadPool(30); + ArrayList> results = new ArrayList<>(); + for (int i = 0; i < 5; i++) { + log.info("Starting concurrent transaction number {}", i); + SavingsTransactionData transactionData1 = SavingsTransactionData.builder().transactionDate(transactionDate) + .transactionAmount(transactionAmount).paymentTypeId(PAYMENT_TYPE_ID).note("note1_" + i).build(); + results.add(executor.submit(() -> { + runDeadlockBatch(batchWithTransactionHelper, savingsId1, savingsId2, transactionData1); + })); + SavingsTransactionData transactionData2 = SavingsTransactionData.builder().transactionDate(transactionDate) + .transactionAmount(transactionAmount).paymentTypeId(PAYMENT_TYPE_ID).note("note2_" + i).build(); + results.add(executor.submit(() -> { + runDeadlockBatch(batchWithTransactionHelper, savingsId2, savingsId1, transactionData2); + })); + } + + executor.shutdown(); + // Wait until all threads are finish + while (!executor.isTerminated()) { + + } + try { + for (Future result : results) { + assertNull(result.get()); + } + } catch (Exception e) { + throw new RuntimeException(e); + } log.info("\nFinished all threads"); } @@ -268,13 +328,13 @@ private Integer createSavingsProductDailyPosting() { public static class TransactionExecutor implements Runnable { - private SavingsAccountHelper savingsHelper; + private final SavingsAccountHelper savingsHelper; private final Integer savingsId; SavingsTransactionData transactionData; - boolean batch; - boolean enclosingTransaction; - String datatableName; - List columnNames; + private final boolean batch; + private final boolean enclosingTransaction; + private final String datatableName; + private final List columnNames; private TransactionExecutor(SavingsAccountHelper savingsHelper, Integer savingsId, SavingsTransactionData transactionData, boolean batch, boolean enclosingTransaction, String datatableName, List columnNames) { @@ -338,7 +398,8 @@ public void run() { assertTrue(SC_OK == statusCode1 || SC_LOCKED == statusCode1); Integer statusCode4 = responses.get(3).getStatusCode(); assertNotNull(statusCode4); - assertTrue(SC_OK == statusCode1 ? (SC_OK == statusCode4 || SC_LOCKED == statusCode4) : SC_FORBIDDEN == statusCode4); + assertTrue(SC_OK == statusCode1 ? (SC_OK == statusCode4 || SC_LOCKED == statusCode4) + : (SC_FORBIDDEN == statusCode4 || SC_LOCKED == statusCode4)); } } else { String json = transactionData.getJson(); @@ -364,6 +425,30 @@ private static boolean checkConcurrentResponse(String response) { } } + private void runDeadlockBatch(SavingsAccountHelper savingsHelper, Integer savingsId1, Integer savingsId2, + SavingsTransactionData transactionData) { + final BatchRequest depositRequest1 = BatchHelper.depositSavingAccount(1L, savingsId1.longValue(), transactionData); + final BatchRequest withdrawRequest1 = BatchHelper.withdrawSavingAccount(2L, savingsId1.longValue(), transactionData); + final BatchRequest depositRequest2 = BatchHelper.depositSavingAccount(3L, savingsId2.longValue(), transactionData); + final BatchRequest withdrawRequest2 = BatchHelper.withdrawSavingAccount(4L, savingsId2.longValue(), transactionData); + String json = BatchHelper.toJsonString(Arrays.asList(depositRequest1, withdrawRequest1, depositRequest2, withdrawRequest2)); + RequestSpecification requestSpec = savingsHelper.getRequestSpec(); + ResponseSpecification responseSpec = savingsHelper.getResponseSpec(); + final List responses = BatchHelper.postBatchRequestsWithEnclosingTransaction(requestSpec, responseSpec, json); + assertNotNull(responses); + Integer statusCode = responses.get(0).getStatusCode(); + assertNotNull(statusCode); + assertTrue(SC_OK == statusCode || SC_LOCKED == statusCode); + if (SC_OK == statusCode) { + assertEquals(4, responses.size()); + Integer statusCode4 = responses.get(3).getStatusCode(); + assertNotNull(statusCode4); + assertEquals(SC_OK, statusCode4); + } else { + assertEquals(1, responses.size()); + } + } + // Reset configuration fields @AfterEach public void tearDown() { diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/BatchHelper.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/BatchHelper.java index 20190f72f1f..ccdbc04fd83 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/BatchHelper.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/BatchHelper.java @@ -1009,7 +1009,7 @@ public static BatchRequest approveRescheduleLoanRequest(final Long requestId, fi * @param responseSpec * @param externalId */ - public static void verifyClientCreatedOnServer(final RequestSpecification requestSpec, final ResponseSpecification responseSpec, + public static void verifyClientNotCreatedOnServer(final RequestSpecification requestSpec, final ResponseSpecification responseSpec, final String externalId) { LOG.info("------------------------------CHECK CLIENT DETAILS------------------------------------\n"); final String CLIENT_URL = "/fineract-provider/api/v1/clients?externalId=" + externalId + "&" + Utils.TENANT_IDENTIFIER;