Refactor the logic and add unit test

-<add>: add `@NotBlank` validation to pet's name.
-<refactor>: delete useless code and add unit test to check duplicate Pet name validation logic.
-<modify>: add `Id` to pet in unit test.
-<refactor>: classify unit test.

<modify>: adjust code format.
This commit is contained in:
YiXuan Ding 2024-11-07 17:37:12 +08:00
parent 14af47d4e5
commit 50866def72
3 changed files with 114 additions and 39 deletions

View file

@ -17,6 +17,7 @@ package org.springframework.samples.petclinic.model;
import jakarta.persistence.Column; import jakarta.persistence.Column;
import jakarta.persistence.MappedSuperclass; import jakarta.persistence.MappedSuperclass;
import jakarta.validation.constraints.NotBlank;
/** /**
* Simple JavaBean domain object adds a name property to <code>BaseEntity</code>. Used as * Simple JavaBean domain object adds a name property to <code>BaseEntity</code>. Used as
@ -29,6 +30,7 @@ import jakarta.persistence.MappedSuperclass;
public class NamedEntity extends BaseEntity { public class NamedEntity extends BaseEntity {
@Column(name = "name") @Column(name = "name")
@NotBlank
private String name; private String name;
public String getName() { public String getName() {

View file

@ -99,45 +99,41 @@ class PetController {
} }
@PostMapping("/pets/new") @PostMapping("/pets/new")
public String processCreationForm(Owner owner, @Valid Pet pet, BindingResult result, ModelMap model, public String processCreationForm(Owner owner, @Valid Pet pet, BindingResult result,
RedirectAttributes redirectAttributes) { RedirectAttributes redirectAttributes) {
if (StringUtils.hasText(pet.getName()) && pet.isNew() && owner.getPet(pet.getName(), true) != null) {
if (StringUtils.hasText(pet.getName()) && pet.isNew() && owner.getPet(pet.getName(), true) != null)
result.rejectValue("name", "duplicate", "already exists"); result.rejectValue("name", "duplicate", "already exists");
}
LocalDate currentDate = LocalDate.now(); LocalDate currentDate = LocalDate.now();
if (pet.getBirthDate() != null && pet.getBirthDate().isAfter(currentDate)) { if (pet.getBirthDate() != null && pet.getBirthDate().isAfter(currentDate)) {
result.rejectValue("birthDate", "typeMismatch.birthDate"); result.rejectValue("birthDate", "typeMismatch.birthDate");
} }
owner.addPet(pet);
if (result.hasErrors()) { if (result.hasErrors()) {
model.put("pet", pet);
return VIEWS_PETS_CREATE_OR_UPDATE_FORM; return VIEWS_PETS_CREATE_OR_UPDATE_FORM;
} }
owner.addPet(pet);
this.owners.save(owner); this.owners.save(owner);
redirectAttributes.addFlashAttribute("message", "New Pet has been Added"); redirectAttributes.addFlashAttribute("message", "New Pet has been Added");
return "redirect:/owners/{ownerId}"; return "redirect:/owners/{ownerId}";
} }
@GetMapping("/pets/{petId}/edit") @GetMapping("/pets/{petId}/edit")
public String initUpdateForm(Owner owner, @PathVariable("petId") int petId, ModelMap model, public String initUpdateForm(Owner owner, @PathVariable("petId") int petId, RedirectAttributes redirectAttributes) {
RedirectAttributes redirectAttributes) {
Pet pet = owner.getPet(petId);
model.put("pet", pet);
return VIEWS_PETS_CREATE_OR_UPDATE_FORM; return VIEWS_PETS_CREATE_OR_UPDATE_FORM;
} }
@PostMapping("/pets/{petId}/edit") @PostMapping("/pets/{petId}/edit")
public String processUpdateForm(@Valid Pet pet, BindingResult result, Owner owner, ModelMap model, public String processUpdateForm(Owner owner, @Valid Pet pet, BindingResult result,
RedirectAttributes redirectAttributes) { RedirectAttributes redirectAttributes) {
String petName = pet.getName(); String petName = pet.getName();
// checking if the pet name already exist for the owner // checking if the pet name already exist for the owner
if (StringUtils.hasText(petName)) { if (StringUtils.hasText(petName)) {
Pet existingPet = owner.getPet(petName.toLowerCase(), false); Pet existingPet = owner.getPet(petName, false);
if (existingPet != null && !existingPet.getId().equals(pet.getId())) { if (existingPet != null && !existingPet.getId().equals(pet.getId())) {
result.rejectValue("name", "duplicate", "already exists"); result.rejectValue("name", "duplicate", "already exists");
} }
@ -149,7 +145,6 @@ class PetController {
} }
if (result.hasErrors()) { if (result.hasErrors()) {
model.put("pet", pet);
return VIEWS_PETS_CREATE_OR_UPDATE_FORM; return VIEWS_PETS_CREATE_OR_UPDATE_FORM;
} }

View file

@ -17,9 +17,12 @@
package org.springframework.samples.petclinic.owner; package org.springframework.samples.petclinic.owner;
import org.assertj.core.util.Lists; import org.assertj.core.util.Lists;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
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;
@ -28,6 +31,10 @@ import org.springframework.context.annotation.FilterType;
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 java.time.LocalDate;
import java.util.Optional;
import static org.junit.Assert.assertNotNull;
import static org.mockito.BDDMockito.given; import static org.mockito.BDDMockito.given;
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;
@ -62,10 +69,16 @@ class PetControllerTests {
cat.setId(3); cat.setId(3);
cat.setName("hamster"); cat.setName("hamster");
given(this.owners.findPetTypes()).willReturn(Lists.newArrayList(cat)); given(this.owners.findPetTypes()).willReturn(Lists.newArrayList(cat));
Owner owner = new Owner(); Owner owner = new Owner();
Pet pet = new Pet(); Pet pet = new Pet();
Pet dog = new Pet();
owner.addPet(pet); owner.addPet(pet);
owner.addPet(dog);
pet.setId(TEST_PET_ID); pet.setId(TEST_PET_ID);
dog.setId(TEST_PET_ID + 1);
pet.setName("petty");
dog.setName("doggy");
given(this.owners.findById(TEST_OWNER_ID)).willReturn(owner); given(this.owners.findById(TEST_OWNER_ID)).willReturn(owner);
} }
@ -87,25 +100,72 @@ class PetControllerTests {
.andExpect(view().name("redirect:/owners/{ownerId}")); .andExpect(view().name("redirect:/owners/{ownerId}"));
} }
@Test @Nested
void testProcessCreationFormHasErrors() throws Exception { class ProcessCreationFormHasErrors {
mockMvc
.perform(post("/owners/{ownerId}/pets/new", TEST_OWNER_ID).param("name", "Betty") @Test
.param("birthDate", "2015-02-12")) void testProcessCreationFormWithBlankName() throws Exception {
.andExpect(model().attributeHasNoErrors("owner")) mockMvc
.andExpect(model().attributeHasErrors("pet")) .perform(post("/owners/{ownerId}/pets/new", TEST_OWNER_ID).param("name", "\t \n")
.andExpect(model().attributeHasFieldErrors("pet", "type")) .param("birthDate", "2015-02-12"))
.andExpect(model().attributeHasFieldErrorCode("pet", "type", "required")) .andExpect(model().attributeHasNoErrors("owner"))
.andExpect(status().isOk()) .andExpect(model().attributeHasErrors("pet"))
.andExpect(view().name("pets/createOrUpdatePetForm")); .andExpect(model().attributeHasFieldErrors("pet", "name"))
} .andExpect(model().attributeHasFieldErrorCode("pet", "name", "required"))
.andExpect(status().isOk())
.andExpect(view().name("pets/createOrUpdatePetForm"));
}
@Test
void testProcessCreationFormWithDuplicateName() throws Exception {
mockMvc
.perform(post("/owners/{ownerId}/pets/new", TEST_OWNER_ID).param("name", "petty")
.param("birthDate", "2015-02-12"))
.andExpect(model().attributeHasNoErrors("owner"))
.andExpect(model().attributeHasErrors("pet"))
.andExpect(model().attributeHasFieldErrors("pet", "name"))
.andExpect(model().attributeHasFieldErrorCode("pet", "name", "duplicate"))
.andExpect(status().isOk())
.andExpect(view().name("pets/createOrUpdatePetForm"));
}
@Test
void testProcessCreationFormWithMissingPetType() throws Exception {
mockMvc
.perform(post("/owners/{ownerId}/pets/new", TEST_OWNER_ID).param("name", "Betty")
.param("birthDate", "2015-02-12"))
.andExpect(model().attributeHasNoErrors("owner"))
.andExpect(model().attributeHasErrors("pet"))
.andExpect(model().attributeHasFieldErrors("pet", "type"))
.andExpect(model().attributeHasFieldErrorCode("pet", "type", "required"))
.andExpect(status().isOk())
.andExpect(view().name("pets/createOrUpdatePetForm"));
}
@Test
void testProcessCreationFormWithInvalidBirthDate() throws Exception {
LocalDate currentDate = LocalDate.now();
String futureBirthDate = currentDate.plusMonths(1).toString();
mockMvc
.perform(post("/owners/{ownerId}/pets/new", TEST_OWNER_ID).param("name", "Betty")
.param("birthDate", futureBirthDate))
.andExpect(model().attributeHasNoErrors("owner"))
.andExpect(model().attributeHasErrors("pet"))
.andExpect(model().attributeHasFieldErrors("pet", "birthDate"))
.andExpect(model().attributeHasFieldErrorCode("pet", "birthDate", "typeMismatch.birthDate"))
.andExpect(status().isOk())
.andExpect(view().name("pets/createOrUpdatePetForm"));
}
@Test
void testInitUpdateForm() throws Exception {
mockMvc.perform(get("/owners/{ownerId}/pets/{petId}/edit", TEST_OWNER_ID, TEST_PET_ID))
.andExpect(status().isOk())
.andExpect(model().attributeExists("pet"))
.andExpect(view().name("pets/createOrUpdatePetForm"));
}
@Test
void testInitUpdateForm() throws Exception {
mockMvc.perform(get("/owners/{ownerId}/pets/{petId}/edit", TEST_OWNER_ID, TEST_PET_ID))
.andExpect(status().isOk())
.andExpect(model().attributeExists("pet"))
.andExpect(view().name("pets/createOrUpdatePetForm"));
} }
@Test @Test
@ -118,15 +178,33 @@ class PetControllerTests {
.andExpect(view().name("redirect:/owners/{ownerId}")); .andExpect(view().name("redirect:/owners/{ownerId}"));
} }
@Test @Nested
void testProcessUpdateFormHasErrors() throws Exception { class ProcessUpdateFormHasErrors {
mockMvc
.perform(post("/owners/{ownerId}/pets/{petId}/edit", TEST_OWNER_ID, TEST_PET_ID).param("name", "Betty") @Test
.param("birthDate", "2015/02/12")) void testProcessUpdateFormWithInvalidBirthDate() throws Exception {
.andExpect(model().attributeHasNoErrors("owner")) mockMvc
.andExpect(model().attributeHasErrors("pet")) .perform(post("/owners/{ownerId}/pets/{petId}/edit", TEST_OWNER_ID, TEST_PET_ID).param("name", " ")
.andExpect(status().isOk()) .param("birthDate", "2015/02/12"))
.andExpect(view().name("pets/createOrUpdatePetForm")); .andExpect(model().attributeHasNoErrors("owner"))
.andExpect(model().attributeHasErrors("pet"))
.andExpect(model().attributeHasFieldErrors("pet", "birthDate"))
.andExpect(model().attributeHasFieldErrorCode("pet", "birthDate", "typeMismatch"))
.andExpect(view().name("pets/createOrUpdatePetForm"));
}
@Test
void testProcessUpdateFormWithBlankName() throws Exception {
mockMvc
.perform(post("/owners/{ownerId}/pets/{petId}/edit", TEST_OWNER_ID, TEST_PET_ID).param("name", " ")
.param("birthDate", "2015-02-12"))
.andExpect(model().attributeHasNoErrors("owner"))
.andExpect(model().attributeHasErrors("pet"))
.andExpect(model().attributeHasFieldErrors("pet", "name"))
.andExpect(model().attributeHasFieldErrorCode("pet", "name", "required"))
.andExpect(view().name("pets/createOrUpdatePetForm"));
}
} }
} }