Bad Request Error while trying to get pet for a specific owner

[firasrg]

The GET/api/owners/{id}/pets/{petId} endpoint seems to be returning an incorrect response or handling errors improperly. This issue was encountered while attempting to retrieve specific pet for a given owner.

cURL:

curl -X 'GET' \
  'http://localhost:9966/petclinic/api/owners/1/pets/1' \
  -H 'accept: application/json'

Response on Swagger-ui: Error: response status is 400

I tried to debug code and found out that it comes from OwnerRestController :

@RestController
@RequestMapping("/api")
public class OwnerRestController implements OwnersApi {

    // ...
    
    @PreAuthorize("hasRole(@roles.OWNER_ADMIN)")
    @Override
    public ResponseEntity<PetDto> getOwnersPet(Integer ownerId, Integer petId) {
        Owner owner = this.clinicService.findOwnerById(ownerId);
        Pet pet = this.clinicService.findPetById(petId);
        if (owner == null || pet == null) {
            return new ResponseEntity<>(HttpStatus.NOT_FOUND);
        } else {
            if (!pet.getOwner().equals(owner)) {
                return new ResponseEntity<>(HttpStatus.BAD_REQUEST);
            } else {
                return new ResponseEntity<>(petMapper.toPetDto(pet), HttpStatus.OK);
            }
        }
    }
}

The problem is here : if (!pet.getOwner().equals(owner)) {}: the equals() check returns false. Ive checked the entity Owner class and found out that it doesn't have equals and hashcode() (same for Pet entity class), which is not recommended !

[arey]

The equals method is not implemented in the Owner entity. Instead of I propose to use the getId() for comparing both instances :

if (!pet.getOwner().getId().equals(owner.getId()))

[firasrg]

I think that it would be more readable if we have something like this :

Pet pet = this.clinicService.findPetByOwner(ownerId, petId);

And the body of findPetByOwner() would be like this:

Owner owner = this.clinicService.findOwnerById(ownerId);
Pet pet = owner.getPets().stream().filter(pet -> pet.getId().equals(petId)).findFirst().orElseThrow();

[arey]

To fix this issue, we can start by adding a failed test case to the OwnerRestControllerTests.

Then, you're right: I'm not sure we need to call the findPetById method. The findOwnerById calls is enough.

For error handling, we have to take care to return a 404 or a 400 http error. I'm not sure the orElseThrow() is the best way to ensure it.

[arey]

@firasrg I let you review my fix in the PR bad request on GET /api/owners/{id}/pets/{pe…](#150)