From 66c486891a524dabbf74eacbcae9ea1560ce9526 Mon Sep 17 00:00:00 2001 From: PEDSF Date: Sat, 10 Oct 2020 12:22:17 +0200 Subject: [PATCH] Change PetTypeFormatter and Validator to use DTO --- .../petclinic/controller/OwnerController.java | 9 +- .../petclinic/controller/PetController.java | 11 +- .../petclinic/validator/PetDTOValidator.java | 65 ++++++++ .../petclinic/validator/PetValidator.java | 2 +- .../controller/OwnerControllerTests.java | 152 +++++++++++------- .../owner/PetTypeDTOFormatterTests.java | 97 +++++++++++ 6 files changed, 263 insertions(+), 73 deletions(-) create mode 100644 src/main/java/org/springframework/samples/petclinic/validator/PetDTOValidator.java create mode 100644 src/test/java/org/springframework/samples/petclinic/owner/PetTypeDTOFormatterTests.java diff --git a/src/main/java/org/springframework/samples/petclinic/controller/OwnerController.java b/src/main/java/org/springframework/samples/petclinic/controller/OwnerController.java index 47ab04ad2..8993161d9 100644 --- a/src/main/java/org/springframework/samples/petclinic/controller/OwnerController.java +++ b/src/main/java/org/springframework/samples/petclinic/controller/OwnerController.java @@ -22,10 +22,7 @@ import org.springframework.stereotype.Controller; import org.springframework.ui.Model; import org.springframework.validation.BindingResult; import org.springframework.web.bind.WebDataBinder; -import org.springframework.web.bind.annotation.GetMapping; -import org.springframework.web.bind.annotation.InitBinder; -import org.springframework.web.bind.annotation.PathVariable; -import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.*; import org.springframework.web.servlet.ModelAndView; import javax.validation.Valid; @@ -53,7 +50,7 @@ class OwnerController { this.visitService = visitService; } - @InitBinder + @InitBinder("owner") public void setAllowedFields(WebDataBinder dataBinder) { dataBinder.setDisallowedFields("id"); } @@ -66,7 +63,7 @@ class OwnerController { } @PostMapping("/owners/new") - public String processCreationForm(@Valid OwnerDTO owner, BindingResult result) { + public String processCreationForm(@ModelAttribute("owner") @Valid OwnerDTO owner, BindingResult result) { if (result.hasErrors()) { return VIEWS_OWNER_CREATE_OR_UPDATE_FORM; } diff --git a/src/main/java/org/springframework/samples/petclinic/controller/PetController.java b/src/main/java/org/springframework/samples/petclinic/controller/PetController.java index 2bd6eb9a3..198b83923 100644 --- a/src/main/java/org/springframework/samples/petclinic/controller/PetController.java +++ b/src/main/java/org/springframework/samples/petclinic/controller/PetController.java @@ -16,7 +16,7 @@ package org.springframework.samples.petclinic.controller; import org.springframework.samples.petclinic.dto.*; -import org.springframework.samples.petclinic.validator.PetValidator; +import org.springframework.samples.petclinic.validator.PetDTOValidator; import org.springframework.samples.petclinic.service.*; import org.springframework.stereotype.Controller; import org.springframework.ui.ModelMap; @@ -69,7 +69,7 @@ class PetController { @InitBinder("pet") public void initPetBinder(WebDataBinder dataBinder) { - dataBinder.setValidator(new PetValidator()); + dataBinder.setValidator(new PetDTOValidator()); } @GetMapping("/pets/new") @@ -81,11 +81,12 @@ class PetController { } @PostMapping("/pets/new") - public String processCreationForm(OwnerDTO ownerDTO, @Valid PetDTO pet, BindingResult result, ModelMap model) { - if (StringUtils.hasLength(pet.getName()) && pet.isNew() && ownerDTO.getPet(pet.getName(), true) != null) { + public String processCreationForm(@ModelAttribute("owner") OwnerDTO owner, + @ModelAttribute("pet") @Valid PetDTO pet, BindingResult result, ModelMap model) { + if (StringUtils.hasLength(pet.getName()) && pet.isNew() && owner.getPet(pet.getName(), true) != null) { result.rejectValue("name", "duplicate", "already exists"); } - ownerDTO.addPet(pet); + owner.addPet(pet); if (result.hasErrors()) { model.put("pet", pet); return VIEWS_PETS_CREATE_OR_UPDATE_FORM; diff --git a/src/main/java/org/springframework/samples/petclinic/validator/PetDTOValidator.java b/src/main/java/org/springframework/samples/petclinic/validator/PetDTOValidator.java new file mode 100644 index 000000000..90333ff6b --- /dev/null +++ b/src/main/java/org/springframework/samples/petclinic/validator/PetDTOValidator.java @@ -0,0 +1,65 @@ +/* + * Copyright 2012-2019 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.validator; + +import org.springframework.samples.petclinic.dto.PetDTO; +import org.springframework.util.StringUtils; +import org.springframework.validation.Errors; +import org.springframework.validation.Validator; + +/** + * Validator for PetDTO forms. + *

+ * We're not using Bean Validation annotations here because it is easier to define such + * validation rule in Java. + *

+ * + * @author Ken Krebs + * @author Juergen Hoeller + */ +public class PetDTOValidator implements Validator { + + private static final String REQUIRED = "required"; + + @Override + public void validate(Object obj, Errors errors) { + PetDTO pet = (PetDTO) obj; + String name = pet.getName(); + // name validation + if (!StringUtils.hasLength(name)) { + errors.rejectValue("name", REQUIRED, REQUIRED); + } + + // type validation + if (pet.isNew() && pet.getType() == null) { + errors.rejectValue("type", REQUIRED, REQUIRED); + } + + // birth date validation + if (pet.getBirthDate() == null) { + errors.rejectValue("birthDate", REQUIRED, REQUIRED); + } + } + + /** + * This Validator validates *just* Pet instances + */ + @Override + public boolean supports(Class clazz) { + return PetDTO.class.isAssignableFrom(clazz); + } + +} diff --git a/src/main/java/org/springframework/samples/petclinic/validator/PetValidator.java b/src/main/java/org/springframework/samples/petclinic/validator/PetValidator.java index 6c96f217d..c85c4113e 100644 --- a/src/main/java/org/springframework/samples/petclinic/validator/PetValidator.java +++ b/src/main/java/org/springframework/samples/petclinic/validator/PetValidator.java @@ -37,7 +37,7 @@ public class PetValidator implements Validator { @Override public void validate(Object obj, Errors errors) { - PetDTO pet = (PetDTO) obj; + Pet pet = (Pet) obj; String name = pet.getName(); // name validation if (!StringUtils.hasLength(name)) { diff --git a/src/test/java/org/springframework/samples/petclinic/controller/OwnerControllerTests.java b/src/test/java/org/springframework/samples/petclinic/controller/OwnerControllerTests.java index 82fda30c4..eff339087 100644 --- a/src/test/java/org/springframework/samples/petclinic/controller/OwnerControllerTests.java +++ b/src/test/java/org/springframework/samples/petclinic/controller/OwnerControllerTests.java @@ -93,111 +93,141 @@ class OwnerControllerTests { @Test void testInitCreationForm() throws Exception { - mockMvc.perform(get("/owners/new")).andExpect(status().isOk()).andExpect(model().attributeExists("owner")) - .andExpect(view().name("owners/createOrUpdateOwnerForm")); + mockMvc.perform(get("/owners/new")) + .andExpect(status().isOk()) + .andExpect(model().attributeExists("owner")) + .andExpect(view().name("owners/createOrUpdateOwnerForm")); } @Test void testProcessCreationFormSuccess() throws Exception { - mockMvc.perform(post("/owners/new").param("firstName", "Joe").param("lastName", "Bloggs") - .param("address", "123 Caramel Street").param("city", "London").param("telephone", "01316761638")) - .andExpect(status().is3xxRedirection()); + mockMvc.perform(post("/owners/new") + .param("firstName", "Joe") + .param("lastName", "Bloggs") + .param("address", "123 Caramel Street") + .param("city", "London") + .param("telephone", "01316761638")) + .andExpect(status().is3xxRedirection()); } @Test void testProcessCreationFormHasErrors() throws Exception { - mockMvc.perform( - post("/owners/new").param("firstName", "Joe").param("lastName", "Bloggs").param("city", "London")) - .andExpect(status().isOk()).andExpect(model().attributeHasErrors("owner")) - .andExpect(model().attributeHasFieldErrors("owner", "address")) - .andExpect(model().attributeHasFieldErrors("owner", "telephone")) - .andExpect(view().name("owners/createOrUpdateOwnerForm")); + mockMvc.perform(post("/owners/new") + .param("firstName", "Joe") + .param("lastName", "Bloggs") + .param("city", "London")) + .andExpect(status().isOk()) + .andExpect(model().attributeHasErrors("owner")) + .andExpect(model().attributeHasFieldErrors("owner", "address")) + .andExpect(model().attributeHasFieldErrors("owner", "telephone")) + .andExpect(view().name("owners/createOrUpdateOwnerForm")); } @Test void testInitFindForm() throws Exception { - mockMvc.perform(get("/owners/find")).andExpect(status().isOk()).andExpect(model().attributeExists("owner")) - .andExpect(view().name("owners/findOwners")); + mockMvc.perform(get("/owners/find")) + .andExpect(status().isOk()) + .andExpect(model().attributeExists("owner")) + .andExpect(view().name("owners/findOwners")); } @Test void testProcessFindFormSuccess() throws Exception { given(this.owners.findByLastName("")).willReturn(Lists.newArrayList(george, new OwnerDTO())); - mockMvc.perform(get("/owners")).andExpect(status().isOk()).andExpect(view().name("owners/ownersList")); + + mockMvc.perform(get("/owners")) + .andExpect(status().isOk()) + .andExpect(view().name("owners/ownersList")); } @Test void testProcessFindFormByLastName() throws Exception { given(this.owners.findByLastName(george.getLastName())).willReturn(Lists.newArrayList(george)); - mockMvc.perform(get("/owners").param("lastName", "Franklin")).andExpect(status().is3xxRedirection()) - .andExpect(view().name("redirect:/owners/" + TEST_OWNER_ID)); + + mockMvc.perform(get("/owners") + .param("lastName", "Franklin")) + .andExpect(status().is3xxRedirection()) + .andExpect(view().name("redirect:/owners/" + TEST_OWNER_ID)); } @Test void testProcessFindFormNoOwnersFound() throws Exception { - mockMvc.perform(get("/owners").param("lastName", "Unknown Surname")).andExpect(status().isOk()) - .andExpect(model().attributeHasFieldErrors("owner", "lastName")) - .andExpect(model().attributeHasFieldErrorCode("owner", "lastName", "notFound")) - .andExpect(view().name("owners/findOwners")); + mockMvc.perform(get("/owners") + .param("lastName", "Unknown Surname")) + .andExpect(status().isOk()) + .andExpect(model().attributeHasFieldErrors("owner", "lastName")) + .andExpect(model().attributeHasFieldErrorCode("owner", "lastName", "notFound")) + .andExpect(view().name("owners/findOwners")); } @Test void testInitUpdateOwnerForm() throws Exception { - mockMvc.perform(get("/owners/{ownerId}/edit", TEST_OWNER_ID)).andExpect(status().isOk()) - .andExpect(model().attributeExists("owner")) - .andExpect(model().attribute("owner", hasProperty("lastName", is("Franklin")))) - .andExpect(model().attribute("owner", hasProperty("firstName", is("George")))) - .andExpect(model().attribute("owner", hasProperty("address", is("110 W. Liberty St.")))) - .andExpect(model().attribute("owner", hasProperty("city", is("Madison")))) - .andExpect(model().attribute("owner", hasProperty("telephone", is("6085551023")))) - .andExpect(view().name("owners/createOrUpdateOwnerForm")); + mockMvc.perform(get("/owners/{ownerId}/edit", TEST_OWNER_ID)) + .andExpect(status().isOk()) + .andExpect(model().attributeExists("owner")) + .andExpect(model().attribute("owner", hasProperty("lastName", is("Franklin")))) + .andExpect(model().attribute("owner", hasProperty("firstName", is("George")))) + .andExpect(model().attribute("owner", hasProperty("address", is("110 W. Liberty St.")))) + .andExpect(model().attribute("owner", hasProperty("city", is("Madison")))) + .andExpect(model().attribute("owner", hasProperty("telephone", is("6085551023")))) + .andExpect(view().name("owners/createOrUpdateOwnerForm")); } @Test void testProcessUpdateOwnerFormSuccess() throws Exception { - mockMvc.perform(post("/owners/{ownerId}/edit", TEST_OWNER_ID).param("firstName", "Joe") - .param("lastName", "Bloggs").param("address", "123 Caramel Street").param("city", "London") - .param("telephone", "01616291589")).andExpect(status().is3xxRedirection()) - .andExpect(view().name("redirect:/owners/{ownerId}")); + mockMvc.perform(post("/owners/{ownerId}/edit", TEST_OWNER_ID) + .param("firstName", "Joe") + .param("lastName", "Bloggs") + .param("address", "123 Caramel Street") + .param("city", "London") + .param("telephone", "01616291589")) + .andExpect(status().is3xxRedirection()) + .andExpect(view().name("redirect:/owners/{ownerId}")); } @Test void testProcessUpdateOwnerFormHasErrors() throws Exception { - mockMvc.perform(post("/owners/{ownerId}/edit", TEST_OWNER_ID).param("firstName", "Joe") - .param("lastName", "Bloggs").param("city", "London")).andExpect(status().isOk()) - .andExpect(model().attributeHasErrors("owner")) - .andExpect(model().attributeHasFieldErrors("owner", "address")) - .andExpect(model().attributeHasFieldErrors("owner", "telephone")) - .andExpect(view().name("owners/createOrUpdateOwnerForm")); + mockMvc.perform(post("/owners/{ownerId}/edit", TEST_OWNER_ID) + .param("firstName", "Joe") + .param("lastName", "Bloggs") + .param("city", "London")) + .andExpect(status().isOk()) + .andExpect(model().attributeHasErrors("owner")) + .andExpect(model().attributeHasFieldErrors("owner", "address")) + .andExpect(model().attributeHasFieldErrors("owner", "telephone")) + .andExpect(view().name("owners/createOrUpdateOwnerForm")); } @Test void testShowOwner() throws Exception { - mockMvc.perform(get("/owners/{ownerId}", TEST_OWNER_ID)).andExpect(status().isOk()) - .andExpect(model().attribute("owner", hasProperty("lastName", is("Franklin")))) - .andExpect(model().attribute("owner", hasProperty("firstName", is("George")))) - .andExpect(model().attribute("owner", hasProperty("address", is("110 W. Liberty St.")))) - .andExpect(model().attribute("owner", hasProperty("city", is("Madison")))) - .andExpect(model().attribute("owner", hasProperty("telephone", is("6085551023")))) - .andExpect(model().attribute("owner", hasProperty("pets", not(empty())))) - .andExpect(model().attribute("owner", hasProperty("pets", new BaseMatcher>() { + mockMvc.perform(get("/owners/{ownerId}", TEST_OWNER_ID)) + .andExpect(status().isOk()) + .andExpect(model().attribute("owner", hasProperty("lastName", is("Franklin")))) + .andExpect(model().attribute("owner", hasProperty("firstName", is("George")))) + .andExpect(model().attribute("owner", hasProperty("address", is("110 W. Liberty St.")))) + .andExpect(model().attribute("owner", hasProperty("city", is("Madison")))) + .andExpect(model().attribute("owner", hasProperty("telephone", is("6085551023")))) + .andExpect(model().attribute("owner", hasProperty("pets", not(empty())))) + .andExpect(model().attribute("owner", hasProperty("pets", new BaseMatcher>() { - @Override - public boolean matches(Object item) { - @SuppressWarnings("unchecked") - List pets = (List) item; - PetDTO pet = pets.get(0); - if (pet.getVisits().isEmpty()) { - return false; - } - return true; - } + @Override + public boolean matches(Object item) { + @SuppressWarnings("unchecked") + List pets = (List) item; + PetDTO pet = pets.get(0); + if (pet.getVisits().isEmpty()) { + return false; + } - @Override - public void describeTo(Description description) { - description.appendText("Max did not have any visits"); - } - }))).andExpect(view().name("owners/ownerDetails")); + return true; + } + + @Override + public void describeTo(Description description) { + description.appendText("Max did not have any visits"); + } + }))) + .andExpect(view().name("owners/ownerDetails")); } } diff --git a/src/test/java/org/springframework/samples/petclinic/owner/PetTypeDTOFormatterTests.java b/src/test/java/org/springframework/samples/petclinic/owner/PetTypeDTOFormatterTests.java new file mode 100644 index 000000000..f66381753 --- /dev/null +++ b/src/test/java/org/springframework/samples/petclinic/owner/PetTypeDTOFormatterTests.java @@ -0,0 +1,97 @@ +/* + * Copyright 2012-2019 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; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.samples.petclinic.dto.PetTypeDTO; +import org.springframework.samples.petclinic.service.PetTypeService; + +import java.text.ParseException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Locale; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.BDDMockito.given; + +/** + * Test class for {@link PetTypeFormatter} + * + * @author Colin But + */ +@ExtendWith(MockitoExtension.class) +class PetTypeDTOFormatterTests { + + @Mock + private PetTypeService petTypeService; + + private PetTypeFormatter petTypeFormatter; + + @BeforeEach + void setup() { + this.petTypeFormatter = new PetTypeFormatter(petTypeService); + } + + @Test + void testPrint() { + PetTypeDTO petType = new PetTypeDTO(); + petType.setName("Hamster"); + String petTypeName = this.petTypeFormatter.print(petType, Locale.ENGLISH); + assertThat(petTypeName).isEqualTo("Hamster"); + } + + @Test + void shouldParse() throws ParseException { + given(this.petTypeService.findPetTypes()).willReturn(makePetTypes()); + PetTypeDTO petType = petTypeFormatter.parse("Bird", Locale.ENGLISH); + assertThat(petType.getName()).isEqualTo("Bird"); + } + + @Test + void shouldThrowParseException() throws ParseException { + given(this.petTypeService.findPetTypes()).willReturn(makePetTypes()); + Assertions.assertThrows(ParseException.class, () -> { + petTypeFormatter.parse("Fish", Locale.ENGLISH); + }); + } + + /** + * Helper method to produce some sample pet types just for test purpose + * @return {@link Collection} of {@link PetType} + */ + private List makePetTypes() { + List petTypes = new ArrayList<>(); + petTypes.add(new PetTypeDTO() { + { + setName("Dog"); + } + }); + petTypes.add(new PetTypeDTO() { + { + setName("Bird"); + } + }); + return petTypes; + } + +}