diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sqm/mutation/internal/temptable/ReactiveGlobalTemporaryTableStrategy.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sqm/mutation/internal/temptable/ReactiveGlobalTemporaryTableStrategy.java index 37c3d23bc..d0204ffff 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sqm/mutation/internal/temptable/ReactiveGlobalTemporaryTableStrategy.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sqm/mutation/internal/temptable/ReactiveGlobalTemporaryTableStrategy.java @@ -27,8 +27,6 @@ public interface ReactiveGlobalTemporaryTableStrategy { - Log LOG = make( Log.class, lookup() ); - static String sessionIdentifier(SharedSessionContractImplementor session) { return session.getSessionIdentifier().toString(); } @@ -65,22 +63,23 @@ default void prepare(MappingModelCreationProcess mappingModelCreationProcess, Jd if ( !createIdTables ) { tableCreatedStage.complete( null ); } - - LOG.debugf( "Creating global-temp ID table : %s", getTemporaryTable().getTableExpression() ); - - connectionStage() - .thenCompose( this::createTable ) - .whenComplete( (connection, throwable) -> releaseConnection( connection ) - .thenAccept( v -> { - if ( throwable == null ) { - setDropIdTables( configService ); - tableCreatedStage.complete( null ); - } - else { - tableCreatedStage.completeExceptionally( throwable ); - } - } ) - ); + else { + make( Log.class, lookup() ).debugf( "Creating global-temp ID table : %s", getTemporaryTable().getTableExpression() ); + + connectionStage() + .thenCompose( this::createTable ) + .whenComplete( (connection, throwable) -> releaseConnection( connection ) + .thenAccept( v -> { + if ( throwable == null ) { + setDropIdTables( configService ); + tableCreatedStage.complete( null ); + } + else { + tableCreatedStage.completeExceptionally( throwable ); + } + } ) + ); + } } private CompletionStage releaseConnection(ReactiveConnection connection) { @@ -103,7 +102,7 @@ private CompletionStage releaseConnection(ReactiveConnection connection) { private static void logConnectionClosedError(Throwable t) { if ( t != null ) { - LOG.debugf( "Ignoring error closing the connection: %s", t.getMessage() ); + make( Log.class, lookup() ).debugf( "Ignoring error closing the connection: %s", t.getMessage() ); } } @@ -147,24 +146,25 @@ default void release( if ( !isDropIdTables() ) { tableDroppedStage.complete( null ); } - - setDropIdTables( false ); - - final TemporaryTable temporaryTable = getTemporaryTable(); - LOG.debugf( "Dropping global-tempk ID table : %s", temporaryTable.getTableExpression() ); - - connectionStage() - .thenCompose( this::dropTable ) - .whenComplete( (connection, throwable) -> releaseConnection( connection ) - .thenAccept( v -> { - if ( throwable == null ) { - tableDroppedStage.complete( null ); - } - else { - tableDroppedStage.completeExceptionally( throwable ); - } - } ) - ); + else { + setDropIdTables( false ); + + final TemporaryTable temporaryTable = getTemporaryTable(); + make( Log.class, lookup() ).debugf( "Dropping global-tempk ID table : %s", temporaryTable.getTableExpression() ); + + connectionStage() + .thenCompose( this::dropTable ) + .whenComplete( (connection, throwable) -> releaseConnection( connection ) + .thenAccept( v -> { + if ( throwable == null ) { + tableDroppedStage.complete( null ); + } + else { + tableDroppedStage.completeExceptionally( throwable ); + } + } ) + ); + } } private CompletionStage dropTable(ReactiveConnection connection) { diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sqm/mutation/internal/temptable/ReactivePersistentTableStrategy.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sqm/mutation/internal/temptable/ReactivePersistentTableStrategy.java index 472dc0ce7..ba70601ad 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sqm/mutation/internal/temptable/ReactivePersistentTableStrategy.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sqm/mutation/internal/temptable/ReactivePersistentTableStrategy.java @@ -68,22 +68,23 @@ default void prepare(MappingModelCreationProcess mappingModelCreationProcess, Jd if ( !createIdTables ) { tableCreatedStage.complete( null ); } - - LOG.debugf( "Creating persistent ID table : %s", getTemporaryTable().getTableExpression() ); - - connectionStage() - .thenCompose( this::createTable ) - .whenComplete( (connection, throwable) -> releaseConnection( connection ) - .thenAccept( v -> { - if ( throwable == null ) { - setDropIdTables( configService ); - tableCreatedStage.complete( null ); - } - else { - tableCreatedStage.completeExceptionally( throwable ); - } - } ) - ); + else { + LOG.debugf( "Creating persistent ID table : %s", getTemporaryTable().getTableExpression() ); + + connectionStage() + .thenCompose( this::createTable ) + .whenComplete( (connection, throwable) -> releaseConnection( connection ) + .thenAccept( v -> { + if ( throwable == null ) { + setDropIdTables( configService ); + tableCreatedStage.complete( null ); + } + else { + tableCreatedStage.completeExceptionally( throwable ); + } + } ) + ); + } } private CompletionStage releaseConnection(ReactiveConnection connection) { @@ -150,24 +151,25 @@ default void release( if ( !isDropIdTables() ) { tableDroppedStage.complete( null ); } - - setDropIdTables( false ); - - final TemporaryTable temporaryTable = getTemporaryTable(); - LOG.debugf( "Dropping persistent ID table : %s", temporaryTable.getTableExpression() ); - - connectionStage() - .thenCompose( this::dropTable ) - .whenComplete( (connection, throwable) -> releaseConnection( connection ) - .thenAccept( v -> { - if ( throwable == null ) { - tableDroppedStage.complete( null ); - } - else { - tableDroppedStage.completeExceptionally( throwable ); - } - } ) - ); + else { + setDropIdTables( false ); + + final TemporaryTable temporaryTable = getTemporaryTable(); + LOG.debugf( "Dropping persistent ID table : %s", temporaryTable.getTableExpression() ); + + connectionStage() + .thenCompose( this::dropTable ) + .whenComplete( (connection, throwable) -> releaseConnection( connection ) + .thenAccept( v -> { + if ( throwable == null ) { + tableDroppedStage.complete( null ); + } + else { + tableDroppedStage.completeExceptionally( throwable ); + } + } ) + ); + } } private CompletionStage dropTable(ReactiveConnection connection) { diff --git a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sqm/mutation/internal/temptable/ReactiveTemporaryTableHelper.java b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sqm/mutation/internal/temptable/ReactiveTemporaryTableHelper.java index d027235a2..4eaecd04f 100644 --- a/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sqm/mutation/internal/temptable/ReactiveTemporaryTableHelper.java +++ b/hibernate-reactive-core/src/main/java/org/hibernate/reactive/query/sqm/mutation/internal/temptable/ReactiveTemporaryTableHelper.java @@ -68,11 +68,8 @@ public TemporaryTableCreationWork( @Override public CompletionStage reactiveExecute(ReactiveConnection connection) { - final JdbcServices jdbcServices = sessionFactory.getJdbcServices(); - try { final String creationCommand = exporter.getSqlCreateCommand( temporaryTable ); - logStatement( creationCommand, jdbcServices ); return connection.executeUnprepared( creationCommand ) .handle( (integer, throwable) -> { @@ -116,11 +113,8 @@ public TemporaryTableDropWork( @Override public CompletionStage reactiveExecute(ReactiveConnection connection) { - final JdbcServices jdbcServices = sessionFactory.getJdbcServices(); - try { final String dropCommand = exporter.getSqlDropCommand( temporaryTable ); - logStatement( dropCommand, jdbcServices ); return connection.update( dropCommand ) .handle( (integer, throwable) -> { diff --git a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/BaseReactiveTest.java b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/BaseReactiveTest.java index 6cd705f2b..1a5f51e3b 100644 --- a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/BaseReactiveTest.java +++ b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/BaseReactiveTest.java @@ -18,6 +18,8 @@ import org.hibernate.dialect.Dialect; import org.hibernate.engine.jdbc.connections.spi.ConnectionProvider; import org.hibernate.engine.spi.SessionFactoryImplementor; +import org.hibernate.query.sqm.mutation.internal.temptable.GlobalTemporaryTableStrategy; +import org.hibernate.query.sqm.mutation.internal.temptable.PersistentTableStrategy; import org.hibernate.reactive.containers.DatabaseConfiguration; import org.hibernate.reactive.containers.DatabaseConfiguration.DBType; import org.hibernate.reactive.mutiny.Mutiny; @@ -105,6 +107,8 @@ public static void setDefaultProperties(Configuration configuration) { configuration.setProperty( Settings.SHOW_SQL, System.getProperty( Settings.SHOW_SQL, "false" ) ); configuration.setProperty( Settings.FORMAT_SQL, System.getProperty( Settings.FORMAT_SQL, "false" ) ); configuration.setProperty( Settings.HIGHLIGHT_SQL, System.getProperty( Settings.HIGHLIGHT_SQL, "true" ) ); + configuration.setProperty( PersistentTableStrategy.DROP_ID_TABLES, "true" ); + configuration.setProperty( GlobalTemporaryTableStrategy.DROP_ID_TABLES, "true" ); } public static final SessionFactoryManager factoryManager = new SessionFactoryManager(); diff --git a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/schema/TemporaryIdTableStrategyTest.java b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/schema/TemporaryIdTableStrategyTest.java new file mode 100644 index 000000000..15ed24341 --- /dev/null +++ b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/schema/TemporaryIdTableStrategyTest.java @@ -0,0 +1,235 @@ +/* Hibernate, Relational Persistence for Idiomatic Java + * + * SPDX-License-Identifier: Apache-2.0 + * Copyright: Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.reactive.schema; + +import java.util.Collection; +import java.util.Set; +import java.util.concurrent.CompletionStage; +import java.util.stream.Stream; + +import org.hibernate.boot.registry.StandardServiceRegistryBuilder; +import org.hibernate.cfg.Configuration; +import org.hibernate.dialect.Dialect; +import org.hibernate.dialect.temptable.TemporaryTable; +import org.hibernate.query.sqm.mutation.internal.temptable.GlobalTemporaryTableStrategy; +import org.hibernate.query.sqm.mutation.internal.temptable.PersistentTableStrategy; +import org.hibernate.reactive.BaseReactiveTest; +import org.hibernate.reactive.annotations.EnabledFor; +import org.hibernate.reactive.provider.Settings; +import org.hibernate.reactive.testing.SqlStatementTracker; +import org.hibernate.reactive.util.impl.CompletionStages; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import io.vertx.junit5.Timeout; +import io.vertx.junit5.VertxTestContext; +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.Inheritance; +import jakarta.persistence.InheritanceType; + +import static java.util.concurrent.TimeUnit.MINUTES; +import static org.assertj.core.api.Assertions.assertThat; +import static org.hibernate.reactive.containers.DatabaseConfiguration.DBType.COCKROACHDB; +import static org.hibernate.reactive.containers.DatabaseConfiguration.DBType.ORACLE; +import static org.hibernate.reactive.util.impl.CompletionStages.voidFuture; +import static org.junit.jupiter.params.provider.Arguments.arguments; + +/** + * Test enabling and disabling of strategies for the creation of temporary tables to store ids. + * + * @see GlobalTemporaryTableStrategy + * @see PersistentTableStrategy + */ +@Timeout(value = 10, timeUnit = MINUTES) +public class TemporaryIdTableStrategyTest extends BaseReactiveTest { + private static SqlStatementTracker sqlStatementTracker; + + final static Dialect[] dialect = new Dialect[1]; + + @Override + protected Collection> annotatedEntities() { + return Set.of( Engineer.class, Doctor.class, Person.class ); + } + + public static Stream settings() { + return Stream.of( + arguments( true, 1, true, 1 ), + arguments( true, 1, false, 0 ), + // I'm assuming Hibernate won't drop the id tables if they haven't been created + arguments( false, 0, true, 0 ), + arguments( false, 0, false, 0 ) + ); + } + + @Override + protected Configuration constructConfiguration() { + Configuration configuration = super.constructConfiguration(); + configuration.setProperty( Settings.HBM2DDL_AUTO, "create" ); + // Collect all the logs, we are going to filter them later + sqlStatementTracker = new SqlStatementTracker( s -> true, configuration.getProperties() ); + return configuration; + } + + @Override + public CompletionStage deleteEntities(Class... entities) { + // Deleting entities is not necessary for this test + return voidFuture(); + } + + @Override + public void before(VertxTestContext context) { + // We need to start and close our own session factories for the test + } + + @AfterEach + @Override + public void after(VertxTestContext context) { + sqlStatementTracker.clear(); + super.after( context ); + } + + @Override + protected void addServices(StandardServiceRegistryBuilder builder) { + if ( sqlStatementTracker != null ) { + sqlStatementTracker.registerService( builder ); + } + } + + @ParameterizedTest(name = "Global Temporary tables - create: {0}, drop: {2}") + @MethodSource("settings") + @EnabledFor(value = ORACLE, reason = "It uses GlobalTemporaryTableStrategy by default") + public void testGlobalTemporaryTablesStrategy( + boolean enableCreateIdTables, + // Expected number of temporary tables created + int expectedTempTablesCreated, + boolean enableDropIdTables, + // Expected number of temporary tables dropped + int expectedTempTablesDropped, + VertxTestContext context) { + Configuration configuration = constructConfiguration(); + configuration.setProperty( GlobalTemporaryTableStrategy.CREATE_ID_TABLES, enableCreateIdTables ); + configuration.setProperty( GlobalTemporaryTableStrategy.DROP_ID_TABLES, enableDropIdTables ); + + testTemporaryIdTablesCreationAndDropping( configuration, expectedTempTablesCreated, expectedTempTablesDropped, context ); + } + + @ParameterizedTest(name = "Persistent tables - create: {0}, drop: {2}") + @MethodSource("settings") + @EnabledFor(value = COCKROACHDB, reason = "It uses PersistentTemporaryTableStrategy by default") + public void testPersistentTemporaryTablesStrategy( + boolean enableCreateIdTables, + // Expected number of temporary tables created + int expectedTempTablesCreated, + boolean enableDropIdTables, + // Expected number of temporary tables dropped + int expectedTempTablesDropped, + VertxTestContext context) { + + Configuration configuration = constructConfiguration(); + configuration.setProperty( PersistentTableStrategy.CREATE_ID_TABLES, enableCreateIdTables ); + configuration.setProperty( PersistentTableStrategy.DROP_ID_TABLES, enableDropIdTables ); + + testTemporaryIdTablesCreationAndDropping( configuration, expectedTempTablesCreated, expectedTempTablesDropped, context ); + } + + private void testTemporaryIdTablesCreationAndDropping( + Configuration configure, + int expectedTempTablesCreated, + int expectedTempTablesDropped, + VertxTestContext context) { + test( context, setupSessionFactory( configure ) + .thenAccept( v -> { + dialect[0] = getDialect(); + assertThat( commandsCount( dialect[0].getTemporaryTableCreateCommand() ) ) + .as( "Unexpected number of temporary tables for ids CREATED" ) + .isEqualTo( expectedTempTablesCreated ); + sqlStatementTracker.clear(); + } ) + // to ensure the factory is always closed even in case of exceptions + .handle( CompletionStages::handle ) + .thenCompose( this::closeFactory ) + .thenAccept( v -> assertThat( commandsCount( dialect[0].getTemporaryTableDropCommand() ) ) + .as( "Unexpected number of temporary tables for ids DROPPED" ) + .isEqualTo( expectedTempTablesDropped ) ) + ); + } + + // Always try to close the factory without losing the original error (if there was one) + private CompletionStage closeFactory(CompletionStages.CompletionStageHandler handler) { + return factoryManager.stop() + .handle( CompletionStages::handle ) + .thenCompose( factoryHandler -> handler + .getResultAsCompletionStage() + // When there's already an exception, we don't care about errors closing the factory + .thenCompose( factoryHandler::getResultAsCompletionStage ) ); + } + + private static long commandsCount(String temporaryTableCommand) { + return sqlStatementTracker.getLoggedQueries().stream() + .filter( q -> q.startsWith( temporaryTableCommand ) && q.contains( TemporaryTable.ID_TABLE_PREFIX ) ) + .count(); + } + + @Entity(name = "Person") + @Inheritance(strategy = InheritanceType.JOINED) + public static class Person { + + @Id + @GeneratedValue + private Long id; + + private String name; + + private boolean employed; + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public boolean isEmployed() { + return employed; + } + + public void setEmployed(boolean employed) { + this.employed = employed; + } + } + + @Entity(name = "Doctor") + public static class Doctor extends Person { + } + + @Entity(name = "Engineer") + public static class Engineer extends Person { + + private boolean fellow; + + public boolean isFellow() { + return fellow; + } + + public void setFellow(boolean fellow) { + this.fellow = fellow; + } + } +} diff --git a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/testing/SessionFactoryManager.java b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/testing/SessionFactoryManager.java index 8262b7c22..be4362a02 100644 --- a/hibernate-reactive-core/src/test/java/org/hibernate/reactive/testing/SessionFactoryManager.java +++ b/hibernate-reactive-core/src/test/java/org/hibernate/reactive/testing/SessionFactoryManager.java @@ -5,20 +5,13 @@ */ package org.hibernate.reactive.testing; -import java.util.ArrayList; -import java.util.List; import java.util.concurrent.CompletionStage; import java.util.function.Supplier; import org.hibernate.SessionFactory; import org.hibernate.engine.spi.SessionFactoryImplementor; -import org.hibernate.metamodel.spi.MappingMetamodelImplementor; -import org.hibernate.persister.entity.EntityPersister; -import org.hibernate.query.sqm.mutation.internal.temptable.PersistentTableStrategy; import org.hibernate.reactive.pool.ReactiveConnectionPool; -import org.hibernate.reactive.query.sqm.mutation.internal.temptable.ReactivePersistentTableStrategy; -import static org.hibernate.reactive.util.impl.CompletionStages.loop; import static org.hibernate.reactive.util.impl.CompletionStages.voidFuture; /** @@ -60,24 +53,8 @@ public ReactiveConnectionPool getReactiveConnectionPool() { public CompletionStage stop() { CompletionStage releasedStage = voidFuture(); if ( sessionFactory != null && sessionFactory.isOpen() ) { - SessionFactoryImplementor sessionFactoryImplementor = sessionFactory.unwrap( SessionFactoryImplementor.class ); - MappingMetamodelImplementor mappingMetamodel = sessionFactoryImplementor - .getRuntimeMetamodels() - .getMappingMetamodel(); - final List reactiveStrategies = new ArrayList<>(); - mappingMetamodel.forEachEntityDescriptor( - entityPersister -> addPersistentTableStrategy( reactiveStrategies, entityPersister ) - ); - if ( !reactiveStrategies.isEmpty() ) { - releasedStage = loop( reactiveStrategies, strategy -> { - ( (PersistentTableStrategy) strategy ) - .release( sessionFactory.unwrap( SessionFactoryImplementor.class ), null ); - return strategy.getDropTableActionStage(); - } ); - - releasedStage = releasedStage - .whenComplete( (unused, throwable) -> sessionFactory.close() ); - } + releasedStage = releasedStage + .whenComplete( (unused, throwable) -> sessionFactory.close() ); } return releasedStage .thenCompose( unused -> { @@ -93,13 +70,4 @@ public CompletionStage stop() { return closeFuture; } ); } - - private void addPersistentTableStrategy(List reactiveStrategies, EntityPersister entityPersister) { - if ( entityPersister.getSqmMultiTableMutationStrategy() instanceof ReactivePersistentTableStrategy ) { - reactiveStrategies.add( (ReactivePersistentTableStrategy) entityPersister.getSqmMultiTableMutationStrategy() ); - } - if ( entityPersister.getSqmMultiTableInsertStrategy() instanceof ReactivePersistentTableStrategy ) { - reactiveStrategies.add( (ReactivePersistentTableStrategy) entityPersister.getSqmMultiTableInsertStrategy() ); - } - } }