mirror of
https://github.com/spring-projects/spring-petclinic.git
synced 2025-04-25 03:42:48 +00:00
Refactor code logic
<refactor>: remove useless logic cod. <refactor>: detele useless annotation which is provided by Jpa. <refactor>: refactor implement of `findByLastName`, use Jpa to simplify query.
This commit is contained in:
parent
668629d5bd
commit
1cad4124b7
5 changed files with 11 additions and 24 deletions
|
@ -127,7 +127,7 @@ class OwnerController {
|
||||||
private Page<Owner> findPaginatedForOwnersLastName(int page, String lastname) {
|
private Page<Owner> findPaginatedForOwnersLastName(int page, String lastname) {
|
||||||
int pageSize = 5;
|
int pageSize = 5;
|
||||||
Pageable pageable = PageRequest.of(page - 1, pageSize);
|
Pageable pageable = PageRequest.of(page - 1, pageSize);
|
||||||
return owners.findByLastName(lastname, pageable);
|
return owners.findByLastNameStartingWith(lastname, pageable);
|
||||||
}
|
}
|
||||||
|
|
||||||
@GetMapping("/owners/{ownerId}/edit")
|
@GetMapping("/owners/{ownerId}/edit")
|
||||||
|
|
|
@ -19,13 +19,11 @@ import java.util.List;
|
||||||
import java.util.Optional;
|
import java.util.Optional;
|
||||||
|
|
||||||
import jakarta.annotation.Nonnull;
|
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.JpaRepository;
|
||||||
import org.springframework.data.jpa.repository.Query;
|
import org.springframework.data.jpa.repository.Query;
|
||||||
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;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -57,9 +55,7 @@ public interface OwnerRepository extends JpaRepository<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% ")
|
Page<Owner> findByLastNameStartingWith(String lastName, Pageable pageable);
|
||||||
@Transactional(readOnly = true)
|
|
||||||
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.
|
||||||
|
@ -79,8 +75,6 @@ public interface OwnerRepository extends JpaRepository<Owner, Integer> {
|
||||||
/**
|
/**
|
||||||
* Returns all the owners from data store
|
* Returns all the owners from data store
|
||||||
**/
|
**/
|
||||||
@Query("SELECT owner FROM Owner owner")
|
|
||||||
@Transactional(readOnly = true)
|
|
||||||
Page<Owner> findAll(Pageable pageable);
|
Page<Owner> findAll(Pageable pageable);
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -62,9 +62,6 @@ class PetController {
|
||||||
Optional<Owner> optionalOwner = this.owners.findById(ownerId);
|
Optional<Owner> optionalOwner = this.owners.findById(ownerId);
|
||||||
Owner owner = optionalOwner.orElseThrow(() -> new IllegalArgumentException(
|
Owner owner = optionalOwner.orElseThrow(() -> new IllegalArgumentException(
|
||||||
"Owner not found with id: " + ownerId + ". Please ensure the ID is correct "));
|
"Owner not found with id: " + ownerId + ". Please ensure the ID is correct "));
|
||||||
if (owner == null) {
|
|
||||||
throw new IllegalArgumentException("Owner ID not found: " + ownerId);
|
|
||||||
}
|
|
||||||
return owner;
|
return owner;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -79,9 +76,6 @@ class PetController {
|
||||||
Optional<Owner> optionalOwner = this.owners.findById(ownerId);
|
Optional<Owner> optionalOwner = this.owners.findById(ownerId);
|
||||||
Owner owner = optionalOwner.orElseThrow(() -> new IllegalArgumentException(
|
Owner owner = optionalOwner.orElseThrow(() -> new IllegalArgumentException(
|
||||||
"Owner not found with id: " + ownerId + ". Please ensure the ID is correct "));
|
"Owner not found with id: " + ownerId + ". Please ensure the ID is correct "));
|
||||||
if (owner == null) {
|
|
||||||
throw new IllegalArgumentException("Owner ID not found: " + ownerId);
|
|
||||||
}
|
|
||||||
return owner.getPet(petId);
|
return owner.getPet(petId);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -99,7 +93,6 @@ class PetController {
|
||||||
public String initCreationForm(Owner owner, ModelMap model) {
|
public String initCreationForm(Owner owner, ModelMap model) {
|
||||||
Pet pet = new Pet();
|
Pet pet = new Pet();
|
||||||
owner.addPet(pet);
|
owner.addPet(pet);
|
||||||
model.put("pet", pet);
|
|
||||||
return VIEWS_PETS_CREATE_OR_UPDATE_FORM;
|
return VIEWS_PETS_CREATE_OR_UPDATE_FORM;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -126,7 +119,7 @@ class PetController {
|
||||||
}
|
}
|
||||||
|
|
||||||
@GetMapping("/pets/{petId}/edit")
|
@GetMapping("/pets/{petId}/edit")
|
||||||
public String initUpdateForm(Owner owner, @PathVariable("petId") int petId, RedirectAttributes redirectAttributes) {
|
public String initUpdateForm() {
|
||||||
return VIEWS_PETS_CREATE_OR_UPDATE_FORM;
|
return VIEWS_PETS_CREATE_OR_UPDATE_FORM;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -91,7 +91,7 @@ class OwnerControllerTests {
|
||||||
void setup() {
|
void setup() {
|
||||||
|
|
||||||
Owner george = george();
|
Owner george = george();
|
||||||
given(this.owners.findByLastName(eq("Franklin"), any(Pageable.class)))
|
given(this.owners.findByLastNameStartingWith(eq("Franklin"), any(Pageable.class)))
|
||||||
.willReturn(new PageImpl<>(Lists.newArrayList(george)));
|
.willReturn(new PageImpl<>(Lists.newArrayList(george)));
|
||||||
|
|
||||||
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)));
|
||||||
|
@ -144,14 +144,14 @@ class OwnerControllerTests {
|
||||||
@Test
|
@Test
|
||||||
void testProcessFindFormSuccess() throws Exception {
|
void testProcessFindFormSuccess() throws Exception {
|
||||||
Page<Owner> tasks = new PageImpl<>(Lists.newArrayList(george(), new Owner()));
|
Page<Owner> tasks = new PageImpl<>(Lists.newArrayList(george(), new Owner()));
|
||||||
when(this.owners.findByLastName(anyString(), any(Pageable.class))).thenReturn(tasks);
|
when(this.owners.findByLastNameStartingWith(anyString(), any(Pageable.class))).thenReturn(tasks);
|
||||||
mockMvc.perform(get("/owners?page=1")).andExpect(status().isOk()).andExpect(view().name("owners/ownersList"));
|
mockMvc.perform(get("/owners?page=1")).andExpect(status().isOk()).andExpect(view().name("owners/ownersList"));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void testProcessFindFormByLastName() throws Exception {
|
void testProcessFindFormByLastName() throws Exception {
|
||||||
Page<Owner> tasks = new PageImpl<>(Lists.newArrayList(george()));
|
Page<Owner> tasks = new PageImpl<>(Lists.newArrayList(george()));
|
||||||
when(this.owners.findByLastName(eq("Franklin"), any(Pageable.class))).thenReturn(tasks);
|
when(this.owners.findByLastNameStartingWith(eq("Franklin"), any(Pageable.class))).thenReturn(tasks);
|
||||||
mockMvc.perform(get("/owners?page=1").param("lastName", "Franklin"))
|
mockMvc.perform(get("/owners?page=1").param("lastName", "Franklin"))
|
||||||
.andExpect(status().is3xxRedirection())
|
.andExpect(status().is3xxRedirection())
|
||||||
.andExpect(view().name("redirect:/owners/" + TEST_OWNER_ID));
|
.andExpect(view().name("redirect:/owners/" + TEST_OWNER_ID));
|
||||||
|
@ -160,7 +160,7 @@ class OwnerControllerTests {
|
||||||
@Test
|
@Test
|
||||||
void testProcessFindFormNoOwnersFound() throws Exception {
|
void testProcessFindFormNoOwnersFound() throws Exception {
|
||||||
Page<Owner> tasks = new PageImpl<>(Lists.newArrayList());
|
Page<Owner> tasks = new PageImpl<>(Lists.newArrayList());
|
||||||
when(this.owners.findByLastName(eq("Unknown Surname"), any(Pageable.class))).thenReturn(tasks);
|
when(this.owners.findByLastNameStartingWith(eq("Unknown Surname"), any(Pageable.class))).thenReturn(tasks);
|
||||||
mockMvc.perform(get("/owners?page=1").param("lastName", "Unknown Surname"))
|
mockMvc.perform(get("/owners?page=1").param("lastName", "Unknown Surname"))
|
||||||
.andExpect(status().isOk())
|
.andExpect(status().isOk())
|
||||||
.andExpect(model().attributeHasFieldErrors("owner", "lastName"))
|
.andExpect(model().attributeHasFieldErrors("owner", "lastName"))
|
||||||
|
|
|
@ -82,10 +82,10 @@ class ClinicServiceTests {
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
void shouldFindOwnersByLastName() {
|
void shouldFindOwnersByLastName() {
|
||||||
Page<Owner> owners = this.owners.findByLastName("Davis", pageable);
|
Page<Owner> owners = this.owners.findByLastNameStartingWith("Davis", pageable);
|
||||||
assertThat(owners).hasSize(2);
|
assertThat(owners).hasSize(2);
|
||||||
|
|
||||||
owners = this.owners.findByLastName("Daviss", pageable);
|
owners = this.owners.findByLastNameStartingWith("Daviss", pageable);
|
||||||
assertThat(owners).isEmpty();
|
assertThat(owners).isEmpty();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -103,7 +103,7 @@ class ClinicServiceTests {
|
||||||
@Test
|
@Test
|
||||||
@Transactional
|
@Transactional
|
||||||
void shouldInsertOwner() {
|
void shouldInsertOwner() {
|
||||||
Page<Owner> owners = this.owners.findByLastName("Schultz", pageable);
|
Page<Owner> owners = this.owners.findByLastNameStartingWith("Schultz", pageable);
|
||||||
int found = (int) owners.getTotalElements();
|
int found = (int) owners.getTotalElements();
|
||||||
|
|
||||||
Owner owner = new Owner();
|
Owner owner = new Owner();
|
||||||
|
@ -115,7 +115,7 @@ class ClinicServiceTests {
|
||||||
this.owners.save(owner);
|
this.owners.save(owner);
|
||||||
assertThat(owner.getId()).isNotZero();
|
assertThat(owner.getId()).isNotZero();
|
||||||
|
|
||||||
owners = this.owners.findByLastName("Schultz", pageable);
|
owners = this.owners.findByLastNameStartingWith("Schultz", pageable);
|
||||||
assertThat(owners.getTotalElements()).isEqualTo(found + 1);
|
assertThat(owners.getTotalElements()).isEqualTo(found + 1);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue