improve some locking-related error messages

gavinking · gavinking · commit c6820d223c9f · 2025-12-04T23:42:30.000+01:00
diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/lock/AbstractPessimisticUpdateLockingStrategy.java b/hibernate-core/src/main/java/org/hibernate/dialect/lock/AbstractPessimisticUpdateLockingStrategy.java
@@ -10,12 +10,11 @@
 import org.hibernate.StaleObjectStateException;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.persister.entity.EntityPersister;
-import org.hibernate.pretty.MessageHelper;
 import org.hibernate.sql.Update;
 
 import java.sql.SQLException;
 
-import static org.hibernate.internal.CoreMessageLogger.CORE_LOGGER;
+import static org.hibernate.pretty.MessageHelper.infoString;
 
 /**
  * Common implementation of {@link PessimisticReadUpdateLockingStrategy}
@@ -39,13 +38,15 @@ public abstract class AbstractPessimisticUpdateLockingStrategy implements Lockin
 	public AbstractPessimisticUpdateLockingStrategy(EntityPersister lockable, LockMode lockMode) {
 		this.lockable = lockable;
 		this.lockMode = lockMode;
-		if ( !lockable.isVersioned() ) {
-			CORE_LOGGER.writeLocksNotSupported( lockable.getEntityName() );
-			this.sql = null;
+		if ( lockMode.lessThan( LockMode.PESSIMISTIC_READ ) ) {
+			throw new HibernateException( "Lock mode " + lockMode
+						+ " not valid for locking via 'update' statement" );
 		}
-		else {
-			this.sql = generateLockString();
+		if ( !lockable.isVersioned() ) {
+			throw new HibernateException( "Entity '" + lockable.getEntityName()
+						+ "' has no version and may not be locked via 'update' statement" );
 		}
+		this.sql = generateLockString();
 	}
 
 	@Override
@@ -54,14 +55,11 @@ public void lock(Object id, Object version, Object object, int timeout, SharedSe
 			doLock( id, version, session );
 		}
 		catch (JDBCException e) {
-			throw new PessimisticEntityLockException( object, "could not obtain pessimistic lock", e );
+			throw new PessimisticEntityLockException( object, "Could not obtain pessimistic lock", e );
 		}
 	}
 
 	void doLock(Object id, Object version, SharedSessionContractImplementor session) {
-		if ( !lockable.isVersioned() ) {
-			throw new HibernateException( "write locks via update not supported for non-versioned entities [" + lockable.getEntityName() + "]" );
-		}
 		try {
 			final var factory = session.getFactory();
 			final var jdbcCoordinator = session.getJdbcCoordinator();
@@ -100,7 +98,7 @@ void doLock(Object id, Object version, SharedSessionContractImplementor session)
 		catch ( SQLException e ) {
 			throw session.getJdbcServices().getSqlExceptionHelper().convert(
 					e,
-					"could not lock: " + MessageHelper.infoString( lockable, id, session.getFactory() ),
+					"could not lock: " + infoString( lockable, id, session.getFactory() ),
 					sql
 			);
 		}
diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/lock/OptimisticForceIncrementLockingStrategy.java b/hibernate-core/src/main/java/org/hibernate/dialect/lock/OptimisticForceIncrementLockingStrategy.java
@@ -34,15 +34,17 @@ public OptimisticForceIncrementLockingStrategy(EntityPersister lockable, LockMod
 		this.lockable = lockable;
 		this.lockMode = lockMode;
 		if ( lockMode.lessThan( LockMode.OPTIMISTIC_FORCE_INCREMENT ) ) {
-			throw new HibernateException( "[" + lockMode + "] not valid for [" + lockable.getEntityName() + "]" );
+			throw new HibernateException( "Entity '" + lockable.getEntityName()
+						+ "' may not be locked at level " + lockMode );
+		}
+		if ( !lockable.isVersioned() ) {
+			throw new HibernateException( "Entity '" + lockable.getEntityName()
+						+ "' has no version and may not be locked at level " + lockMode);
 		}
 	}
 
 	@Override
 	public void lock(Object id, Object version, Object object, int timeout, EventSource session) {
-		if ( !lockable.isVersioned() ) {
-			throw new HibernateException( "[" + lockMode + "] not supported for non-versioned entities [" + lockable.getEntityName() + "]" );
-		}
 //		final EntityEntry entry = session.getPersistenceContextInternal().getEntry( object );
 		// Register the EntityIncrementVersionProcess action to run just prior to transaction commit.
 		session.getActionQueue().registerCallback( new EntityIncrementVersionProcess( object ) );
diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/lock/OptimisticLockingStrategy.java b/hibernate-core/src/main/java/org/hibernate/dialect/lock/OptimisticLockingStrategy.java
@@ -33,15 +33,17 @@ public OptimisticLockingStrategy(EntityPersister lockable, LockMode lockMode) {
 		this.lockable = lockable;
 		this.lockMode = lockMode;
 		if ( lockMode.lessThan( LockMode.OPTIMISTIC ) ) {
-			throw new HibernateException( "[" + lockMode + "] not valid for [" + lockable.getEntityName() + "]" );
+			throw new HibernateException( "Entity '" + lockable.getEntityName()
+						+ "' may not be locked at level " + lockMode );
+		}
+		if ( !lockable.isVersioned() ) {
+			throw new HibernateException( "Entity '" + lockable.getEntityName()
+						+ "' has no version and may not be locked at level " + lockMode);
 		}
 	}
 
 	@Override
 	public void lock(Object id, Object version, Object object, int timeout, EventSource session) {
-		if ( !lockable.isVersioned() ) {
-			throw new HibernateException( "[" + lockMode + "] not supported for non-versioned entities [" + lockable.getEntityName() + "]" );
-		}
 		// Register the EntityVerifyVersionProcess action to run just prior to transaction commit.
 		session.getActionQueue().registerCallback( new EntityVerifyVersionProcess( object ) );
 	}
diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/lock/PessimisticForceIncrementLockingStrategy.java b/hibernate-core/src/main/java/org/hibernate/dialect/lock/PessimisticForceIncrementLockingStrategy.java
@@ -34,15 +34,17 @@ public PessimisticForceIncrementLockingStrategy(EntityPersister lockable, LockMo
 		this.lockMode = lockMode;
 		// ForceIncrement can be used for PESSIMISTIC_READ, PESSIMISTIC_WRITE or PESSIMISTIC_FORCE_INCREMENT
 		if ( lockMode.lessThan( LockMode.PESSIMISTIC_READ ) ) {
-			throw new HibernateException( "[" + lockMode + "] not valid for [" + lockable.getEntityName() + "]" );
+			throw new HibernateException( "Entity '" + lockable.getEntityName()
+						+ "' may not be locked at level " + lockMode );
+		}
+		if ( !lockable.isVersioned() ) {
+			throw new HibernateException( "Entity '" + lockable.getEntityName()
+						+ "' has no version and may not be locked at level " + lockMode);
 		}
 	}
 
 	@Override
 	public void lock(Object id, Object version, Object object, int timeout, SharedSessionContractImplementor session) {
-		if ( !lockable.isVersioned() ) {
-			throw new HibernateException( "[" + lockMode + "] not supported for non-versioned entities [" + lockable.getEntityName() + "]" );
-		}
 		final var entry = session.getPersistenceContextInternal().getEntry( object );
 		OptimisticLockHelper.forceVersionIncrement( object, entry, session );
 	}
diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/lock/PessimisticReadUpdateLockingStrategy.java b/hibernate-core/src/main/java/org/hibernate/dialect/lock/PessimisticReadUpdateLockingStrategy.java
@@ -4,7 +4,6 @@
  */
 package org.hibernate.dialect.lock;
 
-import org.hibernate.HibernateException;
 import org.hibernate.LockMode;
 import org.hibernate.persister.entity.EntityPersister;
 
@@ -30,8 +29,5 @@ public class PessimisticReadUpdateLockingStrategy extends AbstractPessimisticUpd
 	 */
 	public PessimisticReadUpdateLockingStrategy(EntityPersister lockable, LockMode lockMode) {
 		super( lockable, lockMode );
-		if ( lockMode.lessThan( LockMode.PESSIMISTIC_READ ) ) {
-			throw new HibernateException( "[" + lockMode + "] not valid for update statement" );
-		}
 	}
 }
diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/lock/PessimisticWriteUpdateLockingStrategy.java b/hibernate-core/src/main/java/org/hibernate/dialect/lock/PessimisticWriteUpdateLockingStrategy.java
@@ -4,7 +4,6 @@
  */
 package org.hibernate.dialect.lock;
 
-import org.hibernate.HibernateException;
 import org.hibernate.LockMode;
 import org.hibernate.persister.entity.EntityPersister;
 
@@ -29,8 +28,5 @@ public class PessimisticWriteUpdateLockingStrategy extends AbstractPessimisticUp
 	 */
 	public PessimisticWriteUpdateLockingStrategy(EntityPersister lockable, LockMode lockMode) {
 		super( lockable, lockMode );
-		if ( lockMode.lessThan( LockMode.PESSIMISTIC_READ ) ) {
-			throw new HibernateException( "[" + lockMode + "] not valid for update statement" );
-		}
 	}
 }
diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/lock/UpdateLockingStrategy.java b/hibernate-core/src/main/java/org/hibernate/dialect/lock/UpdateLockingStrategy.java
@@ -35,7 +35,7 @@ public class UpdateLockingStrategy extends AbstractPessimisticUpdateLockingStrat
 	public UpdateLockingStrategy(EntityPersister lockable, LockMode lockMode) {
 		super( lockable, lockMode );
 		if ( lockMode.lessThan( LockMode.WRITE ) ) {
-			throw new HibernateException( "[" + lockMode + "] not valid for update statement" );
+			throw new HibernateException( "Lock mode " + lockMode + " not valid for locking via 'update' statement" );
 		}
 	}
 
diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultPostLoadEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultPostLoadEventListener.java
@@ -51,16 +51,18 @@ public void onPostLoad(PostLoadEvent event) {
 						OptimisticLockHelper.forceVersionIncrement( entity, entry, session );
 						break;
 					case OPTIMISTIC_FORCE_INCREMENT:
-						session.getActionQueue().registerCallback( new EntityIncrementVersionProcess( entity ) );
+						session.getActionQueue()
+								.registerCallback( new EntityIncrementVersionProcess( entity ) );
 						break;
 					case OPTIMISTIC:
-						session.getActionQueue().registerCallback( new EntityVerifyVersionProcess( entity ) );
+						session.getActionQueue()
+								.registerCallback( new EntityVerifyVersionProcess( entity ) );
 						break;
 				}
 			}
 			else {
-				throw new HibernateException("[" + lockMode
-						+ "] not supported for non-versioned entities [" + persister.getEntityName() + "]");
+				throw new HibernateException( "Entity '" + persister.getEntityName()
+							+ "' has no version and may not be locked at level " + lockMode);
 			}
 		}
 	}
diff --git a/hibernate-core/src/main/java/org/hibernate/internal/CoreMessageLogger.java b/hibernate-core/src/main/java/org/hibernate/internal/CoreMessageLogger.java
@@ -270,10 +270,6 @@ void missingArguments(
 	@Message(value = "Warnings creating temp table: %s", id = 413)
 	void warningsCreatingTempTable(SQLWarning warning);
 
-	@LogMessage(level = WARN)
-	@Message(value = "Write locks via update not supported for non-versioned entities [%s]", id = 416)
-	void writeLocksNotSupported(String entityName);
-
 	@LogMessage(level = WARN)
 	@Message(
 			value = """

Original file line number	Diff line number	Diff line change
`@@ -4,7 +4,6 @@`
`4`	`4`	`*/`
`5`	`5`	`package org.hibernate.dialect.lock;`
`6`	`6`
`7`		`-import org.hibernate.HibernateException;`
`8`	`7`	`import org.hibernate.LockMode;`
`9`	`8`	`import org.hibernate.persister.entity.EntityPersister;`
`10`	`9`
`@@ -30,8 +29,5 @@ public class PessimisticReadUpdateLockingStrategy extends AbstractPessimisticUpd`
`30`	`29`	`*/`
`31`	`30`	`public PessimisticReadUpdateLockingStrategy(EntityPersister lockable, LockMode lockMode) {`
`32`	`31`	`super( lockable, lockMode );`
`33`		`- if ( lockMode.lessThan( LockMode.PESSIMISTIC_READ ) ) {`
`34`		`- throw new HibernateException( "[" + lockMode + "] not valid for update statement" );`
`35`		`- }`
`36`	`32`	`}`
`37`	`33`	`}`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ public class UpdateLockingStrategy extends AbstractPessimisticUpdateLockingStrat`
`35`	`35`	`public UpdateLockingStrategy(EntityPersister lockable, LockMode lockMode) {`
`36`	`36`	`super( lockable, lockMode );`
`37`	`37`	`if ( lockMode.lessThan( LockMode.WRITE ) ) {`
`38`		`- throw new HibernateException( "[" + lockMode + "] not valid for update statement" );`
	`38`	`+ throw new HibernateException( "Lock mode " + lockMode + " not valid for locking via 'update' statement" );`
`39`	`39`	`}`
`40`	`40`	`}`
`41`	`41`
Original file line number	Diff line number	Diff line change
`@@ -51,16 +51,18 @@ public void onPostLoad(PostLoadEvent event) {`
`51`	`51`	`OptimisticLockHelper.forceVersionIncrement( entity, entry, session );`
`52`	`52`	`break;`
`53`	`53`	`case OPTIMISTIC_FORCE_INCREMENT:`
`54`		`- session.getActionQueue().registerCallback( new EntityIncrementVersionProcess( entity ) );`
	`54`	`+ session.getActionQueue()`
	`55`	`+ .registerCallback( new EntityIncrementVersionProcess( entity ) );`
`55`	`56`	`break;`
`56`	`57`	`case OPTIMISTIC:`
`57`		`- session.getActionQueue().registerCallback( new EntityVerifyVersionProcess( entity ) );`
	`58`	`+ session.getActionQueue()`
	`59`	`+ .registerCallback( new EntityVerifyVersionProcess( entity ) );`
`58`	`60`	`break;`
`59`	`61`	`}`
`60`	`62`	`}`
`61`	`63`	`else {`
`62`		`- throw new HibernateException("[" + lockMode`
`63`		`- + "] not supported for non-versioned entities [" + persister.getEntityName() + "]");`
	`64`	`+ throw new HibernateException( "Entity '" + persister.getEntityName()`
	`65`	`+ + "' has no version and may not be locked at level " + lockMode);`
`64`	`66`	`}`
`65`	`67`	`}`
`66`	`68`	`}`