marta-jankovics commented on code in PR #3555:
URL: https://github.com/apache/fineract/pull/3555#discussion_r1439748909
##########
fineract-core/src/main/java/org/apache/fineract/commands/service/SynchronousCommandProcessingService.java:
##########
@@ -201,6 +203,14 @@ private void setIdempotencyKeyStoreFlag(boolean flag) {
fineractRequestContextHolder.setAttribute(IDEMPOTENCY_KEY_STORE_FLAG,
flag);
}
+ @Override
+ public void logFailedBatchRequestWithEnclosingTransaction(final
CommandWrapper commandRequest, final BatchResponse failedBatchResult) {
+ final AppUser user = context.authenticatedUser(commandRequest);
+ final CommandSource commandSource =
CommandSource.fullEntryForBatchFailed(commandRequest, failedBatchResult, user,
+ idempotencyKeyGenerator.create());
Review Comment:
I do not think that generating a new idempotency key is correct, rather we
should read it from the incoming request header and save that. The main purpose
of the idempotency is that the caller knows it and if it tries to reuse (from
same or different instance) then the same result (without perform anything) is
returned.
##########
fineract-core/src/main/java/org/apache/fineract/batch/service/BatchApiServiceImpl.java:
##########
@@ -116,7 +116,7 @@ public List<BatchResponse>
handleBatchRequestsWithoutEnclosingTransaction(final
*/
@Override
public List<BatchResponse>
handleBatchRequestsWithEnclosingTransaction(final List<BatchRequest>
requestList, final UriInfo uriInfo) {
- return callInTransaction(Function.identity()::apply, () ->
handleBatchRequests(true, requestList, uriInfo));
+ return callInTransaction(Function.identity()::apply, () ->
handleBatchRequests(true, requestList, uriInfo), true);
Review Comment:
You are right that enclosingTransaction flag should be correctly set for
'withoutEnclosingTransaction' case to false. This was a bug and will be fixed
with the 4-eye principle PR
https://github.com/apache/fineract/pull/3649
as maker-checker concept was also not working in this case.
Unfortunately this does not solve the batch api problem of this story.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
