mirror of
https://github.com/spring-projects/spring-petclinic.git
synced 2026-02-11 09:01:10 +00:00
Switch owner-related associations to LAZY and restore MVC rendering via OSIV
Signed-off-by: Mohit Kumar <mohitkmeena2001@gmail.com>
This commit is contained in:
parent
37cc7a3198
commit
29f698abb0
9 changed files with 233 additions and 38 deletions
|
|
@ -61,7 +61,7 @@ public class Owner extends Person {
|
|||
@Pattern(regexp = "\\d{10}", message = "{telephone.invalid}")
|
||||
private String telephone;
|
||||
|
||||
@OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER)
|
||||
@OneToMany(cascade = CascadeType.ALL, fetch = FetchType.LAZY)
|
||||
@JoinColumn(name = "owner_id")
|
||||
@OrderBy("name")
|
||||
private final List<Pet> pets = new ArrayList<>();
|
||||
|
|
|
|||
|
|
@ -62,7 +62,7 @@ class OwnerController {
|
|||
@ModelAttribute("owner")
|
||||
public Owner findOwner(@PathVariable(name = "ownerId", required = false) Integer ownerId) {
|
||||
return ownerId == null ? new Owner()
|
||||
: this.owners.findById(ownerId)
|
||||
: this.owners.findByIdWithPets(ownerId)
|
||||
.orElseThrow(() -> new IllegalArgumentException("Owner not found with id: " + ownerId
|
||||
+ ". Please ensure the ID is correct " + "and the owner exists in the database."));
|
||||
}
|
||||
|
|
@ -89,6 +89,15 @@ class OwnerController {
|
|||
return "owners/findOwners";
|
||||
}
|
||||
|
||||
/**
|
||||
* Optimized owner search using DTO projection to avoid N+1 queries. This method now
|
||||
* executes only 2 queries total: 1. A single query with GROUP_CONCAT to fetch all
|
||||
* owner data with aggregated pet names 2. A count query for pagination
|
||||
* <p>
|
||||
* Previously, this executed 1 + N + M queries where N = number of owners and M =
|
||||
* total number of pets.
|
||||
* </p>
|
||||
*/
|
||||
@GetMapping("/owners")
|
||||
public String processFindForm(@RequestParam(defaultValue = "1") int page, Owner owner, BindingResult result,
|
||||
Model model) {
|
||||
|
|
@ -98,8 +107,8 @@ class OwnerController {
|
|||
lastName = ""; // empty string signifies broadest possible search
|
||||
}
|
||||
|
||||
// find owners by last name
|
||||
Page<Owner> ownersResults = findPaginatedForOwnersLastName(page, lastName);
|
||||
// find owners by last name using optimized DTO projection
|
||||
Page<OwnerSearchResult> ownersResults = findPaginatedForOwnersLastName(page, lastName);
|
||||
if (ownersResults.isEmpty()) {
|
||||
// no owners found
|
||||
result.rejectValue("lastName", "notFound", "not found");
|
||||
|
|
@ -107,17 +116,17 @@ class OwnerController {
|
|||
}
|
||||
|
||||
if (ownersResults.getTotalElements() == 1) {
|
||||
// 1 owner found
|
||||
owner = ownersResults.iterator().next();
|
||||
return "redirect:/owners/" + owner.getId();
|
||||
// 1 owner found - redirect to detail view
|
||||
OwnerSearchResult ownerResult = ownersResults.iterator().next();
|
||||
return "redirect:/owners/" + ownerResult.getId();
|
||||
}
|
||||
|
||||
// multiple owners found
|
||||
return addPaginationModel(page, model, ownersResults);
|
||||
}
|
||||
|
||||
private String addPaginationModel(int page, Model model, Page<Owner> paginated) {
|
||||
List<Owner> listOwners = paginated.getContent();
|
||||
private String addPaginationModel(int page, Model model, Page<OwnerSearchResult> paginated) {
|
||||
List<OwnerSearchResult> listOwners = paginated.getContent();
|
||||
model.addAttribute("currentPage", page);
|
||||
model.addAttribute("totalPages", paginated.getTotalPages());
|
||||
model.addAttribute("totalItems", paginated.getTotalElements());
|
||||
|
|
@ -125,10 +134,10 @@ class OwnerController {
|
|||
return "owners/ownersList";
|
||||
}
|
||||
|
||||
private Page<Owner> findPaginatedForOwnersLastName(int page, String lastname) {
|
||||
private Page<OwnerSearchResult> findPaginatedForOwnersLastName(int page, String lastname) {
|
||||
int pageSize = 5;
|
||||
Pageable pageable = PageRequest.of(page - 1, pageSize);
|
||||
return owners.findByLastNameStartingWith(lastname, pageable);
|
||||
return owners.findOwnerSearchResultsByLastName(lastname, pageable);
|
||||
}
|
||||
|
||||
@GetMapping("/owners/{ownerId}/edit")
|
||||
|
|
@ -137,6 +146,7 @@ class OwnerController {
|
|||
}
|
||||
|
||||
@PostMapping("/owners/{ownerId}/edit")
|
||||
@Transactional
|
||||
public String processUpdateOwnerForm(@Valid Owner owner, BindingResult result, @PathVariable("ownerId") int ownerId,
|
||||
RedirectAttributes redirectAttributes) {
|
||||
if (result.hasErrors()) {
|
||||
|
|
@ -157,27 +167,36 @@ class OwnerController {
|
|||
}
|
||||
|
||||
/**
|
||||
* Custom handler for displaying an owner.
|
||||
* Custom handler for displaying an owner. Uses EntityGraph to efficiently load pets
|
||||
* and visits in a single query.
|
||||
* @param ownerId the ID of the owner to display
|
||||
* @return a ModelMap with the model attributes for the view
|
||||
*/
|
||||
@GetMapping("/owners/{ownerId}.html")
|
||||
public ModelAndView showOwner(@PathVariable("ownerId") int ownerId) {
|
||||
ModelAndView mav = new ModelAndView("owners/ownerDetails");
|
||||
Optional<Owner> optionalOwner = this.owners.findById(ownerId);
|
||||
// Use EntityGraph to load owner with pets and visits in one query
|
||||
Optional<Owner> optionalOwner = this.owners.findByIdWithPetsAndVisits(ownerId);
|
||||
Owner owner = optionalOwner.orElseThrow(() -> new IllegalArgumentException(
|
||||
"Owner not found with id: " + ownerId + ". Please ensure the ID is correct "));
|
||||
mav.addObject(owner);
|
||||
return mav;
|
||||
}
|
||||
|
||||
/**
|
||||
* REST endpoint for owner resource. Uses EntityGraph to efficiently load all related
|
||||
* data in a single query instead of relying on manual initialization.
|
||||
*/
|
||||
@GetMapping("/owners/{ownerId}")
|
||||
@ResponseBody
|
||||
@Transactional(readOnly = true)
|
||||
public Owner showOwnerResource(@PathVariable("ownerId") int ownerId) {
|
||||
|
||||
return this.owners.findById(ownerId)
|
||||
// Use EntityGraph to load owner with all related data efficiently
|
||||
Owner owner = this.owners.findByIdWithPetsAndVisits(ownerId)
|
||||
.orElseThrow(() -> new IllegalArgumentException("Owner not found with id: " + ownerId));
|
||||
|
||||
// No need for manual initialization - EntityGraph already loaded everything
|
||||
return owner;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -19,7 +19,10 @@ import java.util.Optional;
|
|||
|
||||
import org.springframework.data.domain.Page;
|
||||
import org.springframework.data.domain.Pageable;
|
||||
import org.springframework.data.jpa.repository.EntityGraph;
|
||||
import org.springframework.data.jpa.repository.JpaRepository;
|
||||
import org.springframework.data.jpa.repository.Query;
|
||||
import org.springframework.data.repository.query.Param;
|
||||
|
||||
/**
|
||||
* Repository class for <code>Owner</code> domain objects. All method names are compliant
|
||||
|
|
@ -44,6 +47,51 @@ public interface OwnerRepository extends JpaRepository<Owner, Integer> {
|
|||
*/
|
||||
Page<Owner> findByLastNameStartingWith(String lastName, Pageable pageable);
|
||||
|
||||
/**
|
||||
* Internal method that retrieves raw data from the database. Use
|
||||
* findOwnerSearchResultsByLastName() instead.
|
||||
*/
|
||||
@Query(value = """
|
||||
SELECT
|
||||
o.id,
|
||||
o.first_name,
|
||||
o.last_name,
|
||||
o.address,
|
||||
o.city,
|
||||
o.telephone,
|
||||
COALESCE(GROUP_CONCAT(p.name ORDER BY p.name SEPARATOR ', '), '')
|
||||
FROM owners o
|
||||
LEFT JOIN pets p ON o.id = p.owner_id
|
||||
WHERE LOWER(o.last_name) LIKE LOWER(CONCAT(:lastName, '%'))
|
||||
GROUP BY o.id, o.first_name, o.last_name, o.address, o.city, o.telephone
|
||||
""", countQuery = """
|
||||
SELECT COUNT(DISTINCT o.id)
|
||||
FROM owners o
|
||||
WHERE LOWER(o.last_name) LIKE LOWER(CONCAT(:lastName, '%'))
|
||||
""", nativeQuery = true)
|
||||
Page<Object[]> findOwnerSearchResultsRaw(@Param("lastName") String lastName, Pageable pageable);
|
||||
|
||||
/**
|
||||
* Efficiently retrieve owner search results with aggregated pet names using a single
|
||||
* query. This uses a native query with GROUP_CONCAT to avoid N+1 queries.
|
||||
*
|
||||
* The results are projected into OwnerSearchResult DTOs.
|
||||
* @param lastName Value to search for
|
||||
* @param pageable Pagination parameters
|
||||
* @return Page of OwnerSearchResult DTOs
|
||||
*/
|
||||
default Page<OwnerSearchResult> findOwnerSearchResultsByLastName(String lastName, Pageable pageable) {
|
||||
Page<Object[]> rawResults = findOwnerSearchResultsRaw(lastName, pageable);
|
||||
return rawResults.map(row -> new OwnerSearchResult(((Number) row[0]).intValue(), // id
|
||||
(String) row[1], // firstName
|
||||
(String) row[2], // lastName
|
||||
(String) row[3], // address
|
||||
(String) row[4], // city
|
||||
(String) row[5], // telephone
|
||||
(String) row[6] // petNames
|
||||
));
|
||||
}
|
||||
|
||||
/**
|
||||
* Retrieve an {@link Owner} from the data store by id.
|
||||
* <p>
|
||||
|
|
@ -59,4 +107,24 @@ public interface OwnerRepository extends JpaRepository<Owner, Integer> {
|
|||
*/
|
||||
Optional<Owner> findById(Integer id);
|
||||
|
||||
/**
|
||||
* Retrieve an {@link Owner} with pets eagerly loaded using EntityGraph. Use this when
|
||||
* you need to display or work with the owner's pets.
|
||||
* @param id the id to search for
|
||||
* @return an {@link Optional} containing the {@link Owner} with pets loaded
|
||||
*/
|
||||
@EntityGraph(attributePaths = { "pets", "pets.type" })
|
||||
@Query("SELECT o FROM Owner o WHERE o.id = :id")
|
||||
Optional<Owner> findByIdWithPets(@Param("id") Integer id);
|
||||
|
||||
/**
|
||||
* Retrieve an {@link Owner} with complete data graph (pets, types, visits). Use this
|
||||
* for detail views where all related data is needed.
|
||||
* @param id the id to search for
|
||||
* @return an {@link Optional} containing the {@link Owner} with complete data
|
||||
*/
|
||||
@EntityGraph(attributePaths = { "pets", "pets.type", "pets.visits" })
|
||||
@Query("SELECT o FROM Owner o WHERE o.id = :id")
|
||||
Optional<Owner> findByIdWithPetsAndVisits(@Param("id") Integer id);
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,81 @@
|
|||
/*
|
||||
* Copyright 2012-2025 the original author or authors.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
* You may obtain a copy of the License at
|
||||
*
|
||||
* https://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing, software
|
||||
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
* See the License for the specific language governing permissions and
|
||||
* limitations under the License.
|
||||
*/
|
||||
package org.springframework.samples.petclinic.owner;
|
||||
|
||||
/**
|
||||
* DTO for owner search results that efficiently includes aggregated pet names without
|
||||
* triggering N+1 queries.
|
||||
*/
|
||||
public class OwnerSearchResult {
|
||||
|
||||
private final Integer id;
|
||||
|
||||
private final String firstName;
|
||||
|
||||
private final String lastName;
|
||||
|
||||
private final String address;
|
||||
|
||||
private final String city;
|
||||
|
||||
private final String telephone;
|
||||
|
||||
private final String petNames; // Comma-separated pet names
|
||||
|
||||
public OwnerSearchResult(Integer id, String firstName, String lastName, String address, String city,
|
||||
String telephone, String petNames) {
|
||||
this.id = id;
|
||||
this.firstName = firstName;
|
||||
this.lastName = lastName;
|
||||
this.address = address;
|
||||
this.city = city;
|
||||
this.telephone = telephone;
|
||||
this.petNames = petNames;
|
||||
}
|
||||
|
||||
public Integer getId() {
|
||||
return id;
|
||||
}
|
||||
|
||||
public String getFirstName() {
|
||||
return firstName;
|
||||
}
|
||||
|
||||
public String getLastName() {
|
||||
return lastName;
|
||||
}
|
||||
|
||||
public String getAddress() {
|
||||
return address;
|
||||
}
|
||||
|
||||
public String getCity() {
|
||||
return city;
|
||||
}
|
||||
|
||||
public String getTelephone() {
|
||||
return telephone;
|
||||
}
|
||||
|
||||
public String getPetNames() {
|
||||
return petNames != null ? petNames : "";
|
||||
}
|
||||
|
||||
public boolean hasPets() {
|
||||
return petNames != null && !petNames.trim().isEmpty();
|
||||
}
|
||||
|
||||
}
|
||||
|
|
@ -20,6 +20,7 @@ import java.util.Collection;
|
|||
import java.util.LinkedHashSet;
|
||||
import java.util.Set;
|
||||
|
||||
import org.hibernate.annotations.BatchSize;
|
||||
import org.springframework.format.annotation.DateTimeFormat;
|
||||
import org.springframework.samples.petclinic.model.NamedEntity;
|
||||
|
||||
|
|
@ -49,13 +50,18 @@ public class Pet extends NamedEntity {
|
|||
@DateTimeFormat(pattern = "yyyy-MM-dd")
|
||||
private LocalDate birthDate;
|
||||
|
||||
@ManyToOne(fetch = FetchType.LAZY)
|
||||
// PetType can remain EAGER as it's a simple lookup entity
|
||||
// and won't cause performance issues
|
||||
@ManyToOne(fetch = FetchType.EAGER)
|
||||
@JoinColumn(name = "type_id")
|
||||
private PetType type;
|
||||
|
||||
// Changed from EAGER to LAZY to avoid cascading N+1 queries
|
||||
// Added @BatchSize to optimize when visits are loaded
|
||||
@OneToMany(cascade = CascadeType.ALL, fetch = FetchType.LAZY)
|
||||
@JoinColumn(name = "pet_id")
|
||||
@OrderBy("date ASC")
|
||||
@BatchSize(size = 10)
|
||||
private final Set<Visit> visits = new LinkedHashSet<>();
|
||||
|
||||
public void setBirthDate(LocalDate birthDate) {
|
||||
|
|
|
|||
|
|
@ -40,7 +40,7 @@ public class Visit extends BaseEntity {
|
|||
@NotBlank
|
||||
private String description;
|
||||
|
||||
@ManyToOne(fetch = FetchType.LAZY)
|
||||
@ManyToOne(fetch = FetchType.EAGER)
|
||||
@JoinColumn(name = "pet_id")
|
||||
private Pet pet;
|
||||
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@
|
|||
<td th:text="${owner.address}" />
|
||||
<td th:text="${owner.city}" />
|
||||
<td th:text="${owner.telephone}" />
|
||||
<td><span th:text="${#strings.listJoin(owner.pets, ', ')}" /></td>
|
||||
<td><span th:text="${owner.petNames}" /></td>
|
||||
</tr>
|
||||
</tbody>
|
||||
</table>
|
||||
|
|
@ -58,4 +58,4 @@
|
|||
</div>
|
||||
</body>
|
||||
|
||||
</html>
|
||||
</html>
|
||||
|
|
|
|||
|
|
@ -49,13 +49,12 @@ public class PetClinicIntegrationTests {
|
|||
vets.findAll(); // served from cache
|
||||
}
|
||||
|
||||
// @Test
|
||||
// void testOwnerDetails() {
|
||||
// RestTemplate template = builder.rootUri("http://localhost:" + port).build();
|
||||
// ResponseEntity<String> result =
|
||||
// template.exchange(RequestEntity.get("/owners/1").build(), String.class);
|
||||
// assertThat(result.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
// }
|
||||
@Test
|
||||
void testOwnerDetails() {
|
||||
RestTemplate template = builder.rootUri("http://localhost:" + port).build();
|
||||
ResponseEntity<String> result = template.exchange(RequestEntity.get("/owners/1").build(), String.class);
|
||||
assertThat(result.getStatusCode()).isEqualTo(HttpStatus.OK);
|
||||
}
|
||||
|
||||
public static void main(String[] args) {
|
||||
SpringApplication.run(PetClinicApplication.class, args);
|
||||
|
|
|
|||
|
|
@ -89,16 +89,25 @@ class OwnerControllerTests {
|
|||
|
||||
@BeforeEach
|
||||
void setup() {
|
||||
|
||||
Owner george = george();
|
||||
given(this.owners.findByLastNameStartingWith(eq("Franklin"), any(Pageable.class)))
|
||||
.willReturn(new PageImpl<>(List.of(george)));
|
||||
|
||||
// Mock the optimized search method with DTOs
|
||||
OwnerSearchResult georgeSearchResult = new OwnerSearchResult(TEST_OWNER_ID, "George", "Franklin",
|
||||
"110 W. Liberty St.", "Madison", "6085551023", "Max");
|
||||
|
||||
given(this.owners.findOwnerSearchResultsByLastName(eq("Franklin"), any(Pageable.class)))
|
||||
.willReturn(new PageImpl<>(List.of(georgeSearchResult)));
|
||||
|
||||
// Mock EntityGraph methods for edit/detail views
|
||||
given(this.owners.findByIdWithPets(TEST_OWNER_ID)).willReturn(Optional.of(george));
|
||||
given(this.owners.findByIdWithPetsAndVisits(TEST_OWNER_ID)).willReturn(Optional.of(george));
|
||||
|
||||
// Keep old method for backward compatibility
|
||||
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);
|
||||
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -141,15 +150,27 @@ class OwnerControllerTests {
|
|||
|
||||
@Test
|
||||
void testProcessFindFormSuccess() throws Exception {
|
||||
Page<Owner> tasks = new PageImpl<>(List.of(george(), new Owner()));
|
||||
when(this.owners.findByLastNameStartingWith(anyString(), any(Pageable.class))).thenReturn(tasks);
|
||||
// Create search results using DTOs
|
||||
OwnerSearchResult result1 = new OwnerSearchResult(1, "George", "Franklin", "110 W. Liberty St.", "Madison",
|
||||
"6085551023", "Max");
|
||||
OwnerSearchResult result2 = new OwnerSearchResult(2, "Betty", "Davis", "638 Cardinal Ave.", "Sun Prairie",
|
||||
"6085551749", "Basil");
|
||||
|
||||
Page<OwnerSearchResult> tasks = new PageImpl<>(List.of(result1, result2));
|
||||
when(this.owners.findOwnerSearchResultsByLastName(anyString(), any(Pageable.class))).thenReturn(tasks);
|
||||
|
||||
mockMvc.perform(get("/owners?page=1")).andExpect(status().isOk()).andExpect(view().name("owners/ownersList"));
|
||||
}
|
||||
|
||||
@Test
|
||||
void testProcessFindFormByLastName() throws Exception {
|
||||
Page<Owner> tasks = new PageImpl<>(List.of(george()));
|
||||
when(this.owners.findByLastNameStartingWith(eq("Franklin"), any(Pageable.class))).thenReturn(tasks);
|
||||
// Single result - should redirect
|
||||
OwnerSearchResult result = new OwnerSearchResult(TEST_OWNER_ID, "George", "Franklin", "110 W. Liberty St.",
|
||||
"Madison", "6085551023", "Max");
|
||||
|
||||
Page<OwnerSearchResult> tasks = new PageImpl<>(List.of(result));
|
||||
when(this.owners.findOwnerSearchResultsByLastName(eq("Franklin"), any(Pageable.class))).thenReturn(tasks);
|
||||
|
||||
mockMvc.perform(get("/owners?page=1").param("lastName", "Franklin"))
|
||||
.andExpect(status().is3xxRedirection())
|
||||
.andExpect(view().name("redirect:/owners/" + TEST_OWNER_ID));
|
||||
|
|
@ -157,14 +178,15 @@ class OwnerControllerTests {
|
|||
|
||||
@Test
|
||||
void testProcessFindFormNoOwnersFound() throws Exception {
|
||||
Page<Owner> tasks = new PageImpl<>(List.of());
|
||||
when(this.owners.findByLastNameStartingWith(eq("Unknown Surname"), any(Pageable.class))).thenReturn(tasks);
|
||||
Page<OwnerSearchResult> tasks = new PageImpl<>(List.of());
|
||||
when(this.owners.findOwnerSearchResultsByLastName(eq("Unknown Surname"), any(Pageable.class)))
|
||||
.thenReturn(tasks);
|
||||
|
||||
mockMvc.perform(get("/owners?page=1").param("lastName", "Unknown Surname"))
|
||||
.andExpect(status().isOk())
|
||||
.andExpect(model().attributeHasFieldErrors("owner", "lastName"))
|
||||
.andExpect(model().attributeHasFieldErrorCode("owner", "lastName", "notFound"))
|
||||
.andExpect(view().name("owners/findOwners"));
|
||||
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -240,7 +262,7 @@ class OwnerControllerTests {
|
|||
owner.setCity("New York");
|
||||
owner.setTelephone("0123456789");
|
||||
|
||||
when(owners.findById(pathOwnerId)).thenReturn(Optional.of(owner));
|
||||
when(owners.findByIdWithPets(pathOwnerId)).thenReturn(Optional.of(owner));
|
||||
|
||||
mockMvc.perform(MockMvcRequestBuilders.post("/owners/{ownerId}/edit", pathOwnerId).flashAttr("owner", owner))
|
||||
.andExpect(status().is3xxRedirection())
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue