Skip to content

Commit

Permalink
refactor OwnerRepository:
Browse files Browse the repository at this point in the history
-<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.
  • Loading branch information
wickdynex authored and dsyer committed Nov 10, 2024
1 parent a3026bd commit 668629d
Show file tree
Hide file tree
Showing 11 changed files with 85 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
*
* @author Ken Krebs
* @author Juergen Hoeller
* @author Wick Dynex
*/
@MappedSuperclass
public class NamedEntity extends BaseEntity {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
* @author Sam Brannen
* @author Michael Isvy
* @author Oliver Drotbohm
* @author Wick Dynex
*/
@Entity
@Table(name = "owners")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -40,6 +41,7 @@
* @author Ken Krebs
* @author Arjen Poutsma
* @author Michael Isvy
* @author Wick Dynex
*/
@Controller
class OwnerController {
Expand All @@ -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")
Expand Down Expand Up @@ -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<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);
return mav;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -34,8 +38,9 @@
* @author Juergen Hoeller
* @author Sam Brannen
* @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.
Expand All @@ -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
* found)
*/

@Query("SELECT DISTINCT owner FROM Owner owner left join owner.pets WHERE owner.lastName LIKE :lastName% ")
@Transactional(readOnly = true)
Page<Owner> findByLastName(@Param("lastName") String lastName, Pageable pageable);

/**
* 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
* @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<Owner> findById(@Nonnull Integer id);

/**
* Returns all the owners from data store
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
* @author Ken Krebs
* @author Juergen Hoeller
* @author Sam Brannen
* @author Wick Dynex
*/
@Entity
@Table(name = "pets")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,6 +38,7 @@
* @author Juergen Hoeller
* @author Ken Krebs
* @author Arjen Poutsma
* @author Wick Dynex
*/
@Controller
@RequestMapping("/owners/{ownerId}")
Expand All @@ -57,8 +59,9 @@ public Collection<PetType> populatePetTypes() {

@ModelAttribute("owner")
public Owner findOwner(@PathVariable("ownerId") int ownerId) {

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) {
throw new IllegalArgumentException("Owner ID not found: " + ownerId);
}
Expand All @@ -73,7 +76,9 @@ public Pet findPet(@PathVariable("ownerId") int ownerId,
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) {
throw new IllegalArgumentException("Owner ID not found: " + ownerId);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -35,6 +37,7 @@
* @author Arjen Poutsma
* @author Michael Isvy
* @author Dave Syer
* @author Wick Dynex
*/
@Controller
class VisitController {
Expand All @@ -60,7 +63,9 @@ public void setAllowedFields(WebDataBinder dataBinder) {
@ModelAttribute("visit")
public Visit loadPetWithVisit(@PathVariable("ownerId") int ownerId, @PathVariable("petId") int petId,
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);
model.put("pet", pet);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -52,6 +53,7 @@
* Test class for {@link OwnerController}
*
* @author Colin But
* @author Wick Dynex
*/
@WebMvcTest(OwnerController.class)
@DisabledInNativeImage
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -90,7 +91,9 @@ void shouldFindOwnersByLastName() {

@Test
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.getPets()).hasSize(1);
assertThat(owner.getPets().get(0).getType()).isNotNull();
Expand Down Expand Up @@ -119,15 +122,19 @@ void shouldInsertOwner() {
@Test
@Transactional
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 newLastName = oldLastName + "X";

owner.setLastName(newLastName);
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);
}

Expand All @@ -144,7 +151,10 @@ void shouldFindAllPetTypes() {
@Test
@Transactional
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();

Pet pet = new Pet();
Expand All @@ -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");
Expand All @@ -167,15 +179,20 @@ void shouldInsertPetIntoDatabaseAndGenerateId() {
@Test
@Transactional
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);
String oldName = pet7.getName();

String newName = oldName + "X";
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);
}
Expand All @@ -194,7 +211,10 @@ void shouldFindVets() {
@Test
@Transactional
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);
int found = pet7.getVisits().size();
Visit visit = new Visit();
Expand All @@ -210,7 +230,10 @@ void shouldAddNewVisitForPet() {

@Test
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);
Collection<Visit> visits = pet7.getVisits();

Expand Down

0 comments on commit 668629d

Please sign in to comment.