From 786a32d6c20cb7277ba106ae06538c6cf42f159d Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Tue, 26 Feb 2019 22:24:54 -0800 Subject: [PATCH 1/5] HHH-13241 : Constraint violation when deleting entites in bi-directional, lazy OneToMany association with bytecode enhancement (cherry picked from commit 980f24916ca27cd4e5ae658de353358d42c94cd2) --- .../internal/AbstractEntityInsertAction.java | 4 +- .../engine/internal/ForeignKeys.java | 95 ++++- .../internal/DefaultDeleteEventListener.java | 3 +- .../entity/AbstractEntityPersister.java | 16 +- .../lazy/BidirectionalLazyTest.java | 354 ++++++++++++++++++ ...directionalLazyGroupsInEmbeddableTest.java | 225 +++++++++++ .../group/BidirectionalLazyGroupsTest.java | 218 +++++++++++ 7 files changed, 889 insertions(+), 26 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/BidirectionalLazyTest.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/group/BidirectionalLazyGroupsInEmbeddableTest.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/group/BidirectionalLazyGroupsTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/action/internal/AbstractEntityInsertAction.java b/hibernate-core/src/main/java/org/hibernate/action/internal/AbstractEntityInsertAction.java index ad44c4b447a1..6bc20950878c 100644 --- a/hibernate-core/src/main/java/org/hibernate/action/internal/AbstractEntityInsertAction.java +++ b/hibernate-core/src/main/java/org/hibernate/action/internal/AbstractEntityInsertAction.java @@ -110,8 +110,8 @@ public NonNullableTransientDependencies findNonNullableTransientEntities() { */ protected final void nullifyTransientReferencesIfNotAlready() { if ( ! areTransientReferencesNullified ) { - new ForeignKeys.Nullifier( getInstance(), false, isEarlyInsert(), getSession() ) - .nullifyTransientReferences( getState(), getPersister().getPropertyTypes() ); + new ForeignKeys.Nullifier( getInstance(), false, isEarlyInsert(), getSession(), getPersister() ) + .nullifyTransientReferences( getState() ); new Nullability( getSession() ).checkNullability( getState(), getPersister(), false ); areTransientReferencesNullified = true; } diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/ForeignKeys.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/ForeignKeys.java index e98f9e7c7f0c..94e05a7b910f 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/ForeignKeys.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/ForeignKeys.java @@ -13,7 +13,9 @@ import org.hibernate.TransientObjectException; import org.hibernate.bytecode.enhance.spi.LazyPropertyInitializer; import org.hibernate.engine.spi.EntityEntry; +import org.hibernate.engine.spi.SelfDirtinessTracker; import org.hibernate.engine.spi.SharedSessionContractImplementor; +import org.hibernate.internal.util.StringHelper; import org.hibernate.persister.entity.EntityPersister; import org.hibernate.proxy.HibernateProxy; import org.hibernate.proxy.LazyInitializer; @@ -36,6 +38,7 @@ public static class Nullifier { private final boolean isEarlyInsert; private final SharedSessionContractImplementor session; private final Object self; + private final EntityPersister persister; /** * Constructs a Nullifier @@ -44,11 +47,18 @@ public static class Nullifier { * @param isDelete Are we in the middle of a delete action? * @param isEarlyInsert Is this an early insert (INSERT generated id strategy)? * @param session The session + * @param persister The EntityPersister for {@code self} */ - public Nullifier(Object self, boolean isDelete, boolean isEarlyInsert, SharedSessionContractImplementor session) { + public Nullifier( + final Object self, + final boolean isDelete, + final boolean isEarlyInsert, + final SharedSessionContractImplementor session, + final EntityPersister persister) { this.isDelete = isDelete; this.isEarlyInsert = isEarlyInsert; this.session = session; + this.persister = persister; this.self = self; } @@ -57,11 +67,12 @@ public Nullifier(Object self, boolean isDelete, boolean isEarlyInsert, SharedSes * points toward that entity. * * @param values The entity attribute values - * @param types The entity attribute types */ - public void nullifyTransientReferences(final Object[] values, final Type[] types) { + public void nullifyTransientReferences(final Object[] values) { + final String[] propertyNames = persister.getPropertyNames(); + final Type[] types = persister.getPropertyTypes(); for ( int i = 0; i < types.length; i++ ) { - values[i] = nullifyTransientReferences( values[i], types[i] ); + values[i] = nullifyTransientReferences( values[i], propertyNames[i], types[i] ); } } @@ -70,34 +81,47 @@ public void nullifyTransientReferences(final Object[] values, final Type[] types * input argument otherwise. This is how Hibernate avoids foreign key constraint violations. * * @param value An entity attribute value + * @param propertyName An entity attribute name * @param type An entity attribute type * * @return {@code null} if the argument is an unsaved entity; otherwise return the argument. */ - private Object nullifyTransientReferences(final Object value, final Type type) { + private Object nullifyTransientReferences(final Object value, final String propertyName, final Type type) { + final Object returnedValue; if ( value == null ) { - return null; + returnedValue = null; } else if ( type.isEntityType() ) { final EntityType entityType = (EntityType) type; if ( entityType.isOneToOne() ) { - return value; + returnedValue = value; } else { - final String entityName = entityType.getAssociatedEntityName(); - return isNullifiable( entityName, value ) ? null : value; + // If value is lazy, it may need to be initialized to + // determine if the value is nullifiable. + final Object possiblyInitializedValue = initializeIfNecessary( value, propertyName, entityType ); + // If the value is not nullifiable, make sure that the + // possibly initialized value is returned. + returnedValue = isNullifiable( entityType.getAssociatedEntityName(), possiblyInitializedValue ) + ? null + : possiblyInitializedValue; } } else if ( type.isAnyType() ) { - return isNullifiable( null, value ) ? null : value; + returnedValue = isNullifiable( null, value ) ? null : value; } else if ( type.isComponentType() ) { final CompositeType actype = (CompositeType) type; final Object[] subvalues = actype.getPropertyValues( value, session ); final Type[] subtypes = actype.getSubtypes(); + final String[] subPropertyNames = actype.getPropertyNames(); boolean substitute = false; for ( int i = 0; i < subvalues.length; i++ ) { - final Object replacement = nullifyTransientReferences( subvalues[i], subtypes[i] ); + final Object replacement = nullifyTransientReferences( + subvalues[i], + StringHelper.qualify( propertyName, subPropertyNames[i] ), + subtypes[i] + ); if ( replacement != subvalues[i] ) { substitute = true; subvalues[i] = replacement; @@ -107,7 +131,50 @@ else if ( type.isComponentType() ) { // todo : need to account for entity mode on the CompositeType interface :( actype.setPropertyValues( value, subvalues, EntityMode.POJO ); } - return value; + returnedValue = value; + } + else { + returnedValue = value; + } + // value != returnedValue if either: + // 1) returnedValue was nullified (set to null); + // or 2) returnedValue was initialized, but not nullified. + // When bytecode-enhancement is used for dirty-checking, the change should + // only be tracked when returnedValue was nullified (1)). + if ( value != returnedValue && returnedValue == null && SelfDirtinessTracker.class.isInstance( self ) ) { + ( (SelfDirtinessTracker) self ).$$_hibernate_trackChange( propertyName ); + } + return returnedValue; + } + + private Object initializeIfNecessary( + final Object value, + final String propertyName, + final Type type) { + if ( isDelete && + value == LazyPropertyInitializer.UNFETCHED_PROPERTY && + type.isEntityType() && + !session.getPersistenceContext().getNullifiableEntityKeys().isEmpty() ) { + // IMPLEMENTATION NOTE: If cascade-remove was mapped for the attribute, + // then value should have been initialized previously, when the remove operation was + // cascaded to the property (because CascadingAction.DELETE.performOnLazyProperty() + // returns true). This particular situation can only arise when cascade-remove is not + // mapped for the association. + + // There is at least one nullifiable entity. We don't know if the lazy + // associated entity is one of the nullifiable entities. If it is, and + // the property is not nullified, then a constraint violation will result. + // The only way to find out if the associated entity is nullifiable is + // to initialize it. + // TODO: there may be ways to fine-tune when initialization is necessary + // (e.g., only initialize when the associated entity type is a + // superclass or the same as the entity type of a nullifiable entity). + // It is unclear if a more complicated check would impact performance + // more than just initializing the associated entity. + return persister + .getInstrumentationMetadata() + .extractInterceptor( self ) + .fetchAttribute( self, propertyName ); } else { return value; @@ -162,9 +229,7 @@ private boolean isNullifiable(final String entityName, Object object) else { return entityEntry.isNullifiable( isEarlyInsert, session ); } - } - } /** @@ -307,8 +372,8 @@ public static NonNullableTransientDependencies findNonNullableTransientEntities( Object[] values, boolean isEarlyInsert, SharedSessionContractImplementor session) { - final Nullifier nullifier = new Nullifier( entity, false, isEarlyInsert, session ); final EntityPersister persister = session.getEntityPersister( entityName, entity ); + final Nullifier nullifier = new Nullifier( entity, false, isEarlyInsert, session, persister ); final String[] propertyNames = persister.getPropertyNames(); final Type[] types = persister.getPropertyTypes(); final boolean[] nullability = persister.getPropertyNullability(); diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java index bfb60701661c..f04b7504b010 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java @@ -285,8 +285,7 @@ protected final void deleteEntity( cascadeBeforeDelete( session, persister, entity, entityEntry, transientEntities ); - new ForeignKeys.Nullifier( entity, true, false, session ) - .nullifyTransientReferences( entityEntry.getDeletedState(), propTypes ); + new ForeignKeys.Nullifier( entity, true, false, session, persister ).nullifyTransientReferences( entityEntry.getDeletedState() ); new Nullability( session ).checkNullability( entityEntry.getDeletedState(), persister, Nullability.NullabilityCheckType.DELETE ); persistenceContext.getNullifiableEntityKeys().add( key ); diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java index 21ef9cf37c4a..d0b794d91843 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java @@ -1179,7 +1179,6 @@ private Object initializeLazyPropertiesFromDatastore( rs = session.getJdbcCoordinator().getResultSetReturn().extract( ps ); rs.next(); } - final Object[] snapshot = entry.getLoadedState(); for ( LazyAttributeDescriptor fetchGroupAttributeDescriptor : fetchGroupAttributeDescriptors ) { final boolean previousInitialized = initializedLazyAttributeNames.contains( fetchGroupAttributeDescriptor.getName() ); @@ -1210,7 +1209,7 @@ private Object initializeLazyPropertiesFromDatastore( fieldName, entity, session, - snapshot, + entry, fetchGroupAttributeDescriptor.getLazyIndex(), selectedValue ); @@ -1259,7 +1258,6 @@ private Object initializeLazyPropertiesFromCache( Object result = null; Serializable[] disassembledValues = cacheEntry.getDisassembledState(); - final Object[] snapshot = entry.getLoadedState(); for ( int j = 0; j < lazyPropertyNames.length; j++ ) { final Serializable cachedValue = disassembledValues[lazyPropertyNumbers[j]]; final Type lazyPropertyType = lazyPropertyTypes[j]; @@ -1276,7 +1274,7 @@ private Object initializeLazyPropertiesFromCache( session, entity ); - if ( initializeLazyProperty( fieldName, entity, session, snapshot, j, propValue ) ) { + if ( initializeLazyProperty( fieldName, entity, session, entry, j, propValue ) ) { result = propValue; } } @@ -1291,13 +1289,17 @@ private boolean initializeLazyProperty( final String fieldName, final Object entity, final SharedSessionContractImplementor session, - final Object[] snapshot, + final EntityEntry entry, final int j, final Object propValue) { setPropertyValue( entity, lazyPropertyNumbers[j], propValue ); - if ( snapshot != null ) { + if ( entry.getLoadedState() != null ) { // object have been loaded with setReadOnly(true); HHH-2236 - snapshot[lazyPropertyNumbers[j]] = lazyPropertyTypes[j].deepCopy( propValue, factory ); + entry.getLoadedState()[lazyPropertyNumbers[j]] = lazyPropertyTypes[j].deepCopy( propValue, factory ); + } + // If the entity has deleted state, then update that as well + if ( entry.getDeletedState() != null ) { + entry.getDeletedState()[lazyPropertyNumbers[j]] = lazyPropertyTypes[j].deepCopy( propValue, factory ); } return fieldName.equals( lazyPropertyNames[j] ); } diff --git a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/BidirectionalLazyTest.java b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/BidirectionalLazyTest.java new file mode 100644 index 000000000000..867d965be50d --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/BidirectionalLazyTest.java @@ -0,0 +1,354 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.test.bytecode.enhancement.lazy; + +import java.util.HashSet; +import java.util.Set; +import javax.persistence.Entity; +import javax.persistence.FetchType; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.ManyToOne; +import javax.persistence.OneToMany; + +import org.hibernate.Hibernate; +import org.hibernate.Session; +import org.hibernate.annotations.LazyToOne; +import org.hibernate.annotations.LazyToOneOption; +import org.hibernate.bytecode.enhance.spi.DefaultEnhancementContext; +import org.hibernate.bytecode.enhance.spi.LazyPropertyInitializer; +import org.hibernate.bytecode.enhance.spi.UnloadedClass; +import org.hibernate.bytecode.enhance.spi.UnloadedField; +import org.hibernate.engine.spi.EntityEntry; +import org.hibernate.engine.spi.SessionImplementor; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.bytecode.enhancement.BytecodeEnhancerRunner; +import org.hibernate.testing.bytecode.enhancement.CustomEnhancementContext; +import org.hibernate.testing.bytecode.enhancement.EnhancerTestContext; +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.junit.Test; +import org.junit.runner.RunWith; + +import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +/** + * Tests removing non-owning side of the bidirectional association, + * with and without dirty-checking using enhancement. + * + * @author Gail Badner + */ +@TestForIssue(jiraKey = "HHH-13241") +@RunWith(BytecodeEnhancerRunner.class) +@CustomEnhancementContext({ + EnhancerTestContext.class, // supports laziness and dirty-checking + BidirectionalLazyTest.NoDirtyCheckEnhancementContext.class // supports laziness; does not support dirty-checking +}) +public class BidirectionalLazyTest extends BaseCoreFunctionalTestCase { + + public Class[] getAnnotatedClasses() { + return new Class[] { Employer.class, Employee.class, Unrelated.class }; + } + + @Test + public void testRemoveWithDeletedAssociatedEntity() { + + doInHibernate( + this::sessionFactory, session -> { + + Employer employer = new Employer( "RedHat" ); + session.persist( employer ); + employer.addEmployee( new Employee( "Jack" ) ); + employer.addEmployee( new Employee( "Jill" ) ); + employer.addEmployee( new Employee( "John" ) ); + for ( Employee employee : employer.getEmployees() ) { + session.persist( employee ); + } + } + ); + + doInHibernate( + this::sessionFactory, session -> { + Employer employer = session.get( Employer.class, "RedHat" ); + // Delete the associated entity first + session.remove( employer ); + for ( Employee employee : employer.getEmployees() ) { + assertFalse( Hibernate.isPropertyInitialized( employee, "employer" ) ); + session.remove( employee ); + // Should be initialized because at least one entity was deleted beforehand + assertTrue( Hibernate.isPropertyInitialized( employee, "employer" ) ); + assertSame( employer, employee.getEmployer() ); + // employee.getEmployer was initialized, and should be nullified in EntityEntry#deletedState + checkEntityEntryState( session, employee, employer, true ); + } + } + ); + + doInHibernate( + this::sessionFactory, session -> { + assertNull( session.find( Employer.class, "RedHat" ) ); + assertTrue( session.createQuery( "from Employee e", Employee.class ).getResultList().isEmpty() ); + } + ); + } + + @Test + public void testRemoveWithNonAssociatedRemovedEntity() { + + doInHibernate( + this::sessionFactory, session -> { + Employer employer = new Employer( "RedHat" ); + session.persist( employer ); + Employee employee = new Employee( "Jack" ); + employer.addEmployee( employee ); + session.persist( employee ); + session.persist( new Unrelated( 1 ) ); + } + ); + + doInHibernate( + this::sessionFactory, session -> { + // Delete an entity that is not associated with Employee + session.remove( session.get( Unrelated.class, 1 ) ); + final Employee employee = session.get( Employee.class, "Jack" ); + assertFalse( Hibernate.isPropertyInitialized( employee, "employer" ) ); + session.remove( employee ); + // Should be initialized because at least one entity was deleted beforehand + assertTrue( Hibernate.isPropertyInitialized( employee, "employer" ) ); + // employee.getEmployer was initialized, and should not be nullified in EntityEntry#deletedState + checkEntityEntryState( session, employee, employee.getEmployer(), false ); + } + ); + + doInHibernate( + this::sessionFactory, session -> { + assertNull( session.find( Unrelated.class, 1 ) ); + assertNull( session.find( Employee.class, "Jack" ) ); + session.remove( session.find( Employer.class, "RedHat" ) ); + } + ); + } + + @Test + public void testRemoveWithNoRemovedEntities() { + + doInHibernate( + this::sessionFactory, session -> { + Employer employer = new Employer( "RedHat" ); + session.persist( employer ); + Employee employee = new Employee( "Jack" ); + employer.addEmployee( employee ); + session.persist( employee ); + } + ); + + doInHibernate( + this::sessionFactory, session -> { + // Don't delete any entities before deleting the Employee + final Employee employee = session.get( Employee.class, "Jack" ); + assertFalse( Hibernate.isPropertyInitialized( employee, "employer" ) ); + session.remove( employee ); + // There were no other deleted entities before employee was deleted, + // so there was no need to initialize employee.employer. + assertFalse( Hibernate.isPropertyInitialized( employee, "employer" ) ); + // employee.getEmployer was not initialized, and should not be nullified in EntityEntry#deletedState + checkEntityEntryState( session, employee, LazyPropertyInitializer.UNFETCHED_PROPERTY, false ); + } + ); + + doInHibernate( + this::sessionFactory, session -> { + assertNull( session.find( Employee.class, "Jack" ) ); + session.remove( session.find( Employer.class, "RedHat" ) ); + } + ); + } + + private void checkEntityEntryState( + final Session session, + final Employee employee, + final Object employer, + final boolean isEmployerNullified) { + final SessionImplementor sessionImplementor = (SessionImplementor) session; + final EntityEntry entityEntry = sessionImplementor.getPersistenceContext().getEntry( employee ); + final int propertyNumber = entityEntry.getPersister().getEntityMetamodel().getPropertyIndex( "employer" ); + assertEquals( + employer, + entityEntry.getLoadedState()[propertyNumber] + ); + if ( isEmployerNullified ) { + assertEquals( null, entityEntry.getDeletedState()[propertyNumber] ); + } + else { + assertEquals( + employer, + entityEntry.getDeletedState()[propertyNumber] + ); + } + } + + @Entity(name = "Employer") + public static class Employer { + private String name; + + private Set employees; + + public Employer(String name) { + this(); + setName( name ); + } + + @Id + public String getName() { + return name; + } + + @OneToMany(mappedBy = "employer", fetch = FetchType.LAZY) + public Set getEmployees() { + return employees; + } + + public void addEmployee(Employee employee) { + if ( getEmployees() == null ) { + setEmployees( new HashSet<>() ); + } + employees.add( employee ); + employee.setEmployer( this ); + } + + protected Employer() { + // this form used by Hibernate + } + + protected void setName(String name) { + this.name = name; + } + + protected void setEmployees(Set employees) { + this.employees = employees; + } + } + + @Entity(name = "Employee") + public static class Employee { + private long id; + + private String name; + + private Employer employer; + + public Employee(String name) { + this(); + setName( name ); + } + + public long getId() { + return id; + } + + @Id + public String getName() { + return name; + } + + @ManyToOne(fetch = FetchType.LAZY) + @LazyToOne(LazyToOneOption.NO_PROXY) + @JoinColumn(name = "employer_name") + public Employer getEmployer() { + return employer; + } + + protected Employee() { + // this form used by Hibernate + } + + protected void setId(long id) { + this.id = id; + } + + protected void setName(String name) { + this.name = name; + } + + protected void setEmployer(Employer employer) { + this.employer = employer; + } + + public int hashCode() { + if ( name != null ) { + return name.hashCode(); + } + else { + return 0; + } + } + + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + else if ( o instanceof Employee ) { + Employee other = Employee.class.cast( o ); + if ( name != null ) { + return getName().equals( other.getName() ); + } + else { + return other.getName() == null; + } + } + else { + return false; + } + } + } + + @Entity(name = "Manager") + public static class Manager extends Employee { + } + + @Entity(name = "Unrelated") + public static class Unrelated { + private int id; + + public Unrelated() { + } + + public Unrelated(int id) { + setId( id ); + } + + @Id + public int getId() { + return id; + } + public void setId(int id) { + this.id = id; + } + } + + public static class NoDirtyCheckEnhancementContext extends DefaultEnhancementContext { + @Override + public boolean hasLazyLoadableAttributes(UnloadedClass classDescriptor) { + return true; + } + + @Override + public boolean isLazyLoadable(UnloadedField field) { + return true; + } + + @Override + public boolean doDirtyCheckingInline(UnloadedClass classDescriptor) { + return false; + } + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/group/BidirectionalLazyGroupsInEmbeddableTest.java b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/group/BidirectionalLazyGroupsInEmbeddableTest.java new file mode 100644 index 000000000000..d29dca5371a0 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/group/BidirectionalLazyGroupsInEmbeddableTest.java @@ -0,0 +1,225 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.test.bytecode.enhancement.lazy.group; + +import java.util.HashSet; +import java.util.Set; +import javax.persistence.CascadeType; +import javax.persistence.Embeddable; +import javax.persistence.Entity; +import javax.persistence.FetchType; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.ManyToOne; +import javax.persistence.OneToMany; + +import org.hibernate.annotations.LazyGroup; +import org.hibernate.annotations.LazyToOne; +import org.hibernate.annotations.LazyToOneOption; +import org.hibernate.bytecode.enhance.spi.DefaultEnhancementContext; +import org.hibernate.bytecode.enhance.spi.UnloadedClass; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.bytecode.enhancement.BytecodeEnhancerRunner; +import org.hibernate.testing.bytecode.enhancement.CustomEnhancementContext; +import org.hibernate.testing.bytecode.enhancement.EnhancerTestContext; +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.junit.Test; +import org.junit.runner.RunWith; + +import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate; + +/** + * Tests removing non-owning side of the bidirectional association, + * where owning side is in an embeddable. + * + * Tests with and without dirty-checking using enhancement. + * + * @author Gail Badner + */ +@TestForIssue(jiraKey = "HHH-13241") +@RunWith(BytecodeEnhancerRunner.class) +@CustomEnhancementContext({ + EnhancerTestContext.class, + BidirectionalLazyGroupsInEmbeddableTest.NoDirtyCheckEnhancementContext.class +}) +public class BidirectionalLazyGroupsInEmbeddableTest extends BaseCoreFunctionalTestCase { + + public Class[] getAnnotatedClasses() { + return new Class[] { Employer.class, Employee.class }; + } + + @Test + public void test() { + + doInHibernate( + this::sessionFactory, session -> { + + Employer employer = new Employer( "RedHat" ); + session.persist( employer ); + employer.addEmployee( new Employee( "Jack" ) ); + employer.addEmployee( new Employee( "Jill" ) ); + employer.addEmployee( new Employee( "John" ) ); + for ( Employee employee : employer.getEmployees() ) { + session.persist( employee ); + } + } + ); + + doInHibernate( + this::sessionFactory, session -> { + Employer employer = session.createQuery( "from Employer e", Employer.class ).getSingleResult(); + session.remove( employer ); + for ( Employee employee : employer.getEmployees() ) { + session.remove( employee ); + } + } + ); + } + + @Entity(name = "Employer") + public static class Employer { + private String name; + + private Set employees; + + public Employer(String name) { + this(); + setName( name ); + } + + @Id + public String getName() { + return name; + } + + @OneToMany(mappedBy = "employerContainer.employer", fetch = FetchType.LAZY) + @LazyGroup("Employees") + public Set getEmployees() { + return employees; + } + + public void addEmployee(Employee employee) { + if ( getEmployees() == null ) { + setEmployees( new HashSet<>() ); + } + employees.add( employee ); + if ( employee.getEmployerContainer() == null ) { + employee.setEmployerContainer( new EmployerContainer() ); + } + employee.getEmployerContainer().setEmployer( this ); + } + + protected Employer() { + // this form used by Hibernate + } + + protected void setName(String name) { + this.name = name; + } + + protected void setEmployees(Set employees) { + this.employees = employees; + } + } + + @Entity(name = "Employee") + public static class Employee { + private long id; + + private String name; + + public Employee(String name) { + this(); + setName( name ); + } + + @Id + @GeneratedValue + public long getId() { + return id; + } + + public String getName() { + return name; + } + + public EmployerContainer employerContainer; + + protected Employee() { + // this form used by Hibernate + } + + public EmployerContainer getEmployerContainer() { + return employerContainer; + } + + protected void setId(long id) { + this.id = id; + } + + protected void setName(String name) { + this.name = name; + } + + protected void setEmployerContainer(EmployerContainer employerContainer) { + this.employerContainer = employerContainer; + } + + public int hashCode() { + if ( name != null ) { + return name.hashCode(); + } + else { + return 0; + } + } + + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + else if ( o instanceof Employee ) { + Employee other = Employee.class.cast( o ); + if ( name != null ) { + return getName().equals( other.getName() ); + } + else { + return other.getName() == null; + } + } + else { + return false; + } + } + } + + @Embeddable + public static class EmployerContainer { + private Employer employer; + + @ManyToOne(fetch = FetchType.LAZY, cascade = CascadeType.REMOVE) + @LazyToOne(LazyToOneOption.NO_PROXY) + @LazyGroup("EmployerForEmployee") + @JoinColumn(name = "employer_name") + public Employer getEmployer() { + return employer; + } + + protected void setEmployer(Employer employer) { + this.employer = employer; + } + } + + public static class NoDirtyCheckEnhancementContext extends DefaultEnhancementContext { + @Override + public boolean doDirtyCheckingInline(UnloadedClass classDescriptor) { + return false; + } + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/group/BidirectionalLazyGroupsTest.java b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/group/BidirectionalLazyGroupsTest.java new file mode 100644 index 000000000000..8898e38d3008 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/group/BidirectionalLazyGroupsTest.java @@ -0,0 +1,218 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.test.bytecode.enhancement.lazy.group; + +import java.util.HashSet; +import java.util.Set; +import javax.persistence.Entity; +import javax.persistence.FetchType; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.ManyToOne; +import javax.persistence.OneToMany; + +import org.hibernate.Hibernate; +import org.hibernate.annotations.LazyGroup; +import org.hibernate.annotations.LazyToOne; +import org.hibernate.annotations.LazyToOneOption; +import org.hibernate.bytecode.enhance.spi.DefaultEnhancementContext; +import org.hibernate.bytecode.enhance.spi.UnloadedClass; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.bytecode.enhancement.BytecodeEnhancerRunner; +import org.hibernate.testing.bytecode.enhancement.CustomEnhancementContext; +import org.hibernate.testing.bytecode.enhancement.EnhancerTestContext; +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.junit.Test; +import org.junit.runner.RunWith; + +import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +/** + * Tests removing non-owning side of the bidirectional association, + * with and without dirty-checking using enhancement. + * + * @author Gail Badner + */ +@TestForIssue(jiraKey = "HHH-13241") +@RunWith(BytecodeEnhancerRunner.class) +@CustomEnhancementContext({ + EnhancerTestContext.class, + BidirectionalLazyGroupsTest.NoDirtyCheckEnhancementContext.class +}) +public class BidirectionalLazyGroupsTest extends BaseCoreFunctionalTestCase { + + public Class[] getAnnotatedClasses() { + return new Class[] { Employer.class, Employee.class }; + } + + @Test + public void testRemoveCollectionOwnerNoCascade() { + + doInHibernate( + this::sessionFactory, session -> { + + Employer employer = new Employer( "RedHat" ); + session.persist( employer ); + employer.addEmployee( new Employee( "Jack" ) ); + employer.addEmployee( new Employee( "Jill" ) ); + employer.addEmployee( new Employee( "John" ) ); + for ( Employee employee : employer.getEmployees() ) { + session.persist( employee ); + } + } + ); + + doInHibernate( + this::sessionFactory, session -> { + Employer employer = session.createQuery( "from Employer e", Employer.class ).getSingleResult(); + session.remove( employer ); + for ( Employee employee : employer.getEmployees() ) { + assertFalse( Hibernate.isPropertyInitialized( employee, "employer") ); + session.remove( employee ); + assertTrue( Hibernate.isPropertyInitialized( employee, "employer" ) ); + } + } + ); + + doInHibernate( + this::sessionFactory, session -> { + assertNull( session.find( Employer.class, "RedHat" ) ); + assertNull( session.createQuery( "from Employee e", Employee.class ).uniqueResult() ); + } + ); + } + + @Entity(name = "Employer") + public static class Employer { + private String name; + + private Set employees; + + public Employer(String name) { + this(); + setName( name ); + } + + @Id + public String getName() { + return name; + } + + @OneToMany(mappedBy = "employer", fetch = FetchType.LAZY) + @LazyGroup("Employees") + public Set getEmployees() { + return employees; + } + + public void addEmployee(Employee employee) { + if ( getEmployees() == null ) { + setEmployees( new HashSet<>() ); + } + employees.add( employee ); + employee.setEmployer( this ); + } + + protected Employer() { + // this form used by Hibernate + } + + protected void setName(String name) { + this.name = name; + } + + protected void setEmployees(Set employees) { + this.employees = employees; + } + } + + @Entity(name = "Employee") + public static class Employee { + private long id; + + private String name; + + private Employer employer; + + public Employee(String name) { + this(); + setName( name ); + } + + @Id + @GeneratedValue + public long getId() { + return id; + } + + public String getName() { + return name; + } + + @ManyToOne(fetch = FetchType.LAZY) + @LazyToOne(LazyToOneOption.NO_PROXY) + @LazyGroup("EmployerForEmployee") + @JoinColumn(name = "employer_name") + public Employer getEmployer() { + return employer; + } + + protected Employee() { + // this form used by Hibernate + } + + protected void setId(long id) { + this.id = id; + } + + protected void setName(String name) { + this.name = name; + } + + protected void setEmployer(Employer employer) { + this.employer = employer; + } + + public int hashCode() { + if ( name != null ) { + return name.hashCode(); + } + else { + return 0; + } + } + + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + else if ( o instanceof Employee ) { + Employee other = Employee.class.cast( o ); + if ( name != null ) { + return getName().equals( other.getName() ); + } + else { + return other.getName() == null; + } + } + else { + return false; + } + } + } + + public static class NoDirtyCheckEnhancementContext extends DefaultEnhancementContext { + @Override + public boolean doDirtyCheckingInline(UnloadedClass classDescriptor) { + return false; + } + } +} From fc80c350a9b5068771b0fb9a0ec1ee814fbebab4 Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Mon, 3 Dec 2018 15:27:53 +0100 Subject: [PATCH 2/5] HHH-13138 By default, pass the class loader of the test to the EMF Not doing it causes issues when using the BytecodeEnhancerRunner which introduces an enhancing class loader. We could do it on a per test basis but it's easier to do it once and for all. And it can still be overridden anyway. (cherry picked from commit bae98ffaccf55c78bb1eb39b1e249f9b8ae92726) --- .../jpa/test/BaseEntityManagerFunctionalTestCase.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hibernate-core/src/test/java/org/hibernate/jpa/test/BaseEntityManagerFunctionalTestCase.java b/hibernate-core/src/test/java/org/hibernate/jpa/test/BaseEntityManagerFunctionalTestCase.java index 7a0ecd4ca91b..4f0648baac3e 100644 --- a/hibernate-core/src/test/java/org/hibernate/jpa/test/BaseEntityManagerFunctionalTestCase.java +++ b/hibernate-core/src/test/java/org/hibernate/jpa/test/BaseEntityManagerFunctionalTestCase.java @@ -14,6 +14,7 @@ import java.util.List; import java.util.Map; import java.util.Properties; + import javax.persistence.EntityManager; import javax.persistence.SharedCacheMode; import javax.persistence.ValidationMode; @@ -21,20 +22,17 @@ import org.hibernate.boot.registry.internal.StandardServiceRegistryImpl; import org.hibernate.bytecode.enhance.spi.EnhancementContext; +import org.hibernate.cfg.AvailableSettings; import org.hibernate.cfg.Environment; import org.hibernate.dialect.Dialect; import org.hibernate.engine.spi.SessionFactoryImplementor; -import org.hibernate.jpa.AvailableSettings; import org.hibernate.jpa.HibernatePersistenceProvider; import org.hibernate.jpa.boot.spi.Bootstrap; import org.hibernate.jpa.boot.spi.PersistenceUnitDescriptor; - import org.hibernate.testing.junit4.BaseUnitTestCase; import org.junit.After; import org.junit.Before; -import org.jboss.logging.Logger; - /** * A base class for all ejb tests. * @@ -209,6 +207,8 @@ protected Map getConfig() { Map config = Environment.getProperties(); ArrayList classes = new ArrayList(); + config.put( AvailableSettings.CLASSLOADERS, getClass().getClassLoader() ); + classes.addAll( Arrays.asList( getAnnotatedClasses() ) ); config.put( AvailableSettings.LOADED_CLASSES, classes ); for ( Map.Entry entry : getCachedClasses().entrySet() ) { From fd66ba0e7fa14daaf1e0e82eca84f5fa619c518a Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Mon, 3 Dec 2018 19:06:44 +0100 Subject: [PATCH 3/5] HHH-13138 Set the TCCL in BytecodeEnhancerRunner We are not consistently using the ClassLoaderService and we sometimes use the TCCL so better set it correctly. (cherry picked from commit 2a8582be7f114565fa1fcf8b16a85b721d88d47c) --- .../enhancement/BytecodeEnhancerRunner.java | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/bytecode/enhancement/BytecodeEnhancerRunner.java b/hibernate-testing/src/main/java/org/hibernate/testing/bytecode/enhancement/BytecodeEnhancerRunner.java index 6de2bad33354..4801752bc50c 100644 --- a/hibernate-testing/src/main/java/org/hibernate/testing/bytecode/enhancement/BytecodeEnhancerRunner.java +++ b/hibernate-testing/src/main/java/org/hibernate/testing/bytecode/enhancement/BytecodeEnhancerRunner.java @@ -6,23 +6,25 @@ */ package org.hibernate.testing.bytecode.enhancement; +import java.io.BufferedInputStream; +import java.io.File; +import java.io.FileOutputStream; +import java.io.InputStream; +import java.lang.reflect.InvocationTargetException; +import java.util.ArrayList; +import java.util.List; + import org.hibernate.bytecode.enhance.spi.EnhancementContext; import org.hibernate.bytecode.enhance.spi.Enhancer; import org.hibernate.cfg.Environment; import org.hibernate.testing.junit4.CustomRunner; import org.junit.runner.Runner; +import org.junit.runner.notification.RunNotifier; +import org.junit.runners.ParentRunner; import org.junit.runners.Suite; import org.junit.runners.model.InitializationError; import org.junit.runners.model.RunnerBuilder; -import java.io.BufferedInputStream; -import java.io.File; -import java.io.FileOutputStream; -import java.io.InputStream; -import java.lang.reflect.InvocationTargetException; -import java.util.ArrayList; -import java.util.List; - /** * @author Luis Barreiro */ @@ -111,4 +113,22 @@ public Class loadClass(String name) throws ClassNotFoundException { }; } + @Override + protected void runChild(Runner runner, RunNotifier notifier) { + // This is ugly but, for now, ORM class loading is inconsistent. + // It sometimes use ClassLoaderService which takes into account AvailableSettings.CLASSLOADERS, and sometimes + // ReflectHelper#classForName() which uses the TCCL. + // See https://hibernate.atlassian.net/browse/HHH-13136 for more information. + ClassLoader originalClassLoader = Thread.currentThread().getContextClassLoader(); + try { + Thread.currentThread().setContextClassLoader( + ( (ParentRunner) runner ).getTestClass().getJavaClass().getClassLoader() ); + + super.runChild( runner, notifier ); + } + finally { + Thread.currentThread().setContextClassLoader( originalClassLoader ); + } + } + } From 8539cc202f47221d8609ab9b59a7ad3ed5740e87 Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Wed, 20 Mar 2019 17:26:32 -0700 Subject: [PATCH 4/5] HHH-13241 : Added test case with a lazy null many-to-one association (cherry picked from commit 65eebbb96b2a5557867994c27b382e9da6f22b12) --- .../lazy/BidirectionalLazyTest.java | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/BidirectionalLazyTest.java b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/BidirectionalLazyTest.java index 867d965be50d..ed6e755a17fe 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/BidirectionalLazyTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/bytecode/enhancement/lazy/BidirectionalLazyTest.java @@ -173,6 +173,40 @@ public void testRemoveWithNoRemovedEntities() { ); } + @Test + public void testRemoveEntityWithNullLazyManyToOne() { + + doInHibernate( + this::sessionFactory, session -> { + Employer employer = new Employer( "RedHat" ); + session.persist( employer ); + Employee employee = new Employee( "Jack" ); + session.persist( employee ); + } + ); + + doInHibernate( + this::sessionFactory, session -> { + Employee employee = session.get( Employee.class, "Jack" ); + + // Get and delete an Employer that is not associated with employee + Employer employer = session.get( Employer.class, "RedHat" ); + session.remove( employer ); + + // employee.employer is uninitialized. Since the column for employee.employer + // is a foreign key, and there is an Employer that has already been removed, + // employee.employer will need to be iniitialized to determine if + // employee.employee is nullifiable. + assertFalse( Hibernate.isPropertyInitialized( employee, "employer" ) ); + session.remove( employee ); + assertTrue( Hibernate.isPropertyInitialized( employee, "employer" ) ); + } + ); + + + + } + private void checkEntityEntryState( final Session session, final Employee employee, From c0216b70d6494dba195d28ca2e9382123d96e4c1 Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Wed, 20 Mar 2019 17:27:32 -0700 Subject: [PATCH 5/5] HHH-13241 : Fix regression with an uninitialized null many-to-one association (cherry picked from commit b28dc488a11f580ebb8128d620cc01646d832343) --- .../hibernate/engine/internal/ForeignKeys.java | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/ForeignKeys.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/ForeignKeys.java index 94e05a7b910f..78f6b97cc3b4 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/ForeignKeys.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/ForeignKeys.java @@ -100,11 +100,17 @@ else if ( type.isEntityType() ) { // If value is lazy, it may need to be initialized to // determine if the value is nullifiable. final Object possiblyInitializedValue = initializeIfNecessary( value, propertyName, entityType ); - // If the value is not nullifiable, make sure that the - // possibly initialized value is returned. - returnedValue = isNullifiable( entityType.getAssociatedEntityName(), possiblyInitializedValue ) - ? null - : possiblyInitializedValue; + if ( possiblyInitializedValue == null ) { + // The uninitialized value was initialized to null + returnedValue = null; + } + else { + // If the value is not nullifiable, make sure that the + // possibly initialized value is returned. + returnedValue = isNullifiable( entityType.getAssociatedEntityName(), possiblyInitializedValue ) + ? null + : possiblyInitializedValue; + } } } else if ( type.isAnyType() ) {