You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by th...@apache.org on 2020/01/09 06:18:37 UTC
[commons-dbutils] 04/04: DBUTILS-143 Use try-with-resources for all
prepareConnection calls Remove closing of connection by private methods
that are wrapped in convience methods
This is an automated email from the ASF dual-hosted git repository.
thecarlhall pushed a commit to annotated tag 1.8-RC2
in repository https://gitbox.apache.org/repos/asf/commons-dbutils.git
commit 3dc5efb853a4a64176664384d460e28f198c6101
Author: Carl Hall <th...@apache.org>
AuthorDate: Wed Jan 8 22:14:42 2020 -0800
DBUTILS-143 Use try-with-resources for all prepareConnection calls
Remove closing of connection by private methods that are wrapped in convience methods
---
.../org/apache/commons/dbutils/QueryRunner.java | 129 +++++++--------------
.../apache/commons/dbutils/QueryRunnerTest.java | 11 +-
2 files changed, 44 insertions(+), 96 deletions(-)
diff --git a/src/main/java/org/apache/commons/dbutils/QueryRunner.java b/src/main/java/org/apache/commons/dbutils/QueryRunner.java
index f76ce19..6c062f1 100644
--- a/src/main/java/org/apache/commons/dbutils/QueryRunner.java
+++ b/src/main/java/org/apache/commons/dbutils/QueryRunner.java
@@ -146,9 +146,9 @@ public class QueryRunner extends AbstractQueryRunner {
* @since DbUtils 1.1
*/
public int[] batch(final String sql, final Object[][] params) throws SQLException {
- final Connection conn = this.prepareConnection();
-
- return this.batch(conn, true, sql, params);
+ try (final Connection conn = this.prepareConnection()) {
+ return this.batch(conn, true, sql, params);
+ }
}
/**
@@ -167,16 +167,10 @@ public class QueryRunner extends AbstractQueryRunner {
}
if (sql == null) {
- if (closeConn) {
- close(conn);
- }
throw new SQLException("Null SQL statement");
}
if (params == null) {
- if (closeConn) {
- close(conn);
- }
throw new SQLException("Null parameters. If parameters aren't need, pass an empty array.");
}
@@ -195,9 +189,6 @@ public class QueryRunner extends AbstractQueryRunner {
this.rethrow(e, sql, (Object[])params);
} finally {
close(stmt);
- if (closeConn) {
- close(conn);
- }
}
return rows;
@@ -282,9 +273,9 @@ public class QueryRunner extends AbstractQueryRunner {
*/
@Deprecated
public <T> T query(final String sql, final Object param, final ResultSetHandler<T> rsh) throws SQLException {
- final Connection conn = this.prepareConnection();
-
- return this.<T>query(conn, true, sql, rsh, param);
+ try (final Connection conn = this.prepareConnection()) {
+ return this.<T>query(conn, true, sql, rsh, param);
+ }
}
/**
@@ -305,9 +296,9 @@ public class QueryRunner extends AbstractQueryRunner {
*/
@Deprecated
public <T> T query(final String sql, final Object[] params, final ResultSetHandler<T> rsh) throws SQLException {
- final Connection conn = this.prepareConnection();
-
- return this.<T>query(conn, true, sql, rsh, params);
+ try (final Connection conn = this.prepareConnection()) {
+ return this.<T>query(conn, true, sql, rsh, params);
+ }
}
/**
@@ -324,9 +315,9 @@ public class QueryRunner extends AbstractQueryRunner {
* @throws SQLException if a database access error occurs
*/
public <T> T query(final String sql, final ResultSetHandler<T> rsh, final Object... params) throws SQLException {
- final Connection conn = this.prepareConnection();
-
- return this.<T>query(conn, true, sql, rsh, params);
+ try (final Connection conn = this.prepareConnection()) {
+ return this.<T>query(conn, true, sql, rsh, params);
+ }
}
/**
@@ -342,9 +333,9 @@ public class QueryRunner extends AbstractQueryRunner {
* @throws SQLException if a database access error occurs
*/
public <T> T query(final String sql, final ResultSetHandler<T> rsh) throws SQLException {
- final Connection conn = this.prepareConnection();
-
- return this.<T>query(conn, true, sql, rsh, (Object[]) null);
+ try (final Connection conn = this.prepareConnection()) {
+ return this.<T>query(conn, true, sql, rsh, (Object[]) null);
+ }
}
/**
@@ -364,16 +355,10 @@ public class QueryRunner extends AbstractQueryRunner {
}
if (sql == null) {
- if (closeConn) {
- close(conn);
- }
throw new SQLException("Null SQL statement");
}
if (rsh == null) {
- if (closeConn) {
- close(conn);
- }
throw new SQLException("Null ResultSetHandler");
}
@@ -399,9 +384,6 @@ public class QueryRunner extends AbstractQueryRunner {
} finally {
closeQuietly(rs);
closeQuietly(stmt);
- if (closeConn) {
- close(conn);
- }
}
return result;
@@ -459,9 +441,9 @@ public class QueryRunner extends AbstractQueryRunner {
* @return The number of rows updated.
*/
public int update(final String sql) throws SQLException {
- final Connection conn = this.prepareConnection();
-
- return this.update(conn, true, sql, (Object[]) null);
+ try (final Connection conn = this.prepareConnection()) {
+ return this.update(conn, true, sql, (Object[]) null);
+ }
}
/**
@@ -477,9 +459,9 @@ public class QueryRunner extends AbstractQueryRunner {
* @return The number of rows updated.
*/
public int update(final String sql, final Object param) throws SQLException {
- final Connection conn = this.prepareConnection();
-
- return this.update(conn, true, sql, param);
+ try (final Connection conn = this.prepareConnection()) {
+ return this.update(conn, true, sql, param);
+ }
}
/**
@@ -495,9 +477,9 @@ public class QueryRunner extends AbstractQueryRunner {
* @return The number of rows updated.
*/
public int update(final String sql, final Object... params) throws SQLException {
- final Connection conn = this.prepareConnection();
-
- return this.update(conn, true, sql, params);
+ try (final Connection conn = this.prepareConnection()) {
+ return this.update(conn, true, sql, params);
+ }
}
/**
@@ -516,9 +498,6 @@ public class QueryRunner extends AbstractQueryRunner {
}
if (sql == null) {
- if (closeConn) {
- close(conn);
- }
throw new SQLException("Null SQL statement");
}
@@ -541,9 +520,6 @@ public class QueryRunner extends AbstractQueryRunner {
} finally {
close(stmt);
- if (closeConn) {
- close(conn);
- }
}
return rows;
@@ -562,7 +538,9 @@ public class QueryRunner extends AbstractQueryRunner {
* @since 1.6
*/
public <T> T insert(final String sql, final ResultSetHandler<T> rsh) throws SQLException {
- return insert(this.prepareConnection(), true, sql, rsh, (Object[]) null);
+ try (final Connection conn = this.prepareConnection()) {
+ return insert(conn, true, sql, rsh, (Object[]) null);
+ }
}
/**
@@ -580,7 +558,9 @@ public class QueryRunner extends AbstractQueryRunner {
* @since 1.6
*/
public <T> T insert(final String sql, final ResultSetHandler<T> rsh, final Object... params) throws SQLException {
- return insert(this.prepareConnection(), true, sql, rsh, params);
+ try (final Connection conn = this.prepareConnection()) {
+ return insert(conn, true, sql, rsh, params);
+ }
}
/**
@@ -633,16 +613,10 @@ public class QueryRunner extends AbstractQueryRunner {
}
if (sql == null) {
- if (closeConn) {
- close(conn);
- }
throw new SQLException("Null SQL statement");
}
if (rsh == null) {
- if (closeConn) {
- close(conn);
- }
throw new SQLException("Null ResultSetHandler");
}
@@ -665,9 +639,6 @@ public class QueryRunner extends AbstractQueryRunner {
this.rethrow(e, sql, params);
} finally {
close(stmt);
- if (closeConn) {
- close(conn);
- }
}
return generatedKeys;
@@ -688,7 +659,9 @@ public class QueryRunner extends AbstractQueryRunner {
* @since 1.6
*/
public <T> T insertBatch(final String sql, final ResultSetHandler<T> rsh, final Object[][] params) throws SQLException {
- return insertBatch(this.prepareConnection(), true, sql, rsh, params);
+ try (final Connection conn = this.prepareConnection()) {
+ return insertBatch(conn, true, sql, rsh, params);
+ }
}
/**
@@ -726,16 +699,10 @@ public class QueryRunner extends AbstractQueryRunner {
}
if (sql == null) {
- if (closeConn) {
- close(conn);
- }
throw new SQLException("Null SQL statement");
}
if (params == null) {
- if (closeConn) {
- close(conn);
- }
throw new SQLException("Null parameters. If parameters aren't need, pass an empty array.");
}
@@ -756,9 +723,6 @@ public class QueryRunner extends AbstractQueryRunner {
this.rethrow(e, sql, (Object[])params);
} finally {
close(stmt);
- if (closeConn) {
- close(conn);
- }
}
return generatedKeys;
@@ -810,9 +774,9 @@ public class QueryRunner extends AbstractQueryRunner {
* @return The number of rows updated.
*/
public int execute(final String sql, final Object... params) throws SQLException {
- final Connection conn = this.prepareConnection();
-
- return this.execute(conn, true, sql, params);
+ try (final Connection conn = this.prepareConnection()) {
+ return this.execute(conn, true, sql, params);
+ }
}
/**
@@ -863,9 +827,9 @@ public class QueryRunner extends AbstractQueryRunner {
* @throws SQLException if a database access error occurs
*/
public <T> List<T> execute(final String sql, final ResultSetHandler<T> rsh, final Object... params) throws SQLException {
- final Connection conn = this.prepareConnection();
-
- return this.execute(conn, true, sql, rsh, params);
+ try (final Connection conn = this.prepareConnection()) {
+ return this.execute(conn, true, sql, rsh, params);
+ }
}
/**
@@ -885,9 +849,6 @@ public class QueryRunner extends AbstractQueryRunner {
}
if (sql == null) {
- if (closeConn) {
- close(conn);
- }
throw new SQLException("Null SQL statement");
}
@@ -906,9 +867,6 @@ public class QueryRunner extends AbstractQueryRunner {
} finally {
close(stmt);
- if (closeConn) {
- close(conn);
- }
}
return rows;
@@ -932,16 +890,10 @@ public class QueryRunner extends AbstractQueryRunner {
}
if (sql == null) {
- if (closeConn) {
- close(conn);
- }
throw new SQLException("Null SQL statement");
}
if (rsh == null) {
- if (closeConn) {
- close(conn);
- }
throw new SQLException("Null ResultSetHandler");
}
@@ -972,9 +924,6 @@ public class QueryRunner extends AbstractQueryRunner {
} finally {
close(stmt);
- if (closeConn) {
- close(conn);
- }
}
return results;
diff --git a/src/test/java/org/apache/commons/dbutils/QueryRunnerTest.java b/src/test/java/org/apache/commons/dbutils/QueryRunnerTest.java
index 2802c16..c010858 100644
--- a/src/test/java/org/apache/commons/dbutils/QueryRunnerTest.java
+++ b/src/test/java/org/apache/commons/dbutils/QueryRunnerTest.java
@@ -46,7 +46,6 @@ import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.mockito.Mock;
-import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
@@ -645,7 +644,7 @@ public class QueryRunnerTest {
verify(call, times(1)).execute();
verify(call, times(1)).close(); // make sure we closed the statement
- verify(conn, times(1)).close(); // make sure we do not close the connection
+ verify(conn, times(1)).close(); // make sure we closed the connection
// call the other variation of query
when(meta.getParameterCount()).thenReturn(0);
@@ -655,7 +654,7 @@ public class QueryRunnerTest {
verify(call, times(2)).execute();
verify(call, times(2)).close(); // make sure we closed the statement
- verify(conn, times(2)).close(); // make sure we do not close the connection
+ verify(conn, times(2)).close(); // make sure we closed the connection
// Test single OUT parameter
when(meta.getParameterCount()).thenReturn(1);
@@ -669,7 +668,7 @@ public class QueryRunnerTest {
verify(call, times(3)).execute();
verify(call, times(3)).close(); // make sure we closed the statement
- verify(conn, times(3)).close(); // make sure we do not close the connection
+ verify(conn, times(3)).close(); // make sure we closed the connection
// Test OUT parameters with IN parameters
when(meta.getParameterCount()).thenReturn(3);
@@ -682,7 +681,7 @@ public class QueryRunnerTest {
verify(call, times(4)).execute();
verify(call, times(4)).close(); // make sure we closed the statement
- verify(conn, times(4)).close(); // make sure we do not close the connection
+ verify(conn, times(4)).close(); // make sure we closed the connection
// Test INOUT parameters
when(meta.getParameterCount()).thenReturn(3);
@@ -699,7 +698,7 @@ public class QueryRunnerTest {
verify(call, times(5)).execute();
verify(call, times(5)).close(); // make sure we closed the statement
- verify(conn, times(5)).close(); // make sure we do not close the connection
+ verify(conn, times(5)).close(); // make sure we closed the connection
}
@Test
Re: [commons-dbutils] 04/04: DBUTILS-143 Use try-with-resources for
all prepareConnection calls Remove closing of connection by private methods
that are wrapped in convience methods
Posted by Carl Hall <ca...@gmail.com>.
Both?
I’ll have to ask for a bit of guidance since it looks like DbUtils is heading towards having an o.a.c.dbutils2 package with the 2.0 work from Bill. Should we land 1.8 on master (currently on release branch) then move it to a 1.x branch and replace master with the 2.0 work?
> On Jan 9, 2020, at 5:01 AM, Gary Gregory <ga...@gmail.com> wrote:
>
> Should this code go in master or a 1.x branch?
>
> Gary
>
> On Thu, Jan 9, 2020, 01:18 <th...@apache.org> wrote:
>
>> This is an automated email from the ASF dual-hosted git repository.
>>
>> thecarlhall pushed a commit to annotated tag 1.8-RC2
>> in repository https://gitbox.apache.org/repos/asf/commons-dbutils.git
>>
>> commit 3dc5efb853a4a64176664384d460e28f198c6101
>> Author: Carl Hall <th...@apache.org>
>> AuthorDate: Wed Jan 8 22:14:42 2020 -0800
>>
>> DBUTILS-143 Use try-with-resources for all prepareConnection calls
>> Remove closing of connection by private methods that are wrapped in
>> convience methods
>> ---
>> .../org/apache/commons/dbutils/QueryRunner.java | 129
>> +++++++--------------
>> .../apache/commons/dbutils/QueryRunnerTest.java | 11 +-
>> 2 files changed, 44 insertions(+), 96 deletions(-)
>>
>> diff --git a/src/main/java/org/apache/commons/dbutils/QueryRunner.java
>> b/src/main/java/org/apache/commons/dbutils/QueryRunner.java
>> index f76ce19..6c062f1 100644
>> --- a/src/main/java/org/apache/commons/dbutils/QueryRunner.java
>> +++ b/src/main/java/org/apache/commons/dbutils/QueryRunner.java
>> @@ -146,9 +146,9 @@ public class QueryRunner extends AbstractQueryRunner {
>> * @since DbUtils 1.1
>> */
>> public int[] batch(final String sql, final Object[][] params) throws
>> SQLException {
>> - final Connection conn = this.prepareConnection();
>> -
>> - return this.batch(conn, true, sql, params);
>> + try (final Connection conn = this.prepareConnection()) {
>> + return this.batch(conn, true, sql, params);
>> + }
>> }
>>
>> /**
>> @@ -167,16 +167,10 @@ public class QueryRunner extends AbstractQueryRunner
>> {
>> }
>>
>> if (sql == null) {
>> - if (closeConn) {
>> - close(conn);
>> - }
>> throw new SQLException("Null SQL statement");
>> }
>>
>> if (params == null) {
>> - if (closeConn) {
>> - close(conn);
>> - }
>> throw new SQLException("Null parameters. If parameters aren't
>> need, pass an empty array.");
>> }
>>
>> @@ -195,9 +189,6 @@ public class QueryRunner extends AbstractQueryRunner {
>> this.rethrow(e, sql, (Object[])params);
>> } finally {
>> close(stmt);
>> - if (closeConn) {
>> - close(conn);
>> - }
>> }
>>
>> return rows;
>> @@ -282,9 +273,9 @@ public class QueryRunner extends AbstractQueryRunner {
>> */
>> @Deprecated
>> public <T> T query(final String sql, final Object param, final
>> ResultSetHandler<T> rsh) throws SQLException {
>> - final Connection conn = this.prepareConnection();
>> -
>> - return this.<T>query(conn, true, sql, rsh, param);
>> + try (final Connection conn = this.prepareConnection()) {
>> + return this.<T>query(conn, true, sql, rsh, param);
>> + }
>> }
>>
>> /**
>> @@ -305,9 +296,9 @@ public class QueryRunner extends AbstractQueryRunner {
>> */
>> @Deprecated
>> public <T> T query(final String sql, final Object[] params, final
>> ResultSetHandler<T> rsh) throws SQLException {
>> - final Connection conn = this.prepareConnection();
>> -
>> - return this.<T>query(conn, true, sql, rsh, params);
>> + try (final Connection conn = this.prepareConnection()) {
>> + return this.<T>query(conn, true, sql, rsh, params);
>> + }
>> }
>>
>> /**
>> @@ -324,9 +315,9 @@ public class QueryRunner extends AbstractQueryRunner {
>> * @throws SQLException if a database access error occurs
>> */
>> public <T> T query(final String sql, final ResultSetHandler<T> rsh,
>> final Object... params) throws SQLException {
>> - final Connection conn = this.prepareConnection();
>> -
>> - return this.<T>query(conn, true, sql, rsh, params);
>> + try (final Connection conn = this.prepareConnection()) {
>> + return this.<T>query(conn, true, sql, rsh, params);
>> + }
>> }
>>
>> /**
>> @@ -342,9 +333,9 @@ public class QueryRunner extends AbstractQueryRunner {
>> * @throws SQLException if a database access error occurs
>> */
>> public <T> T query(final String sql, final ResultSetHandler<T> rsh)
>> throws SQLException {
>> - final Connection conn = this.prepareConnection();
>> -
>> - return this.<T>query(conn, true, sql, rsh, (Object[]) null);
>> + try (final Connection conn = this.prepareConnection()) {
>> + return this.<T>query(conn, true, sql, rsh, (Object[]) null);
>> + }
>> }
>>
>> /**
>> @@ -364,16 +355,10 @@ public class QueryRunner extends AbstractQueryRunner
>> {
>> }
>>
>> if (sql == null) {
>> - if (closeConn) {
>> - close(conn);
>> - }
>> throw new SQLException("Null SQL statement");
>> }
>>
>> if (rsh == null) {
>> - if (closeConn) {
>> - close(conn);
>> - }
>> throw new SQLException("Null ResultSetHandler");
>> }
>>
>> @@ -399,9 +384,6 @@ public class QueryRunner extends AbstractQueryRunner {
>> } finally {
>> closeQuietly(rs);
>> closeQuietly(stmt);
>> - if (closeConn) {
>> - close(conn);
>> - }
>> }
>>
>> return result;
>> @@ -459,9 +441,9 @@ public class QueryRunner extends AbstractQueryRunner {
>> * @return The number of rows updated.
>> */
>> public int update(final String sql) throws SQLException {
>> - final Connection conn = this.prepareConnection();
>> -
>> - return this.update(conn, true, sql, (Object[]) null);
>> + try (final Connection conn = this.prepareConnection()) {
>> + return this.update(conn, true, sql, (Object[]) null);
>> + }
>> }
>>
>> /**
>> @@ -477,9 +459,9 @@ public class QueryRunner extends AbstractQueryRunner {
>> * @return The number of rows updated.
>> */
>> public int update(final String sql, final Object param) throws
>> SQLException {
>> - final Connection conn = this.prepareConnection();
>> -
>> - return this.update(conn, true, sql, param);
>> + try (final Connection conn = this.prepareConnection()) {
>> + return this.update(conn, true, sql, param);
>> + }
>> }
>>
>> /**
>> @@ -495,9 +477,9 @@ public class QueryRunner extends AbstractQueryRunner {
>> * @return The number of rows updated.
>> */
>> public int update(final String sql, final Object... params) throws
>> SQLException {
>> - final Connection conn = this.prepareConnection();
>> -
>> - return this.update(conn, true, sql, params);
>> + try (final Connection conn = this.prepareConnection()) {
>> + return this.update(conn, true, sql, params);
>> + }
>> }
>>
>> /**
>> @@ -516,9 +498,6 @@ public class QueryRunner extends AbstractQueryRunner {
>> }
>>
>> if (sql == null) {
>> - if (closeConn) {
>> - close(conn);
>> - }
>> throw new SQLException("Null SQL statement");
>> }
>>
>> @@ -541,9 +520,6 @@ public class QueryRunner extends AbstractQueryRunner {
>>
>> } finally {
>> close(stmt);
>> - if (closeConn) {
>> - close(conn);
>> - }
>> }
>>
>> return rows;
>> @@ -562,7 +538,9 @@ public class QueryRunner extends AbstractQueryRunner {
>> * @since 1.6
>> */
>> public <T> T insert(final String sql, final ResultSetHandler<T> rsh)
>> throws SQLException {
>> - return insert(this.prepareConnection(), true, sql, rsh,
>> (Object[]) null);
>> + try (final Connection conn = this.prepareConnection()) {
>> + return insert(conn, true, sql, rsh, (Object[]) null);
>> + }
>> }
>>
>> /**
>> @@ -580,7 +558,9 @@ public class QueryRunner extends AbstractQueryRunner {
>> * @since 1.6
>> */
>> public <T> T insert(final String sql, final ResultSetHandler<T> rsh,
>> final Object... params) throws SQLException {
>> - return insert(this.prepareConnection(), true, sql, rsh, params);
>> + try (final Connection conn = this.prepareConnection()) {
>> + return insert(conn, true, sql, rsh, params);
>> + }
>> }
>>
>> /**
>> @@ -633,16 +613,10 @@ public class QueryRunner extends AbstractQueryRunner
>> {
>> }
>>
>> if (sql == null) {
>> - if (closeConn) {
>> - close(conn);
>> - }
>> throw new SQLException("Null SQL statement");
>> }
>>
>> if (rsh == null) {
>> - if (closeConn) {
>> - close(conn);
>> - }
>> throw new SQLException("Null ResultSetHandler");
>> }
>>
>> @@ -665,9 +639,6 @@ public class QueryRunner extends AbstractQueryRunner {
>> this.rethrow(e, sql, params);
>> } finally {
>> close(stmt);
>> - if (closeConn) {
>> - close(conn);
>> - }
>> }
>>
>> return generatedKeys;
>> @@ -688,7 +659,9 @@ public class QueryRunner extends AbstractQueryRunner {
>> * @since 1.6
>> */
>> public <T> T insertBatch(final String sql, final ResultSetHandler<T>
>> rsh, final Object[][] params) throws SQLException {
>> - return insertBatch(this.prepareConnection(), true, sql, rsh,
>> params);
>> + try (final Connection conn = this.prepareConnection()) {
>> + return insertBatch(conn, true, sql, rsh, params);
>> + }
>> }
>>
>> /**
>> @@ -726,16 +699,10 @@ public class QueryRunner extends AbstractQueryRunner
>> {
>> }
>>
>> if (sql == null) {
>> - if (closeConn) {
>> - close(conn);
>> - }
>> throw new SQLException("Null SQL statement");
>> }
>>
>> if (params == null) {
>> - if (closeConn) {
>> - close(conn);
>> - }
>> throw new SQLException("Null parameters. If parameters aren't
>> need, pass an empty array.");
>> }
>>
>> @@ -756,9 +723,6 @@ public class QueryRunner extends AbstractQueryRunner {
>> this.rethrow(e, sql, (Object[])params);
>> } finally {
>> close(stmt);
>> - if (closeConn) {
>> - close(conn);
>> - }
>> }
>>
>> return generatedKeys;
>> @@ -810,9 +774,9 @@ public class QueryRunner extends AbstractQueryRunner {
>> * @return The number of rows updated.
>> */
>> public int execute(final String sql, final Object... params) throws
>> SQLException {
>> - final Connection conn = this.prepareConnection();
>> -
>> - return this.execute(conn, true, sql, params);
>> + try (final Connection conn = this.prepareConnection()) {
>> + return this.execute(conn, true, sql, params);
>> + }
>> }
>>
>> /**
>> @@ -863,9 +827,9 @@ public class QueryRunner extends AbstractQueryRunner {
>> * @throws SQLException if a database access error occurs
>> */
>> public <T> List<T> execute(final String sql, final
>> ResultSetHandler<T> rsh, final Object... params) throws SQLException {
>> - final Connection conn = this.prepareConnection();
>> -
>> - return this.execute(conn, true, sql, rsh, params);
>> + try (final Connection conn = this.prepareConnection()) {
>> + return this.execute(conn, true, sql, rsh, params);
>> + }
>> }
>>
>> /**
>> @@ -885,9 +849,6 @@ public class QueryRunner extends AbstractQueryRunner {
>> }
>>
>> if (sql == null) {
>> - if (closeConn) {
>> - close(conn);
>> - }
>> throw new SQLException("Null SQL statement");
>> }
>>
>> @@ -906,9 +867,6 @@ public class QueryRunner extends AbstractQueryRunner {
>>
>> } finally {
>> close(stmt);
>> - if (closeConn) {
>> - close(conn);
>> - }
>> }
>>
>> return rows;
>> @@ -932,16 +890,10 @@ public class QueryRunner extends AbstractQueryRunner
>> {
>> }
>>
>> if (sql == null) {
>> - if (closeConn) {
>> - close(conn);
>> - }
>> throw new SQLException("Null SQL statement");
>> }
>>
>> if (rsh == null) {
>> - if (closeConn) {
>> - close(conn);
>> - }
>> throw new SQLException("Null ResultSetHandler");
>> }
>>
>> @@ -972,9 +924,6 @@ public class QueryRunner extends AbstractQueryRunner {
>>
>> } finally {
>> close(stmt);
>> - if (closeConn) {
>> - close(conn);
>> - }
>> }
>>
>> return results;
>> diff --git a/src/test/java/org/apache/commons/dbutils/QueryRunnerTest.java
>> b/src/test/java/org/apache/commons/dbutils/QueryRunnerTest.java
>> index 2802c16..c010858 100644
>> --- a/src/test/java/org/apache/commons/dbutils/QueryRunnerTest.java
>> +++ b/src/test/java/org/apache/commons/dbutils/QueryRunnerTest.java
>> @@ -46,7 +46,6 @@ import org.junit.Assert;
>> import org.junit.Before;
>> import org.junit.Test;
>> import org.mockito.Mock;
>> -import org.mockito.Mockito;
>> import org.mockito.MockitoAnnotations;
>> import org.mockito.invocation.InvocationOnMock;
>> import org.mockito.stubbing.Answer;
>> @@ -645,7 +644,7 @@ public class QueryRunnerTest {
>>
>> verify(call, times(1)).execute();
>> verify(call, times(1)).close(); // make sure we closed the
>> statement
>> - verify(conn, times(1)).close(); // make sure we do not close
>> the connection
>> + verify(conn, times(1)).close(); // make sure we closed the
>> connection
>>
>> // call the other variation of query
>> when(meta.getParameterCount()).thenReturn(0);
>> @@ -655,7 +654,7 @@ public class QueryRunnerTest {
>>
>> verify(call, times(2)).execute();
>> verify(call, times(2)).close(); // make sure we closed the
>> statement
>> - verify(conn, times(2)).close(); // make sure we do not close
>> the connection
>> + verify(conn, times(2)).close(); // make sure we closed the
>> connection
>>
>> // Test single OUT parameter
>> when(meta.getParameterCount()).thenReturn(1);
>> @@ -669,7 +668,7 @@ public class QueryRunnerTest {
>>
>> verify(call, times(3)).execute();
>> verify(call, times(3)).close(); // make sure we closed the
>> statement
>> - verify(conn, times(3)).close(); // make sure we do not close
>> the connection
>> + verify(conn, times(3)).close(); // make sure we closed the
>> connection
>>
>> // Test OUT parameters with IN parameters
>> when(meta.getParameterCount()).thenReturn(3);
>> @@ -682,7 +681,7 @@ public class QueryRunnerTest {
>>
>> verify(call, times(4)).execute();
>> verify(call, times(4)).close(); // make sure we closed the
>> statement
>> - verify(conn, times(4)).close(); // make sure we do not close
>> the connection
>> + verify(conn, times(4)).close(); // make sure we closed the
>> connection
>>
>> // Test INOUT parameters
>> when(meta.getParameterCount()).thenReturn(3);
>> @@ -699,7 +698,7 @@ public class QueryRunnerTest {
>>
>> verify(call, times(5)).execute();
>> verify(call, times(5)).close(); // make sure we closed the
>> statement
>> - verify(conn, times(5)).close(); // make sure we do not close
>> the connection
>> + verify(conn, times(5)).close(); // make sure we closed the
>> connection
>> }
>>
>> @Test
>>
>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [commons-dbutils] 04/04: DBUTILS-143 Use try-with-resources for
all prepareConnection calls Remove closing of connection by private methods
that are wrapped in convience methods
Posted by Gary Gregory <ga...@gmail.com>.
Should this code go in master or a 1.x branch?
Gary
On Thu, Jan 9, 2020, 01:18 <th...@apache.org> wrote:
> This is an automated email from the ASF dual-hosted git repository.
>
> thecarlhall pushed a commit to annotated tag 1.8-RC2
> in repository https://gitbox.apache.org/repos/asf/commons-dbutils.git
>
> commit 3dc5efb853a4a64176664384d460e28f198c6101
> Author: Carl Hall <th...@apache.org>
> AuthorDate: Wed Jan 8 22:14:42 2020 -0800
>
> DBUTILS-143 Use try-with-resources for all prepareConnection calls
> Remove closing of connection by private methods that are wrapped in
> convience methods
> ---
> .../org/apache/commons/dbutils/QueryRunner.java | 129
> +++++++--------------
> .../apache/commons/dbutils/QueryRunnerTest.java | 11 +-
> 2 files changed, 44 insertions(+), 96 deletions(-)
>
> diff --git a/src/main/java/org/apache/commons/dbutils/QueryRunner.java
> b/src/main/java/org/apache/commons/dbutils/QueryRunner.java
> index f76ce19..6c062f1 100644
> --- a/src/main/java/org/apache/commons/dbutils/QueryRunner.java
> +++ b/src/main/java/org/apache/commons/dbutils/QueryRunner.java
> @@ -146,9 +146,9 @@ public class QueryRunner extends AbstractQueryRunner {
> * @since DbUtils 1.1
> */
> public int[] batch(final String sql, final Object[][] params) throws
> SQLException {
> - final Connection conn = this.prepareConnection();
> -
> - return this.batch(conn, true, sql, params);
> + try (final Connection conn = this.prepareConnection()) {
> + return this.batch(conn, true, sql, params);
> + }
> }
>
> /**
> @@ -167,16 +167,10 @@ public class QueryRunner extends AbstractQueryRunner
> {
> }
>
> if (sql == null) {
> - if (closeConn) {
> - close(conn);
> - }
> throw new SQLException("Null SQL statement");
> }
>
> if (params == null) {
> - if (closeConn) {
> - close(conn);
> - }
> throw new SQLException("Null parameters. If parameters aren't
> need, pass an empty array.");
> }
>
> @@ -195,9 +189,6 @@ public class QueryRunner extends AbstractQueryRunner {
> this.rethrow(e, sql, (Object[])params);
> } finally {
> close(stmt);
> - if (closeConn) {
> - close(conn);
> - }
> }
>
> return rows;
> @@ -282,9 +273,9 @@ public class QueryRunner extends AbstractQueryRunner {
> */
> @Deprecated
> public <T> T query(final String sql, final Object param, final
> ResultSetHandler<T> rsh) throws SQLException {
> - final Connection conn = this.prepareConnection();
> -
> - return this.<T>query(conn, true, sql, rsh, param);
> + try (final Connection conn = this.prepareConnection()) {
> + return this.<T>query(conn, true, sql, rsh, param);
> + }
> }
>
> /**
> @@ -305,9 +296,9 @@ public class QueryRunner extends AbstractQueryRunner {
> */
> @Deprecated
> public <T> T query(final String sql, final Object[] params, final
> ResultSetHandler<T> rsh) throws SQLException {
> - final Connection conn = this.prepareConnection();
> -
> - return this.<T>query(conn, true, sql, rsh, params);
> + try (final Connection conn = this.prepareConnection()) {
> + return this.<T>query(conn, true, sql, rsh, params);
> + }
> }
>
> /**
> @@ -324,9 +315,9 @@ public class QueryRunner extends AbstractQueryRunner {
> * @throws SQLException if a database access error occurs
> */
> public <T> T query(final String sql, final ResultSetHandler<T> rsh,
> final Object... params) throws SQLException {
> - final Connection conn = this.prepareConnection();
> -
> - return this.<T>query(conn, true, sql, rsh, params);
> + try (final Connection conn = this.prepareConnection()) {
> + return this.<T>query(conn, true, sql, rsh, params);
> + }
> }
>
> /**
> @@ -342,9 +333,9 @@ public class QueryRunner extends AbstractQueryRunner {
> * @throws SQLException if a database access error occurs
> */
> public <T> T query(final String sql, final ResultSetHandler<T> rsh)
> throws SQLException {
> - final Connection conn = this.prepareConnection();
> -
> - return this.<T>query(conn, true, sql, rsh, (Object[]) null);
> + try (final Connection conn = this.prepareConnection()) {
> + return this.<T>query(conn, true, sql, rsh, (Object[]) null);
> + }
> }
>
> /**
> @@ -364,16 +355,10 @@ public class QueryRunner extends AbstractQueryRunner
> {
> }
>
> if (sql == null) {
> - if (closeConn) {
> - close(conn);
> - }
> throw new SQLException("Null SQL statement");
> }
>
> if (rsh == null) {
> - if (closeConn) {
> - close(conn);
> - }
> throw new SQLException("Null ResultSetHandler");
> }
>
> @@ -399,9 +384,6 @@ public class QueryRunner extends AbstractQueryRunner {
> } finally {
> closeQuietly(rs);
> closeQuietly(stmt);
> - if (closeConn) {
> - close(conn);
> - }
> }
>
> return result;
> @@ -459,9 +441,9 @@ public class QueryRunner extends AbstractQueryRunner {
> * @return The number of rows updated.
> */
> public int update(final String sql) throws SQLException {
> - final Connection conn = this.prepareConnection();
> -
> - return this.update(conn, true, sql, (Object[]) null);
> + try (final Connection conn = this.prepareConnection()) {
> + return this.update(conn, true, sql, (Object[]) null);
> + }
> }
>
> /**
> @@ -477,9 +459,9 @@ public class QueryRunner extends AbstractQueryRunner {
> * @return The number of rows updated.
> */
> public int update(final String sql, final Object param) throws
> SQLException {
> - final Connection conn = this.prepareConnection();
> -
> - return this.update(conn, true, sql, param);
> + try (final Connection conn = this.prepareConnection()) {
> + return this.update(conn, true, sql, param);
> + }
> }
>
> /**
> @@ -495,9 +477,9 @@ public class QueryRunner extends AbstractQueryRunner {
> * @return The number of rows updated.
> */
> public int update(final String sql, final Object... params) throws
> SQLException {
> - final Connection conn = this.prepareConnection();
> -
> - return this.update(conn, true, sql, params);
> + try (final Connection conn = this.prepareConnection()) {
> + return this.update(conn, true, sql, params);
> + }
> }
>
> /**
> @@ -516,9 +498,6 @@ public class QueryRunner extends AbstractQueryRunner {
> }
>
> if (sql == null) {
> - if (closeConn) {
> - close(conn);
> - }
> throw new SQLException("Null SQL statement");
> }
>
> @@ -541,9 +520,6 @@ public class QueryRunner extends AbstractQueryRunner {
>
> } finally {
> close(stmt);
> - if (closeConn) {
> - close(conn);
> - }
> }
>
> return rows;
> @@ -562,7 +538,9 @@ public class QueryRunner extends AbstractQueryRunner {
> * @since 1.6
> */
> public <T> T insert(final String sql, final ResultSetHandler<T> rsh)
> throws SQLException {
> - return insert(this.prepareConnection(), true, sql, rsh,
> (Object[]) null);
> + try (final Connection conn = this.prepareConnection()) {
> + return insert(conn, true, sql, rsh, (Object[]) null);
> + }
> }
>
> /**
> @@ -580,7 +558,9 @@ public class QueryRunner extends AbstractQueryRunner {
> * @since 1.6
> */
> public <T> T insert(final String sql, final ResultSetHandler<T> rsh,
> final Object... params) throws SQLException {
> - return insert(this.prepareConnection(), true, sql, rsh, params);
> + try (final Connection conn = this.prepareConnection()) {
> + return insert(conn, true, sql, rsh, params);
> + }
> }
>
> /**
> @@ -633,16 +613,10 @@ public class QueryRunner extends AbstractQueryRunner
> {
> }
>
> if (sql == null) {
> - if (closeConn) {
> - close(conn);
> - }
> throw new SQLException("Null SQL statement");
> }
>
> if (rsh == null) {
> - if (closeConn) {
> - close(conn);
> - }
> throw new SQLException("Null ResultSetHandler");
> }
>
> @@ -665,9 +639,6 @@ public class QueryRunner extends AbstractQueryRunner {
> this.rethrow(e, sql, params);
> } finally {
> close(stmt);
> - if (closeConn) {
> - close(conn);
> - }
> }
>
> return generatedKeys;
> @@ -688,7 +659,9 @@ public class QueryRunner extends AbstractQueryRunner {
> * @since 1.6
> */
> public <T> T insertBatch(final String sql, final ResultSetHandler<T>
> rsh, final Object[][] params) throws SQLException {
> - return insertBatch(this.prepareConnection(), true, sql, rsh,
> params);
> + try (final Connection conn = this.prepareConnection()) {
> + return insertBatch(conn, true, sql, rsh, params);
> + }
> }
>
> /**
> @@ -726,16 +699,10 @@ public class QueryRunner extends AbstractQueryRunner
> {
> }
>
> if (sql == null) {
> - if (closeConn) {
> - close(conn);
> - }
> throw new SQLException("Null SQL statement");
> }
>
> if (params == null) {
> - if (closeConn) {
> - close(conn);
> - }
> throw new SQLException("Null parameters. If parameters aren't
> need, pass an empty array.");
> }
>
> @@ -756,9 +723,6 @@ public class QueryRunner extends AbstractQueryRunner {
> this.rethrow(e, sql, (Object[])params);
> } finally {
> close(stmt);
> - if (closeConn) {
> - close(conn);
> - }
> }
>
> return generatedKeys;
> @@ -810,9 +774,9 @@ public class QueryRunner extends AbstractQueryRunner {
> * @return The number of rows updated.
> */
> public int execute(final String sql, final Object... params) throws
> SQLException {
> - final Connection conn = this.prepareConnection();
> -
> - return this.execute(conn, true, sql, params);
> + try (final Connection conn = this.prepareConnection()) {
> + return this.execute(conn, true, sql, params);
> + }
> }
>
> /**
> @@ -863,9 +827,9 @@ public class QueryRunner extends AbstractQueryRunner {
> * @throws SQLException if a database access error occurs
> */
> public <T> List<T> execute(final String sql, final
> ResultSetHandler<T> rsh, final Object... params) throws SQLException {
> - final Connection conn = this.prepareConnection();
> -
> - return this.execute(conn, true, sql, rsh, params);
> + try (final Connection conn = this.prepareConnection()) {
> + return this.execute(conn, true, sql, rsh, params);
> + }
> }
>
> /**
> @@ -885,9 +849,6 @@ public class QueryRunner extends AbstractQueryRunner {
> }
>
> if (sql == null) {
> - if (closeConn) {
> - close(conn);
> - }
> throw new SQLException("Null SQL statement");
> }
>
> @@ -906,9 +867,6 @@ public class QueryRunner extends AbstractQueryRunner {
>
> } finally {
> close(stmt);
> - if (closeConn) {
> - close(conn);
> - }
> }
>
> return rows;
> @@ -932,16 +890,10 @@ public class QueryRunner extends AbstractQueryRunner
> {
> }
>
> if (sql == null) {
> - if (closeConn) {
> - close(conn);
> - }
> throw new SQLException("Null SQL statement");
> }
>
> if (rsh == null) {
> - if (closeConn) {
> - close(conn);
> - }
> throw new SQLException("Null ResultSetHandler");
> }
>
> @@ -972,9 +924,6 @@ public class QueryRunner extends AbstractQueryRunner {
>
> } finally {
> close(stmt);
> - if (closeConn) {
> - close(conn);
> - }
> }
>
> return results;
> diff --git a/src/test/java/org/apache/commons/dbutils/QueryRunnerTest.java
> b/src/test/java/org/apache/commons/dbutils/QueryRunnerTest.java
> index 2802c16..c010858 100644
> --- a/src/test/java/org/apache/commons/dbutils/QueryRunnerTest.java
> +++ b/src/test/java/org/apache/commons/dbutils/QueryRunnerTest.java
> @@ -46,7 +46,6 @@ import org.junit.Assert;
> import org.junit.Before;
> import org.junit.Test;
> import org.mockito.Mock;
> -import org.mockito.Mockito;
> import org.mockito.MockitoAnnotations;
> import org.mockito.invocation.InvocationOnMock;
> import org.mockito.stubbing.Answer;
> @@ -645,7 +644,7 @@ public class QueryRunnerTest {
>
> verify(call, times(1)).execute();
> verify(call, times(1)).close(); // make sure we closed the
> statement
> - verify(conn, times(1)).close(); // make sure we do not close
> the connection
> + verify(conn, times(1)).close(); // make sure we closed the
> connection
>
> // call the other variation of query
> when(meta.getParameterCount()).thenReturn(0);
> @@ -655,7 +654,7 @@ public class QueryRunnerTest {
>
> verify(call, times(2)).execute();
> verify(call, times(2)).close(); // make sure we closed the
> statement
> - verify(conn, times(2)).close(); // make sure we do not close
> the connection
> + verify(conn, times(2)).close(); // make sure we closed the
> connection
>
> // Test single OUT parameter
> when(meta.getParameterCount()).thenReturn(1);
> @@ -669,7 +668,7 @@ public class QueryRunnerTest {
>
> verify(call, times(3)).execute();
> verify(call, times(3)).close(); // make sure we closed the
> statement
> - verify(conn, times(3)).close(); // make sure we do not close
> the connection
> + verify(conn, times(3)).close(); // make sure we closed the
> connection
>
> // Test OUT parameters with IN parameters
> when(meta.getParameterCount()).thenReturn(3);
> @@ -682,7 +681,7 @@ public class QueryRunnerTest {
>
> verify(call, times(4)).execute();
> verify(call, times(4)).close(); // make sure we closed the
> statement
> - verify(conn, times(4)).close(); // make sure we do not close
> the connection
> + verify(conn, times(4)).close(); // make sure we closed the
> connection
>
> // Test INOUT parameters
> when(meta.getParameterCount()).thenReturn(3);
> @@ -699,7 +698,7 @@ public class QueryRunnerTest {
>
> verify(call, times(5)).execute();
> verify(call, times(5)).close(); // make sure we closed the
> statement
> - verify(conn, times(5)).close(); // make sure we do not close
> the connection
> + verify(conn, times(5)).close(); // make sure we closed the
> connection
> }
>
> @Test
>
>