From a3dce5f00a99e1e6e93c83c91b2bbe5f3a5df8c1 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Thu, 12 Mar 2020 11:24:08 -0400 Subject: [PATCH] HHH-13756 simplify EntityGraphNavigator's navigate() to never return null value --- .../loader/ast/internal/LoaderSelectBuilder.java | 10 ++++------ .../sql/internal/StandardSqmSelectTranslator.java | 8 +++----- .../sql/results/graph/EntityGraphNavigator.java | 9 ++++----- .../internal/StandardEntityGraphNavigatorImpl.java | 12 +++++++----- .../entitygraph/EntityGraphLoadPlanBuilderTest.java | 11 ++++++----- .../query/hql/entitygraph/HqlEntityGraphTest.java | 3 +-- 6 files changed, 25 insertions(+), 28 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java index c0ee882d25..759e17b829 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java @@ -431,11 +431,9 @@ public class LoaderSelectBuilder { // 'entity graph' takes precedence over 'fetch profile' if ( entityGraphNavigator != null) { - navigation = entityGraphNavigator.navigateIfApplicable( fetchParent, fetchable, isKeyFetchable ); - if ( navigation != null ) { - fetchTiming = navigation.getFetchStrategy(); - joined = navigation.isJoined(); - } + navigation = entityGraphNavigator.navigate( fetchParent, fetchable, isKeyFetchable ); + fetchTiming = navigation.getFetchStrategy(); + joined = navigation.isJoined(); } else if ( loadQueryInfluencers.hasEnabledFetchProfiles() ) { if ( fetchParent instanceof EntityResultGraphNode ) { @@ -493,7 +491,7 @@ public class LoaderSelectBuilder { if ( !( fetchable instanceof BasicValuedModelPart ) ) { fetchDepth--; } - if ( entityGraphNavigator != null && navigation != null ) { + if ( entityGraphNavigator != null ) { entityGraphNavigator.backtrack( navigation.getPreviousContext() ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/StandardSqmSelectTranslator.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/StandardSqmSelectTranslator.java index 767cb0d85c..edf39db328 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/StandardSqmSelectTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/StandardSqmSelectTranslator.java @@ -307,11 +307,9 @@ public class StandardSqmSelectTranslator alias = null; if ( entityGraphNavigator != null ) { - navigation = entityGraphNavigator.navigateIfApplicable( fetchParent, fetchable, isKeyFetchable ); - if ( navigation != null ) { - fetchTiming = navigation.getFetchStrategy(); - joined = navigation.isJoined(); - } + navigation = entityGraphNavigator.navigate( fetchParent, fetchable, isKeyFetchable ); + fetchTiming = navigation.getFetchStrategy(); + joined = navigation.isJoined(); } else if ( fetchInfluencers.hasEnabledFetchProfiles() ) { if ( fetchParent instanceof EntityResultGraphNode ) { diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/EntityGraphNavigator.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/EntityGraphNavigator.java index f8f6cf8c9c..cfd7d92d10 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/EntityGraphNavigator.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/EntityGraphNavigator.java @@ -52,19 +52,18 @@ public interface EntityGraphNavigator { * Mainly reset the current context entity graph node to the passed method parameter. * * @param previousContext The stored previous invocation result; should not be null - * @see #navigateIfApplicable(FetchParent, Fetchable, boolean) + * @see #navigate(FetchParent, Fetchable, boolean) */ void backtrack(GraphImplementor previousContext); /** - * Tries to navigate from parent to child node within entity graph and returns non-null {@code NavigateResult} - * if applicable. Returns null value if not applicable. + * Tries to navigate from parent to child node within entity graph and returns non-null {@code NavigateResult}. * * @apiNote If applicable, internal state will be mutated. Not thread safe and should be used within single thread. * @param parent The FetchParent * @param fetchable The Fetchable * @param exploreKeySubgraph true if only key sub graph is explored; false if key sub graph is excluded - * @return {@link Navigation} if applicable; null otherwise + * @return resulting {@link Navigation}; never null */ - Navigation navigateIfApplicable(FetchParent parent, Fetchable fetchable, boolean exploreKeySubgraph); + Navigation navigate(FetchParent parent, Fetchable fetchable, boolean exploreKeySubgraph); } 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 908efd919f..cfcf5f092e 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 @@ -49,7 +49,7 @@ public class StandardEntityGraphNavigatorImpl implements EntityGraphNavigator { } @Override - public Navigation navigateIfApplicable(FetchParent fetchParent, Fetchable fetchable, boolean exploreKeySubgraph) { + public Navigation navigate(FetchParent fetchParent, Fetchable fetchable, boolean exploreKeySubgraph) { final GraphImplementor previousContextRoot = currentGraphContext; FetchTiming fetchTiming = null; boolean joined = false; @@ -65,7 +65,6 @@ public class StandardEntityGraphNavigatorImpl implements EntityGraphNavigator { if ( fetchable instanceof PluralAttributeMapping ) { PluralAttributeMapping pluralAttributeMapping = (PluralAttributeMapping) fetchable; - // avoid '^' bitwise operator to improve code readability assert exploreKeySubgraph && isJpaMapCollectionType( pluralAttributeMapping ) || !exploreKeySubgraph && !isJpaMapCollectionType( pluralAttributeMapping ); @@ -83,17 +82,20 @@ public class StandardEntityGraphNavigatorImpl implements EntityGraphNavigator { subgraphMap = attributeNode.getSubGraphMap(); subgraphMapKey = fetchable.getJavaTypeDescriptor().getJavaType(); } - if ( subgraphMap == null || subgraphMapKey == null ) { - currentGraphContext = null; + if ( subgraphMap != null && subgraphMapKey != null ) { + currentGraphContext = subgraphMap.get( subgraphMapKey ); } else { - currentGraphContext = subgraphMap.get( subgraphMapKey ); + currentGraphContext = null; } } else { currentGraphContext = null; } } + else { + currentGraphContext = null; + } if ( fetchTiming == null ) { if ( graphSemantic == GraphSemantic.FETCH ) { fetchTiming = FetchTiming.DELAYED; 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 90d34f9f9c..977ef331df 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 @@ -50,6 +50,7 @@ import org.hibernate.testing.orm.junit.DomainModel; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; import org.junit.jupiter.api.Test; +import org.junit.platform.commons.util.CollectionUtils; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.is; @@ -262,11 +263,11 @@ public class EntityGraphLoadPlanBuilderTest { assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses", tableGroup -> { assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) ); - final TableGroup compositeTableGroup = tableGroup.getTableGroupJoins().iterator().next().getJoinedGroup(); + final TableGroup compositeTableGroup = CollectionUtils.getOnlyElement( tableGroup.getTableGroupJoins() ).getJoinedGroup(); assertThat( compositeTableGroup, instanceOf( CompositeTableGroup.class ) ); assertThat( compositeTableGroup.getTableGroupJoins(), hasSize( 1 ) ); - final TableGroup countryTableGroup = compositeTableGroup.getTableGroupJoins().iterator().next().getJoinedGroup(); + final TableGroup countryTableGroup = CollectionUtils.getOnlyElement( compositeTableGroup.getTableGroupJoins() ).getJoinedGroup(); assertThat( countryTableGroup.getModelPart().getPartName(), is( "country" ) ); assertThat( countryTableGroup.getTableGroupJoins(), isEmpty() ); @@ -315,7 +316,7 @@ public class EntityGraphLoadPlanBuilderTest { final TableGroup rootTableGroup = fromClause.getRoots().get( 0 ); assertThat( rootTableGroup.getTableGroupJoins(), hasSize( 1 ) ); - final TableGroup joinedGroup = rootTableGroup.getTableGroupJoins().iterator().next().getJoinedGroup(); + final TableGroup joinedGroup = CollectionUtils.getOnlyElement( rootTableGroup.getTableGroupJoins() ).getJoinedGroup(); assertThat( joinedGroup.getModelPart().getPartName(), is( expectedAttributeName ) ); assertThat( joinedGroup.getModelPart().getJavaTypeDescriptor().getJavaType(), assignableTo( expectedEntityJpaClass ) ); assertThat( joinedGroup.getModelPart(), instanceOf( EntityValuedModelPart.class ) ); @@ -330,7 +331,7 @@ public class EntityGraphLoadPlanBuilderTest { final TableGroup root = fromClause.getRoots().get( 0 ); assertThat( root.getTableGroupJoins(), hasSize( 1 ) ); - final TableGroup joinedGroup = root.getTableGroupJoins().iterator().next().getJoinedGroup(); + final TableGroup joinedGroup = CollectionUtils.getOnlyElement( root.getTableGroupJoins() ).getJoinedGroup(); assertThat( joinedGroup.getModelPart().getPartName(), is( expectedPluralAttributeName ) ); assertThat( joinedGroup.getModelPart(), instanceOf( PluralAttributeMapping.class ) ); tableGroupConsumer.accept( joinedGroup ); @@ -339,7 +340,7 @@ public class EntityGraphLoadPlanBuilderTest { private void assertPersonHomeAddressJoinedGroup(TableGroup tableGroup) { assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) ); - final TableGroup joinedGroup = tableGroup.getTableGroupJoins().iterator().next().getJoinedGroup(); + final TableGroup joinedGroup = CollectionUtils.getOnlyElement( tableGroup.getTableGroupJoins() ).getJoinedGroup(); assertThat( joinedGroup.getModelPart().getPartName(), is( "homeAddress" ) ); assertThat( joinedGroup.getModelPart(), instanceOf( EmbeddedAttributeMapping.class ) ); assertThat( joinedGroup, instanceOf( CompositeTableGroup.class ) ); 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 ee6f85c97c..7b178db0fe 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 @@ -28,7 +28,6 @@ import org.hibernate.graph.spi.RootGraphImplementor; import org.hibernate.metamodel.mapping.EntityValuedModelPart; import org.hibernate.metamodel.mapping.PluralAttributeMapping; import org.hibernate.metamodel.mapping.internal.EmbeddedAttributeMapping; -import org.hibernate.orm.test.loading.entitygraph.EntityGraphLoadPlanBuilderTest; import org.hibernate.query.hql.spi.HqlQueryImplementor; import org.hibernate.query.spi.QueryImplementor; import org.hibernate.query.sqm.internal.QuerySqmImpl; @@ -440,7 +439,7 @@ public class HqlEntityGraphTest { @Embeddable public static class Address { - @ManyToOne(fetch = FetchType.EAGER) + @ManyToOne Country country; }