From d2a19c9a7794df7e2f4367e8d86a9cda05a0d978 Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Mon, 2 Oct 2017 16:48:33 -0700 Subject: [PATCH] HHH-12007 : Executing JPA query results in persistence of new/unmanaged entity --- .../org/hibernate/id/ForeignGenerator.java | 12 ++ .../tuple/entity/AbstractEntityTuplizer.java | 98 +++------------ .../CompositeIdDerivedIdWithIdClassTest.java | 117 +++++++++++++++++- 3 files changed, 145 insertions(+), 82 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/id/ForeignGenerator.java b/hibernate-core/src/main/java/org/hibernate/id/ForeignGenerator.java index 173dd30ca9..a92804464e 100644 --- a/hibernate-core/src/main/java/org/hibernate/id/ForeignGenerator.java +++ b/hibernate-core/src/main/java/org/hibernate/id/ForeignGenerator.java @@ -14,12 +14,16 @@ import org.hibernate.Session; import org.hibernate.TransientObjectException; import org.hibernate.engine.internal.ForeignKeys; import org.hibernate.engine.spi.SharedSessionContractImplementor; +import org.hibernate.internal.CoreMessageLogger; import org.hibernate.loader.PropertyPath; import org.hibernate.persister.entity.EntityPersister; import org.hibernate.service.ServiceRegistry; import org.hibernate.type.EntityType; import org.hibernate.type.Type; +import static org.hibernate.internal.CoreLogging.logger; +import static org.hibernate.internal.CoreLogging.messageLogger; + /** * foreign
*
@@ -31,6 +35,8 @@ import org.hibernate.type.Type; * @author Gavin King */ public class ForeignGenerator implements IdentifierGenerator, Configurable { + private static final CoreMessageLogger LOG = messageLogger( ForeignGenerator.class ); + private String entityName; private String propertyName; @@ -105,6 +111,12 @@ public class ForeignGenerator implements IdentifierGenerator, Configurable { ); } catch (TransientObjectException toe) { + if ( LOG.isDebugEnabled() ) { + LOG.debugf( + "ForeignGenerator detected a transient entity [%s]", + foreignValueSourceType.getAssociatedEntityName() + ); + } id = session.save( foreignValueSourceType.getAssociatedEntityName(), associatedObject ); } diff --git a/hibernate-core/src/main/java/org/hibernate/tuple/entity/AbstractEntityTuplizer.java b/hibernate-core/src/main/java/org/hibernate/tuple/entity/AbstractEntityTuplizer.java index c6e76d968a..0702145823 100644 --- a/hibernate-core/src/main/java/org/hibernate/tuple/entity/AbstractEntityTuplizer.java +++ b/hibernate-core/src/main/java/org/hibernate/tuple/entity/AbstractEntityTuplizer.java @@ -7,7 +7,6 @@ package org.hibernate.tuple.entity; import java.io.Serializable; -import java.util.Collections; import java.util.Iterator; import java.util.Map; import java.util.Set; @@ -18,17 +17,11 @@ import org.hibernate.HibernateException; import org.hibernate.MappingException; import org.hibernate.bytecode.enhance.spi.LazyPropertyInitializer; import org.hibernate.bytecode.spi.BytecodeEnhancementMetadata; -import org.hibernate.engine.internal.ForeignKeys; import org.hibernate.engine.spi.EntityEntry; import org.hibernate.engine.spi.EntityKey; import org.hibernate.engine.spi.PersistenceContext; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; -import org.hibernate.event.service.spi.EventListenerRegistry; -import org.hibernate.event.spi.EventSource; -import org.hibernate.event.spi.EventType; -import org.hibernate.event.spi.PersistEvent; -import org.hibernate.event.spi.PersistEventListener; import org.hibernate.id.Assigned; import org.hibernate.internal.CoreMessageLogger; import org.hibernate.loader.PropertyPath; @@ -177,6 +170,7 @@ public abstract class AbstractEntityTuplizer implements EntityTuplizer { else { identifierMapperType = (CompositeType) mapper.getType(); mappedIdentifierValueMarshaller = buildMappedIdentifierValueMarshaller( + getEntityName(), getFactory(), (ComponentType) entityMetamodel.getIdentifierProperty().getType(), (ComponentType) identifierMapperType @@ -278,6 +272,7 @@ public abstract class AbstractEntityTuplizer implements EntityTuplizer { private final MappedIdentifierValueMarshaller mappedIdentifierValueMarshaller; private static MappedIdentifierValueMarshaller buildMappedIdentifierValueMarshaller( + String entityName, SessionFactoryImplementor sessionFactory, ComponentType mappedIdClassComponentType, ComponentType virtualIdComponent) { @@ -302,9 +297,10 @@ public abstract class AbstractEntityTuplizer implements EntityTuplizer { } } - return wereAllEquivalent - ? new NormalMappedIdentifierValueMarshaller( virtualIdComponent, mappedIdClassComponentType ) - : new IncrediblySillyJpaMapsIdMappedIdentifierValueMarshaller( + return wereAllEquivalent ? + new NormalMappedIdentifierValueMarshaller( virtualIdComponent, mappedIdClassComponentType ) : + new IncrediblySillyJpaMapsIdMappedIdentifierValueMarshaller( + entityName, sessionFactory, virtualIdComponent, mappedIdClassComponentType @@ -342,15 +338,18 @@ public abstract class AbstractEntityTuplizer implements EntityTuplizer { private static class IncrediblySillyJpaMapsIdMappedIdentifierValueMarshaller implements MappedIdentifierValueMarshaller { + private final String entityName; private final SessionFactoryImplementor sessionFactory; private final ComponentType virtualIdComponent; private final ComponentType mappedIdentifierType; private IncrediblySillyJpaMapsIdMappedIdentifierValueMarshaller( + String entityName, SessionFactoryImplementor sessionFactory, ComponentType virtualIdComponent, ComponentType mappedIdentifierType) { this.sessionFactory = sessionFactory; + this.entityName = entityName; this.virtualIdComponent = virtualIdComponent; this.mappedIdentifierType = mappedIdentifierType; } @@ -368,7 +367,7 @@ public abstract class AbstractEntityTuplizer implements EntityTuplizer { } //JPA 2 @MapsId + @IdClass points to the pk of the entity if ( subTypes[i].isAssociationType() && !copierSubTypes[i].isAssociationType() ) { - propertyValues[i] = determineEntityIdPersistIfNecessary( + propertyValues[i] = determineEntityId( propertyValues[i], (AssociationType) subTypes[i], session, @@ -404,6 +403,13 @@ public abstract class AbstractEntityTuplizer implements EntityTuplizer { if ( association == null ) { // otherwise look for an initialized version association = persistenceContext.getEntity( entityKey ); + if ( association == null ) { + // get the association out of the entity itself + association = sessionFactory.getMetamodel().entityPersister( entityName ).getPropertyValue( + entity, + virtualIdComponent.getPropertyNames()[i] + ); + } } injectionValues[i] = association; } @@ -415,19 +421,7 @@ public abstract class AbstractEntityTuplizer implements EntityTuplizer { } } - private static Iterable persistEventListeners(SharedSessionContractImplementor session) { - if ( session == null ) { - return Collections.emptyList(); - } - return session - .getFactory() - .getServiceRegistry() - .getService( EventListenerRegistry.class ) - .getEventListenerGroup( EventType.PERSIST ) - .listeners(); - } - - private static Serializable determineEntityIdPersistIfNecessary( + private static Serializable determineEntityId( Object entity, AssociationType associationType, SharedSessionContractImplementor session, @@ -436,9 +430,6 @@ public abstract class AbstractEntityTuplizer implements EntityTuplizer { return null; } - // NOTE : persist if necessary for proper merge support (HHH-11328) - // but only allow persist if a Session is passed (HHH-11274) - if ( HibernateProxy.class.isInstance( entity ) ) { // entity is a proxy, so we know it is not transient; just return ID from proxy return ( (HibernateProxy) entity ).getHibernateLazyInitializer().getIdentifier(); @@ -459,36 +450,7 @@ public abstract class AbstractEntityTuplizer implements EntityTuplizer { sessionFactory ); - Serializable entityId = persister.getIdentifier( entity, session ); - - if ( entityId == null ) { - if ( session != null ) { - // if we have a session, then follow the HHH-11328 requirements - entityId = persistTransientEntity( entity, session ); - } - // otherwise just let it be null HHH-11274 - } - else { - if ( session != null ) { - // if the entity is in the process of being merged, it may be stored in the - // PC already, but doesn't have an EntityEntry yet. If this is the case, - // then don't persist even if it is transient because doing so can lead - // to having 2 entities in the PC with the same ID (HHH-11328). - final EntityKey entityKey = session.generateEntityKey( entityId, persister ); - if ( session.getPersistenceContext().getEntity( entityKey ) == null && - ForeignKeys.isTransient( - persister.getEntityName(), - entity, - null, - session - ) ) { - // entity is transient and it is not in the PersistenceContext. - // entity needs to be persisted. - persistTransientEntity( entity, session ); - } - } - } - return entityId; + return persister.getIdentifier( entity, session ); } private static EntityPersister resolveEntityPersister( @@ -520,28 +482,6 @@ public abstract class AbstractEntityTuplizer implements EntityTuplizer { return sessionFactory.getMetamodel().entityPersister( entityName ); } - private static Serializable persistTransientEntity( - Object entity, - SharedSessionContractImplementor session) { - assert session != null; - - LOG.debug( "Performing implicit derived identity cascade" ); - final PersistEvent event = new PersistEvent( - null, - entity, - (EventSource) session - ); - - for ( PersistEventListener listener : persistEventListeners( session ) ) { - listener.onPersist( event ); - } - final EntityEntry pcEntry = session.getPersistenceContext().getEntry( entity ); - if ( pcEntry == null || pcEntry.getId() == null ) { - throw new HibernateException( "Unable to process implicit derived identity cascade" ); - } - return pcEntry.getId(); - } - @Override public void resetIdentifier(Object entity, Serializable currentId, Object currentVersion) { // 99% of the time the session is not needed. Its only needed for certain brain-dead diff --git a/hibernate-core/src/test/java/org/hibernate/test/annotations/derivedidentities/bidirectional/CompositeIdDerivedIdWithIdClassTest.java b/hibernate-core/src/test/java/org/hibernate/test/annotations/derivedidentities/bidirectional/CompositeIdDerivedIdWithIdClassTest.java index f28566b877..72d9579927 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/annotations/derivedidentities/bidirectional/CompositeIdDerivedIdWithIdClassTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/annotations/derivedidentities/bidirectional/CompositeIdDerivedIdWithIdClassTest.java @@ -29,6 +29,9 @@ import org.hibernate.testing.TestForIssue; import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; public class CompositeIdDerivedIdWithIdClassTest extends BaseCoreFunctionalTestCase { @Override @@ -94,7 +97,7 @@ public class CompositeIdDerivedIdWithIdClassTest extends BaseCoreFunctionalTestC // merge lineItem with an ID with detached many-to-one s = openSession(); s.getTransaction().begin(); - s.merge(lineItem); + s.merge( lineItem ); s.getTransaction().commit(); s.close(); @@ -107,8 +110,116 @@ public class CompositeIdDerivedIdWithIdClassTest extends BaseCoreFunctionalTestC s.close(); } + @Test + @TestForIssue( jiraKey = "HHH-12007") + public void testBindTransientEntityWithTransientKeyManyToOne() { + + ShoppingCart cart = new ShoppingCart( "cart" ); + LineItem item = new LineItem( 0, "desc", cart ); + + Session s = openSession(); + s.getTransaction().begin(); + + String cartId = s.createQuery( + "select c.id from Cart c left join c.lineItems i where i = :item", + String.class + ).setParameter( "item", item ).uniqueResult(); + + assertNull( cartId ); + + assertFalse( s.contains( item ) ); + assertFalse( s.contains( cart ) ); + + s.getTransaction().commit(); + s.close(); + } + + @Test + @TestForIssue( jiraKey = "HHH-12007") + public void testBindTransientEntityWithPersistentKeyManyToOne() { + ShoppingCart cart = new ShoppingCart( "cart" ); + LineItem item = new LineItem( 0, "desc", cart ); + + Session s = openSession(); + s.getTransaction().begin(); + + session.persist( cart ); + String cartId = s.createQuery( + "select c.id from Cart c left join c.lineItems i where i = :item", + String.class + ).setParameter( "item", item ).uniqueResult(); + + assertNull( cartId ); + + assertFalse( s.contains( item ) ); + assertTrue( s.contains( cart ) ); + + s.getTransaction().commit(); + s.close(); + + } + + @Test + @TestForIssue( jiraKey = "HHH-12007") + public void testBindTransientEntityWithDetachedKeyManyToOne() { + Session s = openSession(); + s.getTransaction().begin(); + + ShoppingCart cart = new ShoppingCart( "cart" ); + + s.getTransaction().commit(); + s.close(); + + LineItem item = new LineItem( 0, "desc", cart ); + + s = openSession(); + s.getTransaction().begin(); + + String cartId = s.createQuery( + "select c.id from Cart c left join c.lineItems i where i = :item", + String.class + ).setParameter( "item", item ).uniqueResult(); + + assertNull( cartId ); + + assertFalse( s.contains( item ) ); + assertFalse( s.contains( cart ) ); + + s.getTransaction().commit(); + s.close(); + } + + @Test + @TestForIssue( jiraKey = "HHH-12007") + public void testBindTransientEntityWithCopiedKeyManyToOne() { + Session s = openSession(); + s.getTransaction().begin(); + + ShoppingCart cart = new ShoppingCart( "cart" ); + s.getTransaction().commit(); + s.close(); + + LineItem item = new LineItem( 0, "desc", new ShoppingCart( "cart" ) ); + + s = openSession(); + s.getTransaction().begin(); + + String cartId = s.createQuery( + "select c.id from Cart c left join c.lineItems i where i = :item", + String.class + ).setParameter( "item", item ).uniqueResult(); + + assertNull( cartId ); + + assertFalse( s.contains( item ) ); + assertFalse( s.contains( cart ) ); + + s.getTransaction().commit(); + s.close(); + } + @Entity(name = "Cart") - public static class ShoppingCart { + public static class ShoppingCart implements Serializable{ @Id @Column(name = "id", nullable = false) private String id; @@ -147,7 +258,7 @@ public class CompositeIdDerivedIdWithIdClassTest extends BaseCoreFunctionalTestC @Entity(name = "LineItem") @IdClass(LineItem.Pk.class) - public static class LineItem { + public static class LineItem implements Serializable { @Id @Column(name = "item_seq_number", nullable = false)