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 ee79c58e1 FINERACT-1909: Retry feature for commands - deadlock
ee79c58e1 is described below
commit ee79c58e157254931f989a7fc86c90b0137c6162
Author: jmarta <[email protected]>
AuthorDate: Fri Nov 17 16:41:32 2023 +0100
FINERACT-1909: Retry feature for commands - deadlock
---
.../journalentry/domain/JournalEntry.java | 2 +-
.../batch/service/BatchApiServiceImpl.java | 6 +-
.../commands/service/CommandSourceService.java | 13 ++-
.../SynchronousCommandProcessingService.java | 34 ++++---
.../core/data/ApiGlobalErrorResponse.java | 3 +-
.../core/exception/ErrorHandler.java | 59 +++++++++++-
.../ConcurrencyFailureExceptionMapper.java | 7 +-
.../src/main/resources/application.properties | 6 +-
.../fineract/integrationtests/BatchApiTest.java | 13 +--
.../SavingsAccountTransactionTest.java | 105 +++++++++++++++++++--
.../integrationtests/common/BatchHelper.java | 2 +-
11 files changed, 200 insertions(+), 50 deletions(-)
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 1feaf76e8..35d56be85 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 @@ public class JournalEntry extends
AbstractAuditableWithUTCDateTimeCustom {
//
}
- 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 66eb003ee..a7d799ada 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 @@ public class BatchApiServiceImpl implements BatchApiService
{
} catch (RuntimeException ex) {
status.setRollbackOnly();
return buildErrorResponses(ex, responseList);
+ } finally {
+
BatchRequestContextHolder.setEnclosingTransaction(Optional.empty());
}
});
} catch (TransactionException | NonTransientDataAccessException ex) {
@@ -329,7 +331,7 @@ public class BatchApiServiceImpl implements BatchApiService
{
String body = null;
Set<Header> 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 @@ public class BatchApiServiceImpl implements BatchApiService
{
String body = null;
Set<Header> 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 dd4602fcd..193041eaf 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 class CommandSourceService {
@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 @@ public class CommandSourceService {
}
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 @@ public class CommandSourceService {
} 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 fa6a94f75..72e9c70e9 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 class SynchronousCommandProcessingService
implements CommandProcessingSer
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 class SynchronousCommandProcessingService
implements CommandProcessingSer
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 class SynchronousCommandProcessingService implements
CommandProcessingSer
}
commandSource =
commandSourceService.saveResultSameTransaction(commandSource);
+ if (sameTransaction) {
+ storeCommandIdInContext(commandSource); // Store command id as a
request attribute
+ }
if (isRollback) {
/*
@@ -147,14 +155,14 @@ public class SynchronousCommandProcessingService
implements CommandProcessingSer
* 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 @@ public class SynchronousCommandProcessingService implements
CommandProcessingSer
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 259f8dfd4..f744f1686 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 class ApiGlobalErrorResponse {
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 207a5283e..4ce0e98a8 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.validation.constraints.NotNull;
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.AllArgsConstructor;
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.beans.factory.annotation.Autowired;
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 @@ import org.springframework.stereotype.Component;
@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 <T extends RuntimeException> ExceptionMapper<T>
findMostSpecificExceptionHandler(T exception) {
Class<?> clazz = exception.getClass();
@@ -120,8 +166,13 @@ public final class ErrorHandler {
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 final class ErrorHandler {
return new RuntimeException(msg, t);
}
- private <T> Set<T> createSet(T[] array) {
+ private static <T> Set<T> 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 665a3ef33..902fb9e92 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 30ce37620..fbbe8662e 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 9bb890740..b040ea45f 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 class BatchApiTest {
*/
@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<BatchRequest> batchRequests = new ArrayList<>();
@@ -200,14 +198,13 @@ public class BatchApiTest {
final List<BatchResponse> 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 6523a6eba..b161ab001 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.anyOf;
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.Set;
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 class SavingsAccountTransactionTest {
String transactionDate = SavingsAccountHelper.TRANSACTION_DATE;
String transactionAmount = "10";
ExecutorService executor = Executors.newFixedThreadPool(30);
- for (int i = 0; i < 10; i++) {
+ ArrayList<Future<?>> 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 class SavingsAccountTransactionTest {
}
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<Future<?>> 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 @@ public class SavingsAccountTransactionTest {
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<String> columnNames;
+ private final boolean batch;
+ private final boolean enclosingTransaction;
+ private final String datatableName;
+ private final List<String> columnNames;
private TransactionExecutor(SavingsAccountHelper savingsHelper,
Integer savingsId, SavingsTransactionData transactionData,
boolean batch, boolean enclosingTransaction, String
datatableName, List<String> columnNames) {
@@ -338,7 +398,8 @@ public class SavingsAccountTransactionTest {
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 @@ public class SavingsAccountTransactionTest {
}
}
+ 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<BatchResponse> 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 20190f72f..ccdbc04fd 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 final class BatchHelper {
* @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;
