Skip to content

Commit e27a0bb

Browse files
marta-jankovicsadamsaghy
marta-jankovics
authored and
adamsaghy
committed
FINERACT-2000: Clean-up retryable error codes
1 parent 2f38d28 commit e27a0bb

6 files changed

Lines changed: 44 additions & 35 deletions

File tree

‎fineract-core/src/main/java/org/apache/fineract/infrastructure/core/data/ApiGlobalErrorResponse.java‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import static org.apache.http.HttpStatus.SC_CONFLICT;
2424
import static org.apache.http.HttpStatus.SC_FORBIDDEN;
2525
import static org.apache.http.HttpStatus.SC_INTERNAL_SERVER_ERROR;
26-
import static org.apache.http.HttpStatus.SC_LOCKED;
2726
import static org.apache.http.HttpStatus.SC_METHOD_NOT_ALLOWED;
2827
import static org.apache.http.HttpStatus.SC_NOT_FOUND;
2928
import static org.apache.http.HttpStatus.SC_SERVICE_UNAVAILABLE;
@@ -107,7 +106,7 @@ public static ApiGlobalErrorResponse loanIsLocked(final Long loanId) {
107106
return create(SC_CONFLICT, "error.msg.loan.locked", msg, msg);
108107
}
109108

110-
public static ApiGlobalErrorResponse locked(String type, String identifier) {
109+
public static ApiGlobalErrorResponse conflict(String type, String identifier) {
111110
String details = "";
112111
if (type == null) {
113112
type = "unknown";
@@ -119,7 +118,7 @@ public static ApiGlobalErrorResponse locked(String type, String identifier) {
119118
}
120119
String msg = "The server is currently unable to handle the request due to concurrent modification " + details
121120
+ ", please try again";
122-
return create(SC_LOCKED, "error.msg.platform.service." + type + ".conflict", msg, msg);
121+
return create(SC_CONFLICT, "error.msg.platform.service." + type + ".conflict", msg, msg);
123122
}
124123

125124
public static ApiGlobalErrorResponse unAuthorized(final String defaultUserMessage) {

‎fineract-core/src/main/java/org/apache/fineract/infrastructure/core/exceptionmapper/ConcurrencyFailureExceptionMapper.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919
package org.apache.fineract.infrastructure.core.exceptionmapper;
2020

21-
import static org.apache.http.HttpStatus.SC_LOCKED;
21+
import static org.apache.http.HttpStatus.SC_CONFLICT;
2222

2323
import jakarta.ws.rs.core.MediaType;
2424
import jakarta.ws.rs.core.Response;
@@ -53,8 +53,8 @@ public Response toResponse(final ConcurrencyFailureException exception) {
5353
type = "lock";
5454
identifier = null;
5555
}
56-
final ApiGlobalErrorResponse dataIntegrityError = ApiGlobalErrorResponse.locked(type, identifier);
57-
return Response.status(SC_LOCKED).entity(dataIntegrityError).type(MediaType.APPLICATION_JSON).build();
56+
final ApiGlobalErrorResponse dataIntegrityError = ApiGlobalErrorResponse.conflict(type, identifier);
57+
return Response.status(SC_CONFLICT).entity(dataIntegrityError).type(MediaType.APPLICATION_JSON).build();
5858
}
5959

6060
@Override

‎fineract-core/src/main/java/org/apache/fineract/infrastructure/core/exceptionmapper/IdempotentCommandExceptionMapper.java‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,11 @@
1919
package org.apache.fineract.infrastructure.core.exceptionmapper;
2020

2121
import static org.apache.fineract.infrastructure.core.exception.AbstractIdempotentCommandException.IDEMPOTENT_CACHE_HEADER;
22+
import static org.apache.http.HttpStatus.SC_INTERNAL_SERVER_ERROR;
23+
import static org.apache.http.HttpStatus.SC_OK;
2224

2325
import jakarta.ws.rs.core.MediaType;
2426
import jakarta.ws.rs.core.Response;
25-
import jakarta.ws.rs.core.Response.Status;
2627
import jakarta.ws.rs.ext.ExceptionMapper;
2728
import jakarta.ws.rs.ext.Provider;
2829
import lombok.extern.slf4j.Slf4j;
@@ -46,18 +47,18 @@ public class IdempotentCommandExceptionMapper implements FineractExceptionMapper
4647
@Override
4748
public Response toResponse(final AbstractIdempotentCommandException exception) {
4849
log.warn("Processing {} request: {}", exception.getClass().getName(), exception.getMessage());
49-
Status status = null;
50+
Integer status = null;
5051
if (exception instanceof IdempotentCommandProcessSucceedException pse) {
5152
Integer statusCode = pse.getStatusCode();
52-
status = statusCode == null ? Status.OK : Status.fromStatusCode(statusCode);
53+
status = statusCode == null ? SC_OK : statusCode;
5354
}
5455
if (exception instanceof IdempotentCommandProcessUnderProcessingException) {
55-
status = Status.CONFLICT;
56+
status = 425;
5657
} else if (exception instanceof IdempotentCommandProcessFailedException pfe) {
57-
status = Status.fromStatusCode(pfe.getStatusCode());
58+
status = pfe.getStatusCode();
5859
}
5960
if (status == null) {
60-
status = Status.INTERNAL_SERVER_ERROR;
61+
status = SC_INTERNAL_SERVER_ERROR;
6162
}
6263
return Response.status(status).entity(exception.getResponse()).header(IDEMPOTENT_CACHE_HEADER, "true")
6364
.type(MediaType.APPLICATION_JSON).build();

‎fineract-core/src/main/java/org/apache/fineract/infrastructure/core/exceptionmapper/OptimisticLockExceptionMapper.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
*/
1919
package org.apache.fineract.infrastructure.core.exceptionmapper;
2020

21-
import static org.apache.http.HttpStatus.SC_LOCKED;
21+
import static org.apache.http.HttpStatus.SC_CONFLICT;
2222

2323
import jakarta.ws.rs.core.MediaType;
2424
import jakarta.ws.rs.core.Response;
@@ -46,12 +46,12 @@ public Response toResponse(final OptimisticLockException exception) {
4646
log.warn("Exception: {}, Message: {}", exception.getClass().getName(), exception.getMessage());
4747
String type = exception.getQuery() == null ? "unknown" : "query";
4848
String identifier = "unknown";
49-
final ApiGlobalErrorResponse dataIntegrityError = ApiGlobalErrorResponse.locked(type, identifier);
50-
return Response.status(SC_LOCKED).entity(dataIntegrityError).type(MediaType.APPLICATION_JSON).build();
49+
final ApiGlobalErrorResponse dataIntegrityError = ApiGlobalErrorResponse.conflict(type, identifier);
50+
return Response.status(SC_CONFLICT).entity(dataIntegrityError).type(MediaType.APPLICATION_JSON).build();
5151
}
5252

5353
@Override
5454
public int errorCode() {
55-
return 4009;
55+
return 4019;
5656
}
5757
}

‎fineract-provider/src/main/java/org/apache/fineract/portfolio/paymentdetail/service/PaymentDetailWritePlatformServiceJpaRepositoryImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public PaymentDetail createPaymentDetail(final JsonCommand command, final Map<St
5151
@Override
5252
@Transactional
5353
public PaymentDetail persistPaymentDetail(final PaymentDetail paymentDetail) {
54-
return this.paymentDetailRepository.save(paymentDetail);
54+
return this.paymentDetailRepository.saveAndFlush(paymentDetail);
5555
}
5656

5757
@Override

‎integration-tests/src/test/java/org/apache/fineract/integrationtests/SavingsAccountTransactionTest.java‎

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@
2020

2121
import static org.apache.fineract.integrationtests.common.savings.SavingsAccountHelper.PAYMENT_TYPE_ID;
2222
import static org.apache.fineract.integrationtests.common.system.DatatableHelper.addDatatableColumn;
23+
import static org.apache.http.HttpStatus.SC_CONFLICT;
2324
import static org.apache.http.HttpStatus.SC_FORBIDDEN;
24-
import static org.apache.http.HttpStatus.SC_LOCKED;
2525
import static org.apache.http.HttpStatus.SC_OK;
2626
import static org.hamcrest.Matchers.anyOf;
2727
import static org.hamcrest.Matchers.is;
@@ -31,6 +31,7 @@
3131
import static org.junit.jupiter.api.Assertions.assertTrue;
3232

3333
import com.fasterxml.jackson.core.JsonProcessingException;
34+
import com.google.common.base.Strings;
3435
import com.google.gson.Gson;
3536
import io.restassured.builder.RequestSpecBuilder;
3637
import io.restassured.builder.ResponseSpecBuilder;
@@ -85,6 +86,7 @@ public class SavingsAccountTransactionTest {
8586

8687
private ResponseSpecification responseSpec;
8788
private ResponseSpecification concurrentResponseSpec;
89+
private ResponseSpecification deadlockResponseSpec;
8890
private RequestSpecification requestSpec;
8991
private SavingsProductHelper savingsProductHelper;
9092
private SavingsAccountHelper savingsAccountHelper;
@@ -96,7 +98,8 @@ public void setup() {
9698
this.requestSpec = new RequestSpecBuilder().setContentType(ContentType.JSON).build();
9799
this.requestSpec.header("Authorization", "Basic " + Utils.loginIntoServerAndGetBase64EncodedAuthenticationKey());
98100
this.responseSpec = new ResponseSpecBuilder().expectStatusCode(SC_OK).build();
99-
this.concurrentResponseSpec = new ResponseSpecBuilder().expectStatusCode(anyOf(is(SC_OK), is(SC_LOCKED))).build();
101+
this.concurrentResponseSpec = new ResponseSpecBuilder().expectStatusCode(anyOf(is(SC_OK), is(SC_CONFLICT))).build();
102+
this.deadlockResponseSpec = new ResponseSpecBuilder().expectStatusCode(anyOf(is(SC_OK), is(SC_CONFLICT), is(SC_FORBIDDEN))).build();
100103
this.savingsAccountHelper = new SavingsAccountHelper(this.requestSpec, this.responseSpec);
101104
this.savingsProductHelper = new SavingsProductHelper();
102105
this.datatableHelper = new DatatableHelper(this.requestSpec, this.responseSpec);
@@ -190,7 +193,7 @@ public void testConcurrentSavingsBatchTransactions() {
190193

191194
SavingsAccountHelper batchWithTransactionHelper = new SavingsAccountHelper(requestSpec, concurrentResponseSpec);
192195
SavingsAccountHelper batchWithoutTransactionHelper = new SavingsAccountHelper(requestSpec,
193-
new ResponseSpecBuilder().expectStatusCode(anyOf(is(SC_OK), is(SC_LOCKED), is(SC_FORBIDDEN))).build());
196+
new ResponseSpecBuilder().expectStatusCode(anyOf(is(SC_OK), is(SC_CONFLICT), is(SC_FORBIDDEN))).build());
194197
String transactionDate = SavingsAccountHelper.TRANSACTION_DATE;
195198
String transactionAmount = "10";
196199
ExecutorService executor = Executors.newFixedThreadPool(30);
@@ -223,7 +226,7 @@ public void testConcurrentSavingsBatchTransactions() {
223226
log.info("\nFinished all threads");
224227
}
225228

226-
// @Test
229+
@Test
227230
public void testDeadlockSavingsBatchTransactions() {
228231
final Integer clientID = ClientHelper.createClient(requestSpec, responseSpec);
229232
ClientHelper.verifyClientCreatedOnServer(requestSpec, responseSpec, clientID);
@@ -239,7 +242,7 @@ public void testDeadlockSavingsBatchTransactions() {
239242
savingsAccountHelper.approveSavings(savingsId2);
240243
savingsAccountHelper.activateSavings(savingsId2);
241244

242-
SavingsAccountHelper batchWithTransactionHelper = new SavingsAccountHelper(requestSpec, concurrentResponseSpec);
245+
SavingsAccountHelper batchWithTransactionHelper = new SavingsAccountHelper(requestSpec, deadlockResponseSpec);
243246
String transactionDate = SavingsAccountHelper.TRANSACTION_DATE;
244247
String transactionAmount = "10";
245248

@@ -293,11 +296,12 @@ private void performSavingsTransaction(Integer savingsId, String amount, LocalDa
293296

294297
assertEquals(transactionId, (Integer) transaction.get("id"), "Check Savings " + transactionType + " Transaction");
295298
LocalDate transactionDateFromResponse = extractLocalDate(transaction, "date");
296-
assertTrue(DateUtils.isEqual(transactionDate, transactionDateFromResponse),
297-
"Transaction Date check for Savings " + transactionType + " Transaction");
298-
LocalDate submittedOnDate = extractLocalDate(transaction, "submittedOnDate");
299-
assertTrue(DateUtils.isEqual(submittedOnDate, Utils.getLocalDateOfTenant()),
300-
"Submitted On Date check for Savings " + transactionType + " Transaction");
299+
assertTrue(DateUtils.isEqual(transactionDate, transactionDateFromResponse), "Transaction Date check for Savings " + transactionType
300+
+ " Transaction. Expected: " + transactionDate + ", current: " + transactionDateFromResponse);
301+
LocalDate submittedOnDate = Utils.getLocalDateOfTenant();
302+
LocalDate submittedOnDateFromResponse = extractLocalDate(transaction, "submittedOnDate");
303+
assertTrue(DateUtils.isEqual(submittedOnDate, submittedOnDateFromResponse), "Submitted On Date check for Savings " + transactionType
304+
+ " Transaction. Expected: " + submittedOnDate + ", current: " + submittedOnDateFromResponse);
301305
}
302306

303307
private LocalDate extractLocalDate(HashMap transactionMap, String fieldName) {
@@ -382,7 +386,7 @@ public void run() {
382386
if (enclosingTransaction) {
383387
Integer statusCode1 = responses.get(0).getStatusCode();
384388
assertNotNull(statusCode1);
385-
assertTrue(SC_OK == statusCode1 || SC_LOCKED == statusCode1);
389+
assertTrue(SC_OK == statusCode1 || SC_CONFLICT == statusCode1, "Status code: " + statusCode1);
386390
if (SC_OK == statusCode1) {
387391
assertEquals(4, responses.size());
388392
Integer statusCode4 = responses.get(3).getStatusCode();
@@ -395,11 +399,11 @@ public void run() {
395399
assertEquals(4, responses.size());
396400
Integer statusCode1 = responses.get(0).getStatusCode();
397401
assertNotNull(statusCode1);
398-
assertTrue(SC_OK == statusCode1 || SC_LOCKED == statusCode1);
402+
assertTrue(SC_OK == statusCode1 || SC_CONFLICT == statusCode1, "Status code: " + statusCode1);
399403
Integer statusCode4 = responses.get(3).getStatusCode();
400404
assertNotNull(statusCode4);
401-
assertTrue(SC_OK == statusCode1 ? (SC_OK == statusCode4 || SC_LOCKED == statusCode4)
402-
: (SC_FORBIDDEN == statusCode4 || SC_LOCKED == statusCode4));
405+
assertTrue(SC_OK == statusCode1 ? (SC_OK == statusCode4 || SC_CONFLICT == statusCode4)
406+
: (SC_FORBIDDEN == statusCode4 || SC_CONFLICT == statusCode4), "Status code: " + statusCode4);
403407
}
404408
} else {
405409
String json = transactionData.getJson();
@@ -415,12 +419,12 @@ public void run() {
415419
private static boolean checkConcurrentResponse(String response) {
416420
assertNotNull(response);
417421
JsonPath res = JsonPath.from(response);
418-
String statusCode = (String) res.get("httpStatusCode");
422+
String statusCode = res.get("httpStatusCode");
419423
if (statusCode == null) {
420424
assertNotNull(res.get(CommonConstants.RESPONSE_RESOURCE_ID));
421425
return true;
422426
}
423-
assertEquals(String.valueOf(SC_LOCKED), statusCode);
427+
assertEquals(String.valueOf(SC_CONFLICT), statusCode);
424428
return false;
425429
}
426430
}
@@ -436,9 +440,14 @@ private void runDeadlockBatch(SavingsAccountHelper savingsHelper, Integer saving
436440
ResponseSpecification responseSpec = savingsHelper.getResponseSpec();
437441
final List<BatchResponse> responses = BatchHelper.postBatchRequestsWithEnclosingTransaction(requestSpec, responseSpec, json);
438442
assertNotNull(responses);
439-
Integer statusCode = responses.get(0).getStatusCode();
443+
BatchResponse response1 = responses.get(0);
444+
Integer statusCode = response1.getStatusCode();
445+
String msg = Strings.nullToEmpty(response1.getBody());
440446
assertNotNull(statusCode);
441-
assertTrue(SC_OK == statusCode || SC_LOCKED == statusCode);
447+
assertTrue(
448+
SC_OK == statusCode || SC_CONFLICT == statusCode
449+
|| (SC_FORBIDDEN == statusCode && msg.contains("Cannot add or update a child row")),
450+
"Status code: " + statusCode + ", message: " + msg);
442451
if (SC_OK == statusCode) {
443452
assertEquals(4, responses.size());
444453
Integer statusCode4 = responses.get(3).getStatusCode();

0 commit comments

Comments
 (0)