HHH-13756 simplify EntityGraphNavigator's navigate() to never return null value

This commit is contained in:
Nathan Xu 2020-03-12 11:24:08 -04:00 committed by Steve Ebersole
parent d4746da853
commit a3dce5f00a
6 changed files with 25 additions and 28 deletions

View File

@ -431,12 +431,10 @@ public class LoaderSelectBuilder {
// 'entity graph' takes precedence over 'fetch profile' // 'entity graph' takes precedence over 'fetch profile'
if ( entityGraphNavigator != null) { if ( entityGraphNavigator != null) {
navigation = entityGraphNavigator.navigateIfApplicable( fetchParent, fetchable, isKeyFetchable ); navigation = entityGraphNavigator.navigate( fetchParent, fetchable, isKeyFetchable );
if ( navigation != null ) {
fetchTiming = navigation.getFetchStrategy(); fetchTiming = navigation.getFetchStrategy();
joined = navigation.isJoined(); joined = navigation.isJoined();
} }
}
else if ( loadQueryInfluencers.hasEnabledFetchProfiles() ) { else if ( loadQueryInfluencers.hasEnabledFetchProfiles() ) {
if ( fetchParent instanceof EntityResultGraphNode ) { if ( fetchParent instanceof EntityResultGraphNode ) {
final EntityResultGraphNode entityFetchParent = (EntityResultGraphNode) fetchParent; final EntityResultGraphNode entityFetchParent = (EntityResultGraphNode) fetchParent;
@ -493,7 +491,7 @@ public class LoaderSelectBuilder {
if ( !( fetchable instanceof BasicValuedModelPart ) ) { if ( !( fetchable instanceof BasicValuedModelPart ) ) {
fetchDepth--; fetchDepth--;
} }
if ( entityGraphNavigator != null && navigation != null ) { if ( entityGraphNavigator != null ) {
entityGraphNavigator.backtrack( navigation.getPreviousContext() ); entityGraphNavigator.backtrack( navigation.getPreviousContext() );
} }
} }

View File

@ -307,12 +307,10 @@ public class StandardSqmSelectTranslator
alias = null; alias = null;
if ( entityGraphNavigator != null ) { if ( entityGraphNavigator != null ) {
navigation = entityGraphNavigator.navigateIfApplicable( fetchParent, fetchable, isKeyFetchable ); navigation = entityGraphNavigator.navigate( fetchParent, fetchable, isKeyFetchable );
if ( navigation != null ) {
fetchTiming = navigation.getFetchStrategy(); fetchTiming = navigation.getFetchStrategy();
joined = navigation.isJoined(); joined = navigation.isJoined();
} }
}
else if ( fetchInfluencers.hasEnabledFetchProfiles() ) { else if ( fetchInfluencers.hasEnabledFetchProfiles() ) {
if ( fetchParent instanceof EntityResultGraphNode ) { if ( fetchParent instanceof EntityResultGraphNode ) {
final EntityResultGraphNode entityFetchParent = (EntityResultGraphNode) fetchParent; final EntityResultGraphNode entityFetchParent = (EntityResultGraphNode) fetchParent;

View File

@ -52,19 +52,18 @@ public interface EntityGraphNavigator {
* Mainly reset the current context entity graph node to the passed method parameter. * Mainly reset the current context entity graph node to the passed method parameter.
* *
* @param previousContext The stored previous invocation result; should not be null * @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); void backtrack(GraphImplementor previousContext);
/** /**
* Tries to navigate from parent to child node within entity graph and returns non-null {@code NavigateResult} * 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.
* *
* @apiNote If applicable, internal state will be mutated. Not thread safe and should be used within single thread. * @apiNote If applicable, internal state will be mutated. Not thread safe and should be used within single thread.
* @param parent The FetchParent * @param parent The FetchParent
* @param fetchable The Fetchable * @param fetchable The Fetchable
* @param exploreKeySubgraph true if only key sub graph is explored; false if key sub graph is excluded * @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);
} }

View File

@ -49,7 +49,7 @@ public class StandardEntityGraphNavigatorImpl implements EntityGraphNavigator {
} }
@Override @Override
public Navigation navigateIfApplicable(FetchParent fetchParent, Fetchable fetchable, boolean exploreKeySubgraph) { public Navigation navigate(FetchParent fetchParent, Fetchable fetchable, boolean exploreKeySubgraph) {
final GraphImplementor previousContextRoot = currentGraphContext; final GraphImplementor previousContextRoot = currentGraphContext;
FetchTiming fetchTiming = null; FetchTiming fetchTiming = null;
boolean joined = false; boolean joined = false;
@ -65,7 +65,6 @@ public class StandardEntityGraphNavigatorImpl implements EntityGraphNavigator {
if ( fetchable instanceof PluralAttributeMapping ) { if ( fetchable instanceof PluralAttributeMapping ) {
PluralAttributeMapping pluralAttributeMapping = (PluralAttributeMapping) fetchable; PluralAttributeMapping pluralAttributeMapping = (PluralAttributeMapping) fetchable;
// avoid '^' bitwise operator to improve code readability
assert exploreKeySubgraph && isJpaMapCollectionType( pluralAttributeMapping ) assert exploreKeySubgraph && isJpaMapCollectionType( pluralAttributeMapping )
|| !exploreKeySubgraph && !isJpaMapCollectionType( pluralAttributeMapping ); || !exploreKeySubgraph && !isJpaMapCollectionType( pluralAttributeMapping );
@ -83,17 +82,20 @@ public class StandardEntityGraphNavigatorImpl implements EntityGraphNavigator {
subgraphMap = attributeNode.getSubGraphMap(); subgraphMap = attributeNode.getSubGraphMap();
subgraphMapKey = fetchable.getJavaTypeDescriptor().getJavaType(); subgraphMapKey = fetchable.getJavaTypeDescriptor().getJavaType();
} }
if ( subgraphMap == null || subgraphMapKey == null ) { if ( subgraphMap != null && subgraphMapKey != null ) {
currentGraphContext = null;
}
else {
currentGraphContext = subgraphMap.get( subgraphMapKey ); currentGraphContext = subgraphMap.get( subgraphMapKey );
} }
else {
currentGraphContext = null;
}
} }
else { else {
currentGraphContext = null; currentGraphContext = null;
} }
} }
else {
currentGraphContext = null;
}
if ( fetchTiming == null ) { if ( fetchTiming == null ) {
if ( graphSemantic == GraphSemantic.FETCH ) { if ( graphSemantic == GraphSemantic.FETCH ) {
fetchTiming = FetchTiming.DELAYED; fetchTiming = FetchTiming.DELAYED;

View File

@ -50,6 +50,7 @@ import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.SessionFactoryScope; import org.hibernate.testing.orm.junit.SessionFactoryScope;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.junit.platform.commons.util.CollectionUtils;
import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.is;
@ -262,11 +263,11 @@ public class EntityGraphLoadPlanBuilderTest {
assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses", tableGroup -> { assertPluralAttributeJoinedGroup( sqlAst, "shipAddresses", tableGroup -> {
assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) ); 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, instanceOf( CompositeTableGroup.class ) );
assertThat( compositeTableGroup.getTableGroupJoins(), hasSize( 1 ) ); 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.getModelPart().getPartName(), is( "country" ) );
assertThat( countryTableGroup.getTableGroupJoins(), isEmpty() ); assertThat( countryTableGroup.getTableGroupJoins(), isEmpty() );
@ -315,7 +316,7 @@ public class EntityGraphLoadPlanBuilderTest {
final TableGroup rootTableGroup = fromClause.getRoots().get( 0 ); final TableGroup rootTableGroup = fromClause.getRoots().get( 0 );
assertThat( rootTableGroup.getTableGroupJoins(), hasSize( 1 ) ); 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().getPartName(), is( expectedAttributeName ) );
assertThat( joinedGroup.getModelPart().getJavaTypeDescriptor().getJavaType(), assignableTo( expectedEntityJpaClass ) ); assertThat( joinedGroup.getModelPart().getJavaTypeDescriptor().getJavaType(), assignableTo( expectedEntityJpaClass ) );
assertThat( joinedGroup.getModelPart(), instanceOf( EntityValuedModelPart.class ) ); assertThat( joinedGroup.getModelPart(), instanceOf( EntityValuedModelPart.class ) );
@ -330,7 +331,7 @@ public class EntityGraphLoadPlanBuilderTest {
final TableGroup root = fromClause.getRoots().get( 0 ); final TableGroup root = fromClause.getRoots().get( 0 );
assertThat( root.getTableGroupJoins(), hasSize( 1 ) ); 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().getPartName(), is( expectedPluralAttributeName ) );
assertThat( joinedGroup.getModelPart(), instanceOf( PluralAttributeMapping.class ) ); assertThat( joinedGroup.getModelPart(), instanceOf( PluralAttributeMapping.class ) );
tableGroupConsumer.accept( joinedGroup ); tableGroupConsumer.accept( joinedGroup );
@ -339,7 +340,7 @@ public class EntityGraphLoadPlanBuilderTest {
private void assertPersonHomeAddressJoinedGroup(TableGroup tableGroup) { private void assertPersonHomeAddressJoinedGroup(TableGroup tableGroup) {
assertThat( tableGroup.getTableGroupJoins(), hasSize( 1 ) ); 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().getPartName(), is( "homeAddress" ) );
assertThat( joinedGroup.getModelPart(), instanceOf( EmbeddedAttributeMapping.class ) ); assertThat( joinedGroup.getModelPart(), instanceOf( EmbeddedAttributeMapping.class ) );
assertThat( joinedGroup, instanceOf( CompositeTableGroup.class ) ); assertThat( joinedGroup, instanceOf( CompositeTableGroup.class ) );

View File

@ -28,7 +28,6 @@ import org.hibernate.graph.spi.RootGraphImplementor;
import org.hibernate.metamodel.mapping.EntityValuedModelPart; import org.hibernate.metamodel.mapping.EntityValuedModelPart;
import org.hibernate.metamodel.mapping.PluralAttributeMapping; import org.hibernate.metamodel.mapping.PluralAttributeMapping;
import org.hibernate.metamodel.mapping.internal.EmbeddedAttributeMapping; 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.hql.spi.HqlQueryImplementor;
import org.hibernate.query.spi.QueryImplementor; import org.hibernate.query.spi.QueryImplementor;
import org.hibernate.query.sqm.internal.QuerySqmImpl; import org.hibernate.query.sqm.internal.QuerySqmImpl;
@ -440,7 +439,7 @@ public class HqlEntityGraphTest {
@Embeddable @Embeddable
public static class Address { public static class Address {
@ManyToOne(fetch = FetchType.EAGER) @ManyToOne
Country country; Country country;
} }