HHH-19482 fix handling of lock modes for reads outside tx

and clean up some error messages

(cherry picked from commit 3189f0e)

Author: gavinking

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

@@ -52,7 +52,7 @@ public enum LockMode implements FindOption, RefreshOption {
 	 * rather than pull it from a cache.
 	 * <p>
 	 * This is the "default" lock mode, the mode requested by calling
-	 * {@link Session#get(Class, Object)} without passing an explicit
+	 * {@link Session#find(Class, Object)} without passing an explicit
 	 * mode. It permits the state of an object to be retrieved from
 	 * the cache without the cost of database access.
 	 *

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

@@ -825,19 +825,32 @@ public interface Session extends SharedSessionContract, EntityManager {
 	void remove(Object object);
 
 	/**
-	 * Determine the current {@link LockMode} of the given managed instance associated
-	 * with this session.
+	 * Determine the current {@linkplain LockMode lock mode} held on the given
+	 * managed instance associated with this session.
+	 * <p>
+	 * Unlike the JPA-standard {@link #getLockMode}, this operation may be
+	 * called when no transaction is active, in which case it should return
+	 * {@link LockMode#NONE}, indicating that no pessimistic lock is held on
+	 * the given entity.
 	 *
 	 * @param object a persistent instance associated with this session
 	 *
-	 * @return the current lock mode
+	 * @return the lock mode currently held on the given entity
+	 *
+	 * @throws IllegalStateException if the given instance is not associated
+	 *                               with this persistence context
+	 * @throws ObjectDeletedException if the given instance was already
+	 *                                {@linkplain #remove removed}
 	 */
 	LockMode getCurrentLockMode(Object object);
 
 	/**
-	 * Completely clear the session. Evict all loaded instances and cancel all pending
-	 * saves, updates and deletions. Do not close open iterators or instances of
-	 * {@link ScrollableResults}.
+	 * Completely clear the persistence context. Evict all loaded instances,
+	 * causing every managed entity currently associated with this session to
+	 * transition to the detached state, and cancel all pending insertions,
+	 * updates, and deletions.
+	 * <p>
+	 * Does not close open iterators or instances of {@link ScrollableResults}.
 	 */
 	@Override
 	void clear();

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

@@ -113,9 +113,10 @@ public static EntityEntry deserialize(
 				(String) ois.readObject(),
 				ois.readObject(),
 				Status.valueOf( (String) ois.readObject() ),
-				( previousStatusString = (String) ois.readObject() ).length() == 0
-						? null
-						: Status.valueOf( previousStatusString ),
+				( previousStatusString = (String) ois.readObject() )
+						.isEmpty()
+							? null
+							: Status.valueOf( previousStatusString ),
 				(Object[]) ois.readObject(),
 				(Object[]) ois.readObject(),
 				ois.readObject(),

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

@@ -18,7 +18,8 @@
  *
  * @author Emmanuel Bernard
  */
-public class ImmutableEntityEntryFactory implements EntityEntryFactory {
+@Deprecated(since = "7", forRemoval = true)
+public final class ImmutableEntityEntryFactory implements EntityEntryFactory {
 	/**
 	 * Singleton access
 	 */

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

@@ -83,9 +83,10 @@ public static EntityEntry deserialize(
 				(String) ois.readObject(),
 				ois.readObject(),
 				Status.valueOf( (String) ois.readObject() ),
-				( previousStatusString = (String) ois.readObject() ).length() == 0
-						? null
-						: Status.valueOf( previousStatusString ),
+				( previousStatusString = (String) ois.readObject() )
+						.isEmpty()
+							? null
+							: Status.valueOf( previousStatusString ),
 				(Object[]) ois.readObject(),
 				(Object[]) ois.readObject(),
 				ois.readObject(),

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

@@ -18,7 +18,8 @@
  *
  * @author Emmanuel Bernard
  */
-public class MutableEntityEntryFactory implements EntityEntryFactory {
+@Deprecated(since = "7", forRemoval = true)
+public final class MutableEntityEntryFactory implements EntityEntryFactory {
 	/**
 	 * Singleton access
 	 */

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

@@ -654,58 +654,34 @@ public EntityEntry addEntry(
 			final EntityPersister persister,
 			final boolean disableVersionIncrement) {
 		assert lockMode != null;
-
-		final EntityEntry e;
-
-		/*
-			IMPORTANT!!!
-
-			The following instanceof checks and castings are intentional.
-
-			DO NOT REFACTOR to make calls through the EntityEntryFactory interface, which would result
-			in polymorphic call sites which will severely impact performance.
-
-			When a virtual method is called via an interface the JVM needs to resolve which concrete
-			implementation to call.  This takes CPU cycles and is a performance penalty.  It also prevents method
-			inlining which further degrades performance.  Casting to an implementation and making a direct method call
-			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 ) {
-			//noinspection RedundantCast
-			e = ( (MutableEntityEntryFactory) persister.getEntityEntryFactory() ).createEntityEntry(
-					status,
-					loadedState,
-					rowId,
-					id,
-					version,
-					lockMode,
-					existsInDatabase,
-					persister,
-					disableVersionIncrement,
-					this
-			);
-		}
-		else {
-			//noinspection RedundantCast
-			e = ( (ImmutableEntityEntryFactory) persister.getEntityEntryFactory() ).createEntityEntry(
-					status,
-					loadedState,
-					rowId,
-					id,
-					version,
-					lockMode,
-					existsInDatabase,
-					persister,
-					disableVersionIncrement,
-					this
-			);
-		}
-
-		entityEntryContext.addEntityEntry( entity, e );
-
+		final EntityEntry entityEntry =
+				persister.isMutable()
+						? new MutableEntityEntry(
+								status,
+								loadedState,
+								rowId,
+								id,
+								version,
+								lockMode,
+								existsInDatabase,
+								persister,
+								disableVersionIncrement,
+								this
+						)
+						: new ImmutableEntityEntry(
+								status,
+								loadedState,
+								rowId,
+								id,
+								version,
+								lockMode,
+								existsInDatabase,
+								persister,
+								disableVersionIncrement
+						);
+		entityEntryContext.addEntityEntry( entity, entityEntry );
 		setHasNonReadOnlyEnties( status );
-		return e;
+		return entityEntry;
 	}
 
 	@Override
@@ -715,7 +691,6 @@ public EntityEntry addReferenceEntry(
 		final EntityEntry entityEntry = asManagedEntity( entity ).$$_hibernate_getEntityEntry();
 		entityEntry.setStatus( status );
 		entityEntryContext.addEntityEntry( entity, entityEntry );
-
 		setHasNonReadOnlyEnties( status );
 		return entityEntry;
 	}

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

@@ -13,7 +13,10 @@
  * Contract to build {@link EntityEntry}
  *
  * @author Emmanuel Bernard
+ *
+ * @deprecated No longer used
  */
+@Deprecated(since = "7", forRemoval = true)
 public interface EntityEntryFactory extends Serializable {
 
 	/**

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

@@ -525,9 +525,7 @@ private void checksBeforeQueryCreation() {
 	public void prepareForQueryExecution(boolean requiresTxn) {
 		checksBeforeQueryCreation();
 		if ( requiresTxn && !isTransactionInProgress() ) {
-			throw new TransactionRequiredException(
-					"Query requires transaction be in progress, but no transaction is known to be in progress"
-			);
+			throw new TransactionRequiredException( "No active transaction" );
 		}
 	}
 

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

@@ -554,13 +554,13 @@ public LockMode getCurrentLockMode(Object object) {
 		if ( e == null ) {
 			throw new IllegalArgumentException( "Given entity is not associated with the persistence context" );
 		}
-
-		if ( e.getStatus().isDeletedOrGone() ) {
-			throw new ObjectDeletedException( "The given object was deleted", e.getId(),
+		else if ( e.getStatus().isDeletedOrGone() ) {
+			throw new ObjectDeletedException( "Given entity was removed", e.getId(),
 					e.getPersister().getEntityName() );
 		}
-
-		return e.getLockMode();
+		else {
+			return e.getLockMode();
+		}
 	}
 
 	@Override
@@ -2611,7 +2611,7 @@ private static CacheStoreMode determineCacheStoreMode(Map<String, Object> settin
 	}
 
 	private void checkTransactionNeededForUpdateOperation() {
-		checkTransactionNeededForUpdateOperation( "no transaction is in progress" );
+		checkTransactionNeededForUpdateOperation( "No active transaction" );
 	}
 
 	@Override
@@ -2772,11 +2772,11 @@ public LockModeType getLockMode(Object entity) {
 		checkOpen();
 
 		if ( !isTransactionInProgress() ) {
-			throw new TransactionRequiredException( "Call to EntityManager#getLockMode should occur within transaction according to spec" );
+			throw new TransactionRequiredException( "No active transaction" );
 		}
 
 		if ( !contains( entity ) ) {
-			throw getExceptionConverter().convert( new IllegalArgumentException( "entity not in the persistence context" ) );
+			throw getExceptionConverter().convert( new IllegalArgumentException( "Entity not associated with the persistence context" ) );
 		}
 
 		return LockModeTypeHelper.getLockModeType( getCurrentLockMode( entity ) );