Skip to content

Commit

Permalink
Refactor:
Browse files Browse the repository at this point in the history
- <optimize>: delete logic `add owner to model` because of the comment `@ModelAttribute("owner")`.
- <fix>: add logical judgment in ordet to avoid `owner` from `form` and `ownerId` from `url` mismatch.
  • Loading branch information
wickdynex committed Nov 6, 2024
1 parent fdc40a7 commit 4042158
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -64,9 +63,7 @@ public Owner findOwner(@PathVariable(name = "ownerId", required = false) Integer
}

@GetMapping("/owners/new")
public String initCreationForm(Map<String, Object> model) {
Owner owner = new Owner();
model.put("owner", owner);
public String initCreationForm() {
return VIEWS_OWNER_CREATE_OR_UPDATE_FORM;
}

Expand Down Expand Up @@ -129,9 +126,7 @@ private Page<Owner> 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;
}

Expand All @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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}
Expand Down Expand Up @@ -143,14 +142,14 @@ void testInitFindForm() throws Exception {
@Test
void testProcessFindFormSuccess() throws Exception {
Page<Owner> 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<Owner> 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));
Expand All @@ -159,7 +158,7 @@ void testProcessFindFormByLastName() throws Exception {
@Test
void testProcessFindFormNoOwnersFound() throws Exception {
Page<Owner> 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"))
Expand Down Expand Up @@ -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"));
}

}

0 comments on commit 4042158

Please sign in to comment.