Skip to content
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
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Void> handleValidationException(MethodArgumentNotValidException exception) {
return ResponseEntity.status(HttpStatus.UNPROCESSABLE_ENTITY).build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
* <li><b>200 OK:</b> Successful retrieval</li>
* <li><b>201 Created:</b> Successful creation (with Location header)</li>
* <li><b>204 No Content:</b> Successful update/delete</li>
* <li><b>400 Bad Request:</b> Validation failure</li>
* <li><b>422 Unprocessable Entity:</b> Validation failure</li>
* <li><b>404 Not Found:</b> Resource not found</li>
* </ul>
*
Expand Down Expand Up @@ -80,14 +80,14 @@ public class PlayersController {
* </p>
*
* @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<Void> post(@RequestBody @Valid PlayerDTO playerDTO) {
PlayerDTO createdPlayer = playersService.create(playerDTO);
Expand Down Expand Up @@ -200,18 +200,18 @@ public ResponseEntity<List<PlayerDTO>> 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<Void> 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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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());
}

Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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());
}

/*
Expand Down
Loading