You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "b0c1 (via GitHub)" <gi...@apache.org> on 2023/04/28 10:46:24 UTC
[GitHub] [fineract] b0c1 opened a new pull request, #3149: FINERACT-1724 - BatchTransaction fix
b0c1 opened a new pull request, #3149:
URL: https://github.com/apache/fineract/pull/3149
- [x] Handle `enclosingTransaction=false` flag correctly.
--
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
[GitHub] [fineract] adamsaghy commented on a diff in pull request #3149: FINERACT-1724 - BatchTransaction fix
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
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
[GitHub] [fineract] adamsaghy merged pull request #3149: FINERACT-1724 - BatchTransaction fix
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy merged PR #3149:
URL: https://github.com/apache/fineract/pull/3149
--
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