FINERACT-1921: BatchAPI - status code mismatch

This commit is contained in:
jmarta
2023-10-03 12:37:22 +02:00
committed by Arnold Gálovics
parent 666c097ec8
commit 80bcb27b0a
16 changed files with 93 additions and 83 deletions
@@ -76,10 +76,14 @@ public final class ErrorHandler {
Set<String> exceptionMappers = createSet(ctx.getBeanNamesForType(forClassWithGenerics(ExceptionMapper.class, clazz)));
Set<String> fineractErrorMappers = createSet(ctx.getBeanNamesForType(FineractExceptionMapper.class));
SetUtils.SetView<String> intersection = SetUtils.intersection(exceptionMappers, fineractErrorMappers);
if (intersection.size() > 0) {
if (!intersection.isEmpty()) {
// noinspection unchecked
return (ExceptionMapper<T>) ctx.getBean(intersection.iterator().next());
}
if (!exceptionMappers.isEmpty()) {
// noinspection unchecked
return (ExceptionMapper<T>) ctx.getBean(exceptionMappers.iterator().next());
}
clazz = clazz.getSuperclass();
} while (!clazz.equals(Exception.class));
// noinspection unchecked
@@ -98,8 +102,10 @@ public final class ErrorHandler {
MultivaluedMap<String, Object> headers = response.getHeaders();
Set<Header> batchHeaders = headers == null ? null
: headers.keySet().stream().map(e -> new Header(e, response.getHeaderString(e))).collect(Collectors.toSet());
return new ErrorInfo(response.getStatus(), ((FineractExceptionMapper) exceptionMapper).errorCode(),
JSON_HELPER.toJson(response.getEntity()), batchHeaders);
Integer errorCode = exceptionMapper instanceof FineractExceptionMapper ? ((FineractExceptionMapper) exceptionMapper).errorCode()
: null;
Object msg = response.getEntity();
return new ErrorInfo(response.getStatus(), errorCode, msg instanceof String ? (String) msg : JSON_HELPER.toJson(msg), batchHeaders);
}
public RuntimeException getMappable(@NotNull Throwable thr) {
@@ -223,7 +223,7 @@ public class BatchApiServiceImpl implements BatchApiService {
callRequestRecursive(resolvedChildRequest, childNode, responseList, uriInfo, enclosingTransaction);
});
} else {
responseList.addAll(parentRequestFailedRecursive(request, requestNode, response, true));
responseList.addAll(parentRequestFailedRecursive(request, requestNode, response, null));
}
// If the current request fails, then all the child requests are not executed. If we want to write out all the
// child requests, here is the place.
@@ -296,16 +296,17 @@ public class BatchApiServiceImpl implements BatchApiService {
* the current request node
* @return {@code BatchResponse} list of the generated batch responses
*/
private List<BatchResponse> parentRequestFailedRecursive(BatchRequest request, BatchRequestNode requestNode, BatchResponse response,
boolean root) {
private List<BatchResponse> parentRequestFailedRecursive(@NotNull BatchRequest request, @NotNull BatchRequestNode requestNode,
@NotNull BatchResponse response, Long parentId) {
List<BatchResponse> responseList = new ArrayList<>();
if (root) {
if (parentId == null) { // root
BatchRequestContextHolder.getEnclosingTransaction().ifPresent(TransactionExecution::setRollbackOnly);
} else {
responseList.add(buildErrorResponse(request.getRequestId(), response.getStatusCode(), response.getBody(), null));
responseList.add(buildErrorResponse(request.getRequestId(), response.getStatusCode(),
"Parent request with id " + parentId + " was erroneous!", null));
}
requestNode.getChildNodes().forEach(
childNode -> responseList.addAll(parentRequestFailedRecursive(childNode.getRequest(), childNode, response, false)));
requestNode.getChildNodes().forEach(childNode -> responseList
.addAll(parentRequestFailedRecursive(childNode.getRequest(), childNode, response, request.getRequestId())));
return responseList;
}
@@ -18,6 +18,9 @@
*/
package org.apache.fineract.commands.domain;
import java.util.Arrays;
import java.util.Map;
import java.util.stream.Collectors;
import lombok.Getter;
import lombok.RequiredArgsConstructor;
@@ -32,6 +35,14 @@ public enum CommandProcessingResultType {
UNDER_PROCESSING(4, "commandProcessingResultType.underProcessing"), //
ERROR(5, "commandProcessingResultType.error");
private static final Map<Integer, CommandProcessingResultType> BY_ID = Arrays.stream(values())
.collect(Collectors.toMap(CommandProcessingResultType::getValue, v -> v));
private final Integer value;
private final String code;
public static CommandProcessingResultType fromInt(final Integer value) {
CommandProcessingResultType transactionType = BY_ID.get(value);
return transactionType == null ? INVALID : transactionType;
}
}
@@ -737,7 +737,6 @@ public class CommandWrapperBuilder {
}
private void commonDatatableSettings(final String datatable, final Long apptableId, final Long datatableId) {
this.entityName = datatable;
this.entityId = apptableId;
this.subentityId = datatableId;
@@ -30,7 +30,6 @@ import java.lang.reflect.Type;
import java.time.Instant;
import java.util.HashMap;
import java.util.Map;
import java.util.function.Function;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.apache.fineract.batch.exception.ErrorHandler;
@@ -48,7 +47,6 @@ import org.apache.fineract.infrastructure.core.data.CommandProcessingResult;
import org.apache.fineract.infrastructure.core.data.CommandProcessingResultBuilder;
import org.apache.fineract.infrastructure.core.domain.BatchRequestContextHolder;
import org.apache.fineract.infrastructure.core.domain.FineractRequestContextHolder;
import org.apache.fineract.infrastructure.core.exception.AbstractIdempotentCommandException;
import org.apache.fineract.infrastructure.core.exception.IdempotentCommandProcessFailedException;
import org.apache.fineract.infrastructure.core.exception.IdempotentCommandProcessSucceedException;
import org.apache.fineract.infrastructure.core.exception.IdempotentCommandProcessUnderProcessingException;
@@ -174,23 +172,21 @@ public class SynchronousCommandProcessingService implements CommandProcessingSer
}
private void exceptionWhenTheRequestAlreadyProcessed(CommandWrapper wrapper, String idempotencyKey, boolean retry) {
CommandSource existingCommand = commandSourceService.findCommandSource(wrapper, idempotencyKey);
if (existingCommand != null) {
idempotentExceptionByStatus(UNDER_PROCESSING, existingCommand,
command -> new IdempotentCommandProcessUnderProcessingException(wrapper));
if (!retry) {
idempotentExceptionByStatus(ERROR, existingCommand,
command -> new IdempotentCommandProcessFailedException(wrapper, command));
}
idempotentExceptionByStatus(PROCESSED, existingCommand,
command -> new IdempotentCommandProcessSucceedException(wrapper, command.getResult(), command.getResultStatusCode()));
CommandSource command = commandSourceService.findCommandSource(wrapper, idempotencyKey);
if (command == null) {
return;
}
}
private void idempotentExceptionByStatus(CommandProcessingResultType status, CommandSource command,
Function<CommandSource, AbstractIdempotentCommandException> exceptionMapper) {
if (status.getValue().equals(command.getStatus())) {
throw exceptionMapper.apply(command);
CommandProcessingResultType status = CommandProcessingResultType.fromInt(command.getStatus());
switch (status) {
case UNDER_PROCESSING -> throw new IdempotentCommandProcessUnderProcessingException(wrapper, idempotencyKey);
case PROCESSED -> throw new IdempotentCommandProcessSucceedException(wrapper, idempotencyKey, command);
case ERROR -> {
if (!retry) {
throw new IdempotentCommandProcessFailedException(wrapper, idempotencyKey, command);
}
}
default -> {
}
}
}
@@ -23,6 +23,7 @@ import static org.apache.http.HttpStatus.SC_BAD_REQUEST;
import static org.apache.http.HttpStatus.SC_CONFLICT;
import static org.apache.http.HttpStatus.SC_FORBIDDEN;
import static org.apache.http.HttpStatus.SC_INTERNAL_SERVER_ERROR;
import static org.apache.http.HttpStatus.SC_LOCKED;
import static org.apache.http.HttpStatus.SC_METHOD_NOT_ALLOWED;
import static org.apache.http.HttpStatus.SC_NOT_FOUND;
import static org.apache.http.HttpStatus.SC_SERVICE_UNAVAILABLE;
@@ -106,7 +107,7 @@ public class ApiGlobalErrorResponse {
return create(SC_CONFLICT, "error.msg.loan.locked", msg, msg);
}
public static ApiGlobalErrorResponse conflict(String type, String identifier) {
public static ApiGlobalErrorResponse locked(String type, String identifier) {
String details = "";
if (type == null) {
type = "unknown";
@@ -117,7 +118,7 @@ public class ApiGlobalErrorResponse {
details += " [" + identifier + ']';
}
String msg = "The server is currently unable to handle the request due to concurrent modification" + details + ", please try again";
return create(SC_CONFLICT, "error.msg.platform.service." + type + ".conflict", msg, msg);
return create(SC_LOCKED, "error.msg.platform.service." + type + ".conflict", msg, msg);
}
public static ApiGlobalErrorResponse unAuthorized(final String defaultUserMessage) {
@@ -18,6 +18,9 @@
*/
package org.apache.fineract.infrastructure.core.exception;
import static org.apache.http.HttpStatus.SC_INTERNAL_SERVER_ERROR;
import jakarta.validation.constraints.NotNull;
import org.apache.fineract.commands.domain.CommandSource;
import org.apache.fineract.commands.domain.CommandWrapper;
@@ -28,16 +31,14 @@ public class IdempotentCommandProcessFailedException extends AbstractIdempotentC
private final Integer statusCode;
public IdempotentCommandProcessFailedException(CommandWrapper wrapper, CommandSource commandSource) {
super(wrapper.actionName(), wrapper.entityName(), wrapper.getIdempotencyKey(), commandSource.getResult());
this.statusCode = commandSource.getResultStatusCode();
public IdempotentCommandProcessFailedException(CommandWrapper wrapper, String idempotencyKey, CommandSource command) {
super(wrapper.actionName(), wrapper.actionName(), idempotencyKey, command.getResult());
this.statusCode = command.getResultStatusCode();
}
@NotNull
public Integer getStatusCode() {
// If the database inconsistent we return http 500 instead of null pointer exception
if (statusCode == null) {
return 500;
}
return statusCode;
return statusCode == null ? SC_INTERNAL_SERVER_ERROR : statusCode;
}
}
@@ -18,6 +18,7 @@
*/
package org.apache.fineract.infrastructure.core.exception;
import org.apache.fineract.commands.domain.CommandSource;
import org.apache.fineract.commands.domain.CommandWrapper;
/**
@@ -27,9 +28,9 @@ public class IdempotentCommandProcessSucceedException extends AbstractIdempotent
private final Integer statusCode;
public IdempotentCommandProcessSucceedException(CommandWrapper wrapper, String response, Integer statusCode) {
super(wrapper.actionName(), wrapper.entityName(), wrapper.getIdempotencyKey(), response);
this.statusCode = statusCode;
public IdempotentCommandProcessSucceedException(CommandWrapper wrapper, String idempotencyKey, CommandSource command) {
super(wrapper.actionName(), wrapper.entityName(), idempotencyKey, command.getResult());
this.statusCode = command.getResultStatusCode();
}
public Integer getStatusCode() {
@@ -25,7 +25,7 @@ import org.apache.fineract.commands.domain.CommandWrapper;
*/
public class IdempotentCommandProcessUnderProcessingException extends AbstractIdempotentCommandException {
public IdempotentCommandProcessUnderProcessingException(CommandWrapper wrapper) {
super(wrapper.actionName(), wrapper.entityName(), wrapper.getIdempotencyKey(), wrapper.getJson());
public IdempotentCommandProcessUnderProcessingException(CommandWrapper wrapper, String idempotencyKey) {
super(wrapper.actionName(), wrapper.entityName(), idempotencyKey, wrapper.getJson());
}
}
@@ -18,9 +18,10 @@
*/
package org.apache.fineract.infrastructure.core.exceptionmapper;
import static org.apache.http.HttpStatus.SC_LOCKED;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.Response.Status;
import jakarta.ws.rs.ext.ExceptionMapper;
import jakarta.ws.rs.ext.Provider;
import lombok.extern.slf4j.Slf4j;
@@ -49,8 +50,8 @@ public class ConcurrencyFailureExceptionMapper implements FineractExceptionMappe
type = olex.getPersistentClassName();
identifier = olex.getIdentifier() == null ? null : String.valueOf(olex.getIdentifier());
}
final ApiGlobalErrorResponse dataIntegrityError = ApiGlobalErrorResponse.conflict(type, identifier);
return Response.status(Status.CONFLICT).entity(dataIntegrityError).type(MediaType.APPLICATION_JSON).build();
final ApiGlobalErrorResponse dataIntegrityError = ApiGlobalErrorResponse.locked(type, identifier);
return Response.status(SC_LOCKED).entity(dataIntegrityError).type(MediaType.APPLICATION_JSON).build();
}
@Override
@@ -48,8 +48,10 @@ public class IdempotentCommandExceptionMapper implements FineractExceptionMapper
log.warn("Processing {} request: {}", exception.getClass().getName(), exception.getMessage());
Status status = null;
if (exception instanceof IdempotentCommandProcessSucceedException pse) {
status = Status.fromStatusCode(pse.getStatusCode());
} else if (exception instanceof IdempotentCommandProcessUnderProcessingException) {
Integer statusCode = pse.getStatusCode();
status = statusCode == null ? Status.OK : Status.fromStatusCode(statusCode);
}
if (exception instanceof IdempotentCommandProcessUnderProcessingException) {
status = Status.CONFLICT;
} else if (exception instanceof IdempotentCommandProcessFailedException pfe) {
status = Status.fromStatusCode(pfe.getStatusCode());
@@ -18,9 +18,10 @@
*/
package org.apache.fineract.infrastructure.core.exceptionmapper;
import static org.apache.http.HttpStatus.SC_LOCKED;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import jakarta.ws.rs.core.Response.Status;
import jakarta.ws.rs.ext.ExceptionMapper;
import jakarta.ws.rs.ext.Provider;
import lombok.extern.slf4j.Slf4j;
@@ -45,8 +46,8 @@ public class OptimisticLockExceptionMapper implements FineractExceptionMapper, E
log.warn("Exception: {}, Message: {}", exception.getClass().getName(), exception.getMessage());
String type = exception.getQuery() == null ? "unknown" : "query";
String identifier = "unknown";
final ApiGlobalErrorResponse dataIntegrityError = ApiGlobalErrorResponse.conflict(type, identifier);
return Response.status(Status.CONFLICT).entity(dataIntegrityError).type(MediaType.APPLICATION_JSON).build();
final ApiGlobalErrorResponse dataIntegrityError = ApiGlobalErrorResponse.locked(type, identifier);
return Response.status(SC_LOCKED).entity(dataIntegrityError).type(MediaType.APPLICATION_JSON).build();
}
@Override
@@ -54,7 +54,7 @@ public class IdempotencyCommandProcessFailedExceptionTest {
CommandWrapper command = new CommandWrapper(null, null, null, null, null, null, null, null, null, null, null, null, null, null,
null, null, null, null);
CommandSource source = CommandSource.fullEntryFrom(command, JsonCommand.from("{}"), null, "dummy-key", null);
IdempotentCommandProcessFailedException exception = new IdempotentCommandProcessFailedException(command, source);
IdempotentCommandProcessFailedException exception = new IdempotentCommandProcessFailedException(command, null, source);
Response result = mapper.toResponse(exception);
assertEquals(500, result.getStatus());
assertEquals("true", result.getHeaderString(IDEMPOTENT_CACHE_HEADER));
@@ -48,27 +48,17 @@ public class CreateDatatableEntryCommandStrategy implements CommandStrategy {
@Override
public BatchResponse execute(BatchRequest request, @SuppressWarnings("unused") UriInfo uriInfo) {
final BatchResponse response = new BatchResponse();
final String responseBody;
response.setRequestId(request.getRequestId());
response.setHeaders(request.getHeaders());
final List<String> pathParameters = Splitter.on('/').splitToList(relativeUrlWithoutVersion(request));
// Pluck out the datatable name & loanId out of the relative path
final String datatableName = pathParameters.get(1);
final Long loanId = Long.parseLong(pathParameters.get(2));
// Calls 'createDatatableEntry' function from
// 'DatatablesApiResource' to create a datatable entry on an existing loan
responseBody = datatablesApiResource.createDatatableEntry(datatableName, loanId, request.getBody());
// Calls 'createDatatableEntry' function from 'DatatablesApiResource' to create a datatable entry on an existing
// loan
final String responseBody = datatablesApiResource.createDatatableEntry(datatableName, loanId, request.getBody());
response.setStatusCode(HttpStatus.SC_OK);
// Sets the body of the response after datatable entry is successfully
// created
response.setBody(responseBody);
return response;
// Create the response after datatable entry is successfully created
return new BatchResponse().setRequestId(request.getRequestId()).setStatusCode(HttpStatus.SC_OK).setBody(responseBody)
.setHeaders(request.getHeaders());
}
}
@@ -1587,13 +1587,13 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ
if (!rs.next()) {
throw new DatatableNotFoundException(entityTable, appTableId);
}
final Long officeId = rs.getLong("officeId");
final Long groupId = rs.getLong("groupId");
final Long clientId = rs.getLong("clientId");
final Long savingsId = rs.getLong("savingsId");
final Long loanId = rs.getLong("loanId");
final Long transactionId = rs.getLong("transactionId");
final Long entityId = rs.getLong("entityId");
final Long officeId = (Long) rs.getObject("officeId");
final Long groupId = (Long) rs.getObject("groupId");
final Long clientId = (Long) rs.getObject("clientId");
final Long savingsId = (Long) rs.getObject("savingsId");
final Long loanId = (Long) rs.getObject("loanId");
final Long transactionId = (Long) rs.getObject("transactionId");
final Long entityId = (Long) rs.getObject("entityId");
if (rs.next()) {
throw new DatatableSystemErrorException("System Error: More than one row returned from data scoping query");
@@ -1604,7 +1604,7 @@ public class ReadWriteNonCoreDataServiceImpl implements ReadWriteNonCoreDataServ
.withGroupId(groupId) //
.withClientId(clientId) //
.withSavingsId(savingsId) //
.withLoanId(loanId).withTransactionId(String.valueOf(transactionId)).withEntityId(entityId)//
.withLoanId(loanId).withTransactionId(transactionId == null ? null : String.valueOf(transactionId)).withEntityId(entityId)//
.build();
}
@@ -20,8 +20,8 @@ package org.apache.fineract.integrationtests;
import static org.apache.fineract.integrationtests.common.savings.SavingsAccountHelper.PAYMENT_TYPE_ID;
import static org.apache.fineract.integrationtests.common.system.DatatableHelper.addDatatableColumn;
import static org.apache.http.HttpStatus.SC_CONFLICT;
import static org.apache.http.HttpStatus.SC_FORBIDDEN;
import static org.apache.http.HttpStatus.SC_LOCKED;
import static org.apache.http.HttpStatus.SC_OK;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.is;
@@ -95,7 +95,7 @@ public class SavingsAccountTransactionTest {
this.requestSpec = new RequestSpecBuilder().setContentType(ContentType.JSON).build();
this.requestSpec.header("Authorization", "Basic " + Utils.loginIntoServerAndGetBase64EncodedAuthenticationKey());
this.responseSpec = new ResponseSpecBuilder().expectStatusCode(SC_OK).build();
this.concurrentResponseSpec = new ResponseSpecBuilder().expectStatusCode(anyOf(is(SC_OK), is(SC_CONFLICT))).build();
this.concurrentResponseSpec = new ResponseSpecBuilder().expectStatusCode(anyOf(is(SC_OK), is(SC_LOCKED))).build();
this.savingsAccountHelper = new SavingsAccountHelper(this.requestSpec, this.responseSpec);
this.savingsProductHelper = new SavingsProductHelper();
this.datatableHelper = new DatatableHelper(this.requestSpec, this.responseSpec);
@@ -189,7 +189,7 @@ public class SavingsAccountTransactionTest {
SavingsAccountHelper batchWithTransactionHelper = new SavingsAccountHelper(requestSpec, concurrentResponseSpec);
SavingsAccountHelper batchWithoutTransactionHelper = new SavingsAccountHelper(requestSpec,
new ResponseSpecBuilder().expectStatusCode(anyOf(is(SC_OK), is(SC_CONFLICT), is(SC_FORBIDDEN))).build());
new ResponseSpecBuilder().expectStatusCode(anyOf(is(SC_OK), is(SC_LOCKED), is(SC_FORBIDDEN))).build());
String transactionDate = SavingsAccountHelper.TRANSACTION_DATE;
String transactionAmount = "10";
ExecutorService executor = Executors.newFixedThreadPool(30);
@@ -322,7 +322,7 @@ public class SavingsAccountTransactionTest {
if (enclosingTransaction) {
Integer statusCode1 = responses.get(0).getStatusCode();
assertNotNull(statusCode1);
assertTrue(SC_OK == statusCode1 || SC_CONFLICT == statusCode1);
assertTrue(SC_OK == statusCode1 || SC_LOCKED == statusCode1);
if (SC_OK == statusCode1) {
assertEquals(4, responses.size());
Integer statusCode4 = responses.get(3).getStatusCode();
@@ -335,10 +335,10 @@ public class SavingsAccountTransactionTest {
assertEquals(4, responses.size());
Integer statusCode1 = responses.get(0).getStatusCode();
assertNotNull(statusCode1);
assertTrue(SC_OK == statusCode1 || SC_CONFLICT == statusCode1);
assertTrue(SC_OK == statusCode1 || SC_LOCKED == statusCode1);
Integer statusCode4 = responses.get(3).getStatusCode();
assertNotNull(statusCode4);
assertTrue(SC_OK == statusCode1 ? (SC_OK == statusCode4 || SC_CONFLICT == statusCode4) : SC_FORBIDDEN == statusCode4);
assertTrue(SC_OK == statusCode1 ? (SC_OK == statusCode4 || SC_LOCKED == statusCode4) : SC_FORBIDDEN == statusCode4);
}
} else {
String json = transactionData.getJson();
@@ -359,7 +359,7 @@ public class SavingsAccountTransactionTest {
assertNotNull(res.get(CommonConstants.RESPONSE_RESOURCE_ID));
return true;
}
assertEquals(String.valueOf(SC_CONFLICT), statusCode);
assertEquals(String.valueOf(SC_LOCKED), statusCode);
return false;
}
}