HHH-14326 Test JDBC resources are released before closing the connection

Signed-off-by: Yoann Rodière <yoann@hibernate.org>

hibernate-core/src/test/java/org/hibernate/test/connections/BeforeCompletionReleaseTest.java

@@ -7,33 +7,45 @@
 
 package org.hibernate.test.connections;
 
-import org.hibernate.cfg.AvailableSettings;
+import java.sql.Connection;
-import org.hibernate.dialect.H2Dialect;
+import java.sql.SQLException;
-import org.hibernate.engine.jdbc.connections.internal.UserSuppliedConnectionProviderImpl;
+import java.sql.Statement;
-import org.hibernate.engine.jdbc.connections.spi.ConnectionProvider;
+import java.util.Map;
-import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase;
-import org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode;
-import org.hibernate.testing.RequiresDialect;
-import org.hibernate.testing.env.ConnectionProviderBuilder;
-import org.hibernate.testing.jta.TestingJtaBootstrap;
-import org.hibernate.testing.jta.TestingJtaPlatformImpl;
-import org.hibernate.testing.transaction.TransactionUtil;
-import org.junit.Test;
-
 import javax.persistence.Entity;
 import javax.persistence.Id;
 import javax.persistence.Table;
 import javax.transaction.RollbackException;
 import javax.transaction.SystemException;
-import javax.transaction.Transaction;
+import javax.transaction.xa.XAException;
 import javax.transaction.xa.XAResource;
-import javax.transaction.xa.Xid;
-import java.sql.Connection;
-import java.sql.SQLException;
-import java.util.Map;
 
-import static org.junit.Assert.assertTrue;
+import org.hibernate.cfg.AvailableSettings;
-import static org.junit.Assert.fail;
+import org.hibernate.dialect.H2Dialect;
+import org.hibernate.engine.jdbc.connections.internal.UserSuppliedConnectionProviderImpl;
+import org.hibernate.engine.jdbc.connections.spi.ConnectionProvider;
+import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase;
+import org.hibernate.resource.jdbc.spi.LogicalConnectionImplementor;
+import org.hibernate.resource.jdbc.spi.PhysicalConnectionHandlingMode;
+import org.hibernate.testing.RequiresDialect;
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.env.ConnectionProviderBuilder;
+import org.hibernate.testing.jta.TestingJtaBootstrap;
+import org.hibernate.testing.jta.TestingJtaPlatformImpl;
+import org.hibernate.testing.transaction.TransactionUtil2;
+import org.junit.Rule;
+import org.junit.Test;
+
+import org.mockito.InOrder;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnit;
+import org.mockito.junit.MockitoRule;
+import org.mockito.quality.Strictness;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyBoolean;
+import static org.mockito.Mockito.inOrder;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
 
 /**
  * @author Luis Barreiro
@@ -41,6 +53,9 @@ import static org.junit.Assert.fail;
 @RequiresDialect( H2Dialect.class )
 public class BeforeCompletionReleaseTest extends BaseEntityManagerFunctionalTestCase {
 
+    @Rule
+    public MockitoRule mockito = MockitoJUnit.rule().strictness( Strictness.STRICT_STUBS );
+
     @Override
     protected Map getConfig() {
         Map config = super.getConfig();
@@ -56,12 +71,40 @@ public class BeforeCompletionReleaseTest extends BaseEntityManagerFunctionalTest
     }
 
     @Test
-    public void testConnectionAcquisitionCount() {
+    @TestForIssue(jiraKey = {"HHH-13976", "HHH-14326"})
-        TransactionUtil.doInJPA( this::entityManagerFactory, entityManager -> {
+    public void testResourcesReleasedThenConnectionClosedThenCommit() throws SQLException, XAException {
+        XAResource transactionSpy = mock( XAResource.class );
+        Connection[] connectionSpies = new Connection[1];
+        Statement statementMock = Mockito.mock( Statement.class );
+
+        TransactionUtil2.inTransaction( entityManagerFactory(), session -> {
+            spyOnTransaction( transactionSpy );
+
             Thing thing = new Thing();
             thing.setId( 1 );
-            entityManager.persist( thing );
+            session.persist( thing );
+
+            LogicalConnectionImplementor logicalConnection = session.getJdbcCoordinator().getLogicalConnection();
+            logicalConnection.getResourceRegistry().register( statementMock, true );
+            connectionSpies[0] = logicalConnection.getPhysicalConnection();
         } );
+
+        Connection connectionSpy = connectionSpies[0];
+
+        // Must close the resources, then the connection, then commit
+        InOrder inOrder = inOrder( statementMock, connectionSpy, transactionSpy );
+        inOrder.verify( statementMock ).close();
+        inOrder.verify( connectionSpy ).close();
+        inOrder.verify( transactionSpy ).commit( any(), anyBoolean() );
+    }
+
+    private void spyOnTransaction(XAResource xaResource) {
+        try {
+            TestingJtaPlatformImpl.transactionManager().getTransaction().enlistResource( xaResource );
+        }
+        catch (RollbackException | SystemException e) {
+            throw new IllegalStateException( e );
+        }
     }
 
     // --- //
@@ -94,63 +137,7 @@ public class BeforeCompletionReleaseTest extends BaseEntityManagerFunctionalTest
 
         @Override
         public Connection getConnection() throws SQLException {
-            Connection connection = dataSource.getConnection();
+            return spy( dataSource.getConnection() );
-
-            try {
-                Transaction tx = TestingJtaPlatformImpl.transactionManager().getTransaction();
-                if ( tx != null) {
-                    tx.enlistResource( new XAResource() {
-
-                        @Override public void commit(Xid xid, boolean onePhase) {
-                            try {
-                                assertTrue( "Connection should be closed prior to commit", connection.isClosed() );
-                            } catch ( SQLException e ) {
-                                fail( "Unexpected SQLException: " + e.getMessage() );
-                            }
-                        }
-
-                        @Override public void end(Xid xid, int flags) {
-                        }
-
-                        @Override public void forget(Xid xid)  {
-                        }
-
-                        @Override public int getTransactionTimeout() {
-                            return 0;
-                        }
-
-                        @Override public boolean isSameRM(XAResource xares) {
-                            return false;
-                        }
-
-                        @Override public int prepare(Xid xid) {
-                            return 0;
-                        }
-
-                        @Override public Xid[] recover(int flag) {
-                            return new Xid[0];
-                        }
-
-                        @Override public void rollback(Xid xid) {
-                            try {
-                                assertTrue( "Connection should be closed prior to rollback", connection.isClosed() );
-                            } catch ( SQLException e ) {
-                                fail( "Unexpected SQLException: " + e.getMessage() );
-                            }
-                        }
-
-                        @Override public boolean setTransactionTimeout(int seconds) {
-                            return false;
-                        }
-
-                        @Override public void start(Xid xid, int flags) {
-                        }
-                    });
-                }
-            } catch ( SystemException | RollbackException e ) {
-                fail( e.getMessage() );
-            }
-            return connection;
         }
 
         @Override

hibernate-testing/src/main/java/org/hibernate/testing/TestForIssue.java

@@ -23,5 +23,5 @@ public @interface TestForIssue {
 	 * The key of a JIRA issue tested.
 	 * @return The jira issue key
 	 */
-	String jiraKey();
+	String[] jiraKey();
 }