Skip to content

Commit b559077

Browse files
author
Dave Syer
committed
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.
1 parent 43beff9 commit b559077

File tree

10 files changed

+25
-60
lines changed

10 files changed

+25
-60
lines changed

src/main/java/org/springframework/samples/petclinic/owner/Owner.java

Lines changed: 10 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,19 @@
1616
package org.springframework.samples.petclinic.owner;
1717

1818
import java.util.ArrayList;
19-
import java.util.Collections;
20-
import java.util.HashSet;
2119
import java.util.List;
22-
import java.util.Set;
2320

2421
import javax.persistence.CascadeType;
2522
import javax.persistence.Column;
2623
import javax.persistence.Entity;
2724
import javax.persistence.FetchType;
25+
import javax.persistence.JoinColumn;
2826
import javax.persistence.OneToMany;
27+
import javax.persistence.OrderBy;
2928
import javax.persistence.Table;
3029
import javax.validation.constraints.Digits;
3130
import javax.validation.constraints.NotEmpty;
3231

33-
import org.springframework.beans.support.MutableSortDefinition;
34-
import org.springframework.beans.support.PropertyComparator;
3532
import org.springframework.core.style.ToStringCreator;
3633
import org.springframework.samples.petclinic.model.Person;
3734

@@ -60,8 +57,10 @@ public class Owner extends Person {
6057
@Digits(fraction = 0, integer = 10)
6158
private String telephone;
6259

63-
@OneToMany(cascade = CascadeType.ALL, mappedBy = "ownerId", fetch = FetchType.EAGER)
64-
private Set<Pet> pets;
60+
@OneToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER)
61+
@JoinColumn(name = "owner_id")
62+
@OrderBy("name")
63+
private List<Pet> pets = new ArrayList<>();
6564

6665
public String getAddress() {
6766
return this.address;
@@ -87,28 +86,14 @@ public void setTelephone(String telephone) {
8786
this.telephone = telephone;
8887
}
8988

90-
protected Set<Pet> getPetsInternal() {
91-
if (this.pets == null) {
92-
this.pets = new HashSet<>();
93-
}
94-
return this.pets;
95-
}
96-
97-
protected void setPetsInternal(Set<Pet> pets) {
98-
this.pets = pets;
99-
}
100-
10189
public List<Pet> getPets() {
102-
List<Pet> sortedPets = new ArrayList<>(getPetsInternal());
103-
PropertyComparator.sort(sortedPets, new MutableSortDefinition("name", true, true));
104-
return Collections.unmodifiableList(sortedPets);
90+
return this.pets;
10591
}
10692

10793
public void addPet(Pet pet) {
10894
if (pet.isNew()) {
109-
getPetsInternal().add(pet);
95+
getPets().add(pet);
11096
}
111-
pet.setOwnerId(getId());
11297
}
11398

11499
/**
@@ -126,7 +111,7 @@ public Pet getPet(String name) {
126111
* @return a pet if pet id is already in use
127112
*/
128113
public Pet getPet(Integer id) {
129-
for (Pet pet : getPetsInternal()) {
114+
for (Pet pet : getPets()) {
130115
if (!pet.isNew()) {
131116
Integer compId = pet.getId();
132117
if (compId.equals(id)) {
@@ -144,7 +129,7 @@ public Pet getPet(Integer id) {
144129
*/
145130
public Pet getPet(String name, boolean ignoreNew) {
146131
name = name.toLowerCase();
147-
for (Pet pet : getPetsInternal()) {
132+
for (Pet pet : getPets()) {
148133
if (!ignoreNew || !pet.isNew()) {
149134
String compName = pet.getName();
150135
compName = compName == null ? "" : compName.toLowerCase();

src/main/java/org/springframework/samples/petclinic/owner/Pet.java

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,17 +52,11 @@ public class Pet extends NamedEntity {
5252
@JoinColumn(name = "type_id")
5353
private PetType type;
5454

55-
@Column
56-
private Integer ownerId;
57-
58-
@OneToMany(cascade = CascadeType.ALL, mappedBy = "petId", fetch = FetchType.LAZY)
55+
@OneToMany(cascade = CascadeType.ALL, fetch = FetchType.LAZY)
56+
@JoinColumn(name = "pet_id")
5957
@OrderBy("visit_date ASC")
6058
private Set<Visit> visits = new LinkedHashSet<>();
6159

62-
public void setOwnerId(Integer ownerId) {
63-
this.ownerId = ownerId;
64-
}
65-
6660
public void setBirthDate(LocalDate birthDate) {
6761
this.birthDate = birthDate;
6862
}
@@ -85,7 +79,6 @@ public Collection<Visit> getVisits() {
8579

8680
public void addVisit(Visit visit) {
8781
getVisits().add(visit);
88-
visit.setPetId(this.getId());
8982
}
9083

9184
}

src/main/java/org/springframework/samples/petclinic/owner/Visit.java

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,8 @@ public class Visit extends BaseEntity {
4040
private LocalDate date;
4141

4242
@NotEmpty
43-
@Column
4443
private String description;
4544

46-
@Column
47-
private Integer petId;
48-
4945
/**
5046
* Creates a new instance of Visit for the current date
5147
*/
@@ -69,12 +65,4 @@ public void setDescription(String description) {
6965
this.description = description;
7066
}
7167

72-
public Integer getPetId() {
73-
return this.petId;
74-
}
75-
76-
public void setPetId(Integer petId) {
77-
this.petId = petId;
78-
}
79-
8068
}

src/main/java/org/springframework/samples/petclinic/owner/VisitController.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,13 @@ public String initNewVisitForm(@PathVariable("petId") int petId, Map<String, Obj
7878
// Spring MVC calls method loadPetWithVisit(...) before processNewVisitForm is
7979
// called
8080
@PostMapping("/owners/{ownerId}/pets/{petId}/visits/new")
81-
public String processNewVisitForm(@ModelAttribute Owner owner, @Valid Visit visit, BindingResult result) {
81+
public String processNewVisitForm(@ModelAttribute Owner owner, @PathVariable("petId") int petId, @Valid Visit visit,
82+
BindingResult result) {
8283
if (result.hasErrors()) {
8384
return "pets/createOrUpdateVisitForm";
8485
}
8586
else {
86-
owner.getPet(visit.getPetId()).addVisit(visit);
87+
owner.getPet(petId).addVisit(visit);
8788
this.owners.save(owner);
8889
return "redirect:/owners/{ownerId}";
8990
}

src/main/resources/db/h2/schema.sql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,15 @@ CREATE TABLE pets (
4848
name VARCHAR(30),
4949
birth_date DATE,
5050
type_id INTEGER NOT NULL,
51-
owner_id INTEGER NOT NULL
51+
owner_id INTEGER
5252
);
5353
ALTER TABLE pets ADD CONSTRAINT fk_pets_owners FOREIGN KEY (owner_id) REFERENCES owners (id);
5454
ALTER TABLE pets ADD CONSTRAINT fk_pets_types FOREIGN KEY (type_id) REFERENCES types (id);
5555
CREATE INDEX pets_name ON pets (name);
5656

5757
CREATE TABLE visits (
5858
id INTEGER IDENTITY PRIMARY KEY,
59-
pet_id INTEGER NOT NULL,
59+
pet_id INTEGER,
6060
visit_date DATE,
6161
description VARCHAR(255)
6262
);

src/main/resources/db/hsqldb/schema.sql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,15 @@ CREATE TABLE pets (
4848
name VARCHAR(30),
4949
birth_date DATE,
5050
type_id INTEGER NOT NULL,
51-
owner_id INTEGER NOT NULL
51+
owner_id INTEGER
5252
);
5353
ALTER TABLE pets ADD CONSTRAINT fk_pets_owners FOREIGN KEY (owner_id) REFERENCES owners (id);
5454
ALTER TABLE pets ADD CONSTRAINT fk_pets_types FOREIGN KEY (type_id) REFERENCES types (id);
5555
CREATE INDEX pets_name ON pets (name);
5656

5757
CREATE TABLE visits (
5858
id INTEGER IDENTITY PRIMARY KEY,
59-
pet_id INTEGER NOT NULL,
59+
pet_id INTEGER,
6060
visit_date DATE,
6161
description VARCHAR(255)
6262
);

src/main/resources/db/mysql/schema.sql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,15 @@ CREATE TABLE IF NOT EXISTS pets (
4040
name VARCHAR(30),
4141
birth_date DATE,
4242
type_id INT(4) UNSIGNED NOT NULL,
43-
owner_id INT(4) UNSIGNED NOT NULL,
43+
owner_id INT(4) UNSIGNED,
4444
INDEX(name),
4545
FOREIGN KEY (owner_id) REFERENCES owners(id),
4646
FOREIGN KEY (type_id) REFERENCES types(id)
4747
) engine=InnoDB;
4848

4949
CREATE TABLE IF NOT EXISTS visits (
5050
id INT(4) UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
51-
pet_id INT(4) UNSIGNED NOT NULL,
51+
pet_id INT(4) UNSIGNED,
5252
visit_date DATE,
5353
description VARCHAR(255),
5454
FOREIGN KEY (pet_id) REFERENCES pets(id)

src/main/resources/db/postgres/schema.sql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,14 @@ CREATE TABLE IF NOT EXISTS pets (
3838
name TEXT,
3939
birth_date DATE,
4040
type_id INT NOT NULL REFERENCES types (id),
41-
owner_id INT NOT NULL REFERENCES owners (id)
41+
owner_id INT REFERENCES owners (id)
4242
);
4343
CREATE INDEX ON pets (name);
4444
CREATE INDEX ON pets (owner_id);
4545

4646
CREATE TABLE IF NOT EXISTS visits (
4747
id INT GENERATED BY DEFAULT AS IDENTITY PRIMARY KEY,
48-
pet_id INT NOT NULL REFERENCES pets (id),
48+
pet_id INT REFERENCES pets (id),
4949
visit_date DATE,
5050
description TEXT
5151
);

src/test/java/org/springframework/samples/petclinic/owner/OwnerControllerTests.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,11 @@ private Owner george() {
7575
Pet max = new Pet();
7676
PetType dog = new PetType();
7777
dog.setName("dog");
78-
max.setId(1);
7978
max.setType(dog);
8079
max.setName("Max");
8180
max.setBirthDate(LocalDate.now());
82-
george.setPetsInternal(Collections.singleton(max));
81+
george.addPet(max);
82+
max.setId(1);
8383
return george;
8484
};
8585

@@ -95,7 +95,6 @@ void setup() {
9595
given(this.owners.findById(TEST_OWNER_ID)).willReturn(george);
9696
Visit visit = new Visit();
9797
visit.setDate(LocalDate.now());
98-
visit.setPetId(george.getPet("Max").getId());
9998
george.getPet("Max").getVisits().add(visit);
10099

101100
}

src/test/java/org/springframework/samples/petclinic/service/ClinicServiceTests.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,6 @@ void shouldFindVisitsByPetId() throws Exception {
216216
assertThat(visits).hasSize(2);
217217
Visit[] visitArr = visits.toArray(new Visit[visits.size()]);
218218
assertThat(visitArr[0].getDate()).isNotNull();
219-
assertThat(visitArr[0].getPetId()).isEqualTo(7);
220219
}
221220

222221
}

0 commit comments

Comments
 (0)