You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2024/12/15 14:45:09 UTC
(commons-dbcp) 05/05: [DBCP-597] Validation query not timing out on connections managed by SharedPoolDataSource
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
commit 7659d9e55f77a0633972f268a62f68dabb8ff5c5
Author: Gary D. Gregory <ga...@gmail.com>
AuthorDate: Sun Dec 15 09:28:59 2024 -0500
[DBCP-597] Validation query not timing out on connections managed by
SharedPoolDataSource
- KeyedCPDSConnectionFactory.validateObject(UserPassKey,
PooledObject<PooledConnectionAndInfo>)
- Test fixture should throw SQLTimeoutException instead of SQLException
for query validation timeouts
---
src/changes/changes.xml | 1 +
.../dbcp2/datasources/KeyedCPDSConnectionFactory.java | 8 ++++++++
.../apache/commons/dbcp2/TesterPreparedStatement.java | 6 ++----
.../java/org/apache/commons/dbcp2/TesterStatement.java | 13 +++++++++----
.../dbcp2/datasources/TestSharedPoolDataSource.java | 18 ++++++++++++++++++
5 files changed, 38 insertions(+), 8 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 7364c62e..048131f7 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -64,6 +64,7 @@ The <action> type attribute can be add,update,fix,remove.
<body>
<release version="2.13.1" date="YYYY-MM-DD" description="This is a minor release, including bug fixes and enhancements.">
<!-- FIX -->
+ <action type="fix" issue="DBCP-597" dev="ggregory" due-to="Xiaotian Bai, Raju Gupta, Gary Gregory">Validation query not timing out on connections managed by SharedPoolDataSource.</action>
<action type="fix" dev="ggregory" due-to="Gary Gregory">Modernize tests to use JUnit 5 features.</action>
<!-- ADD -->
<!-- UPDATE -->
diff --git a/src/main/java/org/apache/commons/dbcp2/datasources/KeyedCPDSConnectionFactory.java b/src/main/java/org/apache/commons/dbcp2/datasources/KeyedCPDSConnectionFactory.java
index bf5a2a08..f0429733 100644
--- a/src/main/java/org/apache/commons/dbcp2/datasources/KeyedCPDSConnectionFactory.java
+++ b/src/main/java/org/apache/commons/dbcp2/datasources/KeyedCPDSConnectionFactory.java
@@ -318,12 +318,16 @@ final class KeyedCPDSConnectionFactory implements KeyedPooledObjectFactory<UserP
/**
* Validates a pooled connection.
+ * <p>
+ * A query validation timeout greater than 0 and less than 1 second is converted to 1 second.
+ * </p>
*
* @param key
* ignored
* @param pooledObject
* wrapped {@code PooledConnectionAndInfo} containing the connection to validate
* @return true if validation succeeds
+ * @throws ArithmeticException if the query validation timeout does not fit as seconds in an int.
*/
@Override
public boolean validateObject(final UserPassKey key, final PooledObject<PooledConnectionAndInfo> pooledObject) {
@@ -361,6 +365,10 @@ final class KeyedCPDSConnectionFactory implements KeyedPooledObjectFactory<UserP
try {
conn = pooledConn.getConnection();
stmt = conn.createStatement();
+ if (!validationQueryTimeoutDuration.isNegative() && !validationQueryTimeoutDuration.isZero()) {
+ long seconds = validationQueryTimeoutDuration.getSeconds();
+ stmt.setQueryTimeout(seconds != 0 ? Math.toIntExact(seconds) : 1);
+ }
rset = stmt.executeQuery(validationQuery);
valid = rset.next();
if (rollbackAfterValidation) {
diff --git a/src/test/java/org/apache/commons/dbcp2/TesterPreparedStatement.java b/src/test/java/org/apache/commons/dbcp2/TesterPreparedStatement.java
index b171c857..d4f3268b 100644
--- a/src/test/java/org/apache/commons/dbcp2/TesterPreparedStatement.java
+++ b/src/test/java/org/apache/commons/dbcp2/TesterPreparedStatement.java
@@ -189,10 +189,7 @@ public class TesterPreparedStatement extends TesterStatement implements Prepared
if ("null".equals(sql)) {
return null;
}
- if (queryTimeout > 0 && queryTimeout < 5) {
- // Simulate timeout if queryTimout is set to less than 5 seconds
- throw new SQLException("query timeout");
- }
+ checkQueryTimeout();
return new TesterResultSet(this, resultSetType, resultSetConcurrency);
}
@@ -202,6 +199,7 @@ public class TesterPreparedStatement extends TesterStatement implements Prepared
if ("null".equals(sql)) {
return null;
}
+ checkQueryTimeout();
return new TesterResultSet(this, resultSetType, resultSetConcurrency);
}
diff --git a/src/test/java/org/apache/commons/dbcp2/TesterStatement.java b/src/test/java/org/apache/commons/dbcp2/TesterStatement.java
index 705df9bf..01251cae 100644
--- a/src/test/java/org/apache/commons/dbcp2/TesterStatement.java
+++ b/src/test/java/org/apache/commons/dbcp2/TesterStatement.java
@@ -20,6 +20,7 @@ package org.apache.commons.dbcp2;
import java.sql.Connection;
import java.sql.ResultSet;
import java.sql.SQLException;
+import java.sql.SQLTimeoutException;
import java.sql.SQLWarning;
import java.sql.Statement;
@@ -190,10 +191,7 @@ public class TesterStatement extends AbandonedTrace implements Statement {
default:
break;
}
- // Simulate timeout if queryTimout is set to less than 5 seconds
- if (queryTimeout > 0 && queryTimeout < 5) {
- throw new SQLException("query timeout");
- }
+ checkQueryTimeout();
return new TesterResultSet(this);
}
@@ -406,4 +404,11 @@ public class TesterStatement extends AbandonedTrace implements Statement {
public <T> T unwrap(final Class<T> iface) throws SQLException {
throw new SQLException("Not implemented.");
}
+
+ protected void checkQueryTimeout() throws SQLTimeoutException {
+ if (queryTimeout > 0 && queryTimeout < 5) {
+ // Simulate timeout if queryTimout is set to less than 5 seconds
+ throw new SQLTimeoutException("query timeout " + queryTimeout);
+ }
+ }
}
diff --git a/src/test/java/org/apache/commons/dbcp2/datasources/TestSharedPoolDataSource.java b/src/test/java/org/apache/commons/dbcp2/datasources/TestSharedPoolDataSource.java
index 3f5e1311..574b79fc 100644
--- a/src/test/java/org/apache/commons/dbcp2/datasources/TestSharedPoolDataSource.java
+++ b/src/test/java/org/apache/commons/dbcp2/datasources/TestSharedPoolDataSource.java
@@ -31,6 +31,7 @@ import java.sql.SQLException;
import java.sql.Statement;
import java.time.Duration;
import java.util.ArrayList;
+import java.util.NoSuchElementException;
import javax.sql.DataSource;
@@ -446,6 +447,23 @@ public class TestSharedPoolDataSource extends TestConnectionPool {
}
}
+ /**
+ * Tests https://issues.apache.org/jira/browse/DBCP-597
+ */
+ @Test
+ public void testDbcp597() throws SQLException {
+ try (final SharedPoolDataSource sharedPoolDataSource = new SharedPoolDataSource()) {
+ sharedPoolDataSource.setConnectionPoolDataSource(pcds);
+ sharedPoolDataSource.setDefaultTestOnBorrow(true);
+ sharedPoolDataSource.setValidationQuery("SELECT 1");
+ // The tester statement throws a SQLTimeoutException when the timeout is > 0 and < 5.
+ sharedPoolDataSource.setValidationQueryTimeout(Duration.ofSeconds(1));
+ // The SQLTimeoutException is lost for now
+ final SQLException e = assertThrows(SQLException.class, sharedPoolDataSource::getConnection);
+ assertEquals(NoSuchElementException.class, e.getCause().getClass());
+ }
+ }
+
/**
* Starting with a successful connection, then incorrect password, then correct password for same user illustrates JIRA: DBCP-245
*/