From 40421585b98e2b0500c105e88ede702549c48682 Mon Sep 17 00:00:00 2001 From: YiXuan Ding <1328032567@qq.com> Date: Wed, 6 Nov 2024 20:09:08 +0800 Subject: [PATCH] Refactor: - : delete logic `add owner to model` because of the comment `@ModelAttribute("owner")`. - : add logical judgment in ordet to avoid `owner` from `form` and `ownerId` from `url` mismatch. --- .../petclinic/owner/OwnerController.java | 15 +++++---- .../petclinic/owner/OwnerControllerTests.java | 33 +++++++++++++++---- 2 files changed, 34 insertions(+), 14 deletions(-) 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 75512893dff..490b1028709 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/OwnerController.java @@ -16,7 +16,6 @@ package org.springframework.samples.petclinic.owner; import java.util.List; -import java.util.Map; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; @@ -64,9 +63,7 @@ public Owner findOwner(@PathVariable(name = "ownerId", required = false) Integer } @GetMapping("/owners/new") - public String initCreationForm(Map model) { - Owner owner = new Owner(); - model.put("owner", owner); + public String initCreationForm() { return VIEWS_OWNER_CREATE_OR_UPDATE_FORM; } @@ -129,9 +126,7 @@ private Page findPaginatedForOwnersLastName(int page, String lastname) { } @GetMapping("/owners/{ownerId}/edit") - public String initUpdateOwnerForm(@PathVariable("ownerId") int ownerId, Model model) { - Owner owner = this.owners.findById(ownerId); - model.addAttribute(owner); + public String initUpdateOwnerForm() { return VIEWS_OWNER_CREATE_OR_UPDATE_FORM; } @@ -143,6 +138,12 @@ public String processUpdateOwnerForm(@Valid Owner owner, BindingResult result, @ return VIEWS_OWNER_CREATE_OR_UPDATE_FORM; } + if (owner.getId() != ownerId) { + result.rejectValue("id", "mismatch", "The owner ID in the form does not match the URL."); + redirectAttributes.addFlashAttribute("error", "Owner ID mismatch. Please try again."); + return "redirect:/owners/{ownerId}/edit"; + } + owner.setId(ownerId); this.owners.save(owner); redirectAttributes.addFlashAttribute("message", "Owner Values Updated"); 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 fc22fb01aeb..f88c1f1ddd9 100644 --- a/src/test/java/org/springframework/samples/petclinic/owner/OwnerControllerTests.java +++ b/src/test/java/org/springframework/samples/petclinic/owner/OwnerControllerTests.java @@ -20,7 +20,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledInNativeImage; -import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; import org.springframework.boot.test.mock.mockito.MockBean; @@ -29,6 +28,7 @@ import org.springframework.data.domain.Pageable; import org.springframework.test.context.aot.DisabledInAotMode; import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; import java.time.LocalDate; @@ -43,11 +43,10 @@ import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.when; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.model; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; -import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.view; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; /** * Test class for {@link OwnerController} @@ -143,14 +142,14 @@ void testInitFindForm() throws Exception { @Test void testProcessFindFormSuccess() throws Exception { Page tasks = new PageImpl<>(Lists.newArrayList(george(), new Owner())); - Mockito.when(this.owners.findByLastName(anyString(), any(Pageable.class))).thenReturn(tasks); + when(this.owners.findByLastName(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<>(Lists.newArrayList(george())); - Mockito.when(this.owners.findByLastName(eq("Franklin"), any(Pageable.class))).thenReturn(tasks); + when(this.owners.findByLastName(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)); @@ -159,7 +158,7 @@ void testProcessFindFormByLastName() throws Exception { @Test void testProcessFindFormNoOwnersFound() throws Exception { Page tasks = new PageImpl<>(Lists.newArrayList()); - Mockito.when(this.owners.findByLastName(eq("Unknown Surname"), any(Pageable.class))).thenReturn(tasks); + when(this.owners.findByLastName(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")) @@ -229,4 +228,24 @@ void testShowOwner() throws Exception { .andExpect(view().name("owners/ownerDetails")); } + @Test + public void testProcessUpdateOwnerFormWithIdMismatch() throws Exception { + int pathOwnerId = 1; + + Owner owner = new Owner(); + owner.setId(2); + owner.setFirstName("John"); + owner.setLastName("Doe"); + owner.setAddress("Center Street"); + owner.setCity("New York"); + owner.setTelephone("0123456789"); + + when(owners.findById(pathOwnerId)).thenReturn(owner); + + mockMvc.perform(MockMvcRequestBuilders.post("/owners/{ownerId}/edit", pathOwnerId).flashAttr("owner", owner)) + .andExpect(status().is3xxRedirection()) + .andExpect(redirectedUrl("/owners/" + pathOwnerId + "/edit")) + .andExpect(flash().attributeExists("error")); + } + }