Skip to content

Commit

Permalink
fix ErrorHandling and LocalDateTime json serialization
Browse files Browse the repository at this point in the history
  • Loading branch information
antonioT90 committed Feb 5, 2025
1 parent 1574340 commit 5fedd01
Show file tree
Hide file tree
Showing 5 changed files with 300 additions and 1 deletion.
3 changes: 2 additions & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ val temporalVersion = "1.27.0"
val protobufJavaVersion = "3.25.5"
val bouncycastleVersion = "1.79"


dependencies {
implementation("org.springframework.boot:spring-boot-starter")
implementation("org.springframework.boot:spring-boot-starter-web")
implementation("org.springframework.boot:spring-boot-starter-validation")
implementation("org.springframework.boot:spring-boot-starter-oauth2-resource-server")
implementation("org.springframework.boot:spring-boot-starter-actuator")
implementation("io.micrometer:micrometer-tracing-bridge-otel:$micrometerVersion")
Expand All @@ -71,6 +71,7 @@ dependencies {

compileOnly("org.projectlombok:lombok")
annotationProcessor("org.projectlombok:lombok")
testAnnotationProcessor("org.projectlombok:lombok")

// Testing
testImplementation("org.springframework.boot:spring-boot-starter-test")
Expand Down
4 changes: 4 additions & 0 deletions gradle.lockfile
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ com.fasterxml.jackson.datatype:jackson-datatype-jdk8:2.18.2=compileClasspath
com.fasterxml.jackson.datatype:jackson-datatype-jsr310:2.18.2=compileClasspath
com.fasterxml.jackson.module:jackson-module-parameter-names:2.18.2=compileClasspath
com.fasterxml.jackson:jackson-bom:2.18.2=compileClasspath
com.fasterxml:classmate:1.7.0=compileClasspath
com.github.stephenc.jcip:jcip-annotations:1.0-1=compileClasspath
com.google.api.grpc:proto-google-common-protos:2.9.0=compileClasspath
com.google.code.findbugs:jsr305:3.0.2=compileClasspath
Expand Down Expand Up @@ -84,6 +85,8 @@ org.apache.tomcat.embed:tomcat-embed-el:10.1.34=compileClasspath
org.apache.tomcat.embed:tomcat-embed-websocket:10.1.34=compileClasspath
org.bouncycastle:bcprov-jdk18on:1.79=compileClasspath
org.checkerframework:checker-qual:3.43.0=compileClasspath
org.hibernate.validator:hibernate-validator:8.0.2.Final=compileClasspath
org.jboss.logging:jboss-logging:3.6.1.Final=compileClasspath
org.jspecify:jspecify:1.0.0=compileClasspath
org.openapitools:jackson-databind-nullable:0.2.6=compileClasspath
org.projectlombok:lombok:1.18.36=compileClasspath
Expand All @@ -101,6 +104,7 @@ org.springframework.boot:spring-boot-starter-json:3.4.1=compileClasspath
org.springframework.boot:spring-boot-starter-logging:3.4.1=compileClasspath
org.springframework.boot:spring-boot-starter-oauth2-resource-server:3.4.1=compileClasspath
org.springframework.boot:spring-boot-starter-tomcat:3.4.1=compileClasspath
org.springframework.boot:spring-boot-starter-validation:3.4.1=compileClasspath
org.springframework.boot:spring-boot-starter-web:3.4.1=compileClasspath
org.springframework.boot:spring-boot-starter:3.4.1=compileClasspath
org.springframework.boot:spring-boot:3.4.1=compileClasspath
Expand Down
14 changes: 14 additions & 0 deletions openapi/p4pa-workflow-worker.openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,20 @@ paths:
security:
- BearerAuth: []
components:
schemas:
WorkerErrorDTO:
type: object
required:
- code
- message
properties:
code:
type: string
enum:
- WORKER_BAD_REQUEST
- WORKER_GENERIC_ERROR
message:
type: string
securitySchemes:
BearerAuth:
type: http
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package it.gov.pagopa.pu.worker.exception;

import com.fasterxml.jackson.databind.JsonMappingException;
import it.gov.pagopa.pu.worker.dto.generated.WorkerErrorDTO;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.validation.ValidationException;
import lombok.extern.slf4j.Slf4j;
import org.springframework.core.Ordered;
import org.springframework.core.annotation.Order;
import org.springframework.http.HttpStatus;
import org.springframework.http.HttpStatusCode;
import org.springframework.http.ResponseEntity;
import org.springframework.http.converter.HttpMessageNotReadableException;
import org.springframework.validation.FieldError;
import org.springframework.web.ErrorResponse;
import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RestControllerAdvice;

import java.util.stream.Collectors;

@RestControllerAdvice
@Slf4j
@Order(Ordered.HIGHEST_PRECEDENCE)
public class WorkerExceptionHandler {

@ExceptionHandler({ValidationException.class, HttpMessageNotReadableException.class, MethodArgumentNotValidException.class})
public ResponseEntity<WorkerErrorDTO> handleViolationException(Exception ex, HttpServletRequest request) {
return handleException(ex, request, HttpStatus.BAD_REQUEST, WorkerErrorDTO.CodeEnum.BAD_REQUEST);
}

@ExceptionHandler({ServletException.class})
public ResponseEntity<WorkerErrorDTO> handleServletException(ServletException ex, HttpServletRequest request) {
HttpStatusCode httpStatus = HttpStatus.INTERNAL_SERVER_ERROR;
WorkerErrorDTO.CodeEnum errorCode = WorkerErrorDTO.CodeEnum.GENERIC_ERROR;
if (ex instanceof ErrorResponse errorResponse) {
httpStatus = errorResponse.getStatusCode();
if (httpStatus.is4xxClientError()) {
errorCode = WorkerErrorDTO.CodeEnum.BAD_REQUEST;
}
}
return handleException(ex, request, httpStatus, errorCode);
}

@ExceptionHandler({RuntimeException.class})
public ResponseEntity<WorkerErrorDTO> handleRuntimeException(RuntimeException ex, HttpServletRequest request) {
return handleException(ex, request, HttpStatus.INTERNAL_SERVER_ERROR, WorkerErrorDTO.CodeEnum.GENERIC_ERROR);
}

static ResponseEntity<WorkerErrorDTO> handleException(Exception ex, HttpServletRequest request, HttpStatusCode httpStatus, WorkerErrorDTO.CodeEnum errorEnum) {
logException(ex, request, httpStatus);

String message = buildReturnedMessage(ex);

return ResponseEntity
.status(httpStatus)
.body(new WorkerErrorDTO(errorEnum, message));
}

private static void logException(Exception ex, HttpServletRequest request, HttpStatusCode httpStatus) {
log.info("A {} occurred handling request {}: HttpStatus {} - {}",
ex.getClass(),
getRequestDetails(request),
httpStatus.value(),
ex.getMessage());
}

private static String buildReturnedMessage(Exception ex) {
if (ex instanceof HttpMessageNotReadableException) {
if(ex.getCause() instanceof JsonMappingException jsonMappingException){
return "Cannot parse body: " +
jsonMappingException.getPath().stream()
.map(JsonMappingException.Reference::getFieldName)
.collect(Collectors.joining(".")) +
": " + jsonMappingException.getOriginalMessage();
}
return "Required request body is missing";
} else if (ex instanceof MethodArgumentNotValidException methodArgumentNotValidException) {
return "Invalid request content:" +
methodArgumentNotValidException.getBindingResult()
.getAllErrors().stream()
.map(e -> " " +
(e instanceof FieldError fieldError? fieldError.getField(): e.getObjectName()) +
": " + e.getDefaultMessage())
.sorted()
.collect(Collectors.joining(";"));
} else {
return ex.getMessage();
}
}

static String getRequestDetails(HttpServletRequest request) {
return "%s %s".formatted(request.getMethod(), request.getRequestURI());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,184 @@
package it.gov.pagopa.pu.worker.exception;

import com.fasterxml.jackson.databind.ObjectMapper;
import it.gov.pagopa.payhub.activities.config.json.JsonConfig;
import jakarta.servlet.ServletException;
import jakarta.validation.ConstraintViolationException;
import jakarta.validation.Valid;
import jakarta.validation.constraints.NotNull;
import jakarta.validation.constraints.Pattern;
import lombok.AllArgsConstructor;
import lombok.Data;
import lombok.NoArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.autoconfigure.security.servlet.SecurityAutoConfiguration;
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
import org.springframework.http.MediaType;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.bean.override.mockito.MockitoSpyBean;
import org.springframework.test.context.junit.jupiter.SpringExtension;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.ResultActions;
import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder;
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
import org.springframework.test.web.servlet.result.MockMvcResultMatchers;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestBody;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.server.ServerErrorException;
import org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter;

import java.time.LocalDateTime;
import java.util.Set;

import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doThrow;

@ExtendWith({SpringExtension.class})
@WebMvcTest(value = {WorkerExceptionHandlerTest.TestController.class}, excludeAutoConfiguration = SecurityAutoConfiguration.class)
@ContextConfiguration(classes = {
WorkerExceptionHandlerTest.TestController.class,
WorkerExceptionHandler.class,
JsonConfig.class})
class WorkerExceptionHandlerTest {

public static final String DATA = "data";
public static final TestRequestBody BODY = new TestRequestBody("bodyData", null, "abc", LocalDateTime.now());

@Autowired
private MockMvc mockMvc;
@Autowired
private ObjectMapper objectMapper;

@MockitoSpyBean
private TestController testControllerSpy;
@MockitoSpyBean
private RequestMappingHandlerAdapter requestMappingHandlerAdapterSpy;

@RestController
@Slf4j
static class TestController {
@PostMapping(value = "/test", produces = MediaType.APPLICATION_JSON_VALUE)
String testEndpoint(@RequestParam(DATA) String data, @Valid @RequestBody TestRequestBody body) {
return "OK";
}
}

@Data
@NoArgsConstructor
@AllArgsConstructor
public static class TestRequestBody {
@NotNull
private String requiredField;
private String notRequiredField;
@Pattern(regexp = "[a-z]+")
private String lowerCaseAlphabeticField;
private LocalDateTime dateTimeField;
}

private ResultActions performRequest(String data, MediaType accept) throws Exception {
return performRequest(data, accept, objectMapper.writeValueAsString(WorkerExceptionHandlerTest.BODY));
}

private ResultActions performRequest(String data, MediaType accept, String body) throws Exception {
MockHttpServletRequestBuilder requestBuilder = MockMvcRequestBuilders.post("/test")
.param(DATA, data)
.accept(accept);

if (body != null) {
requestBuilder
.contentType(MediaType.APPLICATION_JSON)
.content(body);
}

return mockMvc.perform(requestBuilder);
}

@Test
void handleMissingServletRequestParameterException() throws Exception {
performRequest(null, MediaType.APPLICATION_JSON)
.andExpect(MockMvcResultMatchers.status().isBadRequest())
.andExpect(MockMvcResultMatchers.jsonPath("$.code").value("WORKER_BAD_REQUEST"))
.andExpect(MockMvcResultMatchers.jsonPath("$.message").value("Required request parameter 'data' for method parameter type String is not present"));
}

@Test
void handleRuntimeExceptionError() throws Exception {
doThrow(new RuntimeException("Error")).when(testControllerSpy).testEndpoint(DATA, BODY);

performRequest(DATA, MediaType.APPLICATION_JSON)
.andExpect(MockMvcResultMatchers.status().isInternalServerError())
.andExpect(MockMvcResultMatchers.jsonPath("$.code").value("WORKER_GENERIC_ERROR"))
.andExpect(MockMvcResultMatchers.jsonPath("$.message").value("Error"));
}

@Test
void handleGenericServletException() throws Exception {
doThrow(new ServletException("Error"))
.when(requestMappingHandlerAdapterSpy).handle(any(), any(), any());

performRequest(DATA, MediaType.APPLICATION_JSON)
.andExpect(MockMvcResultMatchers.status().isInternalServerError())
.andExpect(MockMvcResultMatchers.jsonPath("$.code").value("WORKER_GENERIC_ERROR"))
.andExpect(MockMvcResultMatchers.jsonPath("$.message").value("Error"));
}

@Test
void handle4xxHttpServletException() throws Exception {
performRequest(DATA, MediaType.parseMediaType("application/hal+json"))
.andExpect(MockMvcResultMatchers.status().isNotAcceptable())
.andExpect(MockMvcResultMatchers.jsonPath("$.code").value("WORKER_BAD_REQUEST"))
.andExpect(MockMvcResultMatchers.jsonPath("$.message").value("No acceptable representation"));
}

@Test
void handleNoBodyException() throws Exception {
performRequest(DATA, MediaType.APPLICATION_JSON, null)
.andExpect(MockMvcResultMatchers.status().isBadRequest())
.andExpect(MockMvcResultMatchers.jsonPath("$.code").value("WORKER_BAD_REQUEST"))
.andExpect(MockMvcResultMatchers.jsonPath("$.message").value("Required request body is missing"));
}

@Test
void handleInvalidBodyException() throws Exception {
performRequest(DATA, MediaType.APPLICATION_JSON,
"{\"notRequiredField\":\"notRequired\",\"lowerCaseAlphabeticField\":\"ABC\"}")
.andExpect(MockMvcResultMatchers.status().isBadRequest())
.andExpect(MockMvcResultMatchers.jsonPath("$.code").value("WORKER_BAD_REQUEST"))
.andExpect(MockMvcResultMatchers.jsonPath("$.message").value("Invalid request content: lowerCaseAlphabeticField: must match \"[a-z]+\"; requiredField: must not be null"));
}

@Test
void handleNotParsableBodyException() throws Exception {
performRequest(DATA, MediaType.APPLICATION_JSON,
"{\"notRequiredField\":\"notRequired\",\"dateTimeField\":\"2025-02-05\"}")
.andExpect(MockMvcResultMatchers.status().isBadRequest())
.andExpect(MockMvcResultMatchers.jsonPath("$.code").value("WORKER_BAD_REQUEST"))
.andExpect(MockMvcResultMatchers.jsonPath("$.message").value("Cannot parse body: dateTimeField: Text '2025-02-05' could not be parsed at index 10"));
}

@Test
void handle5xxHttpServletException() throws Exception {
doThrow(new ServerErrorException("Error", new RuntimeException("Error")))
.when(requestMappingHandlerAdapterSpy).handle(any(), any(), any());

performRequest(DATA, MediaType.APPLICATION_JSON)
.andExpect(MockMvcResultMatchers.status().isInternalServerError())
.andExpect(MockMvcResultMatchers.jsonPath("$.code").value("WORKER_GENERIC_ERROR"))
.andExpect(MockMvcResultMatchers.jsonPath("$.message").value("500 INTERNAL_SERVER_ERROR \"Error\""));
}

@Test
void handleViolationException() throws Exception {
doThrow(new ConstraintViolationException("Error", Set.of())).when(testControllerSpy).testEndpoint(DATA, BODY);

performRequest(DATA, MediaType.APPLICATION_JSON)
.andExpect(MockMvcResultMatchers.status().isBadRequest())
.andExpect(MockMvcResultMatchers.jsonPath("$.code").value("WORKER_BAD_REQUEST"))
.andExpect(MockMvcResultMatchers.jsonPath("$.message").value("Error"));
}
}

0 comments on commit 5fedd01

Please sign in to comment.