From 02ad34091c477a90271928a96d5b772debe2e4f4 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Mon, 7 Nov 2022 12:09:48 +0100 Subject: [PATCH] HHH-15683+HHH-15684 clean up the handling of LockOptions for queries This contains a change to LockOptions.overlay() which is breaking in principle, but more natural and less fragile. It also deprecates SelectionQuery.setAliasSpecificLockMode() which I believe was added in 6.0 by mistake. The method is an overload of setLockMode() in the rest of the hierarchy. --- .../main/java/org/hibernate/LockOptions.java | 12 +++++++++- .../procedure/internal/ProcedureCallImpl.java | 20 ++++++++++------ .../java/org/hibernate/query/NativeQuery.java | 9 ++++---- .../main/java/org/hibernate/query/Query.java | 23 ++++++++++--------- .../org/hibernate/query/SelectionQuery.java | 17 ++++++++++---- .../hibernate/query/spi/AbstractQuery.java | 6 +---- .../query/spi/AbstractSelectionQuery.java | 3 +++ .../query/sql/internal/NativeQueryImpl.java | 16 +++++-------- .../query/sql/spi/NativeQueryImplementor.java | 8 ++----- .../sqm/internal/SqmSelectionQueryImpl.java | 17 +++++++++++--- .../orm/test/jpa/lock/QueryLockingTest.java | 2 +- 11 files changed, 81 insertions(+), 52 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/LockOptions.java b/hibernate-core/src/main/java/org/hibernate/LockOptions.java index 76370383f6..b293056a5f 100644 --- a/hibernate-core/src/main/java/org/hibernate/LockOptions.java +++ b/hibernate-core/src/main/java/org/hibernate/LockOptions.java @@ -432,8 +432,18 @@ public class LockOptions implements Serializable { return copy; } + /** + * Copy the given lock options into this instance, + * merging the alias-specific lock modes. + */ public void overlay(LockOptions lockOptions) { - copy( lockOptions, this ); + setLockMode( lockOptions.getLockMode() ); + setScope( lockOptions.getScope() ); + setTimeOut( lockOptions.getTimeOut() ); + if ( lockOptions.aliasSpecificLockModes != null ) { + lockOptions.aliasSpecificLockModes.forEach(this::setAliasSpecificLockMode); + } + setFollowOnLocking( lockOptions.getFollowOnLocking() ); } /** diff --git a/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureCallImpl.java b/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureCallImpl.java index 6f8dea902d..d2417b00fd 100644 --- a/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureCallImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureCallImpl.java @@ -23,6 +23,7 @@ import java.util.stream.Stream; import org.hibernate.HibernateException; import org.hibernate.LockMode; +import org.hibernate.LockOptions; import org.hibernate.ScrollMode; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; @@ -329,11 +330,6 @@ public class ProcedureCallImpl return queryOptions; } - @Override - public QueryImplementor setLockMode(String alias, LockMode lockMode) { - throw new IllegalStateException( "Cannot set LockMode on a procedure-call" ); - } - @Override public ProcedureParameterMetadataImpl getParameterMetadata() { return parameterMetadata; @@ -1085,14 +1081,24 @@ public class ProcedureCallImpl throw new PersistenceException( "Unrecognized unwrap type : " + cls.getName() ); } + @Override + public QueryImplementor setLockMode(String alias, LockMode lockMode) { + throw new UnsupportedOperationException( "setLockMode does not apply to procedure calls" ); + } + @Override public ProcedureCallImplementor setLockMode(LockModeType lockMode) { - throw new IllegalStateException("jakarta.persistence.Query.setLockMode not valid on jakarta.persistence.StoredProcedureQuery" ); + throw new UnsupportedOperationException( "setLockMode does not apply to procedure calls" ); } @Override public LockModeType getLockMode() { - throw new IllegalStateException( "jakarta.persistence.Query.getHibernateFlushMode not valid on jakarta.persistence.StoredProcedureQuery" ); + throw new UnsupportedOperationException( "getLockMode does not apply to procedure calls" ); + } + + @Override + public QueryImplementor setLockOptions(LockOptions lockOptions) { + throw new UnsupportedOperationException( "setLockOptions does not apply to procedure calls" ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/query/NativeQuery.java b/hibernate-core/src/main/java/org/hibernate/query/NativeQuery.java index 2cdbb70ac8..dd447bf1bb 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/NativeQuery.java +++ b/hibernate-core/src/main/java/org/hibernate/query/NativeQuery.java @@ -39,9 +39,9 @@ import org.hibernate.type.BasicTypeReference; * *
  • * Tables used via {@link #addSynchronizedQuerySpace}, {@link #addSynchronizedEntityName} and - * {@link org.hibernate.query.SynchronizeableQuery#addSynchronizedEntityClass}. This allows Hibernate to know how to properly deal with - * auto-flush checking as well as cached query results if the results of the query are being - * cached. + * {@link org.hibernate.query.SynchronizeableQuery#addSynchronizedEntityClass}. This allows + * Hibernate to know how to properly deal with auto-flush checking as well as cached query + * results if the results of the query are being cached. *
    • * * @@ -51,7 +51,8 @@ import org.hibernate.type.BasicTypeReference; * of its metadata * *
  • - * A pre-defined (defined in metadata and named) mapping can be associated via {@link org.hibernate.Session#createNativeQuery(String, String)} + * A pre-defined (defined in metadata and named) mapping can be associated via + * {@link org.hibernate.Session#createNativeQuery(String, String)} *
    • *
  • * Defined locally per the various {@link #addEntity}, {@link #addRoot}, {@link #addJoin}, diff --git a/hibernate-core/src/main/java/org/hibernate/query/Query.java b/hibernate-core/src/main/java/org/hibernate/query/Query.java index d70e60577e..0963fd72c1 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/Query.java +++ b/hibernate-core/src/main/java/org/hibernate/query/Query.java @@ -317,22 +317,22 @@ public interface Query extends SelectionQuery, MutationQuery, TypedQuery - * Only the following options are taken into consideration: - *
      - *
    1. {@link LockOptions#getLockMode()}
      2. - *
    2. {@link LockOptions#getScope()}
      3. - *
    3. {@link LockOptions#getTimeOut()}
      4. - *
    - * For alias-specific locking, use {@link #setLockMode(String, LockMode)}. + * Apply the given {@linkplain LockOptions lock options} to this + * query. Alias-specific lock modes in the given lock options are + * merged with any alias-specific lock mode which have already been + * {@linkplain #setAliasSpecificLockMode(String, LockMode) set}. If + * a lock mode has already been specified for an alias that is among + * the aliases in the given lock options, the lock mode specified in + * the given lock options overrides the lock mode that was already + * set. * * @param lockOptions The lock options to apply to the query. * @@ -353,13 +353,14 @@ public interface Query extends SelectionQuery, MutationQuery, TypedQuery setLockMode(String alias, LockMode lockMode); /** diff --git a/hibernate-core/src/main/java/org/hibernate/query/SelectionQuery.java b/hibernate-core/src/main/java/org/hibernate/query/SelectionQuery.java index bdaf5fc139..4f2942239d 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/SelectionQuery.java +++ b/hibernate-core/src/main/java/org/hibernate/query/SelectionQuery.java @@ -21,6 +21,7 @@ import org.hibernate.Incubating; import org.hibernate.LockMode; import org.hibernate.LockOptions; import org.hibernate.NonUniqueResultException; +import org.hibernate.Remove; import org.hibernate.ScrollMode; import org.hibernate.ScrollableResults; import org.hibernate.Session; @@ -324,25 +325,33 @@ public interface SelectionQuery extends CommonQueryContract { SelectionQuery setCacheRegion(String cacheRegion); /** - * The LockOptions currently in effect for the query + * The {@link LockOptions} currently in effect for the query */ LockOptions getLockOptions(); /** - * Specify the root LockModeType for the query + * Specify the root {@link LockModeType} for the query * * @see #setHibernateLockMode */ SelectionQuery setLockMode(LockModeType lockMode); /** - * Specify the root LockMode for the query + * Specify the root {@link LockMode} for the query */ SelectionQuery setHibernateLockMode(LockMode lockMode); /** - * Specify a LockMode to apply to a specific alias defined in the query + * Specify a {@link LockMode} to apply to a specific alias defined in the query */ + SelectionQuery setLockMode(String alias, LockMode lockMode); + + /** + * Specify a {@link LockMode} to apply to a specific alias defined in the query + * + * @deprecated use {@link #setLockMode(String, LockMode)} + */ + @Deprecated(since = "6.2") @Remove SelectionQuery setAliasSpecificLockMode(String alias, LockMode lockMode); /** diff --git a/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractQuery.java b/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractQuery.java index e2fd7b7ae1..f9eb1082e1 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractQuery.java +++ b/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractQuery.java @@ -247,16 +247,12 @@ public abstract class AbstractQuery @Override public LockModeType getLockMode() { getSession().checkOpen( false ); - return LockModeTypeHelper.getLockModeType( getQueryOptions().getLockOptions().getLockMode() ); } @Override public QueryImplementor setLockOptions(LockOptions lockOptions) { - getQueryOptions().getLockOptions().setLockMode( lockOptions.getLockMode() ); - getQueryOptions().getLockOptions().setScope( lockOptions.getScope() ); - getQueryOptions().getLockOptions().setTimeOut( lockOptions.getTimeOut() ); - getQueryOptions().getLockOptions().setFollowOnLocking( lockOptions.getFollowOnLocking() ); + getQueryOptions().getLockOptions().overlay( lockOptions ); return this; } diff --git a/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractSelectionQuery.java b/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractSelectionQuery.java index 7dc1ca07e7..144d8068eb 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractSelectionQuery.java +++ b/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractSelectionQuery.java @@ -585,7 +585,10 @@ public abstract class AbstractSelectionQuery /** * Specify a LockMode to apply to a specific alias defined in the query + * + * @deprecated use {{@link #setLockMode(String, LockMode)}} */ + @Override @Deprecated public SelectionQuery setAliasSpecificLockMode(String alias, LockMode lockMode) { getLockOptions().setAliasSpecificLockMode( alias, lockMode ); return this; diff --git a/hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java b/hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java index f296477302..4f99ee4a91 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sql/internal/NativeQueryImpl.java @@ -472,24 +472,22 @@ public class NativeQueryImpl @Override public LockModeType getLockMode() { - throw new IllegalStateException( "Illegal attempt to get lock mode on a native-query" ); + throw new UnsupportedOperationException( "Illegal attempt to get lock mode on a native-query" ); } @Override public NativeQueryImplementor setLockOptions(LockOptions lockOptions) { - super.setLockOptions( lockOptions ); - return this; + throw new UnsupportedOperationException( "Illegal attempt to set lock options for a native query" ); } @Override public NativeQueryImplementor setLockMode(String alias, LockMode lockMode) { - super.setLockMode( alias, lockMode ); - return this; + throw new UnsupportedOperationException( "Illegal attempt to set lock mode for a native query" ); } @Override public NativeQueryImplementor setLockMode(LockModeType lockModeType) { - throw new IllegalStateException( "Illegal attempt to set lock mode on a native-query" ); + throw new UnsupportedOperationException( "Illegal attempt to set lock mode for a native query" ); } @Override @@ -599,10 +597,8 @@ public class NativeQueryImpl private SelectQueryPlan resolveSelectQueryPlan() { final QueryInterpretationCache.Key cacheKey = generateSelectInterpretationsKey( resultSetMapping ); if ( cacheKey != null ) { - return getSession().getFactory().getQueryEngine().getInterpretationCache().resolveSelectQueryPlan( - cacheKey, - () -> createQueryPlan( resultSetMapping ) - ); + return getSession().getFactory().getQueryEngine().getInterpretationCache() + .resolveSelectQueryPlan( cacheKey, () -> createQueryPlan( resultSetMapping ) ); } else { return createQueryPlan( resultSetMapping ); diff --git a/hibernate-core/src/main/java/org/hibernate/query/sql/spi/NativeQueryImplementor.java b/hibernate-core/src/main/java/org/hibernate/query/sql/spi/NativeQueryImplementor.java index c78a55f7f5..1ca116f1a5 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sql/spi/NativeQueryImplementor.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sql/spi/NativeQueryImplementor.java @@ -43,14 +43,10 @@ import jakarta.persistence.metamodel.SingularAttribute; public interface NativeQueryImplementor extends QueryImplementor, NativeQuery, NameableQuery { @Override - default LockOptions getLockOptions() { - return null; - } + LockOptions getLockOptions(); @Override - default LockModeType getLockMode() { - return null; - } + LockModeType getLockMode(); /** * Best guess whether this is a select query. {@code null} diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmSelectionQueryImpl.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmSelectionQueryImpl.java index 3d7be6a7f9..f4088d423a 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmSelectionQueryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmSelectionQueryImpl.java @@ -456,7 +456,7 @@ public class SqmSelectionQueryImpl extends AbstractSelectionQuery implemen } /** - * Specify the root LockMode for the query + * Specify the root {@link LockMode} for the query */ @Override public SqmSelectionQuery setHibernateLockMode(LockMode lockMode) { @@ -465,10 +465,21 @@ public class SqmSelectionQueryImpl extends AbstractSelectionQuery implemen } /** - * Specify a LockMode to apply to a specific alias defined in the query + * Specify a {@link LockMode} to apply to a specific alias defined in the query + * + * @deprecated use {{@link #setLockMode(String, LockMode)}} + */ + @Override @Deprecated + public SqmSelectionQuery setAliasSpecificLockMode(String alias, LockMode lockMode) { + getLockOptions().setAliasSpecificLockMode( alias, lockMode ); + return this; + } + + /** + * Specify a {@link LockMode} to apply to a specific alias defined in the query */ @Override - public SqmSelectionQuery setAliasSpecificLockMode(String alias, LockMode lockMode) { + public SqmSelectionQuery setLockMode(String alias, LockMode lockMode) { getLockOptions().setAliasSpecificLockMode( alias, lockMode ); return this; } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/lock/QueryLockingTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/lock/QueryLockingTest.java index 16021d7224..0ccb47ca30 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/lock/QueryLockingTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/lock/QueryLockingTest.java @@ -120,7 +120,7 @@ public class QueryLockingTest extends BaseEntityManagerFunctionalTestCase { query.setLockMode( LockModeType.READ ); fail( "Should have failed" ); } - catch (IllegalStateException expected) { + catch (UnsupportedOperationException expected) { } // however, we should be able to set it using hints