From 92838e5fff53e9618d36473c186c972482105a2d Mon Sep 17 00:00:00 2001 From: Janos Haber Date: Tue, 21 Mar 2023 23:59:40 +0100 Subject: [PATCH] FINERACT-1724 - Transaction handling fix [x] - Remove requires new transaction from NewCommandSourceHandler [x] - Rewrite batch processing [x] - Fix Idempotency transaction handling [x] - Fix Batch enclosingTransaction handling --- .../batch/service/BatchApiServiceImpl.java | 321 ++++++++++++------ .../service/BatchExecutionException.java | 37 ++ .../handler/NewCommandSourceHandler.java | 4 - .../service/CommandSourceService.java | 20 +- .../SynchronousCommandProcessingService.java | 34 +- .../domain/BatchRequestContextHolder.java | 28 ++ .../filters/BatchRequestPreprocessor.java | 27 ++ .../core/filters/IdempotencyStoreFilter.java | 12 +- .../jobs/filter/LoanCOBApiFilter.java | 52 ++- ...ountTransfersWritePlatformServiceImpl.java | 5 +- .../portfolio/loanaccount/domain/Loan.java | 2 +- .../domain/LoanAccountDomainServiceJpa.java | 13 +- .../service/GuarantorDomainServiceImpl.java | 13 +- ...heduleRequestWritePlatformServiceImpl.java | 10 +- .../LoanChargeWritePlatformServiceImpl.java | 21 +- ...WritePlatformServiceJpaRepositoryImpl.java | 49 ++- 16 files changed, 442 insertions(+), 206 deletions(-) create mode 100644 fineract-provider/src/main/java/org/apache/fineract/batch/service/BatchExecutionException.java create mode 100644 fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/filters/BatchRequestPreprocessor.java diff --git a/fineract-provider/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java index bbfb0d015..f6d5bcfd3 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java @@ -19,17 +19,21 @@ package org.apache.fineract.batch.service; import com.google.gson.Gson; +import io.github.resilience4j.core.functions.Either; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.Supplier; import java.util.stream.Collectors; import javax.persistence.EntityManager; import javax.persistence.PersistenceContext; -import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; import javax.ws.rs.core.UriInfo; import lombok.RequiredArgsConstructor; @@ -51,9 +55,14 @@ import org.apache.fineract.infrastructure.core.exception.IdempotentCommandProces import org.apache.fineract.infrastructure.core.exception.IdempotentCommandProcessUnderProcessingException; import org.apache.fineract.infrastructure.core.filters.BatchCallHandler; import org.apache.fineract.infrastructure.core.filters.BatchFilter; +import org.apache.fineract.infrastructure.core.filters.BatchRequestPreprocessor; +import org.jetbrains.annotations.NotNull; import org.springframework.dao.NonTransientDataAccessException; import org.springframework.stereotype.Service; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.TransactionDefinition; import org.springframework.transaction.TransactionException; +import org.springframework.transaction.TransactionExecution; import org.springframework.transaction.support.TransactionTemplate; /** @@ -72,10 +81,12 @@ public class BatchApiServiceImpl implements BatchApiService { private final CommandStrategyProvider strategyProvider; private final ResolutionHelper resolutionHelper; - private final TransactionTemplate transactionTemplate; + private final PlatformTransactionManager transactionManager; private final List batchFilters; + private final List batchPreprocessors; + @PersistenceContext private final EntityManager entityManager; @@ -87,8 +98,8 @@ public class BatchApiServiceImpl implements BatchApiService { * @param uriInfo * @return {@code List} */ - private List handleBatchRequests(final List requestList, final UriInfo uriInfo, - boolean isEnclosingTransaction) { + private List handleBatchRequests(boolean enclosingTransaction, final List requestList, + final UriInfo uriInfo) { final List responseList = new ArrayList<>(requestList.size()); @@ -103,52 +114,150 @@ public class BatchApiServiceImpl implements BatchApiService { } for (BatchRequestNode rootNode : batchRequestNodes) { - final BatchRequest rootRequest = rootNode.getRequest(); - final CommandStrategy commandStrategy = this.strategyProvider - .getCommandStrategy(CommandContext.resource(rootRequest.getRelativeUrl()).method(rootRequest.getMethod()).build()); - log.debug("Batch request: method [{}], relative url [{}]", rootRequest.getMethod(), rootRequest.getRelativeUrl()); - final BatchResponse rootResponse = safelyExecuteStrategy(commandStrategy, rootRequest, uriInfo, isEnclosingTransaction); - log.debug("Batch response: status code [{}], method [{}], relative url [{}]", rootResponse.getStatusCode(), - rootRequest.getMethod(), rootRequest.getRelativeUrl()); - responseList.add(rootResponse); - responseList.addAll(this.processChildRequests(rootNode, rootResponse, uriInfo, isEnclosingTransaction)); + if (enclosingTransaction) { + this.callRequestRecursive(rootNode.getRequest(), rootNode, responseList, uriInfo); + } else { + responseList.addAll(callInTransaction( + transactionTemplate -> transactionTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW), + () -> { + List localResponseList = new ArrayList<>(); + this.callRequestRecursive(rootNode.getRequest(), rootNode, localResponseList, uriInfo); + return localResponseList; + })); + } } - Collections.sort(responseList, Comparator.comparing(BatchResponse::getRequestId)); - return responseList; - } - private BatchResponse safelyExecuteStrategy(CommandStrategy commandStrategy, BatchRequest request, UriInfo originalUriInfo, - boolean isEnclosingTransaction) { + /** + * Executes the request and call child requests recursively. + * + * @param request + * the current batch request + * @param requestNode + * the batch request holder node + * @param responseList + * the collected responses + * @return {@code BatchResponse} + */ + private void callRequestRecursive(BatchRequest request, BatchRequestNode requestNode, List responseList, + UriInfo uriInfo) { + // 1. run current node + BatchResponse response = executeRequest(request, uriInfo); + responseList.add(response); + if (response.getStatusCode() == 200) { + requestNode.getChildRequests().forEach(childNode -> { + BatchRequest resolvedChildRequest; + try { + resolvedChildRequest = this.resolutionHelper.resoluteRequest(childNode.getRequest(), response); + } catch (RuntimeException ex) { + throw new BatchExecutionException(childNode.getRequest(), ex); + } + callRequestRecursive(resolvedChildRequest, childNode, responseList, uriInfo); + + }); + } else { + responseList.addAll(parentRequestFailedRecursive(request, requestNode)); + } + // If the current request fails, then all the child requests are not executed. If we want to write out all the + // child requests, here is the place. + } + + /** + * All requests recursively are set to status 409 if the parent request fails. + * + * @param request + * the current request + * @param requestNode + * the current request node + * @return {@code BatchResponse} list of the generated batch responses + */ + private List parentRequestFailedRecursive(BatchRequest request, BatchRequestNode requestNode) { + List responseList = new ArrayList<>(); + BatchRequestContextHolder.getEnclosingTransaction().ifPresent(TransactionExecution::setRollbackOnly); + BatchResponse errorResponse = new BatchResponse(); + errorResponse.setRequestId(request.getRequestId()); + errorResponse.setStatusCode(Status.CONFLICT.getStatusCode()); + + // Some detail information about the error + final ErrorInfo conflictError = new ErrorInfo(Status.CONFLICT.getStatusCode(), 8001, + "Parent request with id " + request.getRequestId() + " was erroneous!"); + errorResponse.setBody(conflictError.getMessage()); + requestNode.getChildRequests().stream() + .flatMap(childNode -> parentRequestFailedRecursive(childNode.getRequest(), childNode).stream()).forEach(responseList::add); + return responseList; + } + + /** + * Execute the request + * + * @param request + * @param uriInfo + * @return + */ + private BatchResponse executeRequest(BatchRequest request, UriInfo uriInfo) { + final CommandStrategy commandStrategy = this.strategyProvider + .getCommandStrategy(CommandContext.resource(request.getRelativeUrl()).method(request.getMethod()).build()); + log.debug("Batch request: method [{}], relative url [{}]", request.getMethod(), request.getRelativeUrl()); + Either preprocessorResult = runPreprocessors(request); + if (preprocessorResult.isLeft()) { + throw new BatchExecutionException(request, preprocessorResult.getLeft()); + } else { + request = preprocessorResult.get(); + } try { - if (isEnclosingTransaction) { - entityManager.flush(); - } BatchRequestContextHolder.setRequestAttributes(new HashMap<>(Optional.ofNullable(request.getHeaders()) .map(list -> list.stream().collect(Collectors.toMap(Header::getName, Header::getValue))) .orElse(Collections.emptyMap()))); - - return new BatchCallHandler(this.batchFilters, commandStrategy::execute).serviceCall(request, originalUriInfo); + BatchCallHandler callHandler = new BatchCallHandler(this.batchFilters, commandStrategy::execute); + if (BatchRequestContextHolder.getEnclosingTransaction().isPresent()) { + if (BatchRequestContextHolder.getEnclosingTransaction().get().isRollbackOnly()) { + return new BatchResponse(); + } + entityManager.flush(); + } + final BatchResponse rootResponse = callHandler.serviceCall(request, uriInfo); + log.debug("Batch response: status code [{}], method [{}], relative url [{}]", rootResponse.getStatusCode(), request.getMethod(), + request.getRelativeUrl()); + return rootResponse; } catch (AbstractIdempotentCommandException idempotentException) { return handleIdempotentRequests(idempotentException, request); - } catch (RuntimeException e) { - log.warn("Exception while executing batch strategy {}", commandStrategy.getClass().getSimpleName(), e); - - ErrorInfo ex = ErrorHandler.handler(e); - - final BatchResponse response = new BatchResponse(); - response.setRequestId(request.getRequestId()); - response.setHeaders(request.getHeaders()); - response.setStatusCode(ex.getStatusCode()); - response.setBody(ex.getMessage()); - return response; + } catch (RuntimeException ex) { + throw new BatchExecutionException(request, ex); } finally { BatchRequestContextHolder.resetRequestAttributes(); } } + private Either runPreprocessors(BatchRequest request) { + return runPreprocessor(batchPreprocessors, request); + } + + private Either runPreprocessor(List remainingPreprocessor, + BatchRequest request) { + if (remainingPreprocessor.isEmpty()) { + return Either.right(request); + } else { + BatchRequestPreprocessor preprocessor = remainingPreprocessor.get(0); + Either processingResult = preprocessor.preprocess(request); + if (processingResult.isLeft()) { + return processingResult; + } else { + return runPreprocessor(remainingPreprocessor.subList(1, remainingPreprocessor.size()), processingResult.get()); + } + } + } + + /** + * Return the idempotent response when idempotent exception raised + * + * @param idempotentException + * the idempotent exception + * @param request + * the called request + * @return + */ private BatchResponse handleIdempotentRequests(AbstractIdempotentCommandException idempotentException, BatchRequest request) { BatchResponse response = new BatchResponse(); response.setRequestId(request.getRequestId()); @@ -167,98 +276,96 @@ public class BatchApiServiceImpl implements BatchApiService { return response; } - private List processChildRequests(final BatchRequestNode rootRequest, BatchResponse rootResponse, UriInfo uriInfo, - boolean isEnclosingTransaction) { - - final List childResponses = new ArrayList<>(); - if (!rootRequest.getChildRequests().isEmpty()) { - - for (BatchRequestNode childNode : rootRequest.getChildRequests()) { - - BatchRequest childRequest = childNode.getRequest(); - BatchResponse childResponse; - - try { - - if (rootResponse.getStatusCode().equals(200)) { - childRequest = this.resolutionHelper.resoluteRequest(childRequest, rootResponse); - final CommandStrategy commandStrategy = this.strategyProvider.getCommandStrategy( - CommandContext.resource(childRequest.getRelativeUrl()).method(childRequest.getMethod()).build()); - - childResponse = safelyExecuteStrategy(commandStrategy, childRequest, uriInfo, isEnclosingTransaction); - } else { - // Something went wrong with the parent request, create - // a response with status code 409 - childResponse = new BatchResponse(); - childResponse.setRequestId(childRequest.getRequestId()); - childResponse.setStatusCode(Status.CONFLICT.getStatusCode()); - - // Some detail information about the error - final ErrorInfo conflictError = new ErrorInfo(Status.CONFLICT.getStatusCode(), 8001, - "Parent request with id " + rootResponse.getRequestId() + " was erroneous!"); - childResponse.setBody(conflictError.getMessage()); - } - childResponses.addAll(this.processChildRequests(childNode, childResponse, uriInfo, isEnclosingTransaction)); - - } catch (Throwable ex) { - - childResponse = new BatchResponse(); - childResponse.setRequestId(childRequest.getRequestId()); - childResponse.setStatusCode(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode()); - childResponse.setBody(ex.getMessage()); - } - - childResponses.add(childResponse); - } - } - - return childResponses; - } - + /** + * Run each request root step in a separated transaction + * + * @param requestList + * @param uriInfo + * @return + */ @Override public List handleBatchRequestsWithoutEnclosingTransaction(final List requestList, UriInfo uriInfo) { - - return handleBatchRequests(requestList, uriInfo, false); + BatchRequestContextHolder.setEnclosingTransaction(Optional.empty()); + return handleBatchRequests(false, requestList, uriInfo); } + /** + * Run the batch request in transaction + * + * @param requestList + * @param uriInfo + * @return + */ @Override public List handleBatchRequestsWithEnclosingTransaction(final List requestList, final UriInfo uriInfo) { + return callInTransaction(() -> handleBatchRequests(true, requestList, uriInfo)); + } + + @NotNull + private List createErrorResponse(List responseList, int statusCode) { + BatchResponse errResponse = new BatchResponse(); + + for (BatchResponse res : responseList) { + if (!res.getStatusCode().equals(200)) { + errResponse.setBody("Transaction is being rolled back. First erroneous request: \n" + new Gson().toJson(res)); + errResponse.setRequestId(res.getRequestId()); + if (statusCode == -1) { + statusCode = res.getStatusCode(); + } + break; + } + } + errResponse.setStatusCode(statusCode); + return Arrays.asList(errResponse); + } + + private List callInTransaction(Supplier> request) { + return callInTransaction(Function.identity()::apply, request); + } + + /** + * Helper method to run the command in transaction + * + * @param request + * the enclosing supplier of the command + * @param transactionConfigurator + * consumer to configure the transaction behavior and isolation + * @return + */ + private List callInTransaction(Consumer transactionConfigurator, + Supplier> request) { List responseList = new ArrayList<>(); try { - return this.transactionTemplate.execute(status -> { + TransactionTemplate transactionTemplate = new TransactionTemplate(transactionManager); + transactionConfigurator.accept(transactionTemplate); + return transactionTemplate.execute(status -> { + BatchRequestContextHolder.setEnclosingTransaction(Optional.of(status)); try { - responseList.addAll(handleBatchRequests(requestList, uriInfo, true)); + responseList.addAll(request.get()); + if (status.isRollbackOnly()) { + return createErrorResponse(responseList, -1); + } return responseList; + } catch (BatchExecutionException ex) { + status.setRollbackOnly(); + BatchResponse errResponse = new BatchResponse(); + errResponse.setStatusCode(ex.getErrorInfo().getStatusCode()); + errResponse.setRequestId(ex.getRequest().getRequestId()); + errResponse.setBody(ex.getErrorInfo().getMessage()); + return Arrays.asList(errResponse); } catch (RuntimeException ex) { - + status.setRollbackOnly(); ErrorInfo e = ErrorHandler.handler(ex); BatchResponse errResponse = new BatchResponse(); errResponse.setStatusCode(e.getStatusCode()); errResponse.setBody(e.getMessage()); - - List errResponseList = new ArrayList<>(); - errResponseList.add(errResponse); - - status.setRollbackOnly(); - return errResponseList; + return Arrays.asList(errResponse); } }); } catch (TransactionException | NonTransientDataAccessException ex) { ErrorInfo e = ErrorHandler.handler(ex); - BatchResponse errResponse = new BatchResponse(); - errResponse.setStatusCode(e.getStatusCode()); - - for (BatchResponse res : responseList) { - if (!res.getStatusCode().equals(200)) { - errResponse.setBody("Transaction is being rolled back. First erroneous request: \n" + new Gson().toJson(res)); - break; - } - } - - List errResponseList = new ArrayList<>(); - errResponseList.add(errResponse); - - return errResponseList; + return createErrorResponse(responseList, e.getStatusCode()); } } + } diff --git a/fineract-provider/src/main/java/org/apache/fineract/batch/service/BatchExecutionException.java b/fineract-provider/src/main/java/org/apache/fineract/batch/service/BatchExecutionException.java new file mode 100644 index 000000000..4b9ad606a --- /dev/null +++ b/fineract-provider/src/main/java/org/apache/fineract/batch/service/BatchExecutionException.java @@ -0,0 +1,37 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.batch.service; + +import lombok.Getter; +import org.apache.fineract.batch.domain.BatchRequest; +import org.apache.fineract.batch.exception.ErrorHandler; +import org.apache.fineract.batch.exception.ErrorInfo; + +@Getter +public class BatchExecutionException extends RuntimeException { + + private final BatchRequest request; + private final ErrorInfo errorInfo; + + public BatchExecutionException(BatchRequest request, RuntimeException ex) { + super("Error executing batch request: " + request, ex); + this.request = request; + this.errorInfo = ErrorHandler.handler(ex); + } +} diff --git a/fineract-provider/src/main/java/org/apache/fineract/commands/handler/NewCommandSourceHandler.java b/fineract-provider/src/main/java/org/apache/fineract/commands/handler/NewCommandSourceHandler.java index d07e04cd7..d2fd82ce1 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/commands/handler/NewCommandSourceHandler.java +++ b/fineract-provider/src/main/java/org/apache/fineract/commands/handler/NewCommandSourceHandler.java @@ -20,12 +20,8 @@ package org.apache.fineract.commands.handler; import org.apache.fineract.infrastructure.core.api.JsonCommand; import org.apache.fineract.infrastructure.core.data.CommandProcessingResult; -import org.springframework.transaction.annotation.Isolation; -import org.springframework.transaction.annotation.Propagation; -import org.springframework.transaction.annotation.Transactional; public interface NewCommandSourceHandler { - @Transactional(propagation = Propagation.REQUIRES_NEW, isolation = Isolation.REPEATABLE_READ) CommandProcessingResult processCommand(JsonCommand command); } diff --git a/fineract-provider/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java b/fineract-provider/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java index 882e14041..09406ce1b 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java +++ b/fineract-provider/src/main/java/org/apache/fineract/commands/service/CommandSourceService.java @@ -30,6 +30,7 @@ import org.apache.fineract.commands.domain.CommandWrapper; import org.apache.fineract.commands.exception.CommandNotFoundException; import org.apache.fineract.infrastructure.core.api.JsonCommand; import org.apache.fineract.useradministration.domain.AppUser; +import org.jetbrains.annotations.NotNull; import org.springframework.stereotype.Component; import org.springframework.transaction.annotation.Isolation; import org.springframework.transaction.annotation.Propagation; @@ -48,6 +49,15 @@ public class CommandSourceService { @Transactional(propagation = Propagation.REQUIRES_NEW, isolation = Isolation.REPEATABLE_READ) public CommandSource saveInitial(CommandWrapper wrapper, JsonCommand jsonCommand, AppUser maker, String idempotencyKey) { + return saveInitialInternal(wrapper, jsonCommand, maker, idempotencyKey); + } + + public CommandSource saveInitialNoTransaction(CommandWrapper wrapper, JsonCommand jsonCommand, AppUser maker, String idempotencyKey) { + return saveInitialInternal(wrapper, jsonCommand, maker, idempotencyKey); + } + + @NotNull + private CommandSource saveInitialInternal(CommandWrapper wrapper, JsonCommand jsonCommand, AppUser maker, String idempotencyKey) { CommandSource initialCommandSource = getInitialCommandSource(wrapper, jsonCommand, maker, idempotencyKey); if (initialCommandSource.getCommandJson() == null) { @@ -57,7 +67,6 @@ public class CommandSourceService { return commandSourceRepository.saveAndFlush(initialCommandSource); } - @Transactional(propagation = Propagation.REQUIRES_NEW, isolation = Isolation.REPEATABLE_READ) public void saveFailed(CommandSource commandSource) { commandSource.setStatus(ERROR.getValue()); commandSourceRepository.saveAndFlush(commandSource); @@ -68,6 +77,10 @@ public class CommandSourceService { return commandSourceRepository.saveAndFlush(commandSource); } + public CommandSource saveResultNoTransaction(CommandSource commandSource) { + return commandSourceRepository.saveAndFlush(commandSource); + } + public ErrorInfo generateErrorException(Throwable t) { if (t instanceof final RuntimeException e) { return ErrorHandler.handler(e); @@ -82,6 +95,11 @@ public class CommandSourceService { idempotencyKey); } + public CommandSource findCommandSourceNoTransaction(CommandWrapper wrapper, String idempotencyKey) { + return commandSourceRepository.findByActionNameAndEntityNameAndIdempotencyKey(wrapper.actionName(), wrapper.entityName(), + idempotencyKey); + } + private CommandSource getInitialCommandSource(CommandWrapper wrapper, JsonCommand jsonCommand, AppUser maker, String idempotencyKey) { CommandSource commandSourceResult; if (jsonCommand.commandId() != null) { diff --git a/fineract-provider/src/main/java/org/apache/fineract/commands/service/SynchronousCommandProcessingService.java b/fineract-provider/src/main/java/org/apache/fineract/commands/service/SynchronousCommandProcessingService.java index e95a702d1..899e3b84d 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/commands/service/SynchronousCommandProcessingService.java +++ b/fineract-provider/src/main/java/org/apache/fineract/commands/service/SynchronousCommandProcessingService.java @@ -44,6 +44,7 @@ import org.apache.fineract.infrastructure.configuration.domain.ConfigurationDoma import org.apache.fineract.infrastructure.core.api.JsonCommand; import org.apache.fineract.infrastructure.core.data.CommandProcessingResult; import org.apache.fineract.infrastructure.core.data.CommandProcessingResultBuilder; +import org.apache.fineract.infrastructure.core.domain.BatchRequestContextHolder; import org.apache.fineract.infrastructure.core.domain.FineractRequestContextHolder; import org.apache.fineract.infrastructure.core.exception.AbstractIdempotentCommandException; import org.apache.fineract.infrastructure.core.exception.IdempotentCommandProcessFailedException; @@ -94,9 +95,13 @@ public class SynchronousCommandProcessingService implements CommandProcessingSer exceptionWhenTheRequestAlreadyProcessed(wrapper, idempotencyKey); // Store idempotency key to the request attribute - - CommandSource savedCommandSource = commandSourceService.saveInitial(wrapper, command, context.authenticatedUser(wrapper), - idempotencyKey); + CommandSource savedCommandSource; + if (BatchRequestContextHolder.getEnclosingTransaction().isPresent()) { + savedCommandSource = commandSourceService.saveInitialNoTransaction(wrapper, command, context.authenticatedUser(wrapper), + idempotencyKey); + } else { + savedCommandSource = commandSourceService.saveInitial(wrapper, command, context.authenticatedUser(wrapper), idempotencyKey); + } storeCommandToIdempotentFilter(savedCommandSource); setIdempotencyKeyStoreFlag(true); @@ -104,12 +109,23 @@ public class SynchronousCommandProcessingService implements CommandProcessingSer try { result = findCommandHandler(wrapper).processCommand(command); } catch (Throwable t) { // NOSONAR - commandSourceService.saveFailed(commandSourceService.findCommandSource(wrapper, idempotencyKey)); + CommandSource source; + if (BatchRequestContextHolder.getEnclosingTransaction().isPresent()) { + source = commandSourceService.findCommandSourceNoTransaction(wrapper, idempotencyKey); + } else { + source = commandSourceService.findCommandSource(wrapper, idempotencyKey); + } + commandSourceService.saveFailed(source); publishHookErrorEvent(wrapper, command, t); throw t; } + CommandSource initialCommandSource; + if (BatchRequestContextHolder.getEnclosingTransaction().isPresent()) { + initialCommandSource = commandSourceService.findCommandSourceNoTransaction(wrapper, idempotencyKey); + } else { + initialCommandSource = commandSourceService.findCommandSource(wrapper, idempotencyKey); + } - CommandSource initialCommandSource = commandSourceService.findCommandSource(wrapper, idempotencyKey); initialCommandSource.setResult(toApiJsonSerializer.serializeResult(result)); initialCommandSource.updateResourceId(result.getResourceId()); initialCommandSource.updateForAudit(result); @@ -120,7 +136,11 @@ public class SynchronousCommandProcessingService implements CommandProcessingSer } initialCommandSource.setStatus(CommandProcessingResultType.PROCESSED.getValue()); - commandSourceService.saveResult(initialCommandSource); + if (BatchRequestContextHolder.getEnclosingTransaction().isPresent()) { + commandSourceService.saveResultNoTransaction(initialCommandSource); + } else { + commandSourceService.saveResult(initialCommandSource); + } if ((rollbackTransaction || result.isRollbackTransaction()) && !isApprovedByChecker) { /* @@ -187,7 +207,7 @@ public class SynchronousCommandProcessingService implements CommandProcessingSer if (commandSourceResult.getIdempotencyKey() == null) { commandSourceResult.setIdempotencyKey(idempotencyKeyGenerator.create()); } - commandSourceResult = commandSourceService.saveResult(commandSourceResult); + commandSourceResult = commandSourceService.saveResultNoTransaction(commandSourceResult); return new CommandProcessingResultBuilder().withCommandId(commandSourceResult.getId()) .withEntityId(commandSourceResult.getResourceId()).build(); diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/BatchRequestContextHolder.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/BatchRequestContextHolder.java index 9cdbbf36a..a807a70d9 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/BatchRequestContextHolder.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/domain/BatchRequestContextHolder.java @@ -19,7 +19,9 @@ package org.apache.fineract.infrastructure.core.domain; import java.util.Map; +import java.util.Optional; import org.springframework.core.NamedThreadLocal; +import org.springframework.transaction.TransactionStatus; public final class BatchRequestContextHolder { @@ -27,6 +29,14 @@ public final class BatchRequestContextHolder { private static final ThreadLocal> batchAttributes = new NamedThreadLocal<>("BatchAttributesForProcessing"); + private static final ThreadLocal> enclosingTransaction = new NamedThreadLocal<>("EnclosingTransaction") { + + @Override + protected Optional initialValue() { + return Optional.empty(); + } + }; + /** * True if the batch attributes are set * @@ -36,6 +46,15 @@ public final class BatchRequestContextHolder { return batchAttributes.get() != null; } + /** + * True if the batch attributes are set and the enclosing transaction is set to true + * + * @return + */ + public static Optional getEnclosingTransaction() { + return enclosingTransaction.get(); + } + /** * Set the batch attributes for the current thread. * @@ -61,4 +80,13 @@ public final class BatchRequestContextHolder { public static void resetRequestAttributes() { batchAttributes.remove(); } + + /** + * Set the enclosing transaction flag for the current thread. + * + * @param isEnclosingTransaction + */ + public static void setEnclosingTransaction(Optional enclosingTransaction) { + BatchRequestContextHolder.enclosingTransaction.set(enclosingTransaction); + } } diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/filters/BatchRequestPreprocessor.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/filters/BatchRequestPreprocessor.java new file mode 100644 index 000000000..0addc3efe --- /dev/null +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/filters/BatchRequestPreprocessor.java @@ -0,0 +1,27 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.fineract.infrastructure.core.filters; + +import io.github.resilience4j.core.functions.Either; +import org.apache.fineract.batch.domain.BatchRequest; + +public interface BatchRequestPreprocessor { + + Either preprocess(BatchRequest batchRequest); +} diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/filters/IdempotencyStoreFilter.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/filters/IdempotencyStoreFilter.java index 52d4d2f97..b2a6de590 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/filters/IdempotencyStoreFilter.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/core/filters/IdempotencyStoreFilter.java @@ -70,7 +70,7 @@ public class IdempotencyStoreFilter extends OncePerRequestFilter implements Batc boolean isSuccessWithoutStored = isStoreIdempotencyKey(request) && commandId.isPresent() && isAllowedContentTypeResponse(response) && wrapper.getValue() != null; if (isSuccessWithoutStored) { - storeCommandResult(response.getStatus(), Optional.ofNullable(wrapper.getValue()) + storeCommandResult(false, response.getStatus(), Optional.ofNullable(wrapper.getValue()) .map(ContentCachingResponseWrapper::getContentAsByteArray).map(s -> new String(s, StandardCharsets.UTF_8)).orElse(null), commandId); } @@ -79,11 +79,15 @@ public class IdempotencyStoreFilter extends OncePerRequestFilter implements Batc } } - private void storeCommandResult(int response, String body, Optional commandId) { + private void storeCommandResult(boolean batch, int response, String body, Optional commandId) { commandSourceRepository.findById(commandId.get()).ifPresent(commandSource -> { commandSource.setResultStatusCode(response); commandSource.setResult(body); - commandSourceService.saveResult(commandSource); + if (batch) { + commandSourceService.saveResultNoTransaction(commandSource); + } else { + commandSourceService.saveResult(commandSource); + } }); } @@ -133,7 +137,7 @@ public class IdempotencyStoreFilter extends OncePerRequestFilter implements Batc Optional commandId = getCommandId(null); boolean isSuccessWithoutStored = isStoreIdempotencyKey(null) && commandId.isPresent(); if (isSuccessWithoutStored) { - storeCommandResult(result.getStatusCode(), result.getBody(), commandId); + storeCommandResult(true, result.getStatusCode(), result.getBody(), commandId); } return result; } diff --git a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilter.java b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilter.java index 8c8cb4eb1..02135d9f2 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilter.java +++ b/fineract-provider/src/main/java/org/apache/fineract/infrastructure/jobs/filter/LoanCOBApiFilter.java @@ -19,6 +19,7 @@ package org.apache.fineract.infrastructure.jobs.filter; import com.google.common.collect.Lists; +import io.github.resilience4j.core.functions.Either; import java.io.IOException; import java.math.BigDecimal; import java.util.ArrayList; @@ -30,13 +31,11 @@ import javax.servlet.FilterChain; import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; -import javax.ws.rs.core.UriInfo; import lombok.Getter; import lombok.RequiredArgsConstructor; import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang3.StringUtils; import org.apache.fineract.batch.domain.BatchRequest; -import org.apache.fineract.batch.domain.BatchResponse; import org.apache.fineract.cob.data.LoanIdAndLastClosedBusinessDate; import org.apache.fineract.cob.service.InlineLoanCOBExecutorServiceImpl; import org.apache.fineract.cob.service.LoanAccountLockService; @@ -44,24 +43,27 @@ import org.apache.fineract.infrastructure.businessdate.domain.BusinessDateType; import org.apache.fineract.infrastructure.core.config.FineractProperties; import org.apache.fineract.infrastructure.core.data.ApiGlobalErrorResponse; import org.apache.fineract.infrastructure.core.domain.ExternalId; -import org.apache.fineract.infrastructure.core.filters.BatchFilter; -import org.apache.fineract.infrastructure.core.filters.BatchFilterChain; +import org.apache.fineract.infrastructure.core.filters.BatchRequestPreprocessor; import org.apache.fineract.infrastructure.core.service.ThreadLocalContextUtil; import org.apache.fineract.infrastructure.security.service.PlatformSecurityContext; import org.apache.fineract.portfolio.loanaccount.domain.GLIMAccountInfoRepository; import org.apache.fineract.portfolio.loanaccount.domain.GroupLoanIndividualMonitoringAccount; import org.apache.fineract.portfolio.loanaccount.domain.Loan; import org.apache.fineract.portfolio.loanaccount.domain.LoanRepository; +import org.apache.fineract.portfolio.loanaccount.exception.LoanNotFoundException; import org.apache.fineract.portfolio.loanaccount.rescheduleloan.domain.LoanRescheduleRequestRepository; import org.apache.fineract.useradministration.exception.UnAuthenticatedUserException; import org.apache.http.HttpStatus; import org.springframework.http.HttpMethod; import org.springframework.stereotype.Component; +import org.springframework.transaction.PlatformTransactionManager; +import org.springframework.transaction.TransactionDefinition; +import org.springframework.transaction.support.TransactionTemplate; import org.springframework.web.filter.OncePerRequestFilter; @Component @RequiredArgsConstructor -public class LoanCOBApiFilter extends OncePerRequestFilter implements BatchFilter { +public class LoanCOBApiFilter extends OncePerRequestFilter implements BatchRequestPreprocessor { private final GLIMAccountInfoRepository glimAccountInfoRepository; private final LoanAccountLockService loanAccountLockService; @@ -82,6 +84,8 @@ public class LoanCOBApiFilter extends OncePerRequestFilter implements BatchFilte || LOAN_GLIMACCOUNT_PATH_PATTERN.matcher(s).find(); private static final String JOB_NAME = "INLINE_LOAN_COB"; + private final PlatformTransactionManager transactionManager; + @RequiredArgsConstructor @Getter private static class LoanIdsHardLockedException extends RuntimeException { @@ -107,15 +111,6 @@ public class LoanCOBApiFilter extends OncePerRequestFilter implements BatchFilte response.setStatus(statusCode); response.getWriter().write(message); } - - public BatchResponse toBatchResponse(BatchRequest request) { - BatchResponse response = new BatchResponse(); - response.setStatusCode(statusCode); - response.setBody(message); - response.setHeaders(request.getHeaders()); - response.setRequestId(request.getRequestId()); - return response; - } } @Override @@ -251,28 +246,27 @@ public class LoanCOBApiFilter extends OncePerRequestFilter implements BatchFilte } @Override - public BatchResponse doFilter(BatchRequest batchRequest, UriInfo uriInfo, BatchFilterChain chain) { - if (!isOnApiList("/" + batchRequest.getRelativeUrl(), batchRequest.getMethod())) { - return chain.serviceCall(batchRequest, uriInfo); - } else { + public Either preprocess(BatchRequest batchRequest) { + TransactionTemplate tr = new TransactionTemplate(transactionManager); + tr.setPropagationBehavior(TransactionDefinition.PROPAGATION_NOT_SUPPORTED); + return tr.execute(status -> { try { - boolean bypassUser = isBypassUser(); - if (bypassUser) { - return chain.serviceCall(batchRequest, uriInfo); - } else { - try { + if (isOnApiList("/" + batchRequest.getRelativeUrl(), batchRequest.getMethod())) { + boolean bypassUser = isBypassUser(); + if (!bypassUser) { List result = calculateRelevantLoanIds("/" + batchRequest.getRelativeUrl()); if (!result.isEmpty() && (isLoanSoftLocked(result) || isLoanBehind(result))) { executeInlineCob(result); } - return chain.serviceCall(batchRequest, uriInfo); - } catch (LoanIdsHardLockedException e) { - return Reject.reject(e.getLoanIdFromRequest(), HttpStatus.SC_CONFLICT).toBatchResponse(batchRequest); } } - } catch (UnAuthenticatedUserException e) { - return Reject.reject(null, HttpStatus.SC_UNAUTHORIZED).toBatchResponse(batchRequest); + } catch (LoanNotFoundException e) { + return Either.right(batchRequest); + } catch (RuntimeException e) { + return Either.left(e); } - } + return Either.right(batchRequest); + }); } + } diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/AccountTransfersWritePlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/AccountTransfersWritePlatformServiceImpl.java index ffd011ff0..644d6526d 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/AccountTransfersWritePlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/account/service/AccountTransfersWritePlatformServiceImpl.java @@ -158,7 +158,7 @@ public class AccountTransfersWritePlatformServiceImpl implements AccountTransfer transactionDate, transactionAmount, paymentDetail, transactionBooleanValues, backdatedTxnsAllowedTill); final Long toLoanAccountId = command.longValueOfParameterNamed(toAccountIdParamName); - final Loan toLoanAccount = this.loanAccountAssembler.assembleFrom(toLoanAccountId); + Loan toLoanAccount = this.loanAccountAssembler.assembleFrom(toLoanAccountId); final Boolean isHolidayValidationDone = false; final HolidayDetailDTO holidayDetailDto = null; @@ -169,7 +169,7 @@ public class AccountTransfersWritePlatformServiceImpl implements AccountTransfer final LoanTransaction loanRepaymentTransaction = this.loanAccountDomainService.makeRepayment(LoanTransactionType.REPAYMENT, toLoanAccount, transactionDate, transactionAmount, paymentDetail, null, externalId, isRecoveryRepayment, chargeRefundChargeType, isAccountTransfer, holidayDetailDto, isHolidayValidationDone); - + toLoanAccount = loanRepaymentTransaction.getLoan(); final AccountTransferDetails accountTransferDetails = this.accountTransferAssembler.assembleSavingsToLoanTransfer(command, fromSavingsAccount, toLoanAccount, withdrawal, loanRepaymentTransaction); this.accountTransferDetailRepository.saveAndFlush(accountTransferDetails); @@ -340,6 +340,7 @@ public class AccountTransfersWritePlatformServiceImpl implements AccountTransfer accountTransferDTO.getTransactionDate(), accountTransferDTO.getTransactionAmount(), accountTransferDTO.getPaymentDetail(), null, externalId, isRecoveryRepayment, chargeRefundChargeType, isAccountTransfer, holidayDetailDto, isHolidayValidationDone); + toLoanAccount = loanTransaction.getLoan(); } accountTransferDetails = this.accountTransferAssembler.assembleSavingsToLoanTransfer(accountTransferDTO, fromSavingsAccount, diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java index 6c335697c..e56edebdf 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/Loan.java @@ -2687,7 +2687,7 @@ public class Loan extends AbstractAuditableWithUTCDateTimeCustom { for (final Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) { mapEntry.getValue().updateLoan(this); } - + this.loanTransactions.addAll(changedTransactionDetail.getNewTransactionMappings().values()); } updateLoanSummaryDerivedFields(); } diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanAccountDomainServiceJpa.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanAccountDomainServiceJpa.java index 883fc942c..6a0b36d43 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanAccountDomainServiceJpa.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/domain/LoanAccountDomainServiceJpa.java @@ -216,7 +216,8 @@ public class LoanAccountDomainServiceJpa implements LoanAccountDomainService { * before the latest payment recorded against the loan) ***/ if (changedTransactionDetail != null) { - for (Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) { + for (final Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) { + saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue()); updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); } // Trigger transaction replayed event @@ -752,8 +753,8 @@ public class LoanAccountDomainServiceJpa implements LoanAccountDomainService { } @Override - public LoanTransaction foreCloseLoan(final Loan loan, final LocalDate foreClosureDate, final String noteText, - final ExternalId externalId, Map changes) { + public LoanTransaction foreCloseLoan(Loan loan, final LocalDate foreClosureDate, final String noteText, final ExternalId externalId, + Map changes) { if (loan.isChargedOff() && foreClosureDate.isBefore(loan.getChargedOffOnDate())) { throw new GeneralPlatformDomainRuleException("error.msg.transaction.date.cannot.be.earlier.than.charge.off.date", "Loan: " + loan.getId() @@ -836,14 +837,14 @@ public class LoanAccountDomainServiceJpa implements LoanAccountDomainService { changes.put("eventAmount", payPrincipal.getAmount().negate()); if (changedTransactionDetail != null) { - for (Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) { + for (final Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) { + saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue()); updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); } // Trigger transaction replayed event replayedTransactionBusinessEventService.raiseTransactionReplayedEvents(changedTransactionDetail); } - - saveAndFlushLoanWithDataIntegrityViolationChecks(loan); + loan = saveAndFlushLoanWithDataIntegrityViolationChecks(loan); if (StringUtils.isNotBlank(noteText)) { changes.put("note", noteText); diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/guarantor/service/GuarantorDomainServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/guarantor/service/GuarantorDomainServiceImpl.java index dc988e93d..e531b8a07 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/guarantor/service/GuarantorDomainServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/guarantor/service/GuarantorDomainServiceImpl.java @@ -52,6 +52,7 @@ import org.apache.fineract.portfolio.account.domain.AccountTransferDetails; import org.apache.fineract.portfolio.account.domain.AccountTransferType; import org.apache.fineract.portfolio.account.service.AccountTransfersWritePlatformService; import org.apache.fineract.portfolio.loanaccount.domain.Loan; +import org.apache.fineract.portfolio.loanaccount.domain.LoanRepository; import org.apache.fineract.portfolio.loanaccount.domain.LoanTransaction; import org.apache.fineract.portfolio.loanaccount.guarantor.GuarantorConstants; import org.apache.fineract.portfolio.loanaccount.guarantor.domain.Guarantor; @@ -85,6 +86,7 @@ public class GuarantorDomainServiceImpl implements GuarantorDomainService { private final SavingsAccountAssembler savingsAccountAssembler; private final ConfigurationDomainService configurationDomainService; private final ExternalIdFactory externalIdFactory; + private final LoanRepository loanRepository; @PostConstruct public void addListeners() { @@ -232,28 +234,27 @@ public class GuarantorDomainServiceImpl implements GuarantorDomainService { final AccountTransferDetails accountTransferDetails = null; final String noteText = null; - final SavingsAccount toSavingsAccount = null; - Long loanId = loan.getId(); for (Guarantor guarantor : existGuarantorList) { final List fundingDetails = guarantor.getGuarantorFundDetails(); for (GuarantorFundingDetails guarantorFundingDetails : fundingDetails) { + Loan freshLoan = loanRepository.findById(loanId).orElseThrow(); if (guarantorFundingDetails.getStatus().isActive()) { final SavingsAccount fromSavingsAccount = guarantorFundingDetails.getLinkedSavingsAccount(); final Long fromAccountId = fromSavingsAccount.getId(); releaseLoanIds.put(loanId, guarantorFundingDetails.getId()); try { BigDecimal remainingAmount = guarantorFundingDetails.getAmountRemaining(); - if (loan.getGuaranteeAmount().compareTo(loan.getPrincipal().getAmount()) > 0) { - remainingAmount = remainingAmount.multiply(loan.getPrincipal().getAmount()).divide(loan.getGuaranteeAmount(), - MoneyHelper.getRoundingMode()); + if (freshLoan.getGuaranteeAmount().compareTo(freshLoan.getPrincipal().getAmount()) > 0) { + remainingAmount = remainingAmount.multiply(freshLoan.getPrincipal().getAmount()) + .divide(freshLoan.getGuaranteeAmount(), MoneyHelper.getRoundingMode()); } ExternalId externalId = externalIdFactory.create(); AccountTransferDTO accountTransferDTO = new AccountTransferDTO(transactionDate, remainingAmount, fromAccountType, toAccountType, fromAccountId, toAccountId, description, locale, fmt, paymentDetail, fromTransferType, toTransferType, chargeId, loanInstallmentNumber, transferType, accountTransferDetails, noteText, externalId, - loan, toSavingsAccount, fromSavingsAccount, isRegularTransaction, isExceptionForBalanceCheck); + null, null, fromSavingsAccount, isRegularTransaction, isExceptionForBalanceCheck); transferAmount(accountTransferDTO); } finally { releaseLoanIds.remove(loanId); diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/rescheduleloan/service/LoanRescheduleRequestWritePlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/rescheduleloan/service/LoanRescheduleRequestWritePlatformServiceImpl.java index 13771d033..8368e603d 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/rescheduleloan/service/LoanRescheduleRequestWritePlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/rescheduleloan/service/LoanRescheduleRequestWritePlatformServiceImpl.java @@ -451,15 +451,21 @@ public class LoanRescheduleRequestWritePlatformServiceImpl implements LoanResche // update the status of the request loanRescheduleRequest.approve(appUser, approvedOnDate); + /*** + * TODO Vishwas Batch save is giving me a HibernateOptimisticLockingFailureException, looping and saving for + * the time being, not a major issue for now as this loop is entered only in edge cases (when a adjustment + * is made before the latest payment recorded against the loan) + ***/ if (changedTransactionDetail != null) { for (final Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) { - this.accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); + loanAccountDomainService.saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue()); + accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); } // Trigger transaction replayed event replayedTransactionBusinessEventService.raiseTransactionReplayedEvents(changedTransactionDetail); } - // update the loan object loan = saveAndFlushLoanWithDataIntegrityViolationChecks(loan); + // update the loan object postJournalEntries(loan, existingTransactionIds, existingReversedTransactionIds); loanAccrualTransactionBusinessEventService.raiseBusinessEventForAccrualTransactions(loan, existingTransactionIds); diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanChargeWritePlatformServiceImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanChargeWritePlatformServiceImpl.java index 38d00e516..342ca5583 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanChargeWritePlatformServiceImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanChargeWritePlatformServiceImpl.java @@ -243,7 +243,7 @@ public class LoanChargeWritePlatformServiceImpl implements LoanChargeWritePlatfo if (loan.repaymentScheduleDetail().isInterestRecalculationEnabled()) { if (isAppliedOnBackDate && loan.isFeeCompoundingEnabledForInterestRecalculation()) { - runScheduleRecalculation(loan, recalculateFrom); + loan = runScheduleRecalculation(loan, recalculateFrom); reprocessRequired = false; } this.loanWritePlatformService.updateOriginalSchedule(loan); @@ -252,14 +252,15 @@ public class LoanChargeWritePlatformServiceImpl implements LoanChargeWritePlatfo ChangedTransactionDetail changedTransactionDetail = loan.reprocessTransactions(); if (changedTransactionDetail != null) { for (final Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) { - this.accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); + loanAccountDomainService.saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue()); + accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); } // Trigger transaction replayed event replayedTransactionBusinessEventService.raiseTransactionReplayedEvents(changedTransactionDetail); } + loan = loanAccountDomainService.saveAndFlushLoanWithDataIntegrityViolationChecks(loan); } - loan = loanAccountDomainService.saveAndFlushLoanWithDataIntegrityViolationChecks(loan); postJournalEntries(loan, existingTransactionIds, existingReversedTransactionIds); @@ -760,7 +761,7 @@ public class LoanChargeWritePlatformServiceImpl implements LoanChargeWritePlatfo if (loan.repaymentScheduleDetail().isInterestRecalculationEnabled()) { if (runInterestRecalculation && loan.isFeeCompoundingEnabledForInterestRecalculation()) { - runScheduleRecalculation(loan, recalculateFrom); + loan = runScheduleRecalculation(loan, recalculateFrom); reprocessRequired = false; } this.loanWritePlatformService.updateOriginalSchedule(loan); @@ -772,11 +773,13 @@ public class LoanChargeWritePlatformServiceImpl implements LoanChargeWritePlatfo if (changedTransactionDetail != null) { for (final Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings() .entrySet()) { - this.accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); + loanAccountDomainService.saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue()); + accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); } // Trigger transaction replayed event replayedTransactionBusinessEventService.raiseTransactionReplayedEvents(changedTransactionDetail); } + loan = loanAccountDomainService.saveAndFlushLoanWithDataIntegrityViolationChecks(loan); } postJournalEntries(loan, existingTransactionIds, existingReversedTransactionIds); @@ -1112,22 +1115,24 @@ public class LoanChargeWritePlatformServiceImpl implements LoanChargeWritePlatfo } } - public void runScheduleRecalculation(final Loan loan, final LocalDate recalculateFrom) { + public Loan runScheduleRecalculation(Loan loan, final LocalDate recalculateFrom) { if (loan.repaymentScheduleDetail().isInterestRecalculationEnabled()) { final List existingTransactionIds = loan.findExistingTransactionIds(); ScheduleGeneratorDTO generatorDTO = this.loanUtilService.buildScheduleGeneratorDTO(loan, recalculateFrom); ChangedTransactionDetail changedTransactionDetail = loan .handleRegenerateRepaymentScheduleWithInterestRecalculation(generatorDTO); - this.loanRepositoryWrapper.save(loan); if (changedTransactionDetail != null) { for (final Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) { - this.accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); + loanAccountDomainService.saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue()); + accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); } // Trigger transaction replayed event replayedTransactionBusinessEventService.raiseTransactionReplayedEvents(changedTransactionDetail); } + loan = loanAccountDomainService.saveAndFlushLoanWithDataIntegrityViolationChecks(loan); loanAccrualTransactionBusinessEventService.raiseBusinessEventForAccrualTransactions(loan, existingTransactionIds); } + return loan; } private JsonCommand adaptLoanChargeRefundCommandForFurtherRepaymentProcessing(JsonCommand command, BigDecimal fullRefundAbleAmount) { diff --git a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java index f0e389c26..97bd28814 100644 --- a/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java +++ b/fineract-provider/src/main/java/org/apache/fineract/portfolio/loanaccount/service/LoanWritePlatformServiceJpaRepositoryImpl.java @@ -105,9 +105,7 @@ import org.apache.fineract.portfolio.account.domain.AccountAssociationsRepositor import org.apache.fineract.portfolio.account.domain.AccountTransferDetailRepository; import org.apache.fineract.portfolio.account.domain.AccountTransferDetails; import org.apache.fineract.portfolio.account.domain.AccountTransferRecurrenceType; -import org.apache.fineract.portfolio.account.domain.AccountTransferRepository; import org.apache.fineract.portfolio.account.domain.AccountTransferStandingInstruction; -import org.apache.fineract.portfolio.account.domain.AccountTransferTransaction; import org.apache.fineract.portfolio.account.domain.AccountTransferType; import org.apache.fineract.portfolio.account.domain.StandingInstructionPriority; import org.apache.fineract.portfolio.account.domain.StandingInstructionStatus; @@ -223,7 +221,6 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf private final AccountAssociationsReadPlatformService accountAssociationsReadPlatformService; private final LoanReadPlatformService loanReadPlatformService; private final FromJsonHelper fromApiJsonHelper; - private final AccountTransferRepository accountTransferRepository; private final CalendarRepository calendarRepository; private final LoanScheduleHistoryWritePlatformService loanScheduleHistoryWritePlatformService; private final LoanApplicationCommandFromApiJsonHelper loanApplicationCommandFromApiJsonHelper; @@ -460,13 +457,12 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf if (!changes.isEmpty()) { if (changedTransactionDetail != null) { for (final Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) { - this.loanTransactionRepository.save(mapEntry.getValue()); - this.accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); + loanAccountDomainService.saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue()); + accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); } // Trigger transaction replayed event replayedTransactionBusinessEventService.raiseTransactionReplayedEvents(changedTransactionDetail); } - loan = saveAndFlushLoanWithDataIntegrityViolationChecks(loan); final String noteText = command.stringValueOfParameterNamed("note"); @@ -745,8 +741,8 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf if (changedTransactionDetail != null) { for (final Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings() .entrySet()) { - this.loanTransactionRepository.save(mapEntry.getValue()); - this.accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); + loanAccountDomainService.saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue()); + accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); } // Trigger transaction replayed event replayedTransactionBusinessEventService.raiseTransactionReplayedEvents(changedTransactionDetail); @@ -949,7 +945,7 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf if (!txnExternalId.isEmpty()) { changes.put(LoanApiConstants.externalIdParameterName, txnExternalId); } - final Loan loan = this.loanAssembler.assembleFrom(loanId); + Loan loan = this.loanAssembler.assembleFrom(loanId); final PaymentDetail paymentDetail = this.paymentDetailWritePlatformService.createAndPersistPaymentDetail(command, changes); final Boolean isHolidayValidationDone = false; final HolidayDetailDTO holidayDetailDto = null; @@ -958,7 +954,7 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf LoanTransaction loanTransaction = this.loanAccountDomainService.makeRepayment(repaymentTransactionType, loan, transactionDate, transactionAmount, paymentDetail, noteText, txnExternalId, isRecoveryRepayment, chargeRefundChargeType, isAccountTransfer, holidayDetailDto, isHolidayValidationDone); - + loan = loanTransaction.getLoan(); // Update loan transaction on repayment. if (AccountType.fromInt(loan.getLoanType()).isIndividualAccount()) { Set loanCollateralManagements = loan.getLoanCollateralManagements(); @@ -1156,7 +1152,8 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf ***/ if (changedTransactionDetail != null) { for (final Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) { - this.accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); + loanAccountDomainService.saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue()); + accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); } // Trigger transaction replayed event replayedTransactionBusinessEventService.raiseTransactionReplayedEvents(changedTransactionDetail); @@ -1391,7 +1388,8 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf if (changedTransactionDetail != null) { for (final Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) { - this.accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); + loanAccountDomainService.saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue()); + accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); } // Trigger transaction replayed event replayedTransactionBusinessEventService.raiseTransactionReplayedEvents(changedTransactionDetail); @@ -2171,7 +2169,7 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf @Override public CommandProcessingResult undoWriteOff(Long loanId) { - final Loan loan = this.loanAssembler.assembleFrom(loanId); + Loan loan = this.loanAssembler.assembleFrom(loanId); checkClientOrGroupActive(loan); final List existingTransactionIds = new ArrayList<>(); final List existingReversedTransactionIds = new ArrayList<>(); @@ -2193,13 +2191,13 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf existingReversedTransactionIds, scheduleGeneratorDTO); if (changedTransactionDetail != null) { for (final Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) { - this.loanTransactionRepository.save(mapEntry.getValue()); - this.accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); + loanAccountDomainService.saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue()); + accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); } // Trigger transaction replayed event replayedTransactionBusinessEventService.raiseTransactionReplayedEvents(changedTransactionDetail); } - saveLoanWithDataIntegrityViolationChecks(loan); + loan = saveAndFlushLoanWithDataIntegrityViolationChecks(loan); postJournalEntries(loan, existingTransactionIds, existingReversedTransactionIds); loanAccrualTransactionBusinessEventService.raiseBusinessEventForAccrualTransactions(loan, existingTransactionIds); @@ -2340,9 +2338,9 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf } if (command.entityId() != null && changedTransactionDetail != null) { - for (Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) { - this.loanTransactionRepository.save(mapEntry.getValue()); - updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); + for (final Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) { + loanAccountDomainService.saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue()); + accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); } // Trigger transaction replayed event replayedTransactionBusinessEventService.raiseTransactionReplayedEvents(changedTransactionDetail); @@ -2400,12 +2398,13 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf if (changedTransactionDetail != null) { for (final Map.Entry mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) { - this.accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); + loanAccountDomainService.saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue()); + accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue()); } // Trigger transaction replayed event replayedTransactionBusinessEventService.raiseTransactionReplayedEvents(changedTransactionDetail); } - saveLoanWithDataIntegrityViolationChecks(loan); + loan = saveAndFlushLoanWithDataIntegrityViolationChecks(loan); postJournalEntries(loan, existingTransactionIds, existingReversedTransactionIds); loanAccrualTransactionBusinessEventService.raiseBusinessEventForAccrualTransactions(loan, existingTransactionIds); loanAccountDomainService.recalculateAccruals(loan); @@ -2441,14 +2440,6 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf } } - private void updateLoanTransaction(final Long loanTransactionId, final LoanTransaction newLoanTransaction) { - final AccountTransferTransaction transferTransaction = this.accountTransferRepository.findByToLoanTransactionId(loanTransactionId); - if (transferTransaction != null) { - transferTransaction.updateToLoanTransaction(newLoanTransaction); - this.accountTransferRepository.save(transferTransaction); - } - } - private void createLoanScheduleArchive(final Loan loan, final ScheduleGeneratorDTO scheduleGeneratorDTO) { createAndSaveLoanScheduleArchive(loan, scheduleGeneratorDTO);