From 5238e29449d35618279a2c3f0af485fc14541f29 Mon Sep 17 00:00:00 2001 From: Janos Haber Date: Fri, 21 Apr 2023 15:21:05 +0200 Subject: [PATCH] FINERACT-1724 - BatchTransaction fix - [x] Handle `enclosingTransaction=false` flag correctly. --- .../batch/service/BatchApiServiceImpl.java | 28 +++++---- .../integrationtests/BatchApiTest.java | 62 ++++++++++++++++++- .../integrationtests/common/BatchHelper.java | 34 +++++++++- 3 files changed, 110 insertions(+), 14 deletions(-) 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 5ef0458bc..0ef2b2ac6 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 @@ -115,15 +115,11 @@ public class BatchApiServiceImpl implements BatchApiService { for (BatchRequestNode rootNode : batchRequestNodes) { if (enclosingTransaction) { - this.callRequestRecursive(rootNode.getRequest(), rootNode, responseList, uriInfo); + this.callRequestRecursive(rootNode.getRequest(), rootNode, responseList, uriInfo, enclosingTransaction); } else { - responseList.addAll(callInTransaction( - transactionTemplate -> transactionTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW), - () -> { - List localResponseList = new ArrayList<>(); - this.callRequestRecursive(rootNode.getRequest(), rootNode, localResponseList, uriInfo); - return localResponseList; - })); + List localResponseList = new ArrayList<>(); + this.callRequestRecursive(rootNode.getRequest(), rootNode, localResponseList, uriInfo, enclosingTransaction); + responseList.addAll(localResponseList); } } Collections.sort(responseList, Comparator.comparing(BatchResponse::getRequestId)); @@ -141,10 +137,18 @@ public class BatchApiServiceImpl implements BatchApiService { * the collected responses * @return {@code BatchResponse} */ - private void callRequestRecursive(BatchRequest request, BatchRequestNode requestNode, List responseList, - UriInfo uriInfo) { + private void callRequestRecursive(BatchRequest request, BatchRequestNode requestNode, List responseList, UriInfo uriInfo, + boolean enclosingTransaction) { // 1. run current node - BatchResponse response = executeRequest(request, uriInfo); + BatchResponse response; + if (enclosingTransaction) { + response = executeRequest(request, uriInfo); + } else { + List transactionResponse = callInTransaction( + transactionTemplate -> transactionTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW), + () -> List.of(executeRequest(request, uriInfo))); + response = transactionResponse.get(0); + } responseList.add(response); if (response.getStatusCode() != null && response.getStatusCode() == 200) { requestNode.getChildRequests().forEach(childNode -> { @@ -154,7 +158,7 @@ public class BatchApiServiceImpl implements BatchApiService { } catch (RuntimeException ex) { throw new BatchExecutionException(childNode.getRequest(), ex); } - callRequestRecursive(resolvedChildRequest, childNode, responseList, uriInfo); + callRequestRecursive(resolvedChildRequest, childNode, responseList, uriInfo, enclosingTransaction); }); } else { diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/BatchApiTest.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/BatchApiTest.java index 77aa09b8b..4e4816221 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/BatchApiTest.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/BatchApiTest.java @@ -77,7 +77,6 @@ import org.slf4j.LoggerFactory; * commandStrategy by injecting it with a {@code BatchRequest}. * * @author RishabhShukla - * * @see org.apache.fineract.integrationtests.common.BatchHelper * @see org.apache.fineract.batch.domain.BatchRequest */ @@ -785,6 +784,67 @@ public class BatchApiTest { "Verify Status Code 200 for Credit Balance Refund"); } + @Test + public void partialFailTestForBatchRequest() { + + final String loanProductJSON = new LoanProductTestBuilder() // + .withPrincipal("1000.00") // + .withNumberOfRepayments("24") // + .withRepaymentAfterEvery("1") // + .withRepaymentTypeAsMonth() // + .withinterestRatePerPeriod("2") // + .withInterestRateFrequencyTypeAsMonths() // + .withAmortizationTypeAsEqualPrincipalPayment() // + .withInterestTypeAsDecliningBalance() // + .currencyDetails("0", "100").build(null); + + final Integer clientID = ClientHelper.createClient(this.requestSpec, this.responseSpec); + ClientHelper.verifyClientCreatedOnServer(this.requestSpec, this.responseSpec, clientID); + + final Integer collateralId = CollateralManagementHelper.createCollateralProduct(this.requestSpec, this.responseSpec); + Assertions.assertNotNull(collateralId); + final Integer clientCollateralId = CollateralManagementHelper.createClientCollateral(this.requestSpec, this.responseSpec, + clientID.toString(), collateralId); + Assertions.assertNotNull(clientCollateralId); + + final Integer productId = new LoanTransactionHelper(this.requestSpec, this.responseSpec).getLoanProductId(loanProductJSON); + + final Long createActiveClientRequestId = 4730L; + final Long applyLoanRequestId = createActiveClientRequestId + 1; + final Long approveLoanRequestId = applyLoanRequestId + 1; + final Long disburseLoanRequestId = approveLoanRequestId + 1; + final Long fetchLoanInfoRequestId = disburseLoanRequestId + 1; + + // Create a createClient Request + final BatchRequest br1 = BatchHelper.createActiveClientRequest(createActiveClientRequestId, ""); + + // Create a ApplyLoan Request + final BatchRequest br2 = BatchHelper.applyLoanRequest(applyLoanRequestId, createActiveClientRequestId, productId, + clientCollateralId); + + // Create a wrong approveLoan Request + final BatchRequest br3 = BatchHelper.approveLoanWrongRequest(approveLoanRequestId, applyLoanRequestId); + + // Fetch loan info + final BatchRequest br4 = BatchHelper.getLoanByIdRequest(fetchLoanInfoRequestId, applyLoanRequestId, null); + + final List batchRequests = new ArrayList<>(); + + batchRequests.add(br1); + batchRequests.add(br2); + batchRequests.add(br3); + batchRequests.add(br4); + + final String jsonifiedRequest = BatchHelper.toJsonString(batchRequests); + + final List response = BatchHelper.postBatchRequestsWithoutEnclosingTransaction(this.requestSpec, this.responseSpec, + jsonifiedRequest); + + Assertions.assertEquals(HttpStatus.SC_NOT_IMPLEMENTED, (long) response.get(2).getStatusCode(), "Resource doesn not exists"); + Assertions.assertEquals(HttpStatus.SC_OK, (long) response.get(3).getStatusCode(), + "Verify Status Code 200 for fetch data after the error"); + } + /** * Tests successful run of batch goodwill credit for loans. 200(OK) status is returned for successful responses. It * first creates a new loan, approves and disburses the loan. Then a goodwill credit request is made diff --git a/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/BatchHelper.java b/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/BatchHelper.java index faaa0091e..d846fdb44 100644 --- a/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/BatchHelper.java +++ b/integration-tests/src/test/java/org/apache/fineract/integrationtests/common/BatchHelper.java @@ -53,6 +53,8 @@ public final class BatchHelper { private static final Logger LOG = LoggerFactory.getLogger(BatchHelper.class); private static final String BATCH_API_URL = "/fineract-provider/api/v1/batches?" + Utils.TENANT_IDENTIFIER; private static final String BATCH_API_URL_EXT = BATCH_API_URL + "&enclosingTransaction=true"; + + private static final String BATCH_API_WITHOUT_ENCLOSING_URL_EXT = BATCH_API_URL + "&enclosingTransaction=false"; private static final SecureRandom secureRandom = new SecureRandom(); private BatchHelper() { @@ -100,7 +102,8 @@ public final class BatchHelper { */ public static List postBatchRequestsWithoutEnclosingTransaction(final RequestSpecification requestSpec, final ResponseSpecification responseSpec, final String jsonifiedBatchRequests) { - final String response = Utils.performServerPost(requestSpec, responseSpec, BATCH_API_URL, jsonifiedBatchRequests, null); + final String response = Utils.performServerPost(requestSpec, responseSpec, BATCH_API_WITHOUT_ENCLOSING_URL_EXT, + jsonifiedBatchRequests, null); LOG.info("BatchHelper Response {}", response); return BatchHelper.fromJsonString(response); } @@ -628,6 +631,21 @@ public final class BatchHelper { return approveLoanRequest(requestId, reference, LocalDate.now(ZoneId.systemDefault()).minusDays(10)); } + /** + * Creates a wrong {@link org.apache.fineract.batch.command.internal.ApproveLoanCommandStrategy} Request with given + * requestId and reference. + * + * + * @param requestId + * the request ID + * @param reference + * the reference ID + * @return BatchRequest the batch request + */ + public static BatchRequest approveLoanWrongRequest(final Long requestId, final Long reference) { + return approveLoanWrongRequest(requestId, reference, LocalDate.now(ZoneId.systemDefault()).minusDays(10)); + } + /** * Creates and returns a {@link org.apache.fineract.batch.command.internal.ApproveLoanCommandStrategy} Request with * given requestId and reference. @@ -655,6 +673,20 @@ public final class BatchHelper { return br; } + public static BatchRequest approveLoanWrongRequest(final Long requestId, final Long reference, LocalDate date) { + final BatchRequest br = new BatchRequest(); + + br.setRequestId(requestId); + br.setRelativeUrl("loans/$.loanId?command=approveX"); + br.setReference(reference); + br.setMethod("POST"); + String dateString = date.format(DateTimeFormatter.ofPattern("dd MMMM yyyy")); + br.setBody("{\"locale\": \"en\", \"dateFormat\": \"dd MMMM yyyy\", \"approvedOnDate\": \"" + dateString + "\"," + + "\"note\": \"Loan approval note\"}"); + + return br; + } + /** * Creates and returns a {@link org.apache.fineract.batch.command.internal.DisburseLoanCommandStrategy} Request with * given requestId and reference.