diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/mutiny/impl/MutinySessionImpl.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/mutiny/impl/MutinySessionImpl.java index c80be96ed..5f4ea40f4 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/mutiny/impl/MutinySessionImpl.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/mutiny/impl/MutinySessionImpl.java @@ -221,7 +221,7 @@ public SelectionQuery createNativeQuery(String queryString, ResultSetMapp @Override public Uni find(Class entityClass, Object primaryKey) { - return uni( () -> delegate.reactiveFind( entityClass, primaryKey, null, null ) ); + return uni( () -> delegate.reactiveFind( entityClass, primaryKey ) ); } @Override @@ -236,7 +236,7 @@ public Uni find(Class entityClass, Identifier id) { @Override public Uni find(Class entityClass, Object primaryKey, LockMode lockMode) { - return uni( () -> delegate.reactiveFind( entityClass, primaryKey, new LockOptions( lockMode ), null ) ); + return uni( () -> delegate.reactiveFind( entityClass, primaryKey, lockMode, null ) ); } @Override @@ -252,7 +252,7 @@ public Uni find(Class entityClass, Object primaryKey, LockOptions lock @Override public Uni find(EntityGraph entityGraph, Object id) { Class entityClass = ( (RootGraph) entityGraph ).getGraphedType().getJavaType(); - return uni( () -> delegate.reactiveFind( entityClass, id, null, entityGraph ) ); + return uni( () -> delegate.reactiveFind( entityClass, id, (LockMode) null, entityGraph ) ); } @Override @@ -297,7 +297,7 @@ public Uni refresh(Object entity) { @Override public Uni refresh(Object entity, LockMode lockMode) { - return uni( () -> delegate.reactiveRefresh( entity, new LockOptions( lockMode ) ) ); + return uni( () -> delegate.reactiveRefresh( entity, lockMode ) ); } @Override @@ -317,7 +317,7 @@ public Uni refreshAll(Object... entity) { @Override public Uni lock(Object entity, LockMode lockMode) { - return uni( () -> delegate.reactiveLock( entity, new LockOptions( lockMode ) ) ); + return uni( () -> delegate.reactiveLock( entity, lockMode ) ); } @Override diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/session/ReactiveSession.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/session/ReactiveSession.java index ab2089c0d..db141aadd 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/session/ReactiveSession.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/session/ReactiveSession.java @@ -75,16 +75,33 @@ public interface ReactiveSession extends ReactiveQueryProducer, ReactiveSharedSe CompletionStage reactiveRefresh(Object entity, LockOptions lockMode); + default CompletionStage reactiveRefresh(Object entity, LockMode lockMode) { + return reactiveRefresh( entity, new LockOptions( lockMode ) ); + } + CompletionStage reactiveRefresh(Object child, RefreshContext refreshedAlready); CompletionStage reactiveLock(Object entity, LockOptions lockMode); + default CompletionStage reactiveLock(Object entity, LockMode lockMode){ + return reactiveLock( entity, new LockOptions( lockMode ) ); + } + CompletionStage reactiveLock(String entityName, Object entity, LockOptions lockMode); CompletionStage reactiveGet(Class entityClass, Object id); CompletionStage reactiveFind(Class entityClass, Object id, LockOptions lockOptions, EntityGraph fetchGraph); + default CompletionStage reactiveFind(Class entityClass, Object id){ + return reactiveFind( entityClass, id, (LockOptions) null, null ); + } + + default CompletionStage reactiveFind(Class entityClass, Object id, LockMode lockMode, EntityGraph fetchGraph ){ + return reactiveFind( entityClass, id, new LockOptions( lockMode ), fetchGraph ); + } + + CompletionStage> reactiveFind(Class entityClass, Object... ids); CompletionStage reactiveFind(Class entityClass, Map naturalIds); diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/session/impl/ReactiveSessionImpl.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/session/impl/ReactiveSessionImpl.java index 7b0bd112a..b6f795a1e 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/session/impl/ReactiveSessionImpl.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/session/impl/ReactiveSessionImpl.java @@ -1133,6 +1133,11 @@ public CompletionStage reactiveRefresh(Object entity, LockOptions lockOpti return fireRefresh( new RefreshEvent( entity, lockOptions, this ) ); } + @Override + public CompletionStage reactiveRefresh(Object entity, LockMode lockMode) { + return reactiveRefresh( entity, toLockOptions( lockMode ) ); + } + @Override public CompletionStage reactiveRefresh(Object object, RefreshContext refreshedAlready) { checkOpenOrWaitingForAutoClose(); @@ -1193,6 +1198,11 @@ public CompletionStage reactiveLock(Object object, LockOptions lockOptions return fireLock( new LockEvent( object, lockOptions, this ) ); } + @Override + public CompletionStage reactiveLock(Object entity, LockMode lockMode) { + return reactiveLock( entity, toLockOptions( lockMode ) ); + } + @Override public CompletionStage reactiveLock(String entityName, Object object, LockOptions lockOptions) { checkOpen(); @@ -1254,6 +1264,11 @@ public CompletionStage reactiveFind( } ); } + @Override + public CompletionStage reactiveFind(Class entityClass, Object id, LockMode lockMode, EntityGraph fetchGraph){ + return reactiveFind( entityClass, id, toLockOptions( lockMode ), fetchGraph ); + } + private CompletionStage handleReactiveFindException( Class entityClass, Object primaryKey, @@ -1318,7 +1333,7 @@ public CompletionStage reactiveFind(Class entityClass, Map( this, persister, requireEntityPersister( entityClass ) ) .resolveNaturalId( normalizedIdValues ) - .thenCompose( id -> reactiveFind( entityClass, id, null, null ) ); + .thenCompose( id -> reactiveFind( entityClass, id ) ); } private ReactiveEntityPersister entityPersister(Class entityClass) { @@ -1811,4 +1826,21 @@ public RootGraphImplementor getEntityGraph(Class entity, String name) } return (RootGraphImplementor) entityGraph; } + + /** + * Convert a {@link LockMode} into a {@link LockOptions} object. + *

+ * We need to make sure that we use the method {@link LockOptions#setLockMode(LockMode)} for the conversion + * because it also set a {@link LockOptions#timeout} that will affect the way SQL queries are generated. + * There's also the constructor {@link LockOptions#LockOptions(LockMode)}, but it doesn't set a time-out + * causing some generated SQL queries to not have the expected syntax (for example, it won't apply + * the "nowait" clause in PostgreSQL, even if set to {@link LockMode#UPGRADE_NOWAIT} ). + *

+ * @see Hibernate Reactive issue 2534 + */ + private static LockOptions toLockOptions(LockMode lockMode) { + return lockMode == null + ? null + : new LockOptions().setLockMode( lockMode ); + } } diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/stage/impl/StageSessionImpl.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/stage/impl/StageSessionImpl.java index da09a284b..daf467b2e 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/stage/impl/StageSessionImpl.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/stage/impl/StageSessionImpl.java @@ -135,7 +135,7 @@ public MutationQuery createMutationQuery(JpaCriteriaInsert insert) { @Override public CompletionStage find(Class entityClass, Object primaryKey) { - return delegate.reactiveFind( entityClass, primaryKey, null, null ); + return delegate.reactiveFind( entityClass, primaryKey ); } @Override @@ -150,7 +150,7 @@ public CompletionStage find(Class entityClass, Identifier id) { @Override public CompletionStage find(Class entityClass, Object primaryKey, LockMode lockMode) { - return delegate.reactiveFind( entityClass, primaryKey, new LockOptions( lockMode ), null ); + return delegate.reactiveFind( entityClass, primaryKey, lockMode, null ); } @Override @@ -167,7 +167,7 @@ public CompletionStage find(Class entityClass, Object primaryKey, Lock @Override public CompletionStage find(EntityGraph entityGraph, Object id) { Class entityClass = ( (RootGraph) entityGraph ).getGraphedType().getJavaType(); - return delegate.reactiveFind( entityClass, id, null, entityGraph ); + return delegate.reactiveFind( entityClass, id, (LockOptions) null, entityGraph ); } @Override @@ -212,7 +212,7 @@ public CompletionStage refresh(Object entity) { @Override public CompletionStage refresh(Object entity, LockMode lockMode) { - return delegate.reactiveRefresh( entity, new LockOptions(lockMode) ); + return delegate.reactiveRefresh( entity, lockMode ); } @Override @@ -232,7 +232,7 @@ public CompletionStage refresh(Object... entity) { @Override public CompletionStage lock(Object entity, LockMode lockMode) { - return delegate.reactiveLock( entity, new LockOptions(lockMode) ); + return delegate.reactiveLock( entity, lockMode ); } @Override diff --git a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/FindByIdWithLockTest.java b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/FindByIdWithLockTest.java index e78c549ca..51eece400 100644 --- a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/FindByIdWithLockTest.java +++ b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/FindByIdWithLockTest.java @@ -5,11 +5,17 @@ */ package org.hibernate.reactive; +import org.hibernate.LockMode; +import org.hibernate.boot.registry.StandardServiceRegistryBuilder; +import org.hibernate.cfg.Configuration; +import org.hibernate.reactive.testing.SqlStatementTracker; + import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.concurrent.TimeUnit; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import io.vertx.junit5.Timeout; @@ -22,11 +28,38 @@ import jakarta.persistence.OneToMany; import static org.assertj.core.api.Assertions.assertThat; +import static org.hibernate.reactive.containers.DatabaseConfiguration.dbType; @Timeout(value = 10, timeUnit = TimeUnit.MINUTES) public class FindByIdWithLockTest extends BaseReactiveTest { private static final Long CHILD_ID = 1L; + private static SqlStatementTracker sqlTracker; + + @Override + protected Configuration constructConfiguration() { + Configuration configuration = super.constructConfiguration(); + + // Construct a tracker that collects query statements via the SqlStatementLogger framework. + // Pass in configuration properties to hand off any actual logging properties + sqlTracker = new SqlStatementTracker( FindByIdWithLockTest::selectQueryFilter, configuration.getProperties() ); + return configuration; + } + + @BeforeEach + public void clearTracker() { + sqlTracker.clear(); + } + + @Override + protected void addServices(StandardServiceRegistryBuilder builder) { + sqlTracker.registerService( builder ); + } + + private static boolean selectQueryFilter(String s) { + return s.toLowerCase().startsWith( "select " ); + } + @Override protected Collection> annotatedEntities() { return List.of( Parent.class, Child.class ); @@ -50,6 +83,44 @@ context, getMutinySessionFactory() ); } + @Test + public void testFindUpgradeNoWait(VertxTestContext context) { + Child child = new Child( CHILD_ID, "And" ); + test( + context, getMutinySessionFactory() + .withTransaction( session -> session.persistAll( child ) ) + .invoke( () -> sqlTracker.clear() ) + .chain( () -> getMutinySessionFactory().withTransaction( session -> session + .find( Child.class, CHILD_ID, LockMode.UPGRADE_NOWAIT ) + .invoke( c -> { + assertThat( c ).isNotNull(); + assertThat( c.getId() ).isEqualTo( CHILD_ID ); + String selectQuery = sqlTracker.getLoggedQueries().get( 0 ); + assertThat( sqlTracker.getLoggedQueries() ).hasSize( 1 ); + assertThat( selectQuery ) + .matches( this::noWaitLockingPredicate, "SQL query with nowait lock for " + dbType().name() ); + } + ) ) ) + ); + } + + /** + * @return true if the query contains the expected nowait keyword for the selected database + */ + private boolean noWaitLockingPredicate(String selectQuery) { + return switch ( dbType() ) { + case POSTGRESQL -> selectQuery.endsWith( "for no key update of c1_0 nowait" ); + case COCKROACHDB -> selectQuery.endsWith( "for update of c1_0 nowait" ); + case SQLSERVER -> selectQuery.contains( "with (updlock,holdlock,rowlock,nowait)" ); + case ORACLE -> selectQuery.contains( "for update of c1_0.id nowait" ); + // DB2 does not support nowait + case DB2 -> selectQuery.contains( "for read only with rs use and keep update locks" ); + case MARIA -> selectQuery.contains( "for update nowait" ); + case MYSQL -> selectQuery.contains( "for update of c1_0 nowait" ); + default -> throw new IllegalArgumentException( "Database not recognized: " + dbType().name() ); + }; + } + @Entity(name = "Parent") public static class Parent { @@ -59,7 +130,7 @@ public static class Parent { private String name; @OneToMany(fetch = FetchType.EAGER) - public List children; + public List children = new ArrayList<>(); public Parent() { } @@ -70,9 +141,6 @@ public Parent(Long id, String name) { } public void add(Child child) { - if ( children == null ) { - children = new ArrayList<>(); - } children.add( child ); } @@ -89,7 +157,6 @@ public List getChildren() { } } - @Entity(name = "Child") public static class Child { @@ -109,13 +176,6 @@ public Child(Long id, String name) { this.name = name; } - public Child(Long id, String name, Parent parent) { - this.id = id; - this.name = name; - this.parent = parent; - parent.add( this ); - } - public Long getId() { return id; } @@ -128,6 +188,4 @@ public Parent getParent() { return parent; } } - - }