This is an automated email from the ASF dual-hosted git repository. psteitz pushed a commit to branch pool-update in repository https://gitbox.apache.org/repos/asf/commons-dbcp.git
commit e564056b105e456ab09a4ccfbd109384028e0e81 Author: Phil Steitz <[email protected]> AuthorDate: Fri Nov 28 12:15:43 2025 -0700 Adjust tests and CPDSConnectionFactory#invalidate to accomodate behavior change in the fix for POOL-424. --- src/changes/changes.xml | 1 + .../dbcp2/datasources/CPDSConnectionFactory.java | 4 ++- .../apache/commons/dbcp2/TestBasicDataSource.java | 35 +++++++++++++++++++++- .../apache/commons/dbcp2/TestPoolingDriver.java | 1 - .../datasources/TestCPDSConnectionFactory.java | 12 ++++++-- 5 files changed, 47 insertions(+), 6 deletions(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 64444bca..b552ab45 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -121,6 +121,7 @@ The <action> type attribute can be add,update,fix,remove. <!-- ADD --> <action type="add" dev="ggregory" due-to="Gary Gregory">Add org.apache.commons.dbcp2.datasources.PooledConnectionManager.setPassword(char[]).</action> <!-- UPDATE --> + <action type="update" dev="psteitz">Update tests and CPDSConnectionFactory#invalidate to accomodate changed behavior in the fix for POOL-424.</action> <action type="update" dev="ggregory" due-to="Gary Gregory">Bump org.apache.commons:commons-parent from 78 to 92 #521, #537.</action> <action type="update" dev="ggregory" due-to="Gary Gregory">Bump org.apache.commons:commons-pool2 from 2.12.0 to 2.12.1 #474.</action> <action type="update" dev="ggregory" due-to="Gary Gregory">Port site from Doxia 1 to 2.</action> diff --git a/src/main/java/org/apache/commons/dbcp2/datasources/CPDSConnectionFactory.java b/src/main/java/org/apache/commons/dbcp2/datasources/CPDSConnectionFactory.java index b00c016e..3640e56c 100644 --- a/src/main/java/org/apache/commons/dbcp2/datasources/CPDSConnectionFactory.java +++ b/src/main/java/org/apache/commons/dbcp2/datasources/CPDSConnectionFactory.java @@ -192,9 +192,11 @@ final class CPDSConnectionFactory extends AbstractConnectionFactory throw new IllegalStateException(NO_KEY_MESSAGE); } try { - pool.invalidateObject(pci); // Destroy instance and update pool counters pool.close(); // Clear any other instances in this pool and kill others as they come back + // Calling close before invalidate ensures that invalidate will not trigger a create attempt + pool.invalidateObject(pci); // Destroy instance and update pool counters } catch (final Exception ex) { + ex.printStackTrace(); throw new SQLException("Error invalidating connection", ex); } } diff --git a/src/test/java/org/apache/commons/dbcp2/TestBasicDataSource.java b/src/test/java/org/apache/commons/dbcp2/TestBasicDataSource.java index a7a87408..86800651 100644 --- a/src/test/java/org/apache/commons/dbcp2/TestBasicDataSource.java +++ b/src/test/java/org/apache/commons/dbcp2/TestBasicDataSource.java @@ -568,6 +568,37 @@ public class TestBasicDataSource extends TestConnectionPool { assertNull(ds.getRegisteredJmxName()); } + /** + * Cycle through idle connections and verify that they are all valid. + * Assumes we are the only client of the pool. + * + * @throws Exception if an error occurs + */ + private void checkIdleValid() throws Exception { + final Set<Connection> idleConnections = new HashSet<>(); // idle connections + // Get idle connections by repeatedly making connection requests up to NumIdle + for (int i = 0; i < ds.getNumIdle(); i++) { + final Connection conn = ds.getConnection(); + idleConnections.add(conn); + } + // Cycle through idle connections and verify that they are valid + for (final Connection conn : idleConnections) { + assertTrue(conn.isValid(2), "Connection should be valid"); + conn.close(); + } + } + + /** + * Check that maxTotal and maxIdle are not exceeded + * + * @throws Exception + */ + private void checkLimits() throws Exception { + assertTrue(ds.getNumActive() + ds.getNumIdle() <= getMaxTotal(), "Total connections exceed maxTotal"); + assertTrue(ds.getNumIdle() <= ds.getMaxIdle(), "Idle connections exceed maxIdle"); + } + + @Test void testInvalidateConnection() throws Exception { ds.setMaxTotal(2); @@ -576,9 +607,11 @@ public class TestBasicDataSource extends TestConnectionPool { ds.invalidateConnection(conn1); assertTrue(conn1.isClosed()); assertEquals(1, ds.getNumActive()); - assertEquals(0, ds.getNumIdle()); + checkIdleValid(); + checkLimits(); try (final Connection conn3 = ds.getConnection()) { conn2.close(); + conn3.close(); } } } diff --git a/src/test/java/org/apache/commons/dbcp2/TestPoolingDriver.java b/src/test/java/org/apache/commons/dbcp2/TestPoolingDriver.java index 6d47a58d..964077c9 100644 --- a/src/test/java/org/apache/commons/dbcp2/TestPoolingDriver.java +++ b/src/test/java/org/apache/commons/dbcp2/TestPoolingDriver.java @@ -142,7 +142,6 @@ public class TestPoolingDriver extends TestConnectionPool { driver2.invalidateConnection(conn); assertEquals(0, pool.getNumActive()); - assertEquals(0, pool.getNumIdle()); assertTrue(conn.isClosed()); } diff --git a/src/test/java/org/apache/commons/dbcp2/datasources/TestCPDSConnectionFactory.java b/src/test/java/org/apache/commons/dbcp2/datasources/TestCPDSConnectionFactory.java index 924a9f85..5f94e39d 100644 --- a/src/test/java/org/apache/commons/dbcp2/datasources/TestCPDSConnectionFactory.java +++ b/src/test/java/org/apache/commons/dbcp2/datasources/TestCPDSConnectionFactory.java @@ -50,6 +50,12 @@ public class TestCPDSConnectionFactory { delegate.setPassword("password"); } + private void checkPoolLimits(final GenericObjectPool<PooledConnectionAndInfo> pool) { + assertTrue(pool.getNumActive() + pool.getNumIdle() <= pool.getMaxTotal(), + "Active + Idle should be <= MaxTotal"); + assertTrue(pool.getNumIdle() <= pool.getMaxIdle(), "Idle should be <= MaxIdle"); + } + /** * JIRA DBCP-216 * @@ -77,14 +83,14 @@ public class TestCPDSConnectionFactory { // Throw connectionError event pc.throwConnectionError(); - // Active count should be reduced by 1 and no idle increase + // Active count should be reduced by 1 assertEquals(1, pool.getNumActive()); - assertEquals(0, pool.getNumIdle()); + checkPoolLimits(pool); // Throw another one - should be ignored pc.throwConnectionError(); assertEquals(1, pool.getNumActive()); - assertEquals(0, pool.getNumIdle()); + checkPoolLimits(pool); // Ask for another connection final PooledConnection pcon3 = pool.borrowObject().getPooledConnection();
