HHH-12007 : Executing JPA query results in persistence of new/unmanaged entity

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;
+
 /**
  * <b>foreign</b><br>
  * <br>
@@ -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 );
 		}
 

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
+		return wereAllEquivalent ?
-				? new NormalMappedIdentifierValueMarshaller( virtualIdComponent, mappedIdClassComponentType )
+				new NormalMappedIdentifierValueMarshaller( virtualIdComponent, mappedIdClassComponentType ) :
-				: new IncrediblySillyJpaMapsIdMappedIdentifierValueMarshaller(
+				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<PersistEventListener> persistEventListeners(SharedSessionContractImplementor session) {
+	private static Serializable determineEntityId(
-		if ( session == null ) {
-			return Collections.emptyList();
-		}
-		return session
-				.getFactory()
-				.getServiceRegistry()
-				.getService( EventListenerRegistry.class )
-				.getEventListenerGroup( EventType.PERSIST )
-				.listeners();
-	}
-
-	private static Serializable determineEntityIdPersistIfNecessary(
 			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 );
+		return 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;
 	}
 
 	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

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)