diff --git a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OkapiMicrometerAutoConfiguration.kt b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OkapiMicrometerAutoConfiguration.kt index 22d3b7e..364eaf7 100644 --- a/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OkapiMicrometerAutoConfiguration.kt +++ b/okapi-spring-boot/src/main/kotlin/com/softwaremill/okapi/springboot/OkapiMicrometerAutoConfiguration.kt @@ -5,6 +5,7 @@ import com.softwaremill.okapi.micrometer.MicrometerOutboxListener import com.softwaremill.okapi.micrometer.MicrometerOutboxMetrics import com.softwaremill.okapi.micrometer.OutboxMetricsRefresher import io.micrometer.core.instrument.MeterRegistry +import org.springframework.beans.factory.BeanFactory import org.springframework.beans.factory.ObjectProvider import org.springframework.boot.autoconfigure.AutoConfiguration import org.springframework.boot.autoconfigure.AutoConfigureAfter @@ -40,7 +41,7 @@ import java.time.Clock ) @ConditionalOnClass(name = ["io.micrometer.core.instrument.MeterRegistry"]) @ConditionalOnBean(MeterRegistry::class) -@EnableConfigurationProperties(OkapiMetricsProperties::class) +@EnableConfigurationProperties(OkapiMetricsProperties::class, OkapiProperties::class) class OkapiMicrometerAutoConfiguration { @Bean @ConditionalOnMissingBean @@ -53,8 +54,20 @@ class OkapiMicrometerAutoConfiguration { registry: MeterRegistry, transactionManager: ObjectProvider, clock: ObjectProvider, + beanFactory: BeanFactory, + okapiProperties: OkapiProperties, ): MicrometerOutboxMetrics { - val readOnlyRunner = transactionManager.getIfAvailable()?.let { tm -> + // The PTM is optional here (metrics work fine without a read-only runner), so we can't reuse + // OutboxAutoConfiguration.resolvePlatformTransactionManager() as-is — it throws when no PTM is + // found. When a qualifier is set it still must be honoured (explicit user config), via the + // shared resolvePlatformTransactionManagerByQualifier(); otherwise fall back to + // getIfUnique(), which returns null instead of throwing when multiple PTMs are present. This + // fixes issue #80: getIfAvailable() throws NoUniqueBeanDefinitionException with 2+ PTM beans, + // even when okapi.transaction-manager-qualifier disambiguates which one to use. + val ptm = okapiProperties.transactionManagerQualifier?.let { qualifier -> + OutboxAutoConfiguration.resolvePlatformTransactionManagerByQualifier(beanFactory, qualifier) + } ?: transactionManager.getIfUnique() + val readOnlyRunner = ptm?.let { tm -> SpringTransactionRunner(TransactionTemplate(tm).apply { isReadOnly = true }) } return MicrometerOutboxMetrics( 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 9d9dfc9..af58379 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 @@ -18,6 +18,7 @@ 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.beans.factory.getBean import org.springframework.boot.autoconfigure.AutoConfiguration import org.springframework.boot.autoconfigure.condition.ConditionalOnClass import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression @@ -277,6 +278,44 @@ class OutboxAutoConfiguration( ) } + /** + * Resolves the [PlatformTransactionManager] bean named by `okapi.transaction-manager-qualifier`, + * rewrapping Spring's lookup exceptions with okapi-specific context. Shared by + * [resolvePlatformTransactionManager] (required PTM, e.g. the outbox scheduler) and + * [OkapiMicrometerAutoConfiguration.micrometerOutboxMetrics] (optional PTM) so both honour the + * qualifier identically instead of one silently ignoring it — see issue #80, where + * `micrometerOutboxMetrics` used a bare `ObjectProvider.getIfAvailable()` and threw + * `NoUniqueBeanDefinitionException` whenever multiple PTMs were present, regardless of the + * qualifier. + */ + internal fun resolvePlatformTransactionManagerByQualifier( + beanFactory: BeanFactory, + qualifier: String, + ): PlatformTransactionManager { + return try { + beanFactory.getBean(qualifier) + } catch (e: NoSuchBeanDefinitionException) { + throw NoSuchBeanDefinitionException( + qualifier, + "okapi.transaction-manager-qualifier='$qualifier' — no PlatformTransactionManager bean named " + + "'$qualifier' found. Check the bean name. If you remove the property, okapi will attempt " + + "auto-resolution (may still require a single PTM or an @Primary bean).", + ).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, + ) + } + } + internal fun resolvePlatformTransactionManager( provider: ObjectProvider, beanFactory: BeanFactory, @@ -284,28 +323,7 @@ class OutboxAutoConfiguration( ): 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) } - } 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, - ) - } + return resolvePlatformTransactionManagerByQualifier(beanFactory, qualifier) } val unique = provider.getIfUnique() if (unique != null) return unique diff --git a/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OkapiMicrometerAutoConfigurationTest.kt b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OkapiMicrometerAutoConfigurationTest.kt new file mode 100644 index 0000000..cd3bbac --- /dev/null +++ b/okapi-spring-boot/src/test/kotlin/com/softwaremill/okapi/springboot/OkapiMicrometerAutoConfigurationTest.kt @@ -0,0 +1,190 @@ +package com.softwaremill.okapi.springboot + +import com.softwaremill.okapi.core.OutboxStatus +import com.softwaremill.okapi.core.OutboxStore +import com.softwaremill.okapi.micrometer.MicrometerOutboxMetrics +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.micrometer.core.instrument.MeterRegistry +import io.micrometer.core.instrument.simple.SimpleMeterRegistry +import org.h2.jdbcx.JdbcDataSource +import org.springframework.beans.factory.NoSuchBeanDefinitionException +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.DataSourceTransactionManager +import org.springframework.transaction.PlatformTransactionManager +import org.springframework.transaction.support.TransactionSynchronizationManager +import java.time.Clock +import javax.sql.DataSource + +/** + * Regression coverage for issue #80: `micrometerOutboxMetrics()` used a bare + * `ObjectProvider.getIfAvailable()`, which throws + * `NoUniqueBeanDefinitionException` whenever 2+ PlatformTransactionManager beans are present — + * regardless of `okapi.transaction-manager-qualifier`. The fix reuses + * [OutboxAutoConfiguration.resolvePlatformTransactionManagerByQualifier], the same qualifier + * resolution [OutboxAutoConfiguration] uses for its (required) PTM lookup. + */ +class OkapiMicrometerAutoConfigurationTest : FunSpec({ + + val baseRunner = ApplicationContextRunner() + .withConfiguration(AutoConfigurations.of(OkapiMicrometerAutoConfiguration::class.java)) + .withBean(OutboxStore::class.java, { stubStore() }) + .withBean(MeterRegistry::class.java, { SimpleMeterRegistry() }) + + test("BUG #80 regression: two PlatformTransactionManager beans, none @Primary, no qualifier — context still starts") { + // Pre-fix, this scenario always threw NoUniqueBeanDefinitionException from + // ObjectProvider.getIfAvailable() inside micrometerOutboxMetrics(). + baseRunner + .withBean("tm1", PlatformTransactionManager::class.java, { DataSourceTransactionManager(h2DataSource()) }) + .withBean("tm2", PlatformTransactionManager::class.java, { DataSourceTransactionManager(h2DataSource()) }) + .run { ctx -> + ctx.startupFailure.shouldBeNull() + ctx.getBean(MicrometerOutboxMetrics::class.java).shouldNotBeNull() + } + } + + test("okapi.transaction-manager-qualifier avoids the BUG #80 crash when multiple PTMs are present") { + baseRunner + .withBean("tm1", PlatformTransactionManager::class.java, { DataSourceTransactionManager(h2DataSource()) }) + .withBean("tm2", PlatformTransactionManager::class.java, { DataSourceTransactionManager(h2DataSource()) }) + .withPropertyValues("okapi.transaction-manager-qualifier=tm2") + .run { ctx -> + ctx.startupFailure.shouldBeNull() + ctx.getBean(MicrometerOutboxMetrics::class.java).shouldNotBeNull() + } + } + + test("okapi.transaction-manager-qualifier pointing to a nonexistent bean fails fast with actionable message") { + // Proves the shared resolver (and its error wrapping) is actually reused here, not + // reimplemented — same message shape as OutboxAutoConfigurationTransactionRunnerTest. + baseRunner + .withBean("tm1", PlatformTransactionManager::class.java, { DataSourceTransactionManager(h2DataSource()) }) + .withBean("tm2", PlatformTransactionManager::class.java, { DataSourceTransactionManager(h2DataSource()) }) + .withPropertyValues("okapi.transaction-manager-qualifier=missingTm") + .run { ctx -> + val failure = ctx.startupFailure + failure.shouldNotBeNull() + val chain = generateSequence(failure as Throwable?) { it.cause }.toList() + chain.any { it is NoSuchBeanDefinitionException } shouldBe true + val allMessages = chain.mapNotNull { it.message } + allMessages.any { it.contains("okapi.transaction-manager-qualifier") } shouldBe true + allMessages.any { it.contains("missingTm") } shouldBe true + } + } + + test("no PlatformTransactionManager present: MicrometerOutboxMetrics bean is still created (PTM remains optional)") { + baseRunner.run { ctx -> + ctx.startupFailure.shouldBeNull() + ctx.getBean(MicrometerOutboxMetrics::class.java).shouldNotBeNull() + } + } + + // The tests below call the @Bean factory method directly against a plain (non-autoconfigured) + // GenericApplicationContext, bypassing OkapiMicrometerAutoConfiguration's `outboxMetricsRefresher` + // bean entirely. That bean starts a background thread that calls `refresh()` immediately, which + // would race with the assertions below (both threads reading/binding transactional resources on + // the same DataSource). Driving the factory method directly keeps these deterministic while still + // proving *which* PlatformTransactionManager backs the resulting read-only TransactionRunner. + context("PlatformTransactionManager selection (verified via TransactionSynchronizationManager)") { + + test("qualifier selects the correct PlatformTransactionManager among several (not merely avoids the crash)") { + val ds1: DataSource = h2DataSource() + val ds2: DataSource = h2DataSource() + val beanCtx = GenericApplicationContext().apply { + registerBean("tm1") { DataSourceTransactionManager(ds1) } + registerBean("tm2") { DataSourceTransactionManager(ds2) } + refresh() + } + var ds1Bound: Boolean? = null + var ds2Bound: Boolean? = null + val store = object : OutboxStore by stubStore() { + override fun countByStatuses(): Map { + ds1Bound = TransactionSynchronizationManager.getResource(ds1) != null + ds2Bound = TransactionSynchronizationManager.getResource(ds2) != null + return emptyMap() + } + } + val metrics = OkapiMicrometerAutoConfiguration().micrometerOutboxMetrics( + store = store, + registry = SimpleMeterRegistry(), + transactionManager = beanCtx.beanFactory.getBeanProvider(PlatformTransactionManager::class.java), + clock = beanCtx.beanFactory.getBeanProvider(Clock::class.java), + beanFactory = beanCtx.beanFactory, + okapiProperties = OkapiProperties(transactionManagerQualifier = "tm2"), + ) + metrics.refresh() + ds1Bound shouldBe false + ds2Bound shouldBe true + } + + test( + "no qualifier + single PlatformTransactionManager: it is used for the read-only snapshot (baseline preserved by the refactor)", + ) { + val ds: DataSource = h2DataSource() + val beanCtx = GenericApplicationContext().apply { + registerBean("onlyTm") { DataSourceTransactionManager(ds) } + refresh() + } + var dsBound: Boolean? = null + val store = object : OutboxStore by stubStore() { + override fun countByStatuses(): Map { + dsBound = TransactionSynchronizationManager.getResource(ds) != null + return emptyMap() + } + } + val metrics = OkapiMicrometerAutoConfiguration().micrometerOutboxMetrics( + store = store, + registry = SimpleMeterRegistry(), + transactionManager = beanCtx.beanFactory.getBeanProvider(PlatformTransactionManager::class.java), + clock = beanCtx.beanFactory.getBeanProvider(Clock::class.java), + beanFactory = beanCtx.beanFactory, + okapiProperties = OkapiProperties(), + ) + metrics.refresh() + dsBound shouldBe true + } + + test( + "no qualifier + multiple PlatformTransactionManagers (ambiguous): runs without a transaction " + + "(getIfUnique returns null on ambiguity instead of throwing)", + ) { + val ds1: DataSource = h2DataSource() + val ds2: DataSource = h2DataSource() + val beanCtx = GenericApplicationContext().apply { + registerBean("tm1") { DataSourceTransactionManager(ds1) } + registerBean("tm2") { DataSourceTransactionManager(ds2) } + refresh() + } + var ds1Bound: Boolean? = null + var ds2Bound: Boolean? = null + val store = object : OutboxStore by stubStore() { + override fun countByStatuses(): Map { + ds1Bound = TransactionSynchronizationManager.getResource(ds1) != null + ds2Bound = TransactionSynchronizationManager.getResource(ds2) != null + return emptyMap() + } + } + val metrics = OkapiMicrometerAutoConfiguration().micrometerOutboxMetrics( + store = store, + registry = SimpleMeterRegistry(), + transactionManager = beanCtx.beanFactory.getBeanProvider(PlatformTransactionManager::class.java), + clock = beanCtx.beanFactory.getBeanProvider(Clock::class.java), + beanFactory = beanCtx.beanFactory, + okapiProperties = OkapiProperties(), + ) + metrics.refresh() + ds1Bound shouldBe false + ds2Bound shouldBe false + } + } +}) + +private fun h2DataSource(): DataSource = JdbcDataSource().apply { + setURL("jdbc:h2:mem:micrometer-ptm-${System.nanoTime()};DB_CLOSE_DELAY=-1") + user = "sa" + password = "" +}