From 4f051f804a23723674c76cd4c386a433e44a44d9 Mon Sep 17 00:00:00 2001
From: "Gary D. Gregory" <garydgregory@gmail.com>
Date: Sun, 15 Dec 2024 18:01:31 -0500
Subject: [PATCH] Validation query not timing out on connections managed by
 PerUserPoolDataSource

---
 src/changes/changes.xml                       |  1 +
 .../datasources/CPDSConnectionFactory.java    | 56 -------------------
 .../TestPerUserPoolDataSource.java            | 21 +++++++
 3 files changed, 22 insertions(+), 56 deletions(-)

diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 1f3eb7c89..5cb22a677 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -65,6 +65,7 @@ The <action> type attribute can be add,update,fix,remove.
     <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" issue="DBCP-597" dev="ggregory" due-to="Gary Gregory">Validation query not timing out on connections managed by PerUserPoolDataSource.</action>
       <action type="fix" issue="DBCP-597" dev="ggregory" due-to="Gary Gregory">KeyedCPDSConnectionFactory.validateObject(UserPassKey, PooledObject) does ignores timeouts less than 1 second when there is no validation query.</action>
       <action type="fix" dev="ggregory" due-to="Gary Gregory">Modernize tests to use JUnit 5 features.</action> 
       <!-- ADD -->
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 97fadac0d..e1fb4d11e 100644
--- a/src/main/java/org/apache/commons/dbcp2/datasources/CPDSConnectionFactory.java
+++ b/src/main/java/org/apache/commons/dbcp2/datasources/CPDSConnectionFactory.java
@@ -17,9 +17,7 @@
 package org.apache.commons.dbcp2.datasources;
 
 import java.sql.Connection;
-import java.sql.ResultSet;
 import java.sql.SQLException;
-import java.sql.Statement;
 import java.time.Duration;
 
 import javax.sql.ConnectionEvent;
@@ -27,7 +25,6 @@
 import javax.sql.ConnectionPoolDataSource;
 import javax.sql.PooledConnection;
 
-import org.apache.commons.dbcp2.Utils;
 import org.apache.commons.pool2.ObjectPool;
 import org.apache.commons.pool2.PooledObject;
 import org.apache.commons.pool2.PooledObjectFactory;
@@ -280,57 +277,4 @@ public synchronized String toString() {
         builder.append("]");
         return builder.toString();
     }
-
-    @Override
-    public boolean validateObject(final PooledObject<PooledConnectionAndInfo> pooledObject) {
-        try {
-            validateLifetime(pooledObject);
-        } catch (final Exception e) {
-            return false;
-        }
-        boolean valid = false;
-        final PooledConnection pconn = pooledObject.getObject().getPooledConnection();
-        Connection conn = null;
-        validatingSet.add(pconn);
-        if (null == validationQuery) {
-            Duration timeoutDuration = validationQueryTimeoutDuration;
-            if (timeoutDuration.isNegative()) {
-                timeoutDuration = Duration.ZERO;
-            }
-            try {
-                conn = pconn.getConnection();
-                valid = conn.isValid((int) timeoutDuration.getSeconds());
-            } catch (final SQLException e) {
-                valid = false;
-            } finally {
-                Utils.closeQuietly((AutoCloseable) conn);
-                validatingSet.remove(pconn);
-            }
-        } else {
-            Statement stmt = null;
-            ResultSet rset = null;
-            // logical Connection from the PooledConnection must be closed
-            // before another one can be requested and closing it will
-            // generate an event. Keep track so we know not to return
-            // the PooledConnection
-            validatingSet.add(pconn);
-            try {
-                conn = pconn.getConnection();
-                stmt = conn.createStatement();
-                rset = stmt.executeQuery(validationQuery);
-                valid = rset.next();
-                if (rollbackAfterValidation) {
-                    conn.rollback();
-                }
-            } catch (final Exception e) {
-                valid = false;
-            } finally {
-                Utils.closeQuietly((AutoCloseable) rset);
-                Utils.closeQuietly((AutoCloseable) stmt);
-                Utils.closeQuietly((AutoCloseable) conn);
-                validatingSet.remove(pconn);
-            }
-        }
-        return valid;
-    }
 }
diff --git a/src/test/java/org/apache/commons/dbcp2/datasources/TestPerUserPoolDataSource.java b/src/test/java/org/apache/commons/dbcp2/datasources/TestPerUserPoolDataSource.java
index 82eb083c6..4af534680 100644
--- a/src/test/java/org/apache/commons/dbcp2/datasources/TestPerUserPoolDataSource.java
+++ b/src/test/java/org/apache/commons/dbcp2/datasources/TestPerUserPoolDataSource.java
@@ -35,6 +35,7 @@
 import java.time.Duration;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.NoSuchElementException;
 
 import javax.sql.DataSource;
 
@@ -167,6 +168,26 @@ public void testClosingWithUserName() throws Exception {
         }
     }
 
+    /**
+     * Tests https://issues.apache.org/jira/browse/DBCP-597
+     */
+    @Test
+    public void testDbcp597() throws SQLException {
+        PerUserPoolDataSource tds = (PerUserPoolDataSource) ds;
+        tds.setDefaultTestOnBorrow(true);
+        tds.setValidationQuery("SELECT 1");
+        // The tester statement throws a SQLTimeoutException when the timeout is > 0 and < 5.
+        tds.setValidationQueryTimeout(Duration.ofSeconds(1));
+        // The SQLTimeoutException is lost for now
+        SQLException e = assertThrows(SQLException.class, tds::getConnection);
+        assertEquals(NoSuchElementException.class, e.getCause().getClass());
+        // timeout > 0 and < 1
+        tds.setValidationQueryTimeout(Duration.ofMillis(999));
+        // The SQLTimeoutException is lost for now
+        e = assertThrows(SQLException.class, tds::getConnection);
+        assertEquals(NoSuchElementException.class, e.getCause().getClass());
+    }
+
     // see issue https://issues.apache.org/bugzilla/show_bug.cgi?id=23843
     @Test
     public void testDefaultUser1() throws Exception {