HHH-18552 clean up inappropriate usages of TransientObjectException

and minor cleanups in StatefulPersistenceContext

Signed-off-by: Gavin King <[email protected]>

Author: gavinking

hibernate-core/src/main/java/org/hibernate/engine/internal/ForeignKeys.java

@@ -373,10 +373,9 @@ public static Object getEntityIdentifierIfNotUnsaved(
 
 	private static void throwIfTransient(String entityName, Object object, SharedSessionContractImplementor session) {
 		if ( isTransient( entityName, object, Boolean.FALSE, session ) ) {
-			throw new TransientObjectException(
-					"object references an unsaved transient instance - save the transient instance before flushing: " +
-							(entityName == null ? session.guessEntityName(object) : entityName)
-			);
+			throw new TransientObjectException( "Entity references an unsaved transient instance of '"
+					+ (entityName == null ? session.guessEntityName(object) : entityName)
+					+ "' (make the transient instance persistent before flushing)" );
 		}
 	}
 

hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java

@@ -33,7 +33,6 @@
 import org.hibernate.MappingException;
 import org.hibernate.NonUniqueObjectException;
 import org.hibernate.PersistentObjectException;
-import org.hibernate.TransientObjectException;
 import org.hibernate.bytecode.enhance.spi.interceptor.BytecodeLazyAttributeInterceptor;
 import org.hibernate.bytecode.enhance.spi.interceptor.EnhancementAsProxyLazinessInterceptor;
 import org.hibernate.collection.spi.PersistentCollection;
@@ -79,6 +78,7 @@
 import static org.hibernate.engine.internal.ManagedTypeHelper.asManagedEntity;
 import static org.hibernate.engine.internal.ManagedTypeHelper.asPersistentAttributeInterceptable;
 import static org.hibernate.engine.internal.ManagedTypeHelper.isPersistentAttributeInterceptable;
+import static org.hibernate.proxy.HibernateProxy.extractLazyInitializer;
 
 /**
  * A <em>stateful</em> implementation of the {@link PersistenceContext} contract, meaning that we maintain this
@@ -217,7 +217,7 @@ public boolean hasLoadContext() {
 //
 	@Override
 	public PersistentCollection<?> useUnownedCollection(CollectionKey key) {
-		return ( unownedCollections == null ) ? null : unownedCollections.remove( key );
+		return unownedCollections == null ? null : unownedCollections.remove( key );
 	}
 
 	@Override
@@ -234,7 +234,7 @@ public void clear() {
 			//Strictly avoid lambdas in this case
 			for ( EntityHolderImpl value : entitiesByKey.values() ) {
 				if ( value != null && value.proxy != null ) {
-					HibernateProxy.extractLazyInitializer( value.proxy ).unsetSession();
+					extractLazyInitializer( value.proxy ).unsetSession();
 				}
 			}
 		}
@@ -337,9 +337,8 @@ public Object getNaturalIdSnapshot(Object id, EntityPersister persister) throws
 		if ( cachedValue != null ) {
 			return cachedValue;
 		}
-
 		// check to see if the natural id is mutable/immutable
-		if ( persister.getEntityMetamodel().hasImmutableNaturalId() ) {
+		else if ( persister.getEntityMetamodel().hasImmutableNaturalId() ) {
 			// an immutable natural-id is not retrieved during a normal database-snapshot operation...
 			final Object naturalIdFromDb = persister.getNaturalIdentifierSnapshot( id, session );
 			naturalIdResolutions.cacheResolutionFromLoad( id, naturalIdFromDb, persister );
@@ -353,13 +352,14 @@ public Object getNaturalIdSnapshot(Object id, EntityPersister persister) throws
 			if ( entitySnapshot == NO_ROW || entitySnapshot == null ) {
 				return null;
 			}
-
-			final Object[] naturalIdSnapshotSubSet = new Object[ props.length ];
-			for ( int i = 0; i < props.length; i++ ) {
-				naturalIdSnapshotSubSet[i] = entitySnapshot[ props[i] ];
+			else {
+				final Object[] naturalIdSnapshotSubSet = new Object[ props.length ];
+				for ( int i = 0; i < props.length; i++ ) {
+					naturalIdSnapshotSubSet[i] = entitySnapshot[ props[i] ];
+				}
+				naturalIdResolutions.cacheResolutionFromLoad( id, naturalIdSnapshotSubSet, persister );
+				return naturalIdSnapshotSubSet;
 			}
-			naturalIdResolutions.cacheResolutionFromLoad( id, naturalIdSnapshotSubSet, persister );
-			return naturalIdSnapshotSubSet;
 		}
 	}
 
@@ -429,9 +429,8 @@ public boolean containsEntityHolder(EntityKey key) {
 	public void postLoad(JdbcValuesSourceProcessingState processingState, Consumer<EntityHolder> holderConsumer) {
 		final Callback callback = processingState.getExecutionContext().getCallback();
 		if ( processingState.getLoadingEntityHolders() != null ) {
-			final EventListenerGroup<PostLoadEventListener> listenerGroup = getSession().getFactory()
-					.getFastSessionServices()
-					.eventListenerGroup_POST_LOAD;
+			final EventListenerGroup<PostLoadEventListener> listenerGroup =
+					getSession().getFactory().getFastSessionServices().eventListenerGroup_POST_LOAD;
 			final PostLoadEvent postLoadEvent = processingState.getPostLoadEvent();
 			for ( final EntityHolder holder : processingState.getLoadingEntityHolders() ) {
 				processLoadedEntityHolder( holder, listenerGroup, postLoadEvent, callback, holderConsumer );
@@ -460,26 +459,27 @@ private void processLoadedEntityHolder(
 			// in which case we added an entry with a null proxy and entity.
 			// Remove that empty entry on post load to avoid unwanted side effects
 			entitiesByKey.remove( holder.getEntityKey() );
-			return;
 		}
-		if ( postLoadEvent != null ) {
-			postLoadEvent.reset();
-			postLoadEvent.setEntity( holder.getEntity() )
-					.setId( holder.getEntityKey().getIdentifier() )
-					.setPersister( holder.getDescriptor() );
-			listenerGroup.fireEventOnEachListener(
-					postLoadEvent,
-					PostLoadEventListener::onPostLoad
-			);
-		}
-		if ( callback != null ) {
-			callback.invokeAfterLoadActions(
-					holder.getEntity(),
-					holder.getDescriptor(),
-					getSession()
-			);
+		else {
+			if ( postLoadEvent != null ) {
+				postLoadEvent.reset();
+				postLoadEvent.setEntity( holder.getEntity() )
+						.setId( holder.getEntityKey().getIdentifier() )
+						.setPersister( holder.getDescriptor() );
+				listenerGroup.fireEventOnEachListener(
+						postLoadEvent,
+						PostLoadEventListener::onPostLoad
+				);
+			}
+			if ( callback != null ) {
+				callback.invokeAfterLoadActions(
+						holder.getEntity(),
+						holder.getDescriptor(),
+						getSession()
+				);
+			}
+			( (EntityHolderImpl) holder ).entityInitializer = null;
 		}
-		( (EntityHolderImpl) holder ).entityInitializer = null;
 	}
 
 	@Override
@@ -498,7 +498,8 @@ public EntityHolder addEntityHolder(EntityKey key, Object entity) {
 			holder = oldHolder;
 		}
 		else {
-			entityHolderMap.put( key, holder = EntityHolderImpl.forEntity( key, key.getPersister(), entity ) );
+			holder = EntityHolderImpl.forEntity( key, key.getPersister(), entity );
+			entityHolderMap.put( key, holder );
 		}
 		holder.state = EntityHolderState.INITIALIZED;
 		final BatchFetchQueue fetchQueue = this.batchFetchQueue;
@@ -614,8 +615,8 @@ public EntityEntry addEntity(
 			final boolean existsInDatabase,
 			final EntityPersister persister,
 			final boolean disableVersionIncrement) {
-		EntityHolder entityHolder = addEntityHolder( entityKey, entity );
-		EntityEntry entityEntry = addEntry(
+		final EntityHolder entityHolder = addEntityHolder( entityKey, entity );
+		final EntityEntry entityEntry = addEntry(
 				entity,
 				status,
 				loadedState,
@@ -661,7 +662,7 @@ public EntityEntry addEntry(
 			removes the virtual call, and allows the methods to be inlined.  In this critical code path, it has a very
 			large impact on performance to make virtual method calls.
 		*/
-		if (persister.getEntityEntryFactory() instanceof MutableEntityEntryFactory) {
+		if ( persister.getEntityEntryFactory() instanceof MutableEntityEntryFactory ) {
 			//noinspection RedundantCast
 			e = ( (MutableEntityEntryFactory) persister.getEntityEntryFactory() ).createEntityEntry(
 					status,
@@ -731,7 +732,7 @@ public boolean reassociateIfUninitializedProxy(Object value) throws MappingExcep
 		if ( ! Hibernate.isInitialized( value ) ) {
 
 			// could be a proxy....
-			final LazyInitializer lazyInitializer = HibernateProxy.extractLazyInitializer( value );
+			final LazyInitializer lazyInitializer = extractLazyInitializer( value );
 			if ( lazyInitializer != null ) {
 				reassociateProxy( lazyInitializer, asHibernateProxy( value ) );
 				return true;
@@ -740,7 +741,8 @@ public boolean reassociateIfUninitializedProxy(Object value) throws MappingExcep
 			// or an uninitialized enhanced entity ("bytecode proxy")...
 			if ( isPersistentAttributeInterceptable( value ) ) {
 				final PersistentAttributeInterceptable bytecodeProxy = asPersistentAttributeInterceptable( value );
-				final BytecodeLazyAttributeInterceptor interceptor = (BytecodeLazyAttributeInterceptor) bytecodeProxy.$$_hibernate_getInterceptor();
+				final BytecodeLazyAttributeInterceptor interceptor =
+						(BytecodeLazyAttributeInterceptor) bytecodeProxy.$$_hibernate_getInterceptor();
 				if ( interceptor != null ) {
 					interceptor.setSession( getSession() );
 				}
@@ -754,7 +756,7 @@ public boolean reassociateIfUninitializedProxy(Object value) throws MappingExcep
 
 	@Override
 	public void reassociateProxy(Object value, Object id) throws MappingException {
-		final LazyInitializer lazyInitializer = HibernateProxy.extractLazyInitializer( value );
+		final LazyInitializer lazyInitializer = extractLazyInitializer( value );
 		if ( lazyInitializer != null ) {
 			LOG.debugf( "Setting proxy identifier: %s", id );
 			lazyInitializer.setIdentifier( id );
@@ -792,7 +794,7 @@ private void reassociateProxy(LazyInitializer li, HibernateProxy proxy) {
 
 	@Override
 	public Object unproxy(Object maybeProxy) throws HibernateException {
-		final LazyInitializer lazyInitializer = HibernateProxy.extractLazyInitializer( maybeProxy );
+		final LazyInitializer lazyInitializer = extractLazyInitializer( maybeProxy );
 		if ( lazyInitializer != null ) {
 			if ( lazyInitializer.isUninitialized() ) {
 				throw new PersistentObjectException(
@@ -809,7 +811,7 @@ public Object unproxy(Object maybeProxy) throws HibernateException {
 
 	@Override
 	public Object unproxyAndReassociate(final Object maybeProxy) throws HibernateException {
-		final LazyInitializer lazyInitializer = HibernateProxy.extractLazyInitializer( maybeProxy );
+		final LazyInitializer lazyInitializer = extractLazyInitializer( maybeProxy );
 		if ( lazyInitializer != null ) {
 			reassociateProxy( lazyInitializer, asHibernateProxy( maybeProxy ) );
 			//initialize + unwrap the object and return it
@@ -858,7 +860,7 @@ public Object narrowProxy(Object proxy, EntityPersister persister, EntityKey key
 
 			// Similarly, if the original HibernateProxy is initialized, there
 			// is again no point in creating a proxy.  Just return the impl
-			final LazyInitializer lazyInitializer = HibernateProxy.extractLazyInitializer( proxy );
+			final LazyInitializer lazyInitializer = extractLazyInitializer( proxy );
 			if ( !lazyInitializer.isUninitialized() ) {
 				final Object impl = lazyInitializer.getImplementation();
 				// can we return it?
@@ -871,16 +873,13 @@ public Object narrowProxy(Object proxy, EntityPersister persister, EntityKey key
 
 			// Otherwise, create the narrowed proxy
 			final HibernateProxy narrowedProxy = asHibernateProxy( persister.createProxy( key.getIdentifier(), session ) );
-
 			// set the read-only/modifiable mode in the new proxy to what it was in the original proxy
-			final boolean readOnlyOrig = lazyInitializer.isReadOnly();
-			narrowedProxy.getHibernateLazyInitializer().setReadOnly( readOnlyOrig );
-
+			narrowedProxy.getHibernateLazyInitializer().setReadOnly( lazyInitializer.isReadOnly() );
 			return narrowedProxy;
 		}
 		else {
 			if ( object != null ) {
-				HibernateProxy.extractLazyInitializer( proxy ).setImplementation( object );
+				extractLazyInitializer( proxy ).setImplementation( object );
 			}
 			return proxy;
 		}
@@ -1531,7 +1530,7 @@ && isFoundInParent( propertyName, childEntity, persister, collectionPersister, p
 		if ( mergeMap != null ) {
 			for ( Object o : mergeMap.entrySet() ) {
 				final Entry<?,?> mergeMapEntry = (Entry<?,?>) o;
-				final LazyInitializer lazyInitializer = HibernateProxy.extractLazyInitializer( mergeMapEntry.getKey() );
+				final LazyInitializer lazyInitializer = extractLazyInitializer( mergeMapEntry.getKey() );
 				if ( lazyInitializer != null ) {
 					if ( persister.isSubclassEntityName( lazyInitializer.getEntityName() ) ) {
 						HibernateProxy proxy = asHibernateProxy( mergeMapEntry.getKey() );
@@ -1693,17 +1692,17 @@ private void clearNullProperties() {
 	@Override
 	public boolean isReadOnly(Object entityOrProxy) {
 		if ( entityOrProxy == null ) {
-			throw new AssertionFailure( "object must be non-null." );
+			throw new IllegalArgumentException( "Null entity instance" );
 		}
 		boolean isReadOnly;
-		final LazyInitializer lazyInitializer = HibernateProxy.extractLazyInitializer( entityOrProxy );
+		final LazyInitializer lazyInitializer = extractLazyInitializer( entityOrProxy );
 		if ( lazyInitializer != null ) {
 			isReadOnly = lazyInitializer.isReadOnly();
 		}
 		else {
 			final EntityEntry ee =  getEntry( entityOrProxy );
 			if ( ee == null ) {
-				throw new TransientObjectException( "Instance was not associated with this persistence context" );
+				throw new IllegalArgumentException( "Instance is not associated with this persistence context" );
 			}
 			isReadOnly = ee.isReadOnly();
 		}
@@ -1713,12 +1712,12 @@ public boolean isReadOnly(Object entityOrProxy) {
 	@Override
 	public void setReadOnly(Object object, boolean readOnly) {
 		if ( object == null ) {
-			throw new AssertionFailure( "object must be non-null." );
+			throw new IllegalArgumentException( "Null entity instance" );
 		}
 		if ( isReadOnly( object ) == readOnly ) {
 			return;
 		}
-		final LazyInitializer lazyInitializer = HibernateProxy.extractLazyInitializer( object );
+		final LazyInitializer lazyInitializer = extractLazyInitializer( object );
 		if ( lazyInitializer != null ) {
 			setProxyReadOnly( lazyInitializer, readOnly );
 			if ( ! lazyInitializer.isUninitialized() ) {
@@ -1733,7 +1732,7 @@ public void setReadOnly(Object object, boolean readOnly) {
 			// PersistenceContext.proxyFor( entity ) returns entity if there is no proxy for that entity
 			// so need to check the return value to be sure it is really a proxy
 			final Object maybeProxy = getSession().getPersistenceContextInternal().proxyFor( object );
-			final LazyInitializer lazyInitializer1 = HibernateProxy.extractLazyInitializer( maybeProxy );
+			final LazyInitializer lazyInitializer1 = extractLazyInitializer( maybeProxy );
 			if ( lazyInitializer1 != null ) {
 				setProxyReadOnly( lazyInitializer1, readOnly );
 			}
@@ -1751,7 +1750,7 @@ private void setProxyReadOnly(LazyInitializer hibernateLazyInitializer, boolean
 	private void setEntityReadOnly(Object entity, boolean readOnly) {
 		final EntityEntry entry = getEntry( entity );
 		if ( entry == null ) {
-			throw new TransientObjectException( "Instance was not associated with this persistence context" );
+			throw new IllegalArgumentException( "Instance was not associated with this persistence context" );
 		}
 		entry.setReadOnly( readOnly, entity );
 		hasNonReadOnlyEntities = hasNonReadOnlyEntities || ! readOnly;
@@ -1963,7 +1962,7 @@ public static StatefulPersistenceContext deserialize(
 				final EntityHolderImpl holder = EntityHolderImpl.forEntity( ek, persister, entity );
 				holder.state = state;
 				if ( proxy != null ) {
-					final LazyInitializer lazyInitializer = HibernateProxy.extractLazyInitializer( proxy );
+					final LazyInitializer lazyInitializer = extractLazyInitializer( proxy );
 					if ( lazyInitializer != null ) {
 						lazyInitializer.setSession( session );
 						holder.proxy = proxy;

hibernate-core/src/main/java/org/hibernate/engine/spi/ActionQueue.java

@@ -502,8 +502,8 @@ public void executeActions() throws HibernateException {
 			final String path = transientEntities.getNonNullableTransientPropertyPaths(transientEntity).iterator().next();
 			//TODO: should be TransientPropertyValueException
 			throw new TransientObjectException( "Persistent instance of '" + insertAction.getEntityName()
-					+ "' with id '" + insertAction.getId()
-					+ "' references an unsaved transient instance via attribute '" + path
+					+ "' with id [" + insertAction.getId()
+					+ "] references an unsaved transient instance via attribute '" + path
 					+ "' (save the transient instance before flushing)" );
 		}
 

hibernate-core/src/main/java/org/hibernate/engine/spi/CascadingActions.java

@@ -339,7 +339,7 @@ public void cascade(
 				boolean isCascadeDeleteEnabled)
 				throws HibernateException {
 			if ( child != null && isChildTransient( session, child, entityName ) ) {
-				throw new TransientObjectException( "persistent instance references an unsaved transient instance of '"
+				throw new TransientObjectException( "Persistent instance references an unsaved transient instance of '"
 						+ entityName + "' (save the transient instance before flushing)" );
 				//TODO: should be TransientPropertyValueException
 //				throw new TransientPropertyValueException(

hibernate-core/src/main/java/org/hibernate/event/internal/DefaultDeleteEventListener.java

@@ -179,7 +179,8 @@ private void deleteTransientInstance(DeleteEvent event, DeleteContext transientE
 
 			final Object id = persister.getIdentifier( entity, source );
 			if ( id == null ) {
-				throw new TransientObjectException("the detached instance passed to delete() had a null identifier");
+				throw new TransientObjectException( "Cannot delete instance of entity '" + persister.getEntityName()
+						+ "' because it has a null identifier" );
 			}
 
 			final PersistenceContext persistenceContext = source.getPersistenceContextInternal();

hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLockEventListener.java

@@ -71,8 +71,8 @@ public void onLock(LockEvent event) throws HibernateException {
 			final EntityPersister persister = source.getEntityPersister( event.getEntityName(), entity );
 			final Object id = persister.getIdentifier( entity, source );
 			if ( !ForeignKeys.isNotTransient( event.getEntityName(), entity, Boolean.FALSE, source ) ) {
-				throw new TransientObjectException( "cannot lock an unsaved transient instance: "
-						+ persister.getEntityName() );
+				throw new TransientObjectException( "Cannot lock unsaved transient instance of entity '"
+						+ persister.getEntityName() + "'" );
 			}
 			entry = reassociate( event, entity, id, persister );
 			cascadeOnLock( event, persister, entity );

hibernate-core/src/main/java/org/hibernate/event/internal/DefaultRefreshEventListener.java

@@ -20,7 +20,6 @@
 import org.hibernate.engine.spi.ActionQueue;
 import org.hibernate.engine.spi.CascadingActions;
 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.event.spi.EventSource;
@@ -132,7 +131,8 @@ private static void refresh(RefreshEvent event, RefreshContext refreshedAlready,
 			persister = source.getEntityPersister( event.getEntityName(), object );
 			id = persister.getIdentifier( object, event.getSession() );
 			if ( id == null ) {
-				throw new TransientObjectException( "transient instance passed to refresh");
+				throw new TransientObjectException( "Cannot refresh instance of entity '" + persister.getEntityName()
+						+ "' because it has a null identifier" );
 			}
 			if ( LOG.isTraceEnabled() ) {
 				LOG.tracev(

hibernate-core/src/main/java/org/hibernate/event/internal/DefaultReplicateEventListener.java

@@ -77,7 +77,8 @@ private void doReplicate(ReplicateEvent event, EventSource source, Object entity
 		// except null (that is, even ids which look like they're unsaved)
 		final Object id = persister.getIdentifier( entity, source );
 		if ( id == null ) {
-			throw new TransientObjectException( "instance with null id passed to replicate()" );
+			throw new TransientObjectException( "Cannot replicate instance of entity '" + persister.getEntityName()
+					+ "' because it has a null identifier" );
 		}
 
 		final Object oldVersion = replicationMode == ReplicationMode.EXCEPTION