From d726dcb394e69eec02af2c8c3ac3ebc6679dc19c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yoann=20Rodi=C3=A8re?= Date: Thu, 14 Jan 2021 13:36:51 +0100 Subject: [PATCH] HHH-14326 Test JDBC resources are released before closing the connection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Yoann Rodière --- .../BeforeCompletionReleaseTest.java | 149 ++++++++---------- .../org/hibernate/testing/TestForIssue.java | 2 +- 2 files changed, 69 insertions(+), 82 deletions(-) diff --git a/hibernate-core/src/test/java/org/hibernate/test/connections/BeforeCompletionReleaseTest.java b/hibernate-core/src/test/java/org/hibernate/test/connections/BeforeCompletionReleaseTest.java index f932b558fe..91ed0e6d66 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/connections/BeforeCompletionReleaseTest.java +++ b/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 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.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 java.sql.Connection; +import java.sql.SQLException; +import java.sql.Statement; +import java.util.Map; 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 static org.junit.Assert.fail; +import org.hibernate.cfg.AvailableSettings; +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() { - TransactionUtil.doInJPA( this::entityManagerFactory, entityManager -> { + @TestForIssue(jiraKey = {"HHH-13976", "HHH-14326"}) + 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(); - - 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; + return spy( dataSource.getConnection() ); } @Override diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/TestForIssue.java b/hibernate-testing/src/main/java/org/hibernate/testing/TestForIssue.java index ebe0f4d669..7d76c0e59e 100644 --- a/hibernate-testing/src/main/java/org/hibernate/testing/TestForIssue.java +++ b/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(); }