From 253820a28935d256defdd6dac197d19836a38585 Mon Sep 17 00:00:00 2001 From: mharray Date: Tue, 21 Jun 2016 16:23:40 +1200 Subject: [PATCH] HHH-10874 - @Where annotation is not processed with "Extra-lazy" loading for bidirectional collections For bidirectional collections, the where clause is now considered when calculating the size() of the LazyCollectionOption.EXTRA annotated collections --- .../AbstractCollectionPersister.java | 4 + .../java/org/hibernate/sql/SimpleSelect.java | 7 +- .../test/extralazy/Championship.java | 44 ++++++++ .../test/extralazy/ExtraLazyTest.java | 102 +++++++++++++++++- .../org/hibernate/test/extralazy/School.java | 64 +++++++++++ .../org/hibernate/test/extralazy/Student.java | 83 ++++++++++++++ 6 files changed, 298 insertions(+), 6 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/extralazy/Championship.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/extralazy/School.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/extralazy/Student.java diff --git a/hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java index 6993693797..a3962dfa5c 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java @@ -1012,6 +1012,7 @@ public abstract class AbstractCollectionPersister return new SimpleSelect( dialect ) .setTableName( getTableName() ) .addCondition( getKeyColumnNames(), "=?" ) + .addWhereToken( sqlWhereString ) .addColumn( selectValue ) .toStatementString(); } @@ -1025,6 +1026,7 @@ public abstract class AbstractCollectionPersister .addCondition( getKeyColumnNames(), "=?" ) .addCondition( getIndexColumnNames(), "=?" ) .addCondition( indexFormulas, "=?" ) + .addWhereToken( sqlWhereString ) .addColumn( "1" ) .toStatementString(); } @@ -1038,6 +1040,7 @@ public abstract class AbstractCollectionPersister .addCondition( getKeyColumnNames(), "=?" ) .addCondition( getIndexColumnNames(), "=?" ) .addCondition( indexFormulas, "=?" ) + .addWhereToken( sqlWhereString ) .addColumns( getElementColumnNames(), elementColumnAliases ) .addColumns( indexFormulas, indexColumnAliases ) .toStatementString(); @@ -1049,6 +1052,7 @@ public abstract class AbstractCollectionPersister .addCondition( getKeyColumnNames(), "=?" ) .addCondition( getElementColumnNames(), "=?" ) .addCondition( elementFormulas, "=?" ) + .addWhereToken( sqlWhereString ) .addColumn( "1" ) .toStatementString(); } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/SimpleSelect.java b/hibernate-core/src/main/java/org/hibernate/sql/SimpleSelect.java index 51fbc86482..f27f407b2c 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/SimpleSelect.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/SimpleSelect.java @@ -96,7 +96,12 @@ public class SimpleSelect { } public SimpleSelect addWhereToken(String token) { - whereTokens.add( token ); + if (token != null ) { + if (!whereTokens.isEmpty()) { + and(); + } + whereTokens.add( token ); + } return this; } diff --git a/hibernate-core/src/test/java/org/hibernate/test/extralazy/Championship.java b/hibernate-core/src/test/java/org/hibernate/test/extralazy/Championship.java new file mode 100644 index 0000000000..e59bd4f5de --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/extralazy/Championship.java @@ -0,0 +1,44 @@ +package org.hibernate.test.extralazy; + +import java.util.ArrayList; +import java.util.List; +import javax.persistence.CascadeType; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.OneToMany; +import javax.persistence.Table; + +import org.hibernate.annotations.LazyCollection; +import org.hibernate.annotations.LazyCollectionOption; +import org.hibernate.annotations.Where; + +@Entity +@Table(name = "championship") +public class Championship { + + @Id + private int id; + + @OneToMany(cascade = CascadeType.ALL) + @LazyCollection(LazyCollectionOption.EXTRA) + @Where(clause = " gpa >= 4 ") + private List students = new ArrayList<>(); + + public Championship() {} + + public Championship(int id) { + this.id = id; + } + + public int getId() { + return id; + } + + public void setId(int id) { + this.id = id; + } + + public List getStudents() { + return students; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/extralazy/ExtraLazyTest.java b/hibernate-core/src/test/java/org/hibernate/test/extralazy/ExtraLazyTest.java index 1b7afddbc6..cd621bd067 100755 --- a/hibernate-core/src/test/java/org/hibernate/test/extralazy/ExtraLazyTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/extralazy/ExtraLazyTest.java @@ -5,11 +5,6 @@ * See the lgpl.txt file in the root directory or . */ package org.hibernate.test.extralazy; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; import java.util.List; import java.util.Map; @@ -19,11 +14,18 @@ import org.hibernate.Session; import org.hibernate.Transaction; import org.hibernate.testing.DialectChecks; +import org.hibernate.testing.FailureExpected; import org.hibernate.testing.RequiresDialectFeature; import org.hibernate.testing.TestForIssue; import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; import org.junit.Test; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + /** * @author Gavin King */ @@ -32,6 +34,11 @@ public class ExtraLazyTest extends BaseCoreFunctionalTestCase { public String[] getMappings() { return new String[] { "extralazy/UserGroup.hbm.xml","extralazy/Parent.hbm.xml","extralazy/Child.hbm.xml" }; } + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { School.class, Student.class, Championship.class }; + } @Test public void testOrphanDelete() { @@ -248,5 +255,90 @@ public class ExtraLazyTest extends BaseCoreFunctionalTestCase { assertNotNull(child2); session2.close(); } + + @Test + @TestForIssue(jiraKey = "HHH-10874") + public void testWhereClauseOnBidirectionalCollection() { + Session s = openSession(); + Transaction t = s.beginTransaction(); + + School school = new School(1); + s.persist(school); + + Student gavin = new Student("gavin", 4); + Student turin = new Student("turin", 3); + Student mike = new Student("mike", 5); + Student fred = new Student("fred", 2); + + gavin.setSchool(school); + turin.setSchool(school); + mike.setSchool(school); + fred.setSchool(school); + + s.persist(gavin); + s.persist(turin); + s.persist(mike); + s.persist(fred); + + t.commit(); + s.close(); + + s = openSession(); + School school2 = s.get(School.class, 1); + + assertEquals(4, school2.getStudents().size()); + + assertEquals( 2, school2.getTopStudents().size() ); + assertTrue( school2.getTopStudents().contains( gavin ) ); + assertTrue( school2.getTopStudents().contains( mike ) ); + + assertEquals(2, school2.getStudentsMap().size() ); + assertTrue( school2.getStudentsMap().containsKey( gavin.getId() ) ); + assertTrue( school2.getStudentsMap().containsKey( mike.getId() ) ); + + s.close(); + } + + @Test + @FailureExpected( jiraKey = "HHH-3319" ) + public void testWhereClauseOnUnidirectionalCollection() { + Session s = openSession(); + Transaction t = s.beginTransaction(); + + Championship championship = new Championship( 1 ); + s.persist(championship); + + Student gavin = new Student("gavin", 4); + Student turin = new Student("turin", 3); + Student mike = new Student("mike", 5); + Student fred = new Student("fred", 2); + + championship.getStudents().add( gavin ); + championship.getStudents().add( turin ); + championship.getStudents().add( mike ); + championship.getStudents().add( fred ); + + s.persist(gavin); + s.persist(turin); + s.persist(mike); + s.persist(fred); + + t.commit(); + s.close(); + + s = openSession(); + + Championship championship2 = s.get(Championship.class, 1); + assertEquals( 2, championship2.getStudents().size() ); + assertTrue( championship2.getStudents().contains( gavin ) ); + assertTrue( championship2.getStudents().contains( mike ) ); + + s.close(); + } + + @Override + protected boolean rebuildSessionFactoryOnError() { + return false; + } } diff --git a/hibernate-core/src/test/java/org/hibernate/test/extralazy/School.java b/hibernate-core/src/test/java/org/hibernate/test/extralazy/School.java new file mode 100644 index 0000000000..faf307da80 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/extralazy/School.java @@ -0,0 +1,64 @@ +package org.hibernate.test.extralazy; + +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.MapKey; +import javax.persistence.OneToMany; +import javax.persistence.Table; + +import org.hibernate.annotations.LazyCollection; +import org.hibernate.annotations.LazyCollectionOption; +import org.hibernate.annotations.Where; + +@Entity +@Table(name = "school") +public class School { + + @Id + private int id; + + @OneToMany(mappedBy = "school") + @LazyCollection(LazyCollectionOption.EXTRA) + private Set students = new HashSet(); + + @OneToMany(mappedBy = "school") + @LazyCollection(LazyCollectionOption.EXTRA) + @Where(clause = " gpa >= 4 ") + private Set topStudents = new HashSet(); + + @OneToMany(mappedBy = "school") + @LazyCollection(LazyCollectionOption.EXTRA) + @Where(clause = " gpa >= 4 ") + @MapKey + private Map studentsMap = new HashMap<>(); + + public School() {} + + public School(int id) { + this.id = id; + } + + public int getId() { + return id; + } + + public void setId(int id) { + this.id = id; + } + + public Set getStudents() { + return students; + } + + public Set getTopStudents() { + return topStudents; + } + + public Map getStudentsMap() { + return studentsMap; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/extralazy/Student.java b/hibernate-core/src/test/java/org/hibernate/test/extralazy/Student.java new file mode 100644 index 0000000000..6086d92b59 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/extralazy/Student.java @@ -0,0 +1,83 @@ +package org.hibernate.test.extralazy; + +import java.util.Objects; +import javax.persistence.Entity; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.ManyToOne; +import javax.persistence.Table; + +import static javax.persistence.GenerationType.IDENTITY; + +@Entity +@Table(name = "student") +public class Student { + + @Id + @GeneratedValue(strategy = IDENTITY) + private String id; + + @ManyToOne + private School school; + + private String firstName; + + private int gpa; + + public Student() {} + + public Student(String firstName, int gpa) { + this.firstName = firstName; + this.gpa = gpa; + } + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public School getSchool() { + return school; + } + + public void setSchool(School school) { + this.school = school; + } + + public String getFirstName() { + return firstName; + } + + public void setFirstName(String firstName) { + this.firstName = firstName; + } + + public int getGpa() { + return gpa; + } + + public void setGpa(int gpa) { + this.gpa = gpa; + } + + + @Override + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + if ( !( o instanceof Student ) ) { + return false; + } + Student student = (Student) o; + return Objects.equals( getFirstName(), student.getFirstName() ); + } + + @Override + public int hashCode() { + return Objects.hash( getFirstName() ); + } +}