fix(spring-config): unwrap DataSource at SpringTransactionContextValidator and use it at isInActiveReadWriteTransaction()

okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/DataSourceUnwrapping.kt

@@ -0,0 +1,52 @@
+package com.softwaremill.okapi.springboot
+
+import org.springframework.jdbc.datasource.DelegatingDataSource
+import java.util.Collections
+import java.util.IdentityHashMap
+import javax.sql.DataSource
+
+/**
+ * Result of walking a [DelegatingDataSource] chain (e.g. a `TransactionAwareDataSourceProxy`) to
+ * its concrete backing DataSource.
+ *
+ * Shared between [OutboxAutoConfiguration] (startup PTM<->DataSource binding check) and
+ * [SpringTransactionContextValidator] (runtime `publish()` check) so both agree on what "the same
+ * DataSource" means. Before this was shared (issue #91), the startup check unwrapped
+ * `DelegatingDataSource` chains before comparing identity but the runtime check compared the raw
+ * bean reference — a `TransactionAwareDataSourceProxy` (Spring's own documented pattern: proxy as
+ * the DataSource bean, PlatformTransactionManager on the raw target) passed startup validation and
+ * then made every `publish()` fail, since the PTM's bound resource lives under the raw target, not
+ * the proxy.
+ */
+internal sealed interface Unwrapped {
+    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 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<DataSource> = Collections.newSetFromMap(IdentityHashMap())
+    var current: DataSource = ds
+    while (current is DelegatingDataSource) {
+        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 Unwrapped.Resolved(current)
+}
+
+internal 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})"
+}

okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfiguration.kt

@@ -28,13 +28,10 @@ import org.springframework.boot.context.properties.EnableConfigurationProperties
 import org.springframework.context.annotation.Bean
 import org.springframework.context.annotation.Configuration
 import org.springframework.core.annotation.Order
-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 java.util.Collections
-import java.util.IdentityHashMap
 import javax.sql.DataSource
 
 /**
@@ -456,41 +453,6 @@ class OutboxAutoConfiguration(
             }
         }
 
-        /** Result of walking a [DelegatingDataSource] chain to its concrete backing DataSource. */
-        sealed interface Unwrapped {
-            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 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<DataSource> = Collections.newSetFromMap(IdentityHashMap())
-            var current: DataSource = ds
-            while (current is DelegatingDataSource) {
-                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 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
         // avoids requiring spring-orm on the compile classpath (it's optional for JDBC-only consumers).
         // Hibernate's TM is `org.springframework.orm.jpa.hibernate.HibernateTransactionManager` in

okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/SpringTransactionContextValidator.kt

@@ -24,11 +24,32 @@ import javax.sql.DataSource
  * check is the primary DataSource-specific guard.
  */
 internal class SpringTransactionContextValidator(
-    private val dataSource: DataSource,
+    dataSource: DataSource,
 ) : TransactionContextValidator {
+
+    // Spring's own guidance is that a TransactionAwareDataSourceProxy (or any DelegatingDataSource
+    // wrapper) must NOT be handed to a PlatformTransactionManager -- the PTM is always constructed
+    // with the raw target, so it binds its transactional resource under that raw reference, never
+    // under the wrapper. If [dataSource] is such a wrapper, resolving it here to the concrete
+    // backing DataSource makes getResource() below find what the PTM actually bound, matching what
+    // OutboxAutoConfiguration's startup PTM<->DataSource check already verifies (issue #91: the two
+    // checks previously disagreed -- startup unwrapped the chain, this runtime check compared the
+    // raw bean reference -- so a proxy that passed startup validation then made every publish()
+    // fail). Resolved once here at construction time, not per-publish(): identity never changes
+    // after wiring, and this runs on every publish() call.
+    private val resolvedDataSource: DataSource = when (val unwrapped = unwrapDataSource(dataSource)) {
+        is Unwrapped.Resolved -> unwrapped.ds
+        is Unwrapped.Unresolvable -> error(
+            "Could not resolve the outbox DataSource for transaction validation: " +
+                describeUnwrap("outbox", dataSource, unwrapped) +
+                ". Fix the proxy wiring (cycle in setTargetDataSource, or initialise the lazy " +
+                "proxy's targetDataSource before context refresh).",
+        )
+    }
+
     override fun isInActiveReadWriteTransaction(): Boolean = TransactionSynchronizationManager.isActualTransactionActive() &&
         !TransactionSynchronizationManager.isCurrentTransactionReadOnly() &&
-        TransactionSynchronizationManager.getResource(dataSource) != null
+        TransactionSynchronizationManager.getResource(resolvedDataSource) != null
 
     override val failureMessage: String
         get() = "No active read-write transaction on the outbox DataSource. " +

okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/DataSourceUnwrappingTest.kt

@@ -0,0 +1,74 @@
+package com.softwaremill.okapi.springboot
+
+import io.kotest.core.spec.style.FunSpec
+import io.kotest.matchers.shouldBe
+import io.kotest.matchers.types.shouldBeInstanceOf
+import io.kotest.matchers.types.shouldBeSameInstanceAs
+import org.springframework.jdbc.datasource.LazyConnectionDataSourceProxy
+import org.springframework.jdbc.datasource.SimpleDriverDataSource
+import org.springframework.jdbc.datasource.TransactionAwareDataSourceProxy
+import javax.sql.DataSource
+import kotlin.time.Duration.Companion.seconds
+
+/**
+ * Unit tests for [unwrapDataSource]: nested chains, null targets, and cycles.
+ *
+ * Extracted from [OutboxAutoConfiguration] (issue #91) into its own top-level utility, shared with
+ * [SpringTransactionContextValidator], so the startup PTM<->DataSource check and the runtime
+ * `publish()` check can no longer independently drift on what "the same DataSource" means.
+ */
+class DataSourceUnwrappingTest : FunSpec({
+    test("returns Resolved with the input itself when not a DelegatingDataSource") {
+        val raw: DataSource = SimpleDriverDataSource()
+        val result = unwrapDataSource(raw)
+        result.shouldBeInstanceOf<Unwrapped.Resolved>()
+        result.ds shouldBeSameInstanceAs raw
+    }
+
+    test("unwraps a single-level TransactionAwareDataSourceProxy to Resolved(raw)") {
+        val raw: DataSource = SimpleDriverDataSource()
+        val proxy: DataSource = TransactionAwareDataSourceProxy(raw)
+        val result = unwrapDataSource(proxy)
+        result.shouldBeInstanceOf<Unwrapped.Resolved>()
+        result.ds shouldBeSameInstanceAs raw
+    }
+
+    test("unwraps a nested chain TADP -> LCDP -> raw down to Resolved(raw)") {
+        val raw: DataSource = SimpleDriverDataSource()
+        val nested: DataSource = TransactionAwareDataSourceProxy(LazyConnectionDataSourceProxy(raw))
+        val result = unwrapDataSource(nested)
+        result.shouldBeInstanceOf<Unwrapped.Resolved>()
+        result.ds shouldBeSameInstanceAs raw
+    }
+
+    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. 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()
+        val result = unwrapDataSource(proxy)
+        result.shouldBeInstanceOf<Unwrapped.Unresolvable>()
+        result.stoppedAt shouldBeSameInstanceAs proxy
+        result.reason shouldBe Unwrapped.Reason.NULL_TARGET
+    }
+
+    test("returns Unresolvable(CYCLE) on a self-referencing DelegatingDataSource").config(timeout = 2.seconds) {
+        val cyclic = LazyConnectionDataSourceProxy()
+        cyclic.setTargetDataSource(cyclic)
+        val result = unwrapDataSource(cyclic)
+        result.shouldBeInstanceOf<Unwrapped.Unresolvable>()
+        result.stoppedAt shouldBeSameInstanceAs cyclic
+        result.reason shouldBe Unwrapped.Reason.CYCLE
+    }
+
+    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)
+        val result = unwrapDataSource(a)
+        result.shouldBeInstanceOf<Unwrapped.Unresolvable>()
+        result.stoppedAt shouldBeSameInstanceAs a
+        result.reason shouldBe Unwrapped.Reason.CYCLE
+    }
+})

okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OutboxAutoConfigurationTransactionRunnerTest.kt

@@ -27,7 +27,6 @@ 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
@@ -37,7 +36,6 @@ import org.springframework.transaction.support.DefaultTransactionStatus
 import org.springframework.transaction.support.ResourceTransactionManager
 import org.springframework.transaction.support.TransactionSynchronizationManager
 import javax.sql.DataSource
-import kotlin.time.Duration.Companion.seconds
 
 class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({
 
@@ -612,63 +610,6 @@ class OutboxAutoConfigurationTransactionRunnerTest : FunSpec({
                 ctx.getBean(TransactionRunner::class.java).shouldBeInstanceOf<SpringTransactionRunner>()
             }
     }
-
-    // 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()
-            val result = OutboxAutoConfiguration.unwrapDataSource(raw)
-            result.shouldBeInstanceOf<OutboxAutoConfiguration.Companion.Unwrapped.Resolved>()
-            result.ds shouldBeSameInstanceAs raw
-        }
-
-        test("unwraps a single-level TransactionAwareDataSourceProxy to Resolved(raw)") {
-            val raw: DataSource = SimpleDriverDataSource()
-            val proxy: DataSource = TransactionAwareDataSourceProxy(raw)
-            val result = OutboxAutoConfiguration.unwrapDataSource(proxy)
-            result.shouldBeInstanceOf<OutboxAutoConfiguration.Companion.Unwrapped.Resolved>()
-            result.ds shouldBeSameInstanceAs raw
-        }
-
-        test("unwraps a nested chain TADP -> LCDP -> raw down to Resolved(raw)") {
-            val raw: DataSource = SimpleDriverDataSource()
-            val nested: DataSource = TransactionAwareDataSourceProxy(LazyConnectionDataSourceProxy(raw))
-            val result = OutboxAutoConfiguration.unwrapDataSource(nested)
-            result.shouldBeInstanceOf<OutboxAutoConfiguration.Companion.Unwrapped.Resolved>()
-            result.ds shouldBeSameInstanceAs raw
-        }
-
-        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. 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()
-            val result = OutboxAutoConfiguration.unwrapDataSource(proxy)
-            result.shouldBeInstanceOf<OutboxAutoConfiguration.Companion.Unwrapped.Unresolvable>()
-            result.stoppedAt shouldBeSameInstanceAs proxy
-            result.reason shouldBe OutboxAutoConfiguration.Companion.Unwrapped.Reason.NULL_TARGET
-        }
-
-        test("returns Unresolvable(CYCLE) on a self-referencing DelegatingDataSource").config(timeout = 2.seconds) {
-            val cyclic = LazyConnectionDataSourceProxy()
-            cyclic.setTargetDataSource(cyclic)
-            val result = OutboxAutoConfiguration.unwrapDataSource(cyclic)
-            result.shouldBeInstanceOf<OutboxAutoConfiguration.Companion.Unwrapped.Unresolvable>()
-            result.stoppedAt shouldBeSameInstanceAs cyclic
-            result.reason shouldBe OutboxAutoConfiguration.Companion.Unwrapped.Reason.CYCLE
-        }
-
-        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)
-            val result = OutboxAutoConfiguration.unwrapDataSource(a)
-            result.shouldBeInstanceOf<OutboxAutoConfiguration.Companion.Unwrapped.Unresolvable>()
-            result.stoppedAt shouldBeSameInstanceAs a
-            result.reason shouldBe OutboxAutoConfiguration.Companion.Unwrapped.Reason.CYCLE
-        }
-    }
 })
 
 // Mimics JpaTransactionManager / HibernateTransactionManager: implements ResourceTransactionManager

okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/SpringTransactionContextValidatorTest.kt

@@ -1,8 +1,12 @@
 package com.softwaremill.okapi.springboot
 
+import io.kotest.assertions.throwables.shouldThrow
 import io.kotest.core.spec.style.BehaviorSpec
 import io.kotest.matchers.shouldBe
+import io.kotest.matchers.string.shouldContain
+import org.springframework.jdbc.datasource.LazyConnectionDataSourceProxy
 import org.springframework.jdbc.datasource.SimpleDriverDataSource
+import org.springframework.jdbc.datasource.TransactionAwareDataSourceProxy
 import org.springframework.transaction.support.TransactionSynchronizationManager
 import javax.sql.DataSource
 
@@ -95,4 +99,59 @@ class SpringTransactionContextValidatorTest : BehaviorSpec({
             }
         }
     }
+
+    // The outbox DataSource bean is a TransactionAwareDataSourceProxy (Spring's own
+    // documented pattern -- "TransactionAwareDataSourceProxy should NOT be passed to a PTM"), so
+    // the PlatformTransactionManager binds its resource under the RAW target, never the proxy.
+    // Before this fix, the validator compared against the raw `dataSource` constructor argument
+    // (the proxy here) with no unwrapping, so getResource(proxy) always returned null and
+    // publish() failed on every call despite OutboxAutoConfiguration's startup check having
+    // already verified this exact wiring as correct.
+    given("outbox DataSource bean is a TransactionAwareDataSourceProxy wrapping the PTM's raw target") {
+        then("an active RW transaction with the resource bound to the RAW target is recognised") {
+            val rawDs: DataSource = SimpleDriverDataSource()
+            val proxyDs: DataSource = TransactionAwareDataSourceProxy(rawDs)
+            val proxyValidator = SpringTransactionContextValidator(proxyDs)
+
+            TransactionSynchronizationManager.initSynchronization()
+            TransactionSynchronizationManager.setActualTransactionActive(true)
+            TransactionSynchronizationManager.bindResource(rawDs, Any())
+            try {
+                proxyValidator.isInActiveReadWriteTransaction() shouldBe true
+            } finally {
+                TransactionSynchronizationManager.unbindResource(rawDs)
+                TransactionSynchronizationManager.clearSynchronization()
+                TransactionSynchronizationManager.setActualTransactionActive(false)
+            }
+        }
+
+        then("a nested chain (TADP -> LazyConnectionDataSourceProxy -> raw) also resolves to the raw target") {
+            val rawDs: DataSource = SimpleDriverDataSource()
+            val nested: DataSource = TransactionAwareDataSourceProxy(LazyConnectionDataSourceProxy(rawDs))
+            val nestedValidator = SpringTransactionContextValidator(nested)
+
+            TransactionSynchronizationManager.initSynchronization()
+            TransactionSynchronizationManager.setActualTransactionActive(true)
+            TransactionSynchronizationManager.bindResource(rawDs, Any())
+            try {
+                nestedValidator.isInActiveReadWriteTransaction() shouldBe true
+            } finally {
+                TransactionSynchronizationManager.unbindResource(rawDs)
+                TransactionSynchronizationManager.clearSynchronization()
+                TransactionSynchronizationManager.setActualTransactionActive(false)
+            }
+        }
+    }
+
+    given("outbox DataSource bean is an unresolvable DelegatingDataSource chain") {
+        then("construction fails fast with an actionable message, instead of every later publish() silently failing validation") {
+            val cyclic = LazyConnectionDataSourceProxy()
+            cyclic.setTargetDataSource(cyclic)
+
+            val exception = shouldThrow<IllegalStateException> {
+                SpringTransactionContextValidator(cyclic)
+            }
+            exception.message shouldContain "Could not resolve the outbox DataSource"
+        }
+    }
 })