Skip to content

Commit

Permalink
[MODFISTO-460] Use the same HttpException for consistency
Browse files Browse the repository at this point in the history
  • Loading branch information
damien-git committed Apr 19, 2024
1 parent b3d4b52 commit be59cf7
Show file tree
Hide file tree
Showing 23 changed files with 76 additions and 74 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import javax.ws.rs.core.Response;

import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;
import io.vertx.pgclient.PgException;
import org.folio.rest.persist.HelperUtils;
import org.folio.rest.persist.PgExceptionUtil;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/folio/dao/budget/BudgetPostgresDAO.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

import io.vertx.core.Future;
import io.vertx.core.json.JsonObject;
import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;
import io.vertx.sqlclient.Tuple;

public class BudgetPostgresDAO implements BudgetDAO {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import io.vertx.core.Future;
import javax.ws.rs.core.Response;

import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.folio.rest.jaxrs.model.FiscalYear;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/folio/dao/fund/FundPostgresDAO.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
import java.util.Collections;
import java.util.List;

import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.folio.rest.jaxrs.model.Fund;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/folio/dao/ledger/LedgerPostgresDAO.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import javax.ws.rs.core.Response;

import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.folio.rest.jaxrs.model.Ledger;
Expand Down
32 changes: 27 additions & 5 deletions src/main/java/org/folio/rest/exception/HttpException.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package org.folio.rest.exception;

import java.util.Collections;

import org.apache.commons.lang3.StringUtils;
import org.folio.rest.jaxrs.model.Error;
import org.folio.rest.jaxrs.model.Errors;
import org.folio.rest.jaxrs.model.Parameter;
import org.folio.rest.util.ErrorCodes;

import static java.util.Collections.singletonList;

public class HttpException extends RuntimeException {
private static final long serialVersionUID = 8109197948434861504L;

Expand All @@ -17,14 +18,35 @@ public HttpException(int code, String message) {
super(StringUtils.isNotEmpty(message) ? message : ErrorCodes.GENERIC_ERROR_CODE.getDescription());
this.code = code;
this.errors = new Errors()
.withErrors(Collections.singletonList(new Error().withCode(ErrorCodes.GENERIC_ERROR_CODE.getCode()).withMessage(message)))
.withErrors(singletonList(new Error().withCode(ErrorCodes.GENERIC_ERROR_CODE.getCode()).withMessage(message)))
.withTotalRecords(1);
}

public HttpException(int code, String message, Throwable cause) {
super(message, cause);
this.code = code;
Parameter causeParam = new Parameter().withKey("cause").withValue(cause.getMessage());
Error error = new Error()
.withCode(ErrorCodes.GENERIC_ERROR_CODE.getCode())
.withMessage(message)
.withParameters(singletonList(causeParam));
this.errors = new Errors()
.withErrors(singletonList(error))
.withTotalRecords(1);
}

public HttpException(int code, Throwable cause) {
super(cause.getMessage(), cause);
this.code = code;
this.errors = new Errors()
.withErrors(singletonList(new Error().withCode(ErrorCodes.GENERIC_ERROR_CODE.getCode()).withMessage(cause.getMessage())))
.withTotalRecords(1);
}

public HttpException(int code, ErrorCodes errCodes) {
super(errCodes.getDescription());
this.errors = new Errors()
.withErrors(Collections.singletonList(new Error().withCode(errCodes.getCode()).withMessage(errCodes.getDescription())))
.withErrors(singletonList(new Error().withCode(errCodes.getCode()).withMessage(errCodes.getDescription())))
.withTotalRecords(1);
this.code = code;
}
Expand All @@ -33,7 +55,7 @@ public HttpException(int code, Error error) {
super(error.getMessage());
this.code = code;
this.errors = new Errors()
.withErrors(Collections.singletonList(error))
.withErrors(singletonList(error))
.withTotalRecords(1);
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/folio/rest/impl/FundAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

import javax.ws.rs.core.Response;

import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;
import org.apache.logging.log4j.LogManager;
import org.folio.rest.annotations.Validate;
import org.folio.rest.jaxrs.model.Fund;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/folio/rest/impl/LedgerAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

import javax.ws.rs.core.Response;

import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/folio/rest/impl/LedgerRolloverAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

import javax.ws.rs.core.Response;

import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.folio.rest.annotations.Validate;
Expand Down Expand Up @@ -61,7 +61,7 @@ public void postFinanceStorageLedgerRollovers(LedgerFiscalYearRollover entity, M
t = new HttpException(Response.Status.INTERNAL_SERVER_ERROR.getStatusCode(), t);
}
HttpException cause = (HttpException)t;
if (Response.Status.CONFLICT.getStatusCode() == cause.getStatusCode()) {
if (Response.Status.CONFLICT.getStatusCode() == cause.getCode()) {
logger.error("Rollover already exists with id {}", entity.getId(), cause);
} else {
logger.error("Creating rollover with id {} failed", entity.getId(), cause);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/folio/rest/persist/DBConn.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import io.vertx.core.Future;
import io.vertx.core.json.JsonObject;
import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;
import io.vertx.sqlclient.Row;
import io.vertx.sqlclient.RowSet;
import io.vertx.sqlclient.Tuple;
Expand Down
25 changes: 3 additions & 22 deletions src/main/java/org/folio/rest/persist/HelperUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,12 @@

import java.lang.reflect.Method;
import java.text.MessageFormat;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.function.Function;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

import javax.ws.rs.Path;
import javax.ws.rs.core.MediaType;
Expand All @@ -30,8 +28,7 @@
import io.vertx.core.AsyncResult;
import io.vertx.core.Context;
import io.vertx.core.Handler;
import io.vertx.core.json.JsonObject;
import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;

public final class HelperUtils {
private HelperUtils() { }
Expand All @@ -43,8 +40,8 @@ public static String getEndpoint(Class<?> clazz) {
}

public static void replyWithErrorResponse(Handler<AsyncResult<Response>> asyncResultHandler, HttpException cause) {
asyncResultHandler.handle(succeededFuture(Response.status(cause.getStatusCode())
.entity(Optional.of(cause).map(HttpException::getPayload).orElse(cause.getMessage()))
asyncResultHandler.handle(succeededFuture(Response.status(cause.getCode())
.entity(Optional.of(cause).map(HttpException::getMessage).orElse(cause.getMessage()))
.header(CONTENT_TYPE, MediaType.TEXT_PLAIN)
.build()));
}
Expand Down Expand Up @@ -132,22 +129,6 @@ public static Error buildFieldConstraintError(String entityName, String fieldNam
return error;
}

public static String getQueryValues(List<JsonObject> entities) {
return entities.stream().map(entity -> "('" + entity.getString("id") + "', '" + entity.encode() + "'::json)").collect(Collectors.joining(","));
}

public static List<Error> buildNullValidationError(String value, String key) {
if (value == null) {
Parameter parameter = new Parameter().withKey(key)
.withValue("null");
Error error = new Error().withCode("-1")
.withMessage("may not be null")
.withParameters(Collections.singletonList(parameter));
return Collections.singletonList(error);
}
return Collections.emptyList();
}

/**
* The method allows to compose any elements with the same action in sequence.
*
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/org/folio/rest/util/ErrorCodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ public enum ErrorCodes {
CONFLICT("conflict", "Conflict when updating a record in table {0}: {1}"),
BUDGET_RESTRICTED_EXPENDITURES_ERROR("budgetRestrictedExpendituresError", "Expenditure restriction does not allow this operation"),
BUDGET_RESTRICTED_ENCUMBRANCE_ERROR("budgetRestrictedEncumbranceError", "Encumbrance restriction does not allow this operation"),
PAYMENT_OR_CREDIT_HAS_NEGATIVE_AMOUNT("paymentOrCreditHasNegativeAmount", "A payment or credit has a negative amount");
PAYMENT_OR_CREDIT_HAS_NEGATIVE_AMOUNT("paymentOrCreditHasNegativeAmount", "A payment or credit has a negative amount"),
TRANSACTION_IS_PRESENT_BUDGET_DELETE_ERROR("transactionIsPresentBudgetDeleteError", "Budget related transactions found. Deletion of the budget is forbidden.");
private final String code;
private final String description;

Expand Down
25 changes: 11 additions & 14 deletions src/main/java/org/folio/rest/util/ResponseUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import io.vertx.core.Future;
import io.vertx.core.Handler;
import io.vertx.core.Promise;
import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;

public class ResponseUtils {

Expand All @@ -32,9 +32,8 @@ private ResponseUtils() {

public static Future<Response> handleNoContentResponse(AsyncResult<Void> result, String id, String logMessage) {
if (result.failed()) {
HttpException cause = (HttpException) result.cause();
logger.error(logMessage, cause, id, "or associated data failed to be");
return buildErrorResponse(cause);
logger.error(logMessage, result.cause(), id, "or associated data failed to be");
return buildErrorResponse(result.cause());
} else {
logger.info(logMessage, id, "and associated data were successfully");
return buildNoContentResponse();
Expand All @@ -54,9 +53,8 @@ public static <T> Future<T> handleFailure(Throwable cause) {
public static <T, V> void handleFailure(Promise<T> promise, AsyncResult<V> reply) {
Throwable cause = reply.cause();
logger.error(cause.getMessage());
if (cause instanceof PgException && "23F09".equals(((PgException)cause).getCode())) {
String message = MessageFormat.format(ErrorCodes.CONFLICT.getDescription(), ((PgException)cause).getTable(),
((PgException)cause).getErrorMessage());
if (cause instanceof PgException pgEx && "23F09".equals(pgEx.getSqlState())) {
String message = MessageFormat.format(ErrorCodes.CONFLICT.getDescription(), pgEx.getTable(), pgEx.getErrorMessage());
promise.fail(new HttpException(Response.Status.CONFLICT.getStatusCode(), message, cause));
return;
}
Expand All @@ -76,12 +74,11 @@ public static Future<Response> buildErrorResponse(Throwable throwable) {
final String message;
final int code;

if (throwable instanceof HttpException) {
code = ((HttpException) throwable).getStatusCode();
message = ((HttpException) throwable).getPayload();
} else if (throwable instanceof org.folio.rest.exception.HttpException) {
org.folio.rest.exception.HttpException exception = (org.folio.rest.exception.HttpException) throwable;
return Future.succeededFuture(buildErrorResponse(exception));
if (throwable instanceof io.vertx.ext.web.handler.HttpException exception) {
code = exception.getStatusCode();
message = exception.getPayload();
} else if (throwable instanceof org.folio.rest.exception.HttpException exception) {
return Future.succeededFuture(buildErrorResponseFromHttpException(exception));
} else {
code = INTERNAL_SERVER_ERROR.getStatusCode();
message = throwable.getMessage();
Expand All @@ -97,7 +94,7 @@ private static Response buildErrorResponse(int code, String message) {
.build();
}

private static Response buildErrorResponse(org.folio.rest.exception.HttpException exception) {
private static Response buildErrorResponseFromHttpException(HttpException exception) {
return Response.status(exception.getCode())
.header(CONTENT_TYPE, APPLICATION_JSON)
.entity(exception.getErrors())
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/org/folio/service/budget/BudgetService.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.folio.rest.impl.BudgetAPI.BUDGET_TABLE;
import static org.folio.rest.impl.FundAPI.FUND_TABLE;
import static org.folio.rest.persist.HelperUtils.getFullTableName;
import static org.folio.rest.util.ErrorCodes.TRANSACTION_IS_PRESENT_BUDGET_DELETE_ERROR;
import static org.folio.rest.util.ResponseUtils.handleNoContentResponse;

import java.util.*;
Expand All @@ -22,7 +23,7 @@
import io.vertx.core.Context;
import io.vertx.core.Future;
import io.vertx.core.Handler;
import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;
import io.vertx.sqlclient.Tuple;

public class BudgetService {
Expand All @@ -31,7 +32,6 @@ public class BudgetService {

private static final String GROUP_FUND_FY_TABLE = "group_fund_fiscal_year";
private static final String TRANSACTIONS_TABLE = "transaction";
public static final String TRANSACTION_IS_PRESENT_BUDGET_DELETE_ERROR = "transactionIsPresentBudgetDeleteError";

private final BudgetDAO budgetDAO;

Expand Down Expand Up @@ -162,7 +162,7 @@ private Future<Void> checkTransactions(Budget budget, DBConn conn) {
.map(rowSet -> {
if (rowSet.size() > 0) {
logger.error("checkTransactions:: Transaction is present");
throw new HttpException(Response.Status.BAD_REQUEST.getStatusCode(), TRANSACTION_IS_PRESENT_BUDGET_DELETE_ERROR);
throw new HttpException(Response.Status.BAD_REQUEST.getStatusCode(), TRANSACTION_IS_PRESENT_BUDGET_DELETE_ERROR.toError());
}
return null;
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import io.vertx.core.Promise;
import io.vertx.core.Vertx;
import io.vertx.core.json.JsonObject;
import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;

public class ExpenseClassService {

Expand Down Expand Up @@ -89,7 +89,7 @@ private Future<ExpenseClass> updateExpenseClass(String id, ExpenseClass group) {
promise.fail(nameCodeConstraintErrorBuilder.buildException(reply, ExpenseClass.class));
}
else if(reply.result().rowCount() == 0) {
promise.fail(new HttpException(Response.Status.NOT_FOUND.getStatusCode()));
promise.fail(new HttpException(Response.Status.NOT_FOUND.getStatusCode(), "Expense class not found for update, id=" + id));
}
else {
promise.complete(group);
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/folio/service/group/GroupService.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import io.vertx.core.Promise;
import io.vertx.core.Vertx;
import io.vertx.core.json.JsonObject;
import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;

public class GroupService {

Expand Down Expand Up @@ -89,7 +89,7 @@ private Future<Group> updateGroup(Group group, String id) {
promise.fail(nameCodeConstraintErrorBuilder.buildException(reply, Group.class));
}
else if(reply.result().rowCount() == 0) {
promise.fail(new HttpException(Response.Status.NOT_FOUND.getStatusCode()));
promise.fail(new HttpException(Response.Status.NOT_FOUND.getStatusCode(), "Group not found for update, id=" + id));
}
else {
promise.complete(group);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

import io.vertx.core.Future;
import io.vertx.core.Promise;
import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;
import org.folio.utils.CalculationUtils;

public class LedgerRolloverService {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package org.folio.service.rollover;

import io.vertx.core.Future;
import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;
import org.folio.dao.rollover.RolloverProgressDAO;
import org.folio.rest.jaxrs.model.LedgerFiscalYearRolloverProgress;
import org.folio.rest.jaxrs.model.RolloverStatus;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

import io.vertx.core.Future;
import io.vertx.core.Promise;
import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;
import io.vertx.sqlclient.Tuple;
import io.vertx.sqlclient.impl.ArrayTuple;
import org.folio.rest.persist.DBConn;
Expand Down
6 changes: 3 additions & 3 deletions src/test/java/org/folio/rest/utils/DBClientTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import io.vertx.core.Vertx;
import io.vertx.core.json.JsonObject;
import io.vertx.ext.web.handler.HttpException;
import org.folio.rest.exception.HttpException;
import io.vertx.junit5.VertxExtension;
import io.vertx.junit5.VertxTestContext;
import io.vertx.sqlclient.Tuple;
Expand Down Expand Up @@ -71,7 +71,7 @@ void withTrans(Vertx vertx, VertxTestContext vtc) {

private static void assertHttpException(Throwable e, String expectedSubstring1, String expectedSubstring2) {
assertThat(e, instanceOf(HttpException.class));
assertThat(((HttpException)e).getPayload(), containsString(expectedSubstring1));
assertThat(((HttpException)e).getPayload(), containsString(expectedSubstring2));
assertThat(e.getMessage(), containsString(expectedSubstring1));
assertThat(e.getMessage(), containsString(expectedSubstring2));
}
}
Loading

0 comments on commit be59cf7

Please sign in to comment.