From f44e383462444a51ce5cb632db4554aedf2c85ee Mon Sep 17 00:00:00 2001
From: Mic <misvy@vmware.com>
Date: Wed, 30 Jan 2013 11:20:22 +0800
Subject: [PATCH] fixed lazy loading issues

also renamed some local variables in Unit tests
---
 .../samples/petclinic/Pet.java                |  3 +-
 .../jdbc/JdbcOwnerRepositoryImpl.java         |  6 +--
 .../jpa/JpaOwnerRepositoryImpl.java           | 11 +++-
 .../repository/jpa/JpaVetRepositoryImpl.java  |  2 +-
 .../AbstractOwnerRepositoryTests.java         | 21 +++-----
 .../petclinic/AbstractPetRepositoryTests.java | 50 +++++++++----------
 .../AbstractVisitRepositoryTests.java         | 13 +++--
 7 files changed, 52 insertions(+), 54 deletions(-)

diff --git a/src/main/java/org/springframework/samples/petclinic/Pet.java b/src/main/java/org/springframework/samples/petclinic/Pet.java
index 7350011..6235ba9 100644
--- a/src/main/java/org/springframework/samples/petclinic/Pet.java
+++ b/src/main/java/org/springframework/samples/petclinic/Pet.java
@@ -9,6 +9,7 @@ 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.ManyToOne;
 import javax.persistence.OneToMany;
@@ -41,7 +42,7 @@ public class Pet extends NamedEntity {
     @JoinColumn(name = "owner_id")
 	private Owner owner;
 	
-	@OneToMany(cascade=CascadeType.ALL, mappedBy="pet")
+	@OneToMany(cascade=CascadeType.ALL, mappedBy="pet", fetch=FetchType.EAGER)
 	private Set<Visit> visits;
 
 
diff --git a/src/main/java/org/springframework/samples/petclinic/repository/jdbc/JdbcOwnerRepositoryImpl.java b/src/main/java/org/springframework/samples/petclinic/repository/jdbc/JdbcOwnerRepositoryImpl.java
index 7c28314..6a3f558 100644
--- a/src/main/java/org/springframework/samples/petclinic/repository/jdbc/JdbcOwnerRepositoryImpl.java
+++ b/src/main/java/org/springframework/samples/petclinic/repository/jdbc/JdbcOwnerRepositoryImpl.java
@@ -120,16 +120,16 @@ public class JdbcOwnerRepositoryImpl implements OwnerRepository {
 
 	@Transactional
 	public void save(Owner owner) throws DataAccessException {
+		BeanPropertySqlParameterSource parameterSource = new BeanPropertySqlParameterSource(owner);
 		if (owner.isNew()) {
-			Number newKey = this.insertOwner.executeAndReturnKey(
-					new BeanPropertySqlParameterSource(owner));
+			Number newKey = this.insertOwner.executeAndReturnKey(parameterSource);
 			owner.setId(newKey.intValue());
 		}
 		else {
 			this.namedParameterJdbcTemplate.update(
 					"UPDATE owners SET first_name=:firstName, last_name=:lastName, address=:address, " +
 					"city=:city, telephone=:telephone WHERE id=:id",
-					new BeanPropertySqlParameterSource(owner));
+					parameterSource);
 		}
 	}
 
diff --git a/src/main/java/org/springframework/samples/petclinic/repository/jpa/JpaOwnerRepositoryImpl.java b/src/main/java/org/springframework/samples/petclinic/repository/jpa/JpaOwnerRepositoryImpl.java
index b7a0b71..b5445d1 100644
--- a/src/main/java/org/springframework/samples/petclinic/repository/jpa/JpaOwnerRepositoryImpl.java
+++ b/src/main/java/org/springframework/samples/petclinic/repository/jpa/JpaOwnerRepositoryImpl.java
@@ -6,6 +6,7 @@ import javax.persistence.EntityManager;
 import javax.persistence.PersistenceContext;
 import javax.persistence.Query;
 
+import org.hibernate.Hibernate;
 import org.springframework.samples.petclinic.Owner;
 import org.springframework.samples.petclinic.repository.OwnerRepository;
 import org.springframework.stereotype.Repository;
@@ -32,13 +33,19 @@ public class JpaOwnerRepositoryImpl implements OwnerRepository {
 
 	@SuppressWarnings("unchecked")
 	public Collection<Owner> findByLastName(String lastName) {
-		Query query = this.em.createQuery("SELECT owner FROM Owner owner WHERE owner.lastName LIKE :lastName");
+		// using 'join fetch' because a single query should load both owners and pets
+		// using 'left join fetch' because it might happen that an owner does not have pets yet
+		Query query = this.em.createQuery("SELECT owner FROM Owner owner left join fetch owner.pets WHERE owner.lastName LIKE :lastName");
 		query.setParameter("lastName", lastName + "%");
 		return query.getResultList();
 	}
 
 	public Owner findById(int id) {
-		return this.em.find(Owner.class, id);
+		// using 'join fetch' because a single query should load both owners and pets
+		// using 'left join fetch' because it might happen that an owner does not have pets yet
+		Query query = this.em.createQuery("SELECT owner FROM Owner owner left join fetch owner.pets WHERE owner.id =:id");
+		query.setParameter("id", id);
+		return  (Owner) query.getSingleResult();
 	}
 
 
diff --git a/src/main/java/org/springframework/samples/petclinic/repository/jpa/JpaVetRepositoryImpl.java b/src/main/java/org/springframework/samples/petclinic/repository/jpa/JpaVetRepositoryImpl.java
index 387a29e..04ee001 100644
--- a/src/main/java/org/springframework/samples/petclinic/repository/jpa/JpaVetRepositoryImpl.java
+++ b/src/main/java/org/springframework/samples/petclinic/repository/jpa/JpaVetRepositoryImpl.java
@@ -29,7 +29,7 @@ public class JpaVetRepositoryImpl implements VetRepository {
 
 	@SuppressWarnings("unchecked")
 	public Collection<Vet> findAll() {
-		return this.em.createQuery("SELECT vet FROM Vet vet ORDER BY vet.lastName, vet.firstName").getResultList();
+		return this.em.createQuery("SELECT vet FROM Vet vet join fetch vet.specialties ORDER BY vet.lastName, vet.firstName").getResultList();
 	}
 
 }
diff --git a/src/test/java/org/springframework/samples/petclinic/AbstractOwnerRepositoryTests.java b/src/test/java/org/springframework/samples/petclinic/AbstractOwnerRepositoryTests.java
index 5c25360..a2d0e79 100644
--- a/src/test/java/org/springframework/samples/petclinic/AbstractOwnerRepositoryTests.java
+++ b/src/test/java/org/springframework/samples/petclinic/AbstractOwnerRepositoryTests.java
@@ -97,22 +97,13 @@ public abstract class AbstractOwnerRepositoryTests {
 	}
 
 	@Test @Transactional
-	public void findOwner() {
-		Owner o1 = this.ownerRepository.findById(1);
-		assertTrue(o1.getLastName().startsWith("Franklin"));
-		Owner o10 = this.ownerRepository.findById(10);
-		assertEquals("Carlos", o10.getFirstName());
-
-		// XXX: Add programmatic support for ending transactions with the
-		// TestContext Framework.
-
-		// Check lazy loading, by ending the transaction:
-		// endTransaction();
+	public void findSingleOwner() {
+		Owner owner1 = this.ownerRepository.findById(1);
+		assertTrue(owner1.getLastName().startsWith("Franklin"));
+		Owner owner10 = this.ownerRepository.findById(10);
+		assertEquals("Carlos", owner10.getFirstName());
 
-		// Now Owners are "disconnected" from the data store.
-		// We might need to touch this collection if we switched to lazy loading
-		// in mapping files, but this test would pick this up.
-		o1.getPets();
+		assertEquals(owner1.getPets().size(), 1);
 	}
 
 	@Test @Transactional
diff --git a/src/test/java/org/springframework/samples/petclinic/AbstractPetRepositoryTests.java b/src/test/java/org/springframework/samples/petclinic/AbstractPetRepositoryTests.java
index 8d34312..d097ea3 100644
--- a/src/test/java/org/springframework/samples/petclinic/AbstractPetRepositoryTests.java
+++ b/src/test/java/org/springframework/samples/petclinic/AbstractPetRepositoryTests.java
@@ -97,51 +97,51 @@ public abstract class AbstractPetRepositoryTests {
 	public void getPetTypes() {
 		Collection<PetType> petTypes = this.petRepository.findPetTypes();
 		
-		PetType t1 = EntityUtils.getById(petTypes, PetType.class, 1);
-		assertEquals("cat", t1.getName());
-		PetType t4 = EntityUtils.getById(petTypes, PetType.class, 4);
-		assertEquals("snake", t4.getName());
+		PetType petType1 = EntityUtils.getById(petTypes, PetType.class, 1);
+		assertEquals("cat", petType1.getName());
+		PetType petType4 = EntityUtils.getById(petTypes, PetType.class, 4);
+		assertEquals("snake", petType4.getName());
 	}
 
 	@Test
 	public void findPet() {
 		Collection<PetType> types = this.petRepository.findPetTypes();
-		Pet p7 = this.petRepository.findById(7);
-		assertTrue(p7.getName().startsWith("Samantha"));
-		assertEquals(EntityUtils.getById(types, PetType.class, 1).getId(), p7.getType().getId());
-		assertEquals("Jean", p7.getOwner().getFirstName());
-		Pet p6 = this.petRepository.findById(6);
-		assertEquals("George", p6.getName());
-		assertEquals(EntityUtils.getById(types, PetType.class, 4).getId(), p6.getType().getId());
-		assertEquals("Peter", p6.getOwner().getFirstName());
+		Pet pet7 = this.petRepository.findById(7);
+		assertTrue(pet7.getName().startsWith("Samantha"));
+		assertEquals(EntityUtils.getById(types, PetType.class, 1).getId(), pet7.getType().getId());
+		assertEquals("Jean", pet7.getOwner().getFirstName());
+		Pet pet6 = this.petRepository.findById(6);
+		assertEquals("George", pet6.getName());
+		assertEquals(EntityUtils.getById(types, PetType.class, 4).getId(), pet6.getType().getId());
+		assertEquals("Peter", pet6.getOwner().getFirstName());
 	}
 
 	@Test @Transactional
 	public void insertPet() {
-		Owner o6 = this.ownerRepository.findById(6);
-		int found = o6.getPets().size();
+		Owner owner6 = this.ownerRepository.findById(6);
+		int found = owner6.getPets().size();
 		Pet pet = new Pet();
 		pet.setName("bowser");
 		Collection<PetType> types = this.petRepository.findPetTypes();
 		pet.setType(EntityUtils.getById(types, PetType.class, 2));
 		pet.setBirthDate(new DateTime());
-		o6.addPet(pet);
-		assertEquals(found + 1, o6.getPets().size());
+		owner6.addPet(pet);
+		assertEquals(found + 1, owner6.getPets().size());
 		// both storePet and storeOwner are necessary to cover all ORM tools
 		this.petRepository.save(pet);
-		this.ownerRepository.save(o6);
-		o6 = this.ownerRepository.findById(6);
-		assertEquals(found + 1, o6.getPets().size());
+		this.ownerRepository.save(owner6);
+		owner6 = this.ownerRepository.findById(6);
+		assertEquals(found + 1, owner6.getPets().size());
 	}
 
 	@Test @Transactional
 	public void updatePet() throws Exception {
-		Pet p7 = this.petRepository.findById(7);
-		String old = p7.getName();
-		p7.setName(old + "X");
-		this.petRepository.save(p7);
-		p7 = this.petRepository.findById(7);
-		assertEquals(old + "X", p7.getName());
+		Pet pet7 = this.petRepository.findById(7);
+		String old = pet7.getName();
+		pet7.setName(old + "X");
+		this.petRepository.save(pet7);
+		pet7 = this.petRepository.findById(7);
+		assertEquals(old + "X", pet7.getName());
 	}
 
 }
diff --git a/src/test/java/org/springframework/samples/petclinic/AbstractVisitRepositoryTests.java b/src/test/java/org/springframework/samples/petclinic/AbstractVisitRepositoryTests.java
index 24f7673..ecdc243 100644
--- a/src/test/java/org/springframework/samples/petclinic/AbstractVisitRepositoryTests.java
+++ b/src/test/java/org/springframework/samples/petclinic/AbstractVisitRepositoryTests.java
@@ -90,17 +90,16 @@ public abstract class AbstractVisitRepositoryTests {
 
 	@Test  @Transactional
 	public void insertVisit() {
-		Pet p7 = this.petRepository.findById(7);
-		int found = p7.getVisits().size();
+		Pet pet7 = this.petRepository.findById(7);
+		int found = pet7.getVisits().size();
 		Visit visit = new Visit();
-		p7.addVisit(visit);
+		pet7.addVisit(visit);
 		visit.setDescription("test");
 		// both storeVisit and storePet are necessary to cover all ORM tools
 		this.visitRepository.save(visit);
-		this.petRepository.save(p7);
-		// assertTrue(!visit.isNew()); -- NOT TRUE FOR TOPLINK (before commit)
-		p7 = this.petRepository.findById(7);
-		assertEquals(found + 1, p7.getVisits().size());
+		this.petRepository.save(pet7);
+		pet7 = this.petRepository.findById(7);
+		assertEquals(found + 1, pet7.getVisits().size());
 	}
 
 }
-- 
GitLab