Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ErrorHandling and LocalDateTime json serialization #24

Merged
merged 3 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,15 @@ repositories {
val springDocOpenApiVersion = "2.7.0"
val openApiToolsVersion = "0.2.6"
val micrometerVersion = "1.4.1"
val p4paActivitiesVersion = "1.47.2"
val p4paActivitiesVersion = "1.47.3"
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
6 changes: 5 additions & 1 deletion 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 @@ -71,7 +72,7 @@ io.temporal:temporal-spring-boot-autoconfigure:1.27.0=compileClasspath
io.temporal:temporal-spring-boot-starter:1.27.0=compileClasspath
io.temporal:temporal-test-server:1.27.0=compileClasspath
io.temporal:temporal-testing:1.27.0=compileClasspath
it.gov.pagopa.payhub:p4pa-payhub-activities:1.47.2=compileClasspath
it.gov.pagopa.payhub:p4pa-payhub-activities:1.47.3=compileClasspath
jakarta.activation:jakarta.activation-api:2.1.3=compileClasspath
jakarta.annotation:jakarta.annotation-api:2.1.1=compileClasspath
jakarta.validation:jakarta.validation-api:3.0.2=compileClasspath
Expand All @@ -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"));
}
}