Skip to content

Fix #305: Sanitize error details and add validation tests for owner pets #306

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.

Sign up for GitHub

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

Merged
vfedoriv merged 6 commits into from
Mar 30, 2026
Merged

Fix #305: Sanitize error details and add validation tests for owner pets #306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d18b7c4
fix(advice): sanitize error details and add validation tests for owne…
San-43 Mar 15, 2026
396571e
test(advice): add generic-error response tests for owner/pet endpoints
San-43 Mar 15, 2026
5ee3fa8
test(owner): fix typos in variable names in createOwner error handlin…
San-43 Mar 16, 2026
0dc4e64
fix(advice): include binding errors in validation error handling
San-43 Mar 16, 2026
4c5c1cc
fix(advice): enhance validation error handling to include schema vali…
San-43 Mar 17, 2026
9d3c02b
"fix(advice): Map validation errors to ValidationMessageDTO instead o…
San-43 Mar 25, 2026
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
80 changes: 56 additions & 24 deletions ...ain/java/org/springframework/samples/petclinic/rest/advice/ExceptionControllerAdvice.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,21 +16,26 @@

package org.springframework.samples.petclinic.rest.advice;

import java.net.URI;
import java.time.Instant;
import java.util.List;
import java.util.Objects;

import jakarta.servlet.http.HttpServletRequest;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.dao.DataIntegrityViolationException;
import org.springframework.http.HttpStatus;
import org.springframework.http.ProblemDetail;
import org.springframework.http.ResponseEntity;
import org.springframework.samples.petclinic.rest.controller.BindingErrorsResponse;
import org.springframework.samples.petclinic.rest.dto.ValidationMessageDto;
import org.springframework.validation.BindingResult;
import org.springframework.web.bind.MethodArgumentNotValidException;
import org.springframework.web.bind.annotation.ControllerAdvice;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.ResponseBody;

import java.net.URI;
import java.time.Instant;

/**
* Global Exception handler for REST controllers.
* <p>
Expand All @@ -42,21 +47,27 @@
@ControllerAdvice
public class ExceptionControllerAdvice {

private static final Logger logger = LoggerFactory.getLogger(ExceptionControllerAdvice.class);
private static final String ERROR_UNEXPECTED = "An unexpected error occurred while processing your request";
private static final String ERROR_DATA_INTEGRITY = "The requested resource could not be processed due to a data constraint violation";
private static final String ERROR_INVALID_REQUEST = "The request contains invalid or missing parameters";

/**
* Private method for constructing the {@link ProblemDetail} object passing the name and details of the exception
* class.
*
* @param ex Object referring to the thrown exception.
* @param e Object referring to the thrown exception.
* @param status HTTP response status.
* @param url URL request.
*/
private ProblemDetail detailBuild(Exception ex, HttpStatus status, StringBuffer url) {
ProblemDetail detail = ProblemDetail.forStatus(status);
detail.setType(URI.create(url.toString()));
detail.setTitle(ex.getClass().getSimpleName());
detail.setDetail(ex.getLocalizedMessage());
detail.setProperty("timestamp", Instant.now());
return detail;
private ProblemDetail detailBuild(Exception e, HttpStatus status, StringBuffer url, String detail) {
ProblemDetail problemDetail = ProblemDetail.forStatus(status);
problemDetail.setType(URI.create(url.toString()));
problemDetail.setTitle(e.getClass().getSimpleName());
problemDetail.setDetail(detail);
problemDetail.setProperty("timestamp", Instant.now());
problemDetail.setProperty("schemaValidationErrors", List.<ValidationMessageDto>of());
return problemDetail;
}

/**
Expand All @@ -69,50 +80,71 @@ private ProblemDetail detailBuild(Exception ex, HttpStatus status, StringBuffer
@ExceptionHandler(Exception.class)
@ResponseBody
public ResponseEntity<ProblemDetail> handleGeneralException(Exception e, HttpServletRequest request) {
logger.error("Unexpected error at {} {}", request.getMethod(), request.getRequestURI(), e);
HttpStatus status = HttpStatus.INTERNAL_SERVER_ERROR;
ProblemDetail detail = this.detailBuild(e, status, request.getRequestURL());
ProblemDetail detail = this.detailBuild(e, status, request.getRequestURL(), ERROR_UNEXPECTED);
return ResponseEntity.status(status).body(detail);
}

/**
* Handles {@link DataIntegrityViolationException} which typically indicates database constraint violations. This
* method returns a 404 Not Found status if an entity does not exist.
*
* @param ex The {@link DataIntegrityViolationException} to be handled
* @param e The {@link DataIntegrityViolationException} to be handled
* @param request {@link HttpServletRequest} object referring to the current request.
* @return A {@link ResponseEntity} containing the error information and a 404 Not Found status
*/
@ExceptionHandler(DataIntegrityViolationException.class)
@ResponseBody
public ResponseEntity<ProblemDetail> handleDataIntegrityViolationException(DataIntegrityViolationException ex, HttpServletRequest request) {
public ResponseEntity<ProblemDetail> handleDataIntegrityViolationException(DataIntegrityViolationException e, HttpServletRequest request) {
logger.warn("Data integrity violation at {} {}: {}",
request.getMethod(),
request.getRequestURI(),
e.getMessage());
logger.debug("Data integrity violation stacktrace", e);
HttpStatus status = HttpStatus.NOT_FOUND;
ProblemDetail detail = ProblemDetail.forStatus(status);
detail.setType(URI.create(request.getRequestURL().toString()));
detail.setTitle(ex.getClass().getSimpleName());
detail.setDetail("Request could not be processed");
detail.setProperty("timestamp", Instant.now());
ProblemDetail detail = this.detailBuild(e, status, request.getRequestURL(), ERROR_DATA_INTEGRITY);
return ResponseEntity.status(status).body(detail);
}

/**
* Handles exception thrown by Bean Validation on controller methods parameters
*
* @param ex The {@link MethodArgumentNotValidException} to be handled
* @param e The {@link MethodArgumentNotValidException} to be handled
* @param request {@link HttpServletRequest} object referring to the current request.
* @return A {@link ResponseEntity} containing the error information and a 400 Bad Request status.
*/
@ExceptionHandler(MethodArgumentNotValidException.class)
@ResponseBody
public ResponseEntity<ProblemDetail> handleMethodArgumentNotValidException(MethodArgumentNotValidException ex, HttpServletRequest request) {
public ResponseEntity<ProblemDetail> handleMethodArgumentNotValidException(MethodArgumentNotValidException e, HttpServletRequest request) {
HttpStatus status = HttpStatus.BAD_REQUEST;
BindingErrorsResponse errors = new BindingErrorsResponse();
BindingResult bindingResult = ex.getBindingResult();
BindingResult bindingResult = e.getBindingResult();
ProblemDetail detail = this.detailBuild(e, status, request.getRequestURL(), ERROR_INVALID_REQUEST);
if (bindingResult.hasErrors()) {
errors.addAllErrors(bindingResult);
ProblemDetail detail = this.detailBuild(ex, status, request.getRequestURL());
List<ValidationMessageDto> schemaValidationErrors = bindingResult.getFieldErrors().stream()
.map(fieldError -> {
String rejectedValue = Objects.toString(fieldError.getRejectedValue(), "null");
String defaultMessage = Objects.toString(fieldError.getDefaultMessage(), "Validation failed");
String message = "Field '%s' %s (rejected value: %s)".formatted(
fieldError.getField(),
defaultMessage,
rejectedValue);
return new ValidationMessageDto(message)
.putAdditionalProperty("field", fieldError.getField())
.putAdditionalProperty("rejectedValue", rejectedValue)
.putAdditionalProperty("defaultMessage", defaultMessage);
})
.toList();
logger.debug("Validation error at {} {}: {}",
request.getMethod(),
request.getRequestURI(),
bindingResult.getFieldErrors());
detail.setProperty("schemaValidationErrors", schemaValidationErrors);
return ResponseEntity.status(status).body(detail);
}
return ResponseEntity.status(status).build();
return ResponseEntity.status(status).body(detail);
}

}
110 changes: 109 additions & 1 deletion .../java/org/springframework/samples/petclinic/rest/controller/OwnerRestControllerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ void testCreatePetShouldNotExposeTechnicalDetails() throws Exception {
.content(newPetAsJSON).accept(MediaType.APPLICATION_JSON_VALUE).contentType(MediaType.APPLICATION_JSON_VALUE))
.andDo(MockMvcResultHandlers.print())
.andExpect(status().isNotFound())
.andExpect(jsonPath("$.detail").value("Request could not be processed"));
.andExpect(jsonPath("$.detail").value("The requested resource could not be processed due to a data constraint violation"));
}

@Test
Expand All @@ -407,6 +407,114 @@ void testCreatePetWithUnknownOwnerShouldReturnNotFound() throws Exception {
.andExpect(status().isNotFound());
}

@Test
@WithMockUser(roles = "OWNER_ADMIN")
void testCreatePetWithNullTypeShouldReturnBadRequestWithGenericDetail() throws Exception {
PetDto newPet = pets.get(0);
newPet.setId(null);
newPet.setType(null);
ObjectMapper mapper = JsonMapper.builder()
.defaultDateFormat(new SimpleDateFormat("dd/MM/yyyy"))
.build();
String newPetAsJSON = mapper.writeValueAsString(newPet);
this.mockMvc.perform(post("/api/owners/1/pets")
.content(newPetAsJSON).accept(MediaType.APPLICATION_JSON_VALUE).contentType(MediaType.APPLICATION_JSON_VALUE))
.andDo(MockMvcResultHandlers.print())
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.detail").value("The request contains invalid or missing parameters"))
.andExpect(jsonPath("$.title").value("MethodArgumentNotValidException"))
.andExpect(jsonPath("$.schemaValidationErrors").isArray())
.andExpect(jsonPath("$.schemaValidationErrors[0].message").exists())
.andExpect(jsonPath("$.schemaValidationErrors[0].field").exists())
.andExpect(jsonPath("$.schemaValidationErrors[0].rejectedValue").exists())
.andExpect(jsonPath("$.schemaValidationErrors[0].defaultMessage").exists());
}

@Test
@WithMockUser(roles = "OWNER_ADMIN")
void testCreatePetWithEmptyTypeNameShouldReturnBadRequestWithGenericDetail() throws Exception {
PetDto newPet = pets.get(0);
newPet.setId(null);
newPet.getType().setName("");
ObjectMapper mapper = JsonMapper.builder()
.defaultDateFormat(new SimpleDateFormat("dd/MM/yyyy"))
.build();
String newPetAsJSON = mapper.writeValueAsString(newPet);
this.mockMvc.perform(post("/api/owners/1/pets")
.content(newPetAsJSON).accept(MediaType.APPLICATION_JSON_VALUE).contentType(MediaType.APPLICATION_JSON_VALUE))
.andDo(MockMvcResultHandlers.print())
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.detail").value("The request contains invalid or missing parameters"))
.andExpect(jsonPath("$.title").value("MethodArgumentNotValidException"))
.andExpect(jsonPath("$.schemaValidationErrors").isArray())
.andExpect(jsonPath("$.schemaValidationErrors[0].message").exists())
.andExpect(jsonPath("$.schemaValidationErrors[0].field").exists())
.andExpect(jsonPath("$.schemaValidationErrors[0].rejectedValue").exists())
.andExpect(jsonPath("$.schemaValidationErrors[0].defaultMessage").exists());
}

@Test
@WithMockUser(roles = "OWNER_ADMIN")
void testCreatePetWithNullTypeIdShouldReturnBadRequestWithGenericDetail() throws Exception {
PetDto newPet = pets.get(0);
newPet.setId(null);
newPet.getType().setId(null);
ObjectMapper mapper = JsonMapper.builder()
.defaultDateFormat(new SimpleDateFormat("dd/MM/yyyy"))
.build();
String newPetAsJSON = mapper.writeValueAsString(newPet);
this.mockMvc.perform(post("/api/owners/1/pets")
.content(newPetAsJSON).accept(MediaType.APPLICATION_JSON_VALUE).contentType(MediaType.APPLICATION_JSON_VALUE))
.andDo(MockMvcResultHandlers.print())
.andExpect(status().isBadRequest())
.andExpect(jsonPath("$.detail").value("The request contains invalid or missing parameters"))
.andExpect(jsonPath("$.title").value("MethodArgumentNotValidException"))
.andExpect(jsonPath("$.schemaValidationErrors").isArray())
.andExpect(jsonPath("$.schemaValidationErrors[0].message").exists())
.andExpect(jsonPath("$.schemaValidationErrors[0].field").exists())
.andExpect(jsonPath("$.schemaValidationErrors[0].rejectedValue").exists())
.andExpect(jsonPath("$.schemaValidationErrors[0].defaultMessage").exists());
}

@Test
@WithMockUser(roles = "OWNER_ADMIN")
void testCreatePetWithUnexpectedErrorShouldReturnInternalServerErrorWithGenericDetail() throws Exception {
PetDto newPet = pets.get(0);
newPet.setId(null);
ObjectMapper mapper = JsonMapper.builder()
.defaultDateFormat(new SimpleDateFormat("dd/MM/yyyy"))
.build();
String newPetAsJSON = mapper.writeValueAsString(newPet);
given(this.clinicService.findOwnerById(1)).willReturn(ownerMapper.toOwner(owners.get(0)));
doThrow(new IllegalStateException("JDBC timeout while executing insert into pets"))
.when(this.clinicService).savePet(any());
this.mockMvc.perform(post("/api/owners/1/pets")
.content(newPetAsJSON).accept(MediaType.APPLICATION_JSON_VALUE).contentType(MediaType.APPLICATION_JSON_VALUE))
.andDo(MockMvcResultHandlers.print())
.andExpect(status().isInternalServerError())
.andExpect(jsonPath("$.detail").value("An unexpected error occurred while processing your request"))
.andExpect(jsonPath("$.title").value("IllegalStateException"))
.andExpect(jsonPath("$.timestamp").exists());
}

@Test
@WithMockUser(roles = "OWNER_ADMIN")
void testCreateOwnerWithUnexpectedErrorShouldReturnInternalServerErrorWithGenericDetail() throws Exception {
OwnerDto newOwnerDto = owners.get(0);
newOwnerDto.setId(null);
ObjectMapper mapper = new ObjectMapper();
String newOwnerAsJSON = mapper.writeValueAsString(newOwnerDto);
doThrow(new RuntimeException("Low-level persistence exception details"))
.when(this.clinicService).saveOwner(any());
this.mockMvc.perform(post("/api/owners")
.content(newOwnerAsJSON).accept(MediaType.APPLICATION_JSON_VALUE).contentType(MediaType.APPLICATION_JSON_VALUE))
.andDo(MockMvcResultHandlers.print())
.andExpect(status().isInternalServerError())
.andExpect(jsonPath("$.detail").value("An unexpected error occurred while processing your request"))
.andExpect(jsonPath("$.title").value("RuntimeException"))
.andExpect(jsonPath("$.timestamp").exists());
}

@Test
@WithMockUser(roles = "OWNER_ADMIN")
void testCreateVisitSuccess() throws Exception {
Expand Down
Loading