From b559077f14b6f1e197228ff6cb7395514f09326f Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Thu, 6 Jan 2022 11:18:15 +0000 Subject: [PATCH] Remove manual id management in child entities This is reverting a workaround for a Hibernate "feature". There's no need for the child entities (Pet and Visit) to know about their parent (foreign key). Hibernate can manage that just fine with a @JoinColumn. But it needs a nullable foreign key column in the DB schema. That's the downside. The upside is much less code in Java. --- .../samples/petclinic/owner/Owner.java | 35 ++++++------------- .../samples/petclinic/owner/Pet.java | 11 ++---- .../samples/petclinic/owner/Visit.java | 12 ------- .../petclinic/owner/VisitController.java | 5 +-- src/main/resources/db/h2/schema.sql | 4 +-- src/main/resources/db/hsqldb/schema.sql | 4 +-- src/main/resources/db/mysql/schema.sql | 4 +-- src/main/resources/db/postgres/schema.sql | 4 +-- .../petclinic/owner/OwnerControllerTests.java | 5 ++- .../petclinic/service/ClinicServiceTests.java | 1 - 10 files changed, 25 insertions(+), 60 deletions(-) diff --git a/src/main/java/org/springframework/samples/petclinic/owner/Owner.java b/src/main/java/org/springframework/samples/petclinic/owner/Owner.java index 4b2104690..a38dfea28 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/Owner.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/Owner.java @@ -16,22 +16,19 @@ package org.springframework.samples.petclinic.owner; import java.util.ArrayList; -import java.util.Collections; -import java.util.HashSet; import java.util.List; -import java.util.Set; import javax.persistence.CascadeType; import javax.persistence.Column; import javax.persistence.Entity; import javax.persistence.FetchType; +import javax.persistence.JoinColumn; import javax.persistence.OneToMany; +import javax.persistence.OrderBy; import javax.persistence.Table; import javax.validation.constraints.Digits; import javax.validation.constraints.NotEmpty; -import org.springframework.beans.support.MutableSortDefinition; -import org.springframework.beans.support.PropertyComparator; import org.springframework.core.style.ToStringCreator; import org.springframework.samples.petclinic.model.Person; @@ -60,8 +57,10 @@ public class Owner extends Person { @Digits(fraction = 0, integer = 10) private String telephone; - @OneToMany(cascade = CascadeType.ALL, mappedBy = "ownerId", fetch = FetchType.EAGER) - private Set pets; + @OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER) + @JoinColumn(name = "owner_id") + @OrderBy("name") + private List pets = new ArrayList<>(); public String getAddress() { return this.address; @@ -87,28 +86,14 @@ public class Owner extends Person { this.telephone = telephone; } - protected Set getPetsInternal() { - if (this.pets == null) { - this.pets = new HashSet<>(); - } - return this.pets; - } - - protected void setPetsInternal(Set pets) { - this.pets = pets; - } - public List getPets() { - List sortedPets = new ArrayList<>(getPetsInternal()); - PropertyComparator.sort(sortedPets, new MutableSortDefinition("name", true, true)); - return Collections.unmodifiableList(sortedPets); + return this.pets; } public void addPet(Pet pet) { if (pet.isNew()) { - getPetsInternal().add(pet); + getPets().add(pet); } - pet.setOwnerId(getId()); } /** @@ -126,7 +111,7 @@ public class Owner extends Person { * @return a pet if pet id is already in use */ public Pet getPet(Integer id) { - for (Pet pet : getPetsInternal()) { + for (Pet pet : getPets()) { if (!pet.isNew()) { Integer compId = pet.getId(); if (compId.equals(id)) { @@ -144,7 +129,7 @@ public class Owner extends Person { */ public Pet getPet(String name, boolean ignoreNew) { name = name.toLowerCase(); - for (Pet pet : getPetsInternal()) { + for (Pet pet : getPets()) { if (!ignoreNew || !pet.isNew()) { String compName = pet.getName(); compName = compName == null ? "" : compName.toLowerCase(); diff --git a/src/main/java/org/springframework/samples/petclinic/owner/Pet.java b/src/main/java/org/springframework/samples/petclinic/owner/Pet.java index 8fa1bd6eb..3c389b0ee 100755 --- a/src/main/java/org/springframework/samples/petclinic/owner/Pet.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/Pet.java @@ -52,17 +52,11 @@ public class Pet extends NamedEntity { @JoinColumn(name = "type_id") private PetType type; - @Column - private Integer ownerId; - - @OneToMany(cascade = CascadeType.ALL, mappedBy = "petId", fetch = FetchType.LAZY) + @OneToMany(cascade = CascadeType.ALL, fetch = FetchType.LAZY) + @JoinColumn(name = "pet_id") @OrderBy("visit_date ASC") private Set visits = new LinkedHashSet<>(); - public void setOwnerId(Integer ownerId) { - this.ownerId = ownerId; - } - public void setBirthDate(LocalDate birthDate) { this.birthDate = birthDate; } @@ -85,7 +79,6 @@ public class Pet extends NamedEntity { public void addVisit(Visit visit) { getVisits().add(visit); - visit.setPetId(this.getId()); } } diff --git a/src/main/java/org/springframework/samples/petclinic/owner/Visit.java b/src/main/java/org/springframework/samples/petclinic/owner/Visit.java index c09aa810e..8d9116533 100755 --- a/src/main/java/org/springframework/samples/petclinic/owner/Visit.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/Visit.java @@ -40,12 +40,8 @@ public class Visit extends BaseEntity { private LocalDate date; @NotEmpty - @Column private String description; - @Column - private Integer petId; - /** * Creates a new instance of Visit for the current date */ @@ -69,12 +65,4 @@ public class Visit extends BaseEntity { this.description = description; } - public Integer getPetId() { - return this.petId; - } - - public void setPetId(Integer petId) { - this.petId = petId; - } - } diff --git a/src/main/java/org/springframework/samples/petclinic/owner/VisitController.java b/src/main/java/org/springframework/samples/petclinic/owner/VisitController.java index 6fcae57da..bbd6efc92 100644 --- a/src/main/java/org/springframework/samples/petclinic/owner/VisitController.java +++ b/src/main/java/org/springframework/samples/petclinic/owner/VisitController.java @@ -78,12 +78,13 @@ class VisitController { // Spring MVC calls method loadPetWithVisit(...) before processNewVisitForm is // called @PostMapping("/owners/{ownerId}/pets/{petId}/visits/new") - public String processNewVisitForm(@ModelAttribute Owner owner, @Valid Visit visit, BindingResult result) { + public String processNewVisitForm(@ModelAttribute Owner owner, @PathVariable("petId") int petId, @Valid Visit visit, + BindingResult result) { if (result.hasErrors()) { return "pets/createOrUpdateVisitForm"; } else { - owner.getPet(visit.getPetId()).addVisit(visit); + owner.getPet(petId).addVisit(visit); this.owners.save(owner); return "redirect:/owners/{ownerId}"; } diff --git a/src/main/resources/db/h2/schema.sql b/src/main/resources/db/h2/schema.sql index f3c6947b7..5d6760a4b 100644 --- a/src/main/resources/db/h2/schema.sql +++ b/src/main/resources/db/h2/schema.sql @@ -48,7 +48,7 @@ CREATE TABLE pets ( name VARCHAR(30), birth_date DATE, type_id INTEGER NOT NULL, - owner_id INTEGER NOT NULL + owner_id INTEGER ); ALTER TABLE pets ADD CONSTRAINT fk_pets_owners FOREIGN KEY (owner_id) REFERENCES owners (id); ALTER TABLE pets ADD CONSTRAINT fk_pets_types FOREIGN KEY (type_id) REFERENCES types (id); @@ -56,7 +56,7 @@ CREATE INDEX pets_name ON pets (name); CREATE TABLE visits ( id INTEGER IDENTITY PRIMARY KEY, - pet_id INTEGER NOT NULL, + pet_id INTEGER, visit_date DATE, description VARCHAR(255) ); diff --git a/src/main/resources/db/hsqldb/schema.sql b/src/main/resources/db/hsqldb/schema.sql index f3c6947b7..5d6760a4b 100644 --- a/src/main/resources/db/hsqldb/schema.sql +++ b/src/main/resources/db/hsqldb/schema.sql @@ -48,7 +48,7 @@ CREATE TABLE pets ( name VARCHAR(30), birth_date DATE, type_id INTEGER NOT NULL, - owner_id INTEGER NOT NULL + owner_id INTEGER ); ALTER TABLE pets ADD CONSTRAINT fk_pets_owners FOREIGN KEY (owner_id) REFERENCES owners (id); ALTER TABLE pets ADD CONSTRAINT fk_pets_types FOREIGN KEY (type_id) REFERENCES types (id); @@ -56,7 +56,7 @@ CREATE INDEX pets_name ON pets (name); CREATE TABLE visits ( id INTEGER IDENTITY PRIMARY KEY, - pet_id INTEGER NOT NULL, + pet_id INTEGER, visit_date DATE, description VARCHAR(255) ); diff --git a/src/main/resources/db/mysql/schema.sql b/src/main/resources/db/mysql/schema.sql index eb5d7d5d0..2591a516d 100644 --- a/src/main/resources/db/mysql/schema.sql +++ b/src/main/resources/db/mysql/schema.sql @@ -40,7 +40,7 @@ CREATE TABLE IF NOT EXISTS pets ( name VARCHAR(30), birth_date DATE, type_id INT(4) UNSIGNED NOT NULL, - owner_id INT(4) UNSIGNED NOT NULL, + owner_id INT(4) UNSIGNED, INDEX(name), FOREIGN KEY (owner_id) REFERENCES owners(id), FOREIGN KEY (type_id) REFERENCES types(id) @@ -48,7 +48,7 @@ CREATE TABLE IF NOT EXISTS pets ( CREATE TABLE IF NOT EXISTS visits ( id INT(4) UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, - pet_id INT(4) UNSIGNED NOT NULL, + pet_id INT(4) UNSIGNED, visit_date DATE, description VARCHAR(255), FOREIGN KEY (pet_id) REFERENCES pets(id) diff --git a/src/main/resources/db/postgres/schema.sql b/src/main/resources/db/postgres/schema.sql index 9140a79cf..1bd582dc2 100644 --- a/src/main/resources/db/postgres/schema.sql +++ b/src/main/resources/db/postgres/schema.sql @@ -38,14 +38,14 @@ CREATE TABLE IF NOT EXISTS pets ( name TEXT, birth_date DATE, type_id INT NOT NULL REFERENCES types (id), - owner_id INT NOT NULL REFERENCES owners (id) + owner_id INT REFERENCES owners (id) ); CREATE INDEX ON pets (name); CREATE INDEX ON pets (owner_id); CREATE TABLE IF NOT EXISTS visits ( id INT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY, - pet_id INT NOT NULL REFERENCES pets (id), + pet_id INT REFERENCES pets (id), visit_date DATE, description TEXT ); diff --git a/src/test/java/org/springframework/samples/petclinic/owner/OwnerControllerTests.java b/src/test/java/org/springframework/samples/petclinic/owner/OwnerControllerTests.java index 56c65b89a..c93d03321 100644 --- a/src/test/java/org/springframework/samples/petclinic/owner/OwnerControllerTests.java +++ b/src/test/java/org/springframework/samples/petclinic/owner/OwnerControllerTests.java @@ -75,11 +75,11 @@ class OwnerControllerTests { Pet max = new Pet(); PetType dog = new PetType(); dog.setName("dog"); - max.setId(1); max.setType(dog); max.setName("Max"); max.setBirthDate(LocalDate.now()); - george.setPetsInternal(Collections.singleton(max)); + george.addPet(max); + max.setId(1); return george; }; @@ -95,7 +95,6 @@ class OwnerControllerTests { given(this.owners.findById(TEST_OWNER_ID)).willReturn(george); Visit visit = new Visit(); visit.setDate(LocalDate.now()); - visit.setPetId(george.getPet("Max").getId()); george.getPet("Max").getVisits().add(visit); } diff --git a/src/test/java/org/springframework/samples/petclinic/service/ClinicServiceTests.java b/src/test/java/org/springframework/samples/petclinic/service/ClinicServiceTests.java index 870daa184..a5c0b2952 100644 --- a/src/test/java/org/springframework/samples/petclinic/service/ClinicServiceTests.java +++ b/src/test/java/org/springframework/samples/petclinic/service/ClinicServiceTests.java @@ -216,7 +216,6 @@ class ClinicServiceTests { assertThat(visits).hasSize(2); Visit[] visitArr = visits.toArray(new Visit[visits.size()]); assertThat(visitArr[0].getDate()).isNotNull(); - assertThat(visitArr[0].getPetId()).isEqualTo(7); } }