You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "adamsaghy (via GitHub)" <gi...@apache.org> on 2023/04/28 12:09:16 UTC
[GitHub] [fineract] adamsaghy commented on a diff in pull request #3149: FINERACT-1724 - BatchTransaction fix
adamsaghy commented on code in PR #3149:
URL: https://github.com/apache/fineract/pull/3149#discussion_r1180326734
##########
fineract-provider/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java:
##########
@@ -141,10 +137,18 @@ private List<BatchResponse> handleBatchRequests(boolean enclosingTransaction, fi
* the collected responses
* @return {@code BatchResponse}
*/
- private void callRequestRecursive(BatchRequest request, BatchRequestNode requestNode, List<BatchResponse> responseList,
- UriInfo uriInfo) {
+ private void callRequestRecursive(BatchRequest request, BatchRequestNode requestNode, List<BatchResponse> responseList, UriInfo uriInfo,
+ boolean enclosingTransaction) {
// 1. run current node
- BatchResponse response = executeRequest(request, uriInfo);
+ BatchResponse response;
+ if (enclosingTransaction) {
+ response = executeRequest(request, uriInfo);
+ } else {
+ List<BatchResponse> transactionResponse = callInTransaction(
+ transactionTemplate -> transactionTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW),
+ () -> List.of(executeRequest(request, uriInfo)));
+ response = transactionResponse.get(0);
Review Comment:
only the first?
##########
fineract-provider/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java:
##########
@@ -141,10 +137,18 @@ private List<BatchResponse> handleBatchRequests(boolean enclosingTransaction, fi
* the collected responses
* @return {@code BatchResponse}
*/
- private void callRequestRecursive(BatchRequest request, BatchRequestNode requestNode, List<BatchResponse> responseList,
- UriInfo uriInfo) {
+ private void callRequestRecursive(BatchRequest request, BatchRequestNode requestNode, List<BatchResponse> responseList, UriInfo uriInfo,
+ boolean enclosingTransaction) {
// 1. run current node
- BatchResponse response = executeRequest(request, uriInfo);
+ BatchResponse response;
+ if (enclosingTransaction) {
+ response = executeRequest(request, uriInfo);
+ } else {
+ List<BatchResponse> transactionResponse = callInTransaction(
+ transactionTemplate -> transactionTemplate.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW),
Review Comment:
do we need any transaction here?
##########
integration-tests/src/test/java/org/apache/fineract/integrationtests/common/BatchHelper.java:
##########
@@ -100,7 +102,8 @@ private static List<BatchResponse> fromJsonString(final String json) {
*/
public static List<BatchResponse> 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);
Review Comment:
is there an existing test which checks whether the batch requests that were executed got not reverted?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org