mirror of
https://github.com/spring-projects/spring-petclinic.git
synced 2025-04-24 19:32:48 +00:00
Refactor:
- <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.
This commit is contained in:
parent
fdc40a7048
commit
14af47d4e5
2 changed files with 34 additions and 14 deletions
|
@ -16,7 +16,6 @@
|
||||||
package org.springframework.samples.petclinic.owner;
|
package org.springframework.samples.petclinic.owner;
|
||||||
|
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.Map;
|
|
||||||
|
|
||||||
import org.springframework.data.domain.Page;
|
import org.springframework.data.domain.Page;
|
||||||
import org.springframework.data.domain.PageRequest;
|
import org.springframework.data.domain.PageRequest;
|
||||||
|
@ -64,9 +63,7 @@ class OwnerController {
|
||||||
}
|
}
|
||||||
|
|
||||||
@GetMapping("/owners/new")
|
@GetMapping("/owners/new")
|
||||||
public String initCreationForm(Map<String, Object> model) {
|
public String initCreationForm() {
|
||||||
Owner owner = new Owner();
|
|
||||||
model.put("owner", owner);
|
|
||||||
return VIEWS_OWNER_CREATE_OR_UPDATE_FORM;
|
return VIEWS_OWNER_CREATE_OR_UPDATE_FORM;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -129,9 +126,7 @@ class OwnerController {
|
||||||
}
|
}
|
||||||
|
|
||||||
@GetMapping("/owners/{ownerId}/edit")
|
@GetMapping("/owners/{ownerId}/edit")
|
||||||
public String initUpdateOwnerForm(@PathVariable("ownerId") int ownerId, Model model) {
|
public String initUpdateOwnerForm() {
|
||||||
Owner owner = this.owners.findById(ownerId);
|
|
||||||
model.addAttribute(owner);
|
|
||||||
return VIEWS_OWNER_CREATE_OR_UPDATE_FORM;
|
return VIEWS_OWNER_CREATE_OR_UPDATE_FORM;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -143,6 +138,12 @@ class OwnerController {
|
||||||
return VIEWS_OWNER_CREATE_OR_UPDATE_FORM;
|
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);
|
owner.setId(ownerId);
|
||||||
this.owners.save(owner);
|
this.owners.save(owner);
|
||||||
redirectAttributes.addFlashAttribute("message", "Owner Values Updated");
|
redirectAttributes.addFlashAttribute("message", "Owner Values Updated");
|
||||||
|
|
|
@ -20,7 +20,6 @@ import org.assertj.core.util.Lists;
|
||||||
import org.junit.jupiter.api.BeforeEach;
|
import org.junit.jupiter.api.BeforeEach;
|
||||||
import org.junit.jupiter.api.Test;
|
import org.junit.jupiter.api.Test;
|
||||||
import org.junit.jupiter.api.condition.DisabledInNativeImage;
|
import org.junit.jupiter.api.condition.DisabledInNativeImage;
|
||||||
import org.mockito.Mockito;
|
|
||||||
import org.springframework.beans.factory.annotation.Autowired;
|
import org.springframework.beans.factory.annotation.Autowired;
|
||||||
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
|
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
|
||||||
import org.springframework.boot.test.mock.mockito.MockBean;
|
import org.springframework.boot.test.mock.mockito.MockBean;
|
||||||
|
@ -29,6 +28,7 @@ import org.springframework.data.domain.PageImpl;
|
||||||
import org.springframework.data.domain.Pageable;
|
import org.springframework.data.domain.Pageable;
|
||||||
import org.springframework.test.context.aot.DisabledInAotMode;
|
import org.springframework.test.context.aot.DisabledInAotMode;
|
||||||
import org.springframework.test.web.servlet.MockMvc;
|
import org.springframework.test.web.servlet.MockMvc;
|
||||||
|
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
|
||||||
|
|
||||||
import java.time.LocalDate;
|
import java.time.LocalDate;
|
||||||
|
|
||||||
|
@ -43,11 +43,10 @@ import static org.mockito.ArgumentMatchers.any;
|
||||||
import static org.mockito.ArgumentMatchers.anyString;
|
import static org.mockito.ArgumentMatchers.anyString;
|
||||||
import static org.mockito.ArgumentMatchers.eq;
|
import static org.mockito.ArgumentMatchers.eq;
|
||||||
import static org.mockito.BDDMockito.given;
|
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.get;
|
||||||
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
|
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.*;
|
||||||
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
|
|
||||||
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.view;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Test class for {@link OwnerController}
|
* Test class for {@link OwnerController}
|
||||||
|
@ -143,14 +142,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()));
|
||||||
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"));
|
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()));
|
||||||
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"))
|
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));
|
||||||
|
@ -159,7 +158,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());
|
||||||
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"))
|
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"))
|
||||||
|
@ -229,4 +228,24 @@ class OwnerControllerTests {
|
||||||
.andExpect(view().name("owners/ownerDetails"));
|
.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"));
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue