HHH-8855 HHH-8640 corrected EntityGraph loadplan strategy, support inheritance in SubGraphs

Author: brmeyer

hibernate-core/src/main/java/org/hibernate/engine/query/spi/EntityGraphQueryHint.java

@@ -24,6 +24,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+
 import javax.persistence.AttributeNode;
 import javax.persistence.EntityGraph;
 import javax.persistence.Subgraph;
@@ -36,6 +37,7 @@
 import org.hibernate.hql.internal.ast.tree.FromElementFactory;
 import org.hibernate.persister.collection.QueryableCollection;
 import org.hibernate.sql.JoinType;
+import org.hibernate.type.CollectionType;
 import org.hibernate.type.EntityType;
 import org.hibernate.type.Type;
 
@@ -85,6 +87,8 @@ private List<FromElement> getFromElements(
 
 			final String attributeName = attributeNode.getAttributeName();
 			final String className = origin.getClassName();
+			// TODO: This is ignored by collection types and probably wrong for entity types.  Presumably it screws
+			// with inheritance.
 			final String role = className + "." + attributeName;
 			final String classAlias = origin.getClassAlias();
 			final String originTableAlias = origin.getTableAlias();
@@ -120,16 +124,17 @@ private List<FromElement> getFromElements(
 						);
 					}
 					else if ( propertyType.isCollectionType() ) {
+						CollectionType collectionType = (CollectionType) propertyType;
 						final String[] columns = origin.toColumns( originTableAlias, attributeName, false );
 
 						final FromElementFactory fromElementFactory = new FromElementFactory(
 								fromClause, origin,
 								attributeName, classAlias, columns, false
 						);
 						final QueryableCollection queryableCollection = walker.getSessionFactoryHelper()
-								.requireQueryableCollection( role );
+								.requireQueryableCollection( collectionType.getRole() );
 						fromElement = fromElementFactory.createCollection(
-								queryableCollection, role, JoinType.LEFT_OUTER_JOIN, true, false
+								queryableCollection, collectionType.getRole(), JoinType.LEFT_OUTER_JOIN, true, false
 						);
 					}
 				}

hibernate-core/src/main/java/org/hibernate/loader/plan/build/internal/AbstractEntityGraphVisitationStrategy.java

@@ -49,10 +49,8 @@
 import org.hibernate.loader.plan.spi.Return;
 import org.hibernate.persister.walking.spi.AssociationAttributeDefinition;
 import org.hibernate.persister.walking.spi.AttributeDefinition;
-import org.hibernate.persister.walking.spi.CollectionDefinition;
 import org.hibernate.persister.walking.spi.CollectionElementDefinition;
 import org.hibernate.persister.walking.spi.CollectionIndexDefinition;
-import org.hibernate.persister.walking.spi.CompositionDefinition;
 import org.hibernate.persister.walking.spi.EntityDefinition;
 import org.hibernate.persister.walking.spi.WalkingException;
 import org.jboss.logging.Logger;
@@ -73,6 +71,7 @@
  * it is not, then depends on which property is used to apply this entity graph.
  *
  * @author Strong Liu <[email protected]>
+ * @author Brett Meyer
  */
 public abstract class AbstractEntityGraphVisitationStrategy
 		extends AbstractLoadPlanBuildingAssociationVisitationStrategy {
@@ -87,10 +86,14 @@ public abstract class AbstractEntityGraphVisitationStrategy
 	protected static final FetchStrategy DEFAULT_EAGER = new FetchStrategy( FetchTiming.IMMEDIATE, FetchStyle.JOIN );
 	protected static final FetchStrategy DEFAULT_LAZY = new FetchStrategy( FetchTiming.DELAYED, FetchStyle.SELECT );
 	protected final LoadQueryInfluencers loadQueryInfluencers;
-	protected final ArrayDeque<GraphNodeImplementor> graphStack = new ArrayDeque<GraphNodeImplementor>();
-	protected final ArrayDeque<AttributeNodeImplementor> attributeStack = new ArrayDeque<AttributeNodeImplementor>();
-	//the attribute nodes defined in the current graph node (entity graph or subgraph) we're working on
-	protected Map<String, AttributeNodeImplementor> attributeNodeImplementorMap = Collections.emptyMap();
+	// Queue containing entity/sub graphs to be visited.
+	private final ArrayDeque<GraphNodeImplementor> graphStack = new ArrayDeque<GraphNodeImplementor>();
+	// Queue containing attributes being visited, used eventually to determine the fetch strategy.
+	private final ArrayDeque<AttributeNodeImplementor> attributeStack = new ArrayDeque<AttributeNodeImplementor>();
+	// Queue of maps containing the current graph node's attributes.  Used for fast lookup, instead of iterating
+	// over graphStack.peekLast().attributeImplementorNodes().
+	private final ArrayDeque<Map<String, AttributeNodeImplementor>> attributeMapStack
+			= new ArrayDeque<Map<String, AttributeNodeImplementor>>();
 	private EntityReturn rootEntityReturn;
 	private final LockMode lockMode;
 
@@ -106,24 +109,21 @@ protected AbstractEntityGraphVisitationStrategy(
 	public void start() {
 		super.start();
 		graphStack.addLast( getRootEntityGraph() );
-		attributeNodeImplementorMap = buildAttributeNodeMap();
 	}
 
 	@Override
 	public void finish() {
 		super.finish();
 		graphStack.removeLast();
-		attributeNodeImplementorMap = Collections.emptyMap();
 		//applying a little internal stack checking
-		if ( !graphStack.isEmpty() || !attributeStack.isEmpty() || !attributeNodeImplementorMap.isEmpty() ) {
+		if ( !graphStack.isEmpty() || !attributeStack.isEmpty() || !attributeMapStack.isEmpty() ) {
 			throw new WalkingException( "Internal stack error" );
 		}
 	}
 
 	@Override
 	public void startingEntity(final EntityDefinition entityDefinition) {
-		//TODO check if the passed in entity definition is the same as the root entity graph (a.k.a they are came from same entity class)?
-		//this maybe the root entity graph or a sub graph.
+		attributeMapStack.addLast( buildAttributeNodeMap() );
 		super.startingEntity( entityDefinition );
 	}
 
@@ -143,6 +143,12 @@ protected Map<String, AttributeNodeImplementor> buildAttributeNodeMap() {
 		return attributeNodeImplementorMap;
 	}
 
+	@Override
+	public void finishingEntity(final EntityDefinition entityDefinition) {
+		attributeMapStack.removeLast();
+		super.finishingEntity( entityDefinition );
+	}
+
 	/**
 	 * I'm using NULL-OBJECT pattern here, for attributes that not existing in the EntityGraph,
 	 * a predefined NULL-ATTRIBUTE-NODE is pushed to the stack.
@@ -156,12 +162,13 @@ protected Map<String, AttributeNodeImplementor> buildAttributeNodeMap() {
 	 */
 	@Override
 	public boolean startingAttribute(AttributeDefinition attributeDefinition) {
+		Map<String, AttributeNodeImplementor> attributeMap = attributeMapStack.peekLast();
 		final String attrName = attributeDefinition.getName();
 		AttributeNodeImplementor attributeNode = NON_EXIST_ATTRIBUTE_NODE;
 		GraphNodeImplementor subGraphNode = NON_EXIST_SUBGRAPH_NODE;
 		//the attribute is in the EntityGraph, so, let's continue
-		if ( attributeNodeImplementorMap.containsKey( attrName ) ) {
-			attributeNode = attributeNodeImplementorMap.get( attrName );
+		if ( attributeMap.containsKey( attrName ) ) {
+			attributeNode = attributeMap.get( attrName );
 			//here we need to check if there is a subgraph (or sub key graph if it is an indexed attribute )
 			Map<Class, Subgraph> subGraphs = attributeNode.getSubgraphs();
 			Class javaType = attributeDefinition.getType().getReturnedClass();
@@ -183,52 +190,26 @@ public void finishingAttribute(final AttributeDefinition attributeDefinition) {
 		super.finishingAttribute( attributeDefinition );
 	}
 
-	@Override
-	protected boolean handleAssociationAttribute(
-			final AssociationAttributeDefinition attributeDefinition) {
-		return super.handleAssociationAttribute( attributeDefinition );
-	}
-
-	@Override
-	protected boolean handleCompositeAttribute(
-			final AttributeDefinition attributeDefinition) {
-		return super.handleCompositeAttribute( attributeDefinition );
-	}
-
-
-	@Override
-	public void startingComposite(final CompositionDefinition compositionDefinition) {
-		super.startingComposite( compositionDefinition );
-	}
-
-
-	@Override
-	public void finishingComposite(final CompositionDefinition compositionDefinition) {
-		super.finishingComposite( compositionDefinition );
-	}
-
-
-	@Override
-	public void startingCollection(final CollectionDefinition collectionDefinition) {
-		super.startingCollection( collectionDefinition );
-	}
-
-	@Override
-	public void finishingCollection(final CollectionDefinition collectionDefinition) {
-		super.finishingCollection( collectionDefinition );
-	}
-
 
 	@Override
 	public void startingCollectionElements(
 			final CollectionElementDefinition elementDefinition) {
+		AttributeNodeImplementor attributeNode = attributeStack.peekLast();
+		GraphNodeImplementor subGraphNode = NON_EXIST_SUBGRAPH_NODE;
+		Map<Class, Subgraph> subGraphs = attributeNode.getSubgraphs();
+		Class javaType = elementDefinition.getType().getReturnedClass();
+		if ( !subGraphs.isEmpty() && subGraphs.containsKey( javaType ) ) {
+			subGraphNode = (GraphNodeImplementor) subGraphs.get( javaType );
+		}
+		graphStack.addLast( subGraphNode );
 		super.startingCollectionElements( elementDefinition );
 	}
 
 	@Override
 	public void finishingCollectionElements(
 			final CollectionElementDefinition elementDefinition) {
 		super.finishingCollectionElements( elementDefinition );
+		graphStack.removeLast();
 	}
 
 

hibernate-entitymanager/src/main/java/org/hibernate/jpa/graph/internal/SubgraphImpl.java

@@ -122,7 +122,7 @@ public List<AttributeNode<?>> getAttributeNodes() {
 	@Override
 	@SuppressWarnings("unchecked")
 	protected Attribute<T,?> resolveAttribute(String attributeName) {
-		final Attribute<T,?> attribute = managedType.getDeclaredAttribute( attributeName );
+		final Attribute<T,?> attribute = managedType.getAttribute( attributeName );
 		if ( attribute == null ) {
 			throw new IllegalArgumentException(
 					String.format(

hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/graphs/Company.java

@@ -0,0 +1,54 @@
+/* 
+ * Hibernate, Relational Persistence for Idiomatic Java
+ * 
+ * JBoss, Home of Professional Open Source
+ * Copyright 2014 Red Hat Inc. and/or its affiliates and other contributors
+ * as indicated by the @authors tag. All rights reserved.
+ * See the copyright.txt in the distribution for a
+ * full listing of individual contributors.
+ *
+ * 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, v. 2.1.
+ * This program is distributed in the hope that it will be useful, but WITHOUT A
+ * 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,
+ * v.2.1 along with this distribution; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA  02110-1301, USA.
+ */
+package org.hibernate.jpa.test.graphs;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.persistence.ElementCollection;
+import javax.persistence.Entity;
+import javax.persistence.FetchType;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+import javax.persistence.OneToMany;
+import javax.persistence.OneToOne;
+
+
+/**
+ * @author Brett Meyer
+ */
+@Entity
+public class Company {
+	@Id @GeneratedValue
+	public long id;
+	
+	@OneToMany
+	public Set<Employee> employees = new HashSet<Employee>();
+	
+	@OneToOne(fetch = FetchType.LAZY)
+	public Location location;
+	
+	@ElementCollection
+	public Set<Market> markets = new HashSet<Market>();
+	
+	@ElementCollection(fetch = FetchType.EAGER)
+	public Set<String> phoneNumbers = new HashSet<String>();
+}

hibernate-entitymanager/src/test/java/org/hibernate/jpa/test/graphs/Employee.java

@@ -0,0 +1,48 @@
+/* 
+ * Hibernate, Relational Persistence for Idiomatic Java
+ * 
+ * JBoss, Home of Professional Open Source
+ * Copyright 2014 Red Hat Inc. and/or its affiliates and other contributors
+ * as indicated by the @authors tag. All rights reserved.
+ * See the copyright.txt in the distribution for a
+ * full listing of individual contributors.
+ *
+ * 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, v. 2.1.
+ * This program is distributed in the hope that it will be useful, but WITHOUT A
+ * 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,
+ * v.2.1 along with this distribution; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+ * MA  02110-1301, USA.
+ */
+package org.hibernate.jpa.test.graphs;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+import javax.persistence.Inheritance;
+import javax.persistence.InheritanceType;
+import javax.persistence.ManyToMany;
+
+
+/**
+ * @author Brett Meyer
+ */
+@Entity
+@Inheritance( strategy = InheritanceType.TABLE_PER_CLASS )
+public class Employee {
+	@Id @GeneratedValue
+	public long id;
+	
+	@ManyToMany
+	public Set<Manager> managers = new HashSet<Manager>();
+	
+	@ManyToMany
+	public Set<Employee> friends = new HashSet<Employee>();
+}