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

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<BatchFilter> batchFilters;
 
+    private final List<BatchRequestPreprocessor> batchPreprocessors;
+
     @PersistenceContext
     private final EntityManager entityManager;
 
@@ -87,8 +98,8 @@ public class BatchApiServiceImpl implements BatchApiService {
      * @param uriInfo
      * @return {@code List<BatchResponse>}
      */
-    private List<BatchResponse> handleBatchRequests(final List<BatchRequest> requestList, final UriInfo uriInfo,
+    private List<BatchResponse> handleBatchRequests(boolean enclosingTransaction, final List<BatchRequest> requestList,
-            boolean isEnclosingTransaction) {
+            final UriInfo uriInfo) {
 
         final List<BatchResponse> responseList = new ArrayList<>(requestList.size());
 
@@ -103,52 +114,150 @@ public class BatchApiServiceImpl implements BatchApiService {
         }
 
         for (BatchRequestNode rootNode : batchRequestNodes) {
-            final BatchRequest rootRequest = rootNode.getRequest();
+            if (enclosingTransaction) {
-            final CommandStrategy commandStrategy = this.strategyProvider
+                this.callRequestRecursive(rootNode.getRequest(), rootNode, responseList, uriInfo);
-                    .getCommandStrategy(CommandContext.resource(rootRequest.getRelativeUrl()).method(rootRequest.getMethod()).build());
+            } else {
-            log.debug("Batch request: method [{}], relative url [{}]", rootRequest.getMethod(), rootRequest.getRelativeUrl());
+                responseList.addAll(callInTransaction(
-            final BatchResponse rootResponse = safelyExecuteStrategy(commandStrategy, rootRequest, uriInfo, isEnclosingTransaction);
+                        transactionTemplate -> transactionTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW),
-            log.debug("Batch response: status code [{}], method [{}], relative url [{}]", rootResponse.getStatusCode(),
+                        () -> {
-                    rootRequest.getMethod(), rootRequest.getRelativeUrl());
+                            List<BatchResponse> localResponseList = new ArrayList<>();
-            responseList.add(rootResponse);
+                            this.callRequestRecursive(rootNode.getRequest(), rootNode, localResponseList, uriInfo);
-            responseList.addAll(this.processChildRequests(rootNode, rootResponse, uriInfo, isEnclosingTransaction));
+                            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<BatchResponse> 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<BatchResponse> parentRequestFailedRecursive(BatchRequest request, BatchRequestNode requestNode) {
+        List<BatchResponse> 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<RuntimeException, BatchRequest> 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())));
-
+            BatchCallHandler callHandler = new BatchCallHandler(this.batchFilters, commandStrategy::execute);
-            return new BatchCallHandler(this.batchFilters, commandStrategy::execute).serviceCall(request, originalUriInfo);
+            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) {
+        } catch (RuntimeException ex) {
-            log.warn("Exception while executing batch strategy {}", commandStrategy.getClass().getSimpleName(), e);
+            throw new BatchExecutionException(request, ex);
-
-            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;
         } finally {
             BatchRequestContextHolder.resetRequestAttributes();
         }
     }
 
+    private Either<RuntimeException, BatchRequest> runPreprocessors(BatchRequest request) {
+        return runPreprocessor(batchPreprocessors, request);
+    }
+
+    private Either<RuntimeException, BatchRequest> runPreprocessor(List<BatchRequestPreprocessor> remainingPreprocessor,
+            BatchRequest request) {
+        if (remainingPreprocessor.isEmpty()) {
+            return Either.right(request);
+        } else {
+            BatchRequestPreprocessor preprocessor = remainingPreprocessor.get(0);
+            Either<RuntimeException, BatchRequest> 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<BatchResponse> processChildRequests(final BatchRequestNode rootRequest, BatchResponse rootResponse, UriInfo uriInfo,
+    /**
-            boolean isEnclosingTransaction) {
+     * Run each request root step in a separated transaction
-
+     *
-        final List<BatchResponse> childResponses = new ArrayList<>();
+     * @param requestList
-        if (!rootRequest.getChildRequests().isEmpty()) {
+     * @param uriInfo
-
+     * @return
-            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;
-    }
-
     @Override
     public List<BatchResponse> handleBatchRequestsWithoutEnclosingTransaction(final List<BatchRequest> requestList, UriInfo uriInfo) {
-
+        BatchRequestContextHolder.setEnclosingTransaction(Optional.empty());
-        return handleBatchRequests(requestList, uriInfo, false);
+        return handleBatchRequests(false, requestList, uriInfo);
     }
 
+    /**
+     * Run the batch request in transaction
+     *
+     * @param requestList
+     * @param uriInfo
+     * @return
+     */
     @Override
     public List<BatchResponse> handleBatchRequestsWithEnclosingTransaction(final List<BatchRequest> requestList, final UriInfo uriInfo) {
+        return callInTransaction(() -> handleBatchRequests(true, requestList, uriInfo));
+    }
+
+    @NotNull
+    private List<BatchResponse> createErrorResponse(List<BatchResponse> 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<BatchResponse> callInTransaction(Supplier<List<BatchResponse>> 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<BatchResponse> callInTransaction(Consumer<TransactionTemplate> transactionConfigurator,
+            Supplier<List<BatchResponse>> request) {
         List<BatchResponse> 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());
-
+                    return Arrays.asList(errResponse);
-                    List<BatchResponse> errResponseList = new ArrayList<>();
-                    errResponseList.add(errResponse);
-
-                    status.setRollbackOnly();
-                    return errResponseList;
                 }
             });
         } catch (TransactionException | NonTransientDataAccessException ex) {
             ErrorInfo e = ErrorHandler.handler(ex);
-            BatchResponse errResponse = new BatchResponse();
+            return createErrorResponse(responseList, e.getStatusCode());
-            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<BatchResponse> errResponseList = new ArrayList<>();
-            errResponseList.add(errResponse);
-
-            return errResponseList;
         }
     }
+
 }

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);
+    }
+}

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);
 }

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) {

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;
-        CommandSource savedCommandSource = commandSourceService.saveInitial(wrapper, command, context.authenticatedUser(wrapper),
+        if (BatchRequestContextHolder.getEnclosingTransaction().isPresent()) {
-                idempotencyKey);
+            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();

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<Map<String, Object>> batchAttributes = new NamedThreadLocal<>("BatchAttributesForProcessing");
 
+    private static final ThreadLocal<Optional<TransactionStatus>> enclosingTransaction = new NamedThreadLocal<>("EnclosingTransaction") {
+
+        @Override
+        protected Optional<TransactionStatus> 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<TransactionStatus> 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<TransactionStatus> enclosingTransaction) {
+        BatchRequestContextHolder.enclosingTransaction.set(enclosingTransaction);
+    }
 }

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<RuntimeException, BatchRequest> preprocess(BatchRequest batchRequest);
+}

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<Long> commandId) {
+    private void storeCommandResult(boolean batch, int response, String body, Optional<Long> 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<Long> 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;
     }

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.BatchRequestPreprocessor;
-import org.apache.fineract.infrastructure.core.filters.BatchFilterChain;
 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) {
+    public Either<RuntimeException, BatchRequest> preprocess(BatchRequest batchRequest) {
-        if (!isOnApiList("/" + batchRequest.getRelativeUrl(), batchRequest.getMethod())) {
+        TransactionTemplate tr = new TransactionTemplate(transactionManager);
-            return chain.serviceCall(batchRequest, uriInfo);
+        tr.setPropagationBehavior(TransactionDefinition.PROPAGATION_NOT_SUPPORTED);
-        } else {
+        return tr.execute(status -> {
             try {
-                boolean bypassUser = isBypassUser();
+                if (isOnApiList("/" + batchRequest.getRelativeUrl(), batchRequest.getMethod())) {
-                if (bypassUser) {
+                    boolean bypassUser = isBypassUser();
-                    return chain.serviceCall(batchRequest, uriInfo);
+                    if (!bypassUser) {
-                } else {
-                    try {
                         List<Long> 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) {
+            } catch (LoanNotFoundException e) {
-                return Reject.reject(null, HttpStatus.SC_UNAUTHORIZED).toBatchResponse(batchRequest);
+                return Either.right(batchRequest);
+            } catch (RuntimeException e) {
+                return Either.left(e);
             }
-        }
+            return Either.right(batchRequest);
+        });
     }
+
 }

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,

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<Long, LoanTransaction> mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) {
                     mapEntry.getValue().updateLoan(this);
                 }
-
+                this.loanTransactions.addAll(changedTransactionDetail.getNewTransactionMappings().values());
             }
             updateLoanSummaryDerivedFields();
         }

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<Long, LoanTransaction> mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) {
+            for (final Map.Entry<Long, LoanTransaction> 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,
+    public LoanTransaction foreCloseLoan(Loan loan, final LocalDate foreClosureDate, final String noteText, final ExternalId externalId,
-            final ExternalId externalId, Map<String, Object> changes) {
+            Map<String, Object> 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<Long, LoanTransaction> mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) {
+            for (final Map.Entry<Long, LoanTransaction> mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) {
+                saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue());
                 updateLoanTransaction(mapEntry.getKey(), mapEntry.getValue());
             }
             // Trigger transaction replayed event
             replayedTransactionBusinessEventService.raiseTransactionReplayedEvents(changedTransactionDetail);
         }
-
+        loan = saveAndFlushLoanWithDataIntegrityViolationChecks(loan);
-        saveAndFlushLoanWithDataIntegrityViolationChecks(loan);
 
         if (StringUtils.isNotBlank(noteText)) {
             changes.put("note", noteText);

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<GuarantorFundingDetails> 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) {
+                        if (freshLoan.getGuaranteeAmount().compareTo(freshLoan.getPrincipal().getAmount()) > 0) {
-                            remainingAmount = remainingAmount.multiply(loan.getPrincipal().getAmount()).divide(loan.getGuaranteeAmount(),
+                            remainingAmount = remainingAmount.multiply(freshLoan.getPrincipal().getAmount())
-                                    MoneyHelper.getRoundingMode());
+                                    .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);

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<Long, LoanTransaction> 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);
 

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<Long, LoanTransaction> 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<Long, LoanTransaction> 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<Long> 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<Long, LoanTransaction> 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) {

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<Long, LoanTransaction> mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) {
-                    this.loanTransactionRepository.save(mapEntry.getValue());
+                    loanAccountDomainService.saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue());
-                    this.accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), 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<Long, LoanTransaction> mapEntry : changedTransactionDetail.getNewTransactionMappings()
                             .entrySet()) {
-                        this.loanTransactionRepository.save(mapEntry.getValue());
+                        loanAccountDomainService.saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue());
-                        this.accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), 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<LoanCollateralManagement> loanCollateralManagements = loan.getLoanCollateralManagements();
@@ -1156,7 +1152,8 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
          ***/
         if (changedTransactionDetail != null) {
             for (final Map.Entry<Long, LoanTransaction> 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<Long, LoanTransaction> 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<Long> existingTransactionIds = new ArrayList<>();
         final List<Long> existingReversedTransactionIds = new ArrayList<>();
@@ -2193,13 +2191,13 @@ public class LoanWritePlatformServiceJpaRepositoryImpl implements LoanWritePlatf
                 existingReversedTransactionIds, scheduleGeneratorDTO);
         if (changedTransactionDetail != null) {
             for (final Map.Entry<Long, LoanTransaction> mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) {
-                this.loanTransactionRepository.save(mapEntry.getValue());
+                loanAccountDomainService.saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue());
-                this.accountTransfersWritePlatformService.updateLoanTransaction(mapEntry.getKey(), 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<Long, LoanTransaction> mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) {
+            for (final Map.Entry<Long, LoanTransaction> mapEntry : changedTransactionDetail.getNewTransactionMappings().entrySet()) {
-                this.loanTransactionRepository.save(mapEntry.getValue());
+                loanAccountDomainService.saveLoanTransactionWithDataIntegrityViolationChecks(mapEntry.getValue());
-                updateLoanTransaction(mapEntry.getKey(), 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<Long, LoanTransaction> 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);