HHH-15473 Don't confuse property initialization for collection initialization in tests

Checking for property initialization was acceptable and pretty
much the only way to perform assertions before we fixed HHH-14620,
but now that initializing a property doesn't imply initializing
the collection, it's just plain wrong:

* If you expect the collection *not* to be initialized, then
  checking that that the property is not initialized is too strict:
  the assertion could fail because the property is initialized
  while the collection is not initialized.
* If you expect the collection to be initialized, then
  checking that that the property is initialized is not enough:
  the assertion could pass because the property is initialized
  while the collection is not initialized.

Besides, we can safely call the getter to test the collection
directly with Hibernate.isInitialized(entity.getCollection())
since a call to the getter is not supposed to trigger collection
initialization.
This commit is contained in:
Yoann Rodière 2022-09-01 15:07:02 +02:00 committed by Sanne Grinovero
parent 560722dfaa
commit febfd9d4b8
6 changed files with 45 additions and 68 deletions

View File

@ -72,7 +72,7 @@ public void testManagedWithUninitializedAssociation() {
.setParameter( "name", "PARENT" )
.uniqueResult();
checkInterceptor( loadedParent, false );
assertFalse( Hibernate.isPropertyInitialized( loadedParent, "children" ) );
assertFalse( Hibernate.isInitialized( loadedParent.getChildren() ) );
s.delete( loadedParent );
} );
// If the lazy relation is not fetch on cascade there is a constraint violation on commit
@ -87,8 +87,8 @@ public void testManagedWithInitializedAssociation() {
.setParameter( "name", "PARENT" )
.uniqueResult();
checkInterceptor( loadedParent, false );
loadedParent.getChildren();
assertTrue( Hibernate.isPropertyInitialized( loadedParent, "children" ) );
loadedParent.getChildren().size();
assertTrue( Hibernate.isInitialized( loadedParent.getChildren() ) );
s.delete( loadedParent );
} );
// If the lazy relation is not fetch on cascade there is a constraint violation on commit
@ -101,7 +101,7 @@ public void testDetachedWithUninitializedAssociation() {
return s.get( Parent.class, originalParent.getId() );
} );
assertFalse( Hibernate.isPropertyInitialized( detachedParent, "children" ) );
assertFalse( Hibernate.isInitialized( detachedParent.getChildren() ) );
checkInterceptor( detachedParent, false );
@ -117,14 +117,12 @@ public void testDetachedWithUninitializedAssociation() {
public void testDetachedWithInitializedAssociation() {
final Parent detachedParent = doInHibernate( this::sessionFactory, s -> {
Parent parent = s.get( Parent.class, originalParent.getId() );
assertFalse( Hibernate.isPropertyInitialized( parent, "children" ) );
// initialize collection before detaching
parent.getChildren();
parent.getChildren().size();
return parent;
} );
assertTrue( Hibernate.isPropertyInitialized( detachedParent, "children" ) );
assertTrue( Hibernate.isInitialized( detachedParent.getChildren() ) );
checkInterceptor( detachedParent, false );
@ -192,7 +190,7 @@ void setId(Long id) {
@OneToMany( mappedBy = "parent", cascade = {CascadeType.PERSIST, CascadeType.MERGE, CascadeType.REMOVE}, fetch = FetchType.LAZY )
List<Child> getChildren() {
return Collections.unmodifiableList( children );
return children;
}
void setChildren(List<Child> children) {

View File

@ -153,8 +153,8 @@ public void testMergeDetachedEnhancedEntityWithUninitializedOneToMany() {
}
);
// address should not be initialized
assertFalse( Hibernate.isPropertyInitialized( detachedPerson, "addresses" ) );
// address should not be initialized in order to reproduce the problem
assertFalse( Hibernate.isInitialized( detachedPerson.getAddresses() ) );
detachedPerson.setName( "newName" );
Person mergedPerson = TransactionUtil.doInHibernate(
@ -163,8 +163,8 @@ public void testMergeDetachedEnhancedEntityWithUninitializedOneToMany() {
}
);
// address should be initialized
assertTrue( Hibernate.isPropertyInitialized( mergedPerson, "addresses" ) );
// address still shouldn't be initialized: there's no reason for it to be initialized by a merge.
assertFalse( Hibernate.isInitialized( detachedPerson.getAddresses() ) );
assertEquals( "newName", mergedPerson.getName() );
}
@ -179,8 +179,8 @@ public void testDeleteEnhancedEntityWithUninitializedOneToMany() {
}
);
// address should not be initialized
assertFalse( Hibernate.isPropertyInitialized( detachedPerson, "addresses" ) );
// address should not be initialized in order to reproduce the problem
assertFalse( Hibernate.isInitialized( detachedPerson.getAddresses() ) );
// deleting detachedPerson should result in detachedPerson.address being initialized,
// so that the DELETE operation can be cascaded to it.

View File

@ -19,15 +19,11 @@
import jakarta.persistence.OneToMany;
import jakarta.persistence.Table;
import org.hibernate.Hibernate;
import org.hibernate.boot.internal.SessionFactoryBuilderImpl;
import org.hibernate.boot.internal.SessionFactoryOptionsBuilder;
import org.hibernate.boot.spi.SessionFactoryBuilderService;
import org.hibernate.bytecode.enhance.spi.interceptor.BytecodeLazyAttributeInterceptor;
import org.hibernate.bytecode.enhance.spi.interceptor.LazyAttributeLoadingInterceptor;
import org.hibernate.cfg.Configuration;
import org.hibernate.collection.spi.PersistentCollection;
import org.hibernate.engine.spi.PersistentAttributeInterceptable;
import org.hibernate.engine.spi.SelfDirtinessTracker;
import org.hibernate.testing.TestForIssue;
import org.hibernate.testing.bytecode.enhancement.BytecodeEnhancerRunner;
@ -38,7 +34,6 @@
import static org.hibernate.testing.transaction.TransactionUtil.doInJPA;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
/**
* @author Christian Beikov
@ -83,11 +78,8 @@ public void test() {
doInJPA( this::sessionFactory, entityManager -> {
StringsEntity entity = entityManager.find( StringsEntity.class, 1L );
entityManager.flush();
BytecodeLazyAttributeInterceptor interceptor = (BytecodeLazyAttributeInterceptor) ( (PersistentAttributeInterceptable) entity )
.$$_hibernate_getInterceptor();
assertTrue( interceptor.hasAnyUninitializedAttributes() );
assertFalse( interceptor.isAttributeLoaded( "someStrings" ) );
assertFalse( interceptor.isAttributeLoaded( "someStringEntities" ) );
assertFalse( Hibernate.isInitialized( entity.someStrings ) );
assertFalse( Hibernate.isInitialized( entity.someStringEntities ) );
} );
}

View File

@ -10,7 +10,7 @@
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hibernate.Hibernate.isPropertyInitialized;
import static org.hibernate.Hibernate.isInitialized;
import static org.hibernate.testing.bytecode.enhancement.EnhancerTestUtils.checkDirtyTracking;
import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
import static org.junit.Assert.assertFalse;
@ -70,7 +70,7 @@ public void testDetach() {
assertThat( parent, notNullValue() );
assertThat( parent, not( instanceOf( HibernateProxy.class ) ) );
assertFalse( isPropertyInitialized( parent, "children" ) );
assertFalse( isInitialized( parent.children ) );
checkDirtyTracking( parent );
s.detach( parent );
@ -99,7 +99,7 @@ public void testRefresh() {
assertThat( parent, notNullValue() );
assertThat( parent, not( instanceOf( HibernateProxy.class ) ) );
assertFalse( isPropertyInitialized( parent, "children" ) );
assertFalse( isInitialized( parent.children ) );
checkDirtyTracking( parent );
s.refresh( parent );

View File

@ -49,6 +49,7 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.junit.jupiter.api.Assertions.assertFalse;
/**
* @author Andrea Boriero
@ -117,8 +118,6 @@ public void updateSimpleField() {
.getMappingMetamodel()
.getEntityDescriptor( Child.class.getName() );
final int relativesAttributeIndex = childPersister.getEntityMetamodel().getPropertyIndex( "relatives" );
inTransaction(
session -> {
stats.clear();
@ -127,7 +126,6 @@ public void updateSimpleField() {
final PersistentAttributeInterceptable interceptable = (PersistentAttributeInterceptable) loadedChild;
final PersistentAttributeInterceptor interceptor = interceptable.$$_hibernate_getInterceptor();
MatcherAssert.assertThat( interceptor, instanceOf( EnhancementAsProxyLazinessInterceptor.class ) );
final EnhancementAsProxyLazinessInterceptor proxyInterceptor = (EnhancementAsProxyLazinessInterceptor) interceptor;
loadedChild.setName( updatedName );
@ -139,21 +137,15 @@ public void updateSimpleField() {
assertEquals( 1, stats.getPrepareStatementCount() );
final EntityEntry entry = session.getPersistenceContext().getEntry( loadedChild );
assertThat(
entry.getLoadedState()[ relativesAttributeIndex ],
is( LazyPropertyInitializer.UNFETCHED_PROPERTY )
);
assertFalse( Hibernate.isInitialized( loadedChild.getRelatives() ) );
// force a flush - the relatives collection should still be UNFETCHED_PROPERTY afterwards
// force a flush - the relatives collection should still be uninitialized afterwards
session.flush();
final EntityEntry updatedEntry = session.getPersistenceContext().getEntry( loadedChild );
assertThat( updatedEntry, sameInstance( entry ) );
assertThat(
entry.getLoadedState()[ relativesAttributeIndex ],
is( LazyPropertyInitializer.UNFETCHED_PROPERTY )
);
assertFalse( Hibernate.isInitialized( loadedChild.getRelatives() ) );
session.getEventListenerManager();
}
@ -164,11 +156,7 @@ public void updateSimpleField() {
Child loadedChild = session.load( Child.class, lastChildID );
assertThat( loadedChild.getName(), is( updatedName ) );
final EntityEntry entry = session.getPersistenceContext().getEntry( loadedChild );
assertThat(
entry.getLoadedState()[ relativesAttributeIndex ],
is( LazyPropertyInitializer.UNFETCHED_PROPERTY )
);
assertFalse( Hibernate.isInitialized( loadedChild.getRelatives() ) );
}
);

View File

@ -34,6 +34,7 @@
import java.util.Collections;
import java.util.List;
import static org.hibernate.Hibernate.isInitialized;
import static org.hibernate.Hibernate.isPropertyInitialized;
import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate;
import static org.junit.Assert.assertEquals;
@ -106,28 +107,27 @@ public void testClearedSession() {
// first load the store, making sure collection is not initialized
Store store = s.get( Store.class, 1L );
assertNotNull( store );
assertFalse( isPropertyInitialized( store, "inventories" ) );
assertFalse( isInitialized( store.getInventories() ) );
assertEquals( 1, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 0, sessionFactory().getStatistics().getSessionCloseCount() );
// then clear session and try to initialize collection
s.clear();
assertNotNull( store );
assertFalse( isPropertyInitialized( store, "inventories" ) );
assertFalse( isInitialized( store.getInventories() ) );
store.getInventories().size();
assertTrue( isPropertyInitialized( store, "inventories" ) );
assertTrue( isInitialized( store.getInventories() ) );
// the extra Sessions are the temp Sessions needed to perform the init:
// first the entity, then the collection (since it's lazy)
assertEquals( 3, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 2, sessionFactory().getStatistics().getSessionCloseCount() );
// the extra Session is the temp Sessions needed to perform the collection init (since it's lazy)
assertEquals( 2, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 1, sessionFactory().getStatistics().getSessionCloseCount() );
// clear Session again. The collection should still be recognized as initialized from above
s.clear();
assertNotNull( store );
assertTrue( isPropertyInitialized( store, "inventories" ) );
assertEquals( 3, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 2, sessionFactory().getStatistics().getSessionCloseCount() );
assertTrue( isInitialized( store.getInventories() ) );
assertEquals( 2, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 1, sessionFactory().getStatistics().getSessionCloseCount() );
// lets clear the Session again and this time reload the Store
s.clear();
@ -136,23 +136,22 @@ public void testClearedSession() {
assertNotNull( store );
// collection should be back to uninitialized since we have a new entity instance
assertFalse( isPropertyInitialized( store, "inventories" ) );
assertFalse( isInitialized( store.getInventories() ) );
assertEquals( 2, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 1, sessionFactory().getStatistics().getSessionCloseCount() );
store.getInventories().size();
assertTrue( isInitialized( store.getInventories() ) );
// the extra Session is the temp Sessions needed to perform the collection init (since it's lazy)
assertEquals( 3, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 2, sessionFactory().getStatistics().getSessionCloseCount() );
store.getInventories().size();
assertTrue( isPropertyInitialized( store, "inventories" ) );
// the extra Sessions are the temp Sessions needed to perform the init:
// first the entity, then the collection (since it's lazy)
assertEquals( 5, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 4, sessionFactory().getStatistics().getSessionCloseCount() );
// clear Session again. The collection should still be recognized as initialized from above
s.clear();
assertNotNull( store );
assertTrue( isPropertyInitialized( store, "inventories" ) );
assertEquals( 5, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 4, sessionFactory().getStatistics().getSessionCloseCount() );
assertTrue( isInitialized( store.getInventories() ) );
assertEquals( 3, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 2, sessionFactory().getStatistics().getSessionCloseCount() );
} );
}
@ -202,7 +201,7 @@ Inventory addInventoryProduct(Product product) {
}
public List<Inventory> getInventories() {
return Collections.unmodifiableList( inventories );
return inventories;
}
}