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 480a7a690..216cea6e0 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/Owner.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/Owner.java @@ -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 pets = new ArrayList<>(); 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 9fac6a5f6..d879461f8 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java @@ -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 + *

+ * Previously, this executed 1 + N + M queries where N = number of owners and M = + * total number of pets. + *

+ */ @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 ownersResults = findPaginatedForOwnersLastName(page, lastName); + // find owners by last name using optimized DTO projection + Page 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 paginated) { - List listOwners = paginated.getContent(); + private String addPaginationModel(int page, Model model, Page paginated) { + List 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 findPaginatedForOwnersLastName(int page, String lastname) { + private Page 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 optionalOwner = this.owners.findById(ownerId); + // Use EntityGraph to load owner with pets and visits in one query + Optional 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; } } 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 d2b3dde40..437ac4e97 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/OwnerRepository.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/OwnerRepository.java @@ -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 Owner domain objects. All method names are compliant @@ -44,6 +47,51 @@ public interface OwnerRepository extends JpaRepository { */ Page 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 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 findOwnerSearchResultsByLastName(String lastName, Pageable pageable) { + Page 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. *

@@ -59,4 +107,24 @@ public interface OwnerRepository extends JpaRepository { */ Optional 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 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 findByIdWithPetsAndVisits(@Param("id") Integer id); + } diff --git a/src/main/java/org/springframework/samples/petclinic/owner/OwnerSearchResult.java b/src/main/java/org/springframework/samples/petclinic/owner/OwnerSearchResult.java new file mode 100644 index 000000000..84f5f5f53 --- /dev/null +++ b/src/main/java/org/springframework/samples/petclinic/owner/OwnerSearchResult.java @@ -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(); + } + +} 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 e07735582..9f23ae79c 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/Pet.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/Pet.java @@ -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 visits = new LinkedHashSet<>(); public void setBirthDate(LocalDate birthDate) { diff --git a/src/main/java/org/springframework/samples/petclinic/owner/Visit.java b/src/main/java/org/springframework/samples/petclinic/owner/Visit.java index f2d67a77d..0b613049d 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/Visit.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/Visit.java @@ -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; diff --git a/src/main/resources/templates/owners/ownersList.html b/src/main/resources/templates/owners/ownersList.html index 01223c1c5..63a5221f0 100644 --- a/src/main/resources/templates/owners/ownersList.html +++ b/src/main/resources/templates/owners/ownersList.html @@ -24,7 +24,7 @@ - + @@ -58,4 +58,4 @@ - \ No newline at end of file + diff --git a/src/test/java/org/springframework/samples/petclinic/PetClinicIntegrationTests.java b/src/test/java/org/springframework/samples/petclinic/PetClinicIntegrationTests.java index 7e03efb86..8c0451dfd 100644 --- a/src/test/java/org/springframework/samples/petclinic/PetClinicIntegrationTests.java +++ b/src/test/java/org/springframework/samples/petclinic/PetClinicIntegrationTests.java @@ -49,13 +49,12 @@ public class PetClinicIntegrationTests { vets.findAll(); // served from cache } - // @Test - // void testOwnerDetails() { - // RestTemplate template = builder.rootUri("http://localhost:" + port).build(); - // ResponseEntity 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 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); 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 5c9824276..c959cb2a8 100644 --- a/src/test/java/org/springframework/samples/petclinic/owner/OwnerControllerTests.java +++ b/src/test/java/org/springframework/samples/petclinic/owner/OwnerControllerTests.java @@ -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 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 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 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 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 tasks = new PageImpl<>(List.of()); - when(this.owners.findByLastNameStartingWith(eq("Unknown Surname"), any(Pageable.class))).thenReturn(tasks); + Page 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())