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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
