From ad106c93572b8aee7a2cc2502c875e29ffd6043e Mon Sep 17 00:00:00 2001 From: bercianor Date: Fri, 18 Sep 2026 17:19:29 +0200 Subject: [PATCH 1/2] fix(sql): close non-transactional execution connections --- .../store/sql/SqlAuditStoreTest.java | 47 +++++++++++----- .../targets/AbstractTargetSystem.java | 20 ++++++- .../targetsystem/sql/SqlTargetSystem.java | 11 ++++ .../SqlTargetSystemSharedTxManagerTest.java | 54 +++++++++++++++++++ 4 files changed, 118 insertions(+), 14 deletions(-) diff --git a/community/flamingock-sql-auditstore/src/test/java/io/flamingock/store/sql/SqlAuditStoreTest.java b/community/flamingock-sql-auditstore/src/test/java/io/flamingock/store/sql/SqlAuditStoreTest.java index 984fdc8ee..1d70b7d0c 100644 --- a/community/flamingock-sql-auditstore/src/test/java/io/flamingock/store/sql/SqlAuditStoreTest.java +++ b/community/flamingock-sql-auditstore/src/test/java/io/flamingock/store/sql/SqlAuditStoreTest.java @@ -94,6 +94,27 @@ void tearDown() throws SQLException { } } + @Test + @DisplayName("container-backed setup stops the container when datasource creation fails") + void containerBackedSetupStopsContainerWhenDataSourceCreationFails() { + JdbcDatabaseContainer container = org.mockito.Mockito.mock(JdbcDatabaseContainer.class); + RuntimeException setupFailure = new IllegalStateException("datasource creation failed"); + org.mockito.Mockito.when(container.isRunning()).thenReturn(true); + + try (MockedStatic helper = org.mockito.Mockito.mockStatic(SqlAuditTestHelper.class)) { + helper.when(() -> SqlAuditTestHelper.createContainer("informix")).thenReturn(container); + helper.when(() -> SqlAuditTestHelper.createDataSource(container)).thenThrow(setupFailure); + + RuntimeException thrown = assertThrows(RuntimeException.class, + () -> setupTest(SqlDialect.INFORMIX, "informix")); + + assertSame(setupFailure, thrown); + helper.verify(() -> SqlAuditTestHelper.createDataSource(container)); + org.mockito.Mockito.verify(container).start(); + org.mockito.Mockito.verify(container).stop(); + } + } + private TestContext setupTest(SqlDialect sqlDialect, String dialectName) throws SQLException { if ("h2".equals(dialectName)) { HikariConfig config = new HikariConfig(); @@ -145,22 +166,24 @@ private TestContext setupTest(SqlDialect sqlDialect, String dialectName) throws JdbcDatabaseContainer container = SqlAuditTestHelper.createContainer(dialectName); container.start(); - HikariConfig config = new HikariConfig(); - config.setJdbcUrl(container.getJdbcUrl()); - config.setUsername(container.getUsername()); - config.setPassword(container.getPassword()); - config.setDriverClassName(container.getDriverClassName()); - DataSource dataSource = new HikariDataSource(config); - TestContext testContext = new TestContext(dataSource, container, sqlDialect); - + DataSource dataSource = null; + TestContext testContext = null; try { + dataSource = SqlAuditTestHelper.createDataSource(container); + testContext = new TestContext(dataSource, container, sqlDialect); SqlAuditTestHelper.createTables(dataSource, sqlDialect); - } catch (SQLException exception) { - testContext.cleanup(); + return testContext; + } catch (SQLException | RuntimeException exception) { + TestContext cleanupContext = testContext != null + ? testContext + : new TestContext(dataSource, container, sqlDialect); + try { + cleanupContext.cleanup(); + } catch (SQLException cleanupFailure) { + exception.addSuppressed(cleanupFailure); + } throw exception; } - - return testContext; } private Class[] getChangeClasses(String dialectName, String scenario) { diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/external/targets/AbstractTargetSystem.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/external/targets/AbstractTargetSystem.java index 248143528..2c2bc04f5 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/external/targets/AbstractTargetSystem.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/external/targets/AbstractTargetSystem.java @@ -93,7 +93,11 @@ public String getId() { */ public final T applyChange(Function changeApplier, ExecutionRuntime executionRuntime) { enhanceExecutionRuntime(executionRuntime, false); - return changeApplier.apply(executionRuntime); + try { + return changeApplier.apply(executionRuntime); + } finally { + cleanupExecutionRuntime(executionRuntime); + } } /** @@ -110,10 +114,22 @@ public final T applyChange(Function changeApplier, Exec */ public final T rollbackChange(Function changeRollbacker, ExecutionRuntime executionRuntime) { enhanceExecutionRuntime(executionRuntime, false); - return changeRollbacker.apply(executionRuntime); + try { + return changeRollbacker.apply(executionRuntime); + } finally { + cleanupExecutionRuntime(executionRuntime); + } } + /** + * Hook for cleaning up session-scoped dependencies after non-transactional execution. + * + * @param executionRuntime the runtime whose session-scoped dependencies should be cleaned up + */ + protected void cleanupExecutionRuntime(RuntimeContext executionRuntime) { + } + /** * Hook for injecting session-scoped dependencies into the execution runtime. *

diff --git a/core/target-systems/flamingock-sql-targetsystem/src/main/java/io/flamingock/targetsystem/sql/SqlTargetSystem.java b/core/target-systems/flamingock-sql-targetsystem/src/main/java/io/flamingock/targetsystem/sql/SqlTargetSystem.java index 133b00f35..cc7a83108 100644 --- a/core/target-systems/flamingock-sql-targetsystem/src/main/java/io/flamingock/targetsystem/sql/SqlTargetSystem.java +++ b/core/target-systems/flamingock-sql-targetsystem/src/main/java/io/flamingock/targetsystem/sql/SqlTargetSystem.java @@ -92,6 +92,17 @@ protected void enhanceExecutionRuntime(RuntimeContext executionRuntime, boolean } + @Override + protected void cleanupExecutionRuntime(RuntimeContext executionRuntime) { + executionRuntime.getContext().getDependencyValue(Connection.class).ifPresent(connection -> { + try { + connection.close(); + } catch (SQLException e) { + throw new FlamingockException(e); + } + }); + } + private SqlTxWrapper createTxWrapper(TransactionManager txManager) { return new SqlTxWrapper(txManager); } diff --git a/core/target-systems/flamingock-sql-targetsystem/src/test/java/io/flamingock/targetsystem/sql/SqlTargetSystemSharedTxManagerTest.java b/core/target-systems/flamingock-sql-targetsystem/src/test/java/io/flamingock/targetsystem/sql/SqlTargetSystemSharedTxManagerTest.java index 4a940f4eb..72f05fd3b 100644 --- a/core/target-systems/flamingock-sql-targetsystem/src/test/java/io/flamingock/targetsystem/sql/SqlTargetSystemSharedTxManagerTest.java +++ b/core/target-systems/flamingock-sql-targetsystem/src/test/java/io/flamingock/targetsystem/sql/SqlTargetSystemSharedTxManagerTest.java @@ -17,6 +17,7 @@ import io.flamingock.internal.common.core.context.ContextResolver; import io.flamingock.internal.core.builder.FlamingockEdition; +import io.flamingock.internal.core.runtime.ExecutionRuntime; import io.flamingock.internal.core.transaction.TransactionManager; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -33,6 +34,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; class SqlTargetSystemSharedTxManagerTest { @@ -58,6 +60,58 @@ void txWrapperAndAuditMarkerShouldShareSameTxManager() throws Exception { "SqlTxWrapper and SqlTargetSystemAuditMarker must share the same TransactionManager instance"); } + @Test + @DisplayName("Should close SQL connection after non-transactional apply") + void shouldCloseConnectionAfterNonTransactionalApply() throws Exception { + Connection connection = mock(Connection.class); + DataSource dataSource = mock(DataSource.class); + when(dataSource.getConnection()).thenReturn(connection); + + SqlTargetSystem targetSystem = new SqlTargetSystem("test-sql", dataSource); + targetSystem.applyChange(runtime -> null, mockRuntimeWith(connection)); + + verify(connection).close(); + } + + @Test + @DisplayName("Should close SQL connection after non-transactional rollback") + void shouldCloseConnectionAfterNonTransactionalRollback() throws Exception { + Connection connection = mock(Connection.class); + DataSource dataSource = mock(DataSource.class); + when(dataSource.getConnection()).thenReturn(connection); + + SqlTargetSystem targetSystem = new SqlTargetSystem("test-sql", dataSource); + targetSystem.rollbackChange(runtime -> null, mockRuntimeWith(connection)); + + verify(connection).close(); + } + + @Test + @DisplayName("Should close SQL connection when non-transactional callback fails") + void shouldCloseConnectionWhenNonTransactionalCallbackFails() throws Exception { + Connection connection = mock(Connection.class); + DataSource dataSource = mock(DataSource.class); + when(dataSource.getConnection()).thenReturn(connection); + RuntimeException callbackFailure = new RuntimeException("callback failed"); + + SqlTargetSystem targetSystem = new SqlTargetSystem("test-sql", dataSource); + RuntimeException thrown = assertThrows(RuntimeException.class, + () -> targetSystem.applyChange(runtime -> { + throw callbackFailure; + }, mockRuntimeWith(connection))); + + assertSame(callbackFailure, thrown); + verify(connection).close(); + } + + private static ExecutionRuntime mockRuntimeWith(Connection connection) { + ExecutionRuntime executionRuntime = mock(ExecutionRuntime.class); + ContextResolver contextResolver = mock(ContextResolver.class); + when(contextResolver.getDependencyValue(Connection.class)).thenReturn(Optional.of(connection)); + when(executionRuntime.getContext()).thenReturn(contextResolver); + return executionRuntime; + } + private static DataSource mockDataSource() throws Exception { ResultSet emptyResultSet = mock(ResultSet.class); when(emptyResultSet.next()).thenReturn(false); From 2941d47b7d48fa970b9d2854eb7ea7ab45812556 Mon Sep 17 00:00:00 2001 From: bercianor Date: Sat, 19 Sep 2026 13:44:55 +0200 Subject: [PATCH 2/2] fix(sql): scope non-transactional connection cleanup --- .../targets/AbstractTargetSystem.java | 38 +++++++++---------- .../targetsystem/sql/SqlTargetSystem.java | 37 +++++++++--------- .../SqlTargetSystemSharedTxManagerTest.java | 3 ++ 3 files changed, 39 insertions(+), 39 deletions(-) diff --git a/core/flamingock-core/src/main/java/io/flamingock/internal/core/external/targets/AbstractTargetSystem.java b/core/flamingock-core/src/main/java/io/flamingock/internal/core/external/targets/AbstractTargetSystem.java index 2c2bc04f5..29b97d496 100644 --- a/core/flamingock-core/src/main/java/io/flamingock/internal/core/external/targets/AbstractTargetSystem.java +++ b/core/flamingock-core/src/main/java/io/flamingock/internal/core/external/targets/AbstractTargetSystem.java @@ -83,8 +83,8 @@ public String getId() { * Applies a change operation with session-scoped dependency injection. *

* This method is the entry point for non-transactional change execution. - * It calls {@link #enhanceExecutionRuntime(RuntimeContext, boolean)} to allow - * subclasses to inject session-scoped dependencies before executing the change. + * It delegates lifecycle ownership to {@link #nonTxWrapper(Function, ExecutionRuntime)}, + * which enhances the runtime before invoking the change. * * @param the return type of the change operation * @param changeApplier the function that executes the actual change @@ -92,20 +92,15 @@ public String getId() { * @return the result of the change operation */ public final T applyChange(Function changeApplier, ExecutionRuntime executionRuntime) { - enhanceExecutionRuntime(executionRuntime, false); - try { - return changeApplier.apply(executionRuntime); - } finally { - cleanupExecutionRuntime(executionRuntime); - } + return nonTxWrapper(changeApplier, executionRuntime); } /** * Rolls back (reverts) a previously applied change with session-scoped dependency injection. *

* This method is the entry point for non-transactional rollback execution. - * It calls {@link #enhanceExecutionRuntime(RuntimeContext, boolean)} to allow - * subclasses to inject session-scoped dependencies before executing the rollback. + * It delegates lifecycle ownership to {@link #nonTxWrapper(Function, ExecutionRuntime)}, + * which enhances the runtime before invoking the rollback. * * @param the return type of the rollback operation * @param changeRollbacker the function that executes the actual rollback @@ -113,23 +108,28 @@ public final T applyChange(Function changeApplier, Exec * @return the result of the rollback operation */ public final T rollbackChange(Function changeRollbacker, ExecutionRuntime executionRuntime) { - enhanceExecutionRuntime(executionRuntime, false); - try { - return changeRollbacker.apply(executionRuntime); - } finally { - cleanupExecutionRuntime(executionRuntime); - } + return nonTxWrapper(changeRollbacker, executionRuntime); } /** - * Hook for cleaning up session-scoped dependencies after non-transactional execution. + * Executes a non-transactional callback and owns its runtime lifecycle. + *

+ * The default implementation enhances the runtime once before executing the callback. + * Subclasses that manage resources for non-transactional execution must enhance the runtime + * and release those resources within their override. * - * @param executionRuntime the runtime whose session-scoped dependencies should be cleaned up + * @param changeFunc the callback to execute + * @param executionRuntime the runtime to enhance and pass to the callback + * @param the callback return type + * @return the callback result */ - protected void cleanupExecutionRuntime(RuntimeContext executionRuntime) { + protected T nonTxWrapper(Function changeFunc, ExecutionRuntime executionRuntime) { + enhanceExecutionRuntime(executionRuntime, false); + return changeFunc.apply(executionRuntime); } + /** * Hook for injecting session-scoped dependencies into the execution runtime. *

diff --git a/core/target-systems/flamingock-sql-targetsystem/src/main/java/io/flamingock/targetsystem/sql/SqlTargetSystem.java b/core/target-systems/flamingock-sql-targetsystem/src/main/java/io/flamingock/targetsystem/sql/SqlTargetSystem.java index cc7a83108..5f633e7c5 100644 --- a/core/target-systems/flamingock-sql-targetsystem/src/main/java/io/flamingock/targetsystem/sql/SqlTargetSystem.java +++ b/core/target-systems/flamingock-sql-targetsystem/src/main/java/io/flamingock/targetsystem/sql/SqlTargetSystem.java @@ -22,12 +22,14 @@ import io.flamingock.internal.core.builder.FlamingockEdition; import io.flamingock.internal.core.external.targets.TransactionalTargetSystem; import io.flamingock.internal.core.external.targets.mark.NoOpTargetSystemAuditMarker; +import io.flamingock.internal.core.runtime.ExecutionRuntime; import io.flamingock.internal.core.transaction.TransactionManager; import io.flamingock.internal.common.core.transaction.TransactionWrapper; import javax.sql.DataSource; import java.sql.Connection; import java.sql.SQLException; +import java.util.function.Function; import static io.flamingock.internal.core.builder.FlamingockEdition.COMMUNITY; @@ -79,28 +81,23 @@ public TransactionWrapper getTxWrapper() { return txWrapper; } + /** + * Acquires one connection for a non-transactional callback, injects it into the runtime, + * and closes that same connection after the callback completes or fails. + * + * @param changeFunc the callback to execute + * @param executionRuntime the runtime to receive the connection dependency + * @param the callback return type + * @return the callback result + */ @Override - protected void enhanceExecutionRuntime(RuntimeContext executionRuntime, boolean isTransactional) { - //if transactional, the connection is injected in the wrapInTransaction - if (!isTransactional) { - try { - executionRuntime.addDependency(dataSource.getConnection()); - } catch (SQLException e) { - throw new FlamingockException(e); - } + protected T nonTxWrapper(Function changeFunc, ExecutionRuntime executionRuntime) { + try (Connection connection = dataSource.getConnection()) { + executionRuntime.addDependency(connection); + return changeFunc.apply(executionRuntime); + } catch (SQLException e) { + throw new FlamingockException(e); } - - } - - @Override - protected void cleanupExecutionRuntime(RuntimeContext executionRuntime) { - executionRuntime.getContext().getDependencyValue(Connection.class).ifPresent(connection -> { - try { - connection.close(); - } catch (SQLException e) { - throw new FlamingockException(e); - } - }); } private SqlTxWrapper createTxWrapper(TransactionManager txManager) { diff --git a/core/target-systems/flamingock-sql-targetsystem/src/test/java/io/flamingock/targetsystem/sql/SqlTargetSystemSharedTxManagerTest.java b/core/target-systems/flamingock-sql-targetsystem/src/test/java/io/flamingock/targetsystem/sql/SqlTargetSystemSharedTxManagerTest.java index 72f05fd3b..a1b1c963b 100644 --- a/core/target-systems/flamingock-sql-targetsystem/src/test/java/io/flamingock/targetsystem/sql/SqlTargetSystemSharedTxManagerTest.java +++ b/core/target-systems/flamingock-sql-targetsystem/src/test/java/io/flamingock/targetsystem/sql/SqlTargetSystemSharedTxManagerTest.java @@ -70,6 +70,7 @@ void shouldCloseConnectionAfterNonTransactionalApply() throws Exception { SqlTargetSystem targetSystem = new SqlTargetSystem("test-sql", dataSource); targetSystem.applyChange(runtime -> null, mockRuntimeWith(connection)); + verify(dataSource).getConnection(); verify(connection).close(); } @@ -83,6 +84,7 @@ void shouldCloseConnectionAfterNonTransactionalRollback() throws Exception { SqlTargetSystem targetSystem = new SqlTargetSystem("test-sql", dataSource); targetSystem.rollbackChange(runtime -> null, mockRuntimeWith(connection)); + verify(dataSource).getConnection(); verify(connection).close(); } @@ -101,6 +103,7 @@ void shouldCloseConnectionWhenNonTransactionalCallbackFails() throws Exception { }, mockRuntimeWith(connection))); assertSame(callbackFailure, thrown); + verify(dataSource).getConnection(); verify(connection).close(); }