From f32c73616014996c07a973870ac150a6acc451b9 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Thu, 25 Jul 2013 17:17:02 -0500 Subject: [PATCH] HHH-8276 - Integrate LoadPlans into UniqueEntityLoader (PoC) --- .../entity/BatchingEntityLoaderBuilder.java | 1 + .../AbstractLoadPlanBuilderStrategy.java | 52 ++--- ...tCompositeEntityIdentifierDescription.java | 16 +- .../returns/AbstractCompositeFetch.java | 44 +++- .../returns/AbstractEntityReference.java | 19 +- ...lectionFetchableElementCompositeGraph.java | 34 +-- ...ollectionFetchableIndexCompositeGraph.java | 40 +--- .../internal/returns/CompositeFetchImpl.java | 20 +- ...capsulatedEntityIdentifierDescription.java | 17 +- .../returns/NestedCompositeFetchImpl.java | 19 +- ...capsulatedEntityIdentifierDescription.java | 11 + ...BuildingAssociationVisitationStrategy.java | 134 +++++++----- .../LoadQueryJoinAndFetchProcessor.java | 5 +- .../spi/AssociationVisitationStrategy.java | 79 ++++++- .../walking/spi/MetamodelGraphWalker.java | 16 +- .../plans/LoadPlanStructureAssertionTest.java | 195 +++++++++++------- .../walking/CompositesWalkingTest.java | 10 +- .../LoggingAssociationVisitationStrategy.java | 46 ++--- .../walking/NestedCompositeElementTest.java | 49 +++++ 19 files changed, 491 insertions(+), 316 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/loadplans/walking/NestedCompositeElementTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/loader/entity/BatchingEntityLoaderBuilder.java b/hibernate-core/src/main/java/org/hibernate/loader/entity/BatchingEntityLoaderBuilder.java index 116716903a..a30d4c4bc9 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/entity/BatchingEntityLoaderBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/entity/BatchingEntityLoaderBuilder.java @@ -49,6 +49,7 @@ public abstract class BatchingEntityLoaderBuilder { } default: { return org.hibernate.loader.entity.plan.LegacyBatchingEntityLoaderBuilder.INSTANCE; +// return LegacyBatchingEntityLoaderBuilder.INSTANCE; } } } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/build/AbstractLoadPlanBuilderStrategy.java b/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/build/AbstractLoadPlanBuilderStrategy.java index 37e3854302..7b7fcd4c90 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/build/AbstractLoadPlanBuilderStrategy.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan/spi/build/AbstractLoadPlanBuilderStrategy.java @@ -309,32 +309,32 @@ public abstract class AbstractLoadPlanBuilderStrategy implements LoadPlanBuilder // - the element graph pushed while starting would be popped in finishing/Entity/finishingComposite } - @Override - public void startingCompositeCollectionElement(CompositeCollectionElementDefinition compositeElementDefinition) { - log.tracef( - "%s Starting composite collection element for (%s)", - StringHelper.repeat( ">>", fetchOwnerStack.size() ), - compositeElementDefinition.getCollectionDefinition().getCollectionPersister().getRole() - ); - } - - @Override - public void finishingCompositeCollectionElement(CompositeCollectionElementDefinition compositeElementDefinition) { - // pop the current fetch owner, and make sure what we just popped represents this composition - final FetchOwner poppedFetchOwner = popFromStack(); - - if ( ! CompositeElementGraph.class.isInstance( poppedFetchOwner ) ) { - throw new WalkingException( "Mismatched FetchOwner from stack on pop" ); - } - - // NOTE : not much else we can really check here atm since on the walking spi side we do not have path - - log.tracef( - "%s Finished composite element for : %s", - StringHelper.repeat( "<<", fetchOwnerStack.size() ), - compositeElementDefinition.getCollectionDefinition().getCollectionPersister().getRole() - ); - } +// @Override +// public void startingCompositeCollectionElement(CompositeCollectionElementDefinition compositeElementDefinition) { +// log.tracef( +// "%s Starting composite collection element for (%s)", +// StringHelper.repeat( ">>", fetchOwnerStack.size() ), +// compositeElementDefinition.getCollectionDefinition().getCollectionPersister().getRole() +// ); +// } +// +// @Override +// public void finishingCompositeCollectionElement(CompositeCollectionElementDefinition compositeElementDefinition) { +// // pop the current fetch owner, and make sure what we just popped represents this composition +// final FetchOwner poppedFetchOwner = popFromStack(); +// +// if ( ! CompositeElementGraph.class.isInstance( poppedFetchOwner ) ) { +// throw new WalkingException( "Mismatched FetchOwner from stack on pop" ); +// } +// +// // NOTE : not much else we can really check here atm since on the walking spi side we do not have path +// +// log.tracef( +// "%s Finished composite element for : %s", +// StringHelper.repeat( "<<", fetchOwnerStack.size() ), +// compositeElementDefinition.getCollectionDefinition().getCollectionPersister().getRole() +// ); +// } @Override public void finishingCollection(CollectionDefinition collectionDefinition) { diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/AbstractCompositeEntityIdentifierDescription.java b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/AbstractCompositeEntityIdentifierDescription.java index cbb7ba92d0..45f94d468f 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/AbstractCompositeEntityIdentifierDescription.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/AbstractCompositeEntityIdentifierDescription.java @@ -39,21 +39,14 @@ public abstract class AbstractCompositeEntityIdentifierDescription implements EntityIdentifierDescription, FetchSource, ExpandingEntityIdentifierDescription { private final EntityReference entityReference; - private final CompositeQuerySpace compositeQuerySpace; protected AbstractCompositeEntityIdentifierDescription( EntityReference entityReference, CompositeQuerySpace compositeQuerySpace, CompositeType identifierType, PropertyPath propertyPath) { - super( identifierType, propertyPath ); + super( identifierType, compositeQuerySpace, false, propertyPath ); this.entityReference = entityReference; - this.compositeQuerySpace = compositeQuerySpace; - } - - @Override - protected String getFetchLeftHandSideUid() { - return compositeQuerySpace.getUid(); } @Override @@ -66,11 +59,4 @@ public abstract class AbstractCompositeEntityIdentifierDescription // the source for this (as a Fetch) is the entity reference return (FetchSource) entityReference.getIdentifierDescription(); } - - @Override - public String getQuerySpaceUid() { - return compositeQuerySpace.getUid(); - } - - } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/AbstractCompositeFetch.java b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/AbstractCompositeFetch.java index 2da27b647b..98cc575ff4 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/AbstractCompositeFetch.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/AbstractCompositeFetch.java @@ -35,6 +35,7 @@ import org.hibernate.loader.plan2.build.spi.ExpandingQuerySpace; import org.hibernate.loader.plan2.build.spi.LoadPlanBuildingContext; import org.hibernate.loader.plan2.spi.CollectionFetch; import org.hibernate.loader.plan2.spi.CompositeFetch; +import org.hibernate.loader.plan2.spi.CompositeQuerySpace; import org.hibernate.loader.plan2.spi.EntityFetch; import org.hibernate.loader.plan2.spi.Fetch; import org.hibernate.loader.plan2.spi.Join; @@ -56,16 +57,31 @@ public abstract class AbstractCompositeFetch implements CompositeFetch, Expandin private static final FetchStrategy FETCH_STRATEGY = new FetchStrategy( FetchTiming.IMMEDIATE, FetchStyle.JOIN ); private final CompositeType compositeType; + private final CompositeQuerySpace compositeQuerySpace; private final PropertyPath propertyPath; + private final boolean allowCollectionFetches; private List fetches; - protected AbstractCompositeFetch(CompositeType compositeType, PropertyPath propertyPath) { + protected AbstractCompositeFetch( + CompositeType compositeType, + CompositeQuerySpace compositeQuerySpace, + boolean allowCollectionFetches, PropertyPath propertyPath) { this.compositeType = compositeType; + this.compositeQuerySpace = compositeQuerySpace; + this.allowCollectionFetches = allowCollectionFetches; this.propertyPath = propertyPath; } - protected abstract String getFetchLeftHandSideUid(); + @SuppressWarnings("UnusedParameters") + protected CompositeQuerySpace resolveCompositeQuerySpace(LoadPlanBuildingContext loadPlanBuildingContext) { + return compositeQuerySpace; + } + + @Override + public String getQuerySpaceUid() { + return compositeQuerySpace.getUid(); + } @Override public void validateFetchPlan(FetchStrategy fetchStrategy, AttributeDefinition attributeDefinition) { @@ -92,8 +108,8 @@ public abstract class AbstractCompositeFetch implements CompositeFetch, Expandin ); } - final ExpandingQuerySpace leftHandSide = (ExpandingQuerySpace) loadPlanBuildingContext.getQuerySpaces().getQuerySpaceByUid( - getFetchLeftHandSideUid() + final ExpandingQuerySpace leftHandSide = (ExpandingQuerySpace) resolveCompositeQuerySpace( + loadPlanBuildingContext ); final Join join = leftHandSide.addEntityJoin( attributeDefinition, @@ -122,11 +138,18 @@ public abstract class AbstractCompositeFetch implements CompositeFetch, Expandin public CompositeFetch buildCompositeFetch( CompositionDefinition attributeDefinition, LoadPlanBuildingContext loadPlanBuildingContext) { + final ExpandingQuerySpace leftHandSide = (ExpandingQuerySpace) resolveCompositeQuerySpace( loadPlanBuildingContext ); + final Join join = leftHandSide.addCompositeJoin( + attributeDefinition, + loadPlanBuildingContext.getQuerySpaces().generateImplicitUid() + ); + final NestedCompositeFetchImpl fetch = new NestedCompositeFetchImpl( this, attributeDefinition.getType(), - getPropertyPath(), - getFetchLeftHandSideUid() + (CompositeQuerySpace) join.getRightHandSide(), + allowCollectionFetches, + getPropertyPath() ); addFetch( fetch ); return fetch; @@ -137,6 +160,15 @@ public abstract class AbstractCompositeFetch implements CompositeFetch, Expandin AssociationAttributeDefinition attributeDefinition, FetchStrategy fetchStrategy, LoadPlanBuildingContext loadPlanBuildingContext) { + if ( !allowCollectionFetches ) { + throw new WalkingException( + String.format( + "This composite path [%s] does not allow collection fetches (composite id or composite collection index/element", + propertyPath.getFullPath() + ) + ); + } + // general question here wrt Joins and collection fetches... do we create multiple Joins for many-to-many, // for example, or do we allow the Collection QuerySpace to handle that? diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/AbstractEntityReference.java b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/AbstractEntityReference.java index e2a391b533..e54dad7a93 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/AbstractEntityReference.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/AbstractEntityReference.java @@ -69,7 +69,7 @@ public abstract class AbstractEntityReference implements EntityReference, Expand PropertyPath propertyPath) { this.entityQuerySpace = entityQuerySpace; this.propertyPath = propertyPath; - this.identifierDescription = buildIdentifierDescription( entityQuerySpace ); + this.identifierDescription = buildIdentifierDescription(); } @@ -77,11 +77,9 @@ public abstract class AbstractEntityReference implements EntityReference, Expand * Builds just the first level of identifier description. This will be either a simple id descriptor (String, * Long, etc) or some form of composite id (either encapsulated or not). * - * @param querySpace The entity query space - * * @return the descriptor for the identifier */ - private EntityIdentifierDescription buildIdentifierDescription(EntityQuerySpace querySpace) { + private EntityIdentifierDescription buildIdentifierDescription() { final EntityPersister persister = entityQuerySpace.getEntityPersister(); final EntityIdentifierDefinition identifierDefinition = persister.getEntityKeyDefinition(); @@ -166,9 +164,8 @@ public abstract class AbstractEntityReference implements EntityReference, Expand ) ); } - final ExpandingQuerySpace leftHandSide = (ExpandingQuerySpace) loadPlanBuildingContext.getQuerySpaces().getQuerySpaceByUid( - getQuerySpaceUid() - ); + + final ExpandingQuerySpace leftHandSide = (ExpandingQuerySpace) entityQuerySpace; final Join join = leftHandSide.addEntityJoin( attributeDefinition, fetchedPersister, @@ -196,9 +193,17 @@ public abstract class AbstractEntityReference implements EntityReference, Expand public CompositeFetch buildCompositeFetch( CompositionDefinition attributeDefinition, LoadPlanBuildingContext loadPlanBuildingContext) { + final ExpandingQuerySpace leftHandSide = (ExpandingQuerySpace) entityQuerySpace; + final Join join = leftHandSide.addCompositeJoin( + attributeDefinition, + loadPlanBuildingContext.getQuerySpaces().generateImplicitUid() + ); + final CompositeFetchImpl fetch = new CompositeFetchImpl( this, attributeDefinition.getType(), + (CompositeQuerySpace) join.getRightHandSide(), + true, getPropertyPath() ); addFetch( fetch ); diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/CollectionFetchableElementCompositeGraph.java b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/CollectionFetchableElementCompositeGraph.java index 974d0c6c74..6f9e9f2344 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/CollectionFetchableElementCompositeGraph.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/CollectionFetchableElementCompositeGraph.java @@ -23,19 +23,17 @@ */ package org.hibernate.loader.plan2.build.internal.returns; -import org.hibernate.engine.FetchStrategy; -import org.hibernate.loader.plan2.build.spi.LoadPlanBuildingContext; -import org.hibernate.loader.plan2.spi.CollectionFetch; import org.hibernate.loader.plan2.spi.CollectionFetchableElement; import org.hibernate.loader.plan2.spi.CollectionReference; import org.hibernate.loader.plan2.spi.CompositeFetch; +import org.hibernate.loader.plan2.spi.CompositeQuerySpace; import org.hibernate.loader.plan2.spi.FetchSource; import org.hibernate.loader.plan2.spi.Join; -import org.hibernate.persister.walking.spi.AssociationAttributeDefinition; -import org.hibernate.persister.walking.spi.WalkingException; import org.hibernate.type.CompositeType; /** + * Models the element graph of a collection, where the elements are composite + * * @author Steve Ebersole */ public class CollectionFetchableElementCompositeGraph @@ -43,23 +41,16 @@ public class CollectionFetchableElementCompositeGraph implements CompositeFetch, CollectionFetchableElement { private final CollectionReference collectionReference; - private final Join compositeJoin; - public CollectionFetchableElementCompositeGraph( - CollectionReference collectionReference, - Join compositeJoin) { + public CollectionFetchableElementCompositeGraph(CollectionReference collectionReference, Join compositeJoin) { super( (CompositeType) compositeJoin.getRightHandSide().getPropertyMapping().getType(), + (CompositeQuerySpace) compositeJoin.getRightHandSide(), + false, // these property paths are just informational... collectionReference.getPropertyPath().append( "" ) ); this.collectionReference = collectionReference; - this.compositeJoin = compositeJoin; - } - - @Override - protected String getFetchLeftHandSideUid() { - return compositeJoin.getRightHandSide().getUid(); } @Override @@ -71,17 +62,4 @@ public class CollectionFetchableElementCompositeGraph public FetchSource getSource() { return collectionReference.getElementGraph(); } - - @Override - public CollectionFetch buildCollectionFetch( - AssociationAttributeDefinition attributeDefinition, - FetchStrategy fetchStrategy, - LoadPlanBuildingContext loadPlanBuildingContext) { - throw new WalkingException( "Encountered collection as part of fetched Collection composite-element" ); - } - - @Override - public String getQuerySpaceUid() { - return compositeJoin.getLeftHandSide().getUid(); - } } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/CollectionFetchableIndexCompositeGraph.java b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/CollectionFetchableIndexCompositeGraph.java index 76fd697e2c..0adbcfb348 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/CollectionFetchableIndexCompositeGraph.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/CollectionFetchableIndexCompositeGraph.java @@ -23,21 +23,19 @@ */ package org.hibernate.loader.plan2.build.internal.returns; -import org.hibernate.engine.FetchStrategy; -import org.hibernate.loader.plan2.build.spi.LoadPlanBuildingContext; -import org.hibernate.loader.plan2.spi.CollectionFetch; import org.hibernate.loader.plan2.spi.CollectionFetchableIndex; import org.hibernate.loader.plan2.spi.CollectionReference; import org.hibernate.loader.plan2.spi.CompositeFetch; +import org.hibernate.loader.plan2.spi.CompositeQuerySpace; import org.hibernate.loader.plan2.spi.FetchSource; import org.hibernate.loader.plan2.spi.Join; -import org.hibernate.persister.walking.spi.AssociationAttributeDefinition; -import org.hibernate.persister.walking.spi.AttributeDefinition; -import org.hibernate.persister.walking.spi.WalkingException; import org.hibernate.type.CompositeType; import org.hibernate.type.Type; /** + * Models the index graph of a collection, where the index are composite. This can only be a Map, where the keys are + * composite + * * @author Steve Ebersole */ public class CollectionFetchableIndexCompositeGraph @@ -45,17 +43,17 @@ public class CollectionFetchableIndexCompositeGraph implements CompositeFetch, CollectionFetchableIndex { private final CollectionReference collectionReference; - private final Join compositeJoin; public CollectionFetchableIndexCompositeGraph( CollectionReference collectionReference, Join compositeJoin) { super( extractIndexType( compositeJoin ), + (CompositeQuerySpace) compositeJoin.getRightHandSide(), + false, collectionReference.getPropertyPath().append( "" ) ); this.collectionReference = collectionReference; - this.compositeJoin = compositeJoin; } private static CompositeType extractIndexType(Join compositeJoin) { @@ -67,12 +65,6 @@ public class CollectionFetchableIndexCompositeGraph throw new IllegalArgumentException( "Could note extract collection composite-index" ); } - - @Override - protected String getFetchLeftHandSideUid() { - return compositeJoin.getRightHandSide().getUid(); - } - @Override public CollectionReference getCollectionReference() { return collectionReference; @@ -82,24 +74,4 @@ public class CollectionFetchableIndexCompositeGraph public FetchSource getSource() { return collectionReference.getIndexGraph(); } - - @Override - public void validateFetchPlan(FetchStrategy fetchStrategy, AttributeDefinition attributeDefinition) { - // metamodel should already disallow collections to be defined as part of a collection composite-index - // so, nothing to do here - super.validateFetchPlan( fetchStrategy, attributeDefinition ); - } - - @Override - public CollectionFetch buildCollectionFetch( - AssociationAttributeDefinition attributeDefinition, - FetchStrategy fetchStrategy, - LoadPlanBuildingContext loadPlanBuildingContext) { - throw new WalkingException( "Encountered collection as part of the Map composite-index" ); - } - - @Override - public String getQuerySpaceUid() { - return compositeJoin.getRightHandSide().getUid(); - } } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/CompositeFetchImpl.java b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/CompositeFetchImpl.java index bc51f7bf0c..79b806ce2f 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/CompositeFetchImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/CompositeFetchImpl.java @@ -25,7 +25,7 @@ package org.hibernate.loader.plan2.build.internal.returns; import org.hibernate.loader.PropertyPath; import org.hibernate.loader.plan2.spi.CompositeFetch; -import org.hibernate.loader.plan2.spi.EntityReference; +import org.hibernate.loader.plan2.spi.CompositeQuerySpace; import org.hibernate.loader.plan2.spi.FetchSource; import org.hibernate.type.CompositeType; @@ -33,28 +33,20 @@ import org.hibernate.type.CompositeType; * @author Steve Ebersole */ public class CompositeFetchImpl extends AbstractCompositeFetch implements CompositeFetch { - private final EntityReference source; + private final FetchSource source; protected CompositeFetchImpl( - EntityReference source, + FetchSource source, CompositeType compositeType, + CompositeQuerySpace compositeQuerySpace, + boolean allowCollectionFetches, PropertyPath propertyPath) { - super( compositeType, propertyPath ); + super( compositeType, compositeQuerySpace, allowCollectionFetches, propertyPath ); this.source = source; } - @Override - protected String getFetchLeftHandSideUid() { - return getSource().getQuerySpaceUid(); - } - @Override public FetchSource getSource() { return source; } - - @Override - public String getQuerySpaceUid() { - return source.getQuerySpaceUid(); - } } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/EncapsulatedEntityIdentifierDescription.java b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/EncapsulatedEntityIdentifierDescription.java index 433c269160..3340081cff 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/EncapsulatedEntityIdentifierDescription.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/EncapsulatedEntityIdentifierDescription.java @@ -30,15 +30,28 @@ import org.hibernate.loader.plan2.spi.EntityReference; import org.hibernate.type.CompositeType; /** + * Models a composite entity identifier that is encapsulated (meaning there is a composite class and a single + * attribute that encapsulates the composite value). + * * @author Steve Ebersole */ public class EncapsulatedEntityIdentifierDescription extends AbstractCompositeEntityIdentifierDescription implements ExpandingEntityIdentifierDescription { + + /** + * Build an encapsulated version of a composite EntityIdentifierDescription + * + * @param entityReference The entity whose identifier we describe + * @param compositeQuerySpace The query space we are mapped to. + * @param compositeType The type representing this composition + * @param propertyPath The property path (informational) + */ protected EncapsulatedEntityIdentifierDescription( EntityReference entityReference, CompositeQuerySpace compositeQuerySpace, - CompositeType identifierType, PropertyPath propertyPath) { - super( entityReference, compositeQuerySpace, identifierType, propertyPath ); + CompositeType compositeType, + PropertyPath propertyPath) { + super( entityReference, compositeQuerySpace, compositeType, propertyPath ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/NestedCompositeFetchImpl.java b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/NestedCompositeFetchImpl.java index b05e505a8a..acfb2d58e0 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/NestedCompositeFetchImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/NestedCompositeFetchImpl.java @@ -25,6 +25,7 @@ package org.hibernate.loader.plan2.build.internal.returns; import org.hibernate.loader.PropertyPath; import org.hibernate.loader.plan2.spi.CompositeFetch; +import org.hibernate.loader.plan2.spi.CompositeQuerySpace; import org.hibernate.loader.plan2.spi.FetchSource; import org.hibernate.type.CompositeType; @@ -33,30 +34,18 @@ import org.hibernate.type.CompositeType; */ public class NestedCompositeFetchImpl extends AbstractCompositeFetch { private final CompositeFetch source; - private final String fetchLeftHandSideUid; public NestedCompositeFetchImpl( CompositeFetch source, CompositeType type, - PropertyPath propertyPath, - String fetchLeftHandSideUid) { - super( type, propertyPath ); + CompositeQuerySpace compositeQuerySpace, + boolean allowCollectionFetches, PropertyPath propertyPath) { + super( type, compositeQuerySpace, allowCollectionFetches, propertyPath ); this.source = source; - this.fetchLeftHandSideUid = fetchLeftHandSideUid; - } - - @Override - protected String getFetchLeftHandSideUid() { - return fetchLeftHandSideUid; } @Override public FetchSource getSource() { return source; } - - @Override - public String getQuerySpaceUid() { - return source.getQuerySpaceUid(); - } } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/NonEncapsulatedEntityIdentifierDescription.java b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/NonEncapsulatedEntityIdentifierDescription.java index 29d8607d0c..b8f3304056 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/NonEncapsulatedEntityIdentifierDescription.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/internal/returns/NonEncapsulatedEntityIdentifierDescription.java @@ -29,9 +29,20 @@ import org.hibernate.loader.plan2.spi.EntityReference; import org.hibernate.type.CompositeType; /** + * Models a composite entity identifier that is non-encapsulated (meaning there is no composite class, no + * single attribute that encapsulates the composite value). + * * @author Steve Ebersole */ public class NonEncapsulatedEntityIdentifierDescription extends AbstractCompositeEntityIdentifierDescription { + /** + * Build a non-encapsulated version of a composite EntityIdentifierDescription + * + * @param entityReference The entity whose identifier we describe + * @param compositeQuerySpace The query space we are mapped to. + * @param compositeType The type representing this composition + * @param propertyPath The property path (informational) + */ public NonEncapsulatedEntityIdentifierDescription( EntityReference entityReference, CompositeQuerySpace compositeQuerySpace, diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/spi/AbstractLoadPlanBuildingAssociationVisitationStrategy.java b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/spi/AbstractLoadPlanBuildingAssociationVisitationStrategy.java index 68450639a2..2bae0cd9f3 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/spi/AbstractLoadPlanBuildingAssociationVisitationStrategy.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan2/build/spi/AbstractLoadPlanBuildingAssociationVisitationStrategy.java @@ -58,7 +58,6 @@ 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.CompositeCollectionElementDefinition; import org.hibernate.persister.walking.spi.CompositionDefinition; import org.hibernate.persister.walking.spi.EntityDefinition; import org.hibernate.persister.walking.spi.EntityIdentifierDefinition; @@ -201,7 +200,13 @@ public abstract class AbstractLoadPlanBuildingAssociationVisitationStrategy final ExpandingFetchSource fetchSource = popFromStack(); if ( ! EntityReference.class.isInstance( fetchSource ) ) { - throw new WalkingException( "Mismatched FetchSource from stack on pop" ); + throw new WalkingException( + String.format( + "Mismatched FetchSource from stack on pop. Expecting EntityReference(%s), but found %s", + entityDefinition.getEntityPersister().getEntityName(), + fetchSource + ) + ); } final EntityReference entityReference = (EntityReference) fetchSource; @@ -366,39 +371,79 @@ public abstract class AbstractLoadPlanBuildingAssociationVisitationStrategy } @Override - public void startingCollectionIndex(CollectionIndexDefinition collectionIndexDefinition) { - final Type indexType = collectionIndexDefinition.getType(); - if ( indexType.isAssociationType() || indexType.isComponentType() ) { - final CollectionReference collectionReference = collectionReferenceStack.peekFirst(); - final CollectionFetchableIndex indexGraph = collectionReference.getIndexGraph(); + public void startingCollectionIndex(CollectionIndexDefinition indexDefinition) { + final Type indexType = indexDefinition.getType(); + if ( indexType.isAnyType() ) { + throw new WalkingException( "CollectionIndexDefinition reported any-type mapping as map index" ); + } + + log.tracef( + "%s Starting collection index graph : %s", + StringHelper.repeat( ">>", fetchSourceStack.size() ), + indexDefinition.getCollectionDefinition().getCollectionPersister().getRole() + ); + + final CollectionReference collectionReference = collectionReferenceStack.peekFirst(); + final CollectionFetchableIndex indexGraph = collectionReference.getIndexGraph(); + + if ( indexType.isEntityType() || indexType.isComponentType() ) { if ( indexGraph == null ) { - throw new WalkingException( "Collection reference did not return index handler" ); + throw new WalkingException( + "CollectionReference did not return an expected index graph : " + + indexDefinition.getCollectionDefinition().getCollectionPersister().getRole() + ); } pushToStack( (ExpandingFetchSource) indexGraph ); } + else { + if ( indexGraph != null ) { + throw new WalkingException( + "CollectionReference returned an unexpected index graph : " + + indexDefinition.getCollectionDefinition().getCollectionPersister().getRole() + ); + } + } } @Override - public void finishingCollectionIndex(CollectionIndexDefinition collectionIndexDefinition) { - // nothing to do here - // - the element graph pushed while starting would be popped in finishingEntity/finishingComposite + public void finishingCollectionIndex(CollectionIndexDefinition indexDefinition) { + final Type indexType = indexDefinition.getType(); + if ( indexType.isComponentType() ) { + // todo : validate the stack? + popFromStack(); + + } + + // entity indexes would be popped during finishingEntity processing + + log.tracef( + "%s Finished collection index graph : %s", + StringHelper.repeat( "<<", fetchSourceStack.size() ), + indexDefinition.getCollectionDefinition().getCollectionPersister().getRole() + ); } @Override public void startingCollectionElements(CollectionElementDefinition elementDefinition) { + final Type elementType = elementDefinition.getType(); + if ( elementType.isAnyType() ) { + return; + } + + log.tracef( + "%s Starting collection element graph : %s", + StringHelper.repeat( ">>", fetchSourceStack.size() ), + elementDefinition.getCollectionDefinition().getCollectionPersister().getRole() + ); + final CollectionReference collectionReference = collectionReferenceStack.peekFirst(); final CollectionFetchableElement elementGraph = collectionReference.getElementGraph(); - final Type elementType = elementDefinition.getType(); - final boolean expectFetchSourceElements = - ( elementType.isAssociationType() || elementType.isComponentType() ) - && ! elementType.isAnyType(); - - if ( expectFetchSourceElements ) { + if ( elementType.isAssociationType() || elementType.isComponentType() ) { if ( elementGraph == null ) { throw new IllegalStateException( - "Expecting CollectionFetchableElement, but element graph was null : " - + elementDefinition.getCollectionDefinition().getCollectionPersister().getRole() + "CollectionReference did not return an expected element graph : " + + elementDefinition.getCollectionDefinition().getCollectionPersister().getRole() ); } @@ -407,8 +452,8 @@ public abstract class AbstractLoadPlanBuildingAssociationVisitationStrategy else { if ( elementGraph != null ) { throw new IllegalStateException( - "Not expecting CollectionFetchableElement, but element graph was non-null : " - + elementDefinition.getCollectionDefinition().getCollectionPersister().getRole() + "CollectionReference returned an unexpected element graph : " + + elementDefinition.getCollectionDefinition().getCollectionPersister().getRole() ); } } @@ -416,41 +461,31 @@ public abstract class AbstractLoadPlanBuildingAssociationVisitationStrategy @Override public void finishingCollectionElements(CollectionElementDefinition elementDefinition) { - // nothing to do here - // - the element graph pushed while starting would be popped in finishing/Entity/finishingComposite - } + final Type elementType = elementDefinition.getType(); + if ( elementType.isComponentType() ) { + // pop it from the stack + final ExpandingFetchSource popped = popFromStack(); - @Override - public void startingCompositeCollectionElement(CompositeCollectionElementDefinition compositeElementDefinition) { - log.tracef( - "%s Starting composite collection element for (%s)", - StringHelper.repeat( ">>", fetchSourceStack.size() ), - compositeElementDefinition.getCollectionDefinition().getCollectionPersister().getRole() - ); - } + // validation - @Override - public void finishingCompositeCollectionElement(CompositeCollectionElementDefinition compositeElementDefinition) { - // pop the current fetch owner, and make sure what we just popped represents this composition - final ExpandingFetchSource popped = popFromStack(); - - if ( ! CollectionFetchableElement.class.isInstance( popped ) ) { - throw new WalkingException( "Mismatched FetchSource from stack on pop" ); + if ( ! CollectionFetchableElement.class.isInstance( popped ) ) { + throw new WalkingException( "Mismatched FetchSource from stack on pop" ); + } } - // NOTE : not much else we can really check here atm since on the walking spi side we do not have path + // entity indexes would be popped during finishingEntity processing log.tracef( - "%s Finished composite element for : %s", + "%s Finished collection element graph : %s", StringHelper.repeat( "<<", fetchSourceStack.size() ), - compositeElementDefinition.getCollectionDefinition().getCollectionPersister().getRole() + elementDefinition.getCollectionDefinition().getCollectionPersister().getRole() ); } @Override public void startingComposite(CompositionDefinition compositionDefinition) { log.tracef( - "%s Starting composition : %s", + "%s Starting composite : %s", StringHelper.repeat( ">>", fetchSourceStack.size() ), compositionDefinition.getName() ); @@ -458,6 +493,9 @@ public abstract class AbstractLoadPlanBuildingAssociationVisitationStrategy if ( fetchSourceStack.isEmpty() ) { throw new HibernateException( "A component cannot be the root of a walk nor a graph" ); } + + final CompositeFetch compositeFetch = currentSource().buildCompositeFetch( compositionDefinition, this ); + pushToStack( (ExpandingFetchSource) compositeFetch ); } @Override @@ -469,10 +507,8 @@ public abstract class AbstractLoadPlanBuildingAssociationVisitationStrategy throw new WalkingException( "Mismatched FetchSource from stack on pop" ); } - // NOTE : not much else we can really check here atm since on the walking spi side we do not have path - log.tracef( - "%s Finished composition : %s", + "%s Finishing composite : %s", StringHelper.repeat( "<<", fetchSourceStack.size() ), compositionDefinition.getName() ); @@ -672,9 +708,9 @@ public abstract class AbstractLoadPlanBuildingAssociationVisitationStrategy } protected boolean handleCompositeAttribute(CompositionDefinition attributeDefinition) { - final ExpandingFetchSource currentSource = currentSource(); - final CompositeFetch fetch = currentSource.buildCompositeFetch( attributeDefinition, this ); - pushToStack( (ExpandingFetchSource) fetch ); +// final ExpandingFetchSource currentSource = currentSource(); +// final CompositeFetch fetch = currentSource.buildCompositeFetch( attributeDefinition, this ); +// pushToStack( (ExpandingFetchSource) fetch ); return true; } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan2/exec/internal/LoadQueryJoinAndFetchProcessor.java b/hibernate-core/src/main/java/org/hibernate/loader/plan2/exec/internal/LoadQueryJoinAndFetchProcessor.java index e6d1f52fdb..724afb1dbc 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan2/exec/internal/LoadQueryJoinAndFetchProcessor.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan2/exec/internal/LoadQueryJoinAndFetchProcessor.java @@ -272,7 +272,10 @@ public class LoadQueryJoinAndFetchProcessor { // // long story short, for now we'll use an assumption that the last join in the CollectionQuerySpace is the // element join (that's how the joins are built as of now..) - // todo : remove this assumption ^^ + // + // todo : remove this assumption ^^; maybe we make CollectionQuerySpace "special" and rather than have it + // hold a list of joins, we have it expose the 2 (index, element) separately. + Join collectionElementJoin = null; for ( Join collectionJoin : rightHandSide.getJoins() ) { collectionElementJoin = collectionJoin; diff --git a/hibernate-core/src/main/java/org/hibernate/persister/walking/spi/AssociationVisitationStrategy.java b/hibernate-core/src/main/java/org/hibernate/persister/walking/spi/AssociationVisitationStrategy.java index e8add991f4..8f8a354315 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/walking/spi/AssociationVisitationStrategy.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/walking/spi/AssociationVisitationStrategy.java @@ -49,7 +49,7 @@ public interface AssociationVisitationStrategy { /** * Notification we are starting to walk an entity. * - * @param entityDefinition The entity we are starting to walk + * @param entityDefinition The entity we are preparing to walk */ public void startingEntity(EntityDefinition entityDefinition); @@ -63,7 +63,7 @@ public interface AssociationVisitationStrategy { /** * Notification we are starting to walk the identifier of an entity. * - * @param entityIdentifierDefinition The identifier we are starting to walk + * @param entityIdentifierDefinition The identifier we are preparing to walk */ public void startingEntityIdentifier(EntityIdentifierDefinition entityIdentifierDefinition); @@ -74,22 +74,93 @@ public interface AssociationVisitationStrategy { */ public void finishingEntityIdentifier(EntityIdentifierDefinition entityIdentifierDefinition); + /** + * Notification that we are starting to walk a collection + * + * @param collectionDefinition The collection we are preparing to walk + */ public void startingCollection(CollectionDefinition collectionDefinition); + + /** + * Notification that we are finishing walking a collection + * + * @param collectionDefinition The collection we are finishing + */ public void finishingCollection(CollectionDefinition collectionDefinition); + /** + * Notification that we are starting to walk the index of a collection (List/Map). In the case of a Map, + * if the indices (the keys) are entities this will be followed up by a call to {@link #startingEntity} + * + * @param collectionIndexDefinition The collection index we are preparing to walk. + */ public void startingCollectionIndex(CollectionIndexDefinition collectionIndexDefinition); + + /** + * Notification that we are finishing walking the index of a collection (List/Map). + * + * @param collectionIndexDefinition The collection index we are finishing + */ public void finishingCollectionIndex(CollectionIndexDefinition collectionIndexDefinition); + /** + * Notification that we are starting to look at the element definition for the collection. If the collection + * elements are entities this will be followed up by a call to {@link #startingEntity} + * + * @param elementDefinition The collection element we are preparing to walk.. + */ public void startingCollectionElements(CollectionElementDefinition elementDefinition); + + /** + * Notification that we are finishing walking the elements of a collection (List/Map). + * + * @param elementDefinition The collection element we are finishing + */ public void finishingCollectionElements(CollectionElementDefinition elementDefinition); + /** + * Notification that we are preparing to walk a composite. This is called only for:
    + *
  • + * top-level composites for entity attributes. composite entity identifiers do not route through here, see + * {@link #startingEntityIdentifier} if you need to hook into walking the top-level cid composite. + *
    • + *
  • + * All forms of nested composite paths + *
    • + *
+ * + * @param compositionDefinition The composite we are preparing to walk. + */ public void startingComposite(CompositionDefinition compositionDefinition); + + /** + * Notification that we are done walking a composite. Called on the back-end of the situations listed + * on {@link #startingComposite} + * + * @param compositionDefinition The composite we are finishing + */ public void finishingComposite(CompositionDefinition compositionDefinition); - public void startingCompositeCollectionElement(CompositeCollectionElementDefinition compositionElementDefinition); - public void finishingCompositeCollectionElement(CompositeCollectionElementDefinition compositionElementDefinition); + // get rid of these ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +// public void startingCompositeCollectionElement(CompositeCollectionElementDefinition compositionElementDefinition); +// public void finishingCompositeCollectionElement(CompositeCollectionElementDefinition compositionElementDefinition); + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + /** + * Notification that we are preparing to walk an attribute. May be followed by calls to {@link #startingEntity} + * (one-to-one, many-to-one), {@link #startingComposite}, or {@link #startingCollection}. + * + * @param attributeDefinition The attribute we are preparing to walk. + * + * @return {@code true} if the walking should continue; {@code false} if walking should stop. + */ public boolean startingAttribute(AttributeDefinition attributeDefinition); + + /** + * Notification that we are finishing walking an attribute. + * + * @param attributeDefinition The attribute we are done walking + */ public void finishingAttribute(AttributeDefinition attributeDefinition); public void foundAny(AssociationAttributeDefinition attributeDefinition, AnyMappingDefinition anyDefinition); diff --git a/hibernate-core/src/main/java/org/hibernate/persister/walking/spi/MetamodelGraphWalker.java b/hibernate-core/src/main/java/org/hibernate/persister/walking/spi/MetamodelGraphWalker.java index 6e891c8aa6..b808b8dd18 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/walking/spi/MetamodelGraphWalker.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/walking/spi/MetamodelGraphWalker.java @@ -118,6 +118,8 @@ public class MetamodelGraphWalker { // to make encapsulated and non-encapsulated composite identifiers work the same here, we "cheat" here a // little bit and simply walk the attributes of the composite id in both cases. + // this works because the LoadPlans already build the top-level composite for composite ids + if ( identifierDefinition.isEncapsulated() ) { // in the encapsulated composite id case that means we have a little bit of duplication between here and // visitCompositeDefinition, but in the spirit of consistently handling composite ids, that is much better @@ -125,12 +127,12 @@ public class MetamodelGraphWalker { final EncapsulatedEntityIdentifierDefinition idAsEncapsulated = (EncapsulatedEntityIdentifierDefinition) identifierDefinition; final AttributeDefinition idAttr = idAsEncapsulated.getAttributeDefinition(); if ( CompositionDefinition.class.isInstance( idAttr ) ) { - visitCompositeDefinition( (CompositionDefinition) idAttr ); + visitAttributes( (CompositionDefinition) idAttr ); } } else { // NonEncapsulatedEntityIdentifierDefinition itself is defined as a CompositionDefinition - visitCompositeDefinition( (NonEncapsulatedEntityIdentifierDefinition) identifierDefinition ); + visitAttributes( (NonEncapsulatedEntityIdentifierDefinition) identifierDefinition ); } strategy.finishingEntityIdentifier( identifierDefinition ); @@ -252,7 +254,7 @@ public class MetamodelGraphWalker { strategy.startingCollectionElements( elementDefinition ); if ( elementDefinition.getType().isComponentType() ) { - visitCompositeCollectionElementDefinition( elementDefinition.toCompositeElementDefinition() ); + visitAttributes( elementDefinition.toCompositeElementDefinition() ); } else if ( elementDefinition.getType().isEntityType() ) { visitEntityDefinition( elementDefinition.toEntityDefinition() ); @@ -261,14 +263,6 @@ public class MetamodelGraphWalker { strategy.finishingCollectionElements( elementDefinition ); } - private void visitCompositeCollectionElementDefinition(CompositeCollectionElementDefinition compositionElementDefinition) { - strategy.startingCompositeCollectionElement( compositionElementDefinition ); - - visitAttributes( compositionElementDefinition ); - - strategy.finishingCompositeCollectionElement( compositionElementDefinition ); - } - private final Set visitedAssociationKeys = new HashSet(); /** diff --git a/hibernate-core/src/test/java/org/hibernate/test/loadplans/plans/LoadPlanStructureAssertionTest.java b/hibernate-core/src/test/java/org/hibernate/test/loadplans/plans/LoadPlanStructureAssertionTest.java index cabb5798fb..48279c94c9 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/loadplans/plans/LoadPlanStructureAssertionTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/loadplans/plans/LoadPlanStructureAssertionTest.java @@ -64,10 +64,15 @@ public class LoadPlanStructureAssertionTest extends BaseUnitTestCase { cfg.addResource( "org/hibernate/test/onetoone/joined/Person.hbm.xml" ); SessionFactoryImplementor sf = (SessionFactoryImplementor) cfg.buildSessionFactory(); -// doCompare( sf, (OuterJoinLoadable) sf.getClassMetadata( org.hibernate.test.onetoone.joined.Person.class ) ); - doCompare( sf, (OuterJoinLoadable) sf.getClassMetadata( org.hibernate.test.onetoone.joined.Entity.class ) ); + try { +// doCompare( sf, (OuterJoinLoadable) sf.getClassMetadata( org.hibernate.test.onetoone.joined.Person.class ) ); + doCompare( sf, (OuterJoinLoadable) sf.getClassMetadata( org.hibernate.test.onetoone.joined.Entity.class ) ); -// doCompare( sf, (OuterJoinLoadable) sf.getClassMetadata( org.hibernate.test.onetoone.joined.Address.class ) ); +// doCompare( sf, (OuterJoinLoadable) sf.getClassMetadata( org.hibernate.test.onetoone.joined.Address.class ) ); + } + finally { + sf.close(); + } } @Test @@ -77,7 +82,12 @@ public class LoadPlanStructureAssertionTest extends BaseUnitTestCase { cfg.addResource( "org/hibernate/test/onetoone/formula/Person.hbm.xml" ); SessionFactoryImplementor sf = (SessionFactoryImplementor) cfg.buildSessionFactory(); - doCompare( sf, (OuterJoinLoadable) sf.getClassMetadata( org.hibernate.test.onetoone.formula.Person.class ) ); + try { + doCompare( sf, (OuterJoinLoadable) sf.getClassMetadata( org.hibernate.test.onetoone.formula.Person.class ) ); + } + finally { + sf.close(); + } } @Test @@ -88,8 +98,14 @@ public class LoadPlanStructureAssertionTest extends BaseUnitTestCase { cfg.addAnnotatedClass( EncapsulatedCompositeIdResultSetProcessorTest.CardField.class ); cfg.addAnnotatedClass( EncapsulatedCompositeIdResultSetProcessorTest.Card.class ); SessionFactoryImplementor sf = (SessionFactoryImplementor) cfg.buildSessionFactory(); - doCompare( sf, (OuterJoinLoadable) sf.getClassMetadata( EncapsulatedCompositeIdResultSetProcessorTest.CardField.class ) ); - doCompare( sf, (OuterJoinLoadable) sf.getClassMetadata( EncapsulatedCompositeIdResultSetProcessorTest.Card.class ) ); + + try { + doCompare( sf, (OuterJoinLoadable) sf.getClassMetadata( EncapsulatedCompositeIdResultSetProcessorTest.CardField.class ) ); + doCompare( sf, (OuterJoinLoadable) sf.getClassMetadata( EncapsulatedCompositeIdResultSetProcessorTest.Card.class ) ); + } + finally { + sf.close(); + } } @Test @@ -99,7 +115,13 @@ public class LoadPlanStructureAssertionTest extends BaseUnitTestCase { Configuration cfg = new Configuration(); cfg.addAnnotatedClass( EncapsulatedCompositeIdResultSetProcessorTest.Parent.class ); SessionFactoryImplementor sf = (SessionFactoryImplementor) cfg.buildSessionFactory(); - doCompare( sf, (OuterJoinLoadable) sf.getClassMetadata( EncapsulatedCompositeIdResultSetProcessorTest.Parent.class ) ); + + try { + doCompare( sf, (OuterJoinLoadable) sf.getClassMetadata( EncapsulatedCompositeIdResultSetProcessorTest.Parent.class ) ); + } + finally { + sf.close(); + } } @Test @@ -112,48 +134,53 @@ public class LoadPlanStructureAssertionTest extends BaseUnitTestCase { SessionFactoryImplementor sf = (SessionFactoryImplementor) cfg.buildSessionFactory(); - final OuterJoinLoadable cardFieldPersister = (OuterJoinLoadable) sf.getClassMetadata( CardField.class ); - doCompare( sf, cardFieldPersister ); + try { + final OuterJoinLoadable cardFieldPersister = (OuterJoinLoadable) sf.getClassMetadata( CardField.class ); + doCompare( sf, cardFieldPersister ); - final LoadPlan loadPlan = LoadPlanStructureAssertionHelper.INSTANCE.buildLoadPlan( sf, cardFieldPersister ); - assertEquals( LoadPlan.Disposition.ENTITY_LOADER, loadPlan.getDisposition() ); - assertEquals( 1, loadPlan.getReturns().size() ); - final EntityReturn cardFieldReturn = assertTyping( EntityReturn.class, loadPlan.getReturns().get( 0 ) ); - assertEquals( 0, cardFieldReturn.getFetches().length ); + final LoadPlan loadPlan = LoadPlanStructureAssertionHelper.INSTANCE.buildLoadPlan( sf, cardFieldPersister ); + assertEquals( LoadPlan.Disposition.ENTITY_LOADER, loadPlan.getDisposition() ); + assertEquals( 1, loadPlan.getReturns().size() ); + final EntityReturn cardFieldReturn = assertTyping( EntityReturn.class, loadPlan.getReturns().get( 0 ) ); + assertEquals( 0, cardFieldReturn.getFetches().length ); - // CardField defines a composite pk with 2 fetches : Card and Key (the id description acts as the composite) - assertTrue( cardFieldReturn.getIdentifierDescription().hasFetches() ); - final FetchSource cardFieldIdAsFetchSource = assertTyping( FetchSource.class, cardFieldReturn.getIdentifierDescription() ); - assertEquals( 2, cardFieldIdAsFetchSource.getFetches().length ); + // CardField defines a composite pk with 2 fetches : Card and Key (the id description acts as the composite) + assertTrue( cardFieldReturn.getIdentifierDescription().hasFetches() ); + final FetchSource cardFieldIdAsFetchSource = assertTyping( FetchSource.class, cardFieldReturn.getIdentifierDescription() ); + assertEquals( 2, cardFieldIdAsFetchSource.getFetches().length ); - // First the key-many-to-one to Card... - final EntityFetch cardFieldIdCardFetch = assertTyping( - EntityFetch.class, - cardFieldIdAsFetchSource.getFetches()[0] - ); - assertFalse( cardFieldIdCardFetch.getIdentifierDescription().hasFetches() ); - // i think this one might be a mistake; i think the collection reader still needs to be registered. Its zero - // because the inverse of the key-many-to-one already had a registered AssociationKey and so saw the - // CollectionFetch as a circularity (I think) - assertEquals( 0, cardFieldIdCardFetch.getFetches().length ); + // First the key-many-to-one to Card... + final EntityFetch cardFieldIdCardFetch = assertTyping( + EntityFetch.class, + cardFieldIdAsFetchSource.getFetches()[0] + ); + assertFalse( cardFieldIdCardFetch.getIdentifierDescription().hasFetches() ); + // i think this one might be a mistake; i think the collection reader still needs to be registered. Its zero + // because the inverse of the key-many-to-one already had a registered AssociationKey and so saw the + // CollectionFetch as a circularity (I think) + assertEquals( 0, cardFieldIdCardFetch.getFetches().length ); - // then the Key.. - final EntityFetch cardFieldIdKeyFetch = assertTyping( - EntityFetch.class, - cardFieldIdAsFetchSource.getFetches()[1] - ); - assertFalse( cardFieldIdKeyFetch.getIdentifierDescription().hasFetches() ); - assertEquals( 0, cardFieldIdKeyFetch.getFetches().length ); + // then the Key.. + final EntityFetch cardFieldIdKeyFetch = assertTyping( + EntityFetch.class, + cardFieldIdAsFetchSource.getFetches()[1] + ); + assertFalse( cardFieldIdKeyFetch.getIdentifierDescription().hasFetches() ); + assertEquals( 0, cardFieldIdKeyFetch.getFetches().length ); - // we need the readers ordered in a certain manner. Here specifically: Fetch(Card), Fetch(Key), Return(CardField) - // - // additionally, we need Fetch(Card) and Fetch(Key) to be hydrated/semi-resolved before attempting to - // resolve the EntityKey for Return(CardField) - // - // together those sound like argument enough to continue keeping readers for "identifier fetches" as part of - // a special "identifier reader". generated aliases could help here too to remove cyclic-ness from the graph. - // but at any rate, we need to know still when this becomes circularity + // we need the readers ordered in a certain manner. Here specifically: Fetch(Card), Fetch(Key), Return(CardField) + // + // additionally, we need Fetch(Card) and Fetch(Key) to be hydrated/semi-resolved before attempting to + // resolve the EntityKey for Return(CardField) + // + // together those sound like argument enough to continue keeping readers for "identifier fetches" as part of + // a special "identifier reader". generated aliases could help here too to remove cyclic-ness from the graph. + // but at any rate, we need to know still when this becomes circularity + } + finally { + sf.close(); + } } @Test @@ -165,46 +192,52 @@ public class LoadPlanStructureAssertionTest extends BaseUnitTestCase { cfg.addAnnotatedClass( PrimaryKey.class ); final SessionFactoryImplementor sf = (SessionFactoryImplementor) cfg.buildSessionFactory(); - final OuterJoinLoadable cardPersister = (OuterJoinLoadable) sf.getClassMetadata( Card.class ); - doCompare( sf, cardPersister ); - final LoadPlan cardLoadPlan = LoadPlanStructureAssertionHelper.INSTANCE.buildLoadPlan( sf, cardPersister ); - assertEquals( LoadPlan.Disposition.ENTITY_LOADER, cardLoadPlan.getDisposition() ); - assertEquals( 1, cardLoadPlan.getReturns().size() ); + try { + final OuterJoinLoadable cardPersister = (OuterJoinLoadable) sf.getClassMetadata( Card.class ); + doCompare( sf, cardPersister ); - // Check the root EntityReturn(Card) - final EntityReturn cardReturn = assertTyping( EntityReturn.class, cardLoadPlan.getReturns().get( 0 ) ); - assertFalse( cardReturn.getIdentifierDescription().hasFetches() ); + final LoadPlan cardLoadPlan = LoadPlanStructureAssertionHelper.INSTANCE.buildLoadPlan( sf, cardPersister ); + assertEquals( LoadPlan.Disposition.ENTITY_LOADER, cardLoadPlan.getDisposition() ); + assertEquals( 1, cardLoadPlan.getReturns().size() ); - // Card should have one fetch, the fields collection - assertEquals( 1, cardReturn.getFetches().length ); - final CollectionFetch fieldsFetch = assertTyping( CollectionFetch.class, cardReturn.getFetches()[0] ); - assertNotNull( fieldsFetch.getElementGraph() ); + // Check the root EntityReturn(Card) + final EntityReturn cardReturn = assertTyping( EntityReturn.class, cardLoadPlan.getReturns().get( 0 ) ); + assertFalse( cardReturn.getIdentifierDescription().hasFetches() ); - // the Card.fields collection has entity elements of type CardField... - final CollectionFetchableElementEntityGraph cardFieldElementGraph = assertTyping( CollectionFetchableElementEntityGraph.class, fieldsFetch.getElementGraph() ); - // CardField should have no fetches - assertEquals( 0, cardFieldElementGraph.getFetches().length ); - // But it should have 1 key-many-to-one fetch for Key (Card is already handled) - assertTrue( cardFieldElementGraph.getIdentifierDescription().hasFetches() ); - final FetchSource cardFieldElementGraphIdAsFetchSource = assertTyping( - FetchSource.class, - cardFieldElementGraph.getIdentifierDescription() - ); - assertEquals( 1, cardFieldElementGraphIdAsFetchSource.getFetches().length ); + // Card should have one fetch, the fields collection + assertEquals( 1, cardReturn.getFetches().length ); + final CollectionFetch fieldsFetch = assertTyping( CollectionFetch.class, cardReturn.getFetches()[0] ); + assertNotNull( fieldsFetch.getElementGraph() ); -// BidirectionalEntityFetch circularCardFetch = assertTyping( -// BidirectionalEntityFetch.class, -// fieldsElementCompositeIdFetch.getFetches()[0] -// ); -// assertSame( circularCardFetch.getTargetEntityReference(), cardReturn ); + // the Card.fields collection has entity elements of type CardField... + final CollectionFetchableElementEntityGraph cardFieldElementGraph = assertTyping( CollectionFetchableElementEntityGraph.class, fieldsFetch.getElementGraph() ); + // CardField should have no fetches + assertEquals( 0, cardFieldElementGraph.getFetches().length ); + // But it should have 1 key-many-to-one fetch for Key (Card is already handled) + assertTrue( cardFieldElementGraph.getIdentifierDescription().hasFetches() ); + final FetchSource cardFieldElementGraphIdAsFetchSource = assertTyping( + FetchSource.class, + cardFieldElementGraph.getIdentifierDescription() + ); + assertEquals( 1, cardFieldElementGraphIdAsFetchSource.getFetches().length ); - // the fetch above is to the other key-many-to-one for CardField.primaryKey composite: key - EntityFetch keyFetch = assertTyping( - EntityFetch.class, - cardFieldElementGraphIdAsFetchSource.getFetches()[0] - ); - assertEquals( Key.class.getName(), keyFetch.getEntityPersister().getEntityName() ); +// BidirectionalEntityFetch circularCardFetch = assertTyping( +// BidirectionalEntityFetch.class, +// fieldsElementCompositeIdFetch.getFetches()[0] +// ); +// assertSame( circularCardFetch.getTargetEntityReference(), cardReturn ); + + // the fetch above is to the other key-many-to-one for CardField.primaryKey composite: key + EntityFetch keyFetch = assertTyping( + EntityFetch.class, + cardFieldElementGraphIdAsFetchSource.getFetches()[0] + ); + assertEquals( Key.class.getName(), keyFetch.getEntityPersister().getEntityName() ); + } + finally { + sf.close(); + } } @Test @@ -212,7 +245,13 @@ public class LoadPlanStructureAssertionTest extends BaseUnitTestCase { Configuration cfg = new Configuration(); cfg.addResource( "org/hibernate/test/immutable/entitywithmutablecollection/inverse/ContractVariation.hbm.xml" ); SessionFactoryImplementor sf = (SessionFactoryImplementor) cfg.buildSessionFactory(); - doCompare( sf, (OuterJoinLoadable) sf.getClassMetadata( org.hibernate.test.immutable.entitywithmutablecollection.Contract.class ) ); + + try { + doCompare( sf, (OuterJoinLoadable) sf.getClassMetadata( org.hibernate.test.immutable.entitywithmutablecollection.Contract.class ) ); + } + finally { + sf.close(); + } } diff --git a/hibernate-core/src/test/java/org/hibernate/test/loadplans/walking/CompositesWalkingTest.java b/hibernate-core/src/test/java/org/hibernate/test/loadplans/walking/CompositesWalkingTest.java index 56cc34100a..a29b3b2595 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/loadplans/walking/CompositesWalkingTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/loadplans/walking/CompositesWalkingTest.java @@ -45,8 +45,12 @@ public class CompositesWalkingTest extends BaseUnitTestCase { final SessionFactory sf = new Configuration() .addAnnotatedClass( TestCourse.class ) .buildSessionFactory(); - final EntityPersister ep = (EntityPersister) sf.getClassMetadata( TestCourse.class ); - - MetamodelGraphWalker.visitEntity( new LoggingAssociationVisitationStrategy(), ep ); + try { + final EntityPersister ep = (EntityPersister) sf.getClassMetadata( TestCourse.class ); + MetamodelGraphWalker.visitEntity( new LoggingAssociationVisitationStrategy(), ep ); + } + finally { + sf.close(); + } } } diff --git a/hibernate-core/src/test/java/org/hibernate/test/loadplans/walking/LoggingAssociationVisitationStrategy.java b/hibernate-core/src/test/java/org/hibernate/test/loadplans/walking/LoggingAssociationVisitationStrategy.java index fd4991e196..8713388626 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/loadplans/walking/LoggingAssociationVisitationStrategy.java +++ b/hibernate-core/src/test/java/org/hibernate/test/loadplans/walking/LoggingAssociationVisitationStrategy.java @@ -131,7 +131,7 @@ public class LoggingAssociationVisitationStrategy implements AssociationVisitati System.out.println( String.format( "%s Finishing composite (%s)", - StringHelper.repeat( ">>", depth-- ), + StringHelper.repeat( "<<", depth-- ), compositionDefinition.getName() ) ); @@ -206,28 +206,28 @@ public class LoggingAssociationVisitationStrategy implements AssociationVisitati // why do we have these + startingCollectionElements/finishingCollectionElements ??? - - @Override - public void startingCompositeCollectionElement(CompositeCollectionElementDefinition compositionElementDefinition) { - System.out.println( - String.format( - "%s Starting composite (%s)", - StringHelper.repeat( ">>", ++depth ), - compositionElementDefinition.getCollectionDefinition().getCollectionPersister().getRole() - ) - ); - } - - @Override - public void finishingCompositeCollectionElement(CompositeCollectionElementDefinition compositionElementDefinition) { - System.out.println( - String.format( - "%s Finishing composite (%s)", - StringHelper.repeat( "<<", depth-- ), - compositionElementDefinition.getCollectionDefinition().getCollectionPersister().getRole() - ) - ); - } +// +// @Override +// public void startingCompositeCollectionElement(CompositeCollectionElementDefinition compositionElementDefinition) { +// System.out.println( +// String.format( +// "%s Starting composite (%s)", +// StringHelper.repeat( ">>", ++depth ), +// compositionElementDefinition.getCollectionDefinition().getCollectionPersister().getRole() +// ) +// ); +// } +// +// @Override +// public void finishingCompositeCollectionElement(CompositeCollectionElementDefinition compositionElementDefinition) { +// System.out.println( +// String.format( +// "%s Finishing composite (%s)", +// StringHelper.repeat( "<<", depth-- ), +// compositionElementDefinition.getCollectionDefinition().getCollectionPersister().getRole() +// ) +// ); +// } @Override public void foundAny(AssociationAttributeDefinition attributeDefinition, AnyMappingDefinition anyDefinition) { diff --git a/hibernate-core/src/test/java/org/hibernate/test/loadplans/walking/NestedCompositeElementTest.java b/hibernate-core/src/test/java/org/hibernate/test/loadplans/walking/NestedCompositeElementTest.java new file mode 100644 index 0000000000..40621e7b40 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/loadplans/walking/NestedCompositeElementTest.java @@ -0,0 +1,49 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2013, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * 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, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY 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 + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.test.loadplans.walking; + +import org.hibernate.persister.entity.EntityPersister; +import org.hibernate.persister.walking.spi.MetamodelGraphWalker; + +import org.junit.Test; + +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.hibernate.test.loadplans.process.EncapsulatedCompositeAttributeResultSetProcessorTest.Person; +import org.hibernate.test.loadplans.process.EncapsulatedCompositeAttributeResultSetProcessorTest.Customer; + +/** + * @author Steve Ebersole + */ +public class NestedCompositeElementTest extends BaseCoreFunctionalTestCase { + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { Person.class, Customer.class }; + } + + @Test + public void testWalkingKeyManyToOneGraphs() { + final EntityPersister ep = (EntityPersister) sessionFactory().getClassMetadata( Customer.class ); + MetamodelGraphWalker.visitEntity( new LoggingAssociationVisitationStrategy(), ep ); + } +}