This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-dbcp.git
The following commit(s) were added to refs/heads/master by this push: new 7337960 Use try-with-resources to make tests clean up after themselves. 7337960 is described below commit 7337960cd67e3222855ed2b9ed24d591df6a608e Author: Gary Gregory <gardgreg...@gmail.com> AuthorDate: Mon Apr 1 13:45:06 2019 -0400 Use try-with-resources to make tests clean up after themselves. --- .../apache/commons/dbcp2/TestBasicDataSource.java | 60 ++++++++------ .../org/apache/commons/dbcp2/TestPStmtPooling.java | 6 +- .../dbcp2/TestPStmtPoolingBasicDataSource.java | 5 +- .../java/org/apache/commons/dbcp2/TestUtils.java | 4 +- .../dbcp2/cpdsadapter/TestDriverAdapterCPDS.java | 96 +++++++++++----------- .../dbcp2/managed/TestBasicManagedDataSource.java | 42 +++++----- .../dbcp2/managed/TestConnectionWithNarayana.java | 20 ++--- .../dbcp2/managed/TestSynchronizationOrder.java | 62 +++++++------- .../dbcp2/managed/TestTransactionContext.java | 27 +++--- 9 files changed, 168 insertions(+), 154 deletions(-) diff --git a/src/test/java/org/apache/commons/dbcp2/TestBasicDataSource.java b/src/test/java/org/apache/commons/dbcp2/TestBasicDataSource.java index d5f76bb..aee9a65 100644 --- a/src/test/java/org/apache/commons/dbcp2/TestBasicDataSource.java +++ b/src/test/java/org/apache/commons/dbcp2/TestBasicDataSource.java @@ -196,11 +196,11 @@ public class TestBasicDataSource extends TestConnectionPool { @Test public void testTransactionIsolationBehavior() throws Exception { - final Connection conn = getConnection(); - assertNotNull(conn); - assertEquals(Connection.TRANSACTION_READ_COMMITTED, conn.getTransactionIsolation()); - conn.setTransactionIsolation(Connection.TRANSACTION_READ_UNCOMMITTED); - conn.close(); + try (final Connection conn = getConnection()) { + assertNotNull(conn); + assertEquals(Connection.TRANSACTION_READ_COMMITTED, conn.getTransactionIsolation()); + conn.setTransactionIsolation(Connection.TRANSACTION_READ_UNCOMMITTED); + } final Connection conn2 = getConnection(); assertEquals(Connection.TRANSACTION_READ_COMMITTED, conn2.getTransactionIsolation()); @@ -239,14 +239,15 @@ public class TestBasicDataSource extends TestConnectionPool { ds.setAccessToUnderlyingConnectionAllowed(true); assertTrue(ds.isAccessToUnderlyingConnectionAllowed()); - final Connection conn = getConnection(); - Connection dconn = ((DelegatingConnection<?>) conn).getDelegate(); - assertNotNull(dconn); + try (final Connection conn = getConnection()) { + Connection dconn = ((DelegatingConnection<?>) conn).getDelegate(); + assertNotNull(dconn); - dconn = ((DelegatingConnection<?>) conn).getInnermostDelegate(); - assertNotNull(dconn); + dconn = ((DelegatingConnection<?>) conn).getInnermostDelegate(); + assertNotNull(dconn); - assertTrue(dconn instanceof TesterConnection); + assertTrue(dconn instanceof TesterConnection); + } } @Test @@ -290,8 +291,9 @@ public class TestBasicDataSource extends TestConnectionPool { ds.setTestOnBorrow(true); ds.setTestOnReturn(true); ds.setValidationQueryTimeout(0); - final Connection con = ds.getConnection(); - con.close(); + try (final Connection con = ds.getConnection()) { + // close right away. + } } @Test @@ -299,8 +301,9 @@ public class TestBasicDataSource extends TestConnectionPool { ds.setTestOnBorrow(true); ds.setTestOnReturn(true); ds.setValidationQueryTimeout(-1); - final Connection con = ds.getConnection(); - con.close(); + try (final Connection con = ds.getConnection()) { + // close right away. + } } @Test @@ -308,8 +311,9 @@ public class TestBasicDataSource extends TestConnectionPool { ds.setTestOnBorrow(true); ds.setTestOnReturn(true); ds.setValidationQueryTimeout(100); // Works for TesterStatement - final Connection con = ds.getConnection(); - con.close(); + try (final Connection con = ds.getConnection()) { + // close right away. + } } @Test @@ -553,16 +557,18 @@ public class TestBasicDataSource extends TestConnectionPool { @Test public void testInvalidateConnection() throws Exception { - ds.setMaxTotal(2); - final Connection conn1 = ds.getConnection(); - final Connection conn2 = ds.getConnection(); - ds.invalidateConnection(conn1); - assertTrue(conn1.isClosed()); - assertEquals(1, ds.getNumActive()); - assertEquals(0, ds.getNumIdle()); - final Connection conn3 = ds.getConnection(); - conn2.close(); - conn3.close(); + ds.setMaxTotal(2); + try (final Connection conn1 = ds.getConnection()) { + try (final Connection conn2 = ds.getConnection()) { + ds.invalidateConnection(conn1); + assertTrue(conn1.isClosed()); + assertEquals(1, ds.getNumActive()); + assertEquals(0, ds.getNumIdle()); + try (final Connection conn3 = ds.getConnection()) { + conn2.close(); + } + } + } } /** diff --git a/src/test/java/org/apache/commons/dbcp2/TestPStmtPooling.java b/src/test/java/org/apache/commons/dbcp2/TestPStmtPooling.java index c286f07..6c965e5 100644 --- a/src/test/java/org/apache/commons/dbcp2/TestPStmtPooling.java +++ b/src/test/java/org/apache/commons/dbcp2/TestPStmtPooling.java @@ -44,7 +44,7 @@ public class TestPStmtPooling { @Test public void testStmtPool() throws Exception { - final DataSource ds = createPDS(); + final DataSource ds = createPoolingDataSource(); try (Connection conn = ds.getConnection()) { final Statement stmt1 = conn.prepareStatement("select 1 from dual"); final Statement ustmt1 = ((DelegatingStatement) stmt1).getInnermostDelegate(); @@ -63,7 +63,7 @@ public class TestPStmtPooling { */ @Test public void testMultipleClose() throws Exception { - final DataSource ds = createPDS(); + final DataSource ds = createPoolingDataSource(); PreparedStatement stmt1 = null; final Connection conn = ds.getConnection(); stmt1 = conn.prepareStatement("select 1 from dual"); @@ -101,7 +101,7 @@ public class TestPStmtPooling { } - private DataSource createPDS() throws Exception { + private DataSource createPoolingDataSource() throws Exception { DriverManager.registerDriver(new TesterDriver()); final ConnectionFactory connFactory = new DriverManagerConnectionFactory( "jdbc:apache:commons:testdriver","u1","p1"); diff --git a/src/test/java/org/apache/commons/dbcp2/TestPStmtPoolingBasicDataSource.java b/src/test/java/org/apache/commons/dbcp2/TestPStmtPoolingBasicDataSource.java index 592fcff..734ebcc 100644 --- a/src/test/java/org/apache/commons/dbcp2/TestPStmtPoolingBasicDataSource.java +++ b/src/test/java/org/apache/commons/dbcp2/TestPStmtPoolingBasicDataSource.java @@ -77,8 +77,9 @@ public class TestPStmtPoolingBasicDataSource extends TestBasicDataSource { // normal reuse of statement stmt1.close(); - final PreparedStatement stmt4 = conn.prepareStatement("select 'a' from dual"); - assertNotNull(stmt4); + try (final PreparedStatement stmt4 = conn.prepareStatement("select 'a' from dual")) { + assertNotNull(stmt4); + } } /** diff --git a/src/test/java/org/apache/commons/dbcp2/TestUtils.java b/src/test/java/org/apache/commons/dbcp2/TestUtils.java index 3332f5a..c2cb80c 100644 --- a/src/test/java/org/apache/commons/dbcp2/TestUtils.java +++ b/src/test/java/org/apache/commons/dbcp2/TestUtils.java @@ -17,14 +17,12 @@ package org.apache.commons.dbcp2; -import java.sql.Connection; - import org.junit.jupiter.api.Test; public class TestUtils { @Test public void testClassLoads() { - Utils.closeQuietly((Connection) null); + Utils.closeQuietly((AutoCloseable) null); } } diff --git a/src/test/java/org/apache/commons/dbcp2/cpdsadapter/TestDriverAdapterCPDS.java b/src/test/java/org/apache/commons/dbcp2/cpdsadapter/TestDriverAdapterCPDS.java index ae0bfde..35d12c7 100644 --- a/src/test/java/org/apache/commons/dbcp2/cpdsadapter/TestDriverAdapterCPDS.java +++ b/src/test/java/org/apache/commons/dbcp2/cpdsadapter/TestDriverAdapterCPDS.java @@ -95,30 +95,30 @@ public class TestDriverAdapterCPDS { @Test public void testSimple() throws Exception { - final Connection conn = pcds.getPooledConnection().getConnection(); - assertNotNull(conn); - final PreparedStatement stmt = conn.prepareStatement("select * from dual"); - assertNotNull(stmt); - final ResultSet rset = stmt.executeQuery(); - assertNotNull(rset); - assertTrue(rset.next()); - rset.close(); - stmt.close(); - conn.close(); + try (final Connection conn = pcds.getPooledConnection().getConnection()) { + assertNotNull(conn); + try (final PreparedStatement stmt = conn.prepareStatement("select * from dual")) { + assertNotNull(stmt); + try (final ResultSet rset = stmt.executeQuery()) { + assertNotNull(rset); + assertTrue(rset.next()); + } + } + } } @Test public void testSimpleWithUsername() throws Exception { - final Connection conn = pcds.getPooledConnection("u1", "p1").getConnection(); - assertNotNull(conn); - final PreparedStatement stmt = conn.prepareStatement("select * from dual"); - assertNotNull(stmt); - final ResultSet rset = stmt.executeQuery(); - assertNotNull(rset); - assertTrue(rset.next()); - rset.close(); - stmt.close(); - conn.close(); + try (final Connection conn = pcds.getPooledConnection("u1", "p1").getConnection()) { + assertNotNull(conn); + try (final PreparedStatement stmt = conn.prepareStatement("select * from dual")) { + assertNotNull(stmt); + try (final ResultSet rset = stmt.executeQuery()) { + assertNotNull(rset); + assertTrue(rset.next()); + } + } + } } @Test @@ -231,12 +231,13 @@ public class TestDriverAdapterCPDS { */ @Test public void testNullValidationQuery() throws Exception { - final SharedPoolDataSource spds = new SharedPoolDataSource(); - spds.setConnectionPoolDataSource(pcds); - spds.setDefaultTestOnBorrow(true); - final Connection c = spds.getConnection(); - c.close(); - spds.close(); + try (final SharedPoolDataSource spds = new SharedPoolDataSource()) { + spds.setConnectionPoolDataSource(pcds); + spds.setDefaultTestOnBorrow(true); + try (final Connection c = spds.getConnection()) { + // close right away + } + } } // https://issues.apache.org/jira/browse/DBCP-376 @@ -248,28 +249,29 @@ public class TestDriverAdapterCPDS { pcds.setMaxPreparedStatements(-1); pcds.setAccessToUnderlyingConnectionAllowed(true); - final SharedPoolDataSource spds = new SharedPoolDataSource(); - spds.setConnectionPoolDataSource(pcds); - spds.setMaxTotal(threads.length + 10); - spds.setDefaultMaxWaitMillis(-1); - spds.setDefaultMaxIdle(10); - spds.setDefaultAutoCommit(Boolean.FALSE); - - spds.setValidationQuery("SELECT 1"); - spds.setDefaultTimeBetweenEvictionRunsMillis(10000); - spds.setDefaultNumTestsPerEvictionRun(-1); - spds.setDefaultTestWhileIdle(true); - spds.setDefaultTestOnBorrow(true); - spds.setDefaultTestOnReturn(false); - - for (int i = 0; i < threads.length; i++) { - threads[i] = new ThreadDbcp367(spds); - threads[i].start(); - } + try (final SharedPoolDataSource spds = new SharedPoolDataSource()) { + spds.setConnectionPoolDataSource(pcds); + spds.setMaxTotal(threads.length + 10); + spds.setDefaultMaxWaitMillis(-1); + spds.setDefaultMaxIdle(10); + spds.setDefaultAutoCommit(Boolean.FALSE); + + spds.setValidationQuery("SELECT 1"); + spds.setDefaultTimeBetweenEvictionRunsMillis(10000); + spds.setDefaultNumTestsPerEvictionRun(-1); + spds.setDefaultTestWhileIdle(true); + spds.setDefaultTestOnBorrow(true); + spds.setDefaultTestOnReturn(false); + + for (int i = 0; i < threads.length; i++) { + threads[i] = new ThreadDbcp367(spds); + threads[i].start(); + } - for (int i = 0; i < threads.length; i++) { - threads[i].join(); - Assertions.assertFalse(threads[i].isFailed(), "Thread " + i + " has failed"); + for (int i = 0; i < threads.length; i++) { + threads[i].join(); + Assertions.assertFalse(threads[i].isFailed(), "Thread " + i + " has failed"); + } } } diff --git a/src/test/java/org/apache/commons/dbcp2/managed/TestBasicManagedDataSource.java b/src/test/java/org/apache/commons/dbcp2/managed/TestBasicManagedDataSource.java index ee36fa6..d4576ef 100644 --- a/src/test/java/org/apache/commons/dbcp2/managed/TestBasicManagedDataSource.java +++ b/src/test/java/org/apache/commons/dbcp2/managed/TestBasicManagedDataSource.java @@ -59,27 +59,27 @@ public class TestBasicManagedDataSource extends TestBasicDataSource { */ @Test public void testReallyClose() throws Exception { - final BasicManagedDataSource basicManagedDataSource = new BasicManagedDataSource(); - basicManagedDataSource.setTransactionManager(new TransactionManagerImpl()); - basicManagedDataSource.setDriverClassName("org.apache.commons.dbcp2.TesterDriver"); - basicManagedDataSource.setUrl("jdbc:apache:commons:testdriver"); - basicManagedDataSource.setUsername("userName"); - basicManagedDataSource.setPassword("password"); - basicManagedDataSource.setMaxIdle(1); - // Create two connections - final ManagedConnection<?> conn = (ManagedConnection<?>) basicManagedDataSource.getConnection(); - assertNotNull(basicManagedDataSource.getTransactionRegistry().getXAResource(conn)); - final ManagedConnection<?> conn2 = (ManagedConnection<?>) basicManagedDataSource.getConnection(); - conn2.close(); // Return one connection to the pool - conn.close(); // No room at the inn - this will trigger reallyClose(), which should unregister - try { - basicManagedDataSource.getTransactionRegistry().getXAResource(conn); - fail("Expecting SQLException - XAResources orphaned"); - } catch (final SQLException ex) { - // expected + try (final BasicManagedDataSource basicManagedDataSource = new BasicManagedDataSource()) { + basicManagedDataSource.setTransactionManager(new TransactionManagerImpl()); + basicManagedDataSource.setDriverClassName("org.apache.commons.dbcp2.TesterDriver"); + basicManagedDataSource.setUrl("jdbc:apache:commons:testdriver"); + basicManagedDataSource.setUsername("userName"); + basicManagedDataSource.setPassword("password"); + basicManagedDataSource.setMaxIdle(1); + // Create two connections + final ManagedConnection<?> conn = (ManagedConnection<?>) basicManagedDataSource.getConnection(); + assertNotNull(basicManagedDataSource.getTransactionRegistry().getXAResource(conn)); + final ManagedConnection<?> conn2 = (ManagedConnection<?>) basicManagedDataSource.getConnection(); + conn2.close(); // Return one connection to the pool + conn.close(); // No room at the inn - this will trigger reallyClose(), which should unregister + try { + basicManagedDataSource.getTransactionRegistry().getXAResource(conn); + fail("Expecting SQLException - XAResources orphaned"); + } catch (final SQLException ex) { + // expected + } + conn2.close(); } - conn2.close(); - basicManagedDataSource.close(); } @Test @@ -107,7 +107,7 @@ public class TestBasicManagedDataSource extends TestBasicDataSource { } @Test - public void testSetDriverName() throws SQLException, XAException { + public void testSetDriverName() throws SQLException { try (final BasicManagedDataSource basicManagedDataSource = new BasicManagedDataSource()) { basicManagedDataSource.setDriverClassName("adams"); assertEquals("adams", basicManagedDataSource.getDriverClassName()); diff --git a/src/test/java/org/apache/commons/dbcp2/managed/TestConnectionWithNarayana.java b/src/test/java/org/apache/commons/dbcp2/managed/TestConnectionWithNarayana.java index d681c54..248efcb 100644 --- a/src/test/java/org/apache/commons/dbcp2/managed/TestConnectionWithNarayana.java +++ b/src/test/java/org/apache/commons/dbcp2/managed/TestConnectionWithNarayana.java @@ -85,11 +85,11 @@ public class TestConnectionWithNarayana { mds.setLogExpiredConnections(true); mds.setLifo(false); - final Connection conn = mds.getConnection(); - final PreparedStatement ps = conn.prepareStatement(CREATE_STMT); - ps.execute(); - ps.close(); - conn.close(); + try (final Connection conn = mds.getConnection()) { + try (final PreparedStatement ps = conn.prepareStatement(CREATE_STMT)) { + ps.execute(); + } + } } @Test @@ -224,11 +224,11 @@ public class TestConnectionWithNarayana { @AfterEach public void tearDown() throws Exception { - final Connection conn = mds.getConnection(); - final PreparedStatement ps = conn.prepareStatement(DROP_STMT); - ps.execute(); - ps.close(); - conn.close(); + try (final Connection conn = mds.getConnection()) { + try (final PreparedStatement ps = conn.prepareStatement(DROP_STMT)) { + ps.execute(); + } + } if (mds != null) { mds.close(); } diff --git a/src/test/java/org/apache/commons/dbcp2/managed/TestSynchronizationOrder.java b/src/test/java/org/apache/commons/dbcp2/managed/TestSynchronizationOrder.java index 6f610f8..e8b9b87 100644 --- a/src/test/java/org/apache/commons/dbcp2/managed/TestSynchronizationOrder.java +++ b/src/test/java/org/apache/commons/dbcp2/managed/TestSynchronizationOrder.java @@ -78,45 +78,51 @@ public class TestSynchronizationOrder { pool.setMaxWaitMillis(1000); // finally create the datasource - final ManagedDataSource ds = new ManagedDataSource<>(pool, xaConnectionFactory.getTransactionRegistry()); - ds.setAccessToUnderlyingConnectionAllowed(true); - - transactionManager.begin(); - final DelegatingConnection<?> connectionA = (DelegatingConnection<?>) ds.getConnection(); - connectionA.close(); - transactionManager.commit(); - assertTrue(transactionManagerRegistered); - assertFalse(transactionSynchronizationRegistryRegistered); + try (final ManagedDataSource ds = new ManagedDataSource<>(pool, + xaConnectionFactory.getTransactionRegistry())) { + ds.setAccessToUnderlyingConnectionAllowed(true); + + transactionManager.begin(); + try (final DelegatingConnection<?> connectionA = (DelegatingConnection<?>) ds.getConnection()) { + // close right away. + } + transactionManager.commit(); + assertTrue(transactionManagerRegistered); + assertFalse(transactionSynchronizationRegistryRegistered); + } } } @Test public void testInterposedSynchronization() throws Exception { - final DataSourceXAConnectionFactory xaConnectionFactory = new DataSourceXAConnectionFactory(transactionManager, xads, transactionSynchronizationRegistry); + final DataSourceXAConnectionFactory xaConnectionFactory = new DataSourceXAConnectionFactory(transactionManager, + xads, transactionSynchronizationRegistry); - final PoolableConnectionFactory factory = - new PoolableConnectionFactory(xaConnectionFactory, null); + final PoolableConnectionFactory factory = new PoolableConnectionFactory(xaConnectionFactory, null); factory.setValidationQuery("SELECT DUMMY FROM DUAL"); factory.setDefaultReadOnly(Boolean.TRUE); factory.setDefaultAutoCommit(Boolean.TRUE); // create the pool - final GenericObjectPool pool = new GenericObjectPool<>(factory); - factory.setPool(pool); - pool.setMaxTotal(10); - pool.setMaxWaitMillis(1000); - - // finally create the datasource - final ManagedDataSource ds = new ManagedDataSource<>(pool, xaConnectionFactory.getTransactionRegistry()); - ds.setAccessToUnderlyingConnectionAllowed(true); - - - transactionManager.begin(); - final DelegatingConnection<?> connectionA = (DelegatingConnection<?>) ds.getConnection(); - connectionA.close(); - transactionManager.commit(); - assertFalse(transactionManagerRegistered); - assertTrue(transactionSynchronizationRegistryRegistered); + try (final GenericObjectPool pool = new GenericObjectPool<>(factory)) { + factory.setPool(pool); + pool.setMaxTotal(10); + pool.setMaxWaitMillis(1000); + + // finally create the datasource + try (final ManagedDataSource ds = new ManagedDataSource<>(pool, + xaConnectionFactory.getTransactionRegistry())) { + ds.setAccessToUnderlyingConnectionAllowed(true); + + transactionManager.begin(); + try (final DelegatingConnection<?> connectionA = (DelegatingConnection<?>) ds.getConnection()) { + // Close right away. + } + transactionManager.commit(); + assertFalse(transactionManagerRegistered); + assertTrue(transactionSynchronizationRegistryRegistered); + } + } } @AfterEach diff --git a/src/test/java/org/apache/commons/dbcp2/managed/TestTransactionContext.java b/src/test/java/org/apache/commons/dbcp2/managed/TestTransactionContext.java index a745155..35a03cb 100644 --- a/src/test/java/org/apache/commons/dbcp2/managed/TestTransactionContext.java +++ b/src/test/java/org/apache/commons/dbcp2/managed/TestTransactionContext.java @@ -36,19 +36,20 @@ public class TestTransactionContext { */ @Test public void testSetSharedConnectionEnlistFailure() throws Exception { - final BasicManagedDataSource basicManagedDataSource = new BasicManagedDataSource(); - basicManagedDataSource.setTransactionManager(new TransactionManagerImpl()); - basicManagedDataSource.setDriverClassName("org.apache.commons.dbcp2.TesterDriver"); - basicManagedDataSource.setUrl("jdbc:apache:commons:testdriver"); - basicManagedDataSource.setUsername("userName"); - basicManagedDataSource.setPassword("password"); - basicManagedDataSource.setMaxIdle(1); - final ManagedConnection<?> conn = (ManagedConnection<?>) basicManagedDataSource.getConnection(); - final UncooperativeTransaction transaction = new UncooperativeTransaction(); - final TransactionContext transactionContext = - new TransactionContext(basicManagedDataSource.getTransactionRegistry(), transaction); - assertThrows(SQLException.class, () -> transactionContext.setSharedConnection(conn)); - basicManagedDataSource.close(); + try (final BasicManagedDataSource basicManagedDataSource = new BasicManagedDataSource()) { + basicManagedDataSource.setTransactionManager(new TransactionManagerImpl()); + basicManagedDataSource.setDriverClassName("org.apache.commons.dbcp2.TesterDriver"); + basicManagedDataSource.setUrl("jdbc:apache:commons:testdriver"); + basicManagedDataSource.setUsername("userName"); + basicManagedDataSource.setPassword("password"); + basicManagedDataSource.setMaxIdle(1); + try (final ManagedConnection<?> conn = (ManagedConnection<?>) basicManagedDataSource.getConnection()) { + final UncooperativeTransaction transaction = new UncooperativeTransaction(); + final TransactionContext transactionContext = new TransactionContext( + basicManagedDataSource.getTransactionRegistry(), transaction); + assertThrows(SQLException.class, () -> transactionContext.setSharedConnection(conn)); + } + } } /**