From 5988340221ec1a6ff5357a459d9e05b89938e66d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20Kobyli=C5=84ski?= Date: Sat, 16 May 2026 19:39:56 +0200 Subject: [PATCH 1/8] feat: require TransactionRunner in okapi-spring-boot autoconfig (KOJAK-67) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eliminate the silent JDBC auto-commit fallback that caused duplicate delivery across processor instances when no PlatformTransactionManager was on the classpath. The autoconfig now produces a non-null TransactionRunner via a fail-fast bean factory, or skips it entirely when both scheduler and purger are disabled (publish-only deployments). Production changes (okapi-spring-boot): - new @Bean okapiTransactionRunner — derives SpringTransactionRunner from PlatformTransactionManager (qualifier > getIfUnique > distinct error), gated by @ConditionalOnExpression on schedulers being enabled - new okapi.transaction-manager-qualifier property (mirror of okapi.datasource-qualifier) - validatePtmDataSourceMatch — fails fast for ResourceTransactionManager PTMs bound to a different DataSource; logs a WARN for PTMs that don't expose a JDBC DataSource (JPA's EntityManagerFactory, Hibernate's SessionFactory, Exposed bridge, JTA); falls through gracefully when proxy chains terminate early (cycle, null target) with a distinct WARN - unwrapDataSource — iterative with visited-set so a self-referencing DelegatingDataSource doesn't spin the startup thread - OutboxProcessorScheduler / OutboxPurgerScheduler — non-null TransactionRunner parameter (was nullable TransactionTemplate) - spring-configuration-metadata.json entry for the new property Tests: - OutboxAutoConfigurationTransactionRunnerTest — 13 slice tests + 6 unit tests for unwrapDataSource (cycle / null target / nested chain / TADP unwrap / etc.) - SpringObjectProviderSemanticsAssumptionsTest — pins Spring 7 ObjectProvider.getIfUnique / ResourceTransactionManager semantics so framework upgrades surface loudly - ExposedSpringBridgeEndToEndTest — Postgres testcontainer + Exposed SpringTransactionManager bridge, including a multi-instance amplification test that detects regression to the auto-commit fallback - existing tests updated to register a no-op TransactionRunner where schedulers ran without a PTM (Liquibase / scheduler / qualifier tests) Build: - okapi-spring-boot: opt-in PIT mutation testing via -PenableMutationTesting=true - okapi-integration-tests: spring7-transaction + assertj + spring-boot-test for the Exposed bridge end-to-end coverage README: short migration note in the okapi-spring-boot section. --- README.md | 11 + okapi-integration-tests/build.gradle.kts | 8 + .../ExposedSpringBridgeEndToEndTest.kt | 194 ++++++++ okapi-spring-boot/build.gradle.kts | 32 +- .../okapi/springboot/OkapiProperties.kt | 5 + .../springboot/OutboxAutoConfiguration.kt | 170 ++++++- .../springboot/OutboxProcessorScheduler.kt | 6 +- .../okapi/springboot/OutboxPurgerScheduler.kt | 6 +- .../spring-configuration-metadata.json | 6 + ...ataSourceQualifierAutoConfigurationTest.kt | 6 + .../LiquibaseAutoConfigurationTest.kt | 10 + ...xAutoConfigurationTransactionRunnerTest.kt | 463 ++++++++++++++++++ .../OutboxProcessorAutoConfigurationTest.kt | 38 +- .../OutboxPurgerAutoConfigurationTest.kt | 21 +- ...gObjectProviderSemanticsAssumptionsTest.kt | 107 ++++ 15 files changed, 1061 insertions(+), 22 deletions(-) create mode 100644 okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/ExposedSpringBridgeEndToEndTest.kt create mode 100644 okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt create mode 100644 okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringObjectProviderSemanticsAssumptionsTest.kt diff --git a/README.md b/README.md index 1130bb2..52f7887 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,17 @@ springOutboxPublisher.publish( > **Note:** Spring and Kafka versions are not forced by okapi — you control them. > Okapi uses plain JDBC internally — it works with any `PlatformTransactionManager` (JPA, JDBC, jOOQ, Exposed, etc.). +`okapi-spring-boot` requires a `TransactionRunner` bean to bracket each scheduler tick in a transaction. The autoconfiguration derives one from any `PlatformTransactionManager` on the classpath (`spring-boot-starter-jdbc` or `spring-boot-starter-data-jpa` provide one out of the box) — no extra wiring needed in typical setups. If your application has no `PlatformTransactionManager` (single-instance, no transaction infrastructure) you must opt in explicitly: + +```kotlin +@Bean +fun outboxTransactionRunner(): TransactionRunner = object : TransactionRunner { + override fun runInTransaction(block: () -> T): T = block() +} +``` + +Without bracketing, `FOR UPDATE SKIP LOCKED` collapses to the single SELECT statement under JDBC auto-commit, which silently allows duplicate delivery across processor instances. This opt-in is intentionally manual to keep accidental misconfiguration out of multi-instance deployments. + ## How It Works Okapi implements the [transactional outbox pattern](https://softwaremill.com/microservices-101/) (see also: [microservices.io description](https://microservices.io/patterns/data/transactional-outbox.html)): diff --git a/okapi-integration-tests/build.gradle.kts b/okapi-integration-tests/build.gradle.kts index 10e1a24..f2d8785 100644 --- a/okapi-integration-tests/build.gradle.kts +++ b/okapi-integration-tests/build.gradle.kts @@ -44,5 +44,13 @@ dependencies { testImplementation(libs.springContext) testImplementation(libs.springTx) testImplementation(libs.springBootAutoconfigure) + testImplementation(libs.springBootTest) testImplementation(libs.springJdbc) + // Spring Boot 4.x doesn't pull AssertJ transitively but ApplicationContextRunner needs it + testImplementation(libs.assertjCore) + + // Exposed-Spring bridge (proves autoconfig works with non-DataSourceTransactionManager PTMs) + testImplementation(libs.exposedCore) + testImplementation(libs.exposedJdbc) + testImplementation(libs.exposedSpringTransaction) } diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/ExposedSpringBridgeEndToEndTest.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/ExposedSpringBridgeEndToEndTest.kt new file mode 100644 index 0000000..5e7b83d --- /dev/null +++ b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/ExposedSpringBridgeEndToEndTest.kt @@ -0,0 +1,194 @@ +package com.softwaremill.okapi.test.transaction + +import com.softwaremill.okapi.core.DeliveryInfo +import com.softwaremill.okapi.core.MessageDeliverer +import com.softwaremill.okapi.core.OutboxMessage +import com.softwaremill.okapi.core.OutboxProcessor +import com.softwaremill.okapi.core.TransactionRunner +import com.softwaremill.okapi.postgres.PostgresOutboxStore +import com.softwaremill.okapi.springboot.OkapiLiquibaseAutoConfiguration +import com.softwaremill.okapi.springboot.OutboxAutoConfiguration +import com.softwaremill.okapi.springboot.SpringConnectionProvider +import com.softwaremill.okapi.springboot.SpringOutboxPublisher +import com.softwaremill.okapi.springboot.SpringTransactionRunner +import com.softwaremill.okapi.test.support.CountingDataSource +import com.softwaremill.okapi.test.support.RecordingMessageDeliverer +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.shouldBe +import io.kotest.matchers.types.shouldBeInstanceOf +import org.jetbrains.exposed.v1.spring7.transaction.SpringTransactionManager +import org.postgresql.ds.PGSimpleDataSource +import org.springframework.boot.autoconfigure.AutoConfigurations +import org.springframework.boot.test.context.runner.ApplicationContextRunner +import org.springframework.transaction.PlatformTransactionManager +import org.springframework.transaction.support.TransactionTemplate +import org.testcontainers.containers.PostgreSQLContainer +import java.time.Clock +import java.util.concurrent.CompletableFuture +import java.util.concurrent.CyclicBarrier +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import javax.sql.DataSource + +/** + * Proves [OutboxAutoConfiguration]'s `okapiTransactionRunner` bean factory works with a + * non-`DataSourceTransactionManager` `PlatformTransactionManager` — specifically Exposed's + * `SpringTransactionManager` bridge. If the autoconfig assumed DST, this test would fail. + * + * Two assertions: + * 1. With processor disabled: a single Spring TX wrapping `springOutboxPublisher.publish()` + * borrows exactly one physical connection from the pool. This proves the autoconfig-built + * runner correctly bridges to `SpringConnectionProvider` through Spring's `ConnectionHolder`. + * 2. With processor enabled (200ms tick): an entry published inside a Spring TX is later + * delivered by the background scheduler — proving each scheduler tick is itself bracketed + * by a Spring TX driven by the bridged PTM. + * + * Liquibase schema migration is handled by `okapi-spring-boot` autoconfiguration; no manual + * setup needed. + */ +class ExposedSpringBridgeEndToEndTest : FunSpec({ + + val container = PostgreSQLContainer("postgres:16") + lateinit var counter: CountingDataSource + + beforeSpec { + container.start() + val raw = PGSimpleDataSource().apply { + setURL(container.jdbcUrl) + user = container.username + password = container.password + } + counter = CountingDataSource(raw) + } + + afterSpec { container.stop() } + + // Both okapi-postgres and okapi-mysql are on the test classpath (shared integration-tests + // module). Explicitly register PostgresOutboxStore so the autoconfig's MySQL path is + // unambiguously skipped — otherwise MySQL's `FOR UPDATE SKIP LOCKED` with `FORCE INDEX` + // hint would fail on Postgres. + fun runner(recorder: RecordingMessageDeliverer): ApplicationContextRunner = ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(OutboxAutoConfiguration::class.java, OkapiLiquibaseAutoConfiguration::class.java)) + .withBean(DataSource::class.java, { counter as DataSource }) + .withBean(MessageDeliverer::class.java, { recorder }) + .withBean(PlatformTransactionManager::class.java, { SpringTransactionManager(counter) }) + .withBean(PostgresOutboxStore::class.java, { + PostgresOutboxStore(SpringConnectionProvider(counter), Clock.systemUTC()) + }) + + test("publish inside Spring TX driven by Exposed-bridge PTM uses a single physical connection") { + // Disable processor only — purger is left at its default 1h interval so it never ticks + // during this test, but its enabled=true keeps `okapiTransactionRunner` factory active + // (the factory is gated on at least one scheduler being enabled). + runner(RecordingMessageDeliverer()) + .withPropertyValues("okapi.processor.enabled=false") + .run { ctx -> + ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() + + resetCounterAndTruncate(counter) + + val publisher = ctx.getBean(SpringOutboxPublisher::class.java) + val tm = ctx.getBean(PlatformTransactionManager::class.java) + + TransactionTemplate(tm).execute { + publisher.publish( + OutboxMessage("order.created", """{"orderId":"abc-123"}"""), + RecordingDeliveryInfo, + ) + } + + counter.opened.get() shouldBe counter.closed.get() + counter.opened.get() shouldBe 1 + } + } + + test("processor tick under Exposed-bridge PTM claims and delivers a published entry") { + val recorder = RecordingMessageDeliverer() + runner(recorder) + .withPropertyValues("okapi.processor.interval=200ms") + .run { ctx -> + resetCounterAndTruncate(counter) + + val publisher = ctx.getBean(SpringOutboxPublisher::class.java) + val tm = ctx.getBean(PlatformTransactionManager::class.java) + + TransactionTemplate(tm).execute { + publisher.publish( + OutboxMessage("order.created", """{"orderId":"xyz-789"}"""), + RecordingDeliveryInfo, + ) + } + + val deadline = System.currentTimeMillis() + 5_000 + while (recorder.deliveryCount() == 0 && System.currentTimeMillis() < deadline) { + Thread.sleep(50) + } + recorder.deliveryCount() shouldBe 1 + } + } + + // The single-process happy-path tests above would silently pass even if the autoconfig had + // re-introduced the auto-commit fallback (the bug KOJAK-67 fixes). This test exercises + // contention: 5 concurrent processor invocations against 50 published entries, each tick + // bracketed by the autoconfig-built TransactionRunner. With proper TX bracketing the + // FOR UPDATE SKIP LOCKED rows stay locked across claim+update — no amplification. With a + // no-op TR (or auto-commit fallback) the lock is released between claim and update, + // multiple processors deliver the same entry, and `assertNoAmplification` throws. + test("autoconfig-built TransactionRunner prevents delivery amplification under concurrent processor invocations") { + val recorder = RecordingMessageDeliverer() + runner(recorder) + // Disable processor only — purger stays at its default 1h interval (won't fire in test) + // but keeps `okapiTransactionRunner` factory active. + .withPropertyValues("okapi.processor.enabled=false") + .run { ctx -> + resetCounterAndTruncate(counter) + + val publisher = ctx.getBean(SpringOutboxPublisher::class.java) + val tm = ctx.getBean(PlatformTransactionManager::class.java) + val processor = ctx.getBean(OutboxProcessor::class.java) + val transactionRunner = ctx.getBean(TransactionRunner::class.java) + + val entryCount = 50 + val processorCount = 5 + + repeat(entryCount) { i -> + TransactionTemplate(tm).execute { + publisher.publish( + OutboxMessage("test.event", """{"i":$i}"""), + RecordingDeliveryInfo, + ) + } + } + + val barrier = CyclicBarrier(processorCount) + val executor = Executors.newVirtualThreadPerTaskExecutor() + val futures = (1..processorCount).map { + CompletableFuture.supplyAsync( + { + barrier.await(10, TimeUnit.SECONDS) + transactionRunner.runInTransaction { processor.processNext(entryCount) } + }, + executor, + ) + } + CompletableFuture.allOf(*futures.toTypedArray()).get(30, TimeUnit.SECONDS) + executor.shutdown() + + recorder.assertNoAmplification() + recorder.deliveryCount() shouldBe entryCount + } + } +}) + +private fun resetCounterAndTruncate(counter: CountingDataSource) { + counter.delegate.connection.use { c -> + c.createStatement().use { it.execute("TRUNCATE TABLE okapi_outbox") } + } + counter.opened.set(0) + counter.closed.set(0) +} + +private object RecordingDeliveryInfo : DeliveryInfo { + override val type: String = "recording" + override fun serialize(): String = """{"type":"recording"}""" +} diff --git a/okapi-spring-boot/build.gradle.kts b/okapi-spring-boot/build.gradle.kts index 9033019..25f6003 100644 --- a/okapi-spring-boot/build.gradle.kts +++ b/okapi-spring-boot/build.gradle.kts @@ -1,10 +1,39 @@ plugins { id("buildsrc.convention.kotlin-jvm") id("buildsrc.convention.publish") + // Mutation testing — opt-in only: ./gradlew :okapi-spring-boot:pitest -PenableMutationTesting=true + id("info.solidsoft.pitest") version "1.19.0" apply false } description = "Spring Boot autoconfiguration for Okapi" +if (providers.gradleProperty("enableMutationTesting").orNull?.toBoolean() == true) { + apply(plugin = "info.solidsoft.pitest") + configure { + pitestVersion.set("1.17.0") + junit5PluginVersion.set("1.2.1") + targetClasses.set( + listOf( + "com.softwaremill.okapi.springboot.OutboxAutoConfiguration*", + "com.softwaremill.okapi.springboot.OutboxProcessorScheduler*", + "com.softwaremill.okapi.springboot.OutboxPurgerScheduler*", + "com.softwaremill.okapi.springboot.OkapiProperties*", + "com.softwaremill.okapi.springboot.SpringTransactionRunner*", + ), + ) + targetTests.set(listOf("com.softwaremill.okapi.springboot.*")) + excludedTestClasses.set( + listOf( + // Postgres testcontainer-based tests are too heavy per-mutation + "com.softwaremill.okapi.springboot.OutboxMysqlEndToEndTest", + ), + ) + threads.set(4) + outputFormats.set(listOf("HTML", "XML")) + timestampedReports.set(false) + } +} + dependencies { implementation(project(":okapi-core")) @@ -46,7 +75,8 @@ dependencies { // Brings in the metrics auto-config jar so @AutoConfigureAfter targets are resolvable in tests. testImplementation(libs.springBootStarterActuator) // Logback's ListAppender is used to capture and assert WARN-level log output (e.g. the - // LiquibaseDisabledNotice breadcrumb) — slf4j-simple does not provide an introspectable appender. + // LiquibaseDisabledNotice breadcrumb + our PTM↔DS validation cannot-verify WARN) — slf4j-simple + // does not provide an introspectable appender. testImplementation(libs.logbackClassic) } diff --git a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OkapiProperties.kt b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OkapiProperties.kt index d67d66c..2df8d16 100644 --- a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OkapiProperties.kt +++ b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OkapiProperties.kt @@ -7,12 +7,17 @@ import org.springframework.validation.annotation.Validated @Validated data class OkapiProperties( val datasourceQualifier: String? = null, + val transactionManagerQualifier: String? = null, val liquibase: Liquibase = Liquibase(), ) { init { require(datasourceQualifier == null || datasourceQualifier.isNotBlank()) { "okapi.datasource-qualifier must not be blank. Set it to the bean name of the outbox DataSource, or remove the property." } + require(transactionManagerQualifier == null || transactionManagerQualifier.isNotBlank()) { + "okapi.transaction-manager-qualifier must not be blank. Set it to the bean name of the outbox " + + "PlatformTransactionManager, or remove the property." + } } /** diff --git a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt index 2eb6940..1cf0670 100644 --- a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt +++ b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt @@ -10,17 +10,24 @@ import com.softwaremill.okapi.core.OutboxPurgerConfig import com.softwaremill.okapi.core.OutboxSchedulerConfig import com.softwaremill.okapi.core.OutboxStore import com.softwaremill.okapi.core.RetryPolicy +import com.softwaremill.okapi.core.TransactionRunner import com.softwaremill.okapi.mysql.MysqlOutboxStore import com.softwaremill.okapi.postgres.PostgresOutboxStore +import org.slf4j.LoggerFactory +import org.springframework.beans.factory.BeanFactory +import org.springframework.beans.factory.NoSuchBeanDefinitionException import org.springframework.beans.factory.ObjectProvider import org.springframework.boot.autoconfigure.AutoConfiguration import org.springframework.boot.autoconfigure.condition.ConditionalOnClass +import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty import org.springframework.boot.context.properties.EnableConfigurationProperties import org.springframework.context.annotation.Bean import org.springframework.context.annotation.Configuration +import org.springframework.jdbc.datasource.DelegatingDataSource import org.springframework.transaction.PlatformTransactionManager +import org.springframework.transaction.support.ResourceTransactionManager import org.springframework.transaction.support.TransactionTemplate import java.time.Clock import javax.sql.DataSource @@ -28,6 +35,8 @@ import javax.sql.DataSource /** * Spring Boot autoconfiguration for the outbox processing pipeline. * + * Requires a [TransactionRunner] bean, or a [PlatformTransactionManager] from which one can be derived. + * * Required beans (must be provided by the application): * - One or more [MessageDeliverer] beans — transport implementations * (e.g. HttpMessageDeliverer, KafkaMessageDeliverer). @@ -40,14 +49,15 @@ import javax.sql.DataSource * If both are present, Postgres takes priority. Override by defining your own `@Bean OutboxStore`. * - [Clock] — defaults to [Clock.systemUTC] * - [RetryPolicy] — defaults to `maxRetries = 5` - * - [PlatformTransactionManager] — when present, scheduler/purger wrap each tick in a Spring - * transaction. When absent, store calls run in JDBC auto-commit mode, which narrows - * `FOR UPDATE SKIP LOCKED` to the claim itself and allows duplicate delivery across - * processor instances; configure one for any multi-instance deployment. * - * Multi-datasource support: + * Multi-datasource / multi-PlatformTransactionManager support: * - Set `okapi.datasource-qualifier` to the bean name of the [DataSource] that holds the outbox table. * When not set, the primary (or single) DataSource is used. + * - Set `okapi.transaction-manager-qualifier` to the bean name of the + * [PlatformTransactionManager] that brackets the outbox DataSource. When not set, the unique + * (or `@Primary`) PTM is used. With multi-DS setups always set this explicitly — silent + * PTM↔DataSource mismatch otherwise reduces `FOR UPDATE SKIP LOCKED` to JDBC auto-commit and + * permits duplicate delivery across processor instances. * * Liquibase support is provided by [OkapiLiquibaseAutoConfiguration], which is ordered after this * auto-config so that its `@ConditionalOnBean(OutboxStore::class)` gates can observe which @@ -76,6 +86,27 @@ class OutboxAutoConfiguration( fun springOutboxPublisher(outboxPublisher: OutboxPublisher): SpringOutboxPublisher = SpringOutboxPublisher(delegate = outboxPublisher, dataSource = resolveDataSource()) + // Created only when at least one scheduler is enabled — TransactionRunner has no other consumer. + // Skipping the bean in publish-only deployments (both schedulers disabled) lets users run without + // a PlatformTransactionManager on the classpath at all (e.g. message producer that delegates + // outbox processing to a separate worker). + @Bean + @ConditionalOnMissingBean + @ConditionalOnExpression("\${okapi.processor.enabled:true} or \${okapi.purger.enabled:true}") + fun okapiTransactionRunner( + transactionManager: ObjectProvider, + beanFactory: BeanFactory, + ): TransactionRunner { + val ptm = resolvePlatformTransactionManager(transactionManager, beanFactory, okapiProperties) + validatePtmDataSourceMatch(ptm, resolveDataSource(), okapiProperties) + // Sets the *initial* TX read-only flag to false. NOTE: with PROPAGATION_REQUIRED (the default), + // a tick that joins an outer @Transactional(readOnly = true) inherits the outer's flag and + // this setting is silently ignored. The scheduler runs on a daemon thread with no outer TX, + // so this flag actually takes effect — but invocations from inside an existing read-only TX + // would still hit FOR UPDATE failures. Keep scheduler invocations outside @Transactional scopes. + return SpringTransactionRunner(TransactionTemplate(ptm).apply { isReadOnly = false }) + } + @Bean @ConditionalOnMissingBean fun outboxEntryProcessor( @@ -113,11 +144,11 @@ class OutboxAutoConfiguration( fun outboxProcessorScheduler( props: OutboxProcessorProperties, outboxProcessor: OutboxProcessor, - transactionManager: ObjectProvider, + transactionRunner: TransactionRunner, ): OutboxProcessorScheduler { return OutboxProcessorScheduler( outboxProcessor = outboxProcessor, - transactionTemplate = transactionManager.getIfAvailable()?.let { TransactionTemplate(it) }, + transactionRunner = transactionRunner, config = OutboxSchedulerConfig( interval = props.interval, batchSize = props.batchSize, @@ -131,12 +162,12 @@ class OutboxAutoConfiguration( fun outboxPurgerScheduler( props: OutboxPurgerProperties, outboxStore: OutboxStore, - transactionManager: ObjectProvider, + transactionRunner: TransactionRunner, clock: ObjectProvider, ): OutboxPurgerScheduler { return OutboxPurgerScheduler( outboxStore = outboxStore, - transactionTemplate = transactionManager.getIfAvailable()?.let { TransactionTemplate(it) }, + transactionRunner = transactionRunner, config = OutboxPurgerConfig( retention = props.retention, interval = props.interval, @@ -182,6 +213,8 @@ class OutboxAutoConfiguration( } companion object { + private val logger = LoggerFactory.getLogger(OutboxAutoConfiguration::class.java) + internal fun resolveDataSource( dataSources: Map, primaryDataSource: DataSource, @@ -195,5 +228,124 @@ class OutboxAutoConfiguration( "Available: ${dataSources.keys}", ) } + + internal fun resolvePlatformTransactionManager( + provider: ObjectProvider, + beanFactory: BeanFactory, + properties: OkapiProperties, + ): PlatformTransactionManager { + val qualifier = properties.transactionManagerQualifier + if (qualifier != null) { + return try { + beanFactory.getBean(qualifier, PlatformTransactionManager::class.java) + } catch (e: NoSuchBeanDefinitionException) { + throw NoSuchBeanDefinitionException( + qualifier, + "okapi.transaction-manager-qualifier='$qualifier' — no PlatformTransactionManager bean named " + + "'$qualifier' found. Check the bean name or remove the property to fall back to " + + "auto-resolution.", + ).apply { initCause(e) } + } + } + val unique = provider.getIfUnique() + if (unique != null) return unique + val available = provider.stream().toList() + throw NoSuchBeanDefinitionException( + TransactionRunner::class.java, + if (available.isEmpty()) { + "okapi-spring-boot requires a TransactionRunner bean to bracket each scheduler tick in a " + + "transaction. Configure spring-boot-starter-jdbc or spring-boot-starter-data-jpa (which " + + "provide a PlatformTransactionManager that okapi adapts automatically), or define your own " + + "@Bean TransactionRunner." + } else { + "Multiple PlatformTransactionManager beans found (${available.size}), none marked as @Primary. " + + "Mark the outbox PTM as @Primary, set okapi.transaction-manager-qualifier to disambiguate, " + + "or define an explicit @Bean TransactionRunner." + }, + ) + } + + internal fun validatePtmDataSourceMatch( + ptm: PlatformTransactionManager, + outboxDataSource: DataSource, + properties: OkapiProperties, + ) { + // Only ResourceTransactionManager exposes the underlying resource factory. For RTMs whose + // resourceFactory is a JDBC DataSource (DataSourceTransactionManager and similar), we can + // verify it matches okapi's outbox DataSource. RTMs whose resource is something else + // (JpaTransactionManager → EntityManagerFactory, HibernateTransactionManager → SessionFactory) + // and non-RTM PTMs (Exposed's SpringTransactionManager, JtaTransactionManager) fall through + // to the "cannot verify" WARN path. + val ptmDataSource = (ptm as? ResourceTransactionManager)?.resourceFactory as? DataSource + if (ptmDataSource != null) { + // Spring's recommended pattern wraps the outbox DataSource bean in TransactionAwareDataSourceProxy + // (or LazyConnectionDataSourceProxy) for use by query helpers, while passing the raw DataSource + // to the PTM (Spring docs explicitly say "TransactionAwareDataSourceProxy should NOT be passed + // to a PTM"). Reference equality on those references would falsely fail. Unwrap the + // DelegatingDataSource chain on both sides before comparison. + val unwrappedPtm = unwrapDataSource(ptmDataSource) + val unwrappedOutbox = unwrapDataSource(outboxDataSource) + if (unwrappedPtm !== unwrappedOutbox) { + // If either side is still a DelegatingDataSource after unwrap, the chain terminated + // early — either a cycle (`setTargetDataSource(self)`) or a not-yet-initialised + // `LazyConnectionDataSourceProxy.targetDataSource == null`. Surface that as a distinct + // WARN so the operator looks at the proxy wiring instead of chasing the PTM↔DS error. + if (unwrappedPtm is DelegatingDataSource || unwrappedOutbox is DelegatingDataSource) { + logger.warn( + "Could not fully unwrap one or both DataSource sides — at least one " + + "DelegatingDataSource chain terminated early (cycle, or " + + "LazyConnectionDataSourceProxy with targetDataSource not yet set). " + + "PTM side: {} (stopped at {}). Outbox side: {} (stopped at {}). If the " + + "two are intended to wrap the same DataSource, fix the proxy chain " + + "before relying on the PTM↔DataSource mismatch error below.", + ptmDataSource, + unwrappedPtm, + outboxDataSource, + unwrappedOutbox, + ) + } + error( + "PlatformTransactionManager '${ptm.javaClass.name}' is bound to a different DataSource than " + + "okapi's outbox DataSource. PTM DataSource: $ptmDataSource. Outbox DataSource: " + + "$outboxDataSource (resolved via okapi.datasource-qualifier=" + + "'${properties.datasourceQualifier ?: ""}'). Each scheduler tick would otherwise " + + "wrap a transaction on the wrong DataSource and FOR UPDATE SKIP LOCKED would collapse " + + "to JDBC auto-commit, allowing duplicate delivery. Fix: set okapi.transaction-manager-" + + "qualifier to point at the PTM that brackets the outbox DataSource, or define an explicit " + + "@Bean TransactionRunner.", + ) + } + return + } + if (properties.datasourceQualifier != null) { + logger.warn( + "okapi.datasource-qualifier='{}' is set, but the resolved PlatformTransactionManager '{}' does " + + "not expose a JDBC DataSource (e.g. JpaTransactionManager exposes EntityManagerFactory, " + + "HibernateTransactionManager exposes SessionFactory; non-ResourceTransactionManager PTMs like " + + "JtaTransactionManager or Exposed's SpringTransactionManager don't expose any resource factory " + + "at all) — okapi cannot verify it brackets the outbox DataSource. If the PTM " + + "is bound to a different DataSource, scheduler ticks will silently run in JDBC auto-commit " + + "mode and FOR UPDATE SKIP LOCKED will collapse, allowing duplicate delivery across processor " + + "instances. Set okapi.transaction-manager-qualifier to disambiguate, or define an explicit " + + "@Bean TransactionRunner.", + properties.datasourceQualifier, + ptm.javaClass.name, + ) + } + } + + // Iterative + visited-set: defends against self-referencing or cyclic DelegatingDataSource + // chains (Spring's `setTargetDataSource` is a public setter with no cycle check, so + // misconfiguration like `proxy.setTargetDataSource(proxy)` is legal API). A tailrec form + // would compile to an uninterruptible JVM goto loop and silently spin at startup. + internal fun unwrapDataSource(ds: DataSource): DataSource { + val seen = mutableSetOf() + var current: DataSource = ds + while (current is DelegatingDataSource) { + if (!seen.add(current)) return current + current = current.targetDataSource ?: return current + } + return current + } } } diff --git a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxProcessorScheduler.kt b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxProcessorScheduler.kt index d4fc485..4a1c461 100644 --- a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxProcessorScheduler.kt +++ b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxProcessorScheduler.kt @@ -3,8 +3,8 @@ package com.softwaremill.okapi.springboot import com.softwaremill.okapi.core.OutboxProcessor import com.softwaremill.okapi.core.OutboxScheduler import com.softwaremill.okapi.core.OutboxSchedulerConfig +import com.softwaremill.okapi.core.TransactionRunner import org.springframework.context.SmartLifecycle -import org.springframework.transaction.support.TransactionTemplate /** * Spring lifecycle wrapper for [OutboxScheduler]. @@ -17,13 +17,13 @@ import org.springframework.transaction.support.TransactionTemplate */ class OutboxProcessorScheduler( outboxProcessor: OutboxProcessor, - transactionTemplate: TransactionTemplate?, + transactionRunner: TransactionRunner, config: OutboxSchedulerConfig = OutboxSchedulerConfig(), ) : SmartLifecycle { private val scheduler = OutboxScheduler( outboxProcessor = outboxProcessor, - transactionRunner = transactionTemplate?.let { SpringTransactionRunner(it) }, + transactionRunner = transactionRunner, config = config, ) diff --git a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxPurgerScheduler.kt b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxPurgerScheduler.kt index 1251ed3..2753837 100644 --- a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxPurgerScheduler.kt +++ b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxPurgerScheduler.kt @@ -3,8 +3,8 @@ package com.softwaremill.okapi.springboot import com.softwaremill.okapi.core.OutboxPurger import com.softwaremill.okapi.core.OutboxPurgerConfig import com.softwaremill.okapi.core.OutboxStore +import com.softwaremill.okapi.core.TransactionRunner import org.springframework.context.SmartLifecycle -import org.springframework.transaction.support.TransactionTemplate import java.time.Clock /** @@ -15,14 +15,14 @@ import java.time.Clock */ class OutboxPurgerScheduler( outboxStore: OutboxStore, - transactionTemplate: TransactionTemplate? = null, + transactionRunner: TransactionRunner, config: OutboxPurgerConfig = OutboxPurgerConfig(), clock: Clock = Clock.systemUTC(), ) : SmartLifecycle { private val purger = OutboxPurger( outboxStore = outboxStore, - transactionRunner = transactionTemplate?.let { SpringTransactionRunner(it) }, + transactionRunner = transactionRunner, config = config, clock = clock, ) diff --git a/okapi-spring-boot/src/main/resources/META-INF/spring-configuration-metadata.json b/okapi-spring-boot/src/main/resources/META-INF/spring-configuration-metadata.json index 78305fe..214d96e 100644 --- a/okapi-spring-boot/src/main/resources/META-INF/spring-configuration-metadata.json +++ b/okapi-spring-boot/src/main/resources/META-INF/spring-configuration-metadata.json @@ -81,6 +81,12 @@ "defaultValue": null, "description": "Name of the DataSource bean to use for the outbox. When set, okapi uses this specific DataSource instead of the primary one. Required in multi-datasource setups where the outbox table is not in the primary database." }, + { + "name": "okapi.transaction-manager-qualifier", + "type": "java.lang.String", + "defaultValue": null, + "description": "Name of the PlatformTransactionManager bean that brackets the outbox DataSource. When not set, the unique (or @Primary) PTM is used. In multi-PTM setups always set this explicitly so okapi resolves the PTM bound to the outbox DataSource instead of the application's primary; silent PTM↔DataSource mismatch otherwise reduces FOR UPDATE SKIP LOCKED to JDBC auto-commit and permits duplicate delivery across processor instances." + }, { "name": "okapi.metrics.refresh-interval", "type": "java.time.Duration", diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/DataSourceQualifierAutoConfigurationTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/DataSourceQualifierAutoConfigurationTest.kt index 8bb5c15..c45ad39 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/DataSourceQualifierAutoConfigurationTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/DataSourceQualifierAutoConfigurationTest.kt @@ -5,6 +5,7 @@ import com.softwaremill.okapi.core.MessageDeliverer import com.softwaremill.okapi.core.OutboxEntry import com.softwaremill.okapi.core.OutboxStatus import com.softwaremill.okapi.core.OutboxStore +import com.softwaremill.okapi.core.TransactionRunner import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.nulls.shouldNotBeNull import io.kotest.matchers.string.shouldContain @@ -23,6 +24,7 @@ class DataSourceQualifierAutoConfigurationTest : FunSpec({ .withConfiguration(AutoConfigurations.of(OutboxAutoConfiguration::class.java)) .withBean(OutboxStore::class.java, { stubStore() }) .withBean(MessageDeliverer::class.java, { stubDeliverer() }) + .withBean(TransactionRunner::class.java, { noOpTransactionRunner() }) test("no qualifier set, single datasource — uses that datasource") { val ds = SimpleDriverDataSource() @@ -89,6 +91,10 @@ class DataSourceQualifierAutoConfigurationTest : FunSpec({ } }) +private fun noOpTransactionRunner() = object : TransactionRunner { + override fun runInTransaction(block: () -> T): T = block() +} + private fun stubStore() = object : OutboxStore { override fun persist(entry: OutboxEntry) = entry override fun claimPending(limit: Int) = emptyList() diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/LiquibaseAutoConfigurationTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/LiquibaseAutoConfigurationTest.kt index f31ec54..0a91432 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/LiquibaseAutoConfigurationTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/LiquibaseAutoConfigurationTest.kt @@ -9,6 +9,7 @@ import com.softwaremill.okapi.core.MessageDeliverer import com.softwaremill.okapi.core.OutboxEntry import com.softwaremill.okapi.core.OutboxStatus import com.softwaremill.okapi.core.OutboxStore +import com.softwaremill.okapi.core.TransactionRunner import io.kotest.assertions.throwables.shouldThrow import io.kotest.assertions.withClue import io.kotest.core.spec.style.FunSpec @@ -63,6 +64,7 @@ class LiquibaseAutoConfigurationTest : FunSpec({ .withBean(OutboxStore::class.java, { stubStore() }) .withBean(MessageDeliverer::class.java, { stubDeliverer() }) .withBean(DataSource::class.java, { SimpleDriverDataSource() }) + .withBean(TransactionRunner::class.java, { noOpTransactionRunner() }) .withOkapiLiquibaseDisabled() context("postgres liquibase") { @@ -183,6 +185,7 @@ class LiquibaseAutoConfigurationTest : FunSpec({ .withBean(OutboxStore::class.java, { stubStore() }) .withBean(MessageDeliverer::class.java, { stubDeliverer() }) .withBean(DataSource::class.java, { SimpleDriverDataSource() }) + .withBean(TransactionRunner::class.java, { noOpTransactionRunner() }) .withPropertyValues("okapi.liquibase.enabled=true") .run { ctx -> ctx.getBeansOfType(OkapiLiquibaseAutoConfiguration.LiquibaseDisabledNotice::class.java) @@ -214,6 +217,7 @@ class LiquibaseAutoConfigurationTest : FunSpec({ .withBean(OutboxStore::class.java, { stubStore() }) .withBean(MessageDeliverer::class.java, { stubDeliverer() }) .withBean(DataSource::class.java, { SimpleDriverDataSource() }) + .withBean(TransactionRunner::class.java, { noOpTransactionRunner() }) .withPropertyValues("okapi.liquibase.enabled=false") .run { ctx -> ctx.getBeansOfType(OkapiLiquibaseAutoConfiguration.LiquibaseDisabledNotice::class.java) @@ -413,6 +417,7 @@ class LiquibaseAutoConfigurationTest : FunSpec({ .withConfiguration(AutoConfigurations.of(OutboxAutoConfiguration::class.java, OkapiLiquibaseAutoConfiguration::class.java)) .withBean(MessageDeliverer::class.java, { stubDeliverer() }) .withBean(DataSource::class.java, { SimpleDriverDataSource() }) + .withBean(TransactionRunner::class.java, { noOpTransactionRunner() }) .withInitializer { ctx -> ctx.beanFactory.addBeanPostProcessor(SuppressSpringLiquibaseRun()) } @@ -466,6 +471,7 @@ class LiquibaseAutoConfigurationTest : FunSpec({ .withConfiguration(AutoConfigurations.of(OutboxAutoConfiguration::class.java, OkapiLiquibaseAutoConfiguration::class.java)) .withBean(MessageDeliverer::class.java, { stubDeliverer() }) .withBean(DataSource::class.java, { SimpleDriverDataSource() }) + .withBean(TransactionRunner::class.java, { noOpTransactionRunner() }) .withBean("okapiPostgresLiquibase", SpringLiquibase::class.java, { userBean }) .withInitializer { ctx -> ctx.beanFactory.addBeanPostProcessor(SuppressSpringLiquibaseRun()) @@ -509,6 +515,10 @@ private fun canLoadClass(fqcn: String, classLoader: ClassLoader): Boolean = try false } +private fun noOpTransactionRunner() = object : TransactionRunner { + override fun runInTransaction(block: () -> T): T = block() +} + private fun stubStore() = object : OutboxStore { override fun persist(entry: OutboxEntry) = entry override fun claimPending(limit: Int) = emptyList() diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt new file mode 100644 index 0000000..0f63952 --- /dev/null +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt @@ -0,0 +1,463 @@ +package com.softwaremill.okapi.springboot + +import ch.qos.logback.classic.Level +import ch.qos.logback.classic.Logger +import ch.qos.logback.classic.spi.ILoggingEvent +import ch.qos.logback.core.read.ListAppender +import com.softwaremill.okapi.core.DeliveryResult +import com.softwaremill.okapi.core.MessageDeliverer +import com.softwaremill.okapi.core.OutboxEntry +import com.softwaremill.okapi.core.OutboxStatus +import com.softwaremill.okapi.core.OutboxStore +import com.softwaremill.okapi.core.TransactionRunner +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.collections.shouldNotBeEmpty +import io.kotest.matchers.nulls.shouldBeNull +import io.kotest.matchers.nulls.shouldNotBeNull +import io.kotest.matchers.shouldBe +import io.kotest.matchers.string.shouldContain +import io.kotest.matchers.types.shouldBeInstanceOf +import io.kotest.matchers.types.shouldBeSameInstanceAs +import org.h2.jdbcx.JdbcDataSource +import org.slf4j.LoggerFactory +import org.springframework.beans.factory.NoSuchBeanDefinitionException +import org.springframework.beans.factory.support.BeanDefinitionBuilder +import org.springframework.boot.autoconfigure.AutoConfigurations +import org.springframework.boot.test.context.runner.ApplicationContextRunner +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Configuration +import org.springframework.context.annotation.Primary +import org.springframework.context.support.GenericApplicationContext +import org.springframework.jdbc.datasource.DataSourceTransactionManager +import org.springframework.jdbc.datasource.LazyConnectionDataSourceProxy +import org.springframework.jdbc.datasource.SimpleDriverDataSource +import org.springframework.jdbc.datasource.TransactionAwareDataSourceProxy +import org.springframework.transaction.PlatformTransactionManager +import org.springframework.transaction.TransactionDefinition +import org.springframework.transaction.support.AbstractPlatformTransactionManager +import org.springframework.transaction.support.DefaultTransactionStatus +import org.springframework.transaction.support.ResourceTransactionManager +import org.springframework.transaction.support.TransactionSynchronizationManager +import java.time.Instant +import javax.sql.DataSource +import kotlin.time.Duration.Companion.seconds + +class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ + + val baseRunner = ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(OutboxAutoConfiguration::class.java)) + .withBean(OutboxStore::class.java, { stubStore() }) + .withBean(MessageDeliverer::class.java, { stubDeliverer() }) + + test("derives SpringTransactionRunner from PlatformTransactionManager when present") { + val ds: DataSource = SimpleDriverDataSource() + baseRunner + .withBean(DataSource::class.java, { ds }) + .withBean(PlatformTransactionManager::class.java, { DataSourceTransactionManager(ds) }) + .run { ctx -> + ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() + } + } + + test("auto-built TransactionTemplate is NOT read-only (defends against globally read-only TX defaults)") { + val ds: DataSource = h2DataSource() + baseRunner + .withBean(DataSource::class.java, { ds }) + .withBean(PlatformTransactionManager::class.java, { DataSourceTransactionManager(ds) }) + .run { ctx -> + val runner = ctx.getBean(TransactionRunner::class.java) + runner.runInTransaction { TransactionSynchronizationManager.isCurrentTransactionReadOnly() } shouldBe false + } + } + + test("publish-only deployment: both schedulers disabled + no PTM + no user TransactionRunner — context starts cleanly") { + // Publish-only mode (e.g. message-producing service that delegates outbox processing to a separate + // worker). The TransactionRunner is only needed by scheduler/purger; with both disabled, no PTM + // should be required. Without this gate, okapi-spring-boot would prevent users from running + // publish-only with arbitrary PTMs (or none at all). + baseRunner + .withBean(DataSource::class.java, { SimpleDriverDataSource() }) + .withPropertyValues( + "okapi.processor.enabled=false", + "okapi.purger.enabled=false", + ) + .run { ctx -> + ctx.startupFailure.shouldBeNull() + ctx.containsBean("okapiTransactionRunner") shouldBe false + ctx.containsBean("outboxProcessorScheduler") shouldBe false + ctx.containsBean("outboxPurgerScheduler") shouldBe false + ctx.getBean(SpringOutboxPublisher::class.java).shouldNotBeNull() + } + } + + test("fails context refresh with actionable message when no PTM and no user TransactionRunner") { + baseRunner + .withBean(DataSource::class.java, { SimpleDriverDataSource() }) + .run { ctx -> + val failure = ctx.startupFailure + failure.shouldNotBeNull() + val rootCause = generateSequence(failure as Throwable?) { it.cause }.last() + rootCause.shouldBeInstanceOf() + rootCause.message.shouldNotBeNull().also { + it.shouldContain("okapi-spring-boot requires a TransactionRunner bean") + it.shouldContain("PlatformTransactionManager") + it.shouldContain("@Bean TransactionRunner") + } + } + } + + test("user-provided TransactionRunner bean is honoured without PTM (@ConditionalOnMissingBean)") { + baseRunner + .withBean(DataSource::class.java, { SimpleDriverDataSource() }) + .withUserConfiguration(CustomRunnerConfiguration::class.java) + .run { ctx -> + ctx.getBean(TransactionRunner::class.java).shouldBeSameInstanceAs(CustomRunnerConfiguration.RUNNER) + } + } + + test("fails with distinct message when multiple PTMs are present and none is @Primary") { + val ds: DataSource = SimpleDriverDataSource() + baseRunner + .withBean(DataSource::class.java, { ds }) + .withUserConfiguration(TwoPtmsNoPrimaryUserConfig::class.java) + .run { ctx -> + val failure = ctx.startupFailure + failure.shouldNotBeNull() + val rootCause = generateSequence(failure as Throwable?) { it.cause }.last() + rootCause.shouldBeInstanceOf() + rootCause.message.shouldNotBeNull().also { + it.shouldContain("Multiple PlatformTransactionManager beans found") + it.shouldContain("@Primary") + it.shouldContain("okapi.transaction-manager-qualifier") + } + } + } + + test("uses @Primary PTM when multiple are present") { + val ds: DataSource = SimpleDriverDataSource() + baseRunner + .withBean(DataSource::class.java, { ds }) + .withUserConfiguration(PrimaryDstAndDummyPtmConfig::class.java) + .run { ctx -> + // PrimaryDstAndDummyPtmConfig.primaryTm is the @Primary DST → autoconfig should derive runner from it. + // If autoconfig (wrongly) picked the DummyTm, validation would not fire (DummyTm is not RTM) + // and the runner would still build — so we assert no startup failure as the strongest signal. + ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() + } + } + + test("okapi.transaction-manager-qualifier picks the named PTM (overrides @Primary), proven via DS-match validation") { + // Setup designed so a buggy "ignore qualifier" implementation would FAIL: + // - appDs (primary), outboxDs (secondary) DataSources + // - appTm = DST(appDs), @Primary + // - outboxTm = DST(outboxDs) + // - okapi.datasource-qualifier=outboxDs (so resolveDataSource() == outboxDs) + // - okapi.transaction-manager-qualifier=outboxTm + // Correct: qualifier picks outboxTm → validation: outboxTm.dataSource == outboxDs → ok. + // Buggy (qualifier ignored): @Primary appTm picked → validation: appDs ≠ outboxDs → fail-fast. + val appDs: DataSource = SimpleDriverDataSource() + val outboxDs: DataSource = SimpleDriverDataSource() + ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(OutboxAutoConfiguration::class.java)) + .withBean(OutboxStore::class.java, { stubStore() }) + .withBean(MessageDeliverer::class.java, { stubDeliverer() }) + .withInitializer { context -> + val gac = context as GenericApplicationContext + gac.registerBeanDefinition( + "appDs", + BeanDefinitionBuilder.genericBeanDefinition(DataSource::class.java) { appDs }.beanDefinition + .apply { isPrimary = true }, + ) + gac.registerBeanDefinition( + "outboxDs", + BeanDefinitionBuilder.genericBeanDefinition(DataSource::class.java) { outboxDs }.beanDefinition, + ) + gac.registerBeanDefinition( + "appTm", + BeanDefinitionBuilder.genericBeanDefinition(PlatformTransactionManager::class.java) { + DataSourceTransactionManager(appDs) + }.beanDefinition.apply { isPrimary = true }, + ) + gac.registerBeanDefinition( + "outboxTm", + BeanDefinitionBuilder.genericBeanDefinition(PlatformTransactionManager::class.java) { + DataSourceTransactionManager(outboxDs) + }.beanDefinition, + ) + } + .withPropertyValues( + "okapi.datasource-qualifier=outboxDs", + "okapi.transaction-manager-qualifier=outboxTm", + ) + .run { ctx -> + ctx.startupFailure.shouldBeNull() + ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() + } + } + + test("non-ResourceTransactionManager PTM with okapi.datasource-qualifier set: context starts (DS validation cannot run, WARN logged)") { + // DummyTransactionManager extends AbstractPlatformTransactionManager but does NOT implement + // ResourceTransactionManager — same shape as Exposed's SpringTransactionManager and JpaTransactionManager. + // Validation cannot extract the PTM's DataSource, so it logs a WARN and allows the context to start. + // This documents the non-fail-fast path so users without a DST-style PTM can still wire okapi. + val ds: DataSource = SimpleDriverDataSource() + baseRunner + .withBean("outboxDs", DataSource::class.java, { ds }) + .withBean(PlatformTransactionManager::class.java, { DummyTransactionManager() }) + .withPropertyValues("okapi.datasource-qualifier=outboxDs") + .run { ctx -> + ctx.startupFailure.shouldBeNull() + ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() + } + } + + test("okapi.transaction-manager-qualifier pointing to a nonexistent bean fails with actionable message") { + val ds: DataSource = SimpleDriverDataSource() + baseRunner + .withBean(DataSource::class.java, { ds }) + .withBean(PlatformTransactionManager::class.java, { DataSourceTransactionManager(ds) }) + .withPropertyValues("okapi.transaction-manager-qualifier=missingTm") + .run { ctx -> + val failure = ctx.startupFailure + failure.shouldNotBeNull() + val allMessages = generateSequence(failure as Throwable?) { it.cause }.mapNotNull { it.message }.toList() + allMessages.any { it.contains("okapi.transaction-manager-qualifier") } shouldBe true + allMessages.any { it.contains("missingTm") } shouldBe true + } + } + + test("PTM bound to a different DataSource than outbox fails-fast with PTM↔DS mismatch message") { + val appDs: DataSource = SimpleDriverDataSource() + val outboxDs: DataSource = SimpleDriverDataSource() + ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(OutboxAutoConfiguration::class.java)) + .withBean(OutboxStore::class.java, { stubStore() }) + .withBean(MessageDeliverer::class.java, { stubDeliverer() }) + .withInitializer { context -> + val gac = context as GenericApplicationContext + gac.registerBeanDefinition( + "appDs", + BeanDefinitionBuilder.genericBeanDefinition(DataSource::class.java) { appDs }.beanDefinition + .apply { isPrimary = true }, + ) + gac.registerBeanDefinition( + "outboxDs", + BeanDefinitionBuilder.genericBeanDefinition(DataSource::class.java) { outboxDs }.beanDefinition, + ) + gac.registerBeanDefinition( + "appTm", + BeanDefinitionBuilder.genericBeanDefinition(PlatformTransactionManager::class.java) { + DataSourceTransactionManager(appDs) + }.beanDefinition, + ) + } + .withPropertyValues("okapi.datasource-qualifier=outboxDs") + .run { ctx -> + val failure = ctx.startupFailure + failure.shouldNotBeNull() + val rootCause = generateSequence(failure as Throwable?) { it.cause }.last() + rootCause.message.shouldNotBeNull().also { + it.shouldContain("bound to a different DataSource") + it.shouldContain("FOR UPDATE SKIP LOCKED") + it.shouldContain("okapi.transaction-manager-qualifier") + } + } + } + + // ----------------------------------------------------------------------------------------- + // C1 regression guard: ResourceTransactionManager with non-DataSource resourceFactory. + // JpaTransactionManager and HibernateTransactionManager both implement ResourceTransactionManager + // but their resourceFactory is EntityManagerFactory or SessionFactory respectively — NOT a + // DataSource. (JtaTransactionManager doesn't implement RTM at all and falls through the same + // WARN branch as Exposed's SpringTransactionManager.) Earlier shape of validatePtmDataSourceMatch + // did `as? DataSource ?: return`, silently early-returning and bypassing the WARN. This test + // pins that the cast failure now falls through to the WARN branch instead. + // ----------------------------------------------------------------------------------------- + test( + "BUG C1: RTM with non-DataSource resourceFactory + okapi.datasource-qualifier set: WARN should be logged but currently is silent", + ) { + val ds: DataSource = SimpleDriverDataSource() + val jpaLikeTm = JpaLikeRtmTransactionManager(resourceFactory = Any()) + + val targetLogger = LoggerFactory.getLogger(OutboxAutoConfiguration::class.java) as Logger + val appender = ListAppender().apply { start() } + targetLogger.addAppender(appender) + try { + baseRunner + .withBean("outboxDs", DataSource::class.java, { ds }) + .withBean(PlatformTransactionManager::class.java, { jpaLikeTm }) + .withPropertyValues("okapi.datasource-qualifier=outboxDs") + .run { ctx -> + ctx.startupFailure.shouldBeNull() + ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() + } + val warningsAboutValidation = appender.list.filter { + it.level == Level.WARN && it.formattedMessage.contains("okapi.datasource-qualifier") + } + warningsAboutValidation.shouldNotBeEmpty() + } finally { + targetLogger.detachAppender(appender) + } + } + + // ----------------------------------------------------------------------------------------- + // C2 demonstration: TransactionAwareDataSourceProxy false-positive. + // Spring's recommended pattern for outbox-style scenarios is to register + // TransactionAwareDataSourceProxy(rawDs) as the application-facing bean for query helpers, while + // the PlatformTransactionManager is wired with the RAW DataSource (Spring docs explicitly say: + // "TransactionAwareDataSourceProxy should NOT be passed to a PTM"). With this correct pattern: + // - ptm.resourceFactory === rawDs + // - resolveDataSource() === proxyDs (the bean qualified by okapi.datasource-qualifier) + // Our validation uses reference equality (`!==`) → fail-fast on a *correctly* configured app. + // ----------------------------------------------------------------------------------------- + test("BUG C2: TransactionAwareDataSourceProxy(rawDs) on outbox bean + PTM(rawDs) is Spring's recommended pattern, must not fail") { + val rawDs: DataSource = SimpleDriverDataSource() + val proxyDs: DataSource = TransactionAwareDataSourceProxy(rawDs) + ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(OutboxAutoConfiguration::class.java)) + .withBean(OutboxStore::class.java, { stubStore() }) + .withBean(MessageDeliverer::class.java, { stubDeliverer() }) + .withInitializer { context -> + val gac = context as GenericApplicationContext + gac.registerBeanDefinition( + "outboxDs", + BeanDefinitionBuilder.genericBeanDefinition(DataSource::class.java) { proxyDs }.beanDefinition + .apply { isPrimary = true }, + ) + gac.registerBeanDefinition( + "outboxTm", + BeanDefinitionBuilder.genericBeanDefinition(PlatformTransactionManager::class.java) { + DataSourceTransactionManager(rawDs) + }.beanDefinition, + ) + } + .withPropertyValues("okapi.datasource-qualifier=outboxDs") + .run { ctx -> + ctx.startupFailure.shouldBeNull() + ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() + } + } + + // Direct unit tests for the unwrapDataSource helper. The integration-level BUG C2 test above + // covers the single-level TransactionAwareDataSourceProxy case via real autoconfig wiring. + // These tests pin the helper's contract for nested chains, null targets, and cycles. + context("unwrapDataSource") { + test("returns the input unchanged when not a DelegatingDataSource") { + val raw: DataSource = SimpleDriverDataSource() + OutboxAutoConfiguration.unwrapDataSource(raw) shouldBeSameInstanceAs raw + } + + test("unwraps a single-level TransactionAwareDataSourceProxy to its target") { + val raw: DataSource = SimpleDriverDataSource() + val proxy: DataSource = TransactionAwareDataSourceProxy(raw) + OutboxAutoConfiguration.unwrapDataSource(proxy) shouldBeSameInstanceAs raw + } + + test("unwraps a nested chain TADP -> LCDP -> raw down to the raw DataSource") { + val raw: DataSource = SimpleDriverDataSource() + val nested: DataSource = TransactionAwareDataSourceProxy(LazyConnectionDataSourceProxy(raw)) + OutboxAutoConfiguration.unwrapDataSource(nested) shouldBeSameInstanceAs raw + } + + test("returns the proxy itself when DelegatingDataSource has null targetDataSource (graceful, no NPE)") { + // LazyConnectionDataSourceProxy ships with a no-arg constructor that leaves targetDataSource null + // until setTargetDataSource is called. At validation time some users may have such partially- + // initialised proxies. We must not NPE; we return the proxy itself so the caller's reference + // comparison can proceed (and produce a clear "DataSource mismatch" error if applicable). + val proxy: DataSource = LazyConnectionDataSourceProxy() + OutboxAutoConfiguration.unwrapDataSource(proxy) shouldBeSameInstanceAs proxy + } + + test("does NOT hang on a self-referencing DelegatingDataSource (cycle detection)").config(timeout = 2.seconds) { + val cyclic = LazyConnectionDataSourceProxy() + cyclic.setTargetDataSource(cyclic) + // After A3 fix: cycle detected, returns the cyclic proxy itself. + // Without A3 fix: tailrec compiles to goto → infinite CPU spin → test times out and fails. + OutboxAutoConfiguration.unwrapDataSource(cyclic) shouldBeSameInstanceAs cyclic + } + + test("does NOT hang on a longer cycle (A -> B -> A)").config(timeout = 2.seconds) { + val a = LazyConnectionDataSourceProxy() + val b = LazyConnectionDataSourceProxy() + a.setTargetDataSource(b) + b.setTargetDataSource(a) + // Walk: a -> b -> a; on the second visit to a the set already contains it, so the + // helper returns the current node (a). Any deterministic non-hanging answer is + // acceptable — the contract is "don't loop forever", and the caller will fall back + // to the WARN-or-error path because the unwrapped value isn't a real backing DataSource. + OutboxAutoConfiguration.unwrapDataSource(a) shouldBeSameInstanceAs a + } + } +}) + +// Mimics JpaTransactionManager / HibernateTransactionManager: implements ResourceTransactionManager +// but exposes a non-DataSource (EntityManagerFactory / SessionFactory) as the resource factory. +// JtaTransactionManager does NOT implement ResourceTransactionManager — it falls through the +// non-RTM branch alongside Exposed's SpringTransactionManager. +private class JpaLikeRtmTransactionManager(private val resourceFactory: Any) : + AbstractPlatformTransactionManager(), + ResourceTransactionManager { + override fun getResourceFactory(): Any = resourceFactory + override fun doGetTransaction(): Any = Any() + override fun doBegin(transaction: Any, definition: TransactionDefinition) {} + override fun doCommit(status: DefaultTransactionStatus) {} + override fun doRollback(status: DefaultTransactionStatus) {} +} + +@Configuration(proxyBeanMethods = false) +private class CustomRunnerConfiguration { + @Bean + fun customRunner(): TransactionRunner = RUNNER + + companion object { + val RUNNER: TransactionRunner = object : TransactionRunner { + override fun runInTransaction(block: () -> T): T = block() + } + } +} + +@Configuration(proxyBeanMethods = false) +private class TwoPtmsNoPrimaryUserConfig { + @Bean + fun firstTm(): PlatformTransactionManager = DummyTransactionManager() + + @Bean + fun secondTm(): PlatformTransactionManager = DummyTransactionManager() +} + +@Configuration(proxyBeanMethods = false) +private class PrimaryDstAndDummyPtmConfig { + @Bean + @Primary + fun primaryTm(ds: DataSource): PlatformTransactionManager = DataSourceTransactionManager(ds) + + @Bean + fun secondaryDummyTm(): PlatformTransactionManager = DummyTransactionManager() +} + +private class DummyTransactionManager : AbstractPlatformTransactionManager() { + override fun doGetTransaction(): Any = Any() + override fun doBegin(transaction: Any, definition: org.springframework.transaction.TransactionDefinition) {} + override fun doCommit(status: DefaultTransactionStatus) {} + override fun doRollback(status: DefaultTransactionStatus) {} +} + +private fun h2DataSource(): DataSource = JdbcDataSource().apply { + setURL("jdbc:h2:mem:probe-tx-runner-${System.nanoTime()};DB_CLOSE_DELAY=-1") + user = "sa" + password = "" +} + +private fun stubStore() = object : OutboxStore { + override fun persist(entry: OutboxEntry) = entry + override fun claimPending(limit: Int) = emptyList() + override fun updateAfterProcessing(entry: OutboxEntry) = entry + override fun removeDeliveredBefore(time: Instant, limit: Int) = 0 + override fun findOldestCreatedAt(statuses: Set) = emptyMap() + override fun countByStatuses() = emptyMap() +} + +private fun stubDeliverer() = object : MessageDeliverer { + override val type = "stub" + override fun deliver(entry: OutboxEntry) = DeliveryResult.Success +} diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxProcessorAutoConfigurationTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxProcessorAutoConfigurationTest.kt index 38a60ac..15f0b5c 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxProcessorAutoConfigurationTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxProcessorAutoConfigurationTest.kt @@ -3,8 +3,10 @@ package com.softwaremill.okapi.springboot import com.softwaremill.okapi.core.DeliveryResult import com.softwaremill.okapi.core.MessageDeliverer import com.softwaremill.okapi.core.OutboxEntry +import com.softwaremill.okapi.core.OutboxEntryProcessor import com.softwaremill.okapi.core.OutboxStatus import com.softwaremill.okapi.core.OutboxStore +import com.softwaremill.okapi.core.TransactionRunner import com.softwaremill.okapi.micrometer.MicrometerOutboxListener import com.softwaremill.okapi.micrometer.MicrometerOutboxMetrics import com.softwaremill.okapi.micrometer.OutboxMetricsRefresher @@ -30,6 +32,7 @@ class OutboxProcessorAutoConfigurationTest : FunSpec({ .withBean(OutboxStore::class.java, { stubStore() }) .withBean(MessageDeliverer::class.java, { stubDeliverer() }) .withBean(DataSource::class.java, { SimpleDriverDataSource() }) + .withBean(TransactionRunner::class.java, { noOpTransactionRunner() }) test("processor bean is created by default") { contextRunner.run { ctx -> @@ -69,10 +72,19 @@ class OutboxProcessorAutoConfigurationTest : FunSpec({ } } - test("SmartLifecycle is running after context start") { + test("SmartLifecycle is running after context start, and stop() actually halts it") { contextRunner.run { ctx -> val scheduler = ctx.getBean(OutboxProcessorScheduler::class.java) scheduler.isRunning shouldBe true + scheduler.stop() + scheduler.isRunning shouldBe false + } + } + + test("getPhase returns PROCESSOR_PHASE constant (orders before purger)") { + contextRunner.run { ctx -> + val scheduler = ctx.getBean(OutboxProcessorScheduler::class.java) + scheduler.phase shouldBe OutboxProcessorScheduler.PROCESSOR_PHASE } } @@ -84,15 +96,26 @@ class OutboxProcessorAutoConfigurationTest : FunSpec({ } } - test("stop callback is always invoked") { + test("stop(callback) invokes callback AND actually halts the scheduler") { contextRunner.run { ctx -> val scheduler = ctx.getBean(OutboxProcessorScheduler::class.java) var callbackInvoked = false scheduler.stop { callbackInvoked = true } callbackInvoked shouldBe true + scheduler.isRunning shouldBe false } } + test("multiple MessageDeliverer beans are wrapped in CompositeMessageDeliverer (routed by deliveryType)") { + contextRunner + .withBean("secondDeliverer", MessageDeliverer::class.java, { stubDelivererWithType("second") }) + .run { ctx -> + val processor = ctx.getBean(OutboxEntryProcessor::class.java) + processor.shouldNotBeNull() + ctx.getBeansOfType(MessageDeliverer::class.java).size shouldBe 2 + } + } + test("listener, metrics and refresher are wired when a MeterRegistry bean is provided directly") { contextRunner .withBean(io.micrometer.core.instrument.MeterRegistry::class.java, { @@ -159,6 +182,7 @@ class OutboxProcessorAutoConfigurationTest : FunSpec({ .withBean(OutboxStore::class.java, { stubStore() }) .withBean(MessageDeliverer::class.java, { stubDeliverer() }) .withBean(DataSource::class.java, { SimpleDriverDataSource() }) + .withBean(TransactionRunner::class.java, { noOpTransactionRunner() }) .run { ctx -> ctx.getBean(io.micrometer.core.instrument.MeterRegistry::class.java).shouldNotBeNull() ctx.getBean(MicrometerOutboxListener::class.java).shouldNotBeNull() @@ -208,6 +232,10 @@ private fun resolveSpringBootClass(vararg candidateFqcns: String): Class<*> { } ?: error("None of $candidateFqcns resolves on this Spring Boot runtime; check spring-boot-starter-actuator on the test classpath.") } +private fun noOpTransactionRunner() = object : TransactionRunner { + override fun runInTransaction(block: () -> T): T = block() +} + private fun stubStore() = object : OutboxStore { override fun persist(entry: OutboxEntry) = entry override fun claimPending(limit: Int) = emptyList() @@ -217,7 +245,9 @@ private fun stubStore() = object : OutboxStore { override fun countByStatuses() = emptyMap() } -private fun stubDeliverer() = object : MessageDeliverer { - override val type = "stub" +private fun stubDeliverer() = stubDelivererWithType("stub") + +private fun stubDelivererWithType(t: String) = object : MessageDeliverer { + override val type = t override fun deliver(entry: OutboxEntry) = DeliveryResult.Success } diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxPurgerAutoConfigurationTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxPurgerAutoConfigurationTest.kt index 9c89735..ba7c6ca 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxPurgerAutoConfigurationTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxPurgerAutoConfigurationTest.kt @@ -6,6 +6,7 @@ import com.softwaremill.okapi.core.OutboxEntry import com.softwaremill.okapi.core.OutboxPurgerConfig import com.softwaremill.okapi.core.OutboxStatus import com.softwaremill.okapi.core.OutboxStore +import com.softwaremill.okapi.core.TransactionRunner import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.nulls.shouldNotBeNull import io.kotest.matchers.shouldBe @@ -25,6 +26,7 @@ class OutboxPurgerAutoConfigurationTest : FunSpec({ .withBean(OutboxStore::class.java, { stubStore() }) .withBean(MessageDeliverer::class.java, { stubDeliverer() }) .withBean(DataSource::class.java, { SimpleDriverDataSource() }) + .withBean(TransactionRunner::class.java, { noOpTransactionRunner() }) test("purger bean is created by default") { contextRunner.run { ctx -> @@ -64,10 +66,19 @@ class OutboxPurgerAutoConfigurationTest : FunSpec({ } } - test("SmartLifecycle is running after context start") { + test("SmartLifecycle is running after context start, and stop() actually halts it") { contextRunner.run { ctx -> val scheduler = ctx.getBean(OutboxPurgerScheduler::class.java) scheduler.isRunning shouldBe true + scheduler.stop() + scheduler.isRunning shouldBe false + } + } + + test("getPhase returns PURGER_PHASE constant (orders after processor)") { + contextRunner.run { ctx -> + val scheduler = ctx.getBean(OutboxPurgerScheduler::class.java) + scheduler.phase shouldBe OutboxPurgerScheduler.PURGER_PHASE } } @@ -79,9 +90,10 @@ class OutboxPurgerAutoConfigurationTest : FunSpec({ } } - test("stop callback is always invoked") { + test("stop(callback) invokes callback AND actually halts the purger") { val scheduler = OutboxPurgerScheduler( outboxStore = stubStore(), + transactionRunner = noOpTransactionRunner(), config = OutboxPurgerConfig(interval = ofMinutes(60)), ) scheduler.start() @@ -90,9 +102,14 @@ class OutboxPurgerAutoConfigurationTest : FunSpec({ scheduler.stop { callbackInvoked = true } callbackInvoked shouldBe true + scheduler.isRunning shouldBe false } }) +private fun noOpTransactionRunner() = object : TransactionRunner { + override fun runInTransaction(block: () -> T): T = block() +} + private fun stubStore() = object : OutboxStore { override fun persist(entry: OutboxEntry) = entry override fun claimPending(limit: Int) = emptyList() diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringObjectProviderSemanticsAssumptionsTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringObjectProviderSemanticsAssumptionsTest.kt new file mode 100644 index 0000000..b36f7ff --- /dev/null +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringObjectProviderSemanticsAssumptionsTest.kt @@ -0,0 +1,107 @@ +package com.softwaremill.okapi.springboot + +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.nulls.shouldBeNull +import io.kotest.matchers.shouldBe +import io.kotest.matchers.types.shouldBeSameInstanceAs +import org.springframework.beans.factory.NoUniqueBeanDefinitionException +import org.springframework.boot.test.context.runner.ApplicationContextRunner +import org.springframework.context.annotation.Bean +import org.springframework.context.annotation.Configuration +import org.springframework.context.annotation.Primary +import org.springframework.jdbc.datasource.DataSourceTransactionManager +import org.springframework.jdbc.datasource.SimpleDriverDataSource +import org.springframework.transaction.PlatformTransactionManager +import org.springframework.transaction.support.AbstractPlatformTransactionManager +import org.springframework.transaction.support.DefaultTransactionStatus +import org.springframework.transaction.support.ResourceTransactionManager + +/** + * Pins the Spring 7.0.7 `ObjectProvider` and `PlatformTransactionManager` semantics that + * `OutboxAutoConfiguration.okapiTransactionRunner` relies on. If a future Spring upgrade + * changes any of these behaviors, these tests fail loudly so the autoconfig logic gets + * audited rather than silently breaking. + * + * Specifically the autoconfig assumes: + * 1. `getIfUnique()` returns null (NOT throws) when 2+ PTM beans exist without `@Primary`. + * Used to distinguish "no PTM" from "multiple PTMs" via stream count. + * 2. `getIfUnique()` returns the `@Primary` bean when present. + * 3. `getIfAvailable()` THROWS `NoUniqueBeanDefinitionException` for 2+ non-primary + * candidates — the previous fix used this and surfaced a misleading error; switching to + * `getIfUnique()` was a deliberate semantic change. + * 4. `DataSourceTransactionManager` IS-A `ResourceTransactionManager`, with + * `resourceFactory == DataSource`. Our PTM↔DS validation depends on this cast. + * 5. PTMs that extend `AbstractPlatformTransactionManager` directly (e.g. Exposed + * `SpringTransactionManager`, `JpaTransactionManager`) do NOT implement + * `ResourceTransactionManager` — meaning we cannot extract their DataSource for validation + * and must fall back to a WARN log + require explicit `okapi.transaction-manager-qualifier`. + */ +class SpringObjectProviderSemanticsAssumptionsTest : FunSpec({ + + test("ObjectProvider.getIfUnique() returns the @Primary PTM when multiple PTMs exist") { + ApplicationContextRunner() + .withUserConfiguration(TwoPtmsWithPrimaryConfig::class.java) + .run { ctx -> + ctx.getBeanProvider(PlatformTransactionManager::class.java).getIfUnique() + .shouldBeSameInstanceAs(ctx.getBean("primaryTm", PlatformTransactionManager::class.java)) + } + } + + test("ObjectProvider.getIfUnique() returns null for 2+ non-primary PTMs (does NOT throw)") { + ApplicationContextRunner() + .withUserConfiguration(TwoPtmsNoPrimaryConfig::class.java) + .run { ctx -> + ctx.getBeanProvider(PlatformTransactionManager::class.java).getIfUnique().shouldBeNull() + } + } + + test( + "ObjectProvider.getIfAvailable() THROWS NoUniqueBeanDefinitionException for 2+ non-primary PTMs (this is why we use getIfUnique)", + ) { + ApplicationContextRunner() + .withUserConfiguration(TwoPtmsNoPrimaryConfig::class.java) + .run { ctx -> + val provider = ctx.getBeanProvider(PlatformTransactionManager::class.java) + val thrown = runCatching { provider.getIfAvailable() }.exceptionOrNull() + (thrown is NoUniqueBeanDefinitionException) shouldBe true + } + } + + test("DataSourceTransactionManager implements ResourceTransactionManager and exposes its DataSource via resourceFactory") { + val ds = SimpleDriverDataSource() + val dst: PlatformTransactionManager = DataSourceTransactionManager(ds) + (dst is ResourceTransactionManager) shouldBe true + (dst as ResourceTransactionManager).resourceFactory.shouldBeSameInstanceAs(ds) + } + + test("AbstractPlatformTransactionManager subclasses (e.g. Exposed bridge / JPA-style) do NOT implement ResourceTransactionManager") { + val tm: PlatformTransactionManager = DummyAbstractPtm() + (tm is ResourceTransactionManager) shouldBe false + } +}) + +@Configuration(proxyBeanMethods = false) +private class TwoPtmsNoPrimaryConfig { + @Bean + fun firstTm(): PlatformTransactionManager = DummyAbstractPtm() + + @Bean + fun secondTm(): PlatformTransactionManager = DummyAbstractPtm() +} + +@Configuration(proxyBeanMethods = false) +private class TwoPtmsWithPrimaryConfig { + @Bean + @Primary + fun primaryTm(): PlatformTransactionManager = DummyAbstractPtm() + + @Bean + fun secondaryTm(): PlatformTransactionManager = DummyAbstractPtm() +} + +private class DummyAbstractPtm : AbstractPlatformTransactionManager() { + override fun doGetTransaction(): Any = Any() + override fun doBegin(transaction: Any, definition: org.springframework.transaction.TransactionDefinition) {} + override fun doCommit(status: DefaultTransactionStatus) {} + override fun doRollback(status: DefaultTransactionStatus) {} +} From f296a44ed7a0f65bbfa18c234d27857dcd439b88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20Kobyli=C5=84ski?= Date: Sat, 16 May 2026 19:56:32 +0200 Subject: [PATCH 2/8] =?UTF-8?q?fix:=20harden=20Spring=20autoconfig=20?= =?UTF-8?q?=E2=80=94=20diagnostics,=20user-template=20honoured,=20qualifie?= =?UTF-8?q?r=20safety?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review-round findings on top of the KOJAK-67 baseline: - okapiTransactionRunner factory now reads ObjectProvider and reuses a unique user-supplied bean instead of silently building a fresh TransactionTemplate(ptm). User intent (timeout, propagation, isolation) flows into scheduler ticks. - resolvePlatformTransactionManager catches BeanNotOfRequiredTypeException (typo into a DataSource bean name) and rewraps with an okapi-specific message instead of leaking Spring's generic error. - validatePtmDataSourceMatch now reports the actual resourceFactory class in the WARN message (covers custom RTM implementations) and emits an INFO breadcrumb in the single-DataSource case (no qualifier set, non-RTM PTM) so future multi-DS migrations have a grep target. Tests: - 4 new slice tests in OutboxAutoConfigurationTransactionRunnerTest covering: * property binding: okapi.transaction-manager-qualifier kebab → camelCase * blank-string validation via Spring Binder * BeanNotOfRequiredTypeException → actionable error * user @Bean TransactionTemplate honoured (reference identity check) * actual resourceFactory class surfaced in WARN * INFO breadcrumb in single-DS non-RTM scenario - ExposedSpringBridgeEndToEndTest gains a purger-tick assertion under the Exposed bridge PTM (DELIVERED rows past retention are actually removed). - Existing non-RTM WARN test extended with ListAppender-based content assertions (does not implement RTM / okapi.transaction-manager-qualifier mention) so the diagnostic cannot rot silently. Production code adjustments: - SpringTransactionRunner.transactionTemplate is now internal val so tests can assert reference identity with the user-supplied template. - validatePtmDataSourceMatch computes resourceFactoryDescription once and reuses it across the WARN (multi-DS hint) and INFO (single-DS hint) branches. --- .../ExposedSpringBridgeEndToEndTest.kt | 49 +++++ .../springboot/OutboxAutoConfiguration.kt | 69 +++++-- .../springboot/SpringTransactionRunner.kt | 2 +- ...xAutoConfigurationTransactionRunnerTest.kt | 184 ++++++++++++++++-- 4 files changed, 281 insertions(+), 23 deletions(-) diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/ExposedSpringBridgeEndToEndTest.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/ExposedSpringBridgeEndToEndTest.kt index 5e7b83d..aa8d48f 100644 --- a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/ExposedSpringBridgeEndToEndTest.kt +++ b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/ExposedSpringBridgeEndToEndTest.kt @@ -178,8 +178,57 @@ class ExposedSpringBridgeEndToEndTest : FunSpec({ recorder.deliveryCount() shouldBe entryCount } } + + // Purger uses a different code path than the processor — native SQL delete with limit, no + // claim/update state machine. Under the Exposed `SpringTransactionManager` bridge this needs + // its own E2E coverage: a regression where the bridge mishandles bracketing of the bulk delete + // (e.g. an Exposed upgrade that changes statement execution) would silently leave DELIVERED + // rows accumulating without breaking any other test. + test("purger tick under Exposed-bridge PTM removes DELIVERED entries past retention") { + val recorder = RecordingMessageDeliverer() + runner(recorder) + .withPropertyValues( + "okapi.processor.interval=100ms", + "okapi.purger.interval=200ms", + "okapi.purger.retention=1ms", + ) + .run { ctx -> + resetCounterAndTruncate(counter) + + val publisher = ctx.getBean(SpringOutboxPublisher::class.java) + val tm = ctx.getBean(PlatformTransactionManager::class.java) + + TransactionTemplate(tm).execute { + publisher.publish( + OutboxMessage("test.purger", """{"k":"v"}"""), + RecordingDeliveryInfo, + ) + } + + val deliveredDeadline = System.currentTimeMillis() + 5_000 + while (recorder.deliveryCount() == 0 && System.currentTimeMillis() < deliveredDeadline) { + Thread.sleep(50) + } + recorder.deliveryCount() shouldBe 1 + + val purgedDeadline = System.currentTimeMillis() + 5_000 + while (rowCount(counter) > 0 && System.currentTimeMillis() < purgedDeadline) { + Thread.sleep(100) + } + rowCount(counter) shouldBe 0 + } + } }) +private fun rowCount(counter: CountingDataSource): Int = counter.delegate.connection.use { c -> + c.createStatement().use { stmt -> + stmt.executeQuery("SELECT COUNT(*) FROM okapi_outbox").use { rs -> + rs.next() + rs.getInt(1) + } + } +} + private fun resetCounterAndTruncate(counter: CountingDataSource) { counter.delegate.connection.use { c -> c.createStatement().use { it.execute("TRUNCATE TABLE okapi_outbox") } diff --git a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt index 1cf0670..e911c90 100644 --- a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt +++ b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt @@ -15,6 +15,7 @@ import com.softwaremill.okapi.mysql.MysqlOutboxStore import com.softwaremill.okapi.postgres.PostgresOutboxStore import org.slf4j.LoggerFactory import org.springframework.beans.factory.BeanFactory +import org.springframework.beans.factory.BeanNotOfRequiredTypeException import org.springframework.beans.factory.NoSuchBeanDefinitionException import org.springframework.beans.factory.ObjectProvider import org.springframework.boot.autoconfigure.AutoConfiguration @@ -95,8 +96,17 @@ class OutboxAutoConfiguration( @ConditionalOnExpression("\${okapi.processor.enabled:true} or \${okapi.purger.enabled:true}") fun okapiTransactionRunner( transactionManager: ObjectProvider, + transactionTemplate: ObjectProvider, beanFactory: BeanFactory, ): TransactionRunner { + // If the user has defined exactly one @Bean TransactionTemplate, honour their settings + // (timeout, propagation, isolation, etc.) instead of silently building a fresh one from + // the PTM. Same intent as @ConditionalOnMissingBean on this factory, just at a finer + // granularity — the user's template is THE source of truth for TX semantics. + val userTemplate = transactionTemplate.getIfUnique() + if (userTemplate != null) { + return SpringTransactionRunner(userTemplate) + } val ptm = resolvePlatformTransactionManager(transactionManager, beanFactory, okapiProperties) validatePtmDataSourceMatch(ptm, resolveDataSource(), okapiProperties) // Sets the *initial* TX read-only flag to false. NOTE: with PROPAGATION_REQUIRED (the default), @@ -245,6 +255,18 @@ class OutboxAutoConfiguration( "'$qualifier' found. Check the bean name or remove the property to fall back to " + "auto-resolution.", ).apply { initCause(e) } + } catch (e: BeanNotOfRequiredTypeException) { + // Common typo: qualifier points to e.g. a DataSource bean name instead of a PTM + // bean name. Spring's default message ("Bean named 'X' is expected to be of type ... + // but was actually of type ...") doesn't mention okapi, so users searching for + // "okapi" in startup logs find nothing. Rewrap with okapi-specific context. + throw IllegalStateException( + "okapi.transaction-manager-qualifier='$qualifier' — bean named '$qualifier' exists " + + "but is of type '${e.actualType.name}', not a PlatformTransactionManager. Check " + + "the property value (likely a typo into a DataSource or other bean name) or " + + "remove it to fall back to auto-resolution.", + e, + ) } } val unique = provider.getIfUnique() @@ -275,8 +297,10 @@ class OutboxAutoConfiguration( // verify it matches okapi's outbox DataSource. RTMs whose resource is something else // (JpaTransactionManager → EntityManagerFactory, HibernateTransactionManager → SessionFactory) // and non-RTM PTMs (Exposed's SpringTransactionManager, JtaTransactionManager) fall through - // to the "cannot verify" WARN path. - val ptmDataSource = (ptm as? ResourceTransactionManager)?.resourceFactory as? DataSource + // to the "cannot verify" WARN path. Capture the actual resourceFactory class so the WARN + // reports what the user's PTM actually exposed (not just the static JPA/Hibernate examples). + val rtmResourceFactory = (ptm as? ResourceTransactionManager)?.resourceFactory + val ptmDataSource = rtmResourceFactory as? DataSource if (ptmDataSource != null) { // Spring's recommended pattern wraps the outbox DataSource bean in TransactionAwareDataSourceProxy // (or LazyConnectionDataSourceProxy) for use by query helpers, while passing the raw DataSource @@ -317,19 +341,42 @@ class OutboxAutoConfiguration( } return } + val resourceFactoryDescription = when { + ptm !is ResourceTransactionManager -> + "does not implement ResourceTransactionManager (no resource factory exposed; same shape as " + + "JtaTransactionManager or Exposed's SpringTransactionManager)" + rtmResourceFactory == null -> + "implements ResourceTransactionManager but its getResourceFactory() returned null" + else -> + "implements ResourceTransactionManager but its resourceFactory is of type " + + "'${rtmResourceFactory.javaClass.name}', not a JDBC DataSource (typical for JPA's " + + "EntityManagerFactory or Hibernate's SessionFactory)" + } if (properties.datasourceQualifier != null) { logger.warn( - "okapi.datasource-qualifier='{}' is set, but the resolved PlatformTransactionManager '{}' does " + - "not expose a JDBC DataSource (e.g. JpaTransactionManager exposes EntityManagerFactory, " + - "HibernateTransactionManager exposes SessionFactory; non-ResourceTransactionManager PTMs like " + - "JtaTransactionManager or Exposed's SpringTransactionManager don't expose any resource factory " + - "at all) — okapi cannot verify it brackets the outbox DataSource. If the PTM " + - "is bound to a different DataSource, scheduler ticks will silently run in JDBC auto-commit " + - "mode and FOR UPDATE SKIP LOCKED will collapse, allowing duplicate delivery across processor " + - "instances. Set okapi.transaction-manager-qualifier to disambiguate, or define an explicit " + - "@Bean TransactionRunner.", + "okapi.datasource-qualifier='{}' is set, but the resolved PlatformTransactionManager '{}' {} " + + "— okapi cannot verify it brackets the outbox DataSource. If the PTM is bound to a different " + + "DataSource, scheduler ticks will silently run in JDBC auto-commit mode and FOR UPDATE SKIP " + + "LOCKED will collapse, allowing duplicate delivery across processor instances. Set " + + "okapi.transaction-manager-qualifier to disambiguate, or define an explicit @Bean " + + "TransactionRunner.", properties.datasourceQualifier, ptm.javaClass.name, + resourceFactoryDescription, + ) + } else { + // No qualifier set → single-DS assumption. We cannot validate, but a future multi-DS + // migration that forgets to set the qualifier would silently break with no diagnostic. + // Emit an INFO breadcrumb so an operator debugging duplicate delivery has something + // to grep for. + logger.info( + "PlatformTransactionManager '{}' {} — okapi cannot verify it brackets the outbox " + + "DataSource. Assuming single-DataSource setup (okapi.datasource-qualifier is " + + "unset). If you have or add multiple DataSources, set okapi.transaction-manager-" + + "qualifier (and okapi.datasource-qualifier) explicitly to avoid silent " + + "PTM↔DataSource mismatch.", + ptm.javaClass.name, + resourceFactoryDescription, ) } } diff --git a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionRunner.kt b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionRunner.kt index 60ca8b1..1a7238c 100644 --- a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionRunner.kt +++ b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionRunner.kt @@ -7,7 +7,7 @@ import org.springframework.transaction.support.TransactionTemplate * Spring implementation of [TransactionRunner] using [TransactionTemplate]. */ class SpringTransactionRunner( - private val transactionTemplate: TransactionTemplate, + internal val transactionTemplate: TransactionTemplate, ) : TransactionRunner { override fun runInTransaction(block: () -> T): T = transactionTemplate.execute { block() }!! } diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt index 0f63952..87c7035 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt @@ -70,6 +70,45 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ } } + test("okapi.transaction-manager-qualifier YAML key binds to OkapiProperties.transactionManagerQualifier (kebab → camelCase)") { + // Pins the property name contract — without this a future rename of the Kotlin field + // (e.g. transactionManagerQualifier → txManagerQualifier) silently breaks user + // configuration. Mirror of LiquibaseAutoConfigurationTest's "okapi.liquibase.* properties + // bind to nested config" test. + baseRunner + .withBean(DataSource::class.java, { SimpleDriverDataSource() }) + .withBean(TransactionRunner::class.java, { + object : TransactionRunner { + override fun runInTransaction(block: () -> T): T = block() + } + }) + .withPropertyValues("okapi.transaction-manager-qualifier=outboxTm") + .run { ctx -> + ctx.getBean(OkapiProperties::class.java).transactionManagerQualifier shouldBe "outboxTm" + } + } + + test("blank okapi.transaction-manager-qualifier triggers startup failure with require() message") { + // Pins that init { require(isNotBlank()) } actually propagates through Spring's Binder. + // A future refactor of OkapiProperties that moves the require() outside init or onto a + // getter would silently let blank qualifiers through and cause a confusing + // NoSuchBeanDefinitionException at PTM lookup. Mirror of LiquibaseAutoConfigurationTest's + // "blank changelog-table property triggers startup failure". + baseRunner + .withBean(DataSource::class.java, { SimpleDriverDataSource() }) + .withBean(TransactionRunner::class.java, { + object : TransactionRunner { + override fun runInTransaction(block: () -> T): T = block() + } + }) + .withPropertyValues("okapi.transaction-manager-qualifier= ") + .run { ctx -> + val rootCause = generateSequence(ctx.startupFailure) { it.cause }.last() + rootCause.message.shouldNotBeNull() + .shouldContain("okapi.transaction-manager-qualifier must not be blank") + } + } + test("publish-only deployment: both schedulers disabled + no PTM + no user TransactionRunner — context starts cleanly") { // Publish-only mode (e.g. message-producing service that delegates outbox processing to a separate // worker). The TransactionRunner is only needed by scheduler/purger; with both disabled, no PTM @@ -106,6 +145,22 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ } } + test("user-provided @Bean TransactionTemplate is honoured (autoconfig does not silently shadow it)") { + // If a user defines a custom TransactionTemplate (e.g. with non-default timeout / propagation / + // isolation), the autoconfig MUST use that exact template — not silently build its own fresh + // TransactionTemplate(ptm) from the PTM. A "silent shadow" regression would discard the user's + // intent for scheduler ticks while their @Transactional code paths use the configured template. + val ds: DataSource = SimpleDriverDataSource() + baseRunner + .withBean(DataSource::class.java, { ds }) + .withBean(PlatformTransactionManager::class.java, { DataSourceTransactionManager(ds) }) + .withUserConfiguration(CustomTransactionTemplateConfiguration::class.java) + .run { ctx -> + val runner = ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() + runner.transactionTemplate.shouldBeSameInstanceAs(CustomTransactionTemplateConfiguration.TEMPLATE) + } + } + test("user-provided TransactionRunner bean is honoured without PTM (@ConditionalOnMissingBean)") { baseRunner .withBean(DataSource::class.java, { SimpleDriverDataSource() }) @@ -195,20 +250,86 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ } } - test("non-ResourceTransactionManager PTM with okapi.datasource-qualifier set: context starts (DS validation cannot run, WARN logged)") { + test("ResourceTransactionManager PTM with non-DataSource resourceFactory: WARN includes actual resourceFactory class name") { + // BUG C1 test (above, around line 254) covers the WARN-is-emitted contract for this case. + // This test pins the diagnostic CONTENT: the WARN must mention the actual resourceFactory + // class so a user with a custom RTM (or buggy subclass returning null) sees the real + // resource type, not just the static example list (JpaTransactionManager/Hibernate). Without + // this assertion a refactor that removed the dynamic class reference from the WARN would + // silently downgrade the diagnostic for non-JPA users. + val ds: DataSource = SimpleDriverDataSource() + val customRtm = JpaLikeRtmTransactionManager(resourceFactory = "myDistinctiveResourceFactory") + val targetLogger = LoggerFactory.getLogger(OutboxAutoConfiguration::class.java) as Logger + val appender = ListAppender().apply { start() } + targetLogger.addAppender(appender) + try { + baseRunner + .withBean("outboxDs", DataSource::class.java, { ds }) + .withBean(PlatformTransactionManager::class.java, { customRtm }) + .withPropertyValues("okapi.datasource-qualifier=outboxDs") + .run { ctx -> ctx.startupFailure.shouldBeNull() } + val warnText = appender.list.filter { it.level == Level.WARN }.joinToString("\n") { it.formattedMessage } + // resourceFactory is a String — WARN must mention its class so user sees what was actually + // exposed, not just the JpaTransactionManager/Hibernate examples. + warnText.shouldContain("java.lang.String") + } finally { + targetLogger.detachAppender(appender) + } + } + + test("non-ResourceTransactionManager PTM without okapi.datasource-qualifier (single-DS assumption): INFO breadcrumb emitted") { + // When validation cannot run AND no qualifier is set, the autoconfig assumes single-DS — but + // a future multi-DS migration that forgets to set the qualifier would silently break with no + // diagnostic. An INFO breadcrumb at startup gives the operator something to grep for when + // duplicate delivery starts to occur post-migration. + val ds: DataSource = SimpleDriverDataSource() + val targetLogger = LoggerFactory.getLogger(OutboxAutoConfiguration::class.java) as Logger + val appender = ListAppender().apply { start() } + targetLogger.addAppender(appender) + try { + baseRunner + .withBean(DataSource::class.java, { ds }) + .withBean(PlatformTransactionManager::class.java, { DummyTransactionManager() }) + // no okapi.datasource-qualifier + .run { ctx -> ctx.startupFailure.shouldBeNull() } + val infos = appender.list.filter { it.level == Level.INFO } + val combined = infos.joinToString("\n") { it.formattedMessage } + combined.shouldContain("does not implement ResourceTransactionManager") + combined.shouldContain("okapi.transaction-manager-qualifier") + } finally { + targetLogger.detachAppender(appender) + } + } + + test("non-ResourceTransactionManager PTM with okapi.datasource-qualifier set: context starts AND emits actionable WARN") { // DummyTransactionManager extends AbstractPlatformTransactionManager but does NOT implement - // ResourceTransactionManager — same shape as Exposed's SpringTransactionManager and JpaTransactionManager. + // ResourceTransactionManager — same shape as Exposed's SpringTransactionManager. // Validation cannot extract the PTM's DataSource, so it logs a WARN and allows the context to start. - // This documents the non-fail-fast path so users without a DST-style PTM can still wire okapi. + // Captures the WARN content so deletion of the warn() body (or its message format) is also caught — + // not just deletion of the conditional that emits it. Without this assertion, the operator-facing + // breadcrumb could silently rot without breaking any other test. val ds: DataSource = SimpleDriverDataSource() - baseRunner - .withBean("outboxDs", DataSource::class.java, { ds }) - .withBean(PlatformTransactionManager::class.java, { DummyTransactionManager() }) - .withPropertyValues("okapi.datasource-qualifier=outboxDs") - .run { ctx -> - ctx.startupFailure.shouldBeNull() - ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() - } + val targetLogger = LoggerFactory.getLogger(OutboxAutoConfiguration::class.java) as Logger + val appender = ListAppender().apply { start() } + targetLogger.addAppender(appender) + try { + baseRunner + .withBean("outboxDs", DataSource::class.java, { ds }) + .withBean(PlatformTransactionManager::class.java, { DummyTransactionManager() }) + .withPropertyValues("okapi.datasource-qualifier=outboxDs") + .run { ctx -> + ctx.startupFailure.shouldBeNull() + ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() + } + val warnings = appender.list.filter { it.level == Level.WARN } + warnings.shouldNotBeEmpty() + val combined = warnings.joinToString("\n") { it.formattedMessage } + combined.shouldContain("okapi.datasource-qualifier") + combined.shouldContain("does not implement ResourceTransactionManager") + combined.shouldContain("okapi.transaction-manager-qualifier") + } finally { + targetLogger.detachAppender(appender) + } } test("okapi.transaction-manager-qualifier pointing to a nonexistent bean fails with actionable message") { @@ -226,6 +347,29 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ } } + test("okapi.transaction-manager-qualifier pointing to a bean of wrong type (e.g. DataSource) fails with actionable message") { + // Common typo: user means `okapi.transaction-manager-qualifier=outboxTm` but writes + // `=outboxDs` (the DataSource bean name). Spring throws `BeanNotOfRequiredTypeException`, + // NOT `NoSuchBeanDefinitionException`, so a naive `catch (NoSuchBeanDefinitionException)` + // never wraps the error and the user sees a cryptic message without okapi context. + val outboxDs: DataSource = SimpleDriverDataSource() + baseRunner + .withBean("outboxDs", DataSource::class.java, { outboxDs }) + .withBean("outboxTm", PlatformTransactionManager::class.java, { DataSourceTransactionManager(outboxDs) }) + // typo: pointing to the DataSource bean instead of the PTM bean + .withPropertyValues("okapi.transaction-manager-qualifier=outboxDs") + .run { ctx -> + val failure = ctx.startupFailure + failure.shouldNotBeNull() + val allMessages = generateSequence(failure as Throwable?) { it.cause }.mapNotNull { it.message }.toList() + allMessages.any { it.contains("okapi.transaction-manager-qualifier") } shouldBe true + allMessages.any { it.contains("outboxDs") } shouldBe true + allMessages.any { + it.contains("not a PlatformTransactionManager") || it.contains("PlatformTransactionManager") + } shouldBe true + } + } + test("PTM bound to a different DataSource than outbox fails-fast with PTM↔DS mismatch message") { val appDs: DataSource = SimpleDriverDataSource() val outboxDs: DataSource = SimpleDriverDataSource() @@ -416,6 +560,24 @@ private class CustomRunnerConfiguration { } } +@Configuration(proxyBeanMethods = false) +private class CustomTransactionTemplateConfiguration { + @Bean + fun customTemplate(ptm: PlatformTransactionManager): org.springframework.transaction.support.TransactionTemplate { + TEMPLATE.transactionManager = ptm + return TEMPLATE + } + + companion object { + // Distinctive timeout makes the template easily identifiable; the autoconfig must not silently + // build its own TransactionTemplate(ptm) bypassing this one. + val TEMPLATE: org.springframework.transaction.support.TransactionTemplate = + org.springframework.transaction.support.TransactionTemplate().apply { + timeout = 42 + } + } +} + @Configuration(proxyBeanMethods = false) private class TwoPtmsNoPrimaryUserConfig { @Bean From 069cc990dcc9895baf9185dbbaf56141867b8d9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20Kobyli=C5=84ski?= Date: Sun, 17 May 2026 15:07:10 +0200 Subject: [PATCH 3/8] =?UTF-8?q?feat(spring-boot):=20detect=20Jpa/Hibernate?= =?UTF-8?q?TransactionManager=20DataSource=20and=20always=20validate=20PTM?= =?UTF-8?q?=E2=86=94DS=20binding=20(KOJAK-67)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes a silent-failure path discovered by Spring Boot Test verification: TransactionAutoConfiguration auto-creates a TransactionTemplate bean whenever a single PlatformTransactionManager exists. The previous factory took that TT via ObjectProvider.getIfUnique() and short-circuited PTM resolution + validatePtmDataSourceMatch — every Spring Boot user with multi-DS wiring silently bypassed the safety net. Production logic: - extractDataSource() reflects JpaTransactionManager.getDataSource() and HibernateTransactionManager.getDataSource() (both Spring 6.1- and 6.2+ packages) — narrow NoSuchMethodException catch, all other exceptions intentionally propagate - describeUnextractable() distinct diagnostic per PTM family - okapiTransactionRunner always runs validatePtmDataSourceMatch on the PTM it extracts from the TT (user OR Boot autoconfig), then wraps either the user/Boot TT (preserves timeout/propagation) or a freshly built one - SpringTransactionRunner.transactionTemplate → public (testable API) Tests: - TransactionTemplateHijackProofTest: 3 reliability invariants (PRECONDITION Boot creates TT, INTROSPECTION factory wraps OUR PTM, MISMATCH FAIL-FAST validation runs) — guards regression of the hijack bug - JpaTransactionManagerFailFastTest / MatchedDataSourceTest: real JPA setup proves extractDataSource JPA branch fails fast on mismatch and accepts match - WrongPtmDataSourceAmplificationProofTest: inverted assertion documents residual amplification risk for non-extractable PTMs (JTA, Exposed bridge) - PostgresTestSupport: extracted runOkapiLiquibaseOn() + pgDataSourceOf() helpers (DRY across 3 multi-container tests) Build: - testImpl: spring-orm + hibernate-core (JPA tests), spring-boot-transaction (TransactionAutoConfiguration on Spring Boot 4.0 test classpath) --- gradle/libs.versions.toml | 7 + okapi-integration-tests/build.gradle.kts | 5 + .../okapi/test/support/PostgresTestSupport.kt | 26 +++- .../JpaTransactionManagerFailFastTest.kt | 85 +++++++++++ ...TransactionManagerMatchedDataSourceTest.kt | 88 +++++++++++ .../MultiDataSourceTransactionTest.kt | 30 +--- ...rongPtmDataSourceAmplificationProofTest.kt | 131 ++++++++++++++++ okapi-spring-boot/build.gradle.kts | 4 + .../springboot/OutboxAutoConfiguration.kt | 126 ++++++++++----- .../springboot/SpringTransactionRunner.kt | 2 +- .../TransactionTemplateHijackProofTest.kt | 143 ++++++++++++++++++ 11 files changed, 581 insertions(+), 66 deletions(-) create mode 100644 okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerFailFastTest.kt create mode 100644 okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerMatchedDataSourceTest.kt create mode 100644 okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/WrongPtmDataSourceAmplificationProofTest.kt create mode 100644 okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/TransactionTemplateHijackProofTest.kt diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 20e993f..f69d17b 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -20,6 +20,9 @@ h2 = "2.4.240" micrometer = "1.16.5" jmh = "1.37" jmhGradlePlugin = "0.7.3" +# Hibernate version aligned with what Spring 7.x targets (Hibernate 7.0 ORM); only used in the +# integration-tests module to exercise JpaTransactionManager fail-fast extraction. +hibernate = "7.1.4.Final" [libraries] kotlinGradlePlugin = { module = "org.jetbrains.kotlin:kotlin-gradle-plugin", version.ref = "kotlin" } @@ -44,10 +47,14 @@ kafkaClients = { module = "org.apache.kafka:kafka-clients", version.ref = "kafka springContext = { module = "org.springframework:spring-context", version.ref = "spring" } springTx = { module = "org.springframework:spring-tx", version.ref = "spring" } springJdbc = { module = "org.springframework:spring-jdbc", version.ref = "spring" } +springOrm = { module = "org.springframework:spring-orm", version.ref = "spring" } +hibernateCore = { module = "org.hibernate.orm:hibernate-core", version.ref = "hibernate" } springBootAutoconfigure = { module = "org.springframework.boot:spring-boot-autoconfigure", version.ref = "springBoot" } springBootStarterValidation = { module = "org.springframework.boot:spring-boot-starter-validation", version.ref = "springBoot" } springBootTest = { module = "org.springframework.boot:spring-boot-test", version.ref = "springBoot" } springBootStarterActuator = { module = "org.springframework.boot:spring-boot-starter-actuator", version.ref = "springBoot" } +# Spring Boot 4.0 split TransactionAutoConfiguration into a dedicated module (was in spring-boot-autoconfigure in 3.x). +springBootTransaction = { module = "org.springframework.boot:spring-boot-transaction", version.ref = "springBoot" } assertjCore = { module = "org.assertj:assertj-core", version.ref = "assertj" } micrometerCore = { module = "io.micrometer:micrometer-core", version.ref = "micrometer" } micrometerTest = { module = "io.micrometer:micrometer-test", version.ref = "micrometer" } diff --git a/okapi-integration-tests/build.gradle.kts b/okapi-integration-tests/build.gradle.kts index f2d8785..018fd05 100644 --- a/okapi-integration-tests/build.gradle.kts +++ b/okapi-integration-tests/build.gradle.kts @@ -53,4 +53,9 @@ dependencies { testImplementation(libs.exposedCore) testImplementation(libs.exposedJdbc) testImplementation(libs.exposedSpringTransaction) + + // JPA + Hibernate — proves extractDataSource() pulls JpaTransactionManager.getDataSource() + // and validatePtmDataSourceMatch fails fast on PTM↔DataSource mismatch under JPA. + testImplementation(libs.springOrm) + testImplementation(libs.hibernateCore) } diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/support/PostgresTestSupport.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/support/PostgresTestSupport.kt index 5a5b371..7bb8d18 100644 --- a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/support/PostgresTestSupport.kt +++ b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/support/PostgresTestSupport.kt @@ -37,10 +37,26 @@ class PostgresTestSupport { } } - private fun runLiquibase() { - val connection = DriverManager.getConnection(container.jdbcUrl, container.username, container.password) - val db = DatabaseFactory.getInstance().findCorrectDatabaseImplementation(JdbcConnection(connection)) - Liquibase("com/softwaremill/okapi/db/postgres/changelog.xml", ClassLoaderResourceAccessor(), db).use { it.update("") } - connection.close() + private fun runLiquibase() = runOkapiLiquibaseOn(container) +} + +/** + * Applies okapi's PostgreSQL Liquibase changelog to the given container. For tests that manage + * their own PostgreSQL containers (e.g. 2-DataSource setups) and can't use the single-container + * `PostgresTestSupport` class. + */ +fun runOkapiLiquibaseOn(container: PostgreSQLContainer) { + DriverManager.getConnection(container.jdbcUrl, container.username, container.password).use { conn -> + val db = DatabaseFactory.getInstance().findCorrectDatabaseImplementation(JdbcConnection(conn)) + Liquibase("com/softwaremill/okapi/db/postgres/changelog.xml", ClassLoaderResourceAccessor(), db).use { + it.update("") + } } } + +/** Builds a plain `PGSimpleDataSource` pointing at the given container. */ +fun pgDataSourceOf(container: PostgreSQLContainer): DataSource = PGSimpleDataSource().apply { + setURL(container.jdbcUrl) + user = container.username + password = container.password +} diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerFailFastTest.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerFailFastTest.kt new file mode 100644 index 0000000..44ffc4f --- /dev/null +++ b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerFailFastTest.kt @@ -0,0 +1,85 @@ +package com.softwaremill.okapi.test.transaction + +import com.softwaremill.okapi.core.DeliveryResult +import com.softwaremill.okapi.core.MessageDeliverer +import com.softwaremill.okapi.core.OutboxEntry +import com.softwaremill.okapi.postgres.PostgresOutboxStore +import com.softwaremill.okapi.springboot.OkapiLiquibaseAutoConfiguration +import com.softwaremill.okapi.springboot.OutboxAutoConfiguration +import com.softwaremill.okapi.springboot.SpringConnectionProvider +import com.softwaremill.okapi.test.support.pgDataSourceOf +import com.softwaremill.okapi.test.support.runOkapiLiquibaseOn +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.nulls.shouldNotBeNull +import io.kotest.matchers.string.shouldContain +import org.springframework.beans.factory.config.BeanDefinitionCustomizer +import org.springframework.boot.autoconfigure.AutoConfigurations +import org.springframework.boot.test.context.runner.ApplicationContextRunner +import org.springframework.orm.jpa.JpaTransactionManager +import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean +import org.springframework.orm.jpa.vendor.HibernateJpaVendorAdapter +import org.springframework.transaction.PlatformTransactionManager +import org.testcontainers.containers.PostgreSQLContainer +import javax.sql.DataSource + +/** + * Proves the JPA branch of `extractDataSource` (`OutboxAutoConfiguration.kt`): a + * `JpaTransactionManager` whose auto-detected DataSource differs from okapi's outbox DataSource + * triggers `validatePtmDataSourceMatch` fail-fast at startup. Companion to + * `WrongPtmDataSourceAmplificationProofTest`, which documents the remaining residual risk for + * PTMs that expose no DataSource at all (Exposed bridge, JTA). + */ +class JpaTransactionManagerFailFastTest : FunSpec({ + + val container = PostgreSQLContainer("postgres:16") + lateinit var dsA: DataSource + lateinit var dsB: DataSource + + beforeSpec { + container.start() + dsA = pgDataSourceOf(container) + dsB = pgDataSourceOf(container) + runOkapiLiquibaseOn(container) + } + + afterSpec { container.stop() } + + test("JpaTransactionManager bound to a different DataSource than the outbox DS fails fast at startup") { + val emf = LocalContainerEntityManagerFactoryBean().apply { + dataSource = dsA + jpaVendorAdapter = HibernateJpaVendorAdapter() + // Empty package scan creates an implicit persistence unit without requiring persistence.xml + // — we have no @Entity classes; the EMF only needs to exist so JpaTransactionManager can + // auto-detect its DataSource. + setPackagesToScan() + afterPropertiesSet() + }.`object`.shouldNotBeNull() + + ApplicationContextRunner() + .withConfiguration( + AutoConfigurations.of( + OutboxAutoConfiguration::class.java, + OkapiLiquibaseAutoConfiguration::class.java, + ), + ) + .withBean("dsB", DataSource::class.java, { dsB }, BeanDefinitionCustomizer { it.isPrimary = true }) + .withBean("dsA", DataSource::class.java, { dsA }) + .withBean("jpaTmA", PlatformTransactionManager::class.java, { JpaTransactionManager(emf) }) + .withBean(MessageDeliverer::class.java, { JpaTestStubDeliverer }) + .withBean(PostgresOutboxStore::class.java, { + PostgresOutboxStore(SpringConnectionProvider(dsB), java.time.Clock.systemUTC()) + }) + .withPropertyValues("okapi.liquibase.enabled=false") + .run { ctx -> + val failure = ctx.startupFailure + failure.shouldNotBeNull() + failure.stackTraceToString() shouldContain + "is bound to a different DataSource than okapi's outbox DataSource" + } + } +}) + +private object JpaTestStubDeliverer : MessageDeliverer { + override val type = "stub" + override fun deliver(entry: OutboxEntry) = DeliveryResult.Success +} diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerMatchedDataSourceTest.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerMatchedDataSourceTest.kt new file mode 100644 index 0000000..f9d7668 --- /dev/null +++ b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerMatchedDataSourceTest.kt @@ -0,0 +1,88 @@ +package com.softwaremill.okapi.test.transaction + +import com.softwaremill.okapi.core.DeliveryResult +import com.softwaremill.okapi.core.MessageDeliverer +import com.softwaremill.okapi.core.OutboxEntry +import com.softwaremill.okapi.core.TransactionRunner +import com.softwaremill.okapi.postgres.PostgresOutboxStore +import com.softwaremill.okapi.springboot.OkapiLiquibaseAutoConfiguration +import com.softwaremill.okapi.springboot.OutboxAutoConfiguration +import com.softwaremill.okapi.springboot.SpringConnectionProvider +import com.softwaremill.okapi.springboot.SpringTransactionRunner +import com.softwaremill.okapi.test.support.pgDataSourceOf +import com.softwaremill.okapi.test.support.runOkapiLiquibaseOn +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.nulls.shouldBeNull +import io.kotest.matchers.nulls.shouldNotBeNull +import io.kotest.matchers.types.shouldBeInstanceOf +import io.kotest.matchers.types.shouldBeSameInstanceAs +import org.springframework.boot.autoconfigure.AutoConfigurations +import org.springframework.boot.test.context.runner.ApplicationContextRunner +import org.springframework.orm.jpa.JpaTransactionManager +import org.springframework.orm.jpa.LocalContainerEntityManagerFactoryBean +import org.springframework.orm.jpa.vendor.HibernateJpaVendorAdapter +import org.springframework.transaction.PlatformTransactionManager +import org.testcontainers.containers.PostgreSQLContainer +import javax.sql.DataSource + +/** + * Companion to `JpaTransactionManagerFailFastTest`: proves the MATCH branch of the JPA path. + * When the `JpaTransactionManager`'s auto-detected DataSource equals okapi's outbox DataSource, + * `validatePtmDataSourceMatch` succeeds, the context starts, and `okapiTransactionRunner` is wired + * to a `TransactionTemplate` bound to that PTM. + * + * Without this test the fail-fast version above could theoretically pass even if a regression + * routed JPA through a wrong branch and made every JPA setup fail — match coverage is the + * other half of the proof. + */ +class JpaTransactionManagerMatchedDataSourceTest : FunSpec({ + + val container = PostgreSQLContainer("postgres:16") + lateinit var ds: DataSource + + beforeSpec { + container.start() + ds = pgDataSourceOf(container) + runOkapiLiquibaseOn(container) + } + + afterSpec { container.stop() } + + test("JpaTransactionManager bound to the SAME DataSource as the outbox: context starts cleanly") { + val emf = LocalContainerEntityManagerFactoryBean().apply { + dataSource = ds + jpaVendorAdapter = HibernateJpaVendorAdapter() + setPackagesToScan() + afterPropertiesSet() + }.`object`.shouldNotBeNull() + + val jpaPtm = JpaTransactionManager(emf) + + ApplicationContextRunner() + .withConfiguration( + AutoConfigurations.of( + OutboxAutoConfiguration::class.java, + OkapiLiquibaseAutoConfiguration::class.java, + ), + ) + .withBean(DataSource::class.java, { ds }) + .withBean("jpaTm", PlatformTransactionManager::class.java, { jpaPtm }) + .withBean(MessageDeliverer::class.java, { JpaMatchStubDeliverer }) + .withBean(PostgresOutboxStore::class.java, { + PostgresOutboxStore(SpringConnectionProvider(ds), java.time.Clock.systemUTC()) + }) + .withPropertyValues("okapi.liquibase.enabled=false") + .run { ctx -> + ctx.startupFailure.shouldBeNull() + val runner = ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() + // The runner's TransactionTemplate must point at OUR JPA PTM, proving extractDataSource + // returned the matched DS and validatePtmDataSourceMatch's return-after-match path was taken. + runner.transactionTemplate.transactionManager shouldBeSameInstanceAs jpaPtm + } + } +}) + +private object JpaMatchStubDeliverer : MessageDeliverer { + override val type = "stub" + override fun deliver(entry: OutboxEntry) = DeliveryResult.Success +} diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/MultiDataSourceTransactionTest.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/MultiDataSourceTransactionTest.kt index bf6396e..c96e3a6 100644 --- a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/MultiDataSourceTransactionTest.kt +++ b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/MultiDataSourceTransactionTest.kt @@ -7,19 +7,15 @@ import com.softwaremill.okapi.core.OutboxStatus import com.softwaremill.okapi.postgres.PostgresOutboxStore import com.softwaremill.okapi.springboot.SpringConnectionProvider import com.softwaremill.okapi.springboot.SpringOutboxPublisher +import com.softwaremill.okapi.test.support.pgDataSourceOf +import com.softwaremill.okapi.test.support.runOkapiLiquibaseOn import io.kotest.assertions.throwables.shouldThrow import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.maps.shouldContain import io.kotest.matchers.shouldNotBe -import liquibase.Liquibase -import liquibase.database.DatabaseFactory -import liquibase.database.jvm.JdbcConnection -import liquibase.resource.ClassLoaderResourceAccessor -import org.postgresql.ds.PGSimpleDataSource import org.springframework.jdbc.datasource.DataSourceTransactionManager import org.springframework.transaction.support.TransactionTemplate import org.testcontainers.containers.PostgreSQLContainer -import java.sql.DriverManager import java.time.Clock import javax.sql.DataSource @@ -58,20 +54,11 @@ class MultiDataSourceTransactionTest : FunSpec({ outboxContainer.start() otherContainer.start() - outboxDataSource = PGSimpleDataSource().apply { - setURL(outboxContainer.jdbcUrl) - user = outboxContainer.username - password = outboxContainer.password - } - - otherDataSource = PGSimpleDataSource().apply { - setURL(otherContainer.jdbcUrl) - user = otherContainer.username - password = otherContainer.password - } + outboxDataSource = pgDataSourceOf(outboxContainer) + otherDataSource = pgDataSourceOf(otherContainer) // Run Liquibase migration only on the outbox database - runLiquibase(outboxContainer) + runOkapiLiquibaseOn(outboxContainer) val outboxTxManager = DataSourceTransactionManager(outboxDataSource) outboxTxTemplate = TransactionTemplate(outboxTxManager) @@ -135,10 +122,3 @@ class MultiDataSourceTransactionTest : FunSpec({ counts shouldContain (OutboxStatus.PENDING to 1L) } }) - -private fun runLiquibase(container: PostgreSQLContainer) { - val connection = DriverManager.getConnection(container.jdbcUrl, container.username, container.password) - val db = DatabaseFactory.getInstance().findCorrectDatabaseImplementation(JdbcConnection(connection)) - Liquibase("com/softwaremill/okapi/db/postgres/changelog.xml", ClassLoaderResourceAccessor(), db).use { it.update("") } - connection.close() -} diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/WrongPtmDataSourceAmplificationProofTest.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/WrongPtmDataSourceAmplificationProofTest.kt new file mode 100644 index 0000000..9d330bf --- /dev/null +++ b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/WrongPtmDataSourceAmplificationProofTest.kt @@ -0,0 +1,131 @@ +package com.softwaremill.okapi.test.transaction + +import com.softwaremill.okapi.core.DeliveryInfo +import com.softwaremill.okapi.core.MessageDeliverer +import com.softwaremill.okapi.core.OutboxMessage +import com.softwaremill.okapi.core.OutboxProcessor +import com.softwaremill.okapi.core.TransactionRunner +import com.softwaremill.okapi.postgres.PostgresOutboxStore +import com.softwaremill.okapi.springboot.OkapiLiquibaseAutoConfiguration +import com.softwaremill.okapi.springboot.OutboxAutoConfiguration +import com.softwaremill.okapi.springboot.SpringConnectionProvider +import com.softwaremill.okapi.springboot.SpringOutboxPublisher +import com.softwaremill.okapi.test.support.RecordingMessageDeliverer +import com.softwaremill.okapi.test.support.pgDataSourceOf +import com.softwaremill.okapi.test.support.runOkapiLiquibaseOn +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.maps.shouldNotBeEmpty +import io.kotest.matchers.shouldBe +import org.jetbrains.exposed.v1.spring7.transaction.SpringTransactionManager +import org.springframework.beans.factory.config.BeanDefinitionCustomizer +import org.springframework.boot.autoconfigure.AutoConfigurations +import org.springframework.boot.test.context.runner.ApplicationContextRunner +import org.springframework.jdbc.datasource.DataSourceTransactionManager +import org.springframework.transaction.PlatformTransactionManager +import org.springframework.transaction.support.TransactionTemplate +import org.testcontainers.containers.PostgreSQLContainer +import java.util.concurrent.CompletableFuture +import java.util.concurrent.CyclicBarrier +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import javax.sql.DataSource + +/** + * Documents the residual silent-failure risk for non-extractable PTMs (JTA, Exposed bridge, + * any `PlatformTransactionManager` that exposes neither a `DataSource` resourceFactory nor a + * public `getDataSource()`) when the user wires the outbox to a different DataSource than the + * PTM and does NOT set `okapi.transaction-manager-qualifier`. + * + * `validatePtmDataSourceMatch` can fail-fast for `DataSourceTransactionManager`, + * `JpaTransactionManager`, and `HibernateTransactionManager` (see `extractDataSource`). For the + * remaining PTM families it can only emit a WARN — Spring exposes no public API to derive the + * bound DataSource. This test pins down what "WARN only" empirically means: concurrent processors + * see fully unlocked rows and amplify delivery. + * + * Asserts amplification DID happen (50/50 entries delivered more than once is the typical run). + * If a future change adds extraction support for the Exposed bridge — or pessimistic locking + * starts holding across the spurious auto-commit — this test will fail and force a re-evaluation. + */ +class WrongPtmDataSourceAmplificationProofTest : FunSpec({ + + val dsAContainer = PostgreSQLContainer("postgres:16") + val dsBContainer = PostgreSQLContainer("postgres:16") + + lateinit var dsA: DataSource + lateinit var dsB: DataSource + + beforeSpec { + dsAContainer.start() + dsBContainer.start() + dsA = pgDataSourceOf(dsAContainer) + dsB = pgDataSourceOf(dsBContainer) + // Migrate both: DS-B holds the outbox table the processor reads; DS-A would have it if + // okapi had picked it. Keeping parity rules out "table missing" as a cause of zero deliveries. + runOkapiLiquibaseOn(dsAContainer) + runOkapiLiquibaseOn(dsBContainer) + } + + afterSpec { + dsAContainer.stop() + dsBContainer.stop() + } + + test("non-extractable PTM bound to wrong DataSource permits delivery amplification") { + val recorder = RecordingMessageDeliverer() + + // Publish 50 entries to DS-B via a CORRECTLY-wired DST(DS-B) publisher. + val publishTpl = TransactionTemplate(DataSourceTransactionManager(dsB)) + val publishStore = PostgresOutboxStore(SpringConnectionProvider(dsB), java.time.Clock.systemUTC()) + val publisher = SpringOutboxPublisher( + delegate = com.softwaremill.okapi.core.OutboxPublisher(publishStore, java.time.Clock.systemUTC()), + dataSource = dsB, + ) + repeat(50) { i -> + publishTpl.execute { + publisher.publish(OutboxMessage("test.event", """{"i":$i}"""), AmplificationDeliveryInfo) + } + } + + ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(OutboxAutoConfiguration::class.java, OkapiLiquibaseAutoConfiguration::class.java)) + // DS-B as @Primary so resolveDataSource() picks it as the outbox DS. + .withBean("dsB", DataSource::class.java, { dsB }, BeanDefinitionCustomizer { it.isPrimary = true }) + .withBean("dsA", DataSource::class.java, { dsA }) + // Exposed SpringTransactionManager bound to DS-A — non-extractable: validatePtmDataSourceMatch + // logs a WARN and proceeds (the silent-failure setup this test documents). + .withBean("exposedTmA", PlatformTransactionManager::class.java, { SpringTransactionManager(dsA) }) + .withBean(MessageDeliverer::class.java, { recorder }) + .withBean(PostgresOutboxStore::class.java, { + PostgresOutboxStore(SpringConnectionProvider(dsB), java.time.Clock.systemUTC()) + }) + .withPropertyValues("okapi.processor.enabled=false", "okapi.liquibase.enabled=false") + .run { ctx -> + val processor = ctx.getBean(OutboxProcessor::class.java) + val transactionRunner = ctx.getBean(TransactionRunner::class.java) + + val barrier = CyclicBarrier(5) + val executor = Executors.newVirtualThreadPerTaskExecutor() + val futures = (1..5).map { + CompletableFuture.supplyAsync( + { + barrier.await(10, TimeUnit.SECONDS) + transactionRunner.runInTransaction { processor.processNext(50) } + }, + executor, + ) + } + CompletableFuture.allOf(*futures.toTypedArray()).get(60, TimeUnit.SECONDS) + executor.shutdown() + + recorder.deliveryCount() shouldBe 50 + // The residual risk: with FOR UPDATE SKIP LOCKED collapsed to auto-commit, concurrent + // processors see overlapping result sets and at least one entry is delivered twice. + recorder.deliveries.filter { it.value.size > 1 }.shouldNotBeEmpty() + } + } +}) + +private object AmplificationDeliveryInfo : DeliveryInfo { + override val type: String = "recording" + override fun serialize(): String = """{"type":"recording"}""" +} diff --git a/okapi-spring-boot/build.gradle.kts b/okapi-spring-boot/build.gradle.kts index 25f6003..924ac57 100644 --- a/okapi-spring-boot/build.gradle.kts +++ b/okapi-spring-boot/build.gradle.kts @@ -74,6 +74,10 @@ dependencies { testImplementation(libs.micrometerCore) // Brings in the metrics auto-config jar so @AutoConfigureAfter targets are resolvable in tests. testImplementation(libs.springBootStarterActuator) + // TransactionAutoConfiguration lives here in Spring Boot 4.0+ (was in spring-boot-autoconfigure + // in 3.x). TransactionTemplateHijackProofTest needs it on the classpath to verify the + // factory's behaviour against Boot's auto-created TransactionTemplate bean. + testImplementation(libs.springBootTransaction) // Logback's ListAppender is used to capture and assert WARN-level log output (e.g. the // LiquibaseDisabledNotice breadcrumb + our PTM↔DS validation cannot-verify WARN) — slf4j-simple // does not provide an introspectable appender. diff --git a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt index e911c90..d801cc4 100644 --- a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt +++ b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt @@ -91,6 +91,14 @@ class OutboxAutoConfiguration( // Skipping the bean in publish-only deployments (both schedulers disabled) lets users run without // a PlatformTransactionManager on the classpath at all (e.g. message producer that delegates // outbox processing to a separate worker). + // + // ObjectProvider design: a TT in the context can come from two sources — + // a user-defined @Bean (with custom timeout/propagation/isolation), OR Spring Boot's + // TransactionAutoConfiguration which auto-registers a TT whenever a single PTM exists. The two + // are indistinguishable at this layer. We accept BOTH transparently but always extract the bound + // PTM and run validatePtmDataSourceMatch on it — so the user's TX semantics are honoured AND the + // multi-DS safety net stays armed. See TransactionTemplateHijackProofTest for the empirical proof + // that without `validatePtmDataSourceMatch` here, Boot's auto-TT silently bypasses safety. @Bean @ConditionalOnMissingBean @ConditionalOnExpression("\${okapi.processor.enabled:true} or \${okapi.purger.enabled:true}") @@ -99,22 +107,24 @@ class OutboxAutoConfiguration( transactionTemplate: ObjectProvider, beanFactory: BeanFactory, ): TransactionRunner { - // If the user has defined exactly one @Bean TransactionTemplate, honour their settings - // (timeout, propagation, isolation, etc.) instead of silently building a fresh one from - // the PTM. Same intent as @ConditionalOnMissingBean on this factory, just at a finer - // granularity — the user's template is THE source of truth for TX semantics. - val userTemplate = transactionTemplate.getIfUnique() - if (userTemplate != null) { - return SpringTransactionRunner(userTemplate) - } - val ptm = resolvePlatformTransactionManager(transactionManager, beanFactory, okapiProperties) + val anyTemplate = transactionTemplate.getIfUnique() + // Extract the PTM from the TT if available, else resolve through the provider. TT's + // transactionManager is nullable in the API (a TT can be constructed without one) — fall + // back to the provider path in that pathological case so we still get a validated PTM. + val ptm = anyTemplate?.transactionManager + ?: resolvePlatformTransactionManager(transactionManager, beanFactory, okapiProperties) validatePtmDataSourceMatch(ptm, resolveDataSource(), okapiProperties) - // Sets the *initial* TX read-only flag to false. NOTE: with PROPAGATION_REQUIRED (the default), - // a tick that joins an outer @Transactional(readOnly = true) inherits the outer's flag and - // this setting is silently ignored. The scheduler runs on a daemon thread with no outer TX, - // so this flag actually takes effect — but invocations from inside an existing read-only TX - // would still hit FOR UPDATE failures. Keep scheduler invocations outside @Transactional scopes. - return SpringTransactionRunner(TransactionTemplate(ptm).apply { isReadOnly = false }) + // Use the user-/Boot-supplied TT verbatim when present (preserves timeout, propagation, + // isolation set by whoever defined it). Otherwise build a minimal TT around the validated + // PTM. Sets the *initial* TX read-only flag to false. NOTE: with PROPAGATION_REQUIRED + // (the default), a tick that joins an outer @Transactional(readOnly = true) inherits the + // outer's flag and this setting is silently ignored. The scheduler runs on a daemon + // thread with no outer TX, so this flag actually takes effect — but invocations from + // inside an existing read-only TX would still hit FOR UPDATE failures. Keep scheduler + // invocations outside @Transactional scopes. + return SpringTransactionRunner( + anyTemplate ?: TransactionTemplate(ptm).apply { isReadOnly = false }, + ) } @Bean @@ -292,15 +302,15 @@ class OutboxAutoConfiguration( outboxDataSource: DataSource, properties: OkapiProperties, ) { - // Only ResourceTransactionManager exposes the underlying resource factory. For RTMs whose - // resourceFactory is a JDBC DataSource (DataSourceTransactionManager and similar), we can - // verify it matches okapi's outbox DataSource. RTMs whose resource is something else - // (JpaTransactionManager → EntityManagerFactory, HibernateTransactionManager → SessionFactory) - // and non-RTM PTMs (Exposed's SpringTransactionManager, JtaTransactionManager) fall through - // to the "cannot verify" WARN path. Capture the actual resourceFactory class so the WARN - // reports what the user's PTM actually exposed (not just the static JPA/Hibernate examples). - val rtmResourceFactory = (ptm as? ResourceTransactionManager)?.resourceFactory - val ptmDataSource = rtmResourceFactory as? DataSource + // Tries three extraction strategies in order: + // 1. ResourceTransactionManager whose resourceFactory is a JDBC DataSource (DST + custom) + // 2. JpaTransactionManager.getDataSource() — public getter, autodetected from EntityManagerFactory + // 3. HibernateTransactionManager.getDataSource() — same shape (both pre-/post-6.2 packages) + // Non-extractable cases (JTA, Exposed SpringTransactionManager, JPA/Hibernate with no JDBC DS) + // fall through to the WARN/INFO path. The WrongPtmDataSourceAmplificationProofTest empirically + // shows that wrong-DS bracketing collapses FOR UPDATE SKIP LOCKED to JDBC auto-commit and + // permits 100% delivery amplification — fail-fast when we can verify, log loudly when we cannot. + val ptmDataSource = extractDataSource(ptm) if (ptmDataSource != null) { // Spring's recommended pattern wraps the outbox DataSource bean in TransactionAwareDataSourceProxy // (or LazyConnectionDataSourceProxy) for use by query helpers, while passing the raw DataSource @@ -341,17 +351,7 @@ class OutboxAutoConfiguration( } return } - val resourceFactoryDescription = when { - ptm !is ResourceTransactionManager -> - "does not implement ResourceTransactionManager (no resource factory exposed; same shape as " + - "JtaTransactionManager or Exposed's SpringTransactionManager)" - rtmResourceFactory == null -> - "implements ResourceTransactionManager but its getResourceFactory() returned null" - else -> - "implements ResourceTransactionManager but its resourceFactory is of type " + - "'${rtmResourceFactory.javaClass.name}', not a JDBC DataSource (typical for JPA's " + - "EntityManagerFactory or Hibernate's SessionFactory)" - } + val resourceFactoryDescription = describeUnextractable(ptm) if (properties.datasourceQualifier != null) { logger.warn( "okapi.datasource-qualifier='{}' is set, but the resolved PlatformTransactionManager '{}' {} " + @@ -394,5 +394,61 @@ class OutboxAutoConfiguration( } return current } + + // JPA/Hibernate PTMs that expose a `public DataSource getDataSource()` — reflection by name + // avoids requiring spring-orm on the compile classpath (it's optional for JDBC-only consumers). + // Hibernate's TM moved package in Spring 6.2 — both names are listed so a single okapi build + // works against Spring Framework 6.1.x and 6.2+ without a version matrix. + // Hibernate's TM is named `org.springframework.orm.jpa.hibernate.HibernateTransactionManager` + // in Spring 6.2+ and `org.springframework.orm.hibernate5.HibernateTransactionManager` in + // Spring 6.1-. Both are listed so a single build works across versions. + // `internal` so reflection-resolution tests can verify the set isn't all stale. + internal val JPA_HIBERNATE_PTM_CLASSES = setOf( + "org.springframework.orm.jpa.JpaTransactionManager", + "org.springframework.orm.jpa.hibernate.HibernateTransactionManager", + "org.springframework.orm.hibernate5.HibernateTransactionManager", + ) + + internal fun extractDataSource(ptm: PlatformTransactionManager): DataSource? { + // Strategy 1: ResourceTransactionManager (DST + any RTM whose resourceFactory is a DataSource). + (ptm as? ResourceTransactionManager)?.resourceFactory?.let { rf -> + if (rf is DataSource) return rf + } + // Strategy 2/3: JPA/Hibernate public `getDataSource()`. Narrow catch on + // NoSuchMethodException only — all other exceptions intentionally propagate: + // LinkageError / NoClassDefFoundError → mixed-jar classpath bug, must surface (this + // is why we don't use `runCatching`, which would swallow them). + // InvocationTargetException → JpaTransactionManager constructed without an EMF; + // callable, but unusable for outbox bracketing — surface the original cause. + // IllegalAccessException / ClassCastException → incompatible Spring framework + // version where the contract has changed; not safe to silently fall back. + if (ptm.javaClass.name in JPA_HIBERNATE_PTM_CLASSES) { + return try { + ptm.javaClass.getMethod("getDataSource").invoke(ptm) as DataSource? + } catch (_: NoSuchMethodException) { + null + } + } + return null + } + + private fun describeUnextractable(ptm: PlatformTransactionManager): String { + if (ptm.javaClass.name in JPA_HIBERNATE_PTM_CLASSES) { + return "is ${ptm.javaClass.name} but its getDataSource() returned null — the " + + "EntityManagerFactory/SessionFactory was constructed without a JDBC DataSource " + + "(typical for pure-JTA / JNDI-only setups). okapi cannot verify the binding" + } + val rtmResourceFactory = (ptm as? ResourceTransactionManager)?.resourceFactory + return when { + ptm !is ResourceTransactionManager -> + "does not implement ResourceTransactionManager (no resource factory exposed; same shape as " + + "JtaTransactionManager or Exposed's SpringTransactionManager)" + rtmResourceFactory == null -> + "implements ResourceTransactionManager but its getResourceFactory() returned null" + else -> + "implements ResourceTransactionManager but its resourceFactory is of type " + + "'${rtmResourceFactory.javaClass.name}', not a JDBC DataSource" + } + } } } diff --git a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionRunner.kt b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionRunner.kt index 1a7238c..3890bae 100644 --- a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionRunner.kt +++ b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionRunner.kt @@ -7,7 +7,7 @@ import org.springframework.transaction.support.TransactionTemplate * Spring implementation of [TransactionRunner] using [TransactionTemplate]. */ class SpringTransactionRunner( - internal val transactionTemplate: TransactionTemplate, + val transactionTemplate: TransactionTemplate, ) : TransactionRunner { override fun runInTransaction(block: () -> T): T = transactionTemplate.execute { block() }!! } diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/TransactionTemplateHijackProofTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/TransactionTemplateHijackProofTest.kt new file mode 100644 index 0000000..5302adb --- /dev/null +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/TransactionTemplateHijackProofTest.kt @@ -0,0 +1,143 @@ +package com.softwaremill.okapi.springboot + +import com.softwaremill.okapi.core.DeliveryResult +import com.softwaremill.okapi.core.MessageDeliverer +import com.softwaremill.okapi.core.OutboxEntry +import com.softwaremill.okapi.core.OutboxStatus +import com.softwaremill.okapi.core.OutboxStore +import com.softwaremill.okapi.core.TransactionRunner +import io.kotest.assertions.withClue +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.nulls.shouldNotBeNull +import io.kotest.matchers.shouldBe +import io.kotest.matchers.shouldNotBe +import io.kotest.matchers.string.shouldContain +import io.kotest.matchers.types.shouldBeInstanceOf +import org.springframework.boot.autoconfigure.AutoConfigurations +import org.springframework.boot.test.context.runner.ApplicationContextRunner +import org.springframework.jdbc.datasource.DataSourceTransactionManager +import org.springframework.jdbc.datasource.SimpleDriverDataSource +import org.springframework.transaction.PlatformTransactionManager +import org.springframework.transaction.support.TransactionTemplate +import java.time.Instant +import javax.sql.DataSource + +/** + * Reliable proof tests against the ultrareview claim: + * "Spring Boot's auto-configured TransactionTemplate hijacks okapiTransactionRunner — the factory + * short-circuits to the Boot-supplied template and skips PTM↔DataSource validation entirely." + * + * The previous single-test version asserted only that startup failed in the mismatch scenario, then + * inferred "validation ran". That's brittle — context could fail for unrelated reasons, or autoconfig + * ordering in slice tests could differ from production. These three tests pin down three independent + * invariants so the conclusion is empirically forced: + * + * 1. PRECONDITION: Spring Boot's TransactionAutoConfiguration actually creates a `TransactionTemplate` + * bean in slice tests. If false, the hijack scenario cannot occur in this test harness and the + * other two tests prove nothing — the suite fails loudly instead of silently passing. + * + * 2. INTROSPECTION: in a single-DS happy path, `okapiTransactionRunner` produces a + * `SpringTransactionRunner` whose `TransactionTemplate.transactionManager` is the user's PTM. + * Combined with #1 this proves: even when Boot's TT bean exists, the factory does NOT pick it. + * (If the factory short-circuited via Boot's TT, the embedded PTM identity would not match.) + * + * 3. MISMATCH FAIL-FAST: in a 2-DS scenario with PTM bound to the wrong DS, startup fails with + * a literal substring from `validatePtmDataSourceMatch`'s `error(...)` message that no other + * Spring component emits — passing this assertion proves our validation code path was reached, + * not just that something failed. + */ +class TransactionTemplateHijackProofTest : FunSpec({ + + // Spring Boot 3.x: org.springframework.boot.autoconfigure.transaction.TransactionAutoConfiguration + // Spring Boot 4.x: org.springframework.boot.transaction.autoconfigure.TransactionAutoConfiguration + val txAutoConfigClass: Class<*> = listOf( + "org.springframework.boot.transaction.autoconfigure.TransactionAutoConfiguration", + "org.springframework.boot.autoconfigure.transaction.TransactionAutoConfiguration", + ).firstNotNullOfOrNull { fqcn -> + try { + Class.forName(fqcn) + } catch (_: ClassNotFoundException) { + null + } + } ?: error( + "TransactionAutoConfiguration not on test classpath. Without it the entire hijack scenario " + + "cannot be reproduced — failing the suite loudly rather than silently passing. Check that " + + "spring-boot-transaction (4.x) or spring-boot-autoconfigure (3.x) is on testRuntimeClasspath.", + ) + + test("PRECONDITION: Spring Boot's TransactionAutoConfiguration registers a TransactionTemplate bean") { + val ds: DataSource = SimpleDriverDataSource() + ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(txAutoConfigClass)) + .withBean(DataSource::class.java, { ds }) + .withBean(PlatformTransactionManager::class.java, { DataSourceTransactionManager(ds) }) + .run { ctx -> + withClue( + "If this fails, Spring Boot's TransactionTemplateConfiguration was not triggered in slice " + + "tests — the hijack tests below would silently pass without testing anything. Investigate " + + "before trusting test #2 / #3 results.", + ) { + ctx.getBean(TransactionTemplate::class.java).shouldNotBeNull() + } + } + } + + test("INTROSPECTION: with Boot's TT in context, okapiTransactionRunner builds a TT around OUR PTM") { + val ds: DataSource = SimpleDriverDataSource() + val ourPtm: PlatformTransactionManager = DataSourceTransactionManager(ds) + ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(OutboxAutoConfiguration::class.java, txAutoConfigClass)) + .withBean(DataSource::class.java, { ds }) + .withBean("ourPtm", PlatformTransactionManager::class.java, { ourPtm }) + .withBean(OutboxStore::class.java, { stubStore() }) + .withBean(MessageDeliverer::class.java, { stubDeliverer() }) + .run { ctx -> + ctx.startupFailure shouldBe null + val runner = ctx.getBean(TransactionRunner::class.java) + runner.shouldBeInstanceOf() + // The TT that okapiTransactionRunner wraps must be bound to OUR PTM — not Boot's + // auto-configured TT (which is the hijack failure mode). Reference identity, not + // equality, since each PTM is a distinct object. + withClue("okapiTransactionRunner is wrapping a TT whose internal PTM is NOT our ourPtm — hijack confirmed") { + runner.transactionTemplate.transactionManager shouldBe ourPtm + } + } + } + + test("MISMATCH FAIL-FAST: PTM bound to wrong DataSource triggers validatePtmDataSourceMatch error") { + val dsA: DataSource = SimpleDriverDataSource() + val dsB: DataSource = SimpleDriverDataSource() + ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(OutboxAutoConfiguration::class.java, txAutoConfigClass)) + .withBean( + "dsB", + DataSource::class.java, + { dsB }, + org.springframework.beans.factory.config.BeanDefinitionCustomizer { it.isPrimary = true }, + ) + .withBean("dsA", DataSource::class.java, { dsA }) + .withBean("dstA", PlatformTransactionManager::class.java, { DataSourceTransactionManager(dsA) }) + .withBean(OutboxStore::class.java, { stubStore() }) + .withBean(MessageDeliverer::class.java, { stubDeliverer() }) + .run { ctx -> + val failure = ctx.startupFailure + failure shouldNotBe null + failure!!.stackTraceToString() shouldContain + "is bound to a different DataSource than okapi's outbox DataSource" + } + } +}) + +private fun stubStore() = object : OutboxStore { + override fun persist(entry: OutboxEntry) = entry + override fun claimPending(limit: Int) = emptyList() + override fun updateAfterProcessing(entry: OutboxEntry) = entry + override fun removeDeliveredBefore(time: Instant, limit: Int) = 0 + override fun findOldestCreatedAt(statuses: Set) = emptyMap() + override fun countByStatuses() = emptyMap() +} + +private fun stubDeliverer() = object : MessageDeliverer { + override val type = "stub" + override fun deliver(entry: OutboxEntry) = DeliveryResult.Success +} From e03976c681bee362d39dcf8bd802df40236453c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20Kobyli=C5=84ski?= Date: Sun, 17 May 2026 15:49:19 +0200 Subject: [PATCH 4/8] fix(ci): gate spring-boot-transaction testImpl on Spring Boot major >= 4 (KOJAK-67) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit spring-boot-transaction is a Spring Boot 4.0+ artifact (3.x bundles TransactionAutoConfiguration inside spring-boot-autoconfigure). The CI matrix override -PspringBootVersion=3.5.x rewrites every org.springframework.boot:* coordinate via resolutionStrategy, so the unconditional dependency tried to resolve a non-existent spring-boot-transaction:3.5.12 and failed the "Spring Boot 3.5.12" job. TransactionTemplateHijackProofTest already resolves whichever FQCN is present (3.x autoconfigure package OR 4.x dedicated module) via Class.forName fallback, so on 3.x it finds TransactionAutoConfiguration in spring-boot-autoconfigure without the extra dependency. Verified locally: -PspringBootVersion=3.5.12 -PspringVersion=6.2.7 → all 3 hijack tests PASS, no resolution error. --- okapi-spring-boot/build.gradle.kts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/okapi-spring-boot/build.gradle.kts b/okapi-spring-boot/build.gradle.kts index 924ac57..4d5ef5d 100644 --- a/okapi-spring-boot/build.gradle.kts +++ b/okapi-spring-boot/build.gradle.kts @@ -34,6 +34,14 @@ if (providers.gradleProperty("enableMutationTesting").orNull?.toBoolean() == tru } } +// spring-boot-transaction is a Spring Boot 4.0+ artifact (3.x bundles TransactionAutoConfiguration +// in spring-boot-autoconfigure). The CI matrix override -PspringBootVersion=3.5.x rewrites every +// org.springframework.boot:* coordinate, so unconditionally declaring spring-boot-transaction makes +// it try to resolve a non-existent spring-boot-transaction:3.5.x. Gate on the resolved major. +val springBootMajorForTests = ( + providers.gradleProperty("springBootVersion").orNull ?: libs.versions.springBoot.get() +).substringBefore('.').toInt() + dependencies { implementation(project(":okapi-core")) @@ -74,10 +82,13 @@ dependencies { testImplementation(libs.micrometerCore) // Brings in the metrics auto-config jar so @AutoConfigureAfter targets are resolvable in tests. testImplementation(libs.springBootStarterActuator) - // TransactionAutoConfiguration lives here in Spring Boot 4.0+ (was in spring-boot-autoconfigure - // in 3.x). TransactionTemplateHijackProofTest needs it on the classpath to verify the - // factory's behaviour against Boot's auto-created TransactionTemplate bean. - testImplementation(libs.springBootTransaction) + // TransactionAutoConfiguration: in Spring Boot 4.0+ it lives in its own spring-boot-transaction + // module; in 3.x it ships inside spring-boot-autoconfigure (already on the test classpath). + // TransactionTemplateHijackProofTest resolves whichever FQCN is present, so we only need the + // extra dependency on 4.x. + if (springBootMajorForTests >= 4) { + testImplementation(libs.springBootTransaction) + } // Logback's ListAppender is used to capture and assert WARN-level log output (e.g. the // LiquibaseDisabledNotice breadcrumb + our PTM↔DS validation cannot-verify WARN) — slf4j-simple // does not provide an introspectable appender. From cb48148419ac54e9fff298a5bb5819ebae012e30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20Kobyli=C5=84ski?= Date: Sun, 17 May 2026 16:13:25 +0200 Subject: [PATCH 5/8] fix(ci): correct build script indentation for ktlintKotlinScriptCheck (KOJAK-67) The multiline `springBootMajorForTests` expression failed ktlintKotlinScriptCheck (build.gradle.kts:43 unexpected indentation). That task is separate from ktlintCheck-over-source-sets and was not run locally after the previous fix. Applied ktlintFormat; full `./gradlew ktlintCheck` now clean across all modules and Kotlin scripts. --- okapi-spring-boot/build.gradle.kts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/okapi-spring-boot/build.gradle.kts b/okapi-spring-boot/build.gradle.kts index 4d5ef5d..926f702 100644 --- a/okapi-spring-boot/build.gradle.kts +++ b/okapi-spring-boot/build.gradle.kts @@ -40,7 +40,7 @@ if (providers.gradleProperty("enableMutationTesting").orNull?.toBoolean() == tru // it try to resolve a non-existent spring-boot-transaction:3.5.x. Gate on the resolved major. val springBootMajorForTests = ( providers.gradleProperty("springBootVersion").orNull ?: libs.versions.springBoot.get() -).substringBefore('.').toInt() + ).substringBefore('.').toInt() dependencies { implementation(project(":okapi-core")) From 076e25315503571bb464e9a4950931a46ec5965a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20Kobyli=C5=84ski?= Date: Tue, 19 May 2026 09:24:09 +0200 Subject: [PATCH 6/8] refactor(spring-boot): apply full PR re-review findings #1-#10 (KOJAK-67) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second comprehensive multi-agent review of PR #49. One correctness defect, three documentation/comment fixes, two test-quality additions, and DRY cleanup. Correctness (#1): - validatePtmDataSourceMatch: when a DelegatingDataSource chain terminates early (cycle / uninitialised LazyConnectionDataSourceProxy), the `!==` is inconclusive — we never reached the concrete backing DataSources. Previously logged a WARN then unconditionally threw the generic "bound to a different DataSource" error, asserting an unverified mismatch and misdirecting the operator to qualifier config instead of proxy wiring. Now throws a distinct proxy-chain-unresolvable error; the mismatch error is reserved for the case where both sides fully unwrap to concrete, distinct DataSources. Comments (#3,#4,#8,#9): - Removed duplicate Hibernate package-move paragraph (ktlint-fix commit introduced it); trimmed "Strategy N" labels duplicating the method list; fixed self-referential stale comment ("above, around line 254" was AT 254); corrected Hibernate version comment drift (said 7.0, pin is 7.1.4). Tests (#2,#6,#7): - PROBE test empirically establishes Spring's InitializingBean contract rejects a TransactionTemplate bean with null transactionManager — the factory's `?:` fallback is unreachable-via-Spring defensive code, not an untested gap (production comment updated to point at the proof). - Restored JpaPtmClassesReflectionGuardTest in okapi-integration-tests (spring-orm available there); JPA_HIBERNATE_PTM_CLASSES made public for cross-module access. Guards the set against full bit-rot. - Trimmed deleted-version history from TransactionTemplateHijackProofTest and SpringObjectProviderSemanticsAssumptionsTest KDocs. DRY (#5,#10): - Extracted byte-identical stubStore()/stubDeliverer()/stubDelivererWithType() from 7 test files into OkapiSpringTestStubs.kt; added a reified GenericApplicationContext.registerBean() helper collapsing the BeanDefinitionBuilder.genericBeanDefinition(...).apply{isPrimary} boilerplate (9 call sites). Verified: full ktlintCheck clean, :okapi-spring-boot:test + :okapi-integration-tests:test green, Spring Boot 3.5.12 matrix simulation (-PspringBootVersion=3.5.12 -PspringVersion=6.2.7) green. --- gradle/libs.versions.toml | 4 +- .../JpaPtmClassesReflectionGuardTest.kt | 35 ++++++ .../springboot/OutboxAutoConfiguration.kt | 52 ++++----- ...ataSourceQualifierAutoConfigurationTest.kt | 29 +---- .../LiquibaseAutoConfigurationTest.kt | 18 --- .../okapi/springboot/LiquibaseE2ETest.kt | 7 -- .../okapi/springboot/OkapiSpringTestStubs.kt | 49 ++++++++ ...xAutoConfigurationTransactionRunnerTest.kt | 110 +++++++----------- .../OutboxProcessorAutoConfigurationTest.kt | 20 ---- .../OutboxPurgerAutoConfigurationTest.kt | 18 --- ...gObjectProviderSemanticsAssumptionsTest.kt | 3 +- .../TransactionTemplateHijackProofTest.kt | 24 +--- 12 files changed, 160 insertions(+), 209 deletions(-) create mode 100644 okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaPtmClassesReflectionGuardTest.kt create mode 100644 okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OkapiSpringTestStubs.kt diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index f69d17b..c9a3394 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -20,8 +20,8 @@ h2 = "2.4.240" micrometer = "1.16.5" jmh = "1.37" jmhGradlePlugin = "0.7.3" -# Hibernate version aligned with what Spring 7.x targets (Hibernate 7.0 ORM); only used in the -# integration-tests module to exercise JpaTransactionManager fail-fast extraction. +# Hibernate ORM 7.x (the line Spring Framework 7.x targets); integration-tests only, to exercise +# JpaTransactionManager fail-fast extraction. hibernate = "7.1.4.Final" [libraries] diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaPtmClassesReflectionGuardTest.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaPtmClassesReflectionGuardTest.kt new file mode 100644 index 0000000..79dc5c6 --- /dev/null +++ b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaPtmClassesReflectionGuardTest.kt @@ -0,0 +1,35 @@ +package com.softwaremill.okapi.test.transaction + +import com.softwaremill.okapi.springboot.OutboxAutoConfiguration +import io.kotest.assertions.withClue +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.collections.shouldNotBeEmpty + +/** + * Guards `OutboxAutoConfiguration.JPA_HIBERNATE_PTM_CLASSES` against full bit-rot: if all listed + * FQCNs become unresolvable (Spring rename / package move), `extractDataSource`'s JPA branch + * silently never fires and every JPA user falls through to the WARN-only path. Runs in + * okapi-integration-tests because spring-orm is a `testImplementation` there. Mirrors the + * `@AutoConfigureAfter`-resolves guard on OkapiMicrometerAutoConfiguration. + */ +class JpaPtmClassesReflectionGuardTest : FunSpec({ + + test("at least one FQCN in JPA_HIBERNATE_PTM_CLASSES resolves on the runtime classpath") { + val classLoader = OutboxAutoConfiguration::class.java.classLoader + val resolvable = OutboxAutoConfiguration.JPA_HIBERNATE_PTM_CLASSES.filter { fqcn -> + try { + Class.forName(fqcn, false, classLoader) + true + } catch (_: ClassNotFoundException) { + false + } + } + withClue( + "None of ${OutboxAutoConfiguration.JPA_HIBERNATE_PTM_CLASSES} resolves — the JPA branch " + + "of extractDataSource is now dead code (Spring likely renamed a class or moved a " + + "package). Update the set.", + ) { + resolvable.shouldNotBeEmpty() + } + } +}) diff --git a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt index d801cc4..32fba80 100644 --- a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt +++ b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt @@ -109,8 +109,10 @@ class OutboxAutoConfiguration( ): TransactionRunner { val anyTemplate = transactionTemplate.getIfUnique() // Extract the PTM from the TT if available, else resolve through the provider. TT's - // transactionManager is nullable in the API (a TT can be constructed without one) — fall - // back to the provider path in that pathological case so we still get a validated PTM. + // transactionManager is nullable in the API, but Spring's InitializingBean contract rejects + // a TransactionTemplate bean with a null transactionManager at afterPropertiesSet() (proven + // by the PROBE test in OutboxAutoConfigurationTransactionRunnerTest). The `?:` is therefore + // an unreachable-via-Spring defensive guard, kept correct for any non-Spring caller. val ptm = anyTemplate?.transactionManager ?: resolvePlatformTransactionManager(transactionManager, beanFactory, okapiProperties) validatePtmDataSourceMatch(ptm, resolveDataSource(), okapiProperties) @@ -321,21 +323,21 @@ class OutboxAutoConfiguration( val unwrappedOutbox = unwrapDataSource(outboxDataSource) if (unwrappedPtm !== unwrappedOutbox) { // If either side is still a DelegatingDataSource after unwrap, the chain terminated - // early — either a cycle (`setTargetDataSource(self)`) or a not-yet-initialised - // `LazyConnectionDataSourceProxy.targetDataSource == null`. Surface that as a distinct - // WARN so the operator looks at the proxy wiring instead of chasing the PTM↔DS error. + // early — a cycle (`setTargetDataSource(self)`) or a not-yet-initialised + // `LazyConnectionDataSourceProxy.targetDataSource == null`. In that case the `!==` + // is inconclusive: we never reached the concrete backing DataSources, so we do NOT + // know whether they actually differ. Throwing the PTM↔DS mismatch error here would + // assert something we did not verify and send the operator chasing the wrong fix + // (qualifier config) instead of the real one (proxy wiring). Raise a distinct error. if (unwrappedPtm is DelegatingDataSource || unwrappedOutbox is DelegatingDataSource) { - logger.warn( - "Could not fully unwrap one or both DataSource sides — at least one " + - "DelegatingDataSource chain terminated early (cycle, or " + - "LazyConnectionDataSourceProxy with targetDataSource not yet set). " + - "PTM side: {} (stopped at {}). Outbox side: {} (stopped at {}). If the " + - "two are intended to wrap the same DataSource, fix the proxy chain " + - "before relying on the PTM↔DataSource mismatch error below.", - ptmDataSource, - unwrappedPtm, - outboxDataSource, - unwrappedOutbox, + error( + "Could not verify the PTM↔DataSource binding: a DelegatingDataSource chain " + + "terminated early — a cycle (setTargetDataSource(self)) or an " + + "uninitialised LazyConnectionDataSourceProxy (targetDataSource not yet " + + "set). PTM side: $ptmDataSource (stopped at $unwrappedPtm). Outbox side: " + + "$outboxDataSource (stopped at $unwrappedOutbox). This is NOT necessarily " + + "a DataSource mismatch — fix the proxy wiring so both chains resolve to a " + + "concrete DataSource, or define an explicit @Bean TransactionRunner.", ) } error( @@ -397,25 +399,23 @@ class OutboxAutoConfiguration( // JPA/Hibernate PTMs that expose a `public DataSource getDataSource()` — reflection by name // avoids requiring spring-orm on the compile classpath (it's optional for JDBC-only consumers). - // Hibernate's TM moved package in Spring 6.2 — both names are listed so a single okapi build - // works against Spring Framework 6.1.x and 6.2+ without a version matrix. - // Hibernate's TM is named `org.springframework.orm.jpa.hibernate.HibernateTransactionManager` - // in Spring 6.2+ and `org.springframework.orm.hibernate5.HibernateTransactionManager` in - // Spring 6.1-. Both are listed so a single build works across versions. - // `internal` so reflection-resolution tests can verify the set isn't all stale. - internal val JPA_HIBERNATE_PTM_CLASSES = setOf( + // Hibernate's TM is `org.springframework.orm.jpa.hibernate.HibernateTransactionManager` in + // Spring 6.2+ and `org.springframework.orm.hibernate5.HibernateTransactionManager` in 6.1-; + // both are listed so a single build works across versions without a matrix. + // Public so the cross-module reflection-resolution guard test (in okapi-integration-tests, + // where spring-orm is available) can verify the set isn't entirely stale. + val JPA_HIBERNATE_PTM_CLASSES = setOf( "org.springframework.orm.jpa.JpaTransactionManager", "org.springframework.orm.jpa.hibernate.HibernateTransactionManager", "org.springframework.orm.hibernate5.HibernateTransactionManager", ) internal fun extractDataSource(ptm: PlatformTransactionManager): DataSource? { - // Strategy 1: ResourceTransactionManager (DST + any RTM whose resourceFactory is a DataSource). (ptm as? ResourceTransactionManager)?.resourceFactory?.let { rf -> if (rf is DataSource) return rf } - // Strategy 2/3: JPA/Hibernate public `getDataSource()`. Narrow catch on - // NoSuchMethodException only — all other exceptions intentionally propagate: + // JPA/Hibernate public `getDataSource()`. Narrow catch on NoSuchMethodException only — + // all other exceptions intentionally propagate: // LinkageError / NoClassDefFoundError → mixed-jar classpath bug, must surface (this // is why we don't use `runCatching`, which would swallow them). // InvocationTargetException → JpaTransactionManager constructed without an EMF; diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/DataSourceQualifierAutoConfigurationTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/DataSourceQualifierAutoConfigurationTest.kt index c45ad39..08019f6 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/DataSourceQualifierAutoConfigurationTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/DataSourceQualifierAutoConfigurationTest.kt @@ -1,21 +1,16 @@ package com.softwaremill.okapi.springboot -import com.softwaremill.okapi.core.DeliveryResult import com.softwaremill.okapi.core.MessageDeliverer -import com.softwaremill.okapi.core.OutboxEntry -import com.softwaremill.okapi.core.OutboxStatus import com.softwaremill.okapi.core.OutboxStore import com.softwaremill.okapi.core.TransactionRunner import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.nulls.shouldNotBeNull import io.kotest.matchers.string.shouldContain import io.kotest.matchers.types.shouldBeSameInstanceAs -import org.springframework.beans.factory.support.BeanDefinitionBuilder import org.springframework.boot.autoconfigure.AutoConfigurations import org.springframework.boot.test.context.runner.ApplicationContextRunner import org.springframework.context.support.GenericApplicationContext import org.springframework.jdbc.datasource.SimpleDriverDataSource -import java.time.Instant import javax.sql.DataSource class DataSourceQualifierAutoConfigurationTest : FunSpec({ @@ -41,10 +36,7 @@ class DataSourceQualifierAutoConfigurationTest : FunSpec({ val outboxDs = SimpleDriverDataSource() baseRunner() .withInitializer { context -> - val bd = BeanDefinitionBuilder.genericBeanDefinition(DataSource::class.java) { primaryDs } - .beanDefinition - bd.isPrimary = true - (context as GenericApplicationContext).registerBeanDefinition("primaryDs", bd) + (context as GenericApplicationContext).registerBean("primaryDs", primary = true) { primaryDs } } .withBean("outboxDs", DataSource::class.java, { outboxDs }) .withPropertyValues("okapi.datasource-qualifier=outboxDs") @@ -74,10 +66,7 @@ class DataSourceQualifierAutoConfigurationTest : FunSpec({ val secondaryDs = SimpleDriverDataSource() baseRunner() .withInitializer { context -> - val bd = BeanDefinitionBuilder.genericBeanDefinition(DataSource::class.java) { primaryDs } - .beanDefinition - bd.isPrimary = true - (context as GenericApplicationContext).registerBeanDefinition("primaryDs", bd) + (context as GenericApplicationContext).registerBean("primaryDs", primary = true) { primaryDs } } .withBean("secondaryDs", DataSource::class.java, { secondaryDs }) .run { ctx -> @@ -94,17 +83,3 @@ class DataSourceQualifierAutoConfigurationTest : FunSpec({ private fun noOpTransactionRunner() = object : TransactionRunner { override fun runInTransaction(block: () -> T): T = block() } - -private fun stubStore() = object : OutboxStore { - override fun persist(entry: OutboxEntry) = entry - override fun claimPending(limit: Int) = emptyList() - override fun updateAfterProcessing(entry: OutboxEntry) = entry - override fun removeDeliveredBefore(time: Instant, limit: Int) = 0 - override fun findOldestCreatedAt(statuses: Set) = emptyMap() - override fun countByStatuses() = emptyMap() -} - -private fun stubDeliverer() = object : MessageDeliverer { - override val type = "stub" - override fun deliver(entry: OutboxEntry) = DeliveryResult.Success -} diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/LiquibaseAutoConfigurationTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/LiquibaseAutoConfigurationTest.kt index 0a91432..80ae010 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/LiquibaseAutoConfigurationTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/LiquibaseAutoConfigurationTest.kt @@ -4,10 +4,7 @@ import ch.qos.logback.classic.Level import ch.qos.logback.classic.Logger import ch.qos.logback.classic.spi.ILoggingEvent import ch.qos.logback.core.read.ListAppender -import com.softwaremill.okapi.core.DeliveryResult import com.softwaremill.okapi.core.MessageDeliverer -import com.softwaremill.okapi.core.OutboxEntry -import com.softwaremill.okapi.core.OutboxStatus import com.softwaremill.okapi.core.OutboxStore import com.softwaremill.okapi.core.TransactionRunner import io.kotest.assertions.throwables.shouldThrow @@ -31,7 +28,6 @@ import org.springframework.boot.test.context.FilteredClassLoader import org.springframework.boot.test.context.runner.ApplicationContextRunner import org.springframework.core.annotation.AnnotatedElementUtils import org.springframework.jdbc.datasource.SimpleDriverDataSource -import java.time.Instant import javax.sql.DataSource import io.kotest.matchers.string.shouldContain as stringShouldContain @@ -518,17 +514,3 @@ private fun canLoadClass(fqcn: String, classLoader: ClassLoader): Boolean = try private fun noOpTransactionRunner() = object : TransactionRunner { override fun runInTransaction(block: () -> T): T = block() } - -private fun stubStore() = object : OutboxStore { - override fun persist(entry: OutboxEntry) = entry - override fun claimPending(limit: Int) = emptyList() - override fun updateAfterProcessing(entry: OutboxEntry) = entry - override fun removeDeliveredBefore(time: Instant, limit: Int) = 0 - override fun findOldestCreatedAt(statuses: Set) = emptyMap() - override fun countByStatuses() = emptyMap() -} - -private fun stubDeliverer() = object : MessageDeliverer { - override val type = "stub" - override fun deliver(entry: OutboxEntry) = DeliveryResult.Success -} diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/LiquibaseE2ETest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/LiquibaseE2ETest.kt index 6ead6d0..9a2719f 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/LiquibaseE2ETest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/LiquibaseE2ETest.kt @@ -1,9 +1,7 @@ package com.softwaremill.okapi.springboot import com.mysql.cj.jdbc.MysqlDataSource -import com.softwaremill.okapi.core.DeliveryResult import com.softwaremill.okapi.core.MessageDeliverer -import com.softwaremill.okapi.core.OutboxEntry import com.softwaremill.okapi.mysql.MysqlOutboxStore import com.softwaremill.okapi.postgres.PostgresOutboxStore import io.kotest.core.spec.style.FunSpec @@ -413,8 +411,3 @@ private fun resolveSpringBootClass(vararg candidateFqcns: String): Class<*>? { } } } - -private fun stubDeliverer() = object : MessageDeliverer { - override val type = "stub" - override fun deliver(entry: OutboxEntry) = DeliveryResult.Success -} diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OkapiSpringTestStubs.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OkapiSpringTestStubs.kt new file mode 100644 index 0000000..da0d571 --- /dev/null +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OkapiSpringTestStubs.kt @@ -0,0 +1,49 @@ +package com.softwaremill.okapi.springboot + +import com.softwaremill.okapi.core.DeliveryResult +import com.softwaremill.okapi.core.MessageDeliverer +import com.softwaremill.okapi.core.OutboxEntry +import com.softwaremill.okapi.core.OutboxStatus +import com.softwaremill.okapi.core.OutboxStore +import org.springframework.beans.factory.support.BeanDefinitionBuilder +import org.springframework.context.support.GenericApplicationContext +import java.time.Instant + +/** + * Shared no-op test doubles + a bean-registration helper for okapi-spring-boot autoconfig slice + * tests. Previously these were copy-pasted verbatim into 7+ test files; a single source means an + * `OutboxStore` interface change is a one-line edit, not a 7-file sweep. + */ + +internal fun stubStore() = object : OutboxStore { + override fun persist(entry: OutboxEntry) = entry + override fun claimPending(limit: Int) = emptyList() + override fun updateAfterProcessing(entry: OutboxEntry) = entry + override fun removeDeliveredBefore(time: Instant, limit: Int) = 0 + override fun findOldestCreatedAt(statuses: Set) = emptyMap() + override fun countByStatuses() = emptyMap() +} + +internal fun stubDeliverer() = stubDelivererWithType("stub") + +internal fun stubDelivererWithType(t: String) = object : MessageDeliverer { + override val type = t + override fun deliver(entry: OutboxEntry) = DeliveryResult.Success +} + +/** + * Registers a bean via [GenericApplicationContext.registerBeanDefinition], optionally `@Primary`. + * Collapses the `BeanDefinitionBuilder.genericBeanDefinition(...).beanDefinition.apply { isPrimary }` + * boilerplate that the multi-DataSource tests repeat for every DataSource / PTM bean. + */ +internal inline fun GenericApplicationContext.registerBean( + name: String, + primary: Boolean = false, + crossinline supplier: () -> T, +) { + registerBeanDefinition( + name, + BeanDefinitionBuilder.genericBeanDefinition(T::class.java) { supplier() }.beanDefinition + .apply { isPrimary = primary }, + ) +} diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt index 87c7035..d2469b0 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt @@ -4,12 +4,10 @@ import ch.qos.logback.classic.Level import ch.qos.logback.classic.Logger import ch.qos.logback.classic.spi.ILoggingEvent import ch.qos.logback.core.read.ListAppender -import com.softwaremill.okapi.core.DeliveryResult import com.softwaremill.okapi.core.MessageDeliverer -import com.softwaremill.okapi.core.OutboxEntry -import com.softwaremill.okapi.core.OutboxStatus import com.softwaremill.okapi.core.OutboxStore import com.softwaremill.okapi.core.TransactionRunner +import io.kotest.assertions.withClue import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.collections.shouldNotBeEmpty import io.kotest.matchers.nulls.shouldBeNull @@ -21,7 +19,6 @@ import io.kotest.matchers.types.shouldBeSameInstanceAs import org.h2.jdbcx.JdbcDataSource import org.slf4j.LoggerFactory import org.springframework.beans.factory.NoSuchBeanDefinitionException -import org.springframework.beans.factory.support.BeanDefinitionBuilder import org.springframework.boot.autoconfigure.AutoConfigurations import org.springframework.boot.test.context.runner.ApplicationContextRunner import org.springframework.context.annotation.Bean @@ -38,7 +35,6 @@ import org.springframework.transaction.support.AbstractPlatformTransactionManage import org.springframework.transaction.support.DefaultTransactionStatus import org.springframework.transaction.support.ResourceTransactionManager import org.springframework.transaction.support.TransactionSynchronizationManager -import java.time.Instant import javax.sql.DataSource import kotlin.time.Duration.Companion.seconds @@ -145,6 +141,38 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ } } + test("PROBE: can a TransactionTemplate bean even exist with a null transactionManager?") { + // The okapiTransactionRunner factory has `anyTemplate?.transactionManager ?: resolve(...)`. + // pr-test-analyzer flagged the null branch as untested. But TransactionTemplate implements + // InitializingBean and afterPropertiesSet() throws "Property 'transactionManager' is required" + // when null — Spring invokes that on every @Bean. This probe empirically establishes whether + // the null-TM branch is reachable for a Spring-managed TT at all, or is defensive dead code. + val ds: DataSource = SimpleDriverDataSource() + baseRunner + .withBean(DataSource::class.java, { ds }) + .withBean(PlatformTransactionManager::class.java, { DataSourceTransactionManager(ds) }) + .withBean( + "nullTmTemplate", + org.springframework.transaction.support.TransactionTemplate::class.java, + { org.springframework.transaction.support.TransactionTemplate() }, + ) + .run { ctx -> + // Record the empirically observed behaviour; the assertion below documents it. + val startupFailed = ctx.startupFailure != null + val rootMsg = ctx.startupFailure?.let { f -> + generateSequence(f as Throwable?) { it.cause }.last().message + } + withClue("observed startupFailed=$startupFailed rootMsg=$rootMsg") { + // Spring rejects a TransactionTemplate bean with null transactionManager at + // InitializingBean.afterPropertiesSet(): the factory's `?:` fallback is therefore + // unreachable for a *Spring-managed* TT. It remains a correct defensive guard for + // the theoretical non-Spring path, but cannot be exercised via the bean container. + startupFailed shouldBe true + rootMsg.shouldNotBeNull().shouldContain("transactionManager") + } + } + } + test("user-provided @Bean TransactionTemplate is honoured (autoconfig does not silently shadow it)") { // If a user defines a custom TransactionTemplate (e.g. with non-default timeout / propagation / // isolation), the autoconfig MUST use that exact template — not silently build its own fresh @@ -218,27 +246,10 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ .withBean(MessageDeliverer::class.java, { stubDeliverer() }) .withInitializer { context -> val gac = context as GenericApplicationContext - gac.registerBeanDefinition( - "appDs", - BeanDefinitionBuilder.genericBeanDefinition(DataSource::class.java) { appDs }.beanDefinition - .apply { isPrimary = true }, - ) - gac.registerBeanDefinition( - "outboxDs", - BeanDefinitionBuilder.genericBeanDefinition(DataSource::class.java) { outboxDs }.beanDefinition, - ) - gac.registerBeanDefinition( - "appTm", - BeanDefinitionBuilder.genericBeanDefinition(PlatformTransactionManager::class.java) { - DataSourceTransactionManager(appDs) - }.beanDefinition.apply { isPrimary = true }, - ) - gac.registerBeanDefinition( - "outboxTm", - BeanDefinitionBuilder.genericBeanDefinition(PlatformTransactionManager::class.java) { - DataSourceTransactionManager(outboxDs) - }.beanDefinition, - ) + gac.registerBean("appDs", primary = true) { appDs } + gac.registerBean("outboxDs") { outboxDs } + gac.registerBean("appTm", primary = true) { DataSourceTransactionManager(appDs) } + gac.registerBean("outboxTm") { DataSourceTransactionManager(outboxDs) } } .withPropertyValues( "okapi.datasource-qualifier=outboxDs", @@ -251,7 +262,7 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ } test("ResourceTransactionManager PTM with non-DataSource resourceFactory: WARN includes actual resourceFactory class name") { - // BUG C1 test (above, around line 254) covers the WARN-is-emitted contract for this case. + // The BUG C1 regression guard pins the WARN-is-emitted contract for this case. // This test pins the diagnostic CONTENT: the WARN must mention the actual resourceFactory // class so a user with a custom RTM (or buggy subclass returning null) sees the real // resource type, not just the static example list (JpaTransactionManager/Hibernate). Without @@ -379,21 +390,9 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ .withBean(MessageDeliverer::class.java, { stubDeliverer() }) .withInitializer { context -> val gac = context as GenericApplicationContext - gac.registerBeanDefinition( - "appDs", - BeanDefinitionBuilder.genericBeanDefinition(DataSource::class.java) { appDs }.beanDefinition - .apply { isPrimary = true }, - ) - gac.registerBeanDefinition( - "outboxDs", - BeanDefinitionBuilder.genericBeanDefinition(DataSource::class.java) { outboxDs }.beanDefinition, - ) - gac.registerBeanDefinition( - "appTm", - BeanDefinitionBuilder.genericBeanDefinition(PlatformTransactionManager::class.java) { - DataSourceTransactionManager(appDs) - }.beanDefinition, - ) + gac.registerBean("appDs", primary = true) { appDs } + gac.registerBean("outboxDs") { outboxDs } + gac.registerBean("appTm") { DataSourceTransactionManager(appDs) } } .withPropertyValues("okapi.datasource-qualifier=outboxDs") .run { ctx -> @@ -463,17 +462,8 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ .withBean(MessageDeliverer::class.java, { stubDeliverer() }) .withInitializer { context -> val gac = context as GenericApplicationContext - gac.registerBeanDefinition( - "outboxDs", - BeanDefinitionBuilder.genericBeanDefinition(DataSource::class.java) { proxyDs }.beanDefinition - .apply { isPrimary = true }, - ) - gac.registerBeanDefinition( - "outboxTm", - BeanDefinitionBuilder.genericBeanDefinition(PlatformTransactionManager::class.java) { - DataSourceTransactionManager(rawDs) - }.beanDefinition, - ) + gac.registerBean("outboxDs", primary = true) { proxyDs } + gac.registerBean("outboxTm") { DataSourceTransactionManager(rawDs) } } .withPropertyValues("okapi.datasource-qualifier=outboxDs") .run { ctx -> @@ -609,17 +599,3 @@ private fun h2DataSource(): DataSource = JdbcDataSource().apply { user = "sa" password = "" } - -private fun stubStore() = object : OutboxStore { - override fun persist(entry: OutboxEntry) = entry - override fun claimPending(limit: Int) = emptyList() - override fun updateAfterProcessing(entry: OutboxEntry) = entry - override fun removeDeliveredBefore(time: Instant, limit: Int) = 0 - override fun findOldestCreatedAt(statuses: Set) = emptyMap() - override fun countByStatuses() = emptyMap() -} - -private fun stubDeliverer() = object : MessageDeliverer { - override val type = "stub" - override fun deliver(entry: OutboxEntry) = DeliveryResult.Success -} diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxProcessorAutoConfigurationTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxProcessorAutoConfigurationTest.kt index 15f0b5c..a3ef1e4 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxProcessorAutoConfigurationTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxProcessorAutoConfigurationTest.kt @@ -1,10 +1,7 @@ package com.softwaremill.okapi.springboot -import com.softwaremill.okapi.core.DeliveryResult import com.softwaremill.okapi.core.MessageDeliverer -import com.softwaremill.okapi.core.OutboxEntry import com.softwaremill.okapi.core.OutboxEntryProcessor -import com.softwaremill.okapi.core.OutboxStatus import com.softwaremill.okapi.core.OutboxStore import com.softwaremill.okapi.core.TransactionRunner import com.softwaremill.okapi.micrometer.MicrometerOutboxListener @@ -22,7 +19,6 @@ import org.springframework.jdbc.datasource.SimpleDriverDataSource import java.time.Duration.ofMillis import java.time.Duration.ofMinutes import java.time.Duration.ofSeconds -import java.time.Instant import javax.sql.DataSource class OutboxProcessorAutoConfigurationTest : FunSpec({ @@ -235,19 +231,3 @@ private fun resolveSpringBootClass(vararg candidateFqcns: String): Class<*> { private fun noOpTransactionRunner() = object : TransactionRunner { override fun runInTransaction(block: () -> T): T = block() } - -private fun stubStore() = object : OutboxStore { - override fun persist(entry: OutboxEntry) = entry - override fun claimPending(limit: Int) = emptyList() - override fun updateAfterProcessing(entry: OutboxEntry) = entry - override fun removeDeliveredBefore(time: Instant, limit: Int) = 0 - override fun findOldestCreatedAt(statuses: Set) = emptyMap() - override fun countByStatuses() = emptyMap() -} - -private fun stubDeliverer() = stubDelivererWithType("stub") - -private fun stubDelivererWithType(t: String) = object : MessageDeliverer { - override val type = t - override fun deliver(entry: OutboxEntry) = DeliveryResult.Success -} diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxPurgerAutoConfigurationTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxPurgerAutoConfigurationTest.kt index ba7c6ca..a25055a 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxPurgerAutoConfigurationTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxPurgerAutoConfigurationTest.kt @@ -1,10 +1,7 @@ package com.softwaremill.okapi.springboot -import com.softwaremill.okapi.core.DeliveryResult import com.softwaremill.okapi.core.MessageDeliverer -import com.softwaremill.okapi.core.OutboxEntry import com.softwaremill.okapi.core.OutboxPurgerConfig -import com.softwaremill.okapi.core.OutboxStatus import com.softwaremill.okapi.core.OutboxStore import com.softwaremill.okapi.core.TransactionRunner import io.kotest.core.spec.style.FunSpec @@ -16,7 +13,6 @@ import org.springframework.jdbc.datasource.SimpleDriverDataSource import java.time.Duration.ofDays import java.time.Duration.ofHours import java.time.Duration.ofMinutes -import java.time.Instant import javax.sql.DataSource class OutboxPurgerAutoConfigurationTest : FunSpec({ @@ -109,17 +105,3 @@ class OutboxPurgerAutoConfigurationTest : FunSpec({ private fun noOpTransactionRunner() = object : TransactionRunner { override fun runInTransaction(block: () -> T): T = block() } - -private fun stubStore() = object : OutboxStore { - override fun persist(entry: OutboxEntry) = entry - override fun claimPending(limit: Int) = emptyList() - override fun updateAfterProcessing(entry: OutboxEntry) = entry - override fun removeDeliveredBefore(time: Instant, limit: Int) = 0 - override fun findOldestCreatedAt(statuses: Set) = emptyMap() - override fun countByStatuses() = emptyMap() -} - -private fun stubDeliverer() = object : MessageDeliverer { - override val type = "stub" - override fun deliver(entry: OutboxEntry) = DeliveryResult.Success -} diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringObjectProviderSemanticsAssumptionsTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringObjectProviderSemanticsAssumptionsTest.kt index b36f7ff..244d1a3 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringObjectProviderSemanticsAssumptionsTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringObjectProviderSemanticsAssumptionsTest.kt @@ -27,8 +27,7 @@ import org.springframework.transaction.support.ResourceTransactionManager * Used to distinguish "no PTM" from "multiple PTMs" via stream count. * 2. `getIfUnique()` returns the `@Primary` bean when present. * 3. `getIfAvailable()` THROWS `NoUniqueBeanDefinitionException` for 2+ non-primary - * candidates — the previous fix used this and surfaced a misleading error; switching to - * `getIfUnique()` was a deliberate semantic change. + * candidates (which is why the autoconfig uses `getIfUnique()`, not `getIfAvailable()`). * 4. `DataSourceTransactionManager` IS-A `ResourceTransactionManager`, with * `resourceFactory == DataSource`. Our PTM↔DS validation depends on this cast. * 5. PTMs that extend `AbstractPlatformTransactionManager` directly (e.g. Exposed diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/TransactionTemplateHijackProofTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/TransactionTemplateHijackProofTest.kt index 5302adb..4259087 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/TransactionTemplateHijackProofTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/TransactionTemplateHijackProofTest.kt @@ -1,9 +1,6 @@ package com.softwaremill.okapi.springboot -import com.softwaremill.okapi.core.DeliveryResult import com.softwaremill.okapi.core.MessageDeliverer -import com.softwaremill.okapi.core.OutboxEntry -import com.softwaremill.okapi.core.OutboxStatus import com.softwaremill.okapi.core.OutboxStore import com.softwaremill.okapi.core.TransactionRunner import io.kotest.assertions.withClue @@ -19,7 +16,6 @@ import org.springframework.jdbc.datasource.DataSourceTransactionManager import org.springframework.jdbc.datasource.SimpleDriverDataSource import org.springframework.transaction.PlatformTransactionManager import org.springframework.transaction.support.TransactionTemplate -import java.time.Instant import javax.sql.DataSource /** @@ -27,10 +23,8 @@ import javax.sql.DataSource * "Spring Boot's auto-configured TransactionTemplate hijacks okapiTransactionRunner — the factory * short-circuits to the Boot-supplied template and skips PTM↔DataSource validation entirely." * - * The previous single-test version asserted only that startup failed in the mismatch scenario, then - * inferred "validation ran". That's brittle — context could fail for unrelated reasons, or autoconfig - * ordering in slice tests could differ from production. These three tests pin down three independent - * invariants so the conclusion is empirically forced: + * Three independent invariants pin the conclusion empirically (asserting only "startup failed" + * would be brittle — the context could fail for unrelated reasons): * * 1. PRECONDITION: Spring Boot's TransactionAutoConfiguration actually creates a `TransactionTemplate` * bean in slice tests. If false, the hijack scenario cannot occur in this test harness and the @@ -127,17 +121,3 @@ class TransactionTemplateHijackProofTest : FunSpec({ } } }) - -private fun stubStore() = object : OutboxStore { - override fun persist(entry: OutboxEntry) = entry - override fun claimPending(limit: Int) = emptyList() - override fun updateAfterProcessing(entry: OutboxEntry) = entry - override fun removeDeliveredBefore(time: Instant, limit: Int) = 0 - override fun findOldestCreatedAt(statuses: Set) = emptyMap() - override fun countByStatuses() = emptyMap() -} - -private fun stubDeliverer() = object : MessageDeliverer { - override val type = "stub" - override fun deliver(entry: OutboxEntry) = DeliveryResult.Success -} From 2981991400ff4eaadfb5eb64b3f0a904d687f425 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20Kobyli=C5=84ski?= Date: Wed, 20 May 2026 14:05:39 +0200 Subject: [PATCH 7/8] refactor(spring-boot): apply post-review findings #11-#13 + design fixes (KOJAK-67) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Issue 13: correct KDoc grouping — JpaTransactionManager IS-A ResourceTransactionManager (the non-RTM example is Exposed SpringTransactionManager only) - Issue 11: comment on okapiTransactionRunner deliberate interface return (required for @ConditionalOnMissingBean to suppress on ANY user TransactionRunner impl) - Issue 10: move pitest plugin + tool versions to gradle/libs.versions.toml (was inline in build.gradle.kts, violating .ai/coding.md) - Issue 4 : extractDataSource walks the superclass chain, recognising user subclasses of JpaTransactionManager / HibernateTransactionManager - Issue 6+12: refactor unwrapDataSource to sealed Unwrapped { Resolved | Unresolvable } with IdentityHashMap-backed cycle detection; validatePtmDataSourceMatch now branches on three explicit cases, removing the load-bearing comment - Issue 5 : fail-fast when non-extractable PTM + >=2 DataSource beans + no qualifier (single-DS stays on the existing INFO/WARN path). Replace the flaky amplification-proof integration test with a deterministic startup-failure test (WrongPtmMultiDataSourceFailFastTest) - Issue 7+8: rebuild TransactionTemplateHijackProofTest into two clean contract tests (ADOPTS Boot's TT verbatim + VALIDATES still runs on wrong-DS PTM) and revert SpringTransactionRunner.transactionTemplate from public to internal - Issue 9 : add CHANGELOG BREAKING entries for the constructor change and the new multi-DS fail-fast guard; expand README with multi-DS guidance and a direct-constructor migration note (linking #49, not the JIRA key) --- CHANGELOG.md | 19 +++ README.md | 12 ++ gradle/libs.versions.toml | 5 + .../JpaTransactionManagerFailFastTest.kt | 5 +- ...TransactionManagerMatchedDataSourceTest.kt | 12 +- ...rongPtmDataSourceAmplificationProofTest.kt | 131 --------------- .../WrongPtmMultiDataSourceFailFastTest.kt | 88 ++++++++++ okapi-spring-boot/build.gradle.kts | 8 +- .../springboot/OutboxAutoConfiguration.kt | 157 +++++++++++++----- .../springboot/SpringTransactionRunner.kt | 5 +- ...xAutoConfigurationTransactionRunnerTest.kt | 52 +++--- ...gObjectProviderSemanticsAssumptionsTest.kt | 10 +- .../TransactionTemplateHijackProofTest.kt | 80 +++------ 13 files changed, 323 insertions(+), 261 deletions(-) delete mode 100644 okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/WrongPtmDataSourceAmplificationProofTest.kt create mode 100644 okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/WrongPtmMultiDataSourceFailFastTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index 93ef533..5418d65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,25 @@ Until `1.0.0`, breaking changes may appear in any release and are flagged with * history; the resulting schema is unchanged. Existing installations from an earlier release: the `outbox:001` changeset checksum changed — they must start on a fresh okapi schema, or clear okapi's rows from `okapi_databasechangelog`, before upgrading. +- **`OutboxProcessorScheduler` / `OutboxPurgerScheduler` constructors now require a + non-null `TransactionRunner`** (previously a nullable `TransactionTemplate?`, with + `OutboxPurgerScheduler`'s parameter defaulted to `null`). Spring Boot autoconfig users + are unaffected — the autoconfig derives a `TransactionRunner` from any + `PlatformTransactionManager` on the classpath. Users constructing the schedulers + directly must supply a `TransactionRunner` (e.g. `SpringTransactionRunner(template)` or + a thin lambda wrapping their framework's native transaction primitive). The previous + null-default silently degraded `FOR UPDATE SKIP LOCKED` to JDBC auto-commit, permitting + duplicate delivery across processor instances. ([#49](https://github.com/softwaremill/okapi/pull/49)) +- **`okapi-spring-boot` autoconfig refuses to start when it cannot verify the + PlatformTransactionManager↔outbox-DataSource binding** in a multi-DataSource context. + Specifically: if `extractDataSource` cannot determine the PTM's DataSource (e.g. JTA, + Exposed's `SpringTransactionManager`, or any PTM that exposes neither a `DataSource` + resourceFactory nor a public `getDataSource()`), AND the context has ≥2 `DataSource` + beans, AND neither `okapi.transaction-manager-qualifier` nor + `okapi.datasource-qualifier` is set, the context refresh now fails with an actionable + message. Single-DataSource contexts and explicitly-qualified setups are unaffected. + Escape hatch: supply an explicit `@Bean TransactionRunner` to bypass validation. + ([#49](https://github.com/softwaremill/okapi/pull/49)) ### Added diff --git a/README.md b/README.md index 52f7887..34b5565 100644 --- a/README.md +++ b/README.md @@ -92,6 +92,18 @@ fun outboxTransactionRunner(): TransactionRunner = object : TransactionRunner { Without bracketing, `FOR UPDATE SKIP LOCKED` collapses to the single SELECT statement under JDBC auto-commit, which silently allows duplicate delivery across processor instances. This opt-in is intentionally manual to keep accidental misconfiguration out of multi-instance deployments. +**Multi-DataSource contexts.** If your application has multiple `DataSource` beans and uses a `PlatformTransactionManager` from which okapi cannot extract a `DataSource` (JTA, Exposed's `SpringTransactionManager`, JPA without a JDBC `DataSource`), the autoconfiguration refuses to start until you disambiguate explicitly — either by setting `okapi.transaction-manager-qualifier` (the bean name of the PTM that brackets the outbox `DataSource`) and/or `okapi.datasource-qualifier`, or by supplying your own `@Bean TransactionRunner`. Single-DataSource setups and PTMs whose DataSource can be introspected (`DataSourceTransactionManager`, `JpaTransactionManager`, `HibernateTransactionManager`) are unaffected. + +**Constructing schedulers directly (non-autoconfig usage).** When wiring `OutboxProcessorScheduler` / `OutboxPurgerScheduler` manually (Ktor, custom Spring contexts without autoconfig, etc.), supply a `TransactionRunner` explicitly — the parameter is required, with no default: + +```kotlin +OutboxProcessorScheduler( + outboxProcessor = processor, + transactionRunner = SpringTransactionRunner(template), // or your framework's equivalent + config = OutboxSchedulerConfig(...), +) +``` + ## How It Works Okapi implements the [transactional outbox pattern](https://softwaremill.com/microservices-101/) (see also: [microservices.io description](https://microservices.io/patterns/data/transactional-outbox.html)): diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index c9a3394..8f7c8e0 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -20,6 +20,9 @@ h2 = "2.4.240" micrometer = "1.16.5" jmh = "1.37" jmhGradlePlugin = "0.7.3" +pitestGradlePlugin = "1.19.0" +pitestTool = "1.17.0" +pitestJunit5Plugin = "1.2.1" # Hibernate ORM 7.x (the line Spring Framework 7.x targets); integration-tests only, to exercise # JpaTransactionManager fail-fast extraction. hibernate = "7.1.4.Final" @@ -69,3 +72,5 @@ jmhGeneratorAnnprocess = { module = "org.openjdk.jmh:jmh-generator-annprocess", [plugins] ktlint = { id = "org.jlleitschuh.gradle.ktlint", version.ref = "ktlint" } jmh = { id = "me.champeau.jmh", version.ref = "jmhGradlePlugin" } +# Mutation-testing — opt-in only, declared in okapi-spring-boot but applied conditionally +pitest = { id = "info.solidsoft.pitest", version.ref = "pitestGradlePlugin" } diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerFailFastTest.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerFailFastTest.kt index 44ffc4f..39ffaaa 100644 --- a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerFailFastTest.kt +++ b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerFailFastTest.kt @@ -26,8 +26,9 @@ import javax.sql.DataSource * Proves the JPA branch of `extractDataSource` (`OutboxAutoConfiguration.kt`): a * `JpaTransactionManager` whose auto-detected DataSource differs from okapi's outbox DataSource * triggers `validatePtmDataSourceMatch` fail-fast at startup. Companion to - * `WrongPtmDataSourceAmplificationProofTest`, which documents the remaining residual risk for - * PTMs that expose no DataSource at all (Exposed bridge, JTA). + * `WrongPtmMultiDataSourceFailFastTest`, which fails-fast on the remaining ambiguity for PTMs + * that expose no DataSource at all (Exposed bridge, JTA) combined with multiple `DataSource` + * beans and no qualifier. */ class JpaTransactionManagerFailFastTest : FunSpec({ diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerMatchedDataSourceTest.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerMatchedDataSourceTest.kt index f9d7668..0580915 100644 --- a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerMatchedDataSourceTest.kt +++ b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerMatchedDataSourceTest.kt @@ -15,7 +15,6 @@ import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.nulls.shouldBeNull import io.kotest.matchers.nulls.shouldNotBeNull import io.kotest.matchers.types.shouldBeInstanceOf -import io.kotest.matchers.types.shouldBeSameInstanceAs import org.springframework.boot.autoconfigure.AutoConfigurations import org.springframework.boot.test.context.runner.ApplicationContextRunner import org.springframework.orm.jpa.JpaTransactionManager @@ -73,11 +72,14 @@ class JpaTransactionManagerMatchedDataSourceTest : FunSpec({ }) .withPropertyValues("okapi.liquibase.enabled=false") .run { ctx -> + // Behaviour-level proof (does not peek at SpringTransactionRunner internals): + // - `startupFailure` is null → validatePtmDataSourceMatch took the match-return path + // (the fail-fast companion test JpaTransactionManagerFailFastTest pins what failure + // on mismatch looks like; together they cover both branches of the JPA path). + // - The TransactionRunner is `SpringTransactionRunner`, confirming the autoconfig + // wired the JPA PTM rather than skipping the factory. ctx.startupFailure.shouldBeNull() - val runner = ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() - // The runner's TransactionTemplate must point at OUR JPA PTM, proving extractDataSource - // returned the matched DS and validatePtmDataSourceMatch's return-after-match path was taken. - runner.transactionTemplate.transactionManager shouldBeSameInstanceAs jpaPtm + ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() } } }) diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/WrongPtmDataSourceAmplificationProofTest.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/WrongPtmDataSourceAmplificationProofTest.kt deleted file mode 100644 index 9d330bf..0000000 --- a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/WrongPtmDataSourceAmplificationProofTest.kt +++ /dev/null @@ -1,131 +0,0 @@ -package com.softwaremill.okapi.test.transaction - -import com.softwaremill.okapi.core.DeliveryInfo -import com.softwaremill.okapi.core.MessageDeliverer -import com.softwaremill.okapi.core.OutboxMessage -import com.softwaremill.okapi.core.OutboxProcessor -import com.softwaremill.okapi.core.TransactionRunner -import com.softwaremill.okapi.postgres.PostgresOutboxStore -import com.softwaremill.okapi.springboot.OkapiLiquibaseAutoConfiguration -import com.softwaremill.okapi.springboot.OutboxAutoConfiguration -import com.softwaremill.okapi.springboot.SpringConnectionProvider -import com.softwaremill.okapi.springboot.SpringOutboxPublisher -import com.softwaremill.okapi.test.support.RecordingMessageDeliverer -import com.softwaremill.okapi.test.support.pgDataSourceOf -import com.softwaremill.okapi.test.support.runOkapiLiquibaseOn -import io.kotest.core.spec.style.FunSpec -import io.kotest.matchers.maps.shouldNotBeEmpty -import io.kotest.matchers.shouldBe -import org.jetbrains.exposed.v1.spring7.transaction.SpringTransactionManager -import org.springframework.beans.factory.config.BeanDefinitionCustomizer -import org.springframework.boot.autoconfigure.AutoConfigurations -import org.springframework.boot.test.context.runner.ApplicationContextRunner -import org.springframework.jdbc.datasource.DataSourceTransactionManager -import org.springframework.transaction.PlatformTransactionManager -import org.springframework.transaction.support.TransactionTemplate -import org.testcontainers.containers.PostgreSQLContainer -import java.util.concurrent.CompletableFuture -import java.util.concurrent.CyclicBarrier -import java.util.concurrent.Executors -import java.util.concurrent.TimeUnit -import javax.sql.DataSource - -/** - * Documents the residual silent-failure risk for non-extractable PTMs (JTA, Exposed bridge, - * any `PlatformTransactionManager` that exposes neither a `DataSource` resourceFactory nor a - * public `getDataSource()`) when the user wires the outbox to a different DataSource than the - * PTM and does NOT set `okapi.transaction-manager-qualifier`. - * - * `validatePtmDataSourceMatch` can fail-fast for `DataSourceTransactionManager`, - * `JpaTransactionManager`, and `HibernateTransactionManager` (see `extractDataSource`). For the - * remaining PTM families it can only emit a WARN — Spring exposes no public API to derive the - * bound DataSource. This test pins down what "WARN only" empirically means: concurrent processors - * see fully unlocked rows and amplify delivery. - * - * Asserts amplification DID happen (50/50 entries delivered more than once is the typical run). - * If a future change adds extraction support for the Exposed bridge — or pessimistic locking - * starts holding across the spurious auto-commit — this test will fail and force a re-evaluation. - */ -class WrongPtmDataSourceAmplificationProofTest : FunSpec({ - - val dsAContainer = PostgreSQLContainer("postgres:16") - val dsBContainer = PostgreSQLContainer("postgres:16") - - lateinit var dsA: DataSource - lateinit var dsB: DataSource - - beforeSpec { - dsAContainer.start() - dsBContainer.start() - dsA = pgDataSourceOf(dsAContainer) - dsB = pgDataSourceOf(dsBContainer) - // Migrate both: DS-B holds the outbox table the processor reads; DS-A would have it if - // okapi had picked it. Keeping parity rules out "table missing" as a cause of zero deliveries. - runOkapiLiquibaseOn(dsAContainer) - runOkapiLiquibaseOn(dsBContainer) - } - - afterSpec { - dsAContainer.stop() - dsBContainer.stop() - } - - test("non-extractable PTM bound to wrong DataSource permits delivery amplification") { - val recorder = RecordingMessageDeliverer() - - // Publish 50 entries to DS-B via a CORRECTLY-wired DST(DS-B) publisher. - val publishTpl = TransactionTemplate(DataSourceTransactionManager(dsB)) - val publishStore = PostgresOutboxStore(SpringConnectionProvider(dsB), java.time.Clock.systemUTC()) - val publisher = SpringOutboxPublisher( - delegate = com.softwaremill.okapi.core.OutboxPublisher(publishStore, java.time.Clock.systemUTC()), - dataSource = dsB, - ) - repeat(50) { i -> - publishTpl.execute { - publisher.publish(OutboxMessage("test.event", """{"i":$i}"""), AmplificationDeliveryInfo) - } - } - - ApplicationContextRunner() - .withConfiguration(AutoConfigurations.of(OutboxAutoConfiguration::class.java, OkapiLiquibaseAutoConfiguration::class.java)) - // DS-B as @Primary so resolveDataSource() picks it as the outbox DS. - .withBean("dsB", DataSource::class.java, { dsB }, BeanDefinitionCustomizer { it.isPrimary = true }) - .withBean("dsA", DataSource::class.java, { dsA }) - // Exposed SpringTransactionManager bound to DS-A — non-extractable: validatePtmDataSourceMatch - // logs a WARN and proceeds (the silent-failure setup this test documents). - .withBean("exposedTmA", PlatformTransactionManager::class.java, { SpringTransactionManager(dsA) }) - .withBean(MessageDeliverer::class.java, { recorder }) - .withBean(PostgresOutboxStore::class.java, { - PostgresOutboxStore(SpringConnectionProvider(dsB), java.time.Clock.systemUTC()) - }) - .withPropertyValues("okapi.processor.enabled=false", "okapi.liquibase.enabled=false") - .run { ctx -> - val processor = ctx.getBean(OutboxProcessor::class.java) - val transactionRunner = ctx.getBean(TransactionRunner::class.java) - - val barrier = CyclicBarrier(5) - val executor = Executors.newVirtualThreadPerTaskExecutor() - val futures = (1..5).map { - CompletableFuture.supplyAsync( - { - barrier.await(10, TimeUnit.SECONDS) - transactionRunner.runInTransaction { processor.processNext(50) } - }, - executor, - ) - } - CompletableFuture.allOf(*futures.toTypedArray()).get(60, TimeUnit.SECONDS) - executor.shutdown() - - recorder.deliveryCount() shouldBe 50 - // The residual risk: with FOR UPDATE SKIP LOCKED collapsed to auto-commit, concurrent - // processors see overlapping result sets and at least one entry is delivered twice. - recorder.deliveries.filter { it.value.size > 1 }.shouldNotBeEmpty() - } - } -}) - -private object AmplificationDeliveryInfo : DeliveryInfo { - override val type: String = "recording" - override fun serialize(): String = """{"type":"recording"}""" -} diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/WrongPtmMultiDataSourceFailFastTest.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/WrongPtmMultiDataSourceFailFastTest.kt new file mode 100644 index 0000000..3bc3390 --- /dev/null +++ b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/WrongPtmMultiDataSourceFailFastTest.kt @@ -0,0 +1,88 @@ +package com.softwaremill.okapi.test.transaction + +import com.softwaremill.okapi.core.MessageDeliverer +import com.softwaremill.okapi.postgres.PostgresOutboxStore +import com.softwaremill.okapi.springboot.OkapiLiquibaseAutoConfiguration +import com.softwaremill.okapi.springboot.OutboxAutoConfiguration +import com.softwaremill.okapi.springboot.SpringConnectionProvider +import com.softwaremill.okapi.test.support.RecordingMessageDeliverer +import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.nulls.shouldNotBeNull +import io.kotest.matchers.string.shouldContain +import org.jetbrains.exposed.v1.spring7.transaction.SpringTransactionManager +import org.springframework.beans.factory.config.BeanDefinitionCustomizer +import org.springframework.boot.autoconfigure.AutoConfigurations +import org.springframework.boot.test.context.runner.ApplicationContextRunner +import org.springframework.jdbc.datasource.SimpleDriverDataSource +import org.springframework.transaction.PlatformTransactionManager +import javax.sql.DataSource + +/** + * Replaces the former `WrongPtmDataSourceAmplificationProofTest`, which asserted a positive race + * outcome (`deliveries.filter { size > 1 }.shouldNotBeEmpty()`) — that assertion was both flaky + * (race-dependent) and semantically inverted (it would fail once the underlying risk was actually + * mitigated). The correct fix lives in production code: a non-extractable PTM combined with + * multiple `DataSource` beans and no `okapi.transaction-manager-qualifier` / `datasource-qualifier` + * is now refused at startup, eliminating the silent-duplicate residual risk rather than + * documenting it via a race. + * + * Pins the new behaviour deterministically: context refresh must fail with the new error message. + */ +class WrongPtmMultiDataSourceFailFastTest : FunSpec({ + + test("non-extractable PTM + multiple DataSource beans + no qualifier fails fast at startup") { + val dsA: DataSource = SimpleDriverDataSource() + val dsB: DataSource = SimpleDriverDataSource() + + ApplicationContextRunner() + .withConfiguration( + AutoConfigurations.of(OutboxAutoConfiguration::class.java, OkapiLiquibaseAutoConfiguration::class.java), + ) + // DS-B as @Primary so resolveDataSource() picks it as the outbox DS; DS-A is the + // ambiguous second DataSource that disambiguation would otherwise resolve. + .withBean("dsB", DataSource::class.java, { dsB }, BeanDefinitionCustomizer { it.isPrimary = true }) + .withBean("dsA", DataSource::class.java, { dsA }) + // Exposed's SpringTransactionManager is non-extractable (no DataSource resourceFactory, + // no public getDataSource()) — same shape as JtaTransactionManager. With ≥2 DataSource + // beans and no qualifier set, okapi cannot prove which DS this PTM brackets and refuses + // to start rather than risk silent duplicate delivery. + .withBean("exposedTmA", PlatformTransactionManager::class.java, { SpringTransactionManager(dsA) }) + .withBean(MessageDeliverer::class.java, { RecordingMessageDeliverer() }) + .withBean(PostgresOutboxStore::class.java, { + PostgresOutboxStore(SpringConnectionProvider(dsB), java.time.Clock.systemUTC()) + }) + .withPropertyValues("okapi.processor.enabled=false", "okapi.liquibase.enabled=false") + .run { ctx -> + ctx.startupFailure.shouldNotBeNull() + ctx.startupFailure!!.stackTraceToString() shouldContain + "Cannot verify the PTM↔DataSource binding for a non-extractable" + } + } + + test("setting okapi.transaction-manager-qualifier lets the same setup start cleanly (escape hatch)") { + val dsA: DataSource = SimpleDriverDataSource() + val dsB: DataSource = SimpleDriverDataSource() + + ApplicationContextRunner() + .withConfiguration( + AutoConfigurations.of(OutboxAutoConfiguration::class.java, OkapiLiquibaseAutoConfiguration::class.java), + ) + .withBean("dsB", DataSource::class.java, { dsB }, BeanDefinitionCustomizer { it.isPrimary = true }) + .withBean("dsA", DataSource::class.java, { dsA }) + .withBean("exposedTmA", PlatformTransactionManager::class.java, { SpringTransactionManager(dsA) }) + .withBean(MessageDeliverer::class.java, { RecordingMessageDeliverer() }) + .withBean(PostgresOutboxStore::class.java, { + PostgresOutboxStore(SpringConnectionProvider(dsB), java.time.Clock.systemUTC()) + }) + .withPropertyValues( + "okapi.processor.enabled=false", + "okapi.liquibase.enabled=false", + "okapi.transaction-manager-qualifier=exposedTmA", + ) + .run { ctx -> + // No startup failure: user has explicitly taken responsibility for the binding by + // naming the PTM. The autoconfig still cannot verify, but no longer refuses. + ctx.startupFailure?.let { error("Expected clean startup, got: ${it.message}") } + } + } +}) diff --git a/okapi-spring-boot/build.gradle.kts b/okapi-spring-boot/build.gradle.kts index 926f702..e936339 100644 --- a/okapi-spring-boot/build.gradle.kts +++ b/okapi-spring-boot/build.gradle.kts @@ -2,16 +2,16 @@ plugins { id("buildsrc.convention.kotlin-jvm") id("buildsrc.convention.publish") // Mutation testing — opt-in only: ./gradlew :okapi-spring-boot:pitest -PenableMutationTesting=true - id("info.solidsoft.pitest") version "1.19.0" apply false + alias(libs.plugins.pitest) apply false } description = "Spring Boot autoconfiguration for Okapi" if (providers.gradleProperty("enableMutationTesting").orNull?.toBoolean() == true) { - apply(plugin = "info.solidsoft.pitest") + apply(plugin = libs.plugins.pitest.get().pluginId) configure { - pitestVersion.set("1.17.0") - junit5PluginVersion.set("1.2.1") + pitestVersion.set(libs.versions.pitestTool.get()) + junit5PluginVersion.set(libs.versions.pitestJunit5Plugin.get()) targetClasses.set( listOf( "com.softwaremill.okapi.springboot.OutboxAutoConfiguration*", diff --git a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt index 32fba80..c9799e6 100644 --- a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt +++ b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt @@ -31,6 +31,8 @@ import org.springframework.transaction.PlatformTransactionManager import org.springframework.transaction.support.ResourceTransactionManager import org.springframework.transaction.support.TransactionTemplate import java.time.Clock +import java.util.Collections +import java.util.IdentityHashMap import javax.sql.DataSource /** @@ -99,6 +101,13 @@ class OutboxAutoConfiguration( // PTM and run validatePtmDataSourceMatch on it — so the user's TX semantics are honoured AND the // multi-DS safety net stays armed. See TransactionTemplateHijackProofTest for the empirical proof // that without `validatePtmDataSourceMatch` here, Boot's auto-TT silently bypasses safety. + // Return type is the interface `TransactionRunner`, NOT the concrete `SpringTransactionRunner`, + // deliberately deviating from the project convention of "@Bean methods return concrete types". + // Reason: `@ConditionalOnMissingBean` (no explicit type) matches against the declared @Bean + // return type — declaring `TransactionRunner` lets a user-supplied @Bean of ANY TransactionRunner + // implementation (not just SpringTransactionRunner) suppress this factory, which is the intended + // contract. Declaring the concrete type would only suppress on `SpringTransactionRunner` collisions + // and produce a duplicate runner for custom impls. @Bean @ConditionalOnMissingBean @ConditionalOnExpression("\${okapi.processor.enabled:true} or \${okapi.purger.enabled:true}") @@ -115,7 +124,7 @@ class OutboxAutoConfiguration( // an unreachable-via-Spring defensive guard, kept correct for any non-Spring caller. val ptm = anyTemplate?.transactionManager ?: resolvePlatformTransactionManager(transactionManager, beanFactory, okapiProperties) - validatePtmDataSourceMatch(ptm, resolveDataSource(), okapiProperties) + validatePtmDataSourceMatch(ptm, resolveDataSource(), okapiProperties, dataSources.size) // Use the user-/Boot-supplied TT verbatim when present (preserves timeout, propagation, // isolation set by whoever defined it). Otherwise build a minimal TT around the validated // PTM. Sets the *initial* TX read-only flag to false. NOTE: with PROPAGATION_REQUIRED @@ -303,15 +312,17 @@ class OutboxAutoConfiguration( ptm: PlatformTransactionManager, outboxDataSource: DataSource, properties: OkapiProperties, + dataSourceBeanCount: Int, ) { // Tries three extraction strategies in order: // 1. ResourceTransactionManager whose resourceFactory is a JDBC DataSource (DST + custom) // 2. JpaTransactionManager.getDataSource() — public getter, autodetected from EntityManagerFactory // 3. HibernateTransactionManager.getDataSource() — same shape (both pre-/post-6.2 packages) // Non-extractable cases (JTA, Exposed SpringTransactionManager, JPA/Hibernate with no JDBC DS) - // fall through to the WARN/INFO path. The WrongPtmDataSourceAmplificationProofTest empirically - // shows that wrong-DS bracketing collapses FOR UPDATE SKIP LOCKED to JDBC auto-commit and - // permits 100% delivery amplification — fail-fast when we can verify, log loudly when we cannot. + // fall through to the multi-DS ambiguity guard below: if there are ≥2 DataSource beans and + // no qualifier disambiguation, refuse to start (a wrong-DS PTM would otherwise collapse + // FOR UPDATE SKIP LOCKED to JDBC auto-commit and silently amplify delivery). Single-DS + // contexts stay on the existing INFO/WARN path because the binding is unambiguous. val ptmDataSource = extractDataSource(ptm) if (ptmDataSource != null) { // Spring's recommended pattern wraps the outbox DataSource bean in TransactionAwareDataSourceProxy @@ -321,39 +332,63 @@ class OutboxAutoConfiguration( // DelegatingDataSource chain on both sides before comparison. val unwrappedPtm = unwrapDataSource(ptmDataSource) val unwrappedOutbox = unwrapDataSource(outboxDataSource) - if (unwrappedPtm !== unwrappedOutbox) { - // If either side is still a DelegatingDataSource after unwrap, the chain terminated - // early — a cycle (`setTargetDataSource(self)`) or a not-yet-initialised - // `LazyConnectionDataSourceProxy.targetDataSource == null`. In that case the `!==` - // is inconclusive: we never reached the concrete backing DataSources, so we do NOT - // know whether they actually differ. Throwing the PTM↔DS mismatch error here would - // assert something we did not verify and send the operator chasing the wrong fix - // (qualifier config) instead of the real one (proxy wiring). Raise a distinct error. - if (unwrappedPtm is DelegatingDataSource || unwrappedOutbox is DelegatingDataSource) { + when { + unwrappedPtm is Unwrapped.Unresolvable || unwrappedOutbox is Unwrapped.Unresolvable -> { + // Either chain stopped before reaching a concrete backing DataSource — identity + // comparison would assert something we did not verify. Fail with a distinct error + // that names the side(s) and reason, instead of mis-attributing this as a mismatch. + val ptmSide = describeUnwrap("PTM", ptmDataSource, unwrappedPtm) + val outboxSide = describeUnwrap("outbox", outboxDataSource, unwrappedOutbox) error( "Could not verify the PTM↔DataSource binding: a DelegatingDataSource chain " + - "terminated early — a cycle (setTargetDataSource(self)) or an " + - "uninitialised LazyConnectionDataSourceProxy (targetDataSource not yet " + - "set). PTM side: $ptmDataSource (stopped at $unwrappedPtm). Outbox side: " + - "$outboxDataSource (stopped at $unwrappedOutbox). This is NOT necessarily " + - "a DataSource mismatch — fix the proxy wiring so both chains resolve to a " + - "concrete DataSource, or define an explicit @Bean TransactionRunner.", + "terminated before reaching a concrete backing DataSource. $ptmSide; $outboxSide. " + + "Fix the proxy wiring (cycle in setTargetDataSource, or initialise the lazy " + + "proxy's targetDataSource before context refresh), or define an explicit " + + "@Bean TransactionRunner.", + ) + } + (unwrappedPtm as Unwrapped.Resolved).ds !== (unwrappedOutbox as Unwrapped.Resolved).ds -> { + error( + "PlatformTransactionManager '${ptm.javaClass.name}' is bound to a different DataSource than " + + "okapi's outbox DataSource. PTM DataSource: $ptmDataSource. Outbox DataSource: " + + "$outboxDataSource (resolved via okapi.datasource-qualifier=" + + "'${properties.datasourceQualifier ?: ""}'). Each scheduler tick would otherwise " + + "wrap a transaction on the wrong DataSource and FOR UPDATE SKIP LOCKED would collapse " + + "to JDBC auto-commit, allowing duplicate delivery. Fix: set okapi.transaction-manager-" + + "qualifier to point at the PTM that brackets the outbox DataSource, or define an explicit " + + "@Bean TransactionRunner.", ) } - error( - "PlatformTransactionManager '${ptm.javaClass.name}' is bound to a different DataSource than " + - "okapi's outbox DataSource. PTM DataSource: $ptmDataSource. Outbox DataSource: " + - "$outboxDataSource (resolved via okapi.datasource-qualifier=" + - "'${properties.datasourceQualifier ?: ""}'). Each scheduler tick would otherwise " + - "wrap a transaction on the wrong DataSource and FOR UPDATE SKIP LOCKED would collapse " + - "to JDBC auto-commit, allowing duplicate delivery. Fix: set okapi.transaction-manager-" + - "qualifier to point at the PTM that brackets the outbox DataSource, or define an explicit " + - "@Bean TransactionRunner.", - ) } return } val resourceFactoryDescription = describeUnextractable(ptm) + // Multi-DataSource ambiguity guard: when okapi cannot extract the PTM's DataSource AND + // the context has multiple DataSource beans AND the user has not disambiguated via either + // okapi.datasource-qualifier or okapi.transaction-manager-qualifier, refuse to start. + // Without this, a wrong-DS PTM passes validation silently, FOR UPDATE SKIP LOCKED collapses + // to JDBC auto-commit, and duplicate delivery follows. Single-DS contexts stay on the + // existing INFO/WARN path because the binding is unambiguous by construction. Users with a + // legitimate multi-DS + non-extractable PTM setup (e.g. JTA) opt out by either setting one + // of the qualifiers or supplying an explicit @Bean TransactionRunner (which bypasses this + // factory entirely via @ConditionalOnMissingBean). + if ( + dataSourceBeanCount >= 2 && + properties.datasourceQualifier == null && + properties.transactionManagerQualifier == null + ) { + error( + "Cannot verify the PTM↔DataSource binding for a non-extractable " + + "PlatformTransactionManager '${ptm.javaClass.name}' ($resourceFactoryDescription) " + + "in a multi-DataSource context ($dataSourceBeanCount DataSource beans found). " + + "Without okapi.transaction-manager-qualifier or okapi.datasource-qualifier set, " + + "okapi cannot determine which DataSource this PTM brackets — if it brackets the " + + "wrong one, FOR UPDATE SKIP LOCKED collapses to JDBC auto-commit and scheduler " + + "ticks silently allow duplicate delivery across processor instances. Fix: set " + + "okapi.transaction-manager-qualifier (and/or okapi.datasource-qualifier) to " + + "disambiguate, or define an explicit @Bean TransactionRunner.", + ) + } if (properties.datasourceQualifier != null) { logger.warn( "okapi.datasource-qualifier='{}' is set, but the resolved PlatformTransactionManager '{}' {} " + @@ -383,18 +418,47 @@ class OutboxAutoConfiguration( } } - // Iterative + visited-set: defends against self-referencing or cyclic DelegatingDataSource - // chains (Spring's `setTargetDataSource` is a public setter with no cycle check, so - // misconfiguration like `proxy.setTargetDataSource(proxy)` is legal API). A tailrec form - // would compile to an uninterruptible JVM goto loop and silently spin at startup. - internal fun unwrapDataSource(ds: DataSource): DataSource { - val seen = mutableSetOf() + /** + * Result of walking a [DelegatingDataSource] chain down to its concrete backing DataSource. + * Sealed so callers branch on three explicit cases (resolved-same, resolved-different, + * unresolvable) instead of disambiguating an overloaded `DataSource` return via heuristics. + */ + sealed interface Unwrapped { + /** Reached a concrete (non-[DelegatingDataSource]) DataSource. Identity comparison is meaningful. */ + data class Resolved(val ds: DataSource) : Unwrapped + + /** + * Unwrap stopped before reaching a concrete backing DataSource. Identity comparison would + * be inconclusive — callers must NOT treat this as a mismatch. + */ + data class Unresolvable(val stoppedAt: DataSource, val reason: Reason) : Unwrapped + + enum class Reason { CYCLE, NULL_TARGET } + } + + // Iterative + identity-based visited-set: defends against self-referencing or cyclic + // DelegatingDataSource chains (Spring's `setTargetDataSource` is a public setter with no + // cycle check; `proxy.setTargetDataSource(proxy)` is legal API). IdentityHashMap because + // cycle detection is about object-graph traversal (have we seen THIS proxy object?), not + // value equality — a custom DataSource overriding equals() to delegate to its target would + // otherwise trigger false early termination on a non-cyclic chain. A tailrec form would + // compile to an uninterruptible JVM goto loop and silently spin at startup. + internal fun unwrapDataSource(ds: DataSource): Unwrapped { + val seen: MutableSet = Collections.newSetFromMap(IdentityHashMap()) var current: DataSource = ds while (current is DelegatingDataSource) { - if (!seen.add(current)) return current - current = current.targetDataSource ?: return current + if (!seen.add(current)) return Unwrapped.Unresolvable(current, Unwrapped.Reason.CYCLE) + val target = current.targetDataSource + ?: return Unwrapped.Unresolvable(current, Unwrapped.Reason.NULL_TARGET) + current = target } - return current + return Unwrapped.Resolved(current) + } + + private fun describeUnwrap(side: String, original: DataSource, unwrapped: Unwrapped): String = when (unwrapped) { + is Unwrapped.Resolved -> "$side side: $original resolved to ${unwrapped.ds}" + is Unwrapped.Unresolvable -> + "$side side: $original stopped at ${unwrapped.stoppedAt} (${unwrapped.reason})" } // JPA/Hibernate PTMs that expose a `public DataSource getDataSource()` — reflection by name @@ -410,6 +474,19 @@ class OutboxAutoConfiguration( "org.springframework.orm.hibernate5.HibernateTransactionManager", ) + // Walk the superclass chain (not just `ptm.javaClass.name`) so user-defined subclasses + // of `JpaTransactionManager` / `HibernateTransactionManager` are also recognised. Reads + // `.name` / `.superclass` on the already-loaded hierarchy only — no `Class.forName`, so + // the "no hard spring-orm compile dependency" constraint is preserved. + private fun isJpaHibernatePtm(ptm: PlatformTransactionManager): Boolean { + var c: Class<*>? = ptm.javaClass + while (c != null) { + if (c.name in JPA_HIBERNATE_PTM_CLASSES) return true + c = c.superclass + } + return false + } + internal fun extractDataSource(ptm: PlatformTransactionManager): DataSource? { (ptm as? ResourceTransactionManager)?.resourceFactory?.let { rf -> if (rf is DataSource) return rf @@ -422,7 +499,7 @@ class OutboxAutoConfiguration( // callable, but unusable for outbox bracketing — surface the original cause. // IllegalAccessException / ClassCastException → incompatible Spring framework // version where the contract has changed; not safe to silently fall back. - if (ptm.javaClass.name in JPA_HIBERNATE_PTM_CLASSES) { + if (isJpaHibernatePtm(ptm)) { return try { ptm.javaClass.getMethod("getDataSource").invoke(ptm) as DataSource? } catch (_: NoSuchMethodException) { @@ -433,7 +510,7 @@ class OutboxAutoConfiguration( } private fun describeUnextractable(ptm: PlatformTransactionManager): String { - if (ptm.javaClass.name in JPA_HIBERNATE_PTM_CLASSES) { + if (isJpaHibernatePtm(ptm)) { return "is ${ptm.javaClass.name} but its getDataSource() returned null — the " + "EntityManagerFactory/SessionFactory was constructed without a JDBC DataSource " + "(typical for pure-JTA / JNDI-only setups). okapi cannot verify the binding" diff --git a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionRunner.kt b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionRunner.kt index 3890bae..4c75d5d 100644 --- a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionRunner.kt +++ b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionRunner.kt @@ -7,7 +7,10 @@ import org.springframework.transaction.support.TransactionTemplate * Spring implementation of [TransactionRunner] using [TransactionTemplate]. */ class SpringTransactionRunner( - val transactionTemplate: TransactionTemplate, + // Visible only to same-module tests asserting reference identity with a user-supplied TT — + // public would freeze "implementation via TransactionTemplate" into the library's published + // API. Cross-module tests (e.g. okapi-integration-tests) must verify behaviour, not internals. + internal val transactionTemplate: TransactionTemplate, ) : TransactionRunner { override fun runInTransaction(block: () -> T): T = transactionTemplate.execute { block() }!! } diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt index d2469b0..dc9229e 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt @@ -476,50 +476,60 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ // covers the single-level TransactionAwareDataSourceProxy case via real autoconfig wiring. // These tests pin the helper's contract for nested chains, null targets, and cycles. context("unwrapDataSource") { - test("returns the input unchanged when not a DelegatingDataSource") { + test("returns Resolved with the input itself when not a DelegatingDataSource") { val raw: DataSource = SimpleDriverDataSource() - OutboxAutoConfiguration.unwrapDataSource(raw) shouldBeSameInstanceAs raw + val result = OutboxAutoConfiguration.unwrapDataSource(raw) + result.shouldBeInstanceOf() + result.ds shouldBeSameInstanceAs raw } - test("unwraps a single-level TransactionAwareDataSourceProxy to its target") { + test("unwraps a single-level TransactionAwareDataSourceProxy to Resolved(raw)") { val raw: DataSource = SimpleDriverDataSource() val proxy: DataSource = TransactionAwareDataSourceProxy(raw) - OutboxAutoConfiguration.unwrapDataSource(proxy) shouldBeSameInstanceAs raw + val result = OutboxAutoConfiguration.unwrapDataSource(proxy) + result.shouldBeInstanceOf() + result.ds shouldBeSameInstanceAs raw } - test("unwraps a nested chain TADP -> LCDP -> raw down to the raw DataSource") { + test("unwraps a nested chain TADP -> LCDP -> raw down to Resolved(raw)") { val raw: DataSource = SimpleDriverDataSource() val nested: DataSource = TransactionAwareDataSourceProxy(LazyConnectionDataSourceProxy(raw)) - OutboxAutoConfiguration.unwrapDataSource(nested) shouldBeSameInstanceAs raw + val result = OutboxAutoConfiguration.unwrapDataSource(nested) + result.shouldBeInstanceOf() + result.ds shouldBeSameInstanceAs raw } - test("returns the proxy itself when DelegatingDataSource has null targetDataSource (graceful, no NPE)") { + test("returns Unresolvable(NULL_TARGET) when a DelegatingDataSource has no targetDataSource") { // LazyConnectionDataSourceProxy ships with a no-arg constructor that leaves targetDataSource null - // until setTargetDataSource is called. At validation time some users may have such partially- - // initialised proxies. We must not NPE; we return the proxy itself so the caller's reference - // comparison can proceed (and produce a clear "DataSource mismatch" error if applicable). + // until setTargetDataSource is called. The helper must surface this as an explicit Unresolvable + // outcome so callers do not mistake a not-yet-wired proxy for a mismatched DataSource. val proxy: DataSource = LazyConnectionDataSourceProxy() - OutboxAutoConfiguration.unwrapDataSource(proxy) shouldBeSameInstanceAs proxy + val result = OutboxAutoConfiguration.unwrapDataSource(proxy) + result.shouldBeInstanceOf() + result.stoppedAt shouldBeSameInstanceAs proxy + result.reason shouldBe OutboxAutoConfiguration.Companion.Unwrapped.Reason.NULL_TARGET } - test("does NOT hang on a self-referencing DelegatingDataSource (cycle detection)").config(timeout = 2.seconds) { + test("returns Unresolvable(CYCLE) on a self-referencing DelegatingDataSource").config(timeout = 2.seconds) { val cyclic = LazyConnectionDataSourceProxy() cyclic.setTargetDataSource(cyclic) - // After A3 fix: cycle detected, returns the cyclic proxy itself. - // Without A3 fix: tailrec compiles to goto → infinite CPU spin → test times out and fails. - OutboxAutoConfiguration.unwrapDataSource(cyclic) shouldBeSameInstanceAs cyclic + // Without the fix: tailrec compiles to goto → infinite CPU spin → test times out and fails. + val result = OutboxAutoConfiguration.unwrapDataSource(cyclic) + result.shouldBeInstanceOf() + result.stoppedAt shouldBeSameInstanceAs cyclic + result.reason shouldBe OutboxAutoConfiguration.Companion.Unwrapped.Reason.CYCLE } - test("does NOT hang on a longer cycle (A -> B -> A)").config(timeout = 2.seconds) { + test("returns Unresolvable(CYCLE) on a longer cycle (A -> B -> A)").config(timeout = 2.seconds) { val a = LazyConnectionDataSourceProxy() val b = LazyConnectionDataSourceProxy() a.setTargetDataSource(b) b.setTargetDataSource(a) - // Walk: a -> b -> a; on the second visit to a the set already contains it, so the - // helper returns the current node (a). Any deterministic non-hanging answer is - // acceptable — the contract is "don't loop forever", and the caller will fall back - // to the WARN-or-error path because the unwrapped value isn't a real backing DataSource. - OutboxAutoConfiguration.unwrapDataSource(a) shouldBeSameInstanceAs a + val result = OutboxAutoConfiguration.unwrapDataSource(a) + result.shouldBeInstanceOf() + // Walk: a -> b -> a; second visit to a triggers the visited-set hit. + result.stoppedAt shouldBeSameInstanceAs a + result.reason shouldBe OutboxAutoConfiguration.Companion.Unwrapped.Reason.CYCLE } } }) diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringObjectProviderSemanticsAssumptionsTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringObjectProviderSemanticsAssumptionsTest.kt index 244d1a3..1ae7524 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringObjectProviderSemanticsAssumptionsTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringObjectProviderSemanticsAssumptionsTest.kt @@ -30,10 +30,12 @@ import org.springframework.transaction.support.ResourceTransactionManager * candidates (which is why the autoconfig uses `getIfUnique()`, not `getIfAvailable()`). * 4. `DataSourceTransactionManager` IS-A `ResourceTransactionManager`, with * `resourceFactory == DataSource`. Our PTM↔DS validation depends on this cast. - * 5. PTMs that extend `AbstractPlatformTransactionManager` directly (e.g. Exposed - * `SpringTransactionManager`, `JpaTransactionManager`) do NOT implement - * `ResourceTransactionManager` — meaning we cannot extract their DataSource for validation + * 5. PTMs that do NOT implement `ResourceTransactionManager` (e.g. Exposed + * `SpringTransactionManager`) — okapi cannot extract their DataSource for validation * and must fall back to a WARN log + require explicit `okapi.transaction-manager-qualifier`. + * (`JpaTransactionManager` IS-A `ResourceTransactionManager`, but its `resourceFactory` + * is an `EntityManagerFactory` not a `DataSource`; that case is handled separately by the + * JPA reflection branch in `extractDataSource`.) */ class SpringObjectProviderSemanticsAssumptionsTest : FunSpec({ @@ -73,7 +75,7 @@ class SpringObjectProviderSemanticsAssumptionsTest : FunSpec({ (dst as ResourceTransactionManager).resourceFactory.shouldBeSameInstanceAs(ds) } - test("AbstractPlatformTransactionManager subclasses (e.g. Exposed bridge / JPA-style) do NOT implement ResourceTransactionManager") { + test("AbstractPlatformTransactionManager subclasses that do NOT implement ResourceTransactionManager (e.g. Exposed bridge)") { val tm: PlatformTransactionManager = DummyAbstractPtm() (tm is ResourceTransactionManager) shouldBe false } diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/TransactionTemplateHijackProofTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/TransactionTemplateHijackProofTest.kt index 4259087..d57d755 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/TransactionTemplateHijackProofTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/TransactionTemplateHijackProofTest.kt @@ -5,11 +5,11 @@ import com.softwaremill.okapi.core.OutboxStore import com.softwaremill.okapi.core.TransactionRunner import io.kotest.assertions.withClue import io.kotest.core.spec.style.FunSpec +import io.kotest.matchers.nulls.shouldBeNull import io.kotest.matchers.nulls.shouldNotBeNull -import io.kotest.matchers.shouldBe -import io.kotest.matchers.shouldNotBe import io.kotest.matchers.string.shouldContain import io.kotest.matchers.types.shouldBeInstanceOf +import io.kotest.matchers.types.shouldBeSameInstanceAs import org.springframework.boot.autoconfigure.AutoConfigurations import org.springframework.boot.test.context.runner.ApplicationContextRunner import org.springframework.jdbc.datasource.DataSourceTransactionManager @@ -19,26 +19,17 @@ import org.springframework.transaction.support.TransactionTemplate import javax.sql.DataSource /** - * Reliable proof tests against the ultrareview claim: - * "Spring Boot's auto-configured TransactionTemplate hijacks okapiTransactionRunner — the factory - * short-circuits to the Boot-supplied template and skips PTM↔DataSource validation entirely." + * Two behavioural contracts of `okapiTransactionRunner` when Spring Boot's + * `TransactionAutoConfiguration` has registered a `TransactionTemplate` bean for the single PTM + * in the context: * - * Three independent invariants pin the conclusion empirically (asserting only "startup failed" - * would be brittle — the context could fail for unrelated reasons): + * 1. ADOPTS: the factory uses Boot's `TransactionTemplate` verbatim (reference identity), so the + * user's / Boot's TX semantics — timeout, propagation, isolation — are preserved instead of + * being silently replaced by a fresh default `TransactionTemplate`. * - * 1. PRECONDITION: Spring Boot's TransactionAutoConfiguration actually creates a `TransactionTemplate` - * bean in slice tests. If false, the hijack scenario cannot occur in this test harness and the - * other two tests prove nothing — the suite fails loudly instead of silently passing. - * - * 2. INTROSPECTION: in a single-DS happy path, `okapiTransactionRunner` produces a - * `SpringTransactionRunner` whose `TransactionTemplate.transactionManager` is the user's PTM. - * Combined with #1 this proves: even when Boot's TT bean exists, the factory does NOT pick it. - * (If the factory short-circuited via Boot's TT, the embedded PTM identity would not match.) - * - * 3. MISMATCH FAIL-FAST: in a 2-DS scenario with PTM bound to the wrong DS, startup fails with - * a literal substring from `validatePtmDataSourceMatch`'s `error(...)` message that no other - * Spring component emits — passing this assertion proves our validation code path was reached, - * not just that something failed. + * 2. VALIDATES: even though Boot's TT is adopted, `validatePtmDataSourceMatch` still runs and + * fails the context refresh when the PTM is bound to a different DataSource than okapi's + * outbox DataSource. The presence of Boot's TT does NOT bypass the safety net. */ class TransactionTemplateHijackProofTest : FunSpec({ @@ -54,29 +45,12 @@ class TransactionTemplateHijackProofTest : FunSpec({ null } } ?: error( - "TransactionAutoConfiguration not on test classpath. Without it the entire hijack scenario " + - "cannot be reproduced — failing the suite loudly rather than silently passing. Check that " + - "spring-boot-transaction (4.x) or spring-boot-autoconfigure (3.x) is on testRuntimeClasspath.", + "TransactionAutoConfiguration not on test classpath. Without it neither test below can " + + "exercise the Boot-auto-TT scenario. Check that spring-boot-transaction (4.x) or " + + "spring-boot-autoconfigure (3.x) is on testRuntimeClasspath.", ) - test("PRECONDITION: Spring Boot's TransactionAutoConfiguration registers a TransactionTemplate bean") { - val ds: DataSource = SimpleDriverDataSource() - ApplicationContextRunner() - .withConfiguration(AutoConfigurations.of(txAutoConfigClass)) - .withBean(DataSource::class.java, { ds }) - .withBean(PlatformTransactionManager::class.java, { DataSourceTransactionManager(ds) }) - .run { ctx -> - withClue( - "If this fails, Spring Boot's TransactionTemplateConfiguration was not triggered in slice " + - "tests — the hijack tests below would silently pass without testing anything. Investigate " + - "before trusting test #2 / #3 results.", - ) { - ctx.getBean(TransactionTemplate::class.java).shouldNotBeNull() - } - } - } - - test("INTROSPECTION: with Boot's TT in context, okapiTransactionRunner builds a TT around OUR PTM") { + test("ADOPTS: okapiTransactionRunner reuses Spring Boot's auto-configured TransactionTemplate verbatim") { val ds: DataSource = SimpleDriverDataSource() val ourPtm: PlatformTransactionManager = DataSourceTransactionManager(ds) ApplicationContextRunner() @@ -86,19 +60,20 @@ class TransactionTemplateHijackProofTest : FunSpec({ .withBean(OutboxStore::class.java, { stubStore() }) .withBean(MessageDeliverer::class.java, { stubDeliverer() }) .run { ctx -> - ctx.startupFailure shouldBe null - val runner = ctx.getBean(TransactionRunner::class.java) - runner.shouldBeInstanceOf() - // The TT that okapiTransactionRunner wraps must be bound to OUR PTM — not Boot's - // auto-configured TT (which is the hijack failure mode). Reference identity, not - // equality, since each PTM is a distinct object. - withClue("okapiTransactionRunner is wrapping a TT whose internal PTM is NOT our ourPtm — hijack confirmed") { - runner.transactionTemplate.transactionManager shouldBe ourPtm + ctx.startupFailure.shouldBeNull() + val bootTt = ctx.getBean(TransactionTemplate::class.java).shouldNotBeNull() + val runner = ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() + withClue( + "okapiTransactionRunner must adopt Boot's auto-configured TransactionTemplate by reference " + + "(preserving user/Boot timeout/propagation/isolation), not wrap a fresh default TT around " + + "the PTM — that would silently discard those settings.", + ) { + runner.transactionTemplate.shouldBeSameInstanceAs(bootTt) } } } - test("MISMATCH FAIL-FAST: PTM bound to wrong DataSource triggers validatePtmDataSourceMatch error") { + test("VALIDATES: Boot's auto-TT does NOT bypass validatePtmDataSourceMatch (wrong-DS PTM still fails refresh)") { val dsA: DataSource = SimpleDriverDataSource() val dsB: DataSource = SimpleDriverDataSource() ApplicationContextRunner() @@ -114,9 +89,8 @@ class TransactionTemplateHijackProofTest : FunSpec({ .withBean(OutboxStore::class.java, { stubStore() }) .withBean(MessageDeliverer::class.java, { stubDeliverer() }) .run { ctx -> - val failure = ctx.startupFailure - failure shouldNotBe null - failure!!.stackTraceToString() shouldContain + ctx.startupFailure.shouldNotBeNull() + ctx.startupFailure!!.stackTraceToString() shouldContain "is bound to a different DataSource than okapi's outbox DataSource" } } From 3e9a78524c64adb0b9357f368d29d74267a7b124 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20Kobyli=C5=84ski?= Date: Wed, 20 May 2026 22:38:49 +0200 Subject: [PATCH 8/8] refactor(spring-boot): tighten multi-DS guard and trim review-era comments (KOJAK-67) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Multi-DS ambiguity guard now requires `okapi.transaction-manager-qualifier` to be set when the PTM is non-extractable AND >=2 DataSource beans exist; setting only `okapi.datasource-qualifier` is no longer sufficient (it picks the outbox DS but does not constrain which PTM brackets it — PTM-side ambiguity remains). New test in WrongPtmMultiDataSourceFailFastTest pins the strictness; the previous escape-hatch test still passes. - README and CHANGELOG updated to reflect the tighter contract. - Comment audit across the changed files: removed redundant comments that just restated code/annotations or referenced the deleted amplification-proof test; shortened verbose rationale blocks; preserved load-bearing WHY/gotcha context (Spring proxy idioms, IdentityHashMap rationale, JPA package-rename history, no-Class.forName constraint). Net: ~86 lines of comments removed across 11 files. --- CHANGELOG.md | 10 +- README.md | 2 +- .../ExposedSpringBridgeEndToEndTest.kt | 29 ++-- .../JpaTransactionManagerFailFastTest.kt | 9 +- ...TransactionManagerMatchedDataSourceTest.kt | 13 +- .../WrongPtmMultiDataSourceFailFastTest.kt | 51 ++++-- .../springboot/OutboxAutoConfiguration.kt | 146 ++++++------------ .../springboot/SpringTransactionRunner.kt | 5 +- ...xAutoConfigurationTransactionRunnerTest.kt | 58 ++----- ...gObjectProviderSemanticsAssumptionsTest.kt | 3 +- .../TransactionTemplateHijackProofTest.kt | 6 +- 11 files changed, 123 insertions(+), 209 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5418d65..2937e3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,10 +39,12 @@ Until `1.0.0`, breaking changes may appear in any release and are flagged with * Specifically: if `extractDataSource` cannot determine the PTM's DataSource (e.g. JTA, Exposed's `SpringTransactionManager`, or any PTM that exposes neither a `DataSource` resourceFactory nor a public `getDataSource()`), AND the context has ≥2 `DataSource` - beans, AND neither `okapi.transaction-manager-qualifier` nor - `okapi.datasource-qualifier` is set, the context refresh now fails with an actionable - message. Single-DataSource contexts and explicitly-qualified setups are unaffected. - Escape hatch: supply an explicit `@Bean TransactionRunner` to bypass validation. + beans, AND `okapi.transaction-manager-qualifier` is not set, the context refresh now + fails with an actionable message. `okapi.datasource-qualifier` alone is not + sufficient — it picks the outbox DataSource but does not constrain which PTM + brackets it. Single-DataSource contexts and setups that explicitly name the PTM + via `okapi.transaction-manager-qualifier` are unaffected. Escape hatch: supply an + explicit `@Bean TransactionRunner` to bypass validation. ([#49](https://github.com/softwaremill/okapi/pull/49)) ### Added diff --git a/README.md b/README.md index 34b5565..0756499 100644 --- a/README.md +++ b/README.md @@ -92,7 +92,7 @@ fun outboxTransactionRunner(): TransactionRunner = object : TransactionRunner { Without bracketing, `FOR UPDATE SKIP LOCKED` collapses to the single SELECT statement under JDBC auto-commit, which silently allows duplicate delivery across processor instances. This opt-in is intentionally manual to keep accidental misconfiguration out of multi-instance deployments. -**Multi-DataSource contexts.** If your application has multiple `DataSource` beans and uses a `PlatformTransactionManager` from which okapi cannot extract a `DataSource` (JTA, Exposed's `SpringTransactionManager`, JPA without a JDBC `DataSource`), the autoconfiguration refuses to start until you disambiguate explicitly — either by setting `okapi.transaction-manager-qualifier` (the bean name of the PTM that brackets the outbox `DataSource`) and/or `okapi.datasource-qualifier`, or by supplying your own `@Bean TransactionRunner`. Single-DataSource setups and PTMs whose DataSource can be introspected (`DataSourceTransactionManager`, `JpaTransactionManager`, `HibernateTransactionManager`) are unaffected. +**Multi-DataSource contexts.** If your application has multiple `DataSource` beans and uses a `PlatformTransactionManager` from which okapi cannot extract a `DataSource` (JTA, Exposed's `SpringTransactionManager`, JPA without a JDBC `DataSource`), the autoconfiguration refuses to start until you set `okapi.transaction-manager-qualifier` to the bean name of the PTM that brackets the outbox `DataSource`. `okapi.datasource-qualifier` alone is not sufficient: it picks the outbox `DataSource` but does not constrain which PTM brackets it. Alternative escape hatch: supply your own `@Bean TransactionRunner`. Single-DataSource setups and PTMs whose `DataSource` can be introspected (`DataSourceTransactionManager`, `JpaTransactionManager`, `HibernateTransactionManager`) are unaffected. **Constructing schedulers directly (non-autoconfig usage).** When wiring `OutboxProcessorScheduler` / `OutboxPurgerScheduler` manually (Ktor, custom Spring contexts without autoconfig, etc.), supply a `TransactionRunner` explicitly — the parameter is required, with no default: diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/ExposedSpringBridgeEndToEndTest.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/ExposedSpringBridgeEndToEndTest.kt index aa8d48f..c022721 100644 --- a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/ExposedSpringBridgeEndToEndTest.kt +++ b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/ExposedSpringBridgeEndToEndTest.kt @@ -35,16 +35,12 @@ import javax.sql.DataSource * non-`DataSourceTransactionManager` `PlatformTransactionManager` — specifically Exposed's * `SpringTransactionManager` bridge. If the autoconfig assumed DST, this test would fail. * - * Two assertions: + * Two structural assertions: * 1. With processor disabled: a single Spring TX wrapping `springOutboxPublisher.publish()` - * borrows exactly one physical connection from the pool. This proves the autoconfig-built - * runner correctly bridges to `SpringConnectionProvider` through Spring's `ConnectionHolder`. - * 2. With processor enabled (200ms tick): an entry published inside a Spring TX is later - * delivered by the background scheduler — proving each scheduler tick is itself bracketed - * by a Spring TX driven by the bridged PTM. - * - * Liquibase schema migration is handled by `okapi-spring-boot` autoconfiguration; no manual - * setup needed. + * borrows exactly one physical connection — proving the autoconfig-built runner bridges to + * `SpringConnectionProvider` through Spring's `ConnectionHolder`. + * 2. With processor enabled: an entry published inside a Spring TX is later delivered by the + * background scheduler — proving each tick is bracketed by the bridged PTM. */ class ExposedSpringBridgeEndToEndTest : FunSpec({ @@ -127,18 +123,15 @@ class ExposedSpringBridgeEndToEndTest : FunSpec({ } } - // The single-process happy-path tests above would silently pass even if the autoconfig had - // re-introduced the auto-commit fallback (the bug KOJAK-67 fixes). This test exercises - // contention: 5 concurrent processor invocations against 50 published entries, each tick - // bracketed by the autoconfig-built TransactionRunner. With proper TX bracketing the - // FOR UPDATE SKIP LOCKED rows stay locked across claim+update — no amplification. With a - // no-op TR (or auto-commit fallback) the lock is released between claim and update, - // multiple processors deliver the same entry, and `assertNoAmplification` throws. + // Happy-path tests above would silently pass even if the autoconfig had re-introduced an + // auto-commit fallback. This test exercises contention: 5 concurrent processor invocations + // against 50 published entries. With proper TX bracketing, FOR UPDATE SKIP LOCKED holds + // across claim+update — no amplification. With a no-op TR the lock releases between claim + // and update, multiple processors deliver the same entry, and `assertNoAmplification` throws. test("autoconfig-built TransactionRunner prevents delivery amplification under concurrent processor invocations") { val recorder = RecordingMessageDeliverer() runner(recorder) - // Disable processor only — purger stays at its default 1h interval (won't fire in test) - // but keeps `okapiTransactionRunner` factory active. + // Processor disabled; purger stays at its default 1h interval, keeping the factory active. .withPropertyValues("okapi.processor.enabled=false") .run { ctx -> resetCounterAndTruncate(counter) diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerFailFastTest.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerFailFastTest.kt index 39ffaaa..d618ce5 100644 --- a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerFailFastTest.kt +++ b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerFailFastTest.kt @@ -23,12 +23,9 @@ import org.testcontainers.containers.PostgreSQLContainer import javax.sql.DataSource /** - * Proves the JPA branch of `extractDataSource` (`OutboxAutoConfiguration.kt`): a - * `JpaTransactionManager` whose auto-detected DataSource differs from okapi's outbox DataSource - * triggers `validatePtmDataSourceMatch` fail-fast at startup. Companion to - * `WrongPtmMultiDataSourceFailFastTest`, which fails-fast on the remaining ambiguity for PTMs - * that expose no DataSource at all (Exposed bridge, JTA) combined with multiple `DataSource` - * beans and no qualifier. + * Proves the JPA branch of `extractDataSource`: a `JpaTransactionManager` whose auto-detected + * DataSource differs from okapi's outbox DataSource triggers `validatePtmDataSourceMatch` + * fail-fast at startup. Companion: `WrongPtmMultiDataSourceFailFastTest` (non-extractable PTMs). */ class JpaTransactionManagerFailFastTest : FunSpec({ diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerMatchedDataSourceTest.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerMatchedDataSourceTest.kt index 0580915..9100229 100644 --- a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerMatchedDataSourceTest.kt +++ b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/JpaTransactionManagerMatchedDataSourceTest.kt @@ -27,12 +27,7 @@ import javax.sql.DataSource /** * Companion to `JpaTransactionManagerFailFastTest`: proves the MATCH branch of the JPA path. * When the `JpaTransactionManager`'s auto-detected DataSource equals okapi's outbox DataSource, - * `validatePtmDataSourceMatch` succeeds, the context starts, and `okapiTransactionRunner` is wired - * to a `TransactionTemplate` bound to that PTM. - * - * Without this test the fail-fast version above could theoretically pass even if a regression - * routed JPA through a wrong branch and made every JPA setup fail — match coverage is the - * other half of the proof. + * `validatePtmDataSourceMatch` succeeds and `okapiTransactionRunner` is wired to that PTM. */ class JpaTransactionManagerMatchedDataSourceTest : FunSpec({ @@ -72,12 +67,6 @@ class JpaTransactionManagerMatchedDataSourceTest : FunSpec({ }) .withPropertyValues("okapi.liquibase.enabled=false") .run { ctx -> - // Behaviour-level proof (does not peek at SpringTransactionRunner internals): - // - `startupFailure` is null → validatePtmDataSourceMatch took the match-return path - // (the fail-fast companion test JpaTransactionManagerFailFastTest pins what failure - // on mismatch looks like; together they cover both branches of the JPA path). - // - The TransactionRunner is `SpringTransactionRunner`, confirming the autoconfig - // wired the JPA PTM rather than skipping the factory. ctx.startupFailure.shouldBeNull() ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() } diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/WrongPtmMultiDataSourceFailFastTest.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/WrongPtmMultiDataSourceFailFastTest.kt index 3bc3390..5de957c 100644 --- a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/WrongPtmMultiDataSourceFailFastTest.kt +++ b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/transaction/WrongPtmMultiDataSourceFailFastTest.kt @@ -18,15 +18,9 @@ import org.springframework.transaction.PlatformTransactionManager import javax.sql.DataSource /** - * Replaces the former `WrongPtmDataSourceAmplificationProofTest`, which asserted a positive race - * outcome (`deliveries.filter { size > 1 }.shouldNotBeEmpty()`) — that assertion was both flaky - * (race-dependent) and semantically inverted (it would fail once the underlying risk was actually - * mitigated). The correct fix lives in production code: a non-extractable PTM combined with - * multiple `DataSource` beans and no `okapi.transaction-manager-qualifier` / `datasource-qualifier` - * is now refused at startup, eliminating the silent-duplicate residual risk rather than - * documenting it via a race. - * - * Pins the new behaviour deterministically: context refresh must fail with the new error message. + * A non-extractable PTM (Exposed bridge, JTA) combined with multiple `DataSource` beans and no + * `okapi.transaction-manager-qualifier` is refused at startup — okapi cannot prove which DS the + * PTM brackets, so it fails fast rather than risk silent duplicate delivery. */ class WrongPtmMultiDataSourceFailFastTest : FunSpec({ @@ -38,14 +32,9 @@ class WrongPtmMultiDataSourceFailFastTest : FunSpec({ .withConfiguration( AutoConfigurations.of(OutboxAutoConfiguration::class.java, OkapiLiquibaseAutoConfiguration::class.java), ) - // DS-B as @Primary so resolveDataSource() picks it as the outbox DS; DS-A is the - // ambiguous second DataSource that disambiguation would otherwise resolve. + // DS-B as @Primary so resolveDataSource() picks it as the outbox DS. .withBean("dsB", DataSource::class.java, { dsB }, BeanDefinitionCustomizer { it.isPrimary = true }) .withBean("dsA", DataSource::class.java, { dsA }) - // Exposed's SpringTransactionManager is non-extractable (no DataSource resourceFactory, - // no public getDataSource()) — same shape as JtaTransactionManager. With ≥2 DataSource - // beans and no qualifier set, okapi cannot prove which DS this PTM brackets and refuses - // to start rather than risk silent duplicate delivery. .withBean("exposedTmA", PlatformTransactionManager::class.java, { SpringTransactionManager(dsA) }) .withBean(MessageDeliverer::class.java, { RecordingMessageDeliverer() }) .withBean(PostgresOutboxStore::class.java, { @@ -59,6 +48,35 @@ class WrongPtmMultiDataSourceFailFastTest : FunSpec({ } } + test("okapi.datasource-qualifier alone is NOT sufficient — fail-fast still fires (PTM-side ambiguity remains)") { + val dsA: DataSource = SimpleDriverDataSource() + val dsB: DataSource = SimpleDriverDataSource() + + ApplicationContextRunner() + .withConfiguration( + AutoConfigurations.of(OutboxAutoConfiguration::class.java, OkapiLiquibaseAutoConfiguration::class.java), + ) + .withBean("dsB", DataSource::class.java, { dsB }, BeanDefinitionCustomizer { it.isPrimary = true }) + .withBean("dsA", DataSource::class.java, { dsA }) + .withBean("exposedTmA", PlatformTransactionManager::class.java, { SpringTransactionManager(dsA) }) + .withBean(MessageDeliverer::class.java, { RecordingMessageDeliverer() }) + .withBean(PostgresOutboxStore::class.java, { + PostgresOutboxStore(SpringConnectionProvider(dsB), java.time.Clock.systemUTC()) + }) + .withPropertyValues( + "okapi.processor.enabled=false", + "okapi.liquibase.enabled=false", + // User named the outbox DataSource but NOT the PTM. The PTM-side ambiguity remains, + // so fail-fast must still fire — datasource-qualifier alone does not grant immunity. + "okapi.datasource-qualifier=dsB", + ) + .run { ctx -> + ctx.startupFailure.shouldNotBeNull() + ctx.startupFailure!!.stackTraceToString() shouldContain + "Cannot verify the PTM↔DataSource binding for a non-extractable" + } + } + test("setting okapi.transaction-manager-qualifier lets the same setup start cleanly (escape hatch)") { val dsA: DataSource = SimpleDriverDataSource() val dsB: DataSource = SimpleDriverDataSource() @@ -80,8 +98,7 @@ class WrongPtmMultiDataSourceFailFastTest : FunSpec({ "okapi.transaction-manager-qualifier=exposedTmA", ) .run { ctx -> - // No startup failure: user has explicitly taken responsibility for the binding by - // naming the PTM. The autoconfig still cannot verify, but no longer refuses. + // Qualifier names the PTM explicitly; autoconfig trusts the user and no longer refuses. ctx.startupFailure?.let { error("Expected clean startup, got: ${it.message}") } } } diff --git a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt index c9799e6..b085286 100644 --- a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt +++ b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt @@ -90,24 +90,17 @@ class OutboxAutoConfiguration( SpringOutboxPublisher(delegate = outboxPublisher, dataSource = resolveDataSource()) // Created only when at least one scheduler is enabled — TransactionRunner has no other consumer. - // Skipping the bean in publish-only deployments (both schedulers disabled) lets users run without - // a PlatformTransactionManager on the classpath at all (e.g. message producer that delegates - // outbox processing to a separate worker). + // Skipping the bean in publish-only deployments (both schedulers disabled) lets users omit a + // PlatformTransactionManager entirely (e.g. a pure producer delegating processing to a worker). // - // ObjectProvider design: a TT in the context can come from two sources — - // a user-defined @Bean (with custom timeout/propagation/isolation), OR Spring Boot's - // TransactionAutoConfiguration which auto-registers a TT whenever a single PTM exists. The two - // are indistinguishable at this layer. We accept BOTH transparently but always extract the bound - // PTM and run validatePtmDataSourceMatch on it — so the user's TX semantics are honoured AND the - // multi-DS safety net stays armed. See TransactionTemplateHijackProofTest for the empirical proof - // that without `validatePtmDataSourceMatch` here, Boot's auto-TT silently bypasses safety. - // Return type is the interface `TransactionRunner`, NOT the concrete `SpringTransactionRunner`, - // deliberately deviating from the project convention of "@Bean methods return concrete types". - // Reason: `@ConditionalOnMissingBean` (no explicit type) matches against the declared @Bean - // return type — declaring `TransactionRunner` lets a user-supplied @Bean of ANY TransactionRunner - // implementation (not just SpringTransactionRunner) suppress this factory, which is the intended - // contract. Declaring the concrete type would only suppress on `SpringTransactionRunner` collisions - // and produce a duplicate runner for custom impls. + // We accept a user-defined TransactionTemplate OR Boot's auto-registered one transparently, but + // always extract the PTM and run validatePtmDataSourceMatch — so the user's TX semantics are + // honoured AND the multi-DS safety net stays armed (see TransactionTemplateHijackProofTest). + // + // Return type is `TransactionRunner` (interface), not `SpringTransactionRunner` (concrete) — + // deliberate deviation from the "@Bean returns concrete type" convention. @ConditionalOnMissingBean + // matches against the declared return type, so declaring the interface lets any user-supplied + // TransactionRunner impl suppress this factory; the concrete type would miss custom impls. @Bean @ConditionalOnMissingBean @ConditionalOnExpression("\${okapi.processor.enabled:true} or \${okapi.purger.enabled:true}") @@ -117,22 +110,16 @@ class OutboxAutoConfiguration( beanFactory: BeanFactory, ): TransactionRunner { val anyTemplate = transactionTemplate.getIfUnique() - // Extract the PTM from the TT if available, else resolve through the provider. TT's - // transactionManager is nullable in the API, but Spring's InitializingBean contract rejects - // a TransactionTemplate bean with a null transactionManager at afterPropertiesSet() (proven - // by the PROBE test in OutboxAutoConfigurationTransactionRunnerTest). The `?:` is therefore - // an unreachable-via-Spring defensive guard, kept correct for any non-Spring caller. + // TT.transactionManager is nullable in the API, but Spring rejects a null one at + // afterPropertiesSet() (see OutboxAutoConfigurationTransactionRunnerTest PROBE test) — the + // `?:` is a defensive guard for non-Spring callers only. val ptm = anyTemplate?.transactionManager ?: resolvePlatformTransactionManager(transactionManager, beanFactory, okapiProperties) validatePtmDataSourceMatch(ptm, resolveDataSource(), okapiProperties, dataSources.size) - // Use the user-/Boot-supplied TT verbatim when present (preserves timeout, propagation, - // isolation set by whoever defined it). Otherwise build a minimal TT around the validated - // PTM. Sets the *initial* TX read-only flag to false. NOTE: with PROPAGATION_REQUIRED - // (the default), a tick that joins an outer @Transactional(readOnly = true) inherits the - // outer's flag and this setting is silently ignored. The scheduler runs on a daemon - // thread with no outer TX, so this flag actually takes effect — but invocations from - // inside an existing read-only TX would still hit FOR UPDATE failures. Keep scheduler - // invocations outside @Transactional scopes. + // Use the supplied TT verbatim when present (preserves timeout/propagation/isolation). + // Otherwise build a minimal TT with isReadOnly=false. NOTE: with PROPAGATION_REQUIRED a + // tick joining an outer readOnly=true TX inherits that flag silently — keep scheduler + // invocations outside @Transactional scopes to avoid FOR UPDATE SKIP LOCKED failures. return SpringTransactionRunner( anyTemplate ?: TransactionTemplate(ptm).apply { isReadOnly = false }, ) @@ -208,10 +195,6 @@ class OutboxAutoConfiguration( ) } - /** - * Auto-configures [PostgresOutboxStore] when `okapi-postgres` is on the classpath. - * Skipped if the application provides its own [OutboxStore] bean. - */ @Configuration(proxyBeanMethods = false) @ConditionalOnClass(PostgresOutboxStore::class) class PostgresStoreConfiguration( @@ -314,29 +297,20 @@ class OutboxAutoConfiguration( properties: OkapiProperties, dataSourceBeanCount: Int, ) { - // Tries three extraction strategies in order: - // 1. ResourceTransactionManager whose resourceFactory is a JDBC DataSource (DST + custom) - // 2. JpaTransactionManager.getDataSource() — public getter, autodetected from EntityManagerFactory - // 3. HibernateTransactionManager.getDataSource() — same shape (both pre-/post-6.2 packages) - // Non-extractable cases (JTA, Exposed SpringTransactionManager, JPA/Hibernate with no JDBC DS) - // fall through to the multi-DS ambiguity guard below: if there are ≥2 DataSource beans and - // no qualifier disambiguation, refuse to start (a wrong-DS PTM would otherwise collapse - // FOR UPDATE SKIP LOCKED to JDBC auto-commit and silently amplify delivery). Single-DS - // contexts stay on the existing INFO/WARN path because the binding is unambiguous. + // extractDataSource tries ResourceTransactionManager, then JpaTransactionManager / + // HibernateTransactionManager via reflection. Non-extractable PTMs (JTA, Exposed + // SpringTransactionManager, JPA without a JDBC DS) fall through to the multi-DS + // ambiguity guard: ≥2 DataSource beans + no qualifier → refuse to start. val ptmDataSource = extractDataSource(ptm) if (ptmDataSource != null) { - // Spring's recommended pattern wraps the outbox DataSource bean in TransactionAwareDataSourceProxy - // (or LazyConnectionDataSourceProxy) for use by query helpers, while passing the raw DataSource - // to the PTM (Spring docs explicitly say "TransactionAwareDataSourceProxy should NOT be passed - // to a PTM"). Reference equality on those references would falsely fail. Unwrap the - // DelegatingDataSource chain on both sides before comparison. + // Spring docs say TransactionAwareDataSourceProxy must NOT be passed to a PTM, so + // the PTM holds the raw DS while helpers hold the proxy. Unwrap both chains before + // identity comparison to avoid false mismatch on wrapper/raw pairs. val unwrappedPtm = unwrapDataSource(ptmDataSource) val unwrappedOutbox = unwrapDataSource(outboxDataSource) when { unwrappedPtm is Unwrapped.Unresolvable || unwrappedOutbox is Unwrapped.Unresolvable -> { - // Either chain stopped before reaching a concrete backing DataSource — identity - // comparison would assert something we did not verify. Fail with a distinct error - // that names the side(s) and reason, instead of mis-attributing this as a mismatch. + // Chain didn't reach a concrete DS — report as wiring error, not a mismatch. val ptmSide = describeUnwrap("PTM", ptmDataSource, unwrappedPtm) val outboxSide = describeUnwrap("outbox", outboxDataSource, unwrappedOutbox) error( @@ -363,30 +337,24 @@ class OutboxAutoConfiguration( return } val resourceFactoryDescription = describeUnextractable(ptm) - // Multi-DataSource ambiguity guard: when okapi cannot extract the PTM's DataSource AND - // the context has multiple DataSource beans AND the user has not disambiguated via either - // okapi.datasource-qualifier or okapi.transaction-manager-qualifier, refuse to start. - // Without this, a wrong-DS PTM passes validation silently, FOR UPDATE SKIP LOCKED collapses - // to JDBC auto-commit, and duplicate delivery follows. Single-DS contexts stay on the - // existing INFO/WARN path because the binding is unambiguous by construction. Users with a - // legitimate multi-DS + non-extractable PTM setup (e.g. JTA) opt out by either setting one - // of the qualifiers or supplying an explicit @Bean TransactionRunner (which bypasses this - // factory entirely via @ConditionalOnMissingBean). - if ( - dataSourceBeanCount >= 2 && - properties.datasourceQualifier == null && - properties.transactionManagerQualifier == null - ) { + // Multi-DS ambiguity guard: if okapi cannot extract the PTM's DataSource AND there are + // ≥2 DataSource beans AND no transaction-manager-qualifier was set, refuse to start. + // okapi.datasource-qualifier alone is insufficient — it picks the outbox DS but does + // NOT constrain which PTM brackets it. A wrong-DS PTM would collapse FOR UPDATE SKIP + // LOCKED to JDBC auto-commit and silently permit duplicate delivery. Opt out by setting + // okapi.transaction-manager-qualifier or supplying an explicit @Bean TransactionRunner. + if (dataSourceBeanCount >= 2 && properties.transactionManagerQualifier == null) { error( "Cannot verify the PTM↔DataSource binding for a non-extractable " + "PlatformTransactionManager '${ptm.javaClass.name}' ($resourceFactoryDescription) " + "in a multi-DataSource context ($dataSourceBeanCount DataSource beans found). " + - "Without okapi.transaction-manager-qualifier or okapi.datasource-qualifier set, " + - "okapi cannot determine which DataSource this PTM brackets — if it brackets the " + - "wrong one, FOR UPDATE SKIP LOCKED collapses to JDBC auto-commit and scheduler " + - "ticks silently allow duplicate delivery across processor instances. Fix: set " + - "okapi.transaction-manager-qualifier (and/or okapi.datasource-qualifier) to " + - "disambiguate, or define an explicit @Bean TransactionRunner.", + "okapi.transaction-manager-qualifier is not set, so okapi cannot determine which " + + "DataSource this PTM brackets — if it brackets the wrong one, FOR UPDATE SKIP " + + "LOCKED collapses to JDBC auto-commit and scheduler ticks silently allow " + + "duplicate delivery across processor instances. (okapi.datasource-qualifier alone " + + "is not sufficient: it picks the outbox DataSource but does not constrain which " + + "PTM brackets it.) Fix: set okapi.transaction-manager-qualifier to name the PTM " + + "that brackets the outbox DataSource, or define an explicit @Bean TransactionRunner.", ) } if (properties.datasourceQualifier != null) { @@ -402,10 +370,8 @@ class OutboxAutoConfiguration( resourceFactoryDescription, ) } else { - // No qualifier set → single-DS assumption. We cannot validate, but a future multi-DS - // migration that forgets to set the qualifier would silently break with no diagnostic. - // Emit an INFO breadcrumb so an operator debugging duplicate delivery has something - // to grep for. + // Single-DS assumption. Emit an INFO breadcrumb so a future multi-DS migration that + // forgets to set the qualifier produces something to grep for when debugging delivery. logger.info( "PlatformTransactionManager '{}' {} — okapi cannot verify it brackets the outbox " + "DataSource. Assuming single-DataSource setup (okapi.datasource-qualifier is " + @@ -418,13 +384,8 @@ class OutboxAutoConfiguration( } } - /** - * Result of walking a [DelegatingDataSource] chain down to its concrete backing DataSource. - * Sealed so callers branch on three explicit cases (resolved-same, resolved-different, - * unresolvable) instead of disambiguating an overloaded `DataSource` return via heuristics. - */ + /** Result of walking a [DelegatingDataSource] chain to its concrete backing DataSource. */ sealed interface Unwrapped { - /** Reached a concrete (non-[DelegatingDataSource]) DataSource. Identity comparison is meaningful. */ data class Resolved(val ds: DataSource) : Unwrapped /** @@ -436,13 +397,10 @@ class OutboxAutoConfiguration( enum class Reason { CYCLE, NULL_TARGET } } - // Iterative + identity-based visited-set: defends against self-referencing or cyclic - // DelegatingDataSource chains (Spring's `setTargetDataSource` is a public setter with no - // cycle check; `proxy.setTargetDataSource(proxy)` is legal API). IdentityHashMap because - // cycle detection is about object-graph traversal (have we seen THIS proxy object?), not - // value equality — a custom DataSource overriding equals() to delegate to its target would - // otherwise trigger false early termination on a non-cyclic chain. A tailrec form would - // compile to an uninterruptible JVM goto loop and silently spin at startup. + // Iterative walk with an IdentityHashMap visited-set: guards against cyclic chains + // (Spring's setTargetDataSource has no cycle check). Identity, not equals(), because a + // custom DS overriding equals() to delegate to its target could trigger false early + // termination on a valid chain. internal fun unwrapDataSource(ds: DataSource): Unwrapped { val seen: MutableSet = Collections.newSetFromMap(IdentityHashMap()) var current: DataSource = ds @@ -491,14 +449,10 @@ class OutboxAutoConfiguration( (ptm as? ResourceTransactionManager)?.resourceFactory?.let { rf -> if (rf is DataSource) return rf } - // JPA/Hibernate public `getDataSource()`. Narrow catch on NoSuchMethodException only — - // all other exceptions intentionally propagate: - // LinkageError / NoClassDefFoundError → mixed-jar classpath bug, must surface (this - // is why we don't use `runCatching`, which would swallow them). - // InvocationTargetException → JpaTransactionManager constructed without an EMF; - // callable, but unusable for outbox bracketing — surface the original cause. - // IllegalAccessException / ClassCastException → incompatible Spring framework - // version where the contract has changed; not safe to silently fall back. + // Narrow catch on NoSuchMethodException only — all other exceptions propagate. + // Do NOT use runCatching: it swallows LinkageError/NoClassDefFoundError (mixed-jar + // classpath bug), InvocationTargetException (PTM built without an EMF), and + // IllegalAccessException/ClassCastException (incompatible Spring version). if (isJpaHibernatePtm(ptm)) { return try { ptm.javaClass.getMethod("getDataSource").invoke(ptm) as DataSource? diff --git a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionRunner.kt b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionRunner.kt index 4c75d5d..e0fa895 100644 --- a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionRunner.kt +++ b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionRunner.kt @@ -7,9 +7,8 @@ import org.springframework.transaction.support.TransactionTemplate * Spring implementation of [TransactionRunner] using [TransactionTemplate]. */ class SpringTransactionRunner( - // Visible only to same-module tests asserting reference identity with a user-supplied TT — - // public would freeze "implementation via TransactionTemplate" into the library's published - // API. Cross-module tests (e.g. okapi-integration-tests) must verify behaviour, not internals. + // internal (not public): same-module tests assert reference identity; publishing the field + // would freeze "implemented via TransactionTemplate" as part of the library's API contract. internal val transactionTemplate: TransactionTemplate, ) : TransactionRunner { override fun runInTransaction(block: () -> T): T = transactionTemplate.execute { block() }!! diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt index dc9229e..4255117 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt @@ -67,10 +67,7 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ } test("okapi.transaction-manager-qualifier YAML key binds to OkapiProperties.transactionManagerQualifier (kebab → camelCase)") { - // Pins the property name contract — without this a future rename of the Kotlin field - // (e.g. transactionManagerQualifier → txManagerQualifier) silently breaks user - // configuration. Mirror of LiquibaseAutoConfigurationTest's "okapi.liquibase.* properties - // bind to nested config" test. + // Pins the property name — a field rename silently breaks user config without this gate. baseRunner .withBean(DataSource::class.java, { SimpleDriverDataSource() }) .withBean(TransactionRunner::class.java, { @@ -85,11 +82,8 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ } test("blank okapi.transaction-manager-qualifier triggers startup failure with require() message") { - // Pins that init { require(isNotBlank()) } actually propagates through Spring's Binder. - // A future refactor of OkapiProperties that moves the require() outside init or onto a - // getter would silently let blank qualifiers through and cause a confusing - // NoSuchBeanDefinitionException at PTM lookup. Mirror of LiquibaseAutoConfigurationTest's - // "blank changelog-table property triggers startup failure". + // Pins that the require() in OkapiProperties.init propagates through Spring's Binder — + // moving it to a getter would silently let blank qualifiers through. baseRunner .withBean(DataSource::class.java, { SimpleDriverDataSource() }) .withBean(TransactionRunner::class.java, { @@ -106,10 +100,6 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ } test("publish-only deployment: both schedulers disabled + no PTM + no user TransactionRunner — context starts cleanly") { - // Publish-only mode (e.g. message-producing service that delegates outbox processing to a separate - // worker). The TransactionRunner is only needed by scheduler/purger; with both disabled, no PTM - // should be required. Without this gate, okapi-spring-boot would prevent users from running - // publish-only with arbitrary PTMs (or none at all). baseRunner .withBean(DataSource::class.java, { SimpleDriverDataSource() }) .withPropertyValues( @@ -174,10 +164,8 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ } test("user-provided @Bean TransactionTemplate is honoured (autoconfig does not silently shadow it)") { - // If a user defines a custom TransactionTemplate (e.g. with non-default timeout / propagation / - // isolation), the autoconfig MUST use that exact template — not silently build its own fresh - // TransactionTemplate(ptm) from the PTM. A "silent shadow" regression would discard the user's - // intent for scheduler ticks while their @Transactional code paths use the configured template. + // A "silent shadow" regression would discard the user's TX settings (timeout/propagation/isolation) + // for scheduler ticks while @Transactional code paths use the correctly configured template. val ds: DataSource = SimpleDriverDataSource() baseRunner .withBean(DataSource::class.java, { ds }) @@ -222,9 +210,6 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ .withBean(DataSource::class.java, { ds }) .withUserConfiguration(PrimaryDstAndDummyPtmConfig::class.java) .run { ctx -> - // PrimaryDstAndDummyPtmConfig.primaryTm is the @Primary DST → autoconfig should derive runner from it. - // If autoconfig (wrongly) picked the DummyTm, validation would not fire (DummyTm is not RTM) - // and the runner would still build — so we assert no startup failure as the strongest signal. ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() } } @@ -262,12 +247,8 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ } test("ResourceTransactionManager PTM with non-DataSource resourceFactory: WARN includes actual resourceFactory class name") { - // The BUG C1 regression guard pins the WARN-is-emitted contract for this case. - // This test pins the diagnostic CONTENT: the WARN must mention the actual resourceFactory - // class so a user with a custom RTM (or buggy subclass returning null) sees the real - // resource type, not just the static example list (JpaTransactionManager/Hibernate). Without - // this assertion a refactor that removed the dynamic class reference from the WARN would - // silently downgrade the diagnostic for non-JPA users. + // Pins WARN content: must mention the actual resourceFactory class, not just the static + // JpaTransactionManager/Hibernate examples — so non-JPA users see the real resource type. val ds: DataSource = SimpleDriverDataSource() val customRtm = JpaLikeRtmTransactionManager(resourceFactory = "myDistinctiveResourceFactory") val targetLogger = LoggerFactory.getLogger(OutboxAutoConfiguration::class.java) as Logger @@ -280,8 +261,6 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ .withPropertyValues("okapi.datasource-qualifier=outboxDs") .run { ctx -> ctx.startupFailure.shouldBeNull() } val warnText = appender.list.filter { it.level == Level.WARN }.joinToString("\n") { it.formattedMessage } - // resourceFactory is a String — WARN must mention its class so user sees what was actually - // exposed, not just the JpaTransactionManager/Hibernate examples. warnText.shouldContain("java.lang.String") } finally { targetLogger.detachAppender(appender) @@ -289,10 +268,8 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ } test("non-ResourceTransactionManager PTM without okapi.datasource-qualifier (single-DS assumption): INFO breadcrumb emitted") { - // When validation cannot run AND no qualifier is set, the autoconfig assumes single-DS — but - // a future multi-DS migration that forgets to set the qualifier would silently break with no - // diagnostic. An INFO breadcrumb at startup gives the operator something to grep for when - // duplicate delivery starts to occur post-migration. + // INFO breadcrumb gives operators something to grep for after a multi-DS migration that + // forgot to set okapi.datasource-qualifier — otherwise the misconfiguration is silent. val ds: DataSource = SimpleDriverDataSource() val targetLogger = LoggerFactory.getLogger(OutboxAutoConfiguration::class.java) as Logger val appender = ListAppender().apply { start() } @@ -313,12 +290,8 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ } test("non-ResourceTransactionManager PTM with okapi.datasource-qualifier set: context starts AND emits actionable WARN") { - // DummyTransactionManager extends AbstractPlatformTransactionManager but does NOT implement - // ResourceTransactionManager — same shape as Exposed's SpringTransactionManager. - // Validation cannot extract the PTM's DataSource, so it logs a WARN and allows the context to start. - // Captures the WARN content so deletion of the warn() body (or its message format) is also caught — - // not just deletion of the conditional that emits it. Without this assertion, the operator-facing - // breadcrumb could silently rot without breaking any other test. + // Asserts WARN content (not just presence) so the operator-facing message cannot silently + // rot — e.g. deleting the warn() body would still let the context start, breaking no other test. val ds: DataSource = SimpleDriverDataSource() val targetLogger = LoggerFactory.getLogger(OutboxAutoConfiguration::class.java) as Logger val appender = ListAppender().apply { start() } @@ -472,9 +445,7 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ } } - // Direct unit tests for the unwrapDataSource helper. The integration-level BUG C2 test above - // covers the single-level TransactionAwareDataSourceProxy case via real autoconfig wiring. - // These tests pin the helper's contract for nested chains, null targets, and cycles. + // Unit tests for the unwrapDataSource helper: nested chains, null targets, and cycles. context("unwrapDataSource") { test("returns Resolved with the input itself when not a DelegatingDataSource") { val raw: DataSource = SimpleDriverDataSource() @@ -513,7 +484,6 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ test("returns Unresolvable(CYCLE) on a self-referencing DelegatingDataSource").config(timeout = 2.seconds) { val cyclic = LazyConnectionDataSourceProxy() cyclic.setTargetDataSource(cyclic) - // Without the fix: tailrec compiles to goto → infinite CPU spin → test times out and fails. val result = OutboxAutoConfiguration.unwrapDataSource(cyclic) result.shouldBeInstanceOf() result.stoppedAt shouldBeSameInstanceAs cyclic @@ -527,7 +497,6 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({ b.setTargetDataSource(a) val result = OutboxAutoConfiguration.unwrapDataSource(a) result.shouldBeInstanceOf() - // Walk: a -> b -> a; second visit to a triggers the visited-set hit. result.stoppedAt shouldBeSameInstanceAs a result.reason shouldBe OutboxAutoConfiguration.Companion.Unwrapped.Reason.CYCLE } @@ -569,8 +538,7 @@ private class CustomTransactionTemplateConfiguration { } companion object { - // Distinctive timeout makes the template easily identifiable; the autoconfig must not silently - // build its own TransactionTemplate(ptm) bypassing this one. + // Distinctive timeout makes this template identifiable in reference-identity assertions. val TEMPLATE: org.springframework.transaction.support.TransactionTemplate = org.springframework.transaction.support.TransactionTemplate().apply { timeout = 42 diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringObjectProviderSemanticsAssumptionsTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringObjectProviderSemanticsAssumptionsTest.kt index 1ae7524..7c27d77 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringObjectProviderSemanticsAssumptionsTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringObjectProviderSemanticsAssumptionsTest.kt @@ -34,8 +34,7 @@ import org.springframework.transaction.support.ResourceTransactionManager * `SpringTransactionManager`) — okapi cannot extract their DataSource for validation * and must fall back to a WARN log + require explicit `okapi.transaction-manager-qualifier`. * (`JpaTransactionManager` IS-A `ResourceTransactionManager`, but its `resourceFactory` - * is an `EntityManagerFactory` not a `DataSource`; that case is handled separately by the - * JPA reflection branch in `extractDataSource`.) + * is an `EntityManagerFactory` not a `DataSource` — handled separately by `extractDataSource`.) */ class SpringObjectProviderSemanticsAssumptionsTest : FunSpec({ diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/TransactionTemplateHijackProofTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/TransactionTemplateHijackProofTest.kt index d57d755..9916edf 100644 --- a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/TransactionTemplateHijackProofTest.kt +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/TransactionTemplateHijackProofTest.kt @@ -63,11 +63,7 @@ class TransactionTemplateHijackProofTest : FunSpec({ ctx.startupFailure.shouldBeNull() val bootTt = ctx.getBean(TransactionTemplate::class.java).shouldNotBeNull() val runner = ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf() - withClue( - "okapiTransactionRunner must adopt Boot's auto-configured TransactionTemplate by reference " + - "(preserving user/Boot timeout/propagation/isolation), not wrap a fresh default TT around " + - "the PTM — that would silently discard those settings.", - ) { + withClue("okapiTransactionRunner must adopt Boot's TransactionTemplate by reference, not wrap a fresh default TT") { runner.transactionTemplate.shouldBeSameInstanceAs(bootTt) } }