Mailing-List: contact commons-dev-help@jakarta.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Jakarta Commons Developers List" <commons-dev@jakarta.apache.org>
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r557176 - in /jakarta/commons/proper/dbcp/trunk:
 src/java/org/apache/commons/dbcp/
 src/java/org/apache/commons/dbcp/cpdsadapter/
 src/test/org/apache/commons/dbcp/ src/test/org/apache/commons/dbcp/managed/
 xdocs/
Date: Wed, 18 Jul 2007 06:46:20 -0000
To: commons-cvs@jakarta.apache.org
From: psteitz@apache.org
Message-Id: <20070718064621.9223A1A981A@eris.apache.org>

Author: psteitz
Date: Tue Jul 17 23:46:16 2007
New Revision: 557176

URL: http://svn.apache.org/viewvc?view=rev&rev=557176
Log:
Changed behavior to allow Connection, Statement, PreparedStatement,
CallableStatement and ResultSet to be closed multiple times. The first time
close is called the resource is closed and any subsequent calls have no effect.
This behavior is required as per the JavaDocs for these classes. Also added
tests for closing all types multiple times and updated any tests that
incorrectly assert that a resource can not be closed more then once.

JIRA: DBCP-233
Patch provided by Dain Sundstrom
Fixes DBCP-134, DBCP-3

Modified:
    jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/DelegatingConnection.java
    jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolablePreparedStatement.java
    jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolingDataSource.java
    jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolingDriver.java
    jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/cpdsadapter/ConnectionImpl.java
    jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestConnectionPool.java
    jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestManual.java
    jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterResultSet.java
    jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterStatement.java
    jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/managed/TestManagedDataSource.java
    jakarta/commons/proper/dbcp/trunk/xdocs/changes.xml

Modified: jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/DelegatingConnection.java
URL: http://svn.apache.org/viewvc/jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/DelegatingConnection.java?view=diff&rev=557176&r1=557175&r2=557176
==============================================================================
--- jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/DelegatingConnection.java (original)
+++ jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/DelegatingConnection.java Tue Jul 17 23:46:16 2007
@@ -208,10 +208,17 @@
      * Closes the underlying connection, and close
      * any Statements that were not explicitly closed.
      */
-    public void close() throws SQLException
-    {
-        passivate();
-        _conn.close();
+    public void close() throws SQLException {
+        // close can be called multiple times, but PoolableConnection improperly
+        // throws an exception when a connection is closed twice, so before calling
+        // close we aren't alreayd closed
+        if (!isClosed()) {
+            try {
+                _conn.close();
+            } finally {
+                _closed = true;
+            }
+        }
     }
 
     protected void handleException(SQLException e) throws SQLException {

Modified: jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolablePreparedStatement.java
URL: http://svn.apache.org/viewvc/jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolablePreparedStatement.java?view=diff&rev=557176&r1=557175&r2=557176
==============================================================================
--- jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolablePreparedStatement.java (original)
+++ jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolablePreparedStatement.java Tue Jul 17 23:46:16 2007
@@ -72,9 +72,8 @@
      * Return me to my pool.
      */
     public void close() throws SQLException {
-        if(isClosed()) {
-            throw new SQLException("Already closed");
-        } else {
+        // calling close twice should have no effect
+        if (!isClosed()) {
             try {
                 _pool.returnObject(_key,this);
             } catch(SQLException e) {

Modified: jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolingDataSource.java
URL: http://svn.apache.org/viewvc/jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolingDataSource.java?view=diff&rev=557176&r1=557175&r2=557176
==============================================================================
--- jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolingDataSource.java (original)
+++ jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolingDataSource.java Tue Jul 17 23:46:16 2007
@@ -177,10 +177,11 @@
         }
     
         public void close() throws SQLException {
-            checkOpen();
-            this.delegate.close();
-            this.delegate = null;
-            super.setDelegate(null);
+            if (delegate != null) {
+                this.delegate.close();
+                this.delegate = null;
+                super.setDelegate(null);
+            }
         }
 
         public boolean isClosed() throws SQLException {

Modified: jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolingDriver.java
URL: http://svn.apache.org/viewvc/jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolingDriver.java?view=diff&rev=557176&r1=557175&r2=557176
==============================================================================
--- jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolingDriver.java (original)
+++ jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/PoolingDriver.java Tue Jul 17 23:46:16 2007
@@ -263,12 +263,13 @@
                 throw new SQLException("Connection is closed.");
             }
         }
-    
+
         public void close() throws SQLException {
-            checkOpen();
-            this.delegate.close();
-            this.delegate = null;
-            super.setDelegate(null);
+            if (delegate != null) {
+                this.delegate.close();
+                this.delegate = null;
+                super.setDelegate(null);
+            }
         }
 
         public boolean isClosed() throws SQLException {

Modified: jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/cpdsadapter/ConnectionImpl.java
URL: http://svn.apache.org/viewvc/jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/cpdsadapter/ConnectionImpl.java?view=diff&rev=557176&r1=557175&r2=557176
==============================================================================
--- jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/cpdsadapter/ConnectionImpl.java (original)
+++ jakarta/commons/proper/dbcp/trunk/src/java/org/apache/commons/dbcp/cpdsadapter/ConnectionImpl.java Tue Jul 17 23:46:16 2007
@@ -113,9 +113,10 @@
      * @exception SQLException The database connection couldn't be closed.
      */
     public void close() throws SQLException {
-        assertOpen();
-        isClosed = true;
-        pooledConnection.notifyListeners();
+        if (!isClosed) {
+            isClosed = true;
+            pooledConnection.notifyListeners();
+        }
     }
 
     /**

Modified: jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestConnectionPool.java
URL: http://svn.apache.org/viewvc/jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestConnectionPool.java?view=diff&rev=557176&r1=557175&r2=557176
==============================================================================
--- jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestConnectionPool.java (original)
+++ jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestConnectionPool.java Tue Jul 17 23:46:16 2007
@@ -148,37 +148,92 @@
         }
     }
 
-    public void testCantCloseConnectionTwice() throws Exception {
-        for(int i=0;i<getMaxActive();i++) { // loop to show we *can* close again once we've borrowed it from the pool again
+    /**
+     * Verify the close method can be called multiple times on a single connection without
+     * an exception being thrown.
+     */
+    public void testCanCloseConnectionTwice() throws Exception {
+        for (int i = 0; i < getMaxActive(); i++) { // loop to show we *can* close again once we've borrowed it from the pool again
             Connection conn = newConnection();
             assertTrue(null != conn);
             assertTrue(!conn.isClosed());
             conn.close();
             assertTrue(conn.isClosed());
-            try {
-                conn.close();
-                fail("Expected SQLException on second attempt to close (" + conn.getClass().getName() + ")");
-            } catch(SQLException e) {
-                // expected
-            }
+            conn.close();
             assertTrue(conn.isClosed());
         }
     }
 
-    public void testCantCloseStatementTwice() throws Exception {
+    public void testCanCloseStatementTwice() throws Exception {
+        Connection conn = newConnection();
+        assertTrue(null != conn);
+        assertTrue(!conn.isClosed());
+        for(int i=0;i<2;i++) { // loop to show we *can* close again once we've borrowed it from the pool again
+            Statement stmt = conn.createStatement();
+            assertNotNull(stmt);
+            assertFalse(isClosed(stmt));
+            stmt.close();
+            assertTrue(isClosed(stmt));
+            stmt.close();
+            assertTrue(isClosed(stmt));
+            stmt.close();
+            assertTrue(isClosed(stmt));
+        }
+        conn.close();
+    }
+
+    public void testCanClosePreparedStatementTwice() throws Exception {
         Connection conn = newConnection();
         assertTrue(null != conn);
         assertTrue(!conn.isClosed());
         for(int i=0;i<2;i++) { // loop to show we *can* close again once we've borrowed it from the pool again
             PreparedStatement stmt = conn.prepareStatement("select * from dual");
-            assertTrue(null != stmt);
+            assertNotNull(stmt);
+            assertFalse(isClosed(stmt));
             stmt.close();
-            try {
-                stmt.close();
-                fail("Expected SQLException on second attempt to close (" + stmt.getClass().getName() + ")");
-            } catch(SQLException e) {
-                // expected
-            }
+            assertTrue(isClosed(stmt));
+            stmt.close();
+            assertTrue(isClosed(stmt));
+            stmt.close();
+            assertTrue(isClosed(stmt));
+        }
+        conn.close();
+    }
+
+    public void testCanCloseCallableStatementTwice() throws Exception {
+        Connection conn = newConnection();
+        assertTrue(null != conn);
+        assertTrue(!conn.isClosed());
+        for(int i=0;i<2;i++) { // loop to show we *can* close again once we've borrowed it from the pool again
+            PreparedStatement stmt = conn.prepareCall("select * from dual");
+            assertNotNull(stmt);
+            assertFalse(isClosed(stmt));
+            stmt.close();
+            assertTrue(isClosed(stmt));
+            stmt.close();
+            assertTrue(isClosed(stmt));
+            stmt.close();
+            assertTrue(isClosed(stmt));
+        }
+        conn.close();
+    }
+
+    public void testCanCloseResultSetTwice() throws Exception {
+        Connection conn = newConnection();
+        assertTrue(null != conn);
+        assertTrue(!conn.isClosed());
+        for(int i=0;i<2;i++) { // loop to show we *can* close again once we've borrowed it from the pool again
+            PreparedStatement stmt = conn.prepareStatement("select * from dual");
+            assertNotNull(stmt);
+            ResultSet rset = stmt.executeQuery();
+            assertNotNull(rset);
+            assertFalse(isClosed(rset));
+            rset.close();
+            assertTrue(isClosed(rset));
+            rset.close();
+            assertTrue(isClosed(rset));
+            rset.close();
+            assertTrue(isClosed(rset));
         }
         conn.close();
     }
@@ -529,5 +584,31 @@
         assertNotNull(conn2);
 
         assertTrue(conn1.hashCode() != conn2.hashCode());
+    }
+
+    protected boolean isClosed(Statement statement) {
+        try {
+            statement.getWarnings();
+            return false;
+        } catch (SQLException e) {
+            // getWarnings throws an exception if the statement is
+            // closed, but could throw an exception for other reasons
+            // in this case it is good enought to assume the statement
+            // is closed
+            return true;
+        }
+    }
+
+    protected boolean isClosed(ResultSet resultSet) {
+        try {
+            resultSet.getWarnings();
+            return false;
+        } catch (SQLException e) {
+            // getWarnings throws an exception if the statement is
+            // closed, but could throw an exception for other reasons
+            // in this case it is good enought to assume the result set
+            // is closed
+            return true;
+        }
     }
 }

Modified: jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestManual.java
URL: http://svn.apache.org/viewvc/jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestManual.java?view=diff&rev=557176&r1=557175&r2=557176
==============================================================================
--- jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestManual.java (original)
+++ jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TestManual.java Tue Jul 17 23:46:16 2007
@@ -91,16 +91,20 @@
     public void testReportedBug28912() throws Exception {
         Connection conn1 = getConnection();
         assertNotNull(conn1);
+        assertFalse(conn1.isClosed());
         conn1.close();        
-        
+
         Connection conn2 = getConnection();
         assertNotNull(conn2);
-        
-        try {
-            conn1.close();
-            fail("Expected SQLException");
-        }
-        catch (SQLException e) { }
+
+        assertTrue(conn1.isClosed());
+        assertFalse(conn2.isClosed());
+
+        // should be able to call close multiple times with no effect
+        conn1.close();
+
+        assertTrue(conn1.isClosed());
+        assertFalse(conn2.isClosed());
     }
     
     /** @see http://issues.apache.org/bugzilla/show_bug.cgi?id=12400 */

Modified: jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterResultSet.java
URL: http://svn.apache.org/viewvc/jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterResultSet.java?view=diff&rev=557176&r1=557175&r2=557176
==============================================================================
--- jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterResultSet.java (original)
+++ jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterResultSet.java Tue Jul 17 23:46:16 2007
@@ -79,7 +79,9 @@
     }
 
     public void close() throws SQLException {
-        checkOpen();
+        if (!_open) {
+            return;
+        }
         ((TesterStatement)_statement)._resultSet = null;
         _open = false;
     }

Modified: jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterStatement.java
URL: http://svn.apache.org/viewvc/jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterStatement.java?view=diff&rev=557176&r1=557175&r2=557176
==============================================================================
--- jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterStatement.java (original)
+++ jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/TesterStatement.java Tue Jul 17 23:46:16 2007
@@ -79,7 +79,11 @@
     }
 
     public void close() throws SQLException {
-        checkOpen();
+        // calling close twice has no effect
+        if (!_open) {
+            return;
+        }
+
         _open = false;
         if (_resultSet != null) {
             _resultSet.close();

Modified: jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/managed/TestManagedDataSource.java
URL: http://svn.apache.org/viewvc/jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/managed/TestManagedDataSource.java?view=diff&rev=557176&r1=557175&r2=557176
==============================================================================
--- jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/managed/TestManagedDataSource.java (original)
+++ jakarta/commons/proper/dbcp/trunk/src/test/org/apache/commons/dbcp/managed/TestManagedDataSource.java Tue Jul 17 23:46:16 2007
@@ -126,28 +126,6 @@
         connectionB.close();
     }
 
-    public void testCantCloseConnectionTwice() throws Exception {
-        // this test is invalid... the JavaDoc and spec for the close method specifically
-        // state that the close method on an already closed connection is a no-op
-    }
-
-
-    /**
-     * Verify the close method can be called multiple times on a single connection without
-     * an exception being thrown.
-     */
-    public void testCanCloseConnectionTwice() throws Exception {
-        for (int i = 0; i < getMaxActive(); i++) { // loop to show we *can* close again once we've borrowed it from the pool again
-            Connection conn = newConnection();
-            assertTrue(null != conn);
-            assertTrue(!conn.isClosed());
-            conn.close();
-            assertTrue(conn.isClosed());
-            conn.close();
-            assertTrue(conn.isClosed());
-        }
-    }
-
     public void testManagedConnectionEqualsSameDelegate() throws Exception {
         // Get a maximal set of connections from the pool
         Connection[] c = new Connection[getMaxActive()];

Modified: jakarta/commons/proper/dbcp/trunk/xdocs/changes.xml
URL: http://svn.apache.org/viewvc/jakarta/commons/proper/dbcp/trunk/xdocs/changes.xml?view=diff&rev=557176&r1=557175&r2=557176
==============================================================================
--- jakarta/commons/proper/dbcp/trunk/xdocs/changes.xml (original)
+++ jakarta/commons/proper/dbcp/trunk/xdocs/changes.xml Tue Jul 17 23:46:16 2007
@@ -60,6 +60,15 @@
         methods to create object pool, connection factory and datasource
         instance previously embedded in createDataSource.
       </action>
+      <action dev="psteitz" type="fix" issue="DBCP-233" due-to="Dain Sundstrom">
+        Changed behavior to allow Connection, Statement, PreparedStatement,
+        CallableStatement and ResultSet to be closed multiple times. The first
+        time close is called the resource is closed and any subsequent calls
+        have no effect. This behavior is required as per the JavaDocs for these
+        classes. Also added tests for closing all types multiple times and
+        updated any tests that incorrectly assert that a resource can not be
+        closed more then once.  Fixes DBCP-134 and DBCP-3.
+      </action>
     </release>
     <release version="1.2.2" date="2007-04-04"
       description="This is a maintenance release containing bug fixes


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org