From 4fd933d526647661d3cc3c9e02c2a7c20679e393 Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Mon, 18 Aug 2014 14:00:02 -0700 Subject: [PATCH] HHH-8839 : Eager Map with entity key causes IllegalStateException: Collection element (many-to-many) table alias cannot be empty (cherry picked from commit 35edd5690700fdea153fa20487c2c7e3271dc2f2) --- .../spaces/CollectionQuerySpaceImpl.java | 35 ++++ .../spi/ExpandingCollectionQuerySpace.java | 18 ++ .../LoadQueryJoinAndFetchProcessor.java | 70 ++++++-- .../indexcoll/eager/Atmosphere.java | 84 +++++++++ .../eager/EagerIndexedCollectionTest.java | 170 ++++++++++++++++++ 5 files changed, 359 insertions(+), 18 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/eager/Atmosphere.java create mode 100644 hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/eager/EagerIndexedCollectionTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan/build/internal/spaces/CollectionQuerySpaceImpl.java b/hibernate-core/src/main/java/org/hibernate/loader/plan/build/internal/spaces/CollectionQuerySpaceImpl.java index e8eab8310e..58b0aadcd0 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan/build/internal/spaces/CollectionQuerySpaceImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan/build/internal/spaces/CollectionQuerySpaceImpl.java @@ -26,6 +26,7 @@ package org.hibernate.loader.plan.build.internal.spaces; import org.hibernate.loader.plan.build.spi.ExpandingCollectionQuerySpace; import org.hibernate.loader.plan.build.spi.ExpandingQuerySpaces; import org.hibernate.loader.plan.spi.Join; +import org.hibernate.loader.plan.spi.JoinDefinedByMetadata; import org.hibernate.persister.collection.CollectionPersister; import org.hibernate.persister.collection.CollectionPropertyNames; import org.hibernate.persister.collection.QueryableCollection; @@ -36,6 +37,8 @@ import org.hibernate.persister.entity.PropertyMapping; */ public class CollectionQuerySpaceImpl extends AbstractQuerySpace implements ExpandingCollectionQuerySpace { private final CollectionPersister persister; + private JoinDefinedByMetadata elementJoin; + private JoinDefinedByMetadata indexJoin; public CollectionQuerySpaceImpl( CollectionPersister persister, @@ -78,6 +81,35 @@ public class CollectionQuerySpaceImpl extends AbstractQuerySpace implements Expa @Override public void addJoin(Join join) { + if ( JoinDefinedByMetadata.class.isInstance( join ) ) { + final JoinDefinedByMetadata joinDefinedByMetadata = (JoinDefinedByMetadata) join; + if ( joinDefinedByMetadata.getJoinedPropertyName().equals( CollectionPropertyNames.COLLECTION_ELEMENTS ) ) { + if ( elementJoin == null ) { + elementJoin = joinDefinedByMetadata; + } + else { + throw new IllegalStateException( "Attempt to add an element join, but an element join already exists." ); + } + } + else if ( joinDefinedByMetadata.getJoinedPropertyName().equals( CollectionPropertyNames.COLLECTION_INDICES ) ) { + if ( indexJoin == null ) { + indexJoin = joinDefinedByMetadata; + } + else { + throw new IllegalStateException( "Attempt to add an index join, but an index join already exists." ); + } + } + else { + throw new IllegalArgumentException( + String.format( + "Collection propertyName must be either %s or %s; instead the joined property name was %s.", + CollectionPropertyNames.COLLECTION_ELEMENTS, + CollectionPropertyNames.COLLECTION_INDICES, + joinDefinedByMetadata.getJoinedPropertyName() + ) + ); + } + } internalGetJoins().add( join ); } @@ -86,4 +118,7 @@ public class CollectionQuerySpaceImpl extends AbstractQuerySpace implements Expa return super.getExpandingQuerySpaces(); } + public void addJoin(JoinDefinedByMetadata join) { + addJoin( (Join) join ); + } } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan/build/spi/ExpandingCollectionQuerySpace.java b/hibernate-core/src/main/java/org/hibernate/loader/plan/build/spi/ExpandingCollectionQuerySpace.java index 71d557cc0e..3baccd5cef 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan/build/spi/ExpandingCollectionQuerySpace.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan/build/spi/ExpandingCollectionQuerySpace.java @@ -24,6 +24,9 @@ package org.hibernate.loader.plan.build.spi; import org.hibernate.loader.plan.spi.CollectionQuerySpace; +import org.hibernate.loader.plan.spi.Join; +import org.hibernate.loader.plan.spi.JoinDefinedByMetadata; +import org.hibernate.persister.collection.CollectionPropertyNames; /** * Describes a collection query space that allows adding joins with other @@ -34,4 +37,19 @@ import org.hibernate.loader.plan.spi.CollectionQuerySpace; * @author Gail Badner */ public interface ExpandingCollectionQuerySpace extends CollectionQuerySpace, ExpandingQuerySpace { + + /** + * Adds a join with another query space for either a collection element or index. If {@code join} + * is an instance of {@link JoinDefinedByMetadata}, then the only valid values returned by + * {@link JoinDefinedByMetadata#getJoinedPropertyName} are {@link CollectionPropertyNames#COLLECTION_ELEMENTS} + * and {@link CollectionPropertyNames#COLLECTION_INDICES}, for the collection element or index, respectively. + * + * @param join The element or index join to add. + * + * @throws java.lang.IllegalArgumentException if {@code join} is an instance of {@link JoinDefinedByMetadata} + * and {@code join.getJoinedPropertyName() is neither {@link CollectionPropertyNames#COLLECTION_ELEMENTS} + * nor {@link CollectionPropertyNames#COLLECTION_INDICES}}. + * @throws java.lang.IllegalStateException if there is already an existing join with the same joined property name. + */ + public void addJoin(Join join); } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/plan/exec/internal/LoadQueryJoinAndFetchProcessor.java b/hibernate-core/src/main/java/org/hibernate/loader/plan/exec/internal/LoadQueryJoinAndFetchProcessor.java index 8153255da0..b88a0fab92 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/plan/exec/internal/LoadQueryJoinAndFetchProcessor.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/plan/exec/internal/LoadQueryJoinAndFetchProcessor.java @@ -23,6 +23,7 @@ */ package org.hibernate.loader.plan.exec.internal; +import org.hibernate.AssertionFailure; import org.hibernate.engine.FetchStyle; import org.hibernate.engine.FetchTiming; import org.hibernate.engine.spi.SessionFactoryImplementor; @@ -284,28 +285,43 @@ public class LoadQueryJoinAndFetchProcessor { // For many-to-many, the follow-on join will join to the associated entity element table. For one-to-many, // the collection table is the associated entity table, so the follow-on join will not be rendered.. + // currently we do not explicitly track the joins under the CollectionQuerySpace to know which is + // the element join and which is the index join (maybe we should?). + + JoinDefinedByMetadata collectionElementJoin = null; + JoinDefinedByMetadata collectionIndexJoin = null; + for ( Join collectionJoin : rightHandSide.getJoins() ) { + if ( JoinDefinedByMetadata.class.isInstance( collectionJoin ) ) { + final JoinDefinedByMetadata collectionJoinDefinedByMetadata = (JoinDefinedByMetadata) collectionJoin; + if ( CollectionPropertyNames.COLLECTION_ELEMENTS.equals( collectionJoinDefinedByMetadata.getJoinedPropertyName() ) ) { + if ( collectionElementJoin != null ) { + throw new AssertionFailure( + String.format( + "More than one element join defined for: %s", + rightHandSide.getCollectionPersister().getRole() + ) + ); + } + collectionElementJoin = collectionJoinDefinedByMetadata; + } + if ( CollectionPropertyNames.COLLECTION_INDICES.equals( collectionJoinDefinedByMetadata.getJoinedPropertyName() ) ) { + if ( collectionIndexJoin != null ) { + throw new AssertionFailure( + String.format( + "More than one index join defined for: %s", + rightHandSide.getCollectionPersister().getRole() + ) + ); + } + collectionIndexJoin = collectionJoinDefinedByMetadata; + } + } + } + if ( rightHandSide.getCollectionPersister().isOneToMany() || rightHandSide.getCollectionPersister().isManyToMany() ) { // relatedly, for collections with entity elements (one-to-many, many-to-many) we need to register the // sql aliases to use for the entity. - // - // currently we do not explicitly track the joins under the CollectionQuerySpace to know which is - // the element join and which is the index join (maybe we should?). Another option here is to have the - // "collection join" act as the entity element join in this case (much like I do with entity identifiers). - // The difficulty there is that collections can theoretically could be multiple joins in that case (one - // for element, one for index). However, that's a bit of future-planning as today Hibernate does not - // properly deal with the index anyway in terms of allowing dynamic fetching across a collection index... - // - // 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 ^^; 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; - } if ( collectionElementJoin == null ) { throw new IllegalStateException( String.format( @@ -329,6 +345,24 @@ public class LoadQueryJoinAndFetchProcessor { ); } + if ( rightHandSide.getCollectionPersister().hasIndex() && + rightHandSide.getCollectionPersister().getIndexType().isEntityType() ) { + // for collections with entity index we need to register the + // sql aliases to use for the entity. + if ( collectionIndexJoin == null ) { + throw new IllegalStateException( + String.format( + "Could not locate collection index join within collection join [%s : %s]", + rightHandSide.getUid(), + rightHandSide.getCollectionPersister() + ) + ); + } + aliasResolutionContext.generateEntityReferenceAliases( + collectionIndexJoin.getRightHandSide().getUid(), + rightHandSide.getCollectionPersister().getIndexDefinition().toEntityDefinition().getEntityPersister() + ); + } addJoins( join, joinFragment, diff --git a/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/eager/Atmosphere.java b/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/eager/Atmosphere.java new file mode 100644 index 0000000000..6288435623 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/eager/Atmosphere.java @@ -0,0 +1,84 @@ +//$Id$ +package org.hibernate.test.annotations.indexcoll.eager; +import java.util.Date; +import java.util.HashMap; +import java.util.Map; +import javax.persistence.CascadeType; +import javax.persistence.Column; +import javax.persistence.ElementCollection; +import javax.persistence.Entity; +import javax.persistence.EnumType; +import javax.persistence.FetchType; +import javax.persistence.GeneratedValue; +import javax.persistence.Id; +import javax.persistence.JoinColumn; +import javax.persistence.JoinTable; +import javax.persistence.ManyToMany; +import javax.persistence.MapKeyColumn; +import javax.persistence.MapKeyEnumerated; +import javax.persistence.MapKeyJoinColumn; +import javax.persistence.MapKeyJoinColumns; +import javax.persistence.MapKeyTemporal; +import javax.persistence.TemporalType; + +import org.hibernate.test.annotations.indexcoll.Gas; +import org.hibernate.test.annotations.indexcoll.GasKey; + +/** + * @author Emmanuel Bernard + */ +@Entity +public class Atmosphere { + + public static enum Level { + LOW, + HIGH + } + + @Id + @GeneratedValue + public Integer id; + + @ManyToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER) + @MapKeyColumn(name="gas_name") + public Map gases = new HashMap(); + + @MapKeyTemporal(TemporalType.DATE) + @ElementCollection(fetch = FetchType.EAGER) + @MapKeyColumn(nullable=false) + public Map colorPerDate = new HashMap(); + + @ElementCollection(fetch = FetchType.EAGER) + @MapKeyEnumerated(EnumType.STRING) + @MapKeyColumn(nullable=false) + public Map colorPerLevel = new HashMap(); + + @ManyToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER) + @MapKeyJoinColumn(name="gas_id" ) + @JoinTable(name = "Gas_per_key") + public Map gasesPerKey = new HashMap(); + + @ElementCollection(fetch = FetchType.EAGER) + @Column(name="composition_rate") + @MapKeyJoinColumns( { @MapKeyJoinColumn(name="gas_id" ) } ) //use @MapKeyJoinColumns explicitly for tests + @JoinTable(name = "Composition", joinColumns = @JoinColumn(name = "atmosphere_id")) + public Map composition = new HashMap(); + + //use default JPA 2 column name for map key + @ManyToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER) + @MapKeyColumn + @JoinTable(name="Atm_Gas_Def") + public Map gasesDef = new HashMap(); + + //use default HAN legacy column name for map key + @ManyToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER) + @MapKeyColumn + @JoinTable(name="Atm_Gas_DefLeg") + public Map gasesDefLeg = new HashMap(); + + @ManyToMany(cascade = CascadeType.ALL, fetch = FetchType.EAGER) + @MapKeyJoinColumn + @JoinTable(name = "Gas_p_key_def") + public Map gasesPerKeyDef = new HashMap(); + +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/eager/EagerIndexedCollectionTest.java b/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/eager/EagerIndexedCollectionTest.java new file mode 100644 index 0000000000..e92f245f51 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/annotations/indexcoll/eager/EagerIndexedCollectionTest.java @@ -0,0 +1,170 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2011, 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.annotations.indexcoll.eager; + +import java.util.Date; +import java.util.Iterator; + +import org.junit.Test; + +import org.hibernate.Hibernate; +import org.hibernate.Session; +import org.hibernate.Transaction; +import org.hibernate.mapping.Collection; +import org.hibernate.mapping.Column; +import org.hibernate.test.annotations.indexcoll.Gas; +import org.hibernate.test.annotations.indexcoll.GasKey; +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * Test index collections + * + * @author Emmanuel Bernard + */ +public class EagerIndexedCollectionTest extends BaseCoreFunctionalTestCase { + @Test + public void testJPA2DefaultMapColumns() throws Exception { + isDefaultKeyColumnPresent( Atmosphere.class.getName(), "gasesDef", "_KEY" ); + isDefaultKeyColumnPresent( Atmosphere.class.getName(), "gasesPerKeyDef", "_KEY" ); + isDefaultKeyColumnPresent( Atmosphere.class.getName(), "gasesDefLeg", "_KEY" ); + } + + private void isDefaultKeyColumnPresent(String collectionOwner, String propertyName, String suffix) { + assertTrue( "Could not find " + propertyName + suffix, + isDefaultColumnPresent(collectionOwner, propertyName, suffix) ); + } + + private boolean isDefaultColumnPresent(String collectionOwner, String propertyName, String suffix) { + final Collection collection = configuration().getCollectionMapping( collectionOwner + "." + propertyName ); + final Iterator columnIterator = collection.getCollectionTable().getColumnIterator(); + boolean hasDefault = false; + while ( columnIterator.hasNext() ) { + Column column = (Column) columnIterator.next(); + if ( (propertyName + suffix).equals( column.getName() ) ) hasDefault = true; + } + return hasDefault; + } + + @Test + public void testRealMap() throws Exception { + Session s = openSession(); + Transaction tx = s.beginTransaction(); + Atmosphere atm = new Atmosphere(); + Atmosphere atm2 = new Atmosphere(); + GasKey key = new GasKey(); + key.setName( "O2" ); + Gas o2 = new Gas(); + o2.name = "oxygen"; + atm.gases.put( "100%", o2 ); + atm.gasesPerKey.put( key, o2 ); + atm2.gases.put( "100%", o2 ); + atm2.gasesPerKey.put( key, o2 ); + s.persist( key ); + s.persist( atm ); + s.persist( atm2 ); + + s.flush(); + s.clear(); + + atm = (Atmosphere) s.get( Atmosphere.class, atm.id ); + key = (GasKey) s.get( GasKey.class, key.getName() ); + assertEquals( 1, atm.gases.size() ); + assertEquals( o2.name, atm.gases.get( "100%" ).name ); + assertEquals( o2.name, atm.gasesPerKey.get( key ).name ); + tx.rollback(); + s.close(); + } + + @Test + public void testTemporalKeyMap() throws Exception { + Session s = openSession(); + Transaction tx = s.beginTransaction(); + Atmosphere atm = new Atmosphere(); + atm.colorPerDate.put( new Date(1234567000), "red" ); + s.persist( atm ); + + s.flush(); + s.clear(); + + atm = (Atmosphere) s.get( Atmosphere.class, atm.id ); + assertEquals( 1, atm.colorPerDate.size() ); + final Date date = atm.colorPerDate.keySet().iterator().next(); + final long diff = new Date( 1234567000 ).getTime() - date.getTime(); + assertTrue( "24h diff max", diff > 0 && diff < 24*60*60*1000 ); + tx.rollback(); + s.close(); + } + + @Test + public void testEnumKeyType() throws Exception { + Session s = openSession(); + Transaction tx = s.beginTransaction(); + Atmosphere atm = new Atmosphere(); + atm.colorPerLevel.put( Atmosphere.Level.HIGH, "red" ); + s.persist( atm ); + + s.flush(); + s.clear(); + + atm = (Atmosphere) s.get( Atmosphere.class, atm.id ); + assertEquals( 1, atm.colorPerLevel.size() ); + assertEquals( "red", atm.colorPerLevel.get( Atmosphere.Level.HIGH) ); + tx.rollback(); + s.close(); + } + + @Test + public void testEntityKeyElementTarget() throws Exception { + Session s = openSession(); + Transaction tx = s.beginTransaction(); + Atmosphere atm = new Atmosphere(); + Gas o2 = new Gas(); + o2.name = "oxygen"; + atm.composition.put( o2, 94.3 ); + s.persist( o2 ); + s.persist( atm ); + + s.flush(); + s.clear(); + + atm = (Atmosphere) s.get( Atmosphere.class, atm.id ); + assertTrue( Hibernate.isInitialized( atm.composition ) ); + assertEquals( 1, atm.composition.size() ); + assertEquals( o2.name, atm.composition.keySet().iterator().next().name ); + tx.rollback(); + s.close(); + } + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[]{ + Atmosphere.class, + Gas.class, + GasKey.class + }; + } +}