HHH-7841 - Redesign Loader

hibernate-core/src/main/java/org/hibernate/loader/plan/spi/EntityFetch.java

@@ -60,9 +60,6 @@ public class EntityFetch extends AbstractSingularAttributeFetch implements Entit
 		super( sessionFactory, lockMode, owner, ownerProperty, fetchStrategy );
 
 		this.associationType = entityType;
-		//		(EntityType) owner.retrieveFetchSourcePersister().getPropertyType( ownerProperty );
-		//this.associationType =
-		//		(EntityType) owner.retrieveFetchSourcePersister().getPropertyType( getPropertyPath().getFullPath() );
 		this.persister = sessionFactory.getEntityPersister( associationType.getAssociatedEntityName() );
 	}
 

hibernate-core/src/main/java/org/hibernate/loader/plan/spi/build/AbstractLoadPlanBuilderStrategy.java

@@ -85,8 +85,6 @@ public abstract class AbstractLoadPlanBuilderStrategy implements LoadPlanBuilder
 	private ArrayDeque<FetchOwner> fetchOwnerStack = new ArrayDeque<FetchOwner>();
 	private ArrayDeque<CollectionReference> collectionReferenceStack = new ArrayDeque<CollectionReference>();
 
-	//private AbstractIdentifierAttributeCollector currentIdentifierAttributeCollector = null;
-
 	protected AbstractLoadPlanBuilderStrategy(SessionFactoryImplementor sessionFactory) {
 		this.sessionFactory = sessionFactory;
 	}
@@ -121,7 +119,6 @@ public abstract class AbstractLoadPlanBuilderStrategy implements LoadPlanBuilder
 		MDC.remove( MDC_KEY );
 		fetchOwnerStack.clear();
 		collectionReferenceStack.clear();
-//		currentIdentifierAttributeCollector = null;
 	}
 
 	@Override
@@ -418,21 +415,10 @@ public abstract class AbstractLoadPlanBuilderStrategy implements LoadPlanBuilder
 			);
 		}
 
-		// If we are collecting fetches for the identifier then
-		// currentIdentifierAttributeCollector will be non-null.
-		// In that case, we do not want to continue walking the association, so
-		// don't push associationFetch to the stack.
-		//final boolean continueWalk = currentIdentifierAttributeCollector == null;
-		//if ( continueWalk && FetchOwner.class.isInstance( associationFetch) ) {
 		if ( FetchOwner.class.isInstance( associationFetch) ) {
 			pushToStack( (FetchOwner) associationFetch );
 		}
 
-		//if ( ! continueWalk ) {
-		//	popFromStack();
-		//}
-
-		//return continueWalk;
 		return true;
 	}
 
@@ -447,21 +433,7 @@ public abstract class AbstractLoadPlanBuilderStrategy implements LoadPlanBuilder
 	}
 
 	private void pushToStack(FetchOwner fetchOwner) {
-//		if ( fetchOwner instanceof AbstractIdentifierAttributeCollector ) {
-//			if ( currentIdentifierAttributeCollector != null ) {
-//				throw new WalkingException(
-//						String.format(
-//								"An AbstractIdentifierAttributeCollector is already being processed: %s",
-//								currentIdentifierAttributeCollector
-//						)
-//				);
-//			}
-//			currentIdentifierAttributeCollector = (AbstractIdentifierAttributeCollector) fetchOwner;
-//			log.trace( "Pushing AbstractIdentifierAttributeCollector fetch owner to stack : " + fetchOwner );
-//		}
-//		else {
 		log.trace( "Pushing fetch owner to stack : " + fetchOwner );
-//		}
 		mdcStack().push( fetchOwner.getPropertyPath() );
 		fetchOwnerStack.addFirst( fetchOwner );
 	}
@@ -472,30 +444,7 @@ public abstract class AbstractLoadPlanBuilderStrategy implements LoadPlanBuilder
 
 	private FetchOwner popFromStack() {
 		final FetchOwner last = fetchOwnerStack.removeFirst();
-//		if ( last instanceof AbstractIdentifierAttributeCollector ) {
-//			if ( currentIdentifierAttributeCollector == null ) {
-//				throw new WalkingException(
-//						String.format(
-//								"Popped fetch owner was an AbstractIdentifierAttributeCollector [%s], but none in process (currentIdentifierAttributeCollector == null)",
-//								last
-//						)
-//				);
-//			}
-//			else if ( currentIdentifierAttributeCollector != last ) {
-//				throw new WalkingException(
-//						String.format(
-//								"Expected popped fetch owner to be [%s], but instead it was [%s])",
-//								currentIdentifierAttributeCollector,
-//								last
-//						)
-//				);
-//			}
-//			currentIdentifierAttributeCollector = null;
-//			log.trace( "Popped AbstractIdentifierAttributeCollector fetch owner from stack : " + last );
-//		}
-//		else {
 		log.trace( "Popped fetch owner from stack : " + last );
-//		}
 		mdcStack().pop();
 		if ( FetchStackAware.class.isInstance( last ) ) {
 			( (FetchStackAware) last ).poppedFromStack();

hibernate-core/src/main/java/org/hibernate/persister/walking/spi/MetadataDrivenModelGraphVisitor.java

@@ -123,10 +123,10 @@ public class MetadataDrivenModelGraphVisitor {
 		log.debug( "Visiting attribute path : " + subPath.getFullPath() );
 
 		final boolean continueWalk;
-		if ( attributeDefinition.getType().isAssociationType() ) {
+		if ( attributeDefinition.getType().isAssociationType() &&
-			continueWalk =
+				isDuplicateAssociationKey( ( (AssociationAttributeDefinition) attributeDefinition ).getAssociationKey() ) ) {
-					! isDuplicateAssociationKey( ( (AssociationAttributeDefinition) attributeDefinition ).getAssociationKey() ) &&
+			log.debug( "Property path deemed to be circular : " + subPath.getFullPath() );
-					strategy.startingAttribute( attributeDefinition );
+			continueWalk = false;
 		}
 		else {
 			continueWalk = strategy.startingAttribute( attributeDefinition );
@@ -152,10 +152,7 @@ public class MetadataDrivenModelGraphVisitor {
 	private void visitAssociation(AssociationAttributeDefinition attribute) {
 		// todo : do "too deep" checks; but see note about adding depth to PropertyPath
 
-		if ( !addAssociationKey( attribute.getAssociationKey() ) ) {
+		addAssociationKey( attribute.getAssociationKey() );
-			log.debug( "Property path deemed to be circular : " + currentPropertyPath.getFullPath() );
-			return;
-		}
 
 		if ( attribute.isCollection() ) {
 			visitCollectionDefinition( attribute.toCollectionDefinition() );
@@ -227,17 +224,26 @@ public class MetadataDrivenModelGraphVisitor {
 	private final Set<AssociationKey> visitedAssociationKeys = new HashSet<AssociationKey>();
 
 	/**
-	 * Add association key.
+	 * Add association key to indicate the association is being visited.
 	 * @param associationKey - the association key.
-	 * @return true, if the association key was added;
+	 * @throws WalkingException if the association with the specified association key
-	 *         false, otherwise (indicating the association key was already visited).
+	 *                          has already been visited.
 	 */
-	protected boolean addAssociationKey(AssociationKey associationKey) {
+	protected void addAssociationKey(AssociationKey associationKey) {
-		return visitedAssociationKeys.add( associationKey );
+		if ( ! visitedAssociationKeys.add( associationKey ) ) {
+			throw new WalkingException(
+					String.format( "Association has already been visited: %s", associationKey )
+			);
+		}
 	}
 
+	/**
+	 * Has an association with the specified key been visited already?
+	 * @param associationKey - the association key.
+	 * @return true, if the association with the specified association key has already been visited;
+	 *         false, otherwise.
+	 */
 	protected boolean isDuplicateAssociationKey(AssociationKey associationKey) {
 		return visitedAssociationKeys.contains( associationKey );
 	}
-
 }