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 0c8597da Adjust tests and CPDSConnectionFactory#invalidate to
accomodate behav… (#539)
0c8597da is described below
commit 0c8597da6e53c1fc75a275496f53c822787c79d2
Author: Phil Steitz <[email protected]>
AuthorDate: Fri Nov 28 17:07:59 2025 -0700
Adjust tests and CPDSConnectionFactory#invalidate to accomodate behav…
(#539)
* Adjust tests and CPDSConnectionFactory#invalidate to accomodate behavior
change in the fix for POOL-424.
* Remove `ex.printStackTrace();`
---------
Co-authored-by: Gary Gregory <[email protected]>
---
src/changes/changes.xml | 1 +
.../dbcp2/datasources/CPDSConnectionFactory.java | 3 +-
.../apache/commons/dbcp2/TestBasicDataSource.java | 35 +++++++++++++++++++++-
.../apache/commons/dbcp2/TestPoolingDriver.java | 1 -
.../datasources/TestCPDSConnectionFactory.java | 12 ++++++--
5 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 4d447bb9..37219f39 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 93 #521, #537, #538.</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..f5a68ec4 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 @@ 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) {
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();
