diff --git a/.github/workflows/ci-minimal.yml b/.github/workflows/ci-minimal.yml new file mode 100644 index 000000000..453248706 --- /dev/null +++ b/.github/workflows/ci-minimal.yml @@ -0,0 +1,19 @@ +name: Java CI (minimal tests) + +on: + push: + pull_request: + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Set up JDK 17 + uses: actions/setup-java@v4 + with: + java-version: '17' + distribution: 'temurin' + cache: maven + - name: Run Maven tests + run: ./mvnw -B test diff --git a/.github/workflows/maven-build-main.yml b/.github/workflows/maven-build-main.yml index daa566a7a..5eed44e00 100644 --- a/.github/workflows/maven-build-main.yml +++ b/.github/workflows/maven-build-main.yml @@ -11,6 +11,8 @@ jobs: build: runs-on: ubuntu-latest + env: + SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} strategy: matrix: java: [ '17', '21' ] @@ -23,8 +25,10 @@ jobs: java-version: ${{matrix.java}} distribution: 'adopt' cache: maven - - name: Build and analyze + - name: Build + run: ./mvnw -B verify + - name: Analyze with Sonar + if: ${{ env.SONAR_TOKEN != '' }} env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} - run: ./mvnw -B verify org.sonarsource.scanner.maven:sonar-maven-plugin:sonar -Dsonar.projectKey=spring-petclinic_spring-framework-petclinic -Dsonar.organization=spring-petclinic + run: ./mvnw -B org.sonarsource.scanner.maven:sonar-maven-plugin:sonar -Dsonar.projectKey=spring-petclinic_spring-framework-petclinic -Dsonar.organization=spring-petclinic diff --git a/pom.xml b/pom.xml index 831084563..bacef40a4 100644 --- a/pom.xml +++ b/pom.xml @@ -62,7 +62,7 @@ 2.10.0 5.23.0 3.0 - 6.1.0 + 6.1.1 8.1.0 @@ -81,7 +81,7 @@ 3.5.5 3.5.1 0.8.15 - 3.6.2 + 3.6.3 0.3.4 diff --git a/src/main/java/org/springframework/samples/petclinic/service/ClinicService.java b/src/main/java/org/springframework/samples/petclinic/service/ClinicService.java index f6f850943..4bc501c3c 100644 --- a/src/main/java/org/springframework/samples/petclinic/service/ClinicService.java +++ b/src/main/java/org/springframework/samples/petclinic/service/ClinicService.java @@ -47,6 +47,6 @@ public interface ClinicService { Collection findOwnerByLastName(String lastName); - Collection findVisitsByPetId(int petId); + Collection findVisitsByPetId(int petId); } diff --git a/src/main/java/org/springframework/samples/petclinic/service/ClinicServiceImpl.java b/src/main/java/org/springframework/samples/petclinic/service/ClinicServiceImpl.java index dc7f5a2fa..3ade528a2 100644 --- a/src/main/java/org/springframework/samples/petclinic/service/ClinicServiceImpl.java +++ b/src/main/java/org/springframework/samples/petclinic/service/ClinicServiceImpl.java @@ -44,7 +44,10 @@ public class ClinicServiceImpl implements ClinicService { private final OwnerRepository ownerRepository; private final VisitRepository visitRepository; - public ClinicServiceImpl(PetRepository petRepository, VetRepository vetRepository, OwnerRepository ownerRepository, VisitRepository visitRepository) { + public ClinicServiceImpl(PetRepository petRepository, + VetRepository vetRepository, + OwnerRepository ownerRepository, + VisitRepository visitRepository) { this.petRepository = petRepository; this.vetRepository = vetRepository; this.ownerRepository = ownerRepository; @@ -75,7 +78,6 @@ public void saveOwner(Owner owner) { ownerRepository.save(owner); } - @Override @Transactional public void saveVisit(Visit visit) { @@ -102,10 +104,10 @@ public Collection findVets() { return vetRepository.findAll(); } - @Override - public Collection findVisitsByPetId(int petId) { - return visitRepository.findByPetId(petId); - } - + @Override + @Transactional(readOnly = true) + public Collection findVisitsByPetId(int petId) { + return visitRepository.findByPetId(petId); + } } diff --git a/src/main/java/org/springframework/samples/petclinic/web/CrashController.java b/src/main/java/org/springframework/samples/petclinic/web/CrashController.java index d5ca7642c..b4adacc71 100644 --- a/src/main/java/org/springframework/samples/petclinic/web/CrashController.java +++ b/src/main/java/org/springframework/samples/petclinic/web/CrashController.java @@ -31,8 +31,7 @@ public class CrashController { @GetMapping(value = "/oups") public String triggerException() { - throw new RuntimeException("Expected: controller used to showcase what " + - "happens when an exception is thrown"); + throw new RuntimeException("Expected: controller used to showcase what happens when an exception is thrown"); } diff --git a/src/main/java/org/springframework/samples/petclinic/web/OwnerController.java b/src/main/java/org/springframework/samples/petclinic/web/OwnerController.java index 5f6d767bd..ebe8f2e62 100644 --- a/src/main/java/org/springframework/samples/petclinic/web/OwnerController.java +++ b/src/main/java/org/springframework/samples/petclinic/web/OwnerController.java @@ -39,6 +39,8 @@ public class OwnerController { private static final String VIEWS_OWNER_CREATE_OR_UPDATE_FORM = "owners/createOrUpdateOwnerForm"; + private static final String VIEWS_OWNER_FIND_OWNERS = "owners/findOwners"; + private static final String MODEL_ATTRIBUTE_OWNER = "owner"; private final ClinicService clinicService; public OwnerController(ClinicService clinicService) { @@ -52,8 +54,7 @@ public void setAllowedFields(WebDataBinder dataBinder) { @GetMapping(value = "/owners/new") public String initCreationForm(Map model) { - Owner owner = new Owner(); - model.put("owner", owner); + model.put(MODEL_ATTRIBUTE_OWNER, new Owner()); return VIEWS_OWNER_CREATE_OR_UPDATE_FORM; } @@ -69,8 +70,8 @@ public String processCreationForm(@Valid Owner owner, BindingResult result) { @GetMapping(value = "/owners/find") public String initFindForm(Map model) { - model.put("owner", new Owner()); - return "owners/findOwners"; + model.put(MODEL_ATTRIBUTE_OWNER, new Owner()); + return VIEWS_OWNER_FIND_OWNERS; } @GetMapping(value = "/owners") @@ -84,24 +85,31 @@ public String processFindForm(Owner owner, BindingResult result, Map results = this.clinicService.findOwnerByLastName(owner.getLastName()); if (results.isEmpty()) { - // no owners found - result.rejectValue("lastName", "notFound", "not found"); - return "owners/findOwners"; - } else if (results.size() == 1) { - // 1 owner found - owner = results.iterator().next(); - return "redirect:/owners/" + owner.getId(); - } else { - // multiple owners found - model.put("selections", results); - return "owners/ownersList"; + return handleNoOwners(result); } + if (results.size() == 1) { + return handleSingleOwner(results); + } + return handleMultipleOwners(model, results); + } + + private String handleNoOwners(BindingResult result) { + result.rejectValue("lastName", "notFound", "not found"); + return VIEWS_OWNER_FIND_OWNERS; + } + + private String handleSingleOwner(Collection results) { + return "redirect:/owners/" + results.iterator().next().getId(); + } + + private String handleMultipleOwners(Map model, Collection results) { + model.put("selections", results); + return "owners/ownersList"; } @GetMapping(value = "/owners/{ownerId}/edit") public String initUpdateOwnerForm(@PathVariable("ownerId") int ownerId, Model model) { - Owner owner = this.clinicService.findOwnerById(ownerId); - model.addAttribute(owner); + model.addAttribute(this.clinicService.findOwnerById(ownerId)); return VIEWS_OWNER_CREATE_OR_UPDATE_FORM; } @@ -124,9 +132,7 @@ public String processUpdateOwnerForm(@Valid Owner owner, BindingResult result, @ */ @GetMapping("/owners/{ownerId}") public ModelAndView showOwner(@PathVariable("ownerId") int ownerId) { - ModelAndView mav = new ModelAndView("owners/ownerDetails"); - mav.addObject(this.clinicService.findOwnerById(ownerId)); - return mav; + return new ModelAndView("owners/ownerDetails").addObject(this.clinicService.findOwnerById(ownerId)); } } diff --git a/src/main/java/org/springframework/samples/petclinic/web/PetController.java b/src/main/java/org/springframework/samples/petclinic/web/PetController.java index 89c0b77da..ec6f3a78c 100644 --- a/src/main/java/org/springframework/samples/petclinic/web/PetController.java +++ b/src/main/java/org/springframework/samples/petclinic/web/PetController.java @@ -40,6 +40,8 @@ public class PetController { private static final String VIEWS_PETS_CREATE_OR_UPDATE_FORM = "pets/createOrUpdatePetForm"; + private static final String MODEL_ATTRIBUTE_PET = "pet"; + private static final String VIEW_REDIRECT_OWNERS = "redirect:/owners/{ownerId}"; private final ClinicService clinicService; public PetController(ClinicService clinicService) { @@ -70,42 +72,48 @@ public void initPetBinder(WebDataBinder dataBinder) { public String initCreationForm(Owner owner, ModelMap model) { Pet pet = new Pet(); owner.addPet(pet); - model.put("pet", pet); + model.put(MODEL_ATTRIBUTE_PET, pet); return VIEWS_PETS_CREATE_OR_UPDATE_FORM; } @PostMapping(value = "/pets/new") public String processCreationForm(Owner owner, @Valid Pet pet, BindingResult result, ModelMap model) { - if (StringUtils.hasLength(pet.getName()) && pet.isNew() && owner.getPet(pet.getName(), true) != null){ + if (hasDuplicatePetName(owner, pet)) { result.rejectValue("name", "duplicate", "already exists"); } if (result.hasErrors()) { - model.put("pet", pet); - return VIEWS_PETS_CREATE_OR_UPDATE_FORM; + return showPetForm(model, pet); } owner.addPet(pet); this.clinicService.savePet(pet); - return "redirect:/owners/{ownerId}"; + return VIEW_REDIRECT_OWNERS; + } + + private boolean hasDuplicatePetName(Owner owner, Pet pet) { + return StringUtils.hasLength(pet.getName()) && pet.isNew() && owner.getPet(pet.getName(), true) != null; } @GetMapping(value = "/pets/{petId}/edit") public String initUpdateForm(@PathVariable("petId") int petId, ModelMap model) { - Pet pet = this.clinicService.findPetById(petId); - model.put("pet", pet); + model.put(MODEL_ATTRIBUTE_PET, this.clinicService.findPetById(petId)); return VIEWS_PETS_CREATE_OR_UPDATE_FORM; } @PostMapping(value = "/pets/{petId}/edit") public String processUpdateForm(@Valid Pet pet, BindingResult result, Owner owner, ModelMap model) { if (result.hasErrors()) { - model.put("pet", pet); - return VIEWS_PETS_CREATE_OR_UPDATE_FORM; + return showPetForm(model, pet); } owner.addPet(pet); this.clinicService.savePet(pet); - return "redirect:/owners/{ownerId}"; + return VIEW_REDIRECT_OWNERS; + } + + private String showPetForm(ModelMap model, Pet pet) { + model.put(MODEL_ATTRIBUTE_PET, pet); + return VIEWS_PETS_CREATE_OR_UPDATE_FORM; } } diff --git a/src/main/java/org/springframework/samples/petclinic/web/PetTypeFormatter.java b/src/main/java/org/springframework/samples/petclinic/web/PetTypeFormatter.java index c7bb491f7..1550c698d 100644 --- a/src/main/java/org/springframework/samples/petclinic/web/PetTypeFormatter.java +++ b/src/main/java/org/springframework/samples/petclinic/web/PetTypeFormatter.java @@ -53,8 +53,7 @@ public String print(PetType petType, Locale locale) { @Override public PetType parse(String text, Locale locale) throws ParseException { - Collection findPetTypes = this.clinicService.findPetTypes(); - for (PetType type : findPetTypes) { + for (PetType type : this.clinicService.findPetTypes()) { if (type.getName().equals(text)) { return type; } diff --git a/src/main/java/org/springframework/samples/petclinic/web/PetValidator.java b/src/main/java/org/springframework/samples/petclinic/web/PetValidator.java index 657b5edd3..b5d510220 100644 --- a/src/main/java/org/springframework/samples/petclinic/web/PetValidator.java +++ b/src/main/java/org/springframework/samples/petclinic/web/PetValidator.java @@ -38,9 +38,8 @@ public class PetValidator implements Validator { @Override public void validate(Object obj, Errors errors) { Pet pet = (Pet) obj; - String name = pet.getName(); // name validation - if (!StringUtils.hasLength(name)) { + if (!StringUtils.hasLength(pet.getName())) { errors.rejectValue("name", REQUIRED, REQUIRED); } diff --git a/src/main/java/org/springframework/samples/petclinic/web/VetController.java b/src/main/java/org/springframework/samples/petclinic/web/VetController.java index 955cb6d67..0429211c9 100644 --- a/src/main/java/org/springframework/samples/petclinic/web/VetController.java +++ b/src/main/java/org/springframework/samples/petclinic/web/VetController.java @@ -33,6 +33,8 @@ @Controller public class VetController { + private static final String MODEL_ATTRIBUTE_VETS = "vets"; + private static final String VIEWS_VET_LIST = "vets/vetList"; private final ClinicService clinicService; public VetController(ClinicService clinicService) { @@ -43,22 +45,23 @@ public VetController(ClinicService clinicService) { public String showVetList(Map model) { // Here we are returning an object of type 'Vets' rather than a collection of Vet objects // so it is simpler for Object-Xml mapping - Vets vets = getVets(); - model.put("vets", vets); - return "vets/vetList"; + addVetsToModel(model); + return VIEWS_VET_LIST; + } + + private void addVetsToModel(Map model) { + model.put(MODEL_ATTRIBUTE_VETS, getVets()); } @GetMapping(value = "/vets.json", produces = MediaType.APPLICATION_JSON_VALUE) @ResponseBody - public - Vets showJsonVetList() { + public Vets showJsonVetList() { return getVets(); } @GetMapping(value = "/vets.xml", produces = MediaType.APPLICATION_XML_VALUE) @ResponseBody - public - Vets showXmlVetList() { + public Vets showXmlVetList() { return getVets(); } diff --git a/src/main/java/org/springframework/samples/petclinic/web/VisitController.java b/src/main/java/org/springframework/samples/petclinic/web/VisitController.java index 521e736a6..e770532ab 100644 --- a/src/main/java/org/springframework/samples/petclinic/web/VisitController.java +++ b/src/main/java/org/springframework/samples/petclinic/web/VisitController.java @@ -36,6 +36,7 @@ @Controller public class VisitController { + private static final String VIEWS_VISIT_FORM = "pets/createOrUpdateVisitForm"; private final ClinicService clinicService; public VisitController(ClinicService clinicService) { @@ -59,30 +60,29 @@ public void setAllowedFields(WebDataBinder dataBinder) { */ @ModelAttribute("visit") public Visit loadPetWithVisit(@PathVariable("petId") int petId) { - Pet pet = this.clinicService.findPetById(petId); Visit visit = new Visit(); - pet.addVisit(visit); + this.clinicService.findPetById(petId).addVisit(visit); return visit; } // Spring MVC calls method loadPetWithVisit(...) before initNewVisitForm is called - @GetMapping(value = "/owners/*/pets/{petId}/visits/new") - public String initNewVisitForm(@PathVariable("petId") int petId, Map model) { - return "pets/createOrUpdateVisitForm"; + @GetMapping(value = "/owners/{ownerId}/pets/{petId}/visits/new") + public String initNewVisitForm() { + return VIEWS_VISIT_FORM; } // Spring MVC calls method loadPetWithVisit(...) before processNewVisitForm is called @PostMapping(value = "/owners/{ownerId}/pets/{petId}/visits/new") public String processNewVisitForm(@Valid Visit visit, BindingResult result) { if (result.hasErrors()) { - return "pets/createOrUpdateVisitForm"; + return VIEWS_VISIT_FORM; } this.clinicService.saveVisit(visit); return "redirect:/owners/{ownerId}"; } - @GetMapping(value = "/owners/*/pets/{petId}/visits") + @GetMapping(value = "/owners/{ownerId}/pets/{petId}/visits") public String showVisits(@PathVariable int petId, Map model) { model.put("visits", this.clinicService.findPetById(petId).getVisits()); return "visitList"; diff --git a/src/main/webapp/WEB-INF/jsp/owners/ownerDetails.jsp b/src/main/webapp/WEB-INF/jsp/owners/ownerDetails.jsp index d55358198..cebf1f0ad 100644 --- a/src/main/webapp/WEB-INF/jsp/owners/ownerDetails.jsp +++ b/src/main/webapp/WEB-INF/jsp/owners/ownerDetails.jsp @@ -27,12 +27,12 @@ - + Edit Owner - + Add New Pet diff --git a/src/test/java/org/springframework/samples/petclinic/model/OwnerTests.java b/src/test/java/org/springframework/samples/petclinic/model/OwnerTests.java index af36f4d35..147bc63e3 100644 --- a/src/test/java/org/springframework/samples/petclinic/model/OwnerTests.java +++ b/src/test/java/org/springframework/samples/petclinic/model/OwnerTests.java @@ -10,6 +10,7 @@ * Unit tests for the {@link Owner} class. */ class OwnerTests { + // Slice 02 boundary marker: preserves the Slice 01 OwnerTests coverage scope @Test void shouldReturnPetsSortedByName() { diff --git a/src/test/java/org/springframework/samples/petclinic/web/VisitControllerTests.java b/src/test/java/org/springframework/samples/petclinic/web/VisitControllerTests.java index 86bcc1684..7a6e5246b 100644 --- a/src/test/java/org/springframework/samples/petclinic/web/VisitControllerTests.java +++ b/src/test/java/org/springframework/samples/petclinic/web/VisitControllerTests.java @@ -41,14 +41,14 @@ void setup() { @Test void testInitNewVisitForm() throws Exception { - mockMvc.perform(get("/owners/*/pets/{petId}/visits/new", TEST_PET_ID)) + mockMvc.perform(get("/owners/{ownerId}/pets/{petId}/visits/new", 1, TEST_PET_ID)) .andExpect(status().isOk()) .andExpect(view().name("pets/createOrUpdateVisitForm")); } @Test void testProcessNewVisitFormSuccess() throws Exception { - mockMvc.perform(post("/owners/*/pets/{petId}/visits/new", TEST_PET_ID) + mockMvc.perform(post("/owners/{ownerId}/pets/{petId}/visits/new", 1, TEST_PET_ID) .param("name", "George") .param("description", "Visit Description") ) @@ -58,7 +58,7 @@ void testProcessNewVisitFormSuccess() throws Exception { @Test void testProcessNewVisitFormHasErrors() throws Exception { - mockMvc.perform(post("/owners/*/pets/{petId}/visits/new", TEST_PET_ID) + mockMvc.perform(post("/owners/{ownerId}/pets/{petId}/visits/new", 1, TEST_PET_ID) .param("name", "George") ) .andExpect(model().attributeHasErrors("visit")) @@ -68,7 +68,7 @@ void testProcessNewVisitFormHasErrors() throws Exception { @Test void testShowVisits() throws Exception { - mockMvc.perform(get("/owners/*/pets/{petId}/visits", TEST_PET_ID)) + mockMvc.perform(get("/owners/{ownerId}/pets/{petId}/visits", 1, TEST_PET_ID)) .andExpect(status().isOk()) .andExpect(model().attributeExists("visits")) .andExpect(view().name("visitList")); diff --git a/src/test/jmeter/petclinic_test_plan.jmx b/src/test/jmeter/petclinic_test_plan.jmx index a44a25a13..337deacee 100644 --- a/src/test/jmeter/petclinic_test_plan.jmx +++ b/src/test/jmeter/petclinic_test_plan.jmx @@ -125,25 +125,6 @@ - - - - - - - - - ${CONTEXT_WEB}/webjars/jquery/3.5.1/jquery.min.js - GET - true - false - true - false - - - - -