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

and clean up some error messages

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

@@ -655,58 +655,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
@@ -716,7 +692,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 ) );

hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java

@@ -4585,7 +4585,7 @@ private SQLQueryParser createSqlQueryParser(Table table) {
 
 	@Override
 	public EntityEntryFactory getEntityEntryFactory() {
-		return this.entityEntryFactory;
+		return entityEntryFactory;
 	}
 
 	/**

hibernate-core/src/main/java/org/hibernate/persister/entity/EntityPersister.java

@@ -163,10 +163,11 @@ default String getSqlAliasStem() {
 	// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 	/**
-	 * Get the EntityEntryFactory indicated for the entity mapped by this persister.
+	 * Get the {@link EntityEntryFactory} indicated for the entity mapped by this persister.
 	 *
-	 * @return The proper EntityEntryFactory.
+	 * @deprecated No longer used
 	 */
+	@Deprecated(since = "7", forRemoval = true)
 	EntityEntryFactory getEntityEntryFactory();
 
 	/**

hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureCallImpl.java

@@ -87,7 +87,6 @@
 import jakarta.persistence.ParameterMode;
 import jakarta.persistence.PersistenceException;
 import jakarta.persistence.TemporalType;
-import jakarta.persistence.TransactionRequiredException;
 import jakarta.persistence.metamodel.Type;
 
 import static java.lang.Boolean.parseBoolean;
@@ -823,10 +822,6 @@ protected ProcedureOutputs outputs() {
 
 	@Override
 	protected int doExecuteUpdate() {
-		if ( !getSession().isTransactionInProgress() ) {
-			throw new TransactionRequiredException( "jakarta.persistence.Query.executeUpdate requires active transaction" );
-		}
-
 		// the expectation is that there is just one Output, of type UpdateCountOutput
 		try {
 			execute();

hibernate-core/src/main/java/org/hibernate/query/spi/AbstractQuery.java

@@ -8,7 +8,6 @@
 import java.util.Calendar;
 import java.util.Collection;
 import java.util.Date;
-import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 
@@ -634,8 +633,9 @@ protected void prepareForExecution() {
 
 	@Override
 	public int executeUpdate() throws HibernateException {
-		getSession().checkTransactionNeededForUpdateOperation( "Executing an update/delete query" );
-		final HashSet<String> fetchProfiles = beforeQueryHandlingFetchProfiles();
+		//TODO: refactor copy/paste of QuerySqmImpl.executeUpdate()
+		getSession().checkTransactionNeededForUpdateOperation( "No active transaction for update or delete query" );
+		final var fetchProfiles = beforeQueryHandlingFetchProfiles();
 		boolean success = false;
 		try {
 			final int result = doExecuteUpdate();

hibernate-core/src/main/java/org/hibernate/query/sqm/internal/QuerySqmImpl.java

@@ -86,7 +86,6 @@
 import java.util.Calendar;
 import java.util.Collection;
 import java.util.Date;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.function.BooleanSupplier;
@@ -507,9 +506,10 @@ private QueryInterpretationCache interpretationCache() {
 
 	@Override
 	public int executeUpdate() {
+		//TODO: refactor copy/paste of AbstractQuery.executeUpdate()
 		verifyUpdate();
-		getSession().checkTransactionNeededForUpdateOperation( "Executing an update/delete query" );
-		final HashSet<String> fetchProfiles = beforeQueryHandlingFetchProfiles();
+		getSession().checkTransactionNeededForUpdateOperation( "No active transaction for update or delete query" );
+		final var fetchProfiles = beforeQueryHandlingFetchProfiles();
 		boolean success = false;
 		try {
 			final int result = doExecuteUpdate();

hibernate-core/src/main/java/org/hibernate/sql/exec/internal/BaseExecutionContext.java

@@ -15,9 +15,11 @@
 public class BaseExecutionContext implements ExecutionContext {
 
 	private final SharedSessionContractImplementor session;
+	private final boolean transactionActive;
 
 	public BaseExecutionContext(SharedSessionContractImplementor session) {
 		this.session = session;
+		transactionActive = session.isTransactionInProgress();
 	}
 
 	// Optimization: mark this as final so to avoid a megamorphic call on this
@@ -27,6 +29,11 @@ public final SharedSessionContractImplementor getSession() {
 		return session;
 	}
 
+	@Override
+	public final boolean isTransactionActive() {
+		return transactionActive;
+	}
+
 	// Also marked as final for the same reason
 	@Override
 	public final LoadQueryInfluencers getLoadQueryInfluencers() {

hibernate-core/src/main/java/org/hibernate/sql/exec/spi/ExecutionContext.java

@@ -25,6 +25,10 @@ default boolean isScrollResult(){
 
 	SharedSessionContractImplementor getSession();
 
+	default boolean isTransactionActive() {
+		return getSession().isTransactionInProgress();
+	}
+
 	QueryOptions getQueryOptions();
 
 	LoadQueryInfluencers getLoadQueryInfluencers();