From aac8afa090d25f1d4a325c5f60a2db86f9f77bc2 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Fri, 7 Nov 2014 09:14:21 -0600 Subject: [PATCH] HHH-9476 - bytecode-enhanced lazy to-one does not force selection of foreign-key column(s) when association is fetched --- .../TestFetchingLazyToOneExecutable.java | 177 ++++++++++++++++++ .../test/instrument/domain/Passport.java | 123 ++++++++++++ .../test/instrument/domain/Person.java | 80 ++++++++ ...sformingClassLoaderInstrumentTestCase.java | 9 + 4 files changed, 389 insertions(+) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/instrument/cases/TestFetchingLazyToOneExecutable.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/instrument/domain/Passport.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/instrument/domain/Person.java diff --git a/hibernate-core/src/test/java/org/hibernate/test/instrument/cases/TestFetchingLazyToOneExecutable.java b/hibernate-core/src/test/java/org/hibernate/test/instrument/cases/TestFetchingLazyToOneExecutable.java new file mode 100644 index 0000000000..47d93e9d75 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/instrument/cases/TestFetchingLazyToOneExecutable.java @@ -0,0 +1,177 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2014, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.test.instrument.cases; + +import org.hibernate.Hibernate; +import org.hibernate.Session; +import org.hibernate.SessionFactory; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.cfg.Configuration; +import org.hibernate.service.ServiceRegistry; + +import org.hibernate.testing.ServiceRegistryBuilder; +import org.hibernate.test.instrument.domain.Passport; +import org.hibernate.test.instrument.domain.Person; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +/** + * @author Steve Ebersole + */ +public class TestFetchingLazyToOneExecutable implements Executable { + private ServiceRegistry serviceRegistry; + private SessionFactory factory; + + @Override + public void execute() throws Exception { + doBaselineAssertions(); + + doFetchNonMappedBySideAssertions(); + doFetchMappedBySideAssertions(); + } + + private void doBaselineAssertions() { + { + // First, load from the non-owning side by id. Person#passport should be uninitialized + Session s = factory.openSession(); + s.beginTransaction(); + Person person = (Person) s.get( Person.class, 1 ); + assertTrue( Hibernate.isInitialized( person ) ); + assertFalse( Hibernate.isPropertyInitialized( person, "passport" ) ); + assertNotNull( person.getPassport() ); + s.getTransaction().commit(); + s.close(); + } + + { + // Then, load from the owning side by id. Passport#person should be uninitialized + Session s = factory.openSession(); + s.beginTransaction(); + Passport passport = (Passport) s.get( Passport.class, 1 ); + assertTrue( Hibernate.isInitialized( passport ) ); + assertFalse( Hibernate.isPropertyInitialized( passport, "person" ) ); + assertNotNull( passport.getPerson() ); + s.getTransaction().commit(); + s.close(); + } + } + + private void doFetchNonMappedBySideAssertions() { + // try to eagerly fetch the association from the owning (non-mappedBy) side + Session s = factory.openSession(); + s.beginTransaction(); +// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +// the whole question here is design, and whether the `fetch all properties` should be needed +// Passport p = (Passport) s.createQuery( "select p from Passport p join fetch p.person" ).uniqueResult(); +// versus: +// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + Passport p = (Passport) s.createQuery( "select p from Passport p fetch all properties join fetch p.person" ).uniqueResult(); + assertTrue( Hibernate.isInitialized( p ) ); + assertTrue( + "Assertion that the eager fetch of non-mappedBy association (Passport#person) was performed properly", + Hibernate.isPropertyInitialized( p, "person" ) + ); + assertNotNull( p.getPerson() ); + assertTrue( Hibernate.isInitialized( p.getPerson() ) ); + assertSame( p, p.getPerson().getPassport() ); + s.getTransaction().commit(); + s.close(); + + } + + private void doFetchMappedBySideAssertions() { + // try to eagerly fetch the association from the non-owning (mappedBy) side + Session s = factory.openSession(); + s.beginTransaction(); +// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +// the whole question here is design, and whether the `fetch all properties` should be needed +// Person p = (Person) s.createQuery( "select p from Person p join fetch p.passport" ).uniqueResult(); +// versus: +// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + Person p = (Person) s.createQuery( "select p from Person p fetch all properties join fetch p.passport" ).uniqueResult(); + assertTrue( Hibernate.isInitialized( p ) ); + assertTrue( + "Assertion that the eager fetch of mappedBy association (Person#passport) was performed properly", + Hibernate.isPropertyInitialized( p, "passport" ) + ); + assertNotNull( p.getPassport() ); + assertTrue( Hibernate.isInitialized( p.getPassport() ) ); + assertSame( p, p.getPassport().getPerson() ); + s.getTransaction().commit(); + s.close(); + } + + @Override + public final void prepare() { + Configuration cfg = new Configuration() + .setProperty( AvailableSettings.HBM2DDL_AUTO, "create-drop" ) + .setProperty( AvailableSettings.USE_SECOND_LEVEL_CACHE, "false" ); + cfg.addAnnotatedClass( Person.class ); + cfg.addAnnotatedClass( Passport.class ); + serviceRegistry = ServiceRegistryBuilder.buildServiceRegistry( cfg.getProperties() ); + factory = cfg.buildSessionFactory( serviceRegistry ); + + createData(); + } + + private void createData() { + Person steve = new Person( "Steve" ); + Passport passport = new Passport( steve, "123456789", "Acme Emirates" ); + + Session s = factory.openSession(); + s.beginTransaction(); + s.save( steve ); + s.save( passport ); + s.getTransaction().commit(); + s.close(); + } + + @Override + public final void complete() { + try { + cleanupData(); + } + finally { + factory.close(); + factory = null; + if ( serviceRegistry != null ) { + ServiceRegistryBuilder.destroy( serviceRegistry ); + serviceRegistry = null; + } + } + } + + private void cleanupData() { + Session s = factory.openSession(); + s.beginTransaction(); + s.createQuery( "delete Passport" ).executeUpdate(); + s.createQuery( "delete Person" ).executeUpdate(); + s.getTransaction().commit(); + s.close(); + } + +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/instrument/domain/Passport.java b/hibernate-core/src/test/java/org/hibernate/test/instrument/domain/Passport.java new file mode 100644 index 0000000000..02a39f8562 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/instrument/domain/Passport.java @@ -0,0 +1,123 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2014, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.test.instrument.domain; + +import java.util.Calendar; +import java.util.Date; +import java.util.GregorianCalendar; +import javax.persistence.CascadeType; +import javax.persistence.FetchType; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.OneToOne; + +import org.hibernate.annotations.GenericGenerator; +import org.hibernate.annotations.LazyToOne; +import org.hibernate.annotations.LazyToOneOption; + +/** + * @author Steve Ebersole + */ +@javax.persistence.Entity +public class Passport { + private Integer id; + private Person person; + private String number; + private String issuingCountry; + private Date issueDate; + private Date expirationDate; + + public Passport() { + } + + public Passport(Person person, String number, String issuingCountry) { + this.person = person; + this.number = number; + this.issuingCountry = issuingCountry; + + this.issueDate = new Date(); + + final GregorianCalendar calendar = new GregorianCalendar(); + calendar.setTime( issueDate ); + calendar.set( Calendar.YEAR, calendar.get( Calendar.YEAR ) + 10 ); + this.expirationDate = calendar.getTime(); + + this.person.setPassport( this ); + } + + @Id + @GeneratedValue(generator="increment") + @GenericGenerator(name="increment", strategy = "increment") + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + @OneToOne(fetch= FetchType.LAZY, cascade={CascadeType.MERGE, CascadeType.PERSIST}) + @LazyToOne(value = LazyToOneOption.NO_PROXY) + @JoinColumn(name="person_id") + public Person getPerson() { + return person; + } + + public void setPerson(Person person) { + this.person = person; + } + + public String getNumber() { + return number; + } + + public void setNumber(String number) { + this.number = number; + } + + public String getIssuingCountry() { + return issuingCountry; + } + + public void setIssuingCountry(String issuingCountry) { + this.issuingCountry = issuingCountry; + } + + public Date getIssueDate() { + return issueDate; + } + + public void setIssueDate(Date issueDate) { + this.issueDate = issueDate; + } + + public Date getExpirationDate() { + return expirationDate; + } + + public void setExpirationDate(Date expirationDate) { + this.expirationDate = expirationDate; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/instrument/domain/Person.java b/hibernate-core/src/test/java/org/hibernate/test/instrument/domain/Person.java new file mode 100644 index 0000000000..1631267c5c --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/instrument/domain/Person.java @@ -0,0 +1,80 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2014, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.test.instrument.domain; + +import javax.persistence.CascadeType; +import javax.persistence.FetchType; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.OneToOne; + +import org.hibernate.annotations.GenericGenerator; +import org.hibernate.annotations.LazyToOne; +import org.hibernate.annotations.LazyToOneOption; + +/** + * @author Steve Ebersole + */ +@javax.persistence.Entity +public class Person { + private Integer id; + private String name; + private Passport passport; + + public Person() { + } + + public Person(String name) { + this.name = name; + } + + @Id + @GeneratedValue(generator="increment") + @GenericGenerator(name="increment", strategy = "increment") + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + @OneToOne(fetch= FetchType.LAZY, mappedBy="person",cascade={CascadeType.MERGE, CascadeType.PERSIST, CascadeType.REMOVE}) + @LazyToOne(value = LazyToOneOption.NO_PROXY) + public Passport getPassport() { + return passport; + } + + public void setPassport(Passport passport) { + this.passport = passport; + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/instrument/runtime/AbstractTransformingClassLoaderInstrumentTestCase.java b/hibernate-core/src/test/java/org/hibernate/test/instrument/runtime/AbstractTransformingClassLoaderInstrumentTestCase.java index f87c8de092..257c0a0f05 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/instrument/runtime/AbstractTransformingClassLoaderInstrumentTestCase.java +++ b/hibernate-core/src/test/java/org/hibernate/test/instrument/runtime/AbstractTransformingClassLoaderInstrumentTestCase.java @@ -32,7 +32,10 @@ import org.hibernate.bytecode.spi.BytecodeProvider; import org.hibernate.bytecode.spi.InstrumentedClassLoader; import org.hibernate.dialect.AbstractHANADialect; import org.hibernate.dialect.MySQLDialect; + +import org.hibernate.testing.FailureExpected; import org.hibernate.testing.SkipForDialect; +import org.hibernate.testing.TestForIssue; import org.hibernate.testing.junit4.BaseUnitTestCase; import org.hibernate.testing.junit4.ClassLoadingIsolater; import org.junit.Rule; @@ -89,6 +92,12 @@ public abstract class AbstractTransformingClassLoaderInstrumentTestCase extends executeExecutable( "org.hibernate.test.instrument.cases.TestDirtyCheckExecutable" ); } + @Test + @TestForIssue( jiraKey = "HHH-9476" ) + public void testEagerFetchLazyToOne() { + executeExecutable( "org.hibernate.test.instrument.cases.TestFetchingLazyToOneExecutable" ); + } + @Test @SkipForDialect( value = { MySQLDialect.class, AbstractHANADialect.class }, comment = "wrong sql in mapping, mysql/hana need double type, but it is float type in mapping") public void testFetchAll() throws Exception {