From 668629d5bda4e6a87fb43425c7eb95ffa19b47c7 Mon Sep 17 00:00:00 2001 From: YiXuan Ding <1328032567@qq.com> Date: Fri, 8 Nov 2024 22:05:22 +0800 Subject: [PATCH] refactor `OwnerRepository`: -: use `JpaRepository` to replace `Repository` in `OwnerRepository` class. -: remove `save()` method. JpaRepository provides it by default. -: remove `@Query` because in `Owner` class, the `@OneToMany` annotiation achieved `fetch` in query. -: use `Optional` to recieve the result from `findById()`, and if is null, throw `IllegalArugmentExpection`. -: achieve the assert to judge return value in tests. -: add name to `@author` tag. --- .../samples/petclinic/model/NamedEntity.java | 1 + .../samples/petclinic/owner/Owner.java | 1 + .../petclinic/owner/OwnerController.java | 11 ++++- .../petclinic/owner/OwnerRepository.java | 30 ++++++++------ .../samples/petclinic/owner/Pet.java | 1 + .../petclinic/owner/PetController.java | 11 +++-- .../petclinic/owner/VisitController.java | 7 +++- .../petclinic/owner/OwnerControllerTests.java | 6 ++- .../petclinic/owner/PetControllerTests.java | 3 +- .../petclinic/owner/VisitControllerTests.java | 5 ++- .../petclinic/service/ClinicServiceTests.java | 41 +++++++++++++++---- 11 files changed, 85 insertions(+), 32 deletions(-) diff --git a/src/main/java/org/springframework/samples/petclinic/model/NamedEntity.java b/src/main/java/org/springframework/samples/petclinic/model/NamedEntity.java index cf2ae3645b1..012e8c4be74 100644 --- a/src/main/java/org/springframework/samples/petclinic/model/NamedEntity.java +++ b/src/main/java/org/springframework/samples/petclinic/model/NamedEntity.java @@ -25,6 +25,7 @@ * * @author Ken Krebs * @author Juergen Hoeller + * @author Wick Dynex */ @MappedSuperclass public class NamedEntity extends BaseEntity { diff --git a/src/main/java/org/springframework/samples/petclinic/owner/Owner.java b/src/main/java/org/springframework/samples/petclinic/owner/Owner.java index 612d89d2dc6..675b2140e45 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/Owner.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/Owner.java @@ -41,6 +41,7 @@ * @author Sam Brannen * @author Michael Isvy * @author Oliver Drotbohm + * @author Wick Dynex */ @Entity @Table(name = "owners") diff --git a/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java b/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java index 490b1028709..463a94c7331 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java @@ -16,6 +16,7 @@ package org.springframework.samples.petclinic.owner; import java.util.List; +import java.util.Optional; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; @@ -40,6 +41,7 @@ * @author Ken Krebs * @author Arjen Poutsma * @author Michael Isvy + * @author Wick Dynex */ @Controller class OwnerController { @@ -59,7 +61,10 @@ public void setAllowedFields(WebDataBinder dataBinder) { @ModelAttribute("owner") public Owner findOwner(@PathVariable(name = "ownerId", required = false) Integer ownerId) { - return ownerId == null ? new Owner() : this.owners.findById(ownerId); + return ownerId == null ? new Owner() + : this.owners.findById(ownerId) + .orElseThrow(() -> new IllegalArgumentException("Owner not found with id: " + ownerId + + ". Please ensure the ID is correct " + "and the owner exists in the database.")); } @GetMapping("/owners/new") @@ -158,7 +163,9 @@ public String processUpdateOwnerForm(@Valid Owner owner, BindingResult result, @ @GetMapping("/owners/{ownerId}") public ModelAndView showOwner(@PathVariable("ownerId") int ownerId) { ModelAndView mav = new ModelAndView("owners/ownerDetails"); - Owner owner = this.owners.findById(ownerId); + Optional optionalOwner = this.owners.findById(ownerId); + Owner owner = optionalOwner.orElseThrow(() -> new IllegalArgumentException( + "Owner not found with id: " + ownerId + ". Please ensure the ID is correct ")); mav.addObject(owner); return mav; } diff --git a/src/main/java/org/springframework/samples/petclinic/owner/OwnerRepository.java b/src/main/java/org/springframework/samples/petclinic/owner/OwnerRepository.java index f444494392a..7bbb91e93b0 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/OwnerRepository.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/OwnerRepository.java @@ -16,12 +16,16 @@ package org.springframework.samples.petclinic.owner; import java.util.List; +import java.util.Optional; +import jakarta.annotation.Nonnull; +import jakarta.validation.constraints.NotNull; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; +import org.springframework.data.jpa.repository.JpaRepository; import org.springframework.data.jpa.repository.Query; -import org.springframework.data.repository.Repository; import org.springframework.data.repository.query.Param; +import org.springframework.lang.NonNullApi; import org.springframework.transaction.annotation.Transactional; /** @@ -34,8 +38,9 @@ * @author Juergen Hoeller * @author Sam Brannen * @author Michael Isvy + * @author Wick Dynex */ -public interface OwnerRepository extends Repository { +public interface OwnerRepository extends JpaRepository { /** * Retrieve all {@link PetType}s from the data store. @@ -52,25 +57,24 @@ public interface OwnerRepository extends Repository { * @return a Collection of matching {@link Owner}s (or an empty Collection if none * found) */ - @Query("SELECT DISTINCT owner FROM Owner owner left join owner.pets WHERE owner.lastName LIKE :lastName% ") @Transactional(readOnly = true) Page findByLastName(@Param("lastName") String lastName, Pageable pageable); /** * Retrieve an {@link Owner} from the data store by id. + *

+ * This method returns an {@link Optional} containing the {@link Owner} if found. If + * no {@link Owner} is found with the provided id, it will return an empty + * {@link Optional}. + *

* @param id the id to search for - * @return the {@link Owner} if found - */ - @Query("SELECT owner FROM Owner owner left join fetch owner.pets WHERE owner.id =:id") - @Transactional(readOnly = true) - Owner findById(@Param("id") Integer id); - - /** - * Save an {@link Owner} to the data store, either inserting or updating it. - * @param owner the {@link Owner} to save + * @return an {@link Optional} containing the {@link Owner} if found, or an empty + * {@link Optional} if not found. + * @throws IllegalArgumentException if the id is null (assuming null is not a valid + * input for id) */ - void save(Owner owner); + Optional findById(@Nonnull Integer id); /** * Returns all the owners from data store diff --git a/src/main/java/org/springframework/samples/petclinic/owner/Pet.java b/src/main/java/org/springframework/samples/petclinic/owner/Pet.java index 2e5ea948750..03fd26c9375 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/Pet.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/Pet.java @@ -39,6 +39,7 @@ * @author Ken Krebs * @author Juergen Hoeller * @author Sam Brannen + * @author Wick Dynex */ @Entity @Table(name = "pets") diff --git a/src/main/java/org/springframework/samples/petclinic/owner/PetController.java b/src/main/java/org/springframework/samples/petclinic/owner/PetController.java index cde8e59e345..f5dc7a358a0 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/PetController.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/PetController.java @@ -17,6 +17,7 @@ import java.time.LocalDate; import java.util.Collection; +import java.util.Optional; import org.springframework.stereotype.Controller; import org.springframework.ui.ModelMap; @@ -37,6 +38,7 @@ * @author Juergen Hoeller * @author Ken Krebs * @author Arjen Poutsma + * @author Wick Dynex */ @Controller @RequestMapping("/owners/{ownerId}") @@ -57,8 +59,9 @@ public Collection populatePetTypes() { @ModelAttribute("owner") public Owner findOwner(@PathVariable("ownerId") int ownerId) { - - Owner owner = this.owners.findById(ownerId); + Optional optionalOwner = this.owners.findById(ownerId); + Owner owner = optionalOwner.orElseThrow(() -> new IllegalArgumentException( + "Owner not found with id: " + ownerId + ". Please ensure the ID is correct ")); if (owner == null) { throw new IllegalArgumentException("Owner ID not found: " + ownerId); } @@ -73,7 +76,9 @@ public Pet findPet(@PathVariable("ownerId") int ownerId, return new Pet(); } - Owner owner = this.owners.findById(ownerId); + Optional optionalOwner = this.owners.findById(ownerId); + Owner owner = optionalOwner.orElseThrow(() -> new IllegalArgumentException( + "Owner not found with id: " + ownerId + ". Please ensure the ID is correct ")); if (owner == null) { throw new IllegalArgumentException("Owner ID not found: " + ownerId); } diff --git a/src/main/java/org/springframework/samples/petclinic/owner/VisitController.java b/src/main/java/org/springframework/samples/petclinic/owner/VisitController.java index 6c7a8dd1eac..542d4718415 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/VisitController.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/VisitController.java @@ -16,6 +16,8 @@ package org.springframework.samples.petclinic.owner; import java.util.Map; +import java.util.Optional; +import java.util.OptionalInt; import org.springframework.stereotype.Controller; import org.springframework.validation.BindingResult; @@ -35,6 +37,7 @@ * @author Arjen Poutsma * @author Michael Isvy * @author Dave Syer + * @author Wick Dynex */ @Controller class VisitController { @@ -60,7 +63,9 @@ public void setAllowedFields(WebDataBinder dataBinder) { @ModelAttribute("visit") public Visit loadPetWithVisit(@PathVariable("ownerId") int ownerId, @PathVariable("petId") int petId, Map model) { - Owner owner = this.owners.findById(ownerId); + Optional optionalOwner = owners.findById(ownerId); + Owner owner = optionalOwner.orElseThrow(() -> new IllegalArgumentException( + "Owner not found with id: " + ownerId + ". Please ensure the ID is correct ")); Pet pet = owner.getPet(petId); model.put("pet", pet); diff --git a/src/test/java/org/springframework/samples/petclinic/owner/OwnerControllerTests.java b/src/test/java/org/springframework/samples/petclinic/owner/OwnerControllerTests.java index f88c1f1ddd9..40a40bc54fe 100644 --- a/src/test/java/org/springframework/samples/petclinic/owner/OwnerControllerTests.java +++ b/src/test/java/org/springframework/samples/petclinic/owner/OwnerControllerTests.java @@ -31,6 +31,7 @@ import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; import java.time.LocalDate; +import java.util.Optional; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.greaterThan; @@ -52,6 +53,7 @@ * Test class for {@link OwnerController} * * @author Colin But + * @author Wick Dynex */ @WebMvcTest(OwnerController.class) @DisabledInNativeImage @@ -94,7 +96,7 @@ void setup() { given(this.owners.findAll(any(Pageable.class))).willReturn(new PageImpl<>(Lists.newArrayList(george))); - given(this.owners.findById(TEST_OWNER_ID)).willReturn(george); + given(this.owners.findById(TEST_OWNER_ID)).willReturn(Optional.of(george)); Visit visit = new Visit(); visit.setDate(LocalDate.now()); george.getPet("Max").getVisits().add(visit); @@ -240,7 +242,7 @@ public void testProcessUpdateOwnerFormWithIdMismatch() throws Exception { owner.setCity("New York"); owner.setTelephone("0123456789"); - when(owners.findById(pathOwnerId)).thenReturn(owner); + when(owners.findById(pathOwnerId)).thenReturn(Optional.of(owner)); mockMvc.perform(MockMvcRequestBuilders.post("/owners/{ownerId}/edit", pathOwnerId).flashAttr("owner", owner)) .andExpect(status().is3xxRedirection()) diff --git a/src/test/java/org/springframework/samples/petclinic/owner/PetControllerTests.java b/src/test/java/org/springframework/samples/petclinic/owner/PetControllerTests.java index 657df198041..14ae32576b7 100644 --- a/src/test/java/org/springframework/samples/petclinic/owner/PetControllerTests.java +++ b/src/test/java/org/springframework/samples/petclinic/owner/PetControllerTests.java @@ -46,6 +46,7 @@ * Test class for the {@link PetController} * * @author Colin But + * @author Wick Dynex */ @WebMvcTest(value = PetController.class, includeFilters = @ComponentScan.Filter(value = PetTypeFormatter.class, type = FilterType.ASSIGNABLE_TYPE)) @@ -79,7 +80,7 @@ void setup() { dog.setId(TEST_PET_ID + 1); pet.setName("petty"); dog.setName("doggy"); - given(this.owners.findById(TEST_OWNER_ID)).willReturn(owner); + given(this.owners.findById(TEST_OWNER_ID)).willReturn(Optional.of(owner)); } @Test diff --git a/src/test/java/org/springframework/samples/petclinic/owner/VisitControllerTests.java b/src/test/java/org/springframework/samples/petclinic/owner/VisitControllerTests.java index 3565d92793e..8f44aace96a 100644 --- a/src/test/java/org/springframework/samples/petclinic/owner/VisitControllerTests.java +++ b/src/test/java/org/springframework/samples/petclinic/owner/VisitControllerTests.java @@ -32,10 +32,13 @@ import org.springframework.test.context.aot.DisabledInAotMode; import org.springframework.test.web.servlet.MockMvc; +import java.util.Optional; + /** * Test class for {@link VisitController} * * @author Colin But + * @author Wick Dynex */ @WebMvcTest(VisitController.class) @DisabledInNativeImage @@ -58,7 +61,7 @@ void init() { Pet pet = new Pet(); owner.addPet(pet); pet.setId(TEST_PET_ID); - given(this.owners.findById(TEST_OWNER_ID)).willReturn(owner); + given(this.owners.findById(TEST_OWNER_ID)).willReturn(Optional.of(owner)); } @Test diff --git a/src/test/java/org/springframework/samples/petclinic/service/ClinicServiceTests.java b/src/test/java/org/springframework/samples/petclinic/service/ClinicServiceTests.java index 92a70c38c07..39a444e2da7 100644 --- a/src/test/java/org/springframework/samples/petclinic/service/ClinicServiceTests.java +++ b/src/test/java/org/springframework/samples/petclinic/service/ClinicServiceTests.java @@ -20,6 +20,7 @@ import java.time.LocalDate; import java.util.Collection; +import java.util.Optional; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -90,7 +91,9 @@ void shouldFindOwnersByLastName() { @Test void shouldFindSingleOwnerWithPet() { - Owner owner = this.owners.findById(1); + Optional optionalOwner = this.owners.findById(1); + assertThat(optionalOwner).isPresent(); + Owner owner = optionalOwner.get(); assertThat(owner.getLastName()).startsWith("Franklin"); assertThat(owner.getPets()).hasSize(1); assertThat(owner.getPets().get(0).getType()).isNotNull(); @@ -119,7 +122,9 @@ void shouldInsertOwner() { @Test @Transactional void shouldUpdateOwner() { - Owner owner = this.owners.findById(1); + Optional optionalOwner = this.owners.findById(1); + assertThat(optionalOwner).isPresent(); + Owner owner = optionalOwner.get(); String oldLastName = owner.getLastName(); String newLastName = oldLastName + "X"; @@ -127,7 +132,9 @@ void shouldUpdateOwner() { this.owners.save(owner); // retrieving new name from database - owner = this.owners.findById(1); + optionalOwner = this.owners.findById(1); + assertThat(optionalOwner).isPresent(); + owner = optionalOwner.get(); assertThat(owner.getLastName()).isEqualTo(newLastName); } @@ -144,7 +151,10 @@ void shouldFindAllPetTypes() { @Test @Transactional void shouldInsertPetIntoDatabaseAndGenerateId() { - Owner owner6 = this.owners.findById(6); + Optional optionalOwner = this.owners.findById(6); + assertThat(optionalOwner).isPresent(); + Owner owner6 = optionalOwner.get(); + int found = owner6.getPets().size(); Pet pet = new Pet(); @@ -157,7 +167,9 @@ void shouldInsertPetIntoDatabaseAndGenerateId() { this.owners.save(owner6); - owner6 = this.owners.findById(6); + optionalOwner = this.owners.findById(6); + assertThat(optionalOwner).isPresent(); + owner6 = optionalOwner.get(); assertThat(owner6.getPets()).hasSize(found + 1); // checks that id has been generated pet = owner6.getPet("bowser"); @@ -167,7 +179,10 @@ void shouldInsertPetIntoDatabaseAndGenerateId() { @Test @Transactional void shouldUpdatePetName() { - Owner owner6 = this.owners.findById(6); + Optional optionalOwner = this.owners.findById(6); + assertThat(optionalOwner).isPresent(); + Owner owner6 = optionalOwner.get(); + Pet pet7 = owner6.getPet(7); String oldName = pet7.getName(); @@ -175,7 +190,9 @@ void shouldUpdatePetName() { pet7.setName(newName); this.owners.save(owner6); - owner6 = this.owners.findById(6); + optionalOwner = this.owners.findById(6); + assertThat(optionalOwner).isPresent(); + owner6 = optionalOwner.get(); pet7 = owner6.getPet(7); assertThat(pet7.getName()).isEqualTo(newName); } @@ -194,7 +211,10 @@ void shouldFindVets() { @Test @Transactional void shouldAddNewVisitForPet() { - Owner owner6 = this.owners.findById(6); + Optional optionalOwner = this.owners.findById(6); + assertThat(optionalOwner).isPresent(); + Owner owner6 = optionalOwner.get(); + Pet pet7 = owner6.getPet(7); int found = pet7.getVisits().size(); Visit visit = new Visit(); @@ -210,7 +230,10 @@ void shouldAddNewVisitForPet() { @Test void shouldFindVisitsByPetId() { - Owner owner6 = this.owners.findById(6); + Optional optionalOwner = this.owners.findById(6); + assertThat(optionalOwner).isPresent(); + Owner owner6 = optionalOwner.get(); + Pet pet7 = owner6.getPet(7); Collection visits = pet7.getVisits();