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 2021/01/13 19:33:56 UTC

[commons-dbcp] branch master updated: [DBCP-568] ManagedConnection must clear its cached state after transaction completes #75 (#75)

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 2535b94  [DBCP-568] ManagedConnection must clear its cached state after transaction completes #75 (#75)
2535b94 is described below

commit 2535b94a95c3a1b54b2612109e9ad1267c80ebae
Author: Florent Guillaume <fg...@nuxeo.com>
AuthorDate: Wed Jan 13 20:33:46 2021 +0100

    [DBCP-568] ManagedConnection must clear its cached state after transaction completes #75 (#75)
---
 src/changes/changes.xml                            |   3 +
 .../commons/dbcp2/managed/ManagedConnection.java   |   3 +
 .../org/apache/commons/dbcp2/TesterConnection.java |   3 +
 .../managed/TestManagedConnectionCachedState.java  | 130 +++++++++++++++++++++
 4 files changed, 139 insertions(+)

diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 30a673a..7a157b3 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -92,6 +92,9 @@ The <action> type attribute can be add,update,fix,remove.
       <action dev="ggregory" type="update" issue="DBCP-569" due-to="Florent Guillaume">
          Fix test random failure on TestSynchronizationOrder.testInterposedSynchronization #84.
       </action>
+      <action dev="ggregory" type="fix" issue="DBCP-568" due-to="Florent Guillaume">
+         ManagedConnection must clear its cached state after transaction completes #75.
+      </action>
     </release>
     <release version="2.8.0" date="2020-09-21" description="This is a minor release, including bug fixes and enhancements.">
       <!-- add -->
diff --git a/src/main/java/org/apache/commons/dbcp2/managed/ManagedConnection.java b/src/main/java/org/apache/commons/dbcp2/managed/ManagedConnection.java
index 351fe87..f9f86c3 100644
--- a/src/main/java/org/apache/commons/dbcp2/managed/ManagedConnection.java
+++ b/src/main/java/org/apache/commons/dbcp2/managed/ManagedConnection.java
@@ -216,6 +216,9 @@ public class ManagedConnection<C extends Connection> extends DelegatingConnectio
             isSharedConnection = false;
         }
 
+        // autoCommit may have been changed directly on the underlying connection
+        clearCachedState();
+
         // If this connection was closed during the transaction and there is
         // still a delegate present close it
         final Connection delegate = getDelegateInternal();
diff --git a/src/test/java/org/apache/commons/dbcp2/TesterConnection.java b/src/test/java/org/apache/commons/dbcp2/TesterConnection.java
index 8dc0120..ed26695 100644
--- a/src/test/java/org/apache/commons/dbcp2/TesterConnection.java
+++ b/src/test/java/org/apache/commons/dbcp2/TesterConnection.java
@@ -338,6 +338,9 @@ public class TesterConnection extends AbandonedTrace implements Connection {
         if (isReadOnly()) {
             throw new SQLException("Cannot rollback a readonly connection");
         }
+        if (getAutoCommit()) {
+            throw new SQLException("Cannot rollback a connection in auto-commit");
+        }
     }
 
     @Override
diff --git a/src/test/java/org/apache/commons/dbcp2/managed/TestManagedConnectionCachedState.java b/src/test/java/org/apache/commons/dbcp2/managed/TestManagedConnectionCachedState.java
new file mode 100644
index 0000000..5f34690
--- /dev/null
+++ b/src/test/java/org/apache/commons/dbcp2/managed/TestManagedConnectionCachedState.java
@@ -0,0 +1,130 @@
+/**
+ *
+ * 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.managed;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Properties;
+
+import javax.transaction.TransactionManager;
+import javax.transaction.xa.XAException;
+
+import org.apache.commons.dbcp2.ConnectionFactory;
+import org.apache.commons.dbcp2.DriverConnectionFactory;
+import org.apache.commons.dbcp2.PoolableConnection;
+import org.apache.commons.dbcp2.PoolableConnectionFactory;
+import org.apache.commons.dbcp2.PoolingDataSource;
+import org.apache.commons.dbcp2.TesterDriver;
+import org.apache.commons.pool2.SwallowedExceptionListener;
+import org.apache.commons.pool2.impl.GenericObjectPool;
+import org.apache.geronimo.transaction.manager.TransactionManagerImpl;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Test for ManagedConnection cached state.
+ */
+public class TestManagedConnectionCachedState {
+
+    private PoolingDataSource<PoolableConnection> ds;
+
+    private GenericObjectPool<PoolableConnection> pool;
+
+    private TransactionManager transactionManager;
+
+    private SwallowedExceptionRecorder swallowedExceptionRecorder;
+
+    @BeforeEach
+    public void setUp() throws XAException {
+        // create a GeronimoTransactionManager for testing
+        transactionManager = new TransactionManagerImpl();
+
+        // create a driver connection factory
+        final Properties properties = new Properties();
+        properties.setProperty("user", "userName");
+        properties.setProperty("password", "password");
+        final ConnectionFactory connectionFactory = new DriverConnectionFactory(new TesterDriver(), "jdbc:apache:commons:testdriver", properties);
+
+        // wrap it with a LocalXAConnectionFactory
+        final XAConnectionFactory xaConnectionFactory = new LocalXAConnectionFactory(transactionManager, connectionFactory);
+
+        // create the pool object factory
+        // make sure we ask for state caching
+        final PoolableConnectionFactory factory = new PoolableConnectionFactory(xaConnectionFactory, null);
+        factory.setValidationQuery("SELECT DUMMY FROM DUAL");
+        factory.setCacheState(true);
+
+        // create the pool
+        pool = new GenericObjectPool<>(factory);
+        factory.setPool(pool);
+        // record swallowed exceptions
+        swallowedExceptionRecorder = new SwallowedExceptionRecorder();
+        pool.setSwallowedExceptionListener(swallowedExceptionRecorder);
+
+        // finally create the datasource
+        ds = new ManagedDataSource<>(pool, xaConnectionFactory.getTransactionRegistry());
+        ds.setAccessToUnderlyingConnectionAllowed(true);
+    }
+
+    @AfterEach
+    public void tearDown() {
+        pool.close();
+    }
+
+    public Connection getConnection() throws SQLException {
+        return ds.getConnection();
+    }
+
+    @Test
+    public void testConnectionCachedState() throws Exception {
+        // see DBCP-568
+
+        // begin a transaction
+        transactionManager.begin();
+        // acquire a connection enlisted in the transaction
+        final Connection conn = getConnection();
+        // check the autocommit status to trigger internal caching
+        conn.getAutoCommit();
+        // ask the transaction manager to rollback
+        transactionManager.rollback();
+        // close the connection
+        conn.close();
+        // check that no exceptions about failed rollback during close were logged
+        assertEquals(0, swallowedExceptionRecorder.getExceptions().size());
+    }
+
+    private static class SwallowedExceptionRecorder implements SwallowedExceptionListener {
+
+        private List<Exception> exceptions = new ArrayList<>();
+
+        @Override
+        public void onSwallowException(final Exception e) {
+            exceptions.add(e);
+        }
+
+        public List<Exception> getExceptions() {
+            return exceptions;
+        }
+    }
+
+}