DBCP-592: Support request boundaries

src/main/java/org/apache/commons/dbcp2/DelegatingConnection.java

@@ -16,6 +16,8 @@
  */
 package org.apache.commons.dbcp2;
 
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.sql.Array;
 import java.sql.Blob;
 import java.sql.CallableStatement;
@@ -42,6 +44,8 @@
 import java.util.concurrent.Executor;
 
 import org.apache.commons.dbcp2.managed.ManagedConnection;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 
 /**
  * A base delegating implementation of {@link Connection}.
@@ -76,6 +80,20 @@ public class DelegatingConnection<C extends Connection> extends AbandonedTrace i
     private String cachedCatalog;
     private String cachedSchema;
     private Duration defaultQueryTimeoutDuration;
+    /**
+     * Logger
+     */
+    private static final Log log = LogFactory.getLog(DelegatingConnection.class);
+    /**
+     * Request boundaries
+     */
+    private static final Method beginRequest;
+    private static final Method endRequest;
+
+    static {
+        beginRequest = getConnectionMethod("beginRequest");
+        endRequest = getConnectionMethod("endRequest");
+    }
 
     /**
      * Creates a wrapper for the Connection which traces this Connection in the AbandonedObjectPool.
@@ -86,6 +104,21 @@ public DelegatingConnection(final C connection) {
         this.connection = connection;
     }
 
+    /**
+     * Uses reflection to get the method form the Connection interface
+     * @param methodName name of the method to get
+     * @return the method if it exists, otherwise null
+     */
+    private static Method getConnectionMethod(String methodName) {
+        Method method = null;
+        try {
+            method = Connection.class.getMethod(methodName);
+        } catch (NoSuchMethodException ex) {
+            // Ignore exception and set both methods to null
+        }
+        return method;
+    }
+
     @Override
     public void abort(final Executor executor) throws SQLException {
         try {
@@ -98,8 +131,17 @@ public void abort(final Executor executor) throws SQLException {
     protected void activate() {
         closed = false;
         setLastUsed();
+
         if (connection instanceof DelegatingConnection) {
             ((DelegatingConnection<?>) connection).activate();
+        } else {
+            if (beginRequest != null && connection != null) {
+                try {
+                    beginRequest.invoke(connection);
+                } catch (InvocationTargetException | IllegalAccessException ex) {
+                    log.warn("Error calling beginRequest on connection", ex);
+                }
+            }
         }
     }
 
@@ -662,6 +704,13 @@ protected void passivate() throws SQLException {
                 throw new SQLExceptionList(thrownList);
             }
         }
+        if (endRequest != null && connection != null) {
+            try {
+                endRequest.invoke(connection);
+            } catch (InvocationTargetException | IllegalAccessException ex) {
+                log.warn("Error calling endRequest on connection", ex);
+            }
+        }
         setLastUsed(Instant.EPOCH);
     }
 

src/test/java/org/apache/commons/dbcp2/TestRequestBoundaries.java

@@ -0,0 +1,187 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.dbcp2;
+
+import org.junit.jupiter.api.Test;
+import org.mockito.stubbing.OngoingStubbing;
+
+import java.sql.Connection;
+import java.sql.Driver;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Properties;
+
+import static org.junit.jupiter.api.Assumptions.assumeTrue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.isNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.reset;
+
+public class TestRequestBoundaries {
+    Driver driver = mock(TesterDriver.class);
+
+    @Test
+    public void testBeginRequestOneConnection() throws SQLException {
+        // Verify JDK version
+        assumeTrue(Double.valueOf(System.getProperty("java.class.version")) >= 53);
+
+        // Setup
+        BasicDataSource dataSource = getDataSource(driver);
+        TesterConnection connection = setupPhysicalConnections(1).get(0);
+
+        // Get connection
+        dataSource.getConnection();
+
+        // Verify number of calls
+        assertCallCount(connection, 1, 0);
+    }
+
+    @Test
+    public void testEndRequestOneConnection() throws SQLException {
+        // Verify JDK version
+        assumeTrue(Double.valueOf(System.getProperty("java.class.version")) >= 53);
+
+        // Setup
+        BasicDataSource dataSource = getDataSource(driver);
+        TesterConnection connection = setupPhysicalConnections(1).get(0);
+
+        // Get then close connection
+        dataSource.getConnection().close();
+
+        // Verify number of calls
+        assertCallCount(connection, 1, 1);
+    }
+
+    @Test
+    public void testBeginRequestTwoVirtualConnections() throws SQLException {
+        // Verify JDK version
+        assumeTrue(Double.valueOf(System.getProperty("java.class.version")) >= 53);
+
+        // Setup
+        BasicDataSource dataSource = getDataSource(driver);
+        TesterConnection connection = setupPhysicalConnections(1).get(0);
+
+        // Get connection close it then get another connection
+        dataSource.getConnection().close();
+        dataSource.getConnection();
+
+        // Verify number calls
+        assertCallCount(connection, 2, 1);
+    }
+
+    @Test
+    public void testEndRequestTwoVirtualConnections() throws SQLException {
+        // Verify JDK version
+        assumeTrue(Double.valueOf(System.getProperty("java.class.version")) >= 53);
+
+        // Setup
+        BasicDataSource dataSource = getDataSource(driver);
+        TesterConnection connection = setupPhysicalConnections(1).get(0);
+
+        // Get a connection and close then get another connection and close it
+        dataSource.getConnection().close();
+        dataSource.getConnection().close();
+
+        // Verify number of calls
+        assertCallCount(connection, 2, 2);
+    }
+
+    @Test
+    public void testRequestBoundariesTwoPhysicalConnections() throws SQLException {
+        // Verify JDK version
+        assumeTrue(Double.valueOf(System.getProperty("java.class.version")) >= 53);
+
+        // Setup
+        BasicDataSource dataSource = getDataSource(driver);
+        List<TesterConnection> connections = setupPhysicalConnections(2);
+
+        // Get a connection, then get another connection, then close the first connection
+        Connection fetchedConnection = dataSource.getConnection();
+        dataSource.getConnection();
+        fetchedConnection.close();
+
+        // Verify number of calls
+        assertCallCount(connections.get(0), 1, 1);
+        assertCallCount(connections.get(1), 1, 0);
+    }
+
+    @Test
+    public void testConnectionWithoutRequestBoundaries() throws SQLException {
+        // Verify JDK version
+        assumeTrue(Double.valueOf(System.getProperty("java.class.version")) < 53);
+
+        // Setup
+        BasicDataSource dataSource = getDataSource(driver);
+        TesterConnection connection = setupPhysicalConnections(1).get(0);
+
+        // Get connection
+        dataSource.getConnection().close();
+
+        assertCallCount(connection, 0, 0);
+    }
+
+    public BasicDataSource getDataSource(Driver driver) throws SQLException {
+        reset(driver);
+
+        BasicDataSource dataSource = BasicDataSourceFactory.createDataSource(new Properties());
+        dataSource.setDriver(driver);
+
+        // Before testing the call count of beginRequest and endRequest method we'll make sure that the
+        // connectionFactory has been validated which involves creating a physical connection and destroying it. If we
+        // don't, it's going to be done automatically when the first connection is requested. This is going to mess the
+        // call count.
+        validateConnectionFactory(dataSource, driver);
+
+        return dataSource;
+    }
+
+    public void validateConnectionFactory(BasicDataSource dataSource, Driver driver) throws SQLException {
+        when(driver.connect(isNull(), any(Properties.class))).thenReturn(getTesterConnection());
+        dataSource.getLogWriter();
+    }
+
+    public TesterConnection getTesterConnection() throws SQLException {
+        TesterConnection connection = new TesterConnection(null, null);
+
+        return connection;
+    }
+
+    public List<TesterConnection> setupPhysicalConnections(int numOfConnections) throws SQLException {
+        List<TesterConnection> listOfConnections = new ArrayList<>();
+
+        for (int i = 0; i < numOfConnections; i++) {
+            listOfConnections.add(getTesterConnection());
+        }
+
+        OngoingStubbing<Connection> ongoingStubbing =  when(driver.connect(isNull(), any(Properties.class)));
+
+        for (Connection connection : listOfConnections) {
+            ongoingStubbing = ongoingStubbing.thenReturn(connection);
+        }
+        return listOfConnections;
+    }
+
+    public void assertCallCount(TesterConnection connection, int expectedBeginRequestCalls, int expectedEndRequestCalls)
+            throws SQLException {
+        assertEquals(expectedBeginRequestCalls, connection.beginRequestCount.get());
+        assertEquals(expectedEndRequestCalls, connection.endRequestCount.get());
+    }
+}

src/test/java/org/apache/commons/dbcp2/TesterConnection.java

@@ -34,6 +34,7 @@
 import java.util.Map;
 import java.util.Properties;
 import java.util.concurrent.Executor;
+import java.util.concurrent.atomic.AtomicInteger;
 
 /**
  * A dummy {@link Connection}, for testing purposes.
@@ -53,6 +54,8 @@ public class TesterConnection extends AbandonedTrace implements Connection {
     protected final String userName;
     protected Exception failure;
     protected boolean sqlExceptionOnClose;
+    public AtomicInteger beginRequestCount = new AtomicInteger(0);
+    public AtomicInteger endRequestCount = new AtomicInteger(0);
 
     TesterConnection(final String userName,
             @SuppressWarnings("unused") final String password) {
@@ -433,4 +436,12 @@ public void setWarnings(final SQLWarning warning) {
     public <T> T unwrap(final Class<T> iface) throws SQLException {
         throw new SQLException("Not implemented.");
     }
+
+    public void beginRequest() {
+        beginRequestCount.incrementAndGet();
+    }
+
+    public void endRequest() {
+        endRequestCount.incrementAndGet();
+    }
 }