From 411bd469d25fcb33ec1124e3288c3e31435d6fa7 Mon Sep 17 00:00:00 2001 From: Jeff Jensen Date: Sat, 12 Sep 2026 10:08:11 -0500 Subject: [PATCH] refactor(annotations): Extract InjectedTestCaseTesterBinding from TesterResolver * Move the @DbUnitTestCase round-trip resolution logic (get/set/verify) out of org.dbunit.junit.jupiter.TesterResolver into a new public org.dbunit.annotation.runtime.InjectedTestCaseTesterBinding, beside the package's other binding-agnostic reflection helpers (DefaultMethodOverrideCheck, ProvidedAttribute, ReflectiveInstantiation). * TesterResolver now delegates to it instead of duplicating the logic; no behavior change (DbUnitExtensionTest and AnnotatedTestExecutorTest pass unchanged). Lets a second binding reuse the same round-trip verification instead of re-implementing it. Refs: 754 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01FbQ6r1qSYWiyPqbeniaVCw --- .../InjectedTestCaseTesterBinding.java | 133 +++++++++++++ .../dbunit/junit/jupiter/TesterResolver.java | 77 +------- .../InjectedTestCaseTesterBindingTest.java | 184 ++++++++++++++++++ .../DbUnitExtensionSelfManagedTestCaseIT.java | 4 +- 4 files changed, 328 insertions(+), 70 deletions(-) create mode 100644 src/main/java/org/dbunit/annotation/runtime/InjectedTestCaseTesterBinding.java create mode 100644 src/test/java/org/dbunit/annotation/runtime/InjectedTestCaseTesterBindingTest.java diff --git a/src/main/java/org/dbunit/annotation/runtime/InjectedTestCaseTesterBinding.java b/src/main/java/org/dbunit/annotation/runtime/InjectedTestCaseTesterBinding.java new file mode 100644 index 00000000..133f16df --- /dev/null +++ b/src/main/java/org/dbunit/annotation/runtime/InjectedTestCaseTesterBinding.java @@ -0,0 +1,133 @@ +/* + * + * The DbUnit Database Testing Framework + * Copyright (C)2002-2026, DbUnit.org + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ +package org.dbunit.annotation.runtime; + +import java.util.Objects; +import java.util.concurrent.Callable; + +import org.dbunit.IDatabaseTester; +import org.dbunit.PrepAndExpectedTestCase; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Resolves the {@link IDatabaseTester} for an already-injected {@code @DbUnitTestCase} instance: + * {@link PrepAndExpectedTestCase#getDatabaseTester()} if it already has one, otherwise a + * caller-supplied fallback, wired back onto the test case and round-trip-verified. + * + *

Public so a binding outside {@code org.dbunit.annotation.runtime} - such as + * {@code DbUnitExtension} or a Spring {@code TestExecutionListener} - can reach it too, sharing + * the one round-trip-verification rule rather than each re-implementing it: a test case whose + * type overrides {@code setDatabaseTester()} but does not round-trip + * {@code getDatabaseTester()} back to the same instance is a bug (an operation applied to the + * resolved tester would silently never reach the tester the test case actually uses) and fails + * fast, while one that never overrides {@code setDatabaseTester()} at all is the documented + * self-managed-connection pattern and only logged. + * + * @author Jeff Jensen + * @since 3.6.0 + */ +public class InjectedTestCaseTesterBinding +{ + private static final Logger log = LoggerFactory.getLogger(InjectedTestCaseTesterBinding.class); + + private InjectedTestCaseTesterBinding() + { + } + + /** + * Resolves and wires the tester for {@code testCase}. + * + * @param testCase The already-injected test case, resolved and type-checked by the caller. + * @param fieldDescription Where {@code testCase} came from, for an exception or log message + * - e.g. {@code "field 'testCase' in com.example.MyTest"}. + * @param fallbackTester Supplies a tester when {@code testCase.getDatabaseTester()} is + * {@code null}; not called otherwise. + * @return The resolved tester: {@code testCase.getDatabaseTester()} if already set, otherwise + * {@code fallbackTester}'s value. + * @throws Exception If {@code fallbackTester} fails. + * @throws IllegalStateException If {@code testCase}'s type overrides + * {@code setDatabaseTester()} but does not round-trip {@code getDatabaseTester()} + * back to the same instance. + */ + public static IDatabaseTester resolveTester(final PrepAndExpectedTestCase testCase, + final String fieldDescription, final Callable fallbackTester) + throws Exception + { + final IDatabaseTester existing = testCase.getDatabaseTester(); + if (existing != null) + { + return existing; + } + + final IDatabaseTester tester = Objects.requireNonNull(fallbackTester.call(), + "fallbackTester must supply an IDatabaseTester."); + + // The executor drives this exact, already-injected instance directly rather than + // constructing a fresh one, so the fallback tester must be wired onto it here - + // otherwise it keeps whatever databaseTester it reports for an implementation that does + // not override setDatabaseTester(). + testCase.setDatabaseTester(tester); + if (testCase.getDatabaseTester() == tester) + { + return tester; + } + + if (overridesSetDatabaseTester(testCase)) + { + // Opted into automatic wiring by overriding setDatabaseTester(), so a round-trip + // failure means the override itself is broken - fail fast the same way every other + // @DbUnitConfig-driven setter does for a silent no-op, rather than let + // @DbUnitSetup/@DbUnitTearDown operations quietly target a tester this test case + // never actually uses. + throw new IllegalStateException(fieldDescription + + " is annotated @DbUnitTestCase; its value's type (" + + testCase.getClass().getName() + ") overrides setDatabaseTester(), but" + + " getDatabaseTester() does not return the same instance right after being" + + " given it. @DbUnitSetup/@DbUnitTearDown operations set on the resolved" + + " IDatabaseTester would silently never reach the operations this test case" + + " actually runs. Fix setDatabaseTester()/getDatabaseTester() to round-trip" + + " the same instance."); + } + // A test case that never overrides setDatabaseTester() at all is the documented + // self-managed-connection pattern, not a bug - see annotations.adoc - so this stays a + // diagnostic log, not a failure. + log.debug("PrepAndExpectedTestCase {} does not round-trip getDatabaseTester()/" + + "setDatabaseTester(); @DbUnitSetup/@DbUnitTearDown operations set on the" + + " resolved IDatabaseTester may not reach the operations this test case actually" + + " runs unless it independently uses the same tester instance.", + testCase.getClass().getName()); + return tester; + } + + /** + * Returns whether {@code testCase}'s runtime type overrides + * {@link PrepAndExpectedTestCase#setDatabaseTester(IDatabaseTester)}, rather than inheriting + * the interface's own no-op default body. See + * {@link DefaultMethodOverrideCheck#overridesDefaultMethod} for this check's own known + * limitation. + */ + private static boolean overridesSetDatabaseTester(final PrepAndExpectedTestCase testCase) + { + return DefaultMethodOverrideCheck.overridesDefaultMethod(testCase.getClass(), + PrepAndExpectedTestCase.class, "setDatabaseTester", IDatabaseTester.class); + } +} diff --git a/src/main/java/org/dbunit/junit/jupiter/TesterResolver.java b/src/main/java/org/dbunit/junit/jupiter/TesterResolver.java index 8ffd1860..3ed59a7e 100644 --- a/src/main/java/org/dbunit/junit/jupiter/TesterResolver.java +++ b/src/main/java/org/dbunit/junit/jupiter/TesterResolver.java @@ -33,7 +33,7 @@ import org.dbunit.annotation.DbUnitTestCase; import org.dbunit.annotation.DbUnitTester; import org.dbunit.annotation.runtime.AnnotatedTestConfiguration; -import org.dbunit.annotation.runtime.DefaultMethodOverrideCheck; +import org.dbunit.annotation.runtime.InjectedTestCaseTesterBinding; import org.dbunit.annotation.runtime.ProvidedAttribute; import org.dbunit.annotation.runtime.ReflectiveInstantiation; import org.junit.jupiter.api.extension.ExtensionContext; @@ -98,51 +98,14 @@ Resolution resolve(final ExtensionContext context, } final PrepAndExpectedTestCase testCase = (PrepAndExpectedTestCase) testCaseFieldValue; - IDatabaseTester tester = testCase.getDatabaseTester(); - if (tester == null) - { - // testCase does not override getDatabaseTester()/setDatabaseTester(), or does - // and was simply built without a tester yet (e.g. the no-arg-tester constructor - // form) - fall back to the same resolution a bare @DbUnitTestCase-less test - // would use, rather than leaving Resolution#tester null. - tester = findTester(instances, testClass, configuration); - // The executor drives this exact, already-injected instance directly (see - // AnnotatedTestExecutor#beforeExpectedTest()) rather than constructing a fresh - // one, so the fallback tester must be wired onto it here - otherwise it keeps - // whatever databaseTester it reports for an implementation that does not - // override setDatabaseTester(). - testCase.setDatabaseTester(tester); - if (testCase.getDatabaseTester() != tester) - { - if (overridesSetDatabaseTester(testCase)) - { - // Opted into automatic wiring by overriding setDatabaseTester(), so a - // round-trip failure means the override itself is broken - fail fast the - // same way every other @DbUnitConfig-driven setter does for a silent - // no-op, rather than let @DbUnitSetup/@DbUnitTearDown operations quietly - // target a tester this test case never actually uses. - throw new IllegalStateException("Field '" + testCaseField.field.getName() - + "' in " + testCaseField.instance.getClass().getName() - + " is annotated @DbUnitTestCase; its value's type (" - + testCase.getClass().getName() + ") overrides" - + " setDatabaseTester(), but getDatabaseTester() does not return" - + " the same instance right after being given it." - + " @DbUnitSetup/@DbUnitTearDown operations set on the resolved" - + " IDatabaseTester would silently never reach the operations" - + " this test case actually runs. Fix setDatabaseTester()/" - + "getDatabaseTester() to round-trip the same instance."); - } - // A test case that never overrides setDatabaseTester() at all is the - // documented self-managed-connection pattern, not a bug - see - // annotations.adoc - so this stays a diagnostic log, not a failure. - log.debug("PrepAndExpectedTestCase {} does not round-trip" - + " getDatabaseTester()/setDatabaseTester(); @DbUnitSetup/" - + "@DbUnitTearDown operations set on the resolved IDatabaseTester" - + " may not reach the operations this test case actually runs" - + " unless it independently uses the same tester instance.", - testCase.getClass().getName()); - } - } + final String fieldDescription = "Field '" + testCaseField.field.getName() + "' in " + + testCaseField.instance.getClass().getName(); + // testCase does not override getDatabaseTester()/setDatabaseTester(), or does and + // was simply built without a tester yet (e.g. the no-arg-tester constructor form) - + // the fallback below resolves the same tester a bare @DbUnitTestCase-less test + // would use, rather than leaving Resolution#tester null. + final IDatabaseTester tester = InjectedTestCaseTesterBinding.resolveTester(testCase, + fieldDescription, () -> findTester(instances, testClass, configuration)); return new Resolution(tester, testCase); } @@ -169,28 +132,6 @@ Resolution resolve(final ExtensionContext context, return new Resolution(findTester(instances, testClass, configuration), null); } - /** - * Returns whether {@code testCase}'s runtime type overrides - * {@link PrepAndExpectedTestCase#setDatabaseTester(IDatabaseTester)}, rather than - * inheriting the interface's own no-op default body - the same distinction - * {@code org.dbunit.annotation.runtime.InjectedTestCaseConfigurer} makes for its - * {@code @DbUnitConfig}-driven setters. Used by - * {@link #resolve(ExtensionContext, AnnotatedTestConfiguration)} to tell a - * {@link PrepAndExpectedTestCase} that deliberately manages its own tester - never - * overriding this method, a documented and supported pattern - apart from one that opted - * into automatic wiring by overriding it, but whose override does not actually work. See - * {@link DefaultMethodOverrideCheck#overridesDefaultMethod} for this check's own known - * limitation. - * - * @param testCase The instance to check. - * @return True when {@code testCase}'s type overrides {@code setDatabaseTester()}. - */ - private boolean overridesSetDatabaseTester(final PrepAndExpectedTestCase testCase) - { - return DefaultMethodOverrideCheck.overridesDefaultMethod(testCase.getClass(), - PrepAndExpectedTestCase.class, "setDatabaseTester", IDatabaseTester.class); - } - private List innermostFirst(final TestInstances instances) { final List all = new ArrayList<>(instances.getAllInstances()); diff --git a/src/test/java/org/dbunit/annotation/runtime/InjectedTestCaseTesterBindingTest.java b/src/test/java/org/dbunit/annotation/runtime/InjectedTestCaseTesterBindingTest.java new file mode 100644 index 00000000..a42954b3 --- /dev/null +++ b/src/test/java/org/dbunit/annotation/runtime/InjectedTestCaseTesterBindingTest.java @@ -0,0 +1,184 @@ +/* + * + * The DbUnit Database Testing Framework + * Copyright (C)2002-2026, DbUnit.org + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ +package org.dbunit.annotation.runtime; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.mock; + +import java.util.concurrent.Callable; + +import org.dbunit.DefaultPrepAndExpectedTestCase; +import org.dbunit.IDatabaseTester; +import org.dbunit.PrepAndExpectedTestCase; +import org.dbunit.PrepAndExpectedTestCaseSteps; +import org.dbunit.VerifyTableDefinition; +import org.dbunit.dataset.IDataSet; +import org.junit.jupiter.api.Test; + +class InjectedTestCaseTesterBindingTest +{ + @Test + void testResolveTester_testCaseAlreadyHasTester_returnsExistingWithoutCallingFallback() + throws Exception + { + final IDatabaseTester existing = mock(IDatabaseTester.class); + final DefaultPrepAndExpectedTestCase testCase = new DefaultPrepAndExpectedTestCase(); + testCase.setDatabaseTester(existing); + final Callable fallback = () -> + { + throw new AssertionError("Fallback must not be called when a tester already exists."); + }; + + final IDatabaseTester resolved = + InjectedTestCaseTesterBinding.resolveTester(testCase, "field 'x'", fallback); + + assertThat(resolved).as("The already-set tester must be returned unchanged.") + .isSameAs(existing); + } + + @Test + void testResolveTester_testCaseHasNoTesterAndRoundTrips_setsAndReturnsFallbackTester() + throws Exception + { + final IDatabaseTester fallbackTester = mock(IDatabaseTester.class); + final DefaultPrepAndExpectedTestCase testCase = new DefaultPrepAndExpectedTestCase(); + + final IDatabaseTester resolved = InjectedTestCaseTesterBinding.resolveTester(testCase, + "field 'x'", () -> fallbackTester); + + assertThat(resolved).as("The fallback tester must be returned.") + .isSameAs(fallbackTester); + assertThat(testCase.getDatabaseTester()) + .as("The fallback tester must also be wired onto the test case.") + .isSameAs(fallbackTester); + } + + @Test + void testResolveTester_overridesSetDatabaseTesterButDoesNotRoundTrip_throwsIllegalStateException() + { + final IDatabaseTester fallbackTester = mock(IDatabaseTester.class); + final BrokenRoundTripTestCase testCase = new BrokenRoundTripTestCase(); + + assertThatThrownBy(() -> InjectedTestCaseTesterBinding.resolveTester(testCase, + "field 'testCase' in com.example.MyTest", () -> fallbackTester)) + .as("A setDatabaseTester() override that does not round-trip must fail fast" + + " rather than silently leave operations targeting the wrong tester.") + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("field 'testCase' in com.example.MyTest") + .hasMessageContaining("does not return the same instance"); + } + + @Test + void testResolveTester_doesNotOverrideSetDatabaseTester_logsAndReturnsFallbackTester() + throws Exception + { + final IDatabaseTester fallbackTester = mock(IDatabaseTester.class); + final NonOverridingTestCase testCase = new NonOverridingTestCase(); + + final IDatabaseTester resolved = InjectedTestCaseTesterBinding.resolveTester(testCase, + "field 'x'", () -> fallbackTester); + + assertThat(resolved) + .as("A test case that never overrides setDatabaseTester() is the documented" + + " self-managed-connection pattern, not a bug - the fallback tester" + + " must still be returned rather than failing.") + .isSameAs(fallbackTester); + } + + /** + * Overrides {@code setDatabaseTester()} - so {@link DefaultMethodOverrideCheck} sees a real + * override - but deliberately does not delegate to {@code super}, so the inherited + * {@code getDatabaseTester()} never reflects it. + */ + private static class BrokenRoundTripTestCase extends DefaultPrepAndExpectedTestCase + { + @Override + public void setDatabaseTester(final IDatabaseTester databaseTester) + { + } + } + + /** + * Implements the interface directly, overriding neither {@code getDatabaseTester()} nor + * {@code setDatabaseTester()} - the documented self-managed-connection pattern. Every other + * method is an unused stub: {@link InjectedTestCaseTesterBinding} never calls them. + */ + private static class NonOverridingTestCase implements PrepAndExpectedTestCase + { + @Override + public void configureTest(final VerifyTableDefinition[] verifyTableDefinitions, + final String[] prepDataFiles, final String[] expectedDataFiles) + { + } + + @Override + public void preTest() + { + } + + @Override + public void preTest(final VerifyTableDefinition[] verifyTables, + final String[] prepDataFiles, final String[] expectedDataFiles) + { + } + + @Override + public Object runTest(final VerifyTableDefinition[] verifyTables, + final String[] prepDataFiles, final String[] expectedDataFiles, + final PrepAndExpectedTestCaseSteps testSteps) + { + return null; + } + + @Override + public void postTest() + { + } + + @Override + public void postTest(final boolean verifyData) + { + } + + @Override + public void verifyData() + { + } + + @Override + public void cleanupData() + { + } + + @Override + public IDataSet getPrepDataset() + { + return null; + } + + @Override + public IDataSet getExpectedDataset() + { + return null; + } + } +} diff --git a/src/test/java/org/dbunit/junit/jupiter/DbUnitExtensionSelfManagedTestCaseIT.java b/src/test/java/org/dbunit/junit/jupiter/DbUnitExtensionSelfManagedTestCaseIT.java index 22f7e7ac..ed848d56 100644 --- a/src/test/java/org/dbunit/junit/jupiter/DbUnitExtensionSelfManagedTestCaseIT.java +++ b/src/test/java/org/dbunit/junit/jupiter/DbUnitExtensionSelfManagedTestCaseIT.java @@ -83,8 +83,8 @@ void testAfterTestExecution_selfManagedConnectionTestCaseWithoutTesterOverride_r deleteAllRowsQuietly(environment); SelfManagedSample.testCase = new SelfManagedTestCase(profile); - final Logger extensionLogger = - (Logger) LoggerFactory.getLogger("org.dbunit.junit.jupiter.TesterResolver"); + final Logger extensionLogger = (Logger) LoggerFactory + .getLogger("org.dbunit.annotation.runtime.InjectedTestCaseTesterBinding"); final Level originalLevel = extensionLogger.getLevel(); extensionLogger.setLevel(Level.DEBUG); final ListAppender appender = new ListAppender<>();