refactor OwnerRepository:

-<replace>: use `JpaRepository` to replace `Repository` in `OwnerRepository` class.
-<remove1>: remove `save()` method. JpaRepository provides it by default.
-<remove2>: remove `@Query` because in `Owner` class, the `@OneToMany` annotiation achieved `fetch` in query.
-<refactor1>: use `Optional<Owner>` to recieve the result from `findById()`, and if is null, throw `IllegalArugmentExpection`.
-<refactor2>: achieve the assert to judge return value in tests.
-<add>: add name to `@author` tag.
This commit is contained in:
YiXuan Ding 2024-11-08 22:05:22 +08:00 committed by Dave Syer
parent a3026bddbb
commit 668629d5bd
11 changed files with 85 additions and 32 deletions

View file

@ -25,6 +25,7 @@ import jakarta.validation.constraints.NotBlank;
* *
* @author Ken Krebs * @author Ken Krebs
* @author Juergen Hoeller * @author Juergen Hoeller
* @author Wick Dynex
*/ */
@MappedSuperclass @MappedSuperclass
public class NamedEntity extends BaseEntity { public class NamedEntity extends BaseEntity {

View file

@ -41,6 +41,7 @@ import jakarta.validation.constraints.NotBlank;
* @author Sam Brannen * @author Sam Brannen
* @author Michael Isvy * @author Michael Isvy
* @author Oliver Drotbohm * @author Oliver Drotbohm
* @author Wick Dynex
*/ */
@Entity @Entity
@Table(name = "owners") @Table(name = "owners")

View file

@ -16,6 +16,7 @@
package org.springframework.samples.petclinic.owner; package org.springframework.samples.petclinic.owner;
import java.util.List; import java.util.List;
import java.util.Optional;
import org.springframework.data.domain.Page; import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.PageRequest;
@ -40,6 +41,7 @@ import org.springframework.web.servlet.mvc.support.RedirectAttributes;
* @author Ken Krebs * @author Ken Krebs
* @author Arjen Poutsma * @author Arjen Poutsma
* @author Michael Isvy * @author Michael Isvy
* @author Wick Dynex
*/ */
@Controller @Controller
class OwnerController { class OwnerController {
@ -59,7 +61,10 @@ class OwnerController {
@ModelAttribute("owner") @ModelAttribute("owner")
public Owner findOwner(@PathVariable(name = "ownerId", required = false) Integer ownerId) { 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") @GetMapping("/owners/new")
@ -158,7 +163,9 @@ class OwnerController {
@GetMapping("/owners/{ownerId}") @GetMapping("/owners/{ownerId}")
public ModelAndView showOwner(@PathVariable("ownerId") int ownerId) { public ModelAndView showOwner(@PathVariable("ownerId") int ownerId) {
ModelAndView mav = new ModelAndView("owners/ownerDetails"); ModelAndView mav = new ModelAndView("owners/ownerDetails");
Owner owner = this.owners.findById(ownerId); Optional<Owner> 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); mav.addObject(owner);
return mav; return mav;
} }

View file

@ -16,12 +16,16 @@
package org.springframework.samples.petclinic.owner; package org.springframework.samples.petclinic.owner;
import java.util.List; 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.Page;
import org.springframework.data.domain.Pageable; import org.springframework.data.domain.Pageable;
import org.springframework.data.jpa.repository.JpaRepository;
import org.springframework.data.jpa.repository.Query; import org.springframework.data.jpa.repository.Query;
import org.springframework.data.repository.Repository;
import org.springframework.data.repository.query.Param; import org.springframework.data.repository.query.Param;
import org.springframework.lang.NonNullApi;
import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.annotation.Transactional;
/** /**
@ -34,8 +38,9 @@ import org.springframework.transaction.annotation.Transactional;
* @author Juergen Hoeller * @author Juergen Hoeller
* @author Sam Brannen * @author Sam Brannen
* @author Michael Isvy * @author Michael Isvy
* @author Wick Dynex
*/ */
public interface OwnerRepository extends Repository<Owner, Integer> { public interface OwnerRepository extends JpaRepository<Owner, Integer> {
/** /**
* Retrieve all {@link PetType}s from the data store. * Retrieve all {@link PetType}s from the data store.
@ -52,25 +57,24 @@ public interface OwnerRepository extends Repository<Owner, Integer> {
* @return a Collection of matching {@link Owner}s (or an empty Collection if none * @return a Collection of matching {@link Owner}s (or an empty Collection if none
* found) * found)
*/ */
@Query("SELECT DISTINCT owner FROM Owner owner left join owner.pets WHERE owner.lastName LIKE :lastName% ") @Query("SELECT DISTINCT owner FROM Owner owner left join owner.pets WHERE owner.lastName LIKE :lastName% ")
@Transactional(readOnly = true) @Transactional(readOnly = true)
Page<Owner> findByLastName(@Param("lastName") String lastName, Pageable pageable); Page<Owner> findByLastName(@Param("lastName") String lastName, Pageable pageable);
/** /**
* Retrieve an {@link Owner} from the data store by id. * Retrieve an {@link Owner} from the data store by id.
* <p>
* 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}.
* </p>
* @param id the id to search for * @param id the id to search for
* @return the {@link Owner} if found * @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)
*/ */
@Query("SELECT owner FROM Owner owner left join fetch owner.pets WHERE owner.id =:id") Optional<Owner> findById(@Nonnull Integer 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
*/
void save(Owner owner);
/** /**
* Returns all the owners from data store * Returns all the owners from data store

View file

@ -39,6 +39,7 @@ import jakarta.persistence.Table;
* @author Ken Krebs * @author Ken Krebs
* @author Juergen Hoeller * @author Juergen Hoeller
* @author Sam Brannen * @author Sam Brannen
* @author Wick Dynex
*/ */
@Entity @Entity
@Table(name = "pets") @Table(name = "pets")

View file

@ -17,6 +17,7 @@ package org.springframework.samples.petclinic.owner;
import java.time.LocalDate; import java.time.LocalDate;
import java.util.Collection; import java.util.Collection;
import java.util.Optional;
import org.springframework.stereotype.Controller; import org.springframework.stereotype.Controller;
import org.springframework.ui.ModelMap; import org.springframework.ui.ModelMap;
@ -37,6 +38,7 @@ import org.springframework.web.servlet.mvc.support.RedirectAttributes;
* @author Juergen Hoeller * @author Juergen Hoeller
* @author Ken Krebs * @author Ken Krebs
* @author Arjen Poutsma * @author Arjen Poutsma
* @author Wick Dynex
*/ */
@Controller @Controller
@RequestMapping("/owners/{ownerId}") @RequestMapping("/owners/{ownerId}")
@ -57,8 +59,9 @@ class PetController {
@ModelAttribute("owner") @ModelAttribute("owner")
public Owner findOwner(@PathVariable("ownerId") int ownerId) { public Owner findOwner(@PathVariable("ownerId") int ownerId) {
Optional<Owner> optionalOwner = this.owners.findById(ownerId);
Owner owner = 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) { if (owner == null) {
throw new IllegalArgumentException("Owner ID not found: " + ownerId); throw new IllegalArgumentException("Owner ID not found: " + ownerId);
} }
@ -73,7 +76,9 @@ class PetController {
return new Pet(); return new Pet();
} }
Owner owner = this.owners.findById(ownerId); Optional<Owner> 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) { if (owner == null) {
throw new IllegalArgumentException("Owner ID not found: " + ownerId); throw new IllegalArgumentException("Owner ID not found: " + ownerId);
} }

View file

@ -16,6 +16,8 @@
package org.springframework.samples.petclinic.owner; package org.springframework.samples.petclinic.owner;
import java.util.Map; import java.util.Map;
import java.util.Optional;
import java.util.OptionalInt;
import org.springframework.stereotype.Controller; import org.springframework.stereotype.Controller;
import org.springframework.validation.BindingResult; import org.springframework.validation.BindingResult;
@ -35,6 +37,7 @@ import org.springframework.web.servlet.mvc.support.RedirectAttributes;
* @author Arjen Poutsma * @author Arjen Poutsma
* @author Michael Isvy * @author Michael Isvy
* @author Dave Syer * @author Dave Syer
* @author Wick Dynex
*/ */
@Controller @Controller
class VisitController { class VisitController {
@ -60,7 +63,9 @@ class VisitController {
@ModelAttribute("visit") @ModelAttribute("visit")
public Visit loadPetWithVisit(@PathVariable("ownerId") int ownerId, @PathVariable("petId") int petId, public Visit loadPetWithVisit(@PathVariable("ownerId") int ownerId, @PathVariable("petId") int petId,
Map<String, Object> model) { Map<String, Object> model) {
Owner owner = this.owners.findById(ownerId); Optional<Owner> 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); Pet pet = owner.getPet(petId);
model.put("pet", pet); model.put("pet", pet);

View file

@ -31,6 +31,7 @@ import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
import java.time.LocalDate; import java.time.LocalDate;
import java.util.Optional;
import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.greaterThan;
@ -52,6 +53,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
* Test class for {@link OwnerController} * Test class for {@link OwnerController}
* *
* @author Colin But * @author Colin But
* @author Wick Dynex
*/ */
@WebMvcTest(OwnerController.class) @WebMvcTest(OwnerController.class)
@DisabledInNativeImage @DisabledInNativeImage
@ -94,7 +96,7 @@ class OwnerControllerTests {
given(this.owners.findAll(any(Pageable.class))).willReturn(new PageImpl<>(Lists.newArrayList(george))); 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 visit = new Visit();
visit.setDate(LocalDate.now()); visit.setDate(LocalDate.now());
george.getPet("Max").getVisits().add(visit); george.getPet("Max").getVisits().add(visit);
@ -240,7 +242,7 @@ class OwnerControllerTests {
owner.setCity("New York"); owner.setCity("New York");
owner.setTelephone("0123456789"); 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)) mockMvc.perform(MockMvcRequestBuilders.post("/owners/{ownerId}/edit", pathOwnerId).flashAttr("owner", owner))
.andExpect(status().is3xxRedirection()) .andExpect(status().is3xxRedirection())

View file

@ -46,6 +46,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.
* Test class for the {@link PetController} * Test class for the {@link PetController}
* *
* @author Colin But * @author Colin But
* @author Wick Dynex
*/ */
@WebMvcTest(value = PetController.class, @WebMvcTest(value = PetController.class,
includeFilters = @ComponentScan.Filter(value = PetTypeFormatter.class, type = FilterType.ASSIGNABLE_TYPE)) includeFilters = @ComponentScan.Filter(value = PetTypeFormatter.class, type = FilterType.ASSIGNABLE_TYPE))
@ -79,7 +80,7 @@ class PetControllerTests {
dog.setId(TEST_PET_ID + 1); dog.setId(TEST_PET_ID + 1);
pet.setName("petty"); pet.setName("petty");
dog.setName("doggy"); dog.setName("doggy");
given(this.owners.findById(TEST_OWNER_ID)).willReturn(owner); given(this.owners.findById(TEST_OWNER_ID)).willReturn(Optional.of(owner));
} }
@Test @Test

View file

@ -32,10 +32,13 @@ import org.springframework.boot.test.mock.mockito.MockBean;
import org.springframework.test.context.aot.DisabledInAotMode; import org.springframework.test.context.aot.DisabledInAotMode;
import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.MockMvc;
import java.util.Optional;
/** /**
* Test class for {@link VisitController} * Test class for {@link VisitController}
* *
* @author Colin But * @author Colin But
* @author Wick Dynex
*/ */
@WebMvcTest(VisitController.class) @WebMvcTest(VisitController.class)
@DisabledInNativeImage @DisabledInNativeImage
@ -58,7 +61,7 @@ class VisitControllerTests {
Pet pet = new Pet(); Pet pet = new Pet();
owner.addPet(pet); owner.addPet(pet);
pet.setId(TEST_PET_ID); 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 @Test

View file

@ -20,6 +20,7 @@ import static org.assertj.core.api.Assertions.assertThat;
import java.time.LocalDate; import java.time.LocalDate;
import java.util.Collection; import java.util.Collection;
import java.util.Optional;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Autowired;
@ -90,7 +91,9 @@ class ClinicServiceTests {
@Test @Test
void shouldFindSingleOwnerWithPet() { void shouldFindSingleOwnerWithPet() {
Owner owner = this.owners.findById(1); Optional<Owner> optionalOwner = this.owners.findById(1);
assertThat(optionalOwner).isPresent();
Owner owner = optionalOwner.get();
assertThat(owner.getLastName()).startsWith("Franklin"); assertThat(owner.getLastName()).startsWith("Franklin");
assertThat(owner.getPets()).hasSize(1); assertThat(owner.getPets()).hasSize(1);
assertThat(owner.getPets().get(0).getType()).isNotNull(); assertThat(owner.getPets().get(0).getType()).isNotNull();
@ -119,7 +122,9 @@ class ClinicServiceTests {
@Test @Test
@Transactional @Transactional
void shouldUpdateOwner() { void shouldUpdateOwner() {
Owner owner = this.owners.findById(1); Optional<Owner> optionalOwner = this.owners.findById(1);
assertThat(optionalOwner).isPresent();
Owner owner = optionalOwner.get();
String oldLastName = owner.getLastName(); String oldLastName = owner.getLastName();
String newLastName = oldLastName + "X"; String newLastName = oldLastName + "X";
@ -127,7 +132,9 @@ class ClinicServiceTests {
this.owners.save(owner); this.owners.save(owner);
// retrieving new name from database // 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); assertThat(owner.getLastName()).isEqualTo(newLastName);
} }
@ -144,7 +151,10 @@ class ClinicServiceTests {
@Test @Test
@Transactional @Transactional
void shouldInsertPetIntoDatabaseAndGenerateId() { void shouldInsertPetIntoDatabaseAndGenerateId() {
Owner owner6 = this.owners.findById(6); Optional<Owner> optionalOwner = this.owners.findById(6);
assertThat(optionalOwner).isPresent();
Owner owner6 = optionalOwner.get();
int found = owner6.getPets().size(); int found = owner6.getPets().size();
Pet pet = new Pet(); Pet pet = new Pet();
@ -157,7 +167,9 @@ class ClinicServiceTests {
this.owners.save(owner6); 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); assertThat(owner6.getPets()).hasSize(found + 1);
// checks that id has been generated // checks that id has been generated
pet = owner6.getPet("bowser"); pet = owner6.getPet("bowser");
@ -167,7 +179,10 @@ class ClinicServiceTests {
@Test @Test
@Transactional @Transactional
void shouldUpdatePetName() { void shouldUpdatePetName() {
Owner owner6 = this.owners.findById(6); Optional<Owner> optionalOwner = this.owners.findById(6);
assertThat(optionalOwner).isPresent();
Owner owner6 = optionalOwner.get();
Pet pet7 = owner6.getPet(7); Pet pet7 = owner6.getPet(7);
String oldName = pet7.getName(); String oldName = pet7.getName();
@ -175,7 +190,9 @@ class ClinicServiceTests {
pet7.setName(newName); pet7.setName(newName);
this.owners.save(owner6); this.owners.save(owner6);
owner6 = this.owners.findById(6); optionalOwner = this.owners.findById(6);
assertThat(optionalOwner).isPresent();
owner6 = optionalOwner.get();
pet7 = owner6.getPet(7); pet7 = owner6.getPet(7);
assertThat(pet7.getName()).isEqualTo(newName); assertThat(pet7.getName()).isEqualTo(newName);
} }
@ -194,7 +211,10 @@ class ClinicServiceTests {
@Test @Test
@Transactional @Transactional
void shouldAddNewVisitForPet() { void shouldAddNewVisitForPet() {
Owner owner6 = this.owners.findById(6); Optional<Owner> optionalOwner = this.owners.findById(6);
assertThat(optionalOwner).isPresent();
Owner owner6 = optionalOwner.get();
Pet pet7 = owner6.getPet(7); Pet pet7 = owner6.getPet(7);
int found = pet7.getVisits().size(); int found = pet7.getVisits().size();
Visit visit = new Visit(); Visit visit = new Visit();
@ -210,7 +230,10 @@ class ClinicServiceTests {
@Test @Test
void shouldFindVisitsByPetId() { void shouldFindVisitsByPetId() {
Owner owner6 = this.owners.findById(6); Optional<Owner> optionalOwner = this.owners.findById(6);
assertThat(optionalOwner).isPresent();
Owner owner6 = optionalOwner.get();
Pet pet7 = owner6.getPet(7); Pet pet7 = owner6.getPet(7);
Collection<Visit> visits = pet7.getVisits(); Collection<Visit> visits = pet7.getVisits();