From 77ef126a7e42d533899ef87e68656a3beee4446b Mon Sep 17 00:00:00 2001
From: Michael Hoennig <michael@hoennig.de>
Date: Sun, 16 Oct 2022 10:08:42 +0200
Subject: [PATCH] improved REST-API error handling for broken request body JSON
---
src/main/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandler.java | 57 ++++--------------
src/main/java/net/hostsharing/hsadminng/config/JsonObjectMapperConfiguration.java | 2
src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java | 7 +
src/test/java/net/hostsharing/hsadminng/test/cust/TestCustomerControllerAcceptanceTest.java | 24 +++++++
src/main/java/net/hostsharing/hsadminng/errors/CustomErrorResponse.java | 51 +++++++++++++++++
5 files changed, 94 insertions(+), 47 deletions(-)
diff --git a/src/main/java/net/hostsharing/hsadminng/config/JsonObjectMapperConfiguration.java b/src/main/java/net/hostsharing/hsadminng/config/JsonObjectMapperConfiguration.java
index 8c6cb2b..921d0c3 100644
--- a/src/main/java/net/hostsharing/hsadminng/config/JsonObjectMapperConfiguration.java
+++ b/src/main/java/net/hostsharing/hsadminng/config/JsonObjectMapperConfiguration.java
@@ -1,5 +1,6 @@
package net.hostsharing.hsadminng.config;
+import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.databind.SerializationFeature;
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
import org.openapitools.jackson.nullable.JsonNullableModule;
@@ -16,6 +17,7 @@
public Jackson2ObjectMapperBuilder customObjectMapper() {
return new Jackson2ObjectMapperBuilder()
.modules(new JsonNullableModule(), new JavaTimeModule())
+ .featuresToEnable(JsonParser.Feature.ALLOW_COMMENTS)
.featuresToDisable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);
}
}
diff --git a/src/main/java/net/hostsharing/hsadminng/errors/CustomErrorResponse.java b/src/main/java/net/hostsharing/hsadminng/errors/CustomErrorResponse.java
new file mode 100644
index 0000000..be8bbe6
--- /dev/null
+++ b/src/main/java/net/hostsharing/hsadminng/errors/CustomErrorResponse.java
@@ -0,0 +1,51 @@
+package net.hostsharing.hsadminng.errors;
+
+import com.fasterxml.jackson.annotation.JsonFormat;
+import lombok.Getter;
+import org.springframework.http.HttpStatus;
+import org.springframework.http.ResponseEntity;
+import org.springframework.web.context.request.WebRequest;
+
+import java.time.LocalDateTime;
+
+@Getter
+class CustomErrorResponse {
+
+ static ResponseEntity<CustomErrorResponse> errorResponse(
+ final WebRequest request,
+ final HttpStatus httpStatus,
+ final String message) {
+ return new ResponseEntity<>(
+ new CustomErrorResponse(request.getContextPath(), httpStatus, message), httpStatus);
+ }
+
+ static String firstMessageLine(final Throwable exception) {
+ if (exception.getMessage() != null) {
+ return firstLine(exception.getMessage());
+ }
+ return "ERROR: [500] " + exception.getClass().getName();
+ }
+
+ static String firstLine(final String message) {
+ return message.split("\\r|\\n|\\r\\n", 0)[0];
+ }
+
+ @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd hh:mm:ss")
+ private final LocalDateTime timestamp;
+
+ private final String path;
+
+ private final int status;
+
+ private final String error;
+
+ private final String message;
+
+ CustomErrorResponse(final String path, final HttpStatus status, final String message) {
+ this.timestamp = LocalDateTime.now();
+ this.path = path;
+ this.status = status.value();
+ this.error = status.getReasonPhrase();
+ this.message = message;
+ }
+}
diff --git a/src/main/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandler.java b/src/main/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandler.java
index 89e9f9f..2df06e1 100644
--- a/src/main/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandler.java
+++ b/src/main/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandler.java
@@ -1,13 +1,12 @@
package net.hostsharing.hsadminng.errors;
-import com.fasterxml.jackson.annotation.JsonFormat;
-import lombok.Getter;
import org.iban4j.Iban4jException;
import org.springframework.core.NestedExceptionUtils;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
+import org.springframework.lang.Nullable;
import org.springframework.orm.jpa.JpaObjectRetrievalFailureException;
import org.springframework.orm.jpa.JpaSystemException;
import org.springframework.web.bind.MethodArgumentNotValidException;
@@ -17,10 +16,11 @@
import org.springframework.web.servlet.mvc.method.annotation.ResponseEntityExceptionHandler;
import javax.persistence.EntityNotFoundException;
-import java.time.LocalDateTime;
import java.util.NoSuchElementException;
import java.util.Optional;
import java.util.regex.Pattern;
+
+import static net.hostsharing.hsadminng.errors.CustomErrorResponse.*;
@ControllerAdvice
public class RestResponseEntityExceptionHandler
@@ -72,6 +72,16 @@
}
@Override
+ @SuppressWarnings("unchecked,rawtypes")
+ protected ResponseEntity handleExceptionInternal(
+ Exception exc, @Nullable Object body, HttpHeaders headers, HttpStatus status, WebRequest request) {
+
+ final var response = super.handleExceptionInternal(exc, body, headers, status, request);
+ return errorResponse(request, response.getStatusCode(),
+ Optional.ofNullable(response.getBody()).map(Object::toString).orElse(firstMessageLine(exc)));
+ }
+
+ //@ExceptionHandler({ MethodArgumentNotValidException.class })
@SuppressWarnings("unchecked,rawtypes")
protected ResponseEntity handleMethodArgumentNotValid(
MethodArgumentNotValidException exc,
@@ -126,45 +136,4 @@
return Optional.empty();
}
- private static ResponseEntity<CustomErrorResponse> errorResponse(
- final WebRequest request,
- final HttpStatus httpStatus,
- final String message) {
- return new ResponseEntity<>(
- new CustomErrorResponse(request.getContextPath(), httpStatus, message), httpStatus);
- }
-
- private String firstMessageLine(final Throwable exception) {
- if (exception.getMessage() != null) {
- return firstLine(exception.getMessage());
- }
- return "ERROR: [500] " + exception.getClass().getName();
- }
-
- private String firstLine(final String message) {
- return message.split("\\r|\\n|\\r\\n", 0)[0];
- }
-}
-
-@Getter
-class CustomErrorResponse {
-
- @JsonFormat(shape = JsonFormat.Shape.STRING, pattern = "yyyy-MM-dd hh:mm:ss")
- private final LocalDateTime timestamp;
-
- private final String path;
-
- private final int status;
-
- private final String error;
-
- private final String message;
-
- public CustomErrorResponse(final String path, final HttpStatus status, final String message) {
- this.timestamp = LocalDateTime.now();
- this.path = path;
- this.status = status.value();
- this.error = status.getReasonPhrase();
- this.message = message;
- }
}
diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java
index 91024f0..31391fd 100644
--- a/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java
+++ b/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java
@@ -136,7 +136,7 @@
.port(port)
.when()
.post("http://localhost/api/hs/office/bankaccounts")
- .then().assertThat()
+ .then().log().all().assertThat()
.statusCode(201)
.contentType(ContentType.JSON)
.body("uuid", isUuidValid())
@@ -346,7 +346,10 @@
jpaAttempt.transacted(() -> {
context.define("superuser-alex@hostsharing.net", null);
tempBankAccountUuids.addAll(
- bankAccountRepo.findByOptionalHolderLike("some temp acc").stream().map(HsOfficeBankAccountEntity::getUuid).toList()
+ bankAccountRepo.findByOptionalHolderLike("some temp acc")
+ .stream()
+ .map(HsOfficeBankAccountEntity::getUuid)
+ .toList()
);
});
tempBankAccountUuids.forEach(uuid -> {
diff --git a/src/test/java/net/hostsharing/hsadminng/test/cust/TestCustomerControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/test/cust/TestCustomerControllerAcceptanceTest.java
index 401046b..a2bd0b7 100644
--- a/src/test/java/net/hostsharing/hsadminng/test/cust/TestCustomerControllerAcceptanceTest.java
+++ b/src/test/java/net/hostsharing/hsadminng/test/cust/TestCustomerControllerAcceptanceTest.java
@@ -247,6 +247,26 @@
context.define("superuser-fran@hostsharing.net");
assertThat(testCustomerRepository.findCustomerByOptionalPrefixLike("uuu")).hasSize(0);
}
+
+ @Test
+ void invalidRequestBodyJson_raisesClientError() {
+
+ RestAssured // @formatter:off
+ .given()
+ .header("current-user", "superuser-alex@hostsharing.net")
+ .contentType(ContentType.JSON)
+ .body("{]") // deliberately invalid JSON
+ .port(port)
+ .when()
+ .post("http://localhost/api/test/customers")
+ .then().assertThat()
+ .statusCode(400)
+ .contentType(ContentType.JSON)
+ .body("message", containsString("JSON parse error: Unexpected close marker ']': expected '}'"))
+ .body("message", containsString("line: 1, column: 1"));
+ // @formatter:on
+ }
+
}
private UUID toCleanup(final UUID tempPartnerUuid) {
@@ -262,7 +282,9 @@
System.out.println("DELETING temporary partner: " + uuid);
final var entity = testCustomerRepository.findByUuid(uuid);
final var count = testCustomerRepository.deleteByUuid(uuid);
- System.out.println("DELETED temporary partner: " + uuid + (count > 0 ? " successful" : " failed") + " (" + entity.map(TestCustomerEntity::getPrefix).orElse("???") + ")");
+ System.out.println(
+ "DELETED temporary partner: " + uuid + (count > 0 ? " successful" : " failed") + " (" + entity.map(
+ TestCustomerEntity::getPrefix).orElse("???") + ")");
}).assertSuccessful();
});
}
--
Gitblit v1.9.3