consistently pass along session-level lock options

previously, these were respected by a random subset of session methods

Signed-off-by: Gavin King <gavin@hibernate.org>

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

@@ -106,7 +106,7 @@ public class DefaultLoadEventListener implements LoadEventListener {
 			}
 			else {
 				//return a proxy if appropriate
-				Object result = event.getLockMode() == LockMode.NONE
+				final Object result = event.getLockMode() == LockMode.NONE
 						? proxyOrLoad( event, persister, keyToLoad, loadType )
 						: lockAndLoad( event, persister, keyToLoad, loadType );
 				event.setResult( result );
@@ -123,16 +123,15 @@ public class DefaultLoadEventListener implements LoadEventListener {
 		// we may have the jpa requirement of allowing find-by-id where id is the "simple pk value" of a
 		// dependent objects parent. This is part of its generally goofy derived identity "feature"
 		final EntityIdentifierMapping idMapping = persister.getIdentifierMapping();
-		if ( idMapping instanceof CompositeIdentifierMapping ) {
+		if ( idMapping instanceof CompositeIdentifierMapping compositeIdMapping ) {
-			final CompositeIdentifierMapping compositeIdMapping = (CompositeIdentifierMapping) idMapping;
 			final EmbeddableMappingType partMappingType = compositeIdMapping.getPartMappingType();
 			if ( partMappingType.getNumberOfAttributeMappings() == 1 ) {
 				final AttributeMapping singleIdAttribute = partMappingType.getAttributeMapping( 0 );
-				if ( singleIdAttribute.getMappedType() instanceof EntityMappingType ) {
+				if ( singleIdAttribute.getMappedType() instanceof EntityMappingType parentIdTargetMapping ) {
-					final EntityMappingType parentIdTargetMapping = (EntityMappingType) singleIdAttribute.getMappedType();
 					final EntityIdentifierMapping parentIdTargetIdMapping = parentIdTargetMapping.getIdentifierMapping();
-					final MappingType parentIdType = parentIdTargetIdMapping instanceof CompositeIdentifierMapping
+					final MappingType parentIdType =
-							? ((CompositeIdentifierMapping) parentIdTargetIdMapping).getMappedIdEmbeddableTypeDescriptor()
+							parentIdTargetIdMapping instanceof CompositeIdentifierMapping compositeMapping
+									? compositeMapping.getMappedIdEmbeddableTypeDescriptor()
 									: parentIdTargetIdMapping.getMappedType();
 					if ( parentIdType.getMappedJavaType().getJavaTypeClass().isInstance( event.getEntityId() ) ) {
 						// yep that's what we have...
@@ -309,8 +308,8 @@ public class DefaultLoadEventListener implements LoadEventListener {
 //		else {
 			// if the entity defines a HibernateProxy factory, see if there is an
 			// existing proxy associated with the PC - and if so, use it
-			final Object proxy;
+			final Object proxy = holder == null ? null : holder.getProxy();
-			if ( holder != null && ( proxy = holder.getProxy() ) != null ) {
+			if ( proxy != null ) {
 				LOG.trace( "Entity proxy found in session cache" );
 				if ( LOG.isDebugEnabled() && HibernateProxy.extractLazyInitializer( proxy ).isUnwrap() ) {
 					LOG.debug( "Ignoring NO_PROXY to honor laziness" );
@@ -372,7 +371,7 @@ public class DefaultLoadEventListener implements LoadEventListener {
 		if ( LOG.isTraceEnabled() ) {
 			LOG.trace( "Entity proxy found in session cache" );
 		}
-		LazyInitializer li = HibernateProxy.extractLazyInitializer( proxy );
+		final LazyInitializer li = HibernateProxy.extractLazyInitializer( proxy );
 		if ( li.isUnwrap() ) {
 			return li.getImplementation();
 		}
@@ -389,7 +388,7 @@ public class DefaultLoadEventListener implements LoadEventListener {
 	}
 
 	private Object proxyImplementation(LoadEvent event, EntityPersister persister, EntityKey keyToLoad, LoadType options) {
-		Object entity = load( event, persister, keyToLoad, options );
+		final Object entity = load( event, persister, keyToLoad, options );
 		if ( entity != null ) {
 			return entity;
 		}
@@ -415,7 +414,7 @@ public class DefaultLoadEventListener implements LoadEventListener {
 	 * @param persister The persister corresponding to the entity to be loaded
 	 * @param keyToLoad The key of the entity to be loaded
 	 * @param options The defined load options
-	 * @param holder
+	 * @param holder an {@link EntityHolder} for the key
 	 *
 	 * @return The created/existing proxy
 	 */
@@ -467,7 +466,7 @@ public class DefaultLoadEventListener implements LoadEventListener {
 	 * @return The loaded entity
 	 */
 	private Object lockAndLoad(LoadEvent event, EntityPersister persister, EntityKey keyToLoad, LoadType options) {
-		final SessionImplementor source = event.getSession();;
+		final SessionImplementor source = event.getSession();
 		final EntityDataAccess cache = persister.getCacheAccessStrategy();
 
 		final SoftLock lock;

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

@@ -17,6 +17,7 @@ import java.util.Set;
 import javax.naming.NamingException;
 
 import org.hibernate.HibernateException;
+import org.hibernate.JDBCException;
 import org.hibernate.LockMode;
 import org.hibernate.cache.CacheException;
 import org.hibernate.dialect.Dialect;
@@ -964,4 +965,9 @@ public interface CoreMessageLogger extends BasicLogger {
 	@Message(value = "Failed to discover types for enhancement from class: %s",
 			id = 516)
 	void enhancementDiscoveryFailed(String className, @Cause Throwable cause);
+
+	@LogMessage(level = DEBUG)
+	@Message(value = "JDBCException was thrown for a transaction marked for rollback. " +
+			" This is probably due to an operation failing fast due to the transaction being marked for rollback.")
+	void jdbcExceptionThrownWithTransactionRolledBack(@Cause JDBCException e);
 }

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

@@ -309,14 +309,14 @@ public class SessionImpl
 	}
 
 	private LockOptions getLockOptionsForRead() {
-		return this.lockOptions == null ? fastSessionServices.defaultLockOptions : this.lockOptions;
+		return lockOptions == null ? fastSessionServices.defaultLockOptions : lockOptions;
 	}
 
 	private LockOptions getLockOptionsForWrite() {
-		if ( this.lockOptions == null ) {
+		if ( lockOptions == null ) {
-			this.lockOptions = new LockOptions();
+			lockOptions = new LockOptions();
 		}
-		return this.lockOptions;
+		return lockOptions;
 	}
 
 	protected void applyQuerySettingsAndHints(SelectionQuery<?> query) {
@@ -647,7 +647,9 @@ public class SessionImpl
 
 	@Override
 	public void lock(Object object, LockMode lockMode) throws HibernateException {
-		fireLock( new LockEvent( object, lockMode, this ) );
+		final LockOptions lockOptions = copySessionLockOptions();
+		lockOptions.setLockMode( lockMode );
+		fireLock( new LockEvent( object, lockOptions, this ) );
 	}
 
 	private void fireLock(LockEvent event) {
@@ -852,13 +854,7 @@ public class SessionImpl
 			logRemoveOrphanBeforeUpdates( "before continuing", entityName, object );
 		}
 		fireDelete(
-				new DeleteEvent(
+				new DeleteEvent( entityName, object, isCascadeDeleteEnabled, removingOrphanBeforeUpates, this ),
-						entityName,
-						object,
-						isCascadeDeleteEnabled,
-						removingOrphanBeforeUpates,
-						this
-				),
 				transientEntities
 		);
 		if ( traceEnabled && removingOrphanBeforeUpates ) {
@@ -1159,7 +1155,9 @@ public class SessionImpl
 
 	@Override
 	public void refresh(Object object, LockMode lockMode) throws HibernateException {
-		fireRefresh( new RefreshEvent( object, lockMode, this ) );
+		final LockOptions lockOptions = copySessionLockOptions();
+		lockOptions.setLockMode( lockMode );
+		fireRefresh( new RefreshEvent( object, lockOptions, this ) );
 	}
 
 	@Override
@@ -2260,11 +2258,10 @@ public class SessionImpl
 	public <T> T find(Class<T> entityClass, Object primaryKey, LockModeType lockModeType, Map<String, Object> properties) {
 		checkOpen();
 
-		LockOptions lockOptions = null;
+		final LockOptions lockOptions = lockModeType == null ? null : buildLockOptions( lockModeType, properties );
 		try {
 			if ( lockModeType != null ) {
 				checkTransactionNeededForLock( LockModeTypeHelper.getLockMode( lockModeType ) );
-				lockOptions = buildLockOptions( lockModeType, properties );
 			}
 
 			final EffectiveEntityGraph effectiveEntityGraph = loadQueryInfluencers.getEffectiveEntityGraph();
@@ -2276,33 +2273,34 @@ public class SessionImpl
 					.with( lockOptions )
 					.load( primaryKey );
 		}
-		catch ( EntityNotFoundException enfe ) {
+		catch ( FetchNotFoundException e ) {
 			// This may happen if the entity has an associations mapped with
 			// @NotFound(action = NotFoundAction.EXCEPTION) and this associated
 			// entity is not found
-			if ( enfe instanceof FetchNotFoundException ) {
+			throw e;
-				throw enfe;
 		}
+		catch ( EntityFilterException e ) {
 			// This may happen if the entity has an associations which is
 			// filtered by a FilterDef and this associated entity is not found
-			if ( enfe instanceof EntityFilterException ) {
+			throw e;
-				throw enfe;
-			}
-			// DefaultLoadEventListener#returnNarrowedProxy() may throw ENFE (see HHH-7861 for details),
-			// which find() should not throw.  Find() should return null if the entity was not found.
-			if ( log.isDebugEnabled() ) {
-				String entityName = entityClass != null ? entityClass.getName(): null;
-				String identifierValue = primaryKey != null ? primaryKey.toString() : null ;
-				log.ignoringEntityNotFound( entityName, identifierValue );
 		}
+		catch ( EntityNotFoundException e ) {
+			// We swallow other sorts of EntityNotFoundException and return null
+			// For example, DefaultLoadEventListener.proxyImplementation() throws
+			// EntityNotFoundException if there's an existing proxy in the session,
+			// but the underlying database row has been deleted (see HHH-7861)
+			logIgnoringEntityNotFound( entityClass, primaryKey );
 			return null;
 		}
 		catch ( ObjectDeletedException e ) {
-			//the spec is silent about people doing remove() find() on the same PC
+			// the spec is silent about people doing remove() find() on the same PC
 			return null;
 		}
 		catch ( ObjectNotFoundException e ) {
-			//should not happen on the entity itself with get
+			// should not happen on the entity itself with get
+			// TODO: in fact this will occur instead of EntityNotFoundException
+			//       when using StandardEntityNotFoundDelegate, so probably we
+			//       should return null here, as we do above
 			throw new IllegalArgumentException( e.getMessage(), e );
 		}
 		catch ( MappingException | TypeMismatchException | ClassCastException e ) {
@@ -2310,13 +2308,9 @@ public class SessionImpl
 		}
 		catch ( JDBCException e ) {
 			if ( accessTransaction().isActive() && accessTransaction().getRollbackOnly() ) {
-				// Assume this is similar to the WildFly / IronJacamar "feature" described under HHH-12472.
+				// Assume situation HHH-12472 running on WildFly
-				// Just log the exception and return null.
+				// Just log the exception and return null
-				if ( log.isDebugEnabled() ) {
+				log.jdbcExceptionThrownWithTransactionRolledBack( e );
-					log.debug( "JDBCException was thrown for a transaction marked for rollback; " +
-									"this is probably due to an operation failing fast due to the " +
-									"transaction marked for rollback.", e );
-				}
 				return null;
 			}
 			else {
@@ -2332,11 +2326,20 @@ public class SessionImpl
 		}
 	}
 
+	private static <T> void logIgnoringEntityNotFound(Class<T> entityClass, Object primaryKey) {
+		if ( log.isDebugEnabled() ) {
+			log.ignoringEntityNotFound(
+					entityClass != null ? entityClass.getName(): null,
+					primaryKey != null ? primaryKey.toString() : null
+			);
+		}
+	}
+
 	private <T> IdentifierLoadAccessImpl<T> loadAccessWithOptions(Class<T> entityClass, FindOption[] options) {
-		final IdentifierLoadAccessImpl<T> loadAccess = byId(entityClass);
+		final IdentifierLoadAccessImpl<T> loadAccess = byId( entityClass );
 		CacheStoreMode storeMode = getCacheStoreMode();
 		CacheRetrieveMode retrieveMode = getCacheRetrieveMode();
-		LockOptions lockOptions = new LockOptions();
+		LockOptions lockOptions = copySessionLockOptions();
 		for ( FindOption option : options ) {
 			if ( option instanceof CacheStoreMode cacheStoreMode ) {
 				storeMode = cacheStoreMode;
@@ -2481,8 +2484,9 @@ public class SessionImpl
 		lock( entity, buildLockOptions( lockMode, options ) );
 	}
 
-	private static LockOptions buildLockOptions(LockModeType lockMode, LockOption[] options) {
+	private LockOptions buildLockOptions(LockModeType lockModeType, LockOption[] options) {
-		final LockOptions lockOptions = new LockOptions( LockModeTypeHelper.getLockMode( lockMode ) );
+		final LockOptions lockOptions = copySessionLockOptions();
+		lockOptions.setLockMode( LockModeTypeHelper.getLockMode( lockModeType ) );
 		for ( LockOption option : options ) {
 			if ( option instanceof PessimisticLockScope lockScope ) {
 				lockOptions.setLockScope( lockScope );
@@ -2494,6 +2498,23 @@ public class SessionImpl
 		return lockOptions;
 	}
 
+	private LockOptions buildLockOptions(LockModeType lockModeType, Map<String, Object> properties) {
+		final LockOptions lockOptions = copySessionLockOptions();
+		lockOptions.setLockMode( LockModeTypeHelper.getLockMode( lockModeType ) );
+		if ( properties != null ) {
+			applyPropertiesToLockOptions( properties, () -> lockOptions );
+		}
+		return lockOptions;
+	}
+
+	private LockOptions copySessionLockOptions() {
+		final LockOptions copiedLockOptions = new LockOptions();
+		if ( lockOptions != null ) {
+			LockOptions.copy( lockOptions, copiedLockOptions );
+		}
+		return copiedLockOptions;
+	}
+
 	@Override
 	public void refresh(Object entity, LockModeType lockModeType) {
 		refresh( entity, LockModeTypeHelper.getLockMode( lockModeType ) );
@@ -2526,7 +2547,7 @@ public class SessionImpl
 	@Override
 	public void refresh(Object entity, RefreshOption... options) {
 		CacheStoreMode storeMode = getCacheStoreMode();
-		LockOptions lockOptions = new LockOptions();
+		LockOptions lockOptions = copySessionLockOptions();
 		for ( RefreshOption option : options ) {
 			if ( option instanceof CacheStoreMode cacheStoreMode ) {
 				storeMode = cacheStoreMode;
@@ -2560,18 +2581,6 @@ public class SessionImpl
 		}
 	}
 
-	private LockOptions buildLockOptions(LockModeType lockModeType, Map<String, Object> properties) {
-		final LockOptions lockOptions = new LockOptions();
-		if ( this.lockOptions != null ) { //otherwise the default LockOptions constructor is the same as DEFAULT_LOCK_OPTIONS
-			LockOptions.copy( this.lockOptions, lockOptions );
-		}
-		lockOptions.setLockMode( LockModeTypeHelper.getLockMode( lockModeType ) );
-		if ( properties != null ) {
-			applyPropertiesToLockOptions( properties, () -> lockOptions );
-		}
-		return lockOptions;
-	}
-
 	@Override
 	public void detach(Object entity) {
 		checkOpen();