From 42e39967f077121d2a312bf703e78993759d20a0 Mon Sep 17 00:00:00 2001 From: Jeff Jensen Date: Mon, 31 Aug 2026 10:05:42 -0500 Subject: [PATCH 1/2] build: Set project version to 3.5.2-SNAPSHOT The 3.5.1 release shipped from this branch; development toward the 3.5.2 patch continues here. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Q77iHoBhzZxJCQzkC8f82f --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index c6ba065ce..5b6a0731b 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ org.dbunit dbunit - 3.5.1 + 3.5.2-SNAPSHOT jar dbUnit Extension https://github.com/dbunit/dbunit-extension From b0a9ff34f50afdfe262e17cc187e8f2c020903bb Mon Sep 17 00:00:00 2001 From: Jeff Jensen Date: Mon, 31 Aug 2026 10:05:55 -0500 Subject: [PATCH 2/2] fix(database): Drop a closed connection kept by DefaultPrepAndExpectedTestCase DefaultPrepAndExpectedTestCase with setCloseConnectionAfterTest(false), reused across test methods (a base-class or shared static field paired with a CachingConnectionProvider), pinned the first IDatabaseConnection it acquired in a field and never re-checked it. closeReusableConnection() deliberately keeps that field set - not closing it - when closeConnectionAfterTest is false, with no check that the kept connection is still open. So once the pool or the database dropped that connection between test methods (max-lifetime/reap, a PGConnectionPoolDataSource issuing a fresh logical handle, a bounced application context, an idle-in-transaction kill), every following test on the instance failed on it: in setupData()'s CLEAN_INSERT, then again, suppressed, in cleanupData()'s tear down operation. CachingConnectionProvider's own liveness check, which exists to replace a dead connection transparently, was never reached, because the provider is not consulted again once the field is set. * getReusableConnection() discards the cached connection when closeConnectionAfterTest is false and it reports isClosed(), so the next call re-acquires from the tester and a CachingConnectionProvider behind it hands back a live replacement. isClosed() only - cheap, no round trip, and it does not break the "acquire once, no provider" contract a round-tripping isValid() would. * closeReusableConnectionSuppressing(), only ever called from a lifecycle step that has already failed, now also forgets the connection even when closeConnectionAfterTest is false - so a server-side disconnect the driver reported only by throwing (not by flipping isClosed()) still lets the next test re-acquire, rather than depending on the driver's local closed flag. * The default closeConnectionAfterTest=true path is unchanged. * Add two DatabaseTesterConnectionReuseIT cases: one proving a reused instance keeps reusing its one open connection while it stays alive, one proving it replaces the connection after it is closed between test methods instead of failing every following test on the dead one. Refs: 962 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Q77iHoBhzZxJCQzkC8f82f --- src/changes/changes.xml | 5 ++ .../DefaultPrepAndExpectedTestCase.java | 64 +++++++++++++- .../DatabaseTesterConnectionReuseIT.java | 88 +++++++++++++++++++ 3 files changed, 156 insertions(+), 1 deletion(-) diff --git a/src/changes/changes.xml b/src/changes/changes.xml index df373ad62..e6e5daa0e 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -13,6 +13,11 @@ + + + Fix DefaultPrepAndExpectedTestCase reusing a closed connection when closeConnectionAfterTest is false and one instance is reused across test methods (a base-class or shared static field paired with a CachingConnectionProvider): getReusableConnection() pinned the first IDatabaseConnection in a field and closeReusableConnection() kept it there without closing, so a connection the pool or database dropped between tests - max-lifetime/reap, a PGConnectionPoolDataSource issuing a fresh handle, a bounced application context, an idle-in-transaction kill - left every following test failing on it, in setupData()'s CLEAN_INSERT and then cleanupData()'s tear down operation, with CachingConnectionProvider's own liveness check never reached to replace it. getReusableConnection() now discards a closed cached connection before returning it, and closeReusableConnectionSuppressing() forgets the connection after any failed lifecycle step, so the next test re-acquires a live one from the tester. Present since 3.4.0, when setCloseConnectionAfterTest() was added. + + Fix a 3.5.0 regression (introduced by issue #496's fix) where FlatXmlProducer added every table from an explicitly-supplied metadata IDataSet as an empty table, not just tables actually present in the flat XML body: FlatXmlDataSetBuilder#setMetaDataSet(IDataSet) is documented as supplying column metadata only, but a broad metadata source such as a live database's full IDataSet (e.g. via DatabaseConnection#createDataSet()) was being read as if it enumerated the fixture's own tables, so DELETE_ALL/CLEAN_INSERT ended up touching every table in that broader source instead of only the ones the XML body mentions. FlatXmlProducer's issue #496 empty-table backfill now only runs when the metadata source is DTD-derived (a FlatDtdDataSet, whether parsed inline from the flat XML's own DOCTYPE or supplied via FlatXmlDataSetBuilder#setMetaDataSetFromDtd), restoring pre-3.5.0 behavior for any other explicitly-supplied metadata dataset. diff --git a/src/main/java/org/dbunit/DefaultPrepAndExpectedTestCase.java b/src/main/java/org/dbunit/DefaultPrepAndExpectedTestCase.java index d3b0400c9..409afce0d 100644 --- a/src/main/java/org/dbunit/DefaultPrepAndExpectedTestCase.java +++ b/src/main/java/org/dbunit/DefaultPrepAndExpectedTestCase.java @@ -20,6 +20,7 @@ */ package org.dbunit; +import java.sql.Connection; import java.sql.SQLException; import java.util.ArrayList; import java.util.Arrays; @@ -278,6 +279,20 @@ private boolean lookupFeatureValue(final String featureName) * Return the connection shared by lookupFeatureValue(), setupData(), * verifyData() and cleanupData() for the current test's lifecycle, * acquiring it on first use instead of a fresh connection at each step. + *

+ * When {@link #closeConnectionAfterTest} is false this connection is kept + * across test methods (see {@link #closeReusableConnection()}), where the + * connection pool or the database server can close it between tests - a + * pool max-lifetime or reap, a bounced application context, a + * {@link org.dbunit.database.CachingConnectionProvider#close()}. A closed + * one is discarded here before it is handed back, so the next call + * re-acquires from databaseTester - letting a + * {@link org.dbunit.database.CachingConnectionProvider} behind it supply a + * live replacement - rather than this instance reusing a connection every + * later lifecycle step would only fail on. With + * {@link #closeConnectionAfterTest} left at its default the connection is + * closed and forgotten after each test anyway, so it is not re-checked + * here. * * @return The shared connection. * @throws Exception On dbUnit errors. @@ -285,6 +300,11 @@ private boolean lookupFeatureValue(final String featureName) */ private IDatabaseConnection getReusableConnection() throws Exception { + if (connection != null && !closeConnectionAfterTest + && isReusableConnectionClosed()) + { + connection = null; + } if (connection == null) { connection = getConnection(); @@ -292,13 +312,46 @@ private IDatabaseConnection getReusableConnection() throws Exception return connection; } + /** + * Returns whether the connection currently cached in {@link #connection} has + * been closed - by the connection pool, the database server, or a + * {@link org.dbunit.database.CachingConnectionProvider} behind + * {@code databaseTester} - since this instance last used it. A connection + * that throws while being asked is treated as closed. Uses only the local + * {@link Connection#isClosed()} flag, not a round-tripping + * {@link Connection#isValid(int)}: cheap, side-effect free, and enough to + * catch a connection closed between test methods before the next one's + * first statement. A server-side disconnect the driver has not noticed yet + * instead surfaces once, when a lifecycle step runs a statement against it; + * {@link #closeReusableConnectionSuppressing(Throwable)} then forgets the + * connection so the following test re-acquires regardless. + * + * @return True when the cached connection is known to be closed or unusable. + * @since 3.5.2 + */ + private boolean isReusableConnectionClosed() + { + try + { + return connection.getConnection().isClosed(); + } catch (final SQLException e) + { + log.debug("isReusableConnectionClosed: treating the cached connection" + + " as closed after it failed to report its state", e); + return true; + } + } + /** * Release the connection shared by lookupFeatureValue(), setupData(), * verifyData() and cleanupData(), if one was acquired: closes it and * forgets it when {@link #closeConnectionAfterTest} is true (the * default); otherwise leaves it open and keeps the field set, so a later * lifecycle step's getReusableConnection() call keeps reusing it rather - * than acquiring - and silently orphaning - another one. + * than acquiring - and silently orphaning - another one. That later call + * still drops the kept connection if it has since died (see + * {@link #getReusableConnection()}), so a connection the pool or server + * closed between test methods does not linger to fail every following one. * * @throws SQLException On close errors. * @since 3.4.0 @@ -333,6 +386,14 @@ private void closeReusableConnection() throws SQLException * {@link Throwable#addSuppressed(Throwable)} rather than letting it * replace and hide the primary. Mirrors the exception safety of * {@link #runTest} and {@code DatabaseTestCase.tearDown(Throwable)}. + *

+ * Only ever called from a lifecycle step that has already failed, so it + * also forgets the shared connection even when {@link #closeConnectionAfterTest} + * is false and {@link #closeReusableConnection()} therefore left it open: a + * step that just threw may have broken it, so the next + * {@link #getReusableConnection()} re-acquires rather than reusing it. Any + * {@link org.dbunit.database.CachingConnectionProvider} behind + * {@code databaseTester} still owns closing it. * * @param primary * The exception already in flight to attach a close failure @@ -348,6 +409,7 @@ private void closeReusableConnectionSuppressing(final Throwable primary) { primary.addSuppressed(closeFailure); } + connection = null; } /** diff --git a/src/test/java/org/dbunit/DatabaseTesterConnectionReuseIT.java b/src/test/java/org/dbunit/DatabaseTesterConnectionReuseIT.java index 228966fa8..6c8dc2c6e 100644 --- a/src/test/java/org/dbunit/DatabaseTesterConnectionReuseIT.java +++ b/src/test/java/org/dbunit/DatabaseTesterConnectionReuseIT.java @@ -205,12 +205,100 @@ void testDefaultPrepAndExpectedTestCase_acrossFreshInstancesSharingAProviderWith } } + @Test + void testDefaultPrepAndExpectedTestCase_reusedAcrossTestMethodsWithCloseDisabled_keepsReusingItsOneOpenConnection() + throws Exception + { + final CachingConnectionProvider provider = new CachingConnectionProvider(); + final IDatabaseTester tester = newSharedProviderTester(provider); + tester.setTearDownOperation(DatabaseOperation.DELETE_ALL); + final DefaultPrepAndExpectedTestCase tc = newCloseDisabledTestCase(tester); + final List connectionsSeen = new ArrayList<>(); + try + { + for (int simulatedTestMethod = 0; simulatedTestMethod < 3; simulatedTestMethod++) + { + runOneSimulatedTestMethod(tc); + + final IDatabaseConnection connection = tester.getConnection(); + connectionsSeen.add(connection); + assertThat(connection.getConnection().isClosed()) + .as("closeConnectionAfterTest=false: the connection must still be" + + " open for the next simulated test method to reuse.") + .isFalse(); + } + + assertThat(new HashSet<>(connectionsSeen)) + .as("One DefaultPrepAndExpectedTestCase reused across test methods, backed" + + " by a CachingConnectionProvider, must run every method against" + + " the one cached connection while it stays alive.") + .hasSize(1); + } finally + { + provider.close(); + } + } + + @Test + void testDefaultPrepAndExpectedTestCase_reusedWithCloseDisabled_whenItsCachedConnectionDiesBetweenTestMethods_replacesItRatherThanReusingTheDeadOne() + throws Exception + { + final CachingConnectionProvider provider = new CachingConnectionProvider(); + final IDatabaseTester tester = newSharedProviderTester(provider); + tester.setTearDownOperation(DatabaseOperation.DELETE_ALL); + final DefaultPrepAndExpectedTestCase tc = newCloseDisabledTestCase(tester); + try + { + // First simulated test method: acquires and pins a connection. + runOneSimulatedTestMethod(tc); + final IDatabaseConnection firstConnection = tester.getConnection(); + + // The connection pool or the database server drops that connection + // between test methods - max lifetime, an idle-in-transaction + // timeout, a bounced application context, and so on. + firstConnection.getConnection().close(); + + // Second simulated test method on the SAME instance must notice the + // dead connection it pinned and acquire a live replacement, not fail + // setupData()'s CLEAN_INSERT - and then cleanupData()'s tear down + // operation - on the connection it can no longer use. + runOneSimulatedTestMethod(tc); + + final IDatabaseConnection secondConnection = tester.getConnection(); + assertThat(secondConnection.getConnection().isClosed()) + .as("The reused test case must have replaced the connection killed" + + " between test methods, not kept handing back the dead one.") + .isFalse(); + assertThat(secondConnection) + .as("A fresh connection must have been acquired from the" + + " CachingConnectionProvider once the first one died.") + .isNotSameAs(firstConnection); + } finally + { + provider.close(); + } + } + private DefaultPrepAndExpectedTestCase newTestCase(final IDatabaseTester tester) { final DataFileLoader dataFileLoader = new FlatXmlDataFileLoader(); return new DefaultPrepAndExpectedTestCase(dataFileLoader, tester); } + private DefaultPrepAndExpectedTestCase newCloseDisabledTestCase(final IDatabaseTester tester) + { + final DefaultPrepAndExpectedTestCase tc = newTestCase(tester); + tc.setCloseConnectionAfterTest(false); + return tc; + } + + private static void runOneSimulatedTestMethod(final DefaultPrepAndExpectedTestCase tc) + throws Exception + { + tc.runTest(new VerifyTableDefinition[] {}, new String[] {}, new String[] {}, + () -> null); + } + private IDatabaseTester newSharedProviderTester(final CachingConnectionProvider sharedProvider) throws Exception {