Test batching (#1332)

* [#1333] Create the configuration for tests only if needed

The method that creates the configuration was called
before each test even if when not necessary - we create
the factory only once.

This commit makes sure that the configuration is created
only when the factory is created.

* [#1333] Test batching is actually happening

* [#1333] Test batching with persist(Object[])

* [#1333] SqlStatementTracker javadoc

SqlStatementTracker is not thread-safe

Author: DavideD

hibernate-reactive-core/src/test/java/org/hibernate/reactive/BaseReactiveTest.java

@@ -10,6 +10,7 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionStage;
 import java.util.concurrent.TimeUnit;
+import java.util.function.Supplier;
 import javax.persistence.criteria.CriteriaQuery;
 
 import org.hibernate.SessionFactory;
@@ -173,17 +174,30 @@ private <T> CriteriaQuery<T> queryForDelete(Class<T> entityClass) {
 
 	@Before
 	public void before(TestContext context) {
-		test( context, setupSessionFactory( constructConfiguration() ) );
+		test( context, setupSessionFactory( () -> constructConfiguration() ) );
 	}
 
+	/**
+	 * Set up one session factory shared by the tests in the class.
+	 */
 	protected CompletionStage<Void> setupSessionFactory(Configuration configuration) {
+		return setupSessionFactory( () -> configuration );
+	}
+
+	/**
+	 * Set up the session factory but create the configuration only if necessary.
+	 *
+	 * @param confSupplier supplies the configuration for the factory
+	 * @return a {@link CompletionStage} void that succeeds when the factory is ready.
+	 */
+	protected CompletionStage<Void> setupSessionFactory(Supplier<Configuration> confSupplier) {
 		CompletableFuture<Void> future = new CompletableFuture<>();
 		vertxContextRule.vertx()
 				.executeBlocking(
 						// schema generation is a blocking operation and so it causes an
 						// exception when run on the Vert.x event loop. So call it using
 						// Vertx.executeBlocking()
-						promise -> startFactoryManager( promise, configuration ),
+						promise -> startFactoryManager( promise, confSupplier ),
 						event -> {
 							if ( event.succeeded() ) {
 								future.complete( null );
@@ -196,9 +210,9 @@ protected CompletionStage<Void> setupSessionFactory(Configuration configuration)
 		return future;
 	}
 
-	private void startFactoryManager(Promise<Object> p, Configuration configuration ) {
+	private void startFactoryManager(Promise<Object> p, Supplier<Configuration> confSupplier ) {
 		try {
-			factoryManager.start( () -> createHibernateSessionFactory( configuration ) );
+			factoryManager.start( () -> createHibernateSessionFactory( confSupplier.get() ) );
 			p.complete();
 		}
 		catch (Throwable e) {

hibernate-reactive-core/src/test/java/org/hibernate/reactive/BatchingConnectionTest.java

@@ -5,6 +5,7 @@
  */
 package org.hibernate.reactive;
 
+import org.hibernate.boot.registry.StandardServiceRegistryBuilder;
 import org.hibernate.cfg.AvailableSettings;
 import org.hibernate.cfg.Configuration;
 import org.hibernate.reactive.mutiny.impl.MutinySessionImpl;
@@ -13,6 +14,7 @@
 import org.hibernate.reactive.pool.impl.SqlClientConnection;
 import org.hibernate.reactive.stage.impl.StageSessionImpl;
 import org.hibernate.reactive.stage.impl.StageStatelessSessionImpl;
+import org.hibernate.reactive.testing.SqlStatementTracker;
 
 import org.junit.Test;
 
@@ -24,13 +26,59 @@
 
 public class BatchingConnectionTest extends ReactiveSessionTest {
 
+	private static SqlStatementTracker sqlTracker;
+
 	@Override
 	protected Configuration constructConfiguration() {
 		Configuration configuration = super.constructConfiguration();
 		configuration.setProperty( AvailableSettings.STATEMENT_BATCH_SIZE, "5");
+
+		// 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( BatchingConnectionTest::filter, configuration.getProperties() );
 		return configuration;
 	}
 
+	protected void addServices(StandardServiceRegistryBuilder builder) {
+		sqlTracker.registerService( builder );
+	}
+
+	private static boolean filter(String s) {
+		String[] accepted = { "insert ", "update ", "delete " };
+		for ( String valid : accepted ) {
+			if ( s.toLowerCase().startsWith( valid ) ) {
+				return true;
+			}
+		}
+		return false;
+	}
+
+	@Test
+	public void testBatchingWithPersistAll(TestContext context) {
+		test( context, openSession()
+				.thenCompose( s -> s
+						.persist(
+								new GuineaPig( 11, "One" ),
+								new GuineaPig( 22, "Two" ),
+								new GuineaPig( 33, "Three" )
+						)
+						// Auto-flush
+						.thenCompose( v -> s
+								.createQuery( "select name from GuineaPig" )
+								.getResultList()
+								.thenAccept( names -> {
+									assertThat( names ).containsExactlyInAnyOrder( "One", "Two", "Three" );
+									assertThat( sqlTracker.getLoggedQueries() ).hasSize( 1 );
+									// Parameters are different for different dbs, so we cannot do an exact match
+									assertThat( sqlTracker.getLoggedQueries().get( 0 ) )
+											.startsWith( "insert into pig (name, version, id) values " );
+									sqlTracker.clear();
+								} )
+						)
+				)
+		);
+	}
+
 	@Test
 	public void testBatching(TestContext context) {
 		test(
@@ -40,9 +88,17 @@ public void testBatching(TestContext context) {
 								.thenCompose( v -> s.persist( new GuineaPig(11, "One") ) )
 								.thenCompose( v -> s.persist( new GuineaPig(22, "Two") ) )
 								.thenCompose( v -> s.persist( new GuineaPig(33, "Three") ) )
-								.thenCompose( v -> s.createQuery("select count(*) from GuineaPig")
-										.getSingleResult()
-										.thenAccept( count -> context.assertEquals( 3L, count) )
+								// Auto-flush
+								.thenCompose( v -> s.createQuery("select name from GuineaPig")
+										.getResultList()
+										.thenAccept( names -> {
+											assertThat( names ).containsExactlyInAnyOrder( "One", "Two", "Three" );
+											assertThat( sqlTracker.getLoggedQueries() ).hasSize( 1 );
+											// Parameters are different for different dbs, so we cannot do an exact match
+											assertThat( sqlTracker.getLoggedQueries().get( 0 ) )
+													.startsWith( "insert into pig (name, version, id) values " );
+											sqlTracker.clear();
+										} )
 								)
 						)
 						.thenCompose( v -> openSession() )
@@ -51,15 +107,27 @@ public void testBatching(TestContext context) {
 								.thenAccept( list -> list.forEach( pig -> pig.setName("Zero") ) )
 								.thenCompose( v -> s.<Long>createQuery("select count(*) from GuineaPig where name='Zero'")
 										.getSingleResult()
-										.thenAccept( count -> context.assertEquals( 3L, count) )
+										.thenAccept( count -> {
+											context.assertEquals( 3L, count);
+											assertThat( sqlTracker.getLoggedQueries() ).hasSize( 1 );
+											assertThat( sqlTracker.getLoggedQueries().get( 0 ) )
+													.matches( "update pig set name=.+, version=.+ where id=.+ and version=.+" );
+											sqlTracker.clear();
+										} )
 								) )
 						.thenCompose( v -> openSession() )
 						.thenCompose( s -> s.<GuineaPig>createQuery("from GuineaPig")
 								.getResultList()
 								.thenCompose( list -> loop( list, s::remove ) )
 								.thenCompose( v -> s.<Long>createQuery("select count(*) from GuineaPig")
 										.getSingleResult()
-										.thenAccept( count -> context.assertEquals( 0L, count) )
+										.thenAccept( count -> {
+											context.assertEquals( 0L, count);
+											assertThat( sqlTracker.getLoggedQueries() ).hasSize( 1 );
+											assertThat( sqlTracker.getLoggedQueries().get( 0 ) )
+													.matches( "delete from pig where id=.+ and version=.+" );
+											sqlTracker.clear();
+										} )
 								)
 						)
 		);

hibernate-reactive-core/src/test/java/org/hibernate/reactive/testing/SqlStatementTracker.java

@@ -21,9 +21,10 @@
 import org.hibernate.service.spi.ServiceRegistryImplementor;
 
 /**
- * Track sql queries
+ * Track sql queries.
  * <p>
  * Check {@link #registerService(StandardServiceRegistryBuilder)} to register an instance of this class.
+ * It's not thread-safe.
  * </p>
  */
 public class SqlStatementTracker extends SqlStatementLogger {