From e6f1b9191227da1e5ebdd9b1e64e4591b5fe20df Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 20 Oct 2025 14:18:27 +0000 Subject: [PATCH] Refactor exception handling and add new exception types Co-authored-by: richard.smit.dev --- .../GlobalControllerExceptionHandler.java | 214 ++++++++++++++---- .../GlobalControllerExceptionHandlerTest.java | 54 ++++- 2 files changed, 218 insertions(+), 50 deletions(-) diff --git a/src/main/java/com/fleetops/controller/GlobalControllerExceptionHandler.java b/src/main/java/com/fleetops/controller/GlobalControllerExceptionHandler.java index 569167b..26f2086 100644 --- a/src/main/java/com/fleetops/controller/GlobalControllerExceptionHandler.java +++ b/src/main/java/com/fleetops/controller/GlobalControllerExceptionHandler.java @@ -8,6 +8,7 @@ *
  • Constraint/validation violations -> 400 Bad Request
    • *
  • Not found exceptions -> 404 Not Found
    • *
  • Conflicts such as duplicate resources -> 409 Conflict
    • + *
  • Service layer exceptions -> 500 Internal Server Error
    • *
  • Unhandled errors -> 500 Internal Server Error
    • * * The corresponding response bodies follow a simple error format (see ErrorResponse in OpenAPI). @@ -15,88 +16,213 @@ import com.fleetops.exception.LicensePlateAlreadyExistsException; import com.fleetops.exception.NotFoundExceptionBase; +import com.fleetops.exception.ServiceException; +import jakarta.servlet.http.HttpServletRequest; import jakarta.validation.ConstraintViolationException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.http.converter.HttpMessageNotReadableException; +import org.springframework.security.access.AccessDeniedException; +import org.springframework.security.core.AuthenticationException; import org.springframework.web.bind.MethodArgumentNotValidException; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.RestControllerAdvice; +import org.springframework.web.context.request.WebRequest; import java.time.LocalDateTime; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.stream.Collectors; @RestControllerAdvice public class GlobalControllerExceptionHandler { - @ExceptionHandler(RuntimeException.class) - public ResponseEntity> handleRuntimeException(RuntimeException ex) { + private static final Logger logger = LoggerFactory.getLogger(GlobalControllerExceptionHandler.class); + + /** + * Creates a standardized error response body. + * + * @param status HTTP status + * @param error Error type description + * @param message Error message + * @param request Web request for additional context + * @return Map containing error details + */ + private Map createErrorResponse(HttpStatus status, String error, String message, WebRequest request) { Map body = new HashMap<>(); body.put("timestamp", LocalDateTime.now()); - body.put("status", HttpStatus.INTERNAL_SERVER_ERROR.value()); - body.put("error", "Internal Server Error"); - body.put("message", ex.getMessage()); + body.put("status", status.value()); + body.put("error", error); + body.put("message", message); + body.put("path", request.getDescription(false).replace("uri=", "")); + return body; + } + /** + * Creates a standardized error response body with additional details. + * + * @param status HTTP status + * @param error Error type description + * @param message Error message + * @param request Web request for additional context + * @param details Additional error details + * @return Map containing error details + */ + private Map createErrorResponse(HttpStatus status, String error, String message, WebRequest request, Map details) { + Map body = createErrorResponse(status, error, message, request); + if (details != null && !details.isEmpty()) { + body.putAll(details); + } + return body; + } + + @ExceptionHandler(ServiceException.class) + public ResponseEntity> handleServiceException(ServiceException ex, WebRequest request) { + logger.error("Service exception occurred: {}", ex.getMessage(), ex); + Map body = createErrorResponse( + HttpStatus.INTERNAL_SERVER_ERROR, + "Internal Server Error", + "A service error occurred. Please try again later.", + request + ); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(body); } - @ExceptionHandler(NotFoundExceptionBase.class) - public ResponseEntity> handleNotFound(NotFoundExceptionBase ex) { - Map body = new HashMap<>(); - body.put("timestamp", LocalDateTime.now()); - body.put("status", HttpStatus.NOT_FOUND.value()); - body.put("error", "Not Found"); - body.put("message", ex.getMessage()); + @ExceptionHandler(RuntimeException.class) + public ResponseEntity> handleRuntimeException(RuntimeException ex, WebRequest request) { + logger.error("Unexpected runtime exception occurred: {}", ex.getMessage(), ex); + Map body = createErrorResponse( + HttpStatus.INTERNAL_SERVER_ERROR, + "Internal Server Error", + "An unexpected error occurred. Please try again later.", + request + ); + return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(body); + } + @ExceptionHandler(NotFoundExceptionBase.class) + public ResponseEntity> handleNotFound(NotFoundExceptionBase ex, WebRequest request) { + logger.debug("Resource not found: {}", ex.getMessage()); + Map body = createErrorResponse( + HttpStatus.NOT_FOUND, + "Not Found", + ex.getMessage(), + request + ); return ResponseEntity.status(HttpStatus.NOT_FOUND).body(body); } @ExceptionHandler(LicensePlateAlreadyExistsException.class) - public ResponseEntity> handleConflict(LicensePlateAlreadyExistsException ex) { - Map body = new HashMap<>(); - body.put("timestamp", LocalDateTime.now()); - body.put("status", HttpStatus.CONFLICT.value()); - body.put("error", "Conflict"); - body.put("message", ex.getMessage()); + public ResponseEntity> handleLicensePlateConflict(LicensePlateAlreadyExistsException ex, WebRequest request) { + logger.warn("License plate conflict: {}", ex.getMessage()); + Map body = createErrorResponse( + HttpStatus.CONFLICT, + "Conflict", + ex.getMessage(), + request + ); + return ResponseEntity.status(HttpStatus.CONFLICT).body(body); + } + @ExceptionHandler(DataIntegrityViolationException.class) + public ResponseEntity> handleDataIntegrityViolation(DataIntegrityViolationException ex, WebRequest request) { + logger.warn("Data integrity violation: {}", ex.getMessage()); + Map body = createErrorResponse( + HttpStatus.CONFLICT, + "Conflict", + "The operation conflicts with existing data constraints.", + request + ); return ResponseEntity.status(HttpStatus.CONFLICT).body(body); } @ExceptionHandler(MethodArgumentNotValidException.class) - public ResponseEntity> handleValidation(MethodArgumentNotValidException ex) { - Map body = new HashMap<>(); - body.put("timestamp", LocalDateTime.now()); - body.put("status", HttpStatus.BAD_REQUEST.value()); - body.put("error", "Bad Request"); - body.put("message", "Validation failed"); + public ResponseEntity> handleValidation(MethodArgumentNotValidException ex, WebRequest request) { + logger.debug("Validation failed: {}", ex.getMessage()); + + Map fieldErrors = ex.getBindingResult() + .getFieldErrors() + .stream() + .collect(Collectors.toMap( + fieldError -> fieldError.getField(), + fieldError -> fieldError.getDefaultMessage() != null ? fieldError.getDefaultMessage() : "Invalid value", + (existing, replacement) -> existing + )); + + Map details = new HashMap<>(); + details.put("fieldErrors", fieldErrors); + + Map body = createErrorResponse( + HttpStatus.BAD_REQUEST, + "Validation Failed", + "Request validation failed for one or more fields.", + request, + details + ); return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(body); } @ExceptionHandler(ConstraintViolationException.class) - public ResponseEntity> handleConstraintViolation(ConstraintViolationException ex) { - Map body = new HashMap<>(); - body.put("timestamp", LocalDateTime.now()); - body.put("status", HttpStatus.BAD_REQUEST.value()); - body.put("error", "Bad Request"); - body.put("message", "Constraint violation"); - body.put("violations", ex.getConstraintViolations().stream() - .collect(Collectors.toMap( - v -> v.getPropertyPath().toString(), - v -> v.getMessage(), - (a, b) -> a - ))); - return ResponseEntity.badRequest().body(body); + public ResponseEntity> handleConstraintViolation(ConstraintViolationException ex, WebRequest request) { + logger.debug("Constraint violation: {}", ex.getMessage()); + + Map violations = ex.getConstraintViolations().stream() + .collect(Collectors.toMap( + v -> v.getPropertyPath().toString(), + v -> v.getMessage(), + (existing, replacement) -> existing + )); + + Map details = new HashMap<>(); + details.put("violations", violations); + + Map body = createErrorResponse( + HttpStatus.BAD_REQUEST, + "Constraint Violation", + "Request contains constraint violations.", + request, + details + ); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(body); } @ExceptionHandler(HttpMessageNotReadableException.class) - public ResponseEntity> handleBadJson(HttpMessageNotReadableException ex) { - Map body = new HashMap<>(); - body.put("timestamp", LocalDateTime.now()); - body.put("status", HttpStatus.BAD_REQUEST.value()); - body.put("error", "Bad Request"); - body.put("message", "Malformed JSON request"); - return ResponseEntity.badRequest().body(body); + public ResponseEntity> handleBadJson(HttpMessageNotReadableException ex, WebRequest request) { + logger.debug("Malformed JSON request: {}", ex.getMessage()); + Map body = createErrorResponse( + HttpStatus.BAD_REQUEST, + "Bad Request", + "Malformed JSON request. Please check your request format.", + request + ); + return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(body); + } + + @ExceptionHandler(AuthenticationException.class) + public ResponseEntity> handleAuthenticationException(AuthenticationException ex, WebRequest request) { + logger.warn("Authentication failed: {}", ex.getMessage()); + Map body = createErrorResponse( + HttpStatus.UNAUTHORIZED, + "Unauthorized", + "Authentication is required to access this resource.", + request + ); + return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(body); + } + + @ExceptionHandler(AccessDeniedException.class) + public ResponseEntity> handleAccessDeniedException(AccessDeniedException ex, WebRequest request) { + logger.warn("Access denied: {}", ex.getMessage()); + Map body = createErrorResponse( + HttpStatus.FORBIDDEN, + "Forbidden", + "You do not have permission to access this resource.", + request + ); + return ResponseEntity.status(HttpStatus.FORBIDDEN).body(body); } } diff --git a/src/test/java/com/fleetops/controller/GlobalControllerExceptionHandlerTest.java b/src/test/java/com/fleetops/controller/GlobalControllerExceptionHandlerTest.java index 2830541..662ae37 100644 --- a/src/test/java/com/fleetops/controller/GlobalControllerExceptionHandlerTest.java +++ b/src/test/java/com/fleetops/controller/GlobalControllerExceptionHandlerTest.java @@ -2,9 +2,11 @@ import com.fleetops.exception.LicensePlateAlreadyExistsException; import com.fleetops.exception.NotFoundExceptionBase; +import com.fleetops.exception.ServiceException; import jakarta.validation.Valid; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.http.MediaType; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.setup.MockMvcBuilders; @@ -35,7 +37,9 @@ void notFound_IsMappedTo404Json() throws Exception { .andExpect(status().isNotFound()) .andExpect(jsonPath("$.status").value(404)) .andExpect(jsonPath("$.error").value("Not Found")) - .andExpect(jsonPath("$.message", containsString("missing"))); + .andExpect(jsonPath("$.message", containsString("missing"))) + .andExpect(jsonPath("$.timestamp").exists()) + .andExpect(jsonPath("$.path").exists()); } @Test @@ -43,7 +47,9 @@ void conflict_IsMappedTo409Json() throws Exception { mockMvc.perform(get("/throw/conflict")) .andExpect(status().isConflict()) .andExpect(jsonPath("$.status").value(409)) - .andExpect(jsonPath("$.error").value("Conflict")); + .andExpect(jsonPath("$.error").value("Conflict")) + .andExpect(jsonPath("$.timestamp").exists()) + .andExpect(jsonPath("$.path").exists()); } @Test @@ -51,7 +57,9 @@ void runtime_IsMappedTo500Json() throws Exception { mockMvc.perform(get("/throw/runtime")) .andExpect(status().isInternalServerError()) .andExpect(jsonPath("$.status").value(500)) - .andExpect(jsonPath("$.error").value("Internal Server Error")); + .andExpect(jsonPath("$.error").value("Internal Server Error")) + .andExpect(jsonPath("$.timestamp").exists()) + .andExpect(jsonPath("$.path").exists()); } @Test @@ -63,7 +71,10 @@ void methodArgumentNotValid_IsMappedTo400Json() throws Exception { .content(json)) .andExpect(status().isBadRequest()) .andExpect(jsonPath("$.status").value(400)) - .andExpect(jsonPath("$.error").value("Bad Request")); + .andExpect(jsonPath("$.error").value("Validation Failed")) + .andExpect(jsonPath("$.fieldErrors").exists()) + .andExpect(jsonPath("$.timestamp").exists()) + .andExpect(jsonPath("$.path").exists()); } @Test @@ -71,7 +82,10 @@ void constraintViolation_IsMappedTo400Json() throws Exception { mockMvc.perform(get("/throw/constraint/-1")) .andExpect(status().isBadRequest()) .andExpect(jsonPath("$.status").value(400)) - .andExpect(jsonPath("$.error").value("Bad Request")); + .andExpect(jsonPath("$.error").value("Constraint Violation")) + .andExpect(jsonPath("$.violations").exists()) + .andExpect(jsonPath("$.timestamp").exists()) + .andExpect(jsonPath("$.path").exists()); } @Test @@ -82,7 +96,29 @@ void malformedJson_IsMappedTo400Json() throws Exception { .content(badJson)) .andExpect(status().isBadRequest()) .andExpect(jsonPath("$.status").value(400)) - .andExpect(jsonPath("$.error").value("Bad Request")); + .andExpect(jsonPath("$.error").value("Bad Request")) + .andExpect(jsonPath("$.timestamp").exists()) + .andExpect(jsonPath("$.path").exists()); + } + + @Test + void serviceException_IsMappedTo500Json() throws Exception { + mockMvc.perform(get("/throw/service")) + .andExpect(status().isInternalServerError()) + .andExpect(jsonPath("$.status").value(500)) + .andExpect(jsonPath("$.error").value("Internal Server Error")) + .andExpect(jsonPath("$.timestamp").exists()) + .andExpect(jsonPath("$.path").exists()); + } + + @Test + void dataIntegrityViolation_IsMappedTo409Json() throws Exception { + mockMvc.perform(get("/throw/dataintegrity")) + .andExpect(status().isConflict()) + .andExpect(jsonPath("$.status").value(409)) + .andExpect(jsonPath("$.error").value("Conflict")) + .andExpect(jsonPath("$.timestamp").exists()) + .andExpect(jsonPath("$.path").exists()); } // Minimal controller solely for triggering exceptions handled by the advice @@ -105,6 +141,12 @@ private static class ThrowingController { public String constraint(@PathVariable long id) { throw new jakarta.validation.ConstraintViolationException("invalid", java.util.Collections.emptySet()); } + + @GetMapping("/throw/service") + public String service() { throw new ServiceException("Service failed", new RuntimeException("Database error")); } + + @GetMapping("/throw/dataintegrity") + public String dataIntegrity() { throw new DataIntegrityViolationException("Data constraint violated"); } } private static class SampleDto {