From d4746da853f5f7aef6f23f328517a6bcf228ca25 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Wed, 11 Mar 2020 18:11:45 -0400 Subject: [PATCH] HHH-13756 fix some bug in EmbeddableFetchImpl --- .../internal/EmbeddableFetchImpl.java | 10 +- .../StandardEntityGraphNavigatorImpl.java | 23 ++++- .../EntityGraphLoadPlanBuilderTest.java | 96 +++++++++---------- .../hql/entitygraph/HqlEntityGraphTest.java | 67 +++++++++---- 4 files changed, 118 insertions(+), 78 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/EmbeddableFetchImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/EmbeddableFetchImpl.java index 4a65270542..2ed6a2a049 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/EmbeddableFetchImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/embeddable/internal/EmbeddableFetchImpl.java @@ -14,6 +14,7 @@ import org.hibernate.metamodel.mapping.EmbeddableMappingType; import org.hibernate.metamodel.mapping.EmbeddableValuedModelPart; import org.hibernate.query.NavigablePath; import org.hibernate.sql.ast.SqlAstJoinType; +import org.hibernate.sql.ast.tree.from.TableGroup; import org.hibernate.sql.ast.tree.from.TableGroupJoin; import org.hibernate.sql.results.graph.AbstractFetchParent; import org.hibernate.sql.results.graph.AssemblerCreationState; @@ -54,11 +55,12 @@ public class EmbeddableFetchImpl extends AbstractFetchParent implements Embeddab creationState.getSqlAstCreationState().getFromClauseAccess().resolveTableGroup( getNavigablePath(), np -> { + final TableGroup lhsTableGroup = creationState.getSqlAstCreationState() + .getFromClauseAccess() + .findTableGroup( fetchParent.getNavigablePath() ); final TableGroupJoin tableGroupJoin = getReferencedMappingContainer().createTableGroupJoin( getNavigablePath(), - creationState.getSqlAstCreationState() - .getFromClauseAccess() - .findTableGroup( fetchParent.getNavigablePath() ), + lhsTableGroup, null, nullable ? SqlAstJoinType.LEFT : SqlAstJoinType.INNER, LockMode.NONE, @@ -66,7 +68,7 @@ public class EmbeddableFetchImpl extends AbstractFetchParent implements Embeddab creationState.getSqlAstCreationState().getSqlExpressionResolver(), creationState.getSqlAstCreationState().getCreationContext() ); - + lhsTableGroup.addTableGroupJoin( tableGroupJoin ); return tableGroupJoin.getJoinedGroup(); } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/internal/StandardEntityGraphNavigatorImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/results/internal/StandardEntityGraphNavigatorImpl.java index f5ae1d28e5..908efd919f 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/internal/StandardEntityGraphNavigatorImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/internal/StandardEntityGraphNavigatorImpl.java @@ -16,8 +16,10 @@ import org.hibernate.graph.GraphSemantic; import org.hibernate.graph.spi.AttributeNodeImplementor; import org.hibernate.graph.spi.GraphImplementor; import org.hibernate.graph.spi.SubGraphImplementor; +import org.hibernate.metamodel.mapping.CollectionPart; import org.hibernate.metamodel.mapping.EntityMappingType; import org.hibernate.metamodel.mapping.PluralAttributeMapping; +import org.hibernate.metamodel.mapping.internal.EntityCollectionPart; import org.hibernate.metamodel.model.domain.EntityDomainType; import org.hibernate.sql.results.graph.EntityGraphNavigator; import org.hibernate.sql.results.graph.FetchParent; @@ -69,11 +71,11 @@ public class StandardEntityGraphNavigatorImpl implements EntityGraphNavigator { if ( exploreKeySubgraph ) { subgraphMap = attributeNode.getKeySubGraphMap(); - subgraphMapKey = pluralAttributeMapping.getIndexDescriptor().getClass(); + subgraphMapKey = getEntityCollectionPartJavaClass( pluralAttributeMapping.getIndexDescriptor() ); } else { subgraphMap = attributeNode.getSubGraphMap(); - subgraphMapKey = pluralAttributeMapping.getElementDescriptor().getClass(); + subgraphMapKey = getEntityCollectionPartJavaClass( pluralAttributeMapping.getElementDescriptor() ); } } else { @@ -81,7 +83,12 @@ public class StandardEntityGraphNavigatorImpl implements EntityGraphNavigator { subgraphMap = attributeNode.getSubGraphMap(); subgraphMapKey = fetchable.getJavaTypeDescriptor().getJavaType(); } - currentGraphContext = subgraphMap == null ? null : subgraphMap.get( subgraphMapKey ); + if ( subgraphMap == null || subgraphMapKey == null ) { + currentGraphContext = null; + } + else { + currentGraphContext = subgraphMap.get( subgraphMapKey ); + } } else { currentGraphContext = null; @@ -100,6 +107,16 @@ public class StandardEntityGraphNavigatorImpl implements EntityGraphNavigator { return new Navigation( previousContextRoot, fetchTiming, joined ); } + private Class getEntityCollectionPartJavaClass(CollectionPart collectionPart) { + if ( collectionPart instanceof EntityCollectionPart ) { + EntityCollectionPart entityCollectionPart = (EntityCollectionPart) collectionPart; + return entityCollectionPart.getEntityMappingType().getJavaTypeDescriptor().getJavaType(); + } + else { + return null; + } + } + private boolean appliesTo(FetchParent fetchParent) { if ( currentGraphContext == null || !( fetchParent instanceof EntityResultGraphNode ) ) { return false; diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/loading/entitygraph/EntityGraphLoadPlanBuilderTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/loading/entitygraph/EntityGraphLoadPlanBuilderTest.java index 3aa99bf43c..90d34f9f9c 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/loading/entitygraph/EntityGraphLoadPlanBuilderTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/loading/entitygraph/EntityGraphLoadPlanBuilderTest.java @@ -225,7 +225,7 @@ public class EntityGraphLoadPlanBuilderTest { final SelectStatement sqlAst = buildSqlSelectAst( Dog.class, eg, GraphSemantic.LOAD, scope ); // Check the from-clause - assertPluralAttributeJoinedGroup( sqlAst, "favorites" ); + assertPluralAttributeJoinedGroup( sqlAst, "favorites", tableGroup -> {} ); } ); } @@ -240,13 +240,13 @@ public class EntityGraphLoadPlanBuilderTest { final SelectStatement sqlAst = buildSqlSelectAst( Dog.class, eg, GraphSemantic.FETCH, scope ); // Check the from-clause - assertPluralAttributeJoinedGroup( sqlAst, "favorites" ); + assertPluralAttributeJoinedGroup( sqlAst, "favorites", tableGroup -> {} ); } ); } @Test - void testEmbeddedCollectionLoadSubgraph(SessionFactoryScope scope) { + void testEmbeddedCollectionLoadGraph(SessionFactoryScope scope) { scope.inTransaction( em -> { final RootGraphImplementor eg = em.createEntityGraph( ExpressCompany.class ); @@ -259,7 +259,40 @@ public class EntityGraphLoadPlanBuilderTest { ); // Check the from-clause - assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses" ); + assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses", tableGroup -> { + assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) ); + + final TableGroup compositeTableGroup = tableGroup.getTableGroupJoins().iterator().next().getJoinedGroup(); + assertThat( compositeTableGroup, instanceOf( CompositeTableGroup.class ) ); + assertThat( compositeTableGroup.getTableGroupJoins(), hasSize( 1 ) ); + + final TableGroup countryTableGroup = compositeTableGroup.getTableGroupJoins().iterator().next().getJoinedGroup(); + assertThat( countryTableGroup.getModelPart().getPartName(), is( "country" ) ); + + assertThat( countryTableGroup.getTableGroupJoins(), isEmpty() ); + } ); + + } + ); + } + + @Test + void testEmbeddedCollectionFetchGraph(SessionFactoryScope scope) { + scope.inTransaction( + em -> { + final RootGraphImplementor eg = em.createEntityGraph( ExpressCompany.class ); + eg.addAttributeNodes( "shipAddresses" ); + + final SelectStatement sqlAst = buildSqlSelectAst( + ExpressCompany.class, + eg, GraphSemantic.FETCH, + scope + ); + + // Check the from-clause + assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses", tableGroup -> + assertThat( tableGroup.getTableGroupJoins(), isEmpty() ) + ); } ); @@ -283,15 +316,14 @@ public class EntityGraphLoadPlanBuilderTest { assertThat( rootTableGroup.getTableGroupJoins(), hasSize( 1 ) ); final TableGroup joinedGroup = rootTableGroup.getTableGroupJoins().iterator().next().getJoinedGroup(); + assertThat( joinedGroup.getModelPart().getPartName(), is( expectedAttributeName ) ); + assertThat( joinedGroup.getModelPart().getJavaTypeDescriptor().getJavaType(), assignableTo( expectedEntityJpaClass ) ); assertThat( joinedGroup.getModelPart(), instanceOf( EntityValuedModelPart.class ) ); - final EntityValuedModelPart entityValuedModelPart = (EntityValuedModelPart) joinedGroup.getModelPart(); - assertThat( entityValuedModelPart.getPartName(), is( expectedAttributeName ) ); - assertThat( entityValuedModelPart.getEntityMappingType().getJavaTypeDescriptor().getJavaType(), assignableTo( expectedEntityJpaClass ) ); tableGroupConsumer.accept( joinedGroup ); } - private void assertPluralAttributeJoinedGroup(SelectStatement sqlAst, String expectedPluralAttributeName) { + private void assertPluralAttributeJoinedGroup(SelectStatement sqlAst, String expectedPluralAttributeName, Consumer tableGroupConsumer) { final FromClause fromClause = sqlAst.getQuerySpec().getFromClause(); assertThat( fromClause.getRoots(), hasSize( 1 ) ); @@ -299,24 +331,18 @@ public class EntityGraphLoadPlanBuilderTest { assertThat( root.getTableGroupJoins(), hasSize( 1 ) ); final TableGroup joinedGroup = root.getTableGroupJoins().iterator().next().getJoinedGroup(); + assertThat( joinedGroup.getModelPart().getPartName(), is( expectedPluralAttributeName ) ); assertThat( joinedGroup.getModelPart(), instanceOf( PluralAttributeMapping.class ) ); - - final PluralAttributeMapping pluralAttributeMapping = (PluralAttributeMapping) joinedGroup.getModelPart(); - assertThat( pluralAttributeMapping.getAttributeName(), is( expectedPluralAttributeName ) ); - assertThat( joinedGroup.getTableGroupJoins(), isEmpty() ); + tableGroupConsumer.accept( joinedGroup ); } private void assertPersonHomeAddressJoinedGroup(TableGroup tableGroup) { assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) ); - final TableGroupJoin tableGroupJoin = tableGroup.getTableGroupJoins().iterator().next(); - assertThat( tableGroupJoin.getJoinedGroup(), instanceOf( CompositeTableGroup.class ) ); - - final CompositeTableGroup compositeTableGroup = (CompositeTableGroup) tableGroupJoin.getJoinedGroup(); - assertThat( compositeTableGroup.getModelPart(), instanceOf( EmbeddedAttributeMapping.class ) ); - - final EmbeddedAttributeMapping embeddedAttributeMapping = (EmbeddedAttributeMapping) compositeTableGroup.getModelPart(); - assertThat( embeddedAttributeMapping.getPartName(), is( "homeAddress" ) ); + final TableGroup joinedGroup = tableGroup.getTableGroupJoins().iterator().next().getJoinedGroup(); + assertThat( joinedGroup.getModelPart().getPartName(), is( "homeAddress" ) ); + assertThat( joinedGroup.getModelPart(), instanceOf( EmbeddedAttributeMapping.class ) ); + assertThat( joinedGroup, instanceOf( CompositeTableGroup.class ) ); } // util methods for verifying 'domain-result' graph ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -345,36 +371,6 @@ public class EntityGraphLoadPlanBuilderTest { entityFetchConsumer.accept( entityFetch ); } - @Test - @TestForIssue( jiraKey = "HHH-13756" ) - void testEmbeddedCollectionFetchSubgraph(SessionFactoryScope scope) { - scope.inTransaction( - em -> { - final RootGraphImplementor eg = em.createEntityGraph( ExpressCompany.class ); - eg.addAttributeNodes( "shipAddresses" ); - - final SelectStatement sqlAst = buildSqlSelectAst( - ExpressCompany.class, - eg, GraphSemantic.FETCH, - scope - ); - - final FromClause fromClause = sqlAst.getQuerySpec().getFromClause(); - assertThat( fromClause.getRoots(), hasSize( 1 ) ); - - final TableGroup root = fromClause.getRoots().get( 0 ); - assertThat( root.getTableGroupJoins(), hasSize( 1 ) ); - - final TableGroup joinedGroup = root.getTableGroupJoins().iterator().next().getJoinedGroup(); - assertThat( joinedGroup.getModelPart(), instanceOf( PluralAttributeMapping.class ) ); - - final PluralAttributeMapping pluralAttributeMapping = (PluralAttributeMapping) joinedGroup.getModelPart(); - assertThat( pluralAttributeMapping.getAttributeName(), is( "shipAddresses" ) ); - assertThat( joinedGroup.getTableGroupJoins(), isEmpty() ); - } - ); - } - private SelectStatement buildSqlSelectAst( Class entityType, RootGraphImplementor entityGraph, diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/entitygraph/HqlEntityGraphTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/entitygraph/HqlEntityGraphTest.java index e6305389ec..ee6f85c97c 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/entitygraph/HqlEntityGraphTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/entitygraph/HqlEntityGraphTest.java @@ -228,7 +228,7 @@ public class HqlEntityGraphTest { final SelectStatement sqlAst = buildSqlSelectAst( HqlEntityGraphTest.Dog.class, "select d from Dog as d", eg, GraphSemantic.LOAD, session ); // Check the from-clause - assertPluralAttributeJoinedGroup( sqlAst, "favorites" ); + assertPluralAttributeJoinedGroup( sqlAst, "favorites", tableGroup -> {} ); } ); } @@ -243,13 +243,13 @@ public class HqlEntityGraphTest { final SelectStatement sqlAst = buildSqlSelectAst( HqlEntityGraphTest.Dog.class, "select d from Dog as d", eg, GraphSemantic.FETCH, session ); // Check the from-clause - assertPluralAttributeJoinedGroup( sqlAst, "favorites" ); + assertPluralAttributeJoinedGroup( sqlAst, "favorites", tableGroup -> {} ); } ); } @Test - void testEmbeddedCollectionLoadSubgraph(SessionFactoryScope scope) { + void testEmbeddedCollectionLoadGraph(SessionFactoryScope scope) { scope.inTransaction( session -> { final RootGraphImplementor eg = session.createEntityGraph( HqlEntityGraphTest.ExpressCompany.class ); @@ -263,7 +263,39 @@ public class HqlEntityGraphTest { ); // Check the from-clause - assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses" ); + assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses", tableGroup -> { + assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) ); + + final TableGroup compositeTableGroup = tableGroup.getTableGroupJoins().iterator().next().getJoinedGroup(); + assertThat( compositeTableGroup, instanceOf( CompositeTableGroup.class ) ); + assertThat( compositeTableGroup.getTableGroupJoins(), hasSize( 1 ) ); + + final TableGroup countryTableGroup = compositeTableGroup.getTableGroupJoins().iterator().next().getJoinedGroup(); + assertThat( countryTableGroup.getModelPart().getPartName(), is( "country" ) ); + + assertThat( countryTableGroup.getTableGroupJoins(), isEmpty() ); + } ); + + } + ); + } + + @Test + void testEmbeddedCollectionFetchGraph(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + final RootGraphImplementor eg = session.createEntityGraph( HqlEntityGraphTest.ExpressCompany.class ); + eg.addAttributeNodes( "shipAddresses" ); + + final SelectStatement sqlAst = buildSqlSelectAst( + HqlEntityGraphTest.ExpressCompany.class, + "select company from ExpressCompany as company", + eg, GraphSemantic.FETCH, + session + ); + + // Check the from-clause + assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses", tableGroup -> assertThat( tableGroup.getTableGroupJoins(), isEmpty() ) ); } ); @@ -287,15 +319,14 @@ public class HqlEntityGraphTest { assertThat( rootTableGroup.getTableGroupJoins(), hasSize( 1 ) ); final TableGroup joinedGroup = rootTableGroup.getTableGroupJoins().iterator().next().getJoinedGroup(); + assertThat( joinedGroup.getModelPart().getPartName(), is( expectedAttributeName ) ); + assertThat( joinedGroup.getModelPart().getJavaTypeDescriptor().getJavaType(), assignableTo( expectedEntityJpaClass ) ); assertThat( joinedGroup.getModelPart(), instanceOf( EntityValuedModelPart.class ) ); - final EntityValuedModelPart entityValuedModelPart = (EntityValuedModelPart) joinedGroup.getModelPart(); - assertThat( entityValuedModelPart.getPartName(), is( expectedAttributeName ) ); - assertThat( entityValuedModelPart.getEntityMappingType().getJavaTypeDescriptor().getJavaType(), assignableTo( expectedEntityJpaClass ) ); tableGroupConsumer.accept( joinedGroup ); } - private void assertPluralAttributeJoinedGroup(SelectStatement sqlAst, String expectedPluralAttributeName) { + private void assertPluralAttributeJoinedGroup(SelectStatement sqlAst, String expectedPluralAttributeName, Consumer tableGroupConsumer) { final FromClause fromClause = sqlAst.getQuerySpec().getFromClause(); assertThat( fromClause.getRoots(), hasSize( 1 ) ); @@ -303,24 +334,18 @@ public class HqlEntityGraphTest { assertThat( root.getTableGroupJoins(), hasSize( 1 ) ); final TableGroup joinedGroup = root.getTableGroupJoins().iterator().next().getJoinedGroup(); + assertThat( joinedGroup.getModelPart().getPartName(), is( expectedPluralAttributeName ) ); assertThat( joinedGroup.getModelPart(), instanceOf( PluralAttributeMapping.class ) ); - - final PluralAttributeMapping pluralAttributeMapping = (PluralAttributeMapping) joinedGroup.getModelPart(); - assertThat( pluralAttributeMapping.getAttributeName(), is( expectedPluralAttributeName ) ); - assertThat( joinedGroup.getTableGroupJoins(), isEmpty() ); + tableGroupConsumer.accept( joinedGroup ); } private void assertPersonHomeAddressJoinedGroup(TableGroup tableGroup) { assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) ); - final TableGroupJoin tableGroupJoin = tableGroup.getTableGroupJoins().iterator().next(); - assertThat( tableGroupJoin.getJoinedGroup(), instanceOf( CompositeTableGroup.class ) ); - - final CompositeTableGroup compositeTableGroup = (CompositeTableGroup) tableGroupJoin.getJoinedGroup(); - assertThat( compositeTableGroup.getModelPart(), instanceOf( EmbeddedAttributeMapping.class ) ); - - final EmbeddedAttributeMapping embeddedAttributeMapping = (EmbeddedAttributeMapping) compositeTableGroup.getModelPart(); - assertThat( embeddedAttributeMapping.getPartName(), is( "homeAddress" ) ); + final TableGroup joinedGroup = tableGroup.getTableGroupJoins().iterator().next().getJoinedGroup(); + assertThat( joinedGroup.getModelPart().getPartName(), is( "homeAddress" ) ); + assertThat( joinedGroup.getModelPart(), instanceOf( EmbeddedAttributeMapping.class ) ); + assertThat( joinedGroup, instanceOf( CompositeTableGroup.class ) ); } // util methods for verifying 'domain-result' graph ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -415,7 +440,7 @@ public class HqlEntityGraphTest { @Embeddable public static class Address { - @ManyToOne + @ManyToOne(fetch = FetchType.EAGER) Country country; }