From 072f200378107f06953f486a1f3f761bebd42986 Mon Sep 17 00:00:00 2001
From: Antoine Rey <antoine.rey@free.fr>
Date: Sat, 21 Sep 2024 07:16:36 +0200
Subject: [PATCH] Fix #145 bad request on GET /api/owners/{id}/pets/{petId}
(#150)
* Fix #145 bad request on GET /api/owners/{id}/pets/{petId}
* Fix #145 404 error for both: owner and pet not found
* Fix #145 404 error for both: update the OAS description
* Fix #145 404 error for both: refactoring by introducing a Owner::getPet(Integer petId) method
---
.../samples/petclinic/model/Owner.java | 4 ++++
.../rest/controller/OwnerRestController.java | 11 ++++-------
src/main/resources/openapi.yml | 2 +-
.../controller/OwnerRestControllerTests.java | 16 +++++++++++-----
4 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/src/main/java/org/springframework/samples/petclinic/model/Owner.java b/src/main/java/org/springframework/samples/petclinic/model/Owner.java
index 80c89359..8cf3a6d6 100644
--- a/src/main/java/org/springframework/samples/petclinic/model/Owner.java
+++ b/src/main/java/org/springframework/samples/petclinic/model/Owner.java
@@ -133,6 +133,10 @@ public class Owner extends Person {
return null;
}
+ public Pet getPet(Integer petId) {
+ return getPetsInternal().stream().filter(p -> p.getId().equals(petId)).findFirst().orElse(null);
+ }
+
@Override
public String toString() {
return new ToStringCreator(this)
diff --git a/src/main/java/org/springframework/samples/petclinic/rest/controller/OwnerRestController.java b/src/main/java/org/springframework/samples/petclinic/rest/controller/OwnerRestController.java
index 2b010733..906f8f54 100644
--- a/src/main/java/org/springframework/samples/petclinic/rest/controller/OwnerRestController.java
+++ b/src/main/java/org/springframework/samples/petclinic/rest/controller/OwnerRestController.java
@@ -166,15 +166,12 @@ public class OwnerRestController implements OwnersApi {
@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 {
+ if (owner != null) {
+ Pet pet = owner.getPet(petId);
+ if (pet != null) {
return new ResponseEntity<>(petMapper.toPetDto(pet), HttpStatus.OK);
}
}
+ return new ResponseEntity<>(HttpStatus.NOT_FOUND);
}
}
diff --git a/src/main/resources/openapi.yml b/src/main/resources/openapi.yml
index 86699c03..704df3dd 100755
--- a/src/main/resources/openapi.yml
+++ b/src/main/resources/openapi.yml
@@ -389,7 +389,7 @@ paths:
schema:
$ref: '#/components/schemas/RestError'
404:
- description: Pet not found.
+ description: Owner or pet not found.
content:
application/json:
schema:
diff --git a/src/test/java/org/springframework/samples/petclinic/rest/controller/OwnerRestControllerTests.java b/src/test/java/org/springframework/samples/petclinic/rest/controller/OwnerRestControllerTests.java
index 07bb8cb9..c4561289 100644
--- a/src/test/java/org/springframework/samples/petclinic/rest/controller/OwnerRestControllerTests.java
+++ b/src/test/java/org/springframework/samples/petclinic/rest/controller/OwnerRestControllerTests.java
@@ -29,7 +29,6 @@ import org.springframework.samples.petclinic.mapper.OwnerMapper;
import org.springframework.samples.petclinic.mapper.PetMapper;
import org.springframework.samples.petclinic.mapper.VisitMapper;
import org.springframework.samples.petclinic.model.Owner;
-import org.springframework.samples.petclinic.model.Pet;
import org.springframework.samples.petclinic.rest.advice.ExceptionControllerAdvice;
import org.springframework.samples.petclinic.rest.dto.OwnerDto;
import org.springframework.samples.petclinic.rest.dto.PetDto;
@@ -399,9 +398,6 @@ class OwnerRestControllerTests {
@Test
@WithMockUser(roles = "OWNER_ADMIN")
void testGetOwnerPetSuccess() throws Exception {
- owners.remove(0);
- owners.remove(1);
- given(this.clinicService.findAllOwners()).willReturn(ownerMapper.toOwners(owners));
var owner = ownerMapper.toOwner(owners.get(0));
given(this.clinicService.findOwnerById(2)).willReturn(owner);
var pet = petMapper.toPet(pets.get(0));
@@ -415,7 +411,7 @@ class OwnerRestControllerTests {
@Test
@WithMockUser(roles = "OWNER_ADMIN")
- void testGetOwnersPetsNotFound() throws Exception {
+ void testGetOwnersPetsWithOwnerNotFound() throws Exception {
owners.clear();
given(this.clinicService.findAllOwners()).willReturn(ownerMapper.toOwners(owners));
this.mockMvc.perform(get("/api/owners/1/pets/1")
@@ -423,5 +419,15 @@ class OwnerRestControllerTests {
.andExpect(status().isNotFound());
}
+ @Test
+ @WithMockUser(roles = "OWNER_ADMIN")
+ void testGetOwnersPetsWithPetNotFound() throws Exception {
+ var owner1 = ownerMapper.toOwner(owners.get(0));
+ given(this.clinicService.findOwnerById(1)).willReturn(owner1);
+ this.mockMvc.perform(get("/api/owners/1/pets/2")
+ .accept(MediaType.APPLICATION_JSON))
+ .andExpect(status().isNotFound());
+ }
+
}
--
GitLab