diff --git a/CHANGELOG.md b/CHANGELOG.md index 6da9ab4..9155dc0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,13 @@ Release names follow the **historic football clubs** naming convention (A–Z): ### Changed +- Return `422 Unprocessable Entity` for field validation failures (`@Valid` + constraint violations and squad number mismatch) instead of `400 Bad Request`; + reserve `400` for genuinely malformed requests (unparseable JSON, wrong + `Content-Type`); introduce `GlobalExceptionHandler` (`@ControllerAdvice`) to + intercept `MethodArgumentNotValidException`; update OpenAPI `@ApiResponse` + annotations and test assertions accordingly (#319) + ### Fixed ### Removed diff --git a/src/main/java/ar/com/nanotaboada/java/samples/spring/boot/controllers/GlobalExceptionHandler.java b/src/main/java/ar/com/nanotaboada/java/samples/spring/boot/controllers/GlobalExceptionHandler.java new file mode 100644 index 0000000..deb160e --- /dev/null +++ b/src/main/java/ar/com/nanotaboada/java/samples/spring/boot/controllers/GlobalExceptionHandler.java @@ -0,0 +1,16 @@ +package ar.com.nanotaboada.java.samples.spring.boot.controllers; + +import org.springframework.http.HttpStatus; +import org.springframework.http.ResponseEntity; +import org.springframework.web.bind.MethodArgumentNotValidException; +import org.springframework.web.bind.annotation.ExceptionHandler; +import org.springframework.web.bind.annotation.RestControllerAdvice; + +@RestControllerAdvice +public class GlobalExceptionHandler { + + @ExceptionHandler(MethodArgumentNotValidException.class) + public ResponseEntity handleValidationException(MethodArgumentNotValidException exception) { + return ResponseEntity.status(HttpStatus.UNPROCESSABLE_ENTITY).build(); + } +} diff --git a/src/main/java/ar/com/nanotaboada/java/samples/spring/boot/controllers/PlayersController.java b/src/main/java/ar/com/nanotaboada/java/samples/spring/boot/controllers/PlayersController.java index 62ae863..0563f9b 100644 --- a/src/main/java/ar/com/nanotaboada/java/samples/spring/boot/controllers/PlayersController.java +++ b/src/main/java/ar/com/nanotaboada/java/samples/spring/boot/controllers/PlayersController.java @@ -51,7 +51,7 @@ *
  • 200 OK: Successful retrieval
    • *
  • 201 Created: Successful creation (with Location header)
    • *
  • 204 No Content: Successful update/delete
    • - *
  • 400 Bad Request: Validation failure
    • + *
  • 422 Unprocessable Entity: Validation failure
    • *
  • 404 Not Found: Resource not found
    • * * @@ -80,14 +80,14 @@ public class PlayersController { *

    * * @param playerDTO the player data to create (validated with JSR-380 constraints) - * @return 201 Created with Location header, 400 Bad Request if validation fails, or 409 Conflict if squad number exists + * @return 201 Created with Location header, 409 Conflict if squad number exists, or 422 Unprocessable Entity if validation fails */ @PostMapping("/players") @Operation(summary = "Creates a new player") @ApiResponses(value = { @ApiResponse(responseCode = "201", description = "Created", content = @Content), - @ApiResponse(responseCode = "400", description = "Bad Request - Validation failure", content = @Content), - @ApiResponse(responseCode = "409", description = "Conflict - Squad number already exists", content = @Content) + @ApiResponse(responseCode = "409", description = "Conflict - Squad number already exists", content = @Content), + @ApiResponse(responseCode = "422", description = "Unprocessable Entity - Validation failure", content = @Content) }) public ResponseEntity post(@RequestBody @Valid PlayerDTO playerDTO) { PlayerDTO createdPlayer = playersService.create(playerDTO); @@ -200,18 +200,18 @@ public ResponseEntity> searchByLeague(@PathVariable String leagu * * @param squadNumber the squad number (natural key) of the player to update * @param playerDTO the complete player data (must pass validation) - * @return 204 No Content if successful, 404 Not Found if player doesn't exist, or 400 Bad Request if validation fails + * @return 204 No Content if successful, 404 Not Found if player doesn't exist, or 422 Unprocessable Entity if validation fails */ @PutMapping("/players/{squadNumber}") @Operation(summary = "Updates (entirely) a player by squad number") @ApiResponses(value = { @ApiResponse(responseCode = "204", description = "No Content", content = @Content), - @ApiResponse(responseCode = "400", description = "Bad Request", content = @Content), - @ApiResponse(responseCode = "404", description = "Not Found", content = @Content) + @ApiResponse(responseCode = "404", description = "Not Found", content = @Content), + @ApiResponse(responseCode = "422", description = "Unprocessable Entity - Validation failure", content = @Content) }) public ResponseEntity put(@PathVariable Integer squadNumber, @RequestBody @Valid PlayerDTO playerDTO) { if (!playerDTO.getSquadNumber().equals(squadNumber)) { - return ResponseEntity.status(HttpStatus.BAD_REQUEST).build(); + return ResponseEntity.status(HttpStatus.UNPROCESSABLE_ENTITY).build(); } playerDTO.setSquadNumber(squadNumber); boolean updated = playersService.update(squadNumber, playerDTO); diff --git a/src/test/java/ar/com/nanotaboada/java/samples/spring/boot/test/controllers/PlayersControllerTests.java b/src/test/java/ar/com/nanotaboada/java/samples/spring/boot/test/controllers/PlayersControllerTests.java index 1488175..ac7076f 100644 --- a/src/test/java/ar/com/nanotaboada/java/samples/spring/boot/test/controllers/PlayersControllerTests.java +++ b/src/test/java/ar/com/nanotaboada/java/samples/spring/boot/test/controllers/PlayersControllerTests.java @@ -103,10 +103,10 @@ void givenValidPlayer_whenPost_thenReturnsCreated() /** * Given invalid player data is provided (validation fails) * When attempting to create a player - * Then response status is 400 Bad Request and service is never called + * Then response status is 422 Unprocessable Entity and service is never called */ @Test - void givenInvalidPlayer_whenPost_thenReturnsBadRequest() + void givenInvalidPlayer_whenPost_thenReturnsUnprocessableEntity() throws Exception { // Given PlayerDTO dto = PlayerDTOFakes.createOneInvalid(); @@ -122,6 +122,29 @@ void givenInvalidPlayer_whenPost_thenReturnsBadRequest() .getResponse(); // Then verify(playersServiceMock, never()).create(any(PlayerDTO.class)); + then(response.getStatus()).isEqualTo(HttpStatus.UNPROCESSABLE_ENTITY.value()); + } + + /** + * Given malformed JSON is provided (unparseable body) + * When attempting to create a player + * Then response status is 400 Bad Request and service is never called + */ + @Test + void givenMalformedJson_whenPost_thenReturnsBadRequest() + throws Exception { + // Given + MockHttpServletRequestBuilder request = MockMvcRequestBuilders + .post(PATH) + .content("{ not-valid-json ") + .contentType(MediaType.APPLICATION_JSON); + // When + MockHttpServletResponse response = application + .perform(request) + .andReturn() + .getResponse(); + // Then + verify(playersServiceMock, never()).create(any(PlayerDTO.class)); then(response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value()); } @@ -437,10 +460,10 @@ void givenUnknownPlayer_whenPut_thenReturnsNotFound() /** * Given invalid player data is provided (validation fails) * When attempting to update a player - * Then response status is 400 Bad Request and service is never called + * Then response status is 422 Unprocessable Entity and service is never called */ @Test - void givenInvalidPlayer_whenPut_thenReturnsBadRequest() + void givenInvalidPlayer_whenPut_thenReturnsUnprocessableEntity() throws Exception { // Given PlayerDTO dto = PlayerDTOFakes.createOneInvalid(); @@ -456,16 +479,16 @@ void givenInvalidPlayer_whenPut_thenReturnsBadRequest() .getResponse(); // Then verify(playersServiceMock, never()).update(anyInt(), any(PlayerDTO.class)); - then(response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value()); + then(response.getStatus()).isEqualTo(HttpStatus.UNPROCESSABLE_ENTITY.value()); } /** * Given the path squad number does not match the body squad number * When attempting to update a player - * Then response status is 400 Bad Request and service is never called + * Then response status is 422 Unprocessable Entity and service is never called */ @Test - void givenSquadNumberMismatch_whenPut_thenReturnsBadRequest() + void givenSquadNumberMismatch_whenPut_thenReturnsUnprocessableEntity() throws Exception { // Given PlayerDTO dto = PlayerDTOFakes.createOneValid(); @@ -483,16 +506,16 @@ void givenSquadNumberMismatch_whenPut_thenReturnsBadRequest() .getResponse(); // Then verify(playersServiceMock, never()).update(anyInt(), any(PlayerDTO.class)); - then(response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value()); + then(response.getStatus()).isEqualTo(HttpStatus.UNPROCESSABLE_ENTITY.value()); } /** * Given the body squad number is null * When attempting to update a player - * Then response status is 400 Bad Request (squad number is required) + * Then response status is 422 Unprocessable Entity (squad number is required) */ @Test - void givenNullBodySquadNumber_whenPut_thenReturnsBadRequest() + void givenNullBodySquadNumber_whenPut_thenReturnsUnprocessableEntity() throws Exception { // Given PlayerDTO dto = PlayerDTOFakes.createOneValid(); @@ -509,7 +532,7 @@ void givenNullBodySquadNumber_whenPut_thenReturnsBadRequest() .getResponse(); // Then verify(playersServiceMock, never()).update(anyInt(), any(PlayerDTO.class)); - then(response.getStatus()).isEqualTo(HttpStatus.BAD_REQUEST.value()); + then(response.getStatus()).isEqualTo(HttpStatus.UNPROCESSABLE_ENTITY.value()); } /*