Should this code go in master or a 1.x branch? Gary
On Thu, Jan 9, 2020, 01:18 <thecarlh...@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 <thecarlh...@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 > >
