Fix BiDirectionalFetchImpl CircularFetchAssembler#assemble for collec…

…tion initialization
hibernate · Feb 17, 2020 · 484589c · 484589c
1 parent 7de3be2
commit 484589c
Show file tree

Hide file tree

Showing 5 changed files with 94 additions and 6 deletions.
diff --git a/...-core/src/main/java/org/hibernate/sql/results/graph/collection/CollectionInitializer.java b/...-core/src/main/java/org/hibernate/sql/results/graph/collection/CollectionInitializer.java
@@ -7,10 +7,12 @@
 package org.hibernate.sql.results.graph.collection;
 
 import org.hibernate.collection.spi.PersistentCollection;
+import org.hibernate.engine.spi.CollectionKey;
 import org.hibernate.sql.results.graph.Initializer;
 import org.hibernate.metamodel.mapping.PluralAttributeMapping;
 import org.hibernate.persister.collection.CollectionPersister;
 import org.hibernate.sql.exec.spi.ExecutionContext;
+import org.hibernate.sql.results.jdbc.spi.RowProcessingState;
 
 /**
  * Initializer implementation for initializing collections (plural attributes)
@@ -38,4 +40,6 @@ default Object getInitializedInstance() {
 	default void endLoading(ExecutionContext context) {
 		// by default - nothing to do
 	}
+
+	CollectionKey resolveCollectionKey(RowProcessingState rowProcessingState);
 }
diff --git a/...va/org/hibernate/sql/results/graph/collection/internal/AbstractCollectionInitializer.java b/...va/org/hibernate/sql/results/graph/collection/internal/AbstractCollectionInitializer.java
@@ -112,6 +112,7 @@ protected Object getKeyCollectionValue() {
 		return keyCollectionValue;
 	}
 
+	@Override
 	public CollectionKey resolveCollectionKey(RowProcessingState rowProcessingState) {
 		resolveKey( rowProcessingState );
 		return collectionKey;

diff --git a/.../java/org/hibernate/sql/results/graph/collection/internal/DelayedCollectionAssembler.java b/.../java/org/hibernate/sql/results/graph/collection/internal/DelayedCollectionAssembler.java
@@ -153,5 +153,11 @@ public PluralAttributeMapping getInitializedPart() {
 		public PersistentCollection getCollectionInstance() {
 			return instance;
 		}
+
+		@Override
+		public CollectionKey resolveCollectionKey(RowProcessingState rowProcessingState) {
+			resolveKey( rowProcessingState );
+			return collectionKey;
+		}
 	}
 }
diff --git a/...-core/src/main/java/org/hibernate/sql/results/internal/domain/BiDirectionalFetchImpl.java b/...-core/src/main/java/org/hibernate/sql/results/internal/domain/BiDirectionalFetchImpl.java
@@ -12,12 +12,19 @@
 import org.hibernate.NotYetImplementedFor6Exception;
 import org.hibernate.engine.FetchStrategy;
 import org.hibernate.engine.FetchTiming;
+import org.hibernate.engine.spi.CollectionKey;
+import org.hibernate.engine.spi.EntityKey;
+import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.metamodel.mapping.Association;
+import org.hibernate.metamodel.mapping.AttributeMapping;
 import org.hibernate.metamodel.mapping.EntityMappingType;
+import org.hibernate.metamodel.mapping.EntityValuedModelPart;
 import org.hibernate.metamodel.mapping.ForeignKeyDescriptor;
 import org.hibernate.metamodel.mapping.MappingType;
 import org.hibernate.metamodel.mapping.ModelPart;
+import org.hibernate.metamodel.mapping.internal.SingularAssociationAttributeMapping;
 import org.hibernate.metamodel.model.domain.NavigableRole;
+import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.query.NavigablePath;
 import org.hibernate.sql.results.graph.AssemblerCreationState;
 import org.hibernate.sql.results.graph.BiDirectionalFetch;
@@ -28,7 +35,11 @@
 import org.hibernate.sql.results.graph.FetchParentAccess;
 import org.hibernate.sql.results.graph.Fetchable;
 import org.hibernate.sql.results.graph.Initializer;
+import org.hibernate.sql.results.graph.collection.CollectionInitializer;
+import org.hibernate.sql.results.graph.collection.internal.AbstractCollectionInitializer;
 import org.hibernate.sql.results.graph.entity.EntityInitializer;
+import org.hibernate.sql.results.graph.entity.LoadingEntityEntry;
+import org.hibernate.sql.results.graph.entity.internal.EntityResultImpl;
 import org.hibernate.sql.results.jdbc.spi.JdbcValuesSourceProcessingOptions;
 import org.hibernate.sql.results.jdbc.spi.RowProcessingState;
 import org.hibernate.type.descriptor.java.JavaTypeDescriptor;
@@ -93,6 +104,7 @@ public DomainResultAssembler createAssembler(
 			Consumer<Initializer> collector,
 			AssemblerCreationState creationState) {
 		return new CircularFetchAssembler(
+				fetchable,
 				getReferencedPath(),
 				fetchable.getJavaTypeDescriptor()
 		);
@@ -170,17 +182,38 @@ public Fetch generateFetch(
 	private static class CircularFetchAssembler implements DomainResultAssembler {
 		private final NavigablePath circularPath;
 		private final JavaTypeDescriptor javaTypeDescriptor;
+		private final Fetchable fetchable;
 
 		public CircularFetchAssembler(
+				Fetchable fetchable,
 				NavigablePath circularPath,
 				JavaTypeDescriptor javaTypeDescriptor) {
+			this.fetchable = fetchable;
 			this.circularPath = circularPath;
 			this.javaTypeDescriptor = javaTypeDescriptor;
 		}
 
 		@Override
 		public Object assemble(RowProcessingState rowProcessingState, JdbcValuesSourceProcessingOptions options) {
 			final EntityInitializer initializer = resolveCircularInitializer( rowProcessingState );
+			if ( initializer == null ) {
+
+				final Initializer parentInitializer = rowProcessingState.resolveInitializer(
+						circularPath.getParent() );
+				assert parentInitializer instanceof CollectionInitializer;
+				final CollectionInitializer circ = (CollectionInitializer) parentInitializer;
+				final CollectionKey collectionKey = circ.resolveCollectionKey( rowProcessingState );
+				final EntityKey entityKey = new EntityKey(
+						collectionKey.getKey(),
+						(EntityPersister) ( (AttributeMapping) fetchable ).getMappedTypeDescriptor()
+				);
+
+				final SharedSessionContractImplementor session = rowProcessingState.getJdbcValuesSourceProcessingState()
+						.getSession();
+				return session.getPersistenceContext()
+						.getEntity( entityKey );
+
+			}
 			if ( initializer.getInitializedInstance() == null ) {
 				initializer.resolveKey( rowProcessingState );
 				initializer.resolveInstance( rowProcessingState );
@@ -199,9 +232,14 @@ private EntityInitializer resolveCircularInitializer(RowProcessingState rowProce
 
 			NavigablePath path = circularPath.getParent();
 			Initializer parentInitializer = rowProcessingState.resolveInitializer( path );
-			while ( ! ( parentInitializer instanceof EntityInitializer ) ) {
+			while ( !( parentInitializer instanceof EntityInitializer) && path.getParent() != null ) {
 				path = path.getParent();
 				parentInitializer = rowProcessingState.resolveInitializer( path );
+
+			}
+
+			if ( !( parentInitializer instanceof EntityInitializer ) ) {
+				return null;
 			}
 
 			return (EntityInitializer) parentInitializer;

diff --git a/.../test/java/org/hibernate/orm/test/loading/multiLoad/MultiLoadSubSelectCollectionTest.java b/.../test/java/org/hibernate/orm/test/loading/multiLoad/MultiLoadSubSelectCollectionTest.java
@@ -48,13 +48,9 @@
 		MultiLoadSubSelectCollectionTest.Child.class
 })
 @ServiceRegistry
-@SessionFactory
+@SessionFactory(generateStatistics = true)
 public class MultiLoadSubSelectCollectionTest {
 
-	protected void addSettings(Map settings) {
-		settings.put( AvailableSettings.GENERATE_STATISTICS, "true" );
-	}
-
 	@BeforeEach
 	public void before(SessionFactoryScope scope) {
 		scope.inTransaction( session -> {
@@ -118,6 +114,49 @@ public void testSubselect(SessionFactoryScope scope) {
 		);
 	}
 
+	@Test
+	@TestForIssue(jiraKey = "HHH-12740")
+	public void testSubselect_2(SessionFactoryScope scope) {
+		scope.inTransaction(
+				session -> {
+					List<Parent> list = session.byMultipleIds( Parent.class ).multiLoad( ids( 1 ) );
+					assertEquals( 1, list.size() );
+
+					// None of the collections should be loaded yet
+					for ( Parent p : list ) {
+						assertFalse( Hibernate.isInitialized( p.children ) );
+					}
+
+					Hibernate.initialize( list.get( 0 ).children );
+
+
+//					// When the first collection is loaded, the full batch of 50 collections
+//					// should be loaded.
+//					Hibernate.initialize( list.get( 0 ).children );
+//
+//					for ( int i = 0; i < 50; i++ ) {
+//						assertTrue( Hibernate.isInitialized( list.get( i ).children ) );
+//						assertEquals( i + 1, list.get( i ).children.size() );
+//					}
+//
+//					// The collections for the 51st through 56th entities should still be uninitialized
+//					for ( int i = 50; i < 56; i++ ) {
+//						assertFalse( Hibernate.isInitialized( list.get( i ).children ) );
+//					}
+//
+//					// When the 51st collection gets initialized, the remaining collections should
+//					// also be initialized.
+//					Hibernate.initialize( list.get( 50 ).children );
+//
+//					for ( int i = 50; i < 56; i++ ) {
+//						assertTrue( Hibernate.isInitialized( list.get( i ).children ) );
+//						assertEquals( i + 1, list.get( i ).children.size() );
+//					}
+				}
+		);
+	}
+
+
 	private Integer[] ids(int count) {
 		Integer[] ids = new Integer[count];
 		for ( int i = 1; i <= count; i++ ) {