HHH-19941 simplify Session.contains() / deprecate Session.contains(String, Object)

and introduce SharedSessionContractImplementor.isManaged()

- Session.contains(String, Object) was not making use of its first parameter
- Session.contains(Object) was doing a duplicate lookup for the EntityEntry
- The use of Session.contains(String, Object) as an SPI was not very efficient

Author: gavinking

hibernate-core/src/main/java/org/hibernate/Session.java

@@ -470,7 +470,10 @@ public interface Session extends SharedSessionContract, EntityManager {
 	 * @param object an instance of a persistent class
 	 *
 	 * @return {@code true} if the given instance is associated with this {@code Session}
+	 *
+	 * @deprecated Use {@link #contains(Object)} instead.
 	 */
+	@Deprecated(since = "7.2", forRemoval = true)
 	boolean contains(String entityName, Object object);
 
 	/**

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

@@ -800,6 +800,11 @@ public boolean contains(Object object) {
 		return delegate.contains( object );
 	}
 
+	@Override
+	public boolean isManaged(Object entity) {
+		return delegate.isManaged( entity );
+	}
+
 	@Override
 	public LockModeType getLockMode(Object entity) {
 		return delegate.getLockMode( entity );

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

@@ -503,6 +503,15 @@ default Integer getConfiguredJdbcBatchSize() {
 	 */
 	PersistenceContext getPersistenceContextInternal();
 
+	/**
+	 * Is the given entity managed by this session?
+	 *
+	 * @return true if this is a stateful session and
+	 *         the entity belongs to its persistence
+	 *         context and was not removed
+	 */
+	boolean isManaged(Object entity);
+
 	/**
 	 * detect in-memory changes, determine if the changes are to tables
 	 * named in the query and, if so, complete execution the flush

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

@@ -742,4 +742,9 @@ public SharedSessionBuilder sessionWithOptions() {
 	public TransactionCompletionCallbacksImplementor getTransactionCompletionCallbacksImplementor() {
 		return delegate.getTransactionCompletionCallbacksImplementor();
 	}
+
+	@Override
+	public boolean isManaged(Object entity) {
+		return delegate.isManaged( entity );
+	}
 }

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

@@ -74,7 +74,7 @@ private static void handleUninitializedProxy(
 			EventSource source,
 			Object object,
 			PersistenceContext persistenceContext) {
-		final boolean isTransient = isTransient( event, source, object );
+		final boolean isTransient = !source.isManaged( object );
 		// If refreshAlready is nonempty then the refresh is the result of a cascade refresh and the
 		// refresh of the parent will take care of initializing the lazy entity and setting the
 		// correct lock. This is needed only when the refresh is called directly on a lazy entity.
@@ -117,11 +117,6 @@ else if ( isTransient ) {
 		}
 	}
 
-	private static boolean isTransient(RefreshEvent event, EventSource source, Object object) {
-		final String entityName = event.getEntityName();
-		return entityName == null ? !source.contains( object ) : !source.contains( entityName, object );
-	}
-
 	private static void refresh(RefreshEvent event, RefreshContext refreshedAlready, Object object) {
 		final var source = event.getSession();
 		final var persistenceContext = source.getPersistenceContextInternal();

hibernate-core/src/main/java/org/hibernate/id/IdentifierGeneratorHelper.java

@@ -20,7 +20,6 @@
 import org.hibernate.TransientObjectException;
 import org.hibernate.boot.registry.selector.spi.StrategySelector;
 import org.hibernate.engine.config.spi.ConfigurationService;
-import org.hibernate.engine.spi.SessionImplementor;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.id.enhanced.ImplicitDatabaseObjectNamingStrategy;
 import org.hibernate.id.enhanced.StandardNamingStrategy;
@@ -82,16 +81,15 @@ else if ( integralType == BigDecimal.class ) {
 
 	public static Object getForeignId(
 			String entityName, String propertyName, SharedSessionContractImplementor sessionImplementor, Object object) {
-		final var persister =
-				sessionImplementor.getFactory().getMappingMetamodel()
-						.getEntityDescriptor( entityName );
-		if ( sessionImplementor instanceof SessionImplementor statefulSession
-				&& statefulSession.contains( entityName, object ) ) {
+		if ( sessionImplementor.isManaged( object ) ) {
 			//abort the save (the object is already saved by a circular cascade)
 			return SHORT_CIRCUIT_INDICATOR;
 			//throw new IdentifierGenerationException("save associated object first, or disable cascade for inverse association");
 		}
 		else {
+			final var persister =
+					sessionImplementor.getFactory().getMappingMetamodel()
+							.getEntityDescriptor( entityName );
 			return identifier( sessionImplementor, entityType( propertyName, persister ),
 					associatedEntity( entityName, propertyName, object, persister ) );
 		}

hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java

@@ -1338,13 +1338,45 @@ private void fireRefresh(final RefreshContext refreshedAlready, final RefreshEve
 	}
 
 	private void checkEntityManaged(String entityName, Object entity) {
-		if ( !managed( entityName, entity ) ) {
+		if ( !isManaged( entity ) ) {
 			throw new IllegalArgumentException( "Given entity is not associated with the persistence context" );
 		}
 	}
 
-	private boolean managed(String entityName, Object entity) {
-		return entityName == null ? contains( entity ) : contains( entityName, entity );
+	@Override
+	public boolean isManaged(Object entity) {
+		try {
+			final var lazyInitializer = extractLazyInitializer( entity );
+			if ( lazyInitializer != null ) {
+				//do not use proxiesByKey, since not all
+				//proxies that point to this session's
+				//instances are in that collection!
+				if ( lazyInitializer.isUninitialized() ) {
+					//if it is an uninitialized proxy, pointing
+					//with this session, then when it is accessed,
+					//the underlying instance will be "contained"
+					return lazyInitializer.getSession() == this;
+				}
+				else {
+					//if it is initialized, see if the underlying
+					//instance is contained, since we need to
+					//account for the fact that it might have been
+					//evicted
+					entity = lazyInitializer.getImplementation();
+				}
+			}
+			// A session is considered to contain an entity only if the entity has
+			// an entry in the session's persistence context and the entry reports
+			// that the entity has not been removed
+			final var entry = persistenceContext.getEntry( entity );
+			return entry != null && !entry.getStatus().isDeletedOrGone();
+		}
+		catch ( MappingException e ) {
+			throw new IllegalArgumentException( e.getMessage(), e );
+		}
+		catch ( RuntimeException e ) {
+			throw getExceptionConverter().convert( e );
+		}
 	}
 
 	// replicate() operations ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -1632,8 +1664,10 @@ else if ( isPersistentAttributeInterceptable( object ) ) {
 	public boolean contains(Object object) {
 		checkOpen();
 		pulseTransactionCoordinator();
+		delayedAfterCompletion();
 
 		if ( object == null ) {
+			//TODO: this should throw IllegalArgumentException
 			return false;
 		}
 
@@ -1662,25 +1696,10 @@ public boolean contains(Object object) {
 			// an entry in the session's persistence context and the entry reports
 			// that the entity has not been removed
 			final var entry = persistenceContext.getEntry( object );
-			delayedAfterCompletion();
-
 			if ( entry == null ) {
-				if ( lazyInitializer == null && persistenceContext.getEntry( object ) == null ) {
-					// check if it is even an entity -> if not throw an exception (per JPA)
-					try {
-						final String entityName = getEntityNameResolver().resolveEntityName( object );
-						if ( entityName == null ) {
-							throw new IllegalArgumentException( "Could not resolve entity name for class '"
-									+ object.getClass() + "'" );
-						}
-						else {
-							requireEntityPersister( entityName );
-						}
-					}
-					catch ( HibernateException e ) {
-						throw new IllegalArgumentException( "Class '" + object.getClass()
-								+ "' is not an entity class", e );
-					}
+				if ( lazyInitializer == null ) {
+					// if not an entity, throw an exception, as required by spec
+					assertInstanceOfEntityType( object );
 				}
 				return false;
 			}
@@ -1696,60 +1715,28 @@ public boolean contains(Object object) {
 		}
 	}
 
-	@Override
-	public boolean contains(String entityName, Object object) {
-		checkOpenOrWaitingForAutoClose();
-		pulseTransactionCoordinator();
-
-		if ( object == null ) {
-			return false;
-		}
-
+	private void assertInstanceOfEntityType(Object object) {
 		try {
-			final var lazyInitializer = extractLazyInitializer( object );
-			if ( lazyInitializer == null && persistenceContext.getEntry( object ) == null ) {
-				// check if it is an entity -> if not throw an exception (per JPA)
-				try {
-					requireEntityPersister( entityName );
-				}
-				catch (HibernateException e) {
-					throw new IllegalArgumentException( "Not an entity [" + entityName + "] : " + object );
-				}
+			final String entityName = getEntityNameResolver().resolveEntityName( object );
+			if ( entityName == null ) {
+				throw new IllegalArgumentException( "Could not resolve entity name for class '"
+													+ object.getClass() + "'" );
 			}
-
-			if ( lazyInitializer != null ) {
-				//do not use proxiesByKey, since not all
-				//proxies that point to this session's
-				//instances are in that collection!
-				if ( lazyInitializer.isUninitialized() ) {
-					//if it is an uninitialized proxy, pointing
-					//with this session, then when it is accessed,
-					//the underlying instance will be "contained"
-					return lazyInitializer.getSession() == this;
-				}
-				else {
-					//if it is initialized, see if the underlying
-					//instance is contained, since we need to
-					//account for the fact that it might have been
-					//evicted
-					object = lazyInitializer.getImplementation();
-				}
+			else {
+				requireEntityPersister( entityName );
 			}
-			// A session is considered to contain an entity only if the entity has
-			// an entry in the session's persistence context and the entry reports
-			// that the entity has not been removed
-			final var entry = persistenceContext.getEntry( object );
-			delayedAfterCompletion();
-			return entry != null && !entry.getStatus().isDeletedOrGone();
 		}
-		catch ( MappingException e ) {
-			throw new IllegalArgumentException( e.getMessage(), e );
-		}
-		catch ( RuntimeException e ) {
-			throw getExceptionConverter().convert( e );
+		catch ( HibernateException e ) {
+			throw new IllegalArgumentException( "Class '" + object.getClass()
+												+ "' is not an entity class", e );
 		}
 	}
 
+	@Override @Deprecated(forRemoval = true)
+	public boolean contains(String entityName, Object object) {
+		return contains( object );
+	}
+
 	@Override
 	public ProcedureCall createStoredProcedureCall(String procedureName) {
 		checkOpen();

hibernate-core/src/main/java/org/hibernate/internal/StatelessSessionImpl.java

@@ -1302,6 +1302,11 @@ public boolean isIdentifierRollbackEnabled() {
 		return false;
 	}
 
+	@Override
+	public boolean isManaged(Object entity) {
+		return false;
+	}
+
 	/////////////////////////////////////////////////////////////////////////////////////////////////////
 
 	//TODO: COPY/PASTE FROM SessionImpl, pull up!