[commons-dbcp] branch master updated: Fix DelegatingConnection readOnly and autoCommit caching mechanism (#35)

[gg...@apache.org]

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 79422dc  Fix DelegatingConnection readOnly and autoCommit caching mechanism (#35)
79422dc is described below

commit 79422dc193221b745617984d30c321436c2a849c
Author: louislatreille <lo...@users.noreply.github.com>
AuthorDate: Mon Aug 10 15:38:57 2020 -0400

    Fix DelegatingConnection readOnly and autoCommit caching mechanism (#35)
    
    Previously, we would cache the input of setReadOnly and setAutoCommit directly without checking if the underlying connection succeeded in setting the configuration parameter. This would result in wrong results from isReadOnly and getAutoCommit if the underlying connection didn't support read-only/non-read-only or auto-commit/non-auto-commit connections, as with SqlServerConnection.
    
    This fixes the issue by caching what is return by isReadOnly and getAutoCommit from the underlying connection.
    
    Co-authored-by: louisl <lo...@trendmicro.com>
---
 .../apache/commons/dbcp2/DelegatingConnection.java |  4 +-
 .../commons/dbcp2/TestDelegatingConnection.java    | 56 ++++++++++++++++++++++
 2 files changed, 58 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/apache/commons/dbcp2/DelegatingConnection.java b/src/main/java/org/apache/commons/dbcp2/DelegatingConnection.java
index f005b21..96f65b9 100644
--- a/src/main/java/org/apache/commons/dbcp2/DelegatingConnection.java
+++ b/src/main/java/org/apache/commons/dbcp2/DelegatingConnection.java
@@ -536,7 +536,7 @@ public class DelegatingConnection<C extends Connection> extends AbandonedTrace i
         try {
             connection.setAutoCommit(autoCommit);
             if (cacheState) {
-                autoCommitCached = Boolean.valueOf(autoCommit);
+                autoCommitCached = Boolean.valueOf(connection.getAutoCommit());
             }
         } catch (final SQLException e) {
             autoCommitCached = null;
@@ -560,7 +560,7 @@ public class DelegatingConnection<C extends Connection> extends AbandonedTrace i
         try {
             connection.setReadOnly(readOnly);
             if (cacheState) {
-                readOnlyCached = Boolean.valueOf(readOnly);
+                readOnlyCached = Boolean.valueOf(connection.isReadOnly());
             }
         } catch (final SQLException e) {
             readOnlyCached = null;
diff --git a/src/test/java/org/apache/commons/dbcp2/TestDelegatingConnection.java b/src/test/java/org/apache/commons/dbcp2/TestDelegatingConnection.java
index 56bb998..02a836c 100644
--- a/src/test/java/org/apache/commons/dbcp2/TestDelegatingConnection.java
+++ b/src/test/java/org/apache/commons/dbcp2/TestDelegatingConnection.java
@@ -53,6 +53,40 @@ public class TestDelegatingConnection {
 
     }
 
+    /**
+     * Delegate that doesn't support read-only or auto-commit.
+     * It will merely take the input value of setReadOnly and
+     * setAutoCommit and discard it, to keep false.
+     */
+    static class NoReadOnlyOrAutoCommitConnection extends TesterConnection {
+        private final boolean readOnly = false;
+        private final boolean autoCommit = false;
+
+        public NoReadOnlyOrAutoCommitConnection() {
+            super("","");
+        }
+        
+        @Override
+        public void setReadOnly(boolean readOnly) {
+            // Do nothing
+        }
+
+        @Override
+        public boolean isReadOnly() throws SQLException {
+            return readOnly;
+        }
+
+        @Override
+        public void setAutoCommit(boolean autoCommit) {
+            // Do nothing
+        }
+
+        @Override
+        public boolean getAutoCommit() {
+            return autoCommit;
+        }
+    }
+
     private DelegatingConnection<? extends Connection> delegatingConnection;
     private Connection connection;
     private Connection connection2;
@@ -200,4 +234,26 @@ public class TestDelegatingConnection {
         }
     }
 
+    @Test
+    public void testReadOnlyCaching() throws SQLException {
+        Connection con = new NoReadOnlyOrAutoCommitConnection();
+        DelegatingConnection<Connection> delCon = new DelegatingConnection<>(con);
+
+        delCon.setReadOnly(true);
+
+        assertFalse(con.isReadOnly());
+        assertFalse(delCon.isReadOnly());
+    }
+
+    @Test
+    public void testAutoCommitCaching() throws SQLException {
+        Connection con = new NoReadOnlyOrAutoCommitConnection();
+        DelegatingConnection<Connection> delCon = new DelegatingConnection<>(con);
+
+        delCon.setAutoCommit(true);
+
+        assertFalse(con.getAutoCommit());
+        assertFalse(delCon.getAutoCommit());
+    }
+
 }