diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 4d447bb99..37219f397 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -121,6 +121,7 @@ The type attribute can be add,update,fix,remove. Add org.apache.commons.dbcp2.datasources.PooledConnectionManager.setPassword(char[]). + Update tests and CPDSConnectionFactory#invalidate to accomodate changed behavior in the fix for POOL-424. Bump org.apache.commons:commons-parent from 78 to 93 #521, #537, #538. Bump org.apache.commons:commons-pool2 from 2.12.0 to 2.12.1 #474. Port site from Doxia 1 to 2. 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 b00c016e1..f5a68ec44 100644 --- a/src/main/java/org/apache/commons/dbcp2/datasources/CPDSConnectionFactory.java +++ b/src/main/java/org/apache/commons/dbcp2/datasources/CPDSConnectionFactory.java @@ -192,8 +192,9 @@ public void invalidate(final PooledConnection pc) throws SQLException { 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) { 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 a7a87408e..86800651b 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 @@ void testInstanceNotFoundExceptionLogSuppressed() throws Exception { 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 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 @@ void testInvalidateConnection() throws Exception { 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 6d47a58d5..964077c9f 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 @@ void testInvalidateConnection() throws Exception { 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 924a9f851..5f94e39d3 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 void setUp() throws Exception { delegate.setPassword("password"); } + private void checkPoolLimits(final GenericObjectPool 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 @@ void testConnectionErrorCleanup() throws Exception { // 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();