From c7feec1ae99ea891d73269a1cd237251b032d2d0 Mon Sep 17 00:00:00 2001 From: kaklakariada Date: Mon, 6 Jan 2025 20:34:10 +0100 Subject: [PATCH] Close statement when closing result set --- CHANGELOG.md | 1 + .../itsallcode/jdbc/ConnectionWrapper.java | 2 - .../jdbc/SimplePreparedStatement.java | 2 +- .../org/itsallcode/jdbc/SimpleStatement.java | 2 +- .../jdbc/resultset/SimpleResultSet.java | 14 ++++++- .../jdbc/SimplePreparedStatementTest.java | 9 +++++ .../itsallcode/jdbc/SimpleStatementTest.java | 8 ++++ .../jdbc/resultset/SimpleResultSetTest.java | 38 ++++++++++++++++--- 8 files changed, 65 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23f26f4..4993b25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - [PR #43](https://github.com/itsallcode/simple-jdbc/pull/43): Allow direct access to `Connection` from `DbOperations` - [PR #44](https://github.com/itsallcode/simple-jdbc/pull/44): Add `GenericDialect` for unsupported databases - [PR #45](https://github.com/itsallcode/simple-jdbc/pull/45): Rename `executeStatement()` to `executeUpdate()` and return row count (**Breaking change**) +- [PR #46](https://github.com/itsallcode/simple-jdbc/pull/46): Close `Statement` / `PreparedStatement` when closing the result set. ## [0.9.0] - 2024-12-23 diff --git a/src/main/java/org/itsallcode/jdbc/ConnectionWrapper.java b/src/main/java/org/itsallcode/jdbc/ConnectionWrapper.java index aa852c2..14f6c9d 100644 --- a/src/main/java/org/itsallcode/jdbc/ConnectionWrapper.java +++ b/src/main/java/org/itsallcode/jdbc/ConnectionWrapper.java @@ -61,7 +61,6 @@ void executeScript(final String sqlScript) { SimpleResultSet query(final String sql) { LOG.finest(() -> "Executing query '" + sql + "'..."); final SimpleStatement statement = createSimpleStatement(); - // TODO: close statement when resultset is closed return statement.executeQuery(sql, ContextRowMapper.create(ContextRowMapper.generic(dialect))); } @@ -70,7 +69,6 @@ SimpleResultSet query(final String sql, final PreparedStatementSetter pre LOG.finest(() -> "Executing query '" + sql + "'..."); final SimplePreparedStatement statement = prepareStatement(sql); statement.setValues(preparedStatementSetter); - // TODO: close statement when resultset is closed return statement.executeQuery(ContextRowMapper.create(rowMapper)); } diff --git a/src/main/java/org/itsallcode/jdbc/SimplePreparedStatement.java b/src/main/java/org/itsallcode/jdbc/SimplePreparedStatement.java index 36d49eb..f8eb274 100644 --- a/src/main/java/org/itsallcode/jdbc/SimplePreparedStatement.java +++ b/src/main/java/org/itsallcode/jdbc/SimplePreparedStatement.java @@ -26,7 +26,7 @@ public class SimplePreparedStatement implements AutoCloseable { SimpleResultSet executeQuery(final ContextRowMapper rowMapper) { final ResultSet resultSet = doExecuteQuery(); final ResultSet convertingResultSet = ConvertingResultSet.create(dialect, resultSet); - return new SimpleResultSet<>(context, convertingResultSet, rowMapper); + return new SimpleResultSet<>(context, convertingResultSet, rowMapper, this); } private ResultSet doExecuteQuery() { diff --git a/src/main/java/org/itsallcode/jdbc/SimpleStatement.java b/src/main/java/org/itsallcode/jdbc/SimpleStatement.java index 69e9f5e..3cc2a3c 100644 --- a/src/main/java/org/itsallcode/jdbc/SimpleStatement.java +++ b/src/main/java/org/itsallcode/jdbc/SimpleStatement.java @@ -23,7 +23,7 @@ public class SimpleStatement implements AutoCloseable { SimpleResultSet executeQuery(final String sql, final ContextRowMapper rowMapper) { final ResultSet resultSet = doExecuteQuery(sql); final ResultSet convertingResultSet = ConvertingResultSet.create(dialect, resultSet); - return new SimpleResultSet<>(context, convertingResultSet, rowMapper); + return new SimpleResultSet<>(context, convertingResultSet, rowMapper, this); } private ResultSet doExecuteQuery(final String sql) { diff --git a/src/main/java/org/itsallcode/jdbc/resultset/SimpleResultSet.java b/src/main/java/org/itsallcode/jdbc/resultset/SimpleResultSet.java index a6665e9..a4131c2 100644 --- a/src/main/java/org/itsallcode/jdbc/resultset/SimpleResultSet.java +++ b/src/main/java/org/itsallcode/jdbc/resultset/SimpleResultSet.java @@ -19,6 +19,7 @@ public class SimpleResultSet implements AutoCloseable, Iterable { private final ResultSet resultSet; private final ContextRowMapper rowMapper; private final Context context; + private final AutoCloseable statement; private Iterator iterator; /** @@ -27,11 +28,15 @@ public class SimpleResultSet implements AutoCloseable, Iterable { * @param context database context * @param resultSet the underlying result set * @param rowMapper a row mapper for converting each row + * @param statement the statement that created the result set. This will be + * closed when the result set is closed. */ - public SimpleResultSet(final Context context, final ResultSet resultSet, final ContextRowMapper rowMapper) { + public SimpleResultSet(final Context context, final ResultSet resultSet, final ContextRowMapper rowMapper, + final AutoCloseable statement) { this.context = context; this.resultSet = resultSet; this.rowMapper = rowMapper; + this.statement = statement; } /** @@ -71,7 +76,7 @@ public Stream stream() { } /** - * Close the underlying {@link ResultSet}. + * Close the underlying {@link ResultSet} and the statement that created it. * * @throws UncheckedSQLException if closing fails. */ @@ -82,6 +87,11 @@ public void close() { } catch (final SQLException e) { throw new UncheckedSQLException("Error closing resultset", e); } + try { + statement.close(); + } catch (final Exception e) { + throw new IllegalStateException("Error closing statement: " + e.getMessage(), e); + } } private boolean next() { diff --git a/src/test/java/org/itsallcode/jdbc/SimplePreparedStatementTest.java b/src/test/java/org/itsallcode/jdbc/SimplePreparedStatementTest.java index dd0f22e..0bcf755 100644 --- a/src/test/java/org/itsallcode/jdbc/SimplePreparedStatementTest.java +++ b/src/test/java/org/itsallcode/jdbc/SimplePreparedStatementTest.java @@ -39,6 +39,15 @@ void executeQuery() throws SQLException { verify(statementMock).executeQuery(); } + @Test + void closingResultSetClosesStatement() throws SQLException { + when(statementMock.executeQuery()).thenReturn(resultSetMock); + when(resultSetMock.getMetaData()).thenReturn(metaDataMock); + testee().executeQuery(rowMapperMock).close(); + + verify(statementMock).close(); + } + @Test void executeQueryFails() throws SQLException { when(statementMock.executeQuery()).thenThrow(new SQLException("expected")); diff --git a/src/test/java/org/itsallcode/jdbc/SimpleStatementTest.java b/src/test/java/org/itsallcode/jdbc/SimpleStatementTest.java index a1926d5..27e6078 100644 --- a/src/test/java/org/itsallcode/jdbc/SimpleStatementTest.java +++ b/src/test/java/org/itsallcode/jdbc/SimpleStatementTest.java @@ -36,6 +36,14 @@ void executeQuery() throws SQLException { verify(stmtMock).executeQuery("sql"); } + @Test + void closingResultSetClosesStatement() throws SQLException { + when(stmtMock.executeQuery("sql")).thenReturn(resultSetMock); + when(resultSetMock.getMetaData()).thenReturn(metaDataMock); + testee().executeQuery("sql", rowMapperMock).close(); + verify(stmtMock).close(); + } + @Test void executeQueryFails() throws SQLException { when(stmtMock.executeQuery("sql")).thenThrow(new SQLException("expected")); diff --git a/src/test/java/org/itsallcode/jdbc/resultset/SimpleResultSetTest.java b/src/test/java/org/itsallcode/jdbc/resultset/SimpleResultSetTest.java index eae6939..f1dc267 100644 --- a/src/test/java/org/itsallcode/jdbc/resultset/SimpleResultSetTest.java +++ b/src/test/java/org/itsallcode/jdbc/resultset/SimpleResultSetTest.java @@ -5,8 +5,7 @@ import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.*; -import java.sql.ResultSet; -import java.sql.SQLException; +import java.sql.*; import org.itsallcode.jdbc.UncheckedSQLException; import org.junit.jupiter.api.Test; @@ -18,12 +17,14 @@ class SimpleResultSetTest { @Mock - private ResultSet resultSetMock; + ResultSet resultSetMock; @Mock - private ContextRowMapper rowMapper; + ContextRowMapper rowMapper; + @Mock + Statement statementMock; SimpleResultSet testee() { - return new SimpleResultSet<>(null, resultSetMock, rowMapper); + return new SimpleResultSet<>(null, resultSetMock, rowMapper, statementMock); } @Test @@ -69,18 +70,45 @@ void closeSucceeds() throws SQLException { verify(resultSetMock).close(); } + @Test + void closeClosesStatement() throws SQLException { + testee().close(); + verify(statementMock).close(); + } + + @Test + void closingStatementFails() throws SQLException { + doThrow(new SQLException("expected")).when(statementMock).close(); + final SimpleResultSet testee = testee(); + assertThatThrownBy(testee::close) + .isInstanceOf(IllegalStateException.class) + .hasMessage("Error closing statement: expected"); + } + @Test void streamClosesResultSet() throws SQLException { testee().stream().close(); verify(resultSetMock).close(); } + @Test + void streamClosesStatement() throws SQLException { + testee().stream().close(); + verify(statementMock).close(); + } + @Test void toListClosesResultSet() throws SQLException { testee().toList(); verify(resultSetMock).close(); } + @Test + void toListClosesStatement() throws SQLException { + testee().toList(); + verify(statementMock).close(); + } + private void simulateRowMapper() throws SQLException { when(rowMapper.mapRow(eq(null), same(resultSetMock), anyInt())) .thenAnswer(invocation -> new TestingRowType(invocation.getArgument(2, Integer.class)));