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 af58379..0f43017 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 @@ -27,6 +27,7 @@ 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.core.annotation.Order import org.springframework.jdbc.datasource.DelegatingDataSource import org.springframework.transaction.PlatformTransactionManager import org.springframework.transaction.support.ResourceTransactionManager @@ -50,8 +51,11 @@ import javax.sql.DataSource * * Optional beans with defaults: * - [OutboxStore] — auto-configured to [PostgresOutboxStore] or [MysqlOutboxStore] - * depending on which module (`okapi-postgres` / `okapi-mysql`) is on the classpath. - * If both are present, Postgres takes priority. Override by defining your own `@Bean OutboxStore`. + * depending on which module (`okapi-postgres` / `okapi-mysql`) is on the classpath. If *both* + * are present, Postgres takes priority — enforced deterministically via `@Order` on + * [PostgresStoreConfiguration] / [MysqlStoreConfiguration] (issue #90: this was previously only + * documented, not enforced, and MySQL could silently win instead depending on undocumented + * nested-`@Configuration` processing order). Override by defining your own `@Bean OutboxStore`. * - [Clock] — defaults to [Clock.systemUTC] * - [RetryPolicy] — defaults to `maxRetries = 5` * @@ -232,8 +236,23 @@ class OutboxAutoConfiguration( ) } + /** + * Auto-detects [PostgresOutboxStore] when `okapi-postgres` is on the classpath. + * + * **Precedence when both `okapi-postgres` and `okapi-mysql` are present:** `@Order(1)` here + * vs. `@Order(2)` on [MysqlStoreConfiguration] deterministically makes Postgres win. Spring's + * `ConfigurationClassParser.processMemberClasses` sorts sibling nested `@Configuration` + * candidates via `OrderComparator` *before* processing them, so `PostgresStoreConfiguration`'s + * `@Bean outboxStore()` is always registered first — by the time `MysqlStoreConfiguration`'s + * own `@ConditionalOnMissingBean(OutboxStore::class)` is evaluated, Postgres's bean already + * exists and MySQL's is skipped. Before this annotation existed (issue #90), nothing enforced + * an order at all: which of the two candidates Spring happened to process first was + * undocumented and not declaration-order-guaranteed, and in practice MySQL could silently win + * — applying its DDL/SQL against a Postgres database while the app looked healthy at startup. + */ @Configuration(proxyBeanMethods = false) @ConditionalOnClass(PostgresOutboxStore::class) + @Order(1) class PostgresStoreConfiguration( private val dataSources: Map, private val primaryDataSource: DataSource, @@ -246,9 +265,10 @@ class OutboxAutoConfiguration( ) } - /** When both Postgres and MySQL modules are on the classpath, [PostgresStoreConfiguration] takes priority. */ + /** Loses precedence to [PostgresStoreConfiguration] when both are present -- see its KDoc. */ @Configuration(proxyBeanMethods = false) @ConditionalOnClass(MysqlOutboxStore::class) + @Order(2) class MysqlStoreConfiguration( private val dataSources: Map, private val primaryDataSource: DataSource, 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 4cfe78f..556fe5a 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 @@ -17,6 +17,7 @@ import io.kotest.matchers.nulls.shouldNotBeNull import io.kotest.matchers.shouldBe import io.kotest.matchers.types.shouldBeInstanceOf import io.kotest.matchers.types.shouldBeSameInstanceAs +import io.kotest.matchers.types.shouldNotBeInstanceOf import liquibase.integration.spring.SpringLiquibase import org.slf4j.LoggerFactory import org.springframework.beans.factory.config.BeanPostProcessor @@ -391,22 +392,26 @@ class LiquibaseAutoConfigurationTest : FunSpec({ cond.value.toList().shouldBeEmpty() } - test("dual-module classpath: only ONE okapi*Liquibase bean activates — matching OutboxStore winner") { - // Pins the OutboxStore-precedence contract for Liquibase auto-config (issue #38 - // / KOJAK-80). Both `okapi-postgres` and `okapi-mysql` are on the test classpath - // (see okapi-spring-boot/build.gradle.kts testImplementation declarations). The - // `*OutboxStore` factories share - // `@ConditionalOnMissingBean(OutboxStore::class)`, so exactly ONE store bean wins. - // The Liquibase configs MUST mirror that precedence: registering both - // `okapiPostgresLiquibase` and `okapiMysqlLiquibase` against the same DataSource - // would let the second-evaluated Liquibase apply wrong-engine DDL at startup and - // fail (duplicate index, wrong-engine syntax, or shared tracking-table collisions). + test("dual-module classpath: PostgresOutboxStore deterministically wins, only okapiPostgresLiquibase activates (issue #90)") { + // Historically (issue #38 / KOJAK-80) this test pinned "exactly one Liquibase + // activates, matching whichever OutboxStore won the nested-@Configuration processing + // order" — without asserting *which* one. That undocumented, non-deterministic race + // is exactly what issue #90 found going wrong in practice: MySQL silently won over + // the KDoc's claimed "Postgres takes priority", applying MySQL DDL/SQL (FORCE INDEX, + // etc.) against a Postgres database — the app started cleanly, looked healthy, and + // then failed every processor tick with a syntax error, never having delivered + // anything. // - // The production fix lives in [OkapiLiquibaseAutoConfiguration] — a separate - // `@AutoConfiguration(after = OutboxAutoConfiguration)` so that the per-engine - // `@ConditionalOnBean(OutboxStore)` gates fire AFTER the store factories have - // registered their winning bean. Within a single auto-config those gates would - // evaluate before sibling beans are visible and would always skip. + // The fix (see OutboxAutoConfiguration KDoc) makes the precedence deterministic + // instead of merely documented: @Order(1) on PostgresStoreConfiguration vs @Order(2) + // on MysqlStoreConfiguration makes Spring register Postgres's OutboxStore bean first, + // so MysqlStoreConfiguration's own @ConditionalOnMissingBean(OutboxStore::class) then + // correctly sees it's not needed. Liquibase mirrors that winner via its per-engine + // @ConditionalOnBean(OutboxStore) gates. + // + // Both `okapi-postgres` and `okapi-mysql` are on the test classpath (see + // okapi-spring-boot/build.gradle.kts testImplementation declarations) — no + // FilteredClassLoader needed to reach the dual-module scenario. // // SuppressSpringLiquibaseRun prevents afterPropertiesSet() from trying to migrate // a fake DataSource — we're asserting bean activation, not migration behaviour. @@ -420,29 +425,33 @@ class LiquibaseAutoConfigurationTest : FunSpec({ } .run { ctx -> ctx.startupFailure shouldBe null + ctx.getBean(OutboxStore::class.java).shouldBeInstanceOf() + ctx.containsBean("okapiPostgresLiquibase") shouldBe true + ctx.containsBean("okapiMysqlLiquibase") shouldBe false + } + } - val storeBean = ctx.getBean(OutboxStore::class.java) - val expected = when (storeBean) { - is com.softwaremill.okapi.postgres.PostgresOutboxStore -> "okapiPostgresLiquibase" - is com.softwaremill.okapi.mysql.MysqlOutboxStore -> "okapiMysqlLiquibase" - else -> error("unexpected OutboxStore type ${storeBean::class}") - } - val active = listOf("okapiPostgresLiquibase", "okapiMysqlLiquibase") - .filter { ctx.containsBean(it) } - - // Diagnostic: when the assertion fails, surface what each registered - // SpringLiquibase bean would have done (changelog path + dataSource - // identity) so the reader sees concretely why dual activation is broken. - val diagnostic = active.joinToString("\n") { name -> - val bean = ctx.getBean(name, SpringLiquibase::class.java) - " $name → changelog=${bean.changeLog}, dataSource=${System.identityHashCode(bean.dataSource)}" - } - withClue( - "Active OutboxStore is ${storeBean::class.simpleName}; expected exactly the " + - "matching Liquibase bean ($expected) to activate, but found: $active\n$diagnostic", - ) { - active shouldBe listOf(expected) - } + test("dual-module classpath + explicit @Bean OutboxStore: escape hatch overrides the Postgres-wins default") { + // Pins that the documented override ("define an explicit @Bean OutboxStore to + // disambiguate") still works with both modules on the classpath: the escape-hatch + // bean satisfies PostgresStoreConfiguration's own @ConditionalOnMissingBean(OutboxStore::class) + // first (it's @Order(1)), so neither okapi-postgres's nor okapi-mysql's factory fires. + ApplicationContextRunner() + .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(OutboxStore::class.java, { stubStore() }) + .run { ctx -> + ctx.startupFailure shouldBe null + // Neither okapi-postgres's nor okapi-mysql's own factory won -- the escape + // hatch bean is what's actually wired in, not a coincidental auto-detected one. + ctx.getBean(OutboxStore::class.java) + .shouldNotBeInstanceOf() + ctx.getBean(OutboxStore::class.java) + .shouldNotBeInstanceOf() + ctx.containsBean("okapiPostgresLiquibase") shouldBe false + ctx.containsBean("okapiMysqlLiquibase") shouldBe false } } 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 9a2719f..6d15a43 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 @@ -2,6 +2,7 @@ package com.softwaremill.okapi.springboot import com.mysql.cj.jdbc.MysqlDataSource import com.softwaremill.okapi.core.MessageDeliverer +import com.softwaremill.okapi.core.OutboxStore import com.softwaremill.okapi.mysql.MysqlOutboxStore import com.softwaremill.okapi.postgres.PostgresOutboxStore import io.kotest.core.spec.style.FunSpec @@ -9,6 +10,7 @@ import io.kotest.matchers.collections.shouldContain import io.kotest.matchers.collections.shouldNotContain import io.kotest.matchers.nulls.shouldBeNull import io.kotest.matchers.shouldBe +import io.kotest.matchers.types.shouldBeInstanceOf import liquibase.integration.spring.SpringLiquibase import org.postgresql.ds.PGSimpleDataSource import org.springframework.beans.factory.support.BeanDefinitionBuilder @@ -72,11 +74,9 @@ class LiquibaseE2ETest : FunSpec({ // Hide MysqlOutboxStore from the classpath so that PostgresStoreConfiguration is the only // store factory that activates and `okapiPostgresLiquibase` is the only Liquibase bean // that registers (its `@ConditionalOnBean(PostgresOutboxStore)` gate matches the winner). - // Without this filter the OutboxStore precedence in the test JVM is non-deterministic - // between Postgres and MySQL, and these tests need to deterministically exercise the - // Postgres path against a real Postgres DataSource. The dual-module coexistence (both - // modules visible, only the matching Liquibase activates) is covered by - // ["both okapi-postgres and okapi-mysql on classpath: exactly one okapi*Liquibase activates..."]. + // Not strictly necessary since @Order(1) on PostgresStoreConfiguration already makes it + // win deterministically when both modules are visible (see the dual-module test below), + // but keeping the filter here isolates these tests from that mechanism entirely. fun runner(ds: DataSource) = ApplicationContextRunner() .withClassLoader(FilteredClassLoader(MysqlOutboxStore::class.java)) .withConfiguration(AutoConfigurations.of(OutboxAutoConfiguration::class.java, OkapiLiquibaseAutoConfiguration::class.java)) @@ -89,13 +89,22 @@ class LiquibaseE2ETest : FunSpec({ beforeEach { resetSchema() } - test("both okapi-postgres and okapi-mysql on classpath: exactly one okapi*Liquibase activates against a real Postgres database") { - // Regression test for issue #38 / KOJAK-80. Before the per-engine + test("both okapi-postgres and okapi-mysql on classpath: Postgres wins and migrates the real Postgres database") { + // Originally a regression test for issue #38 / KOJAK-80: before the per-engine // @ConditionalOnBean(OutboxStore) gate on each *LiquibaseConfiguration, both // `okapiPostgresLiquibase` and `okapiMysqlLiquibase` registered against the same // DataSource and the second-evaluated Liquibase failed at startup with a - // duplicate-object error from the wrong-engine changelog - // (e.g. ERROR: relation "idx_okapi_outbox_status_last_attempt" already exists). + // duplicate-object error from the wrong-engine changelog. + // + // That gate fixed the dual-registration crash, but left the underlying ambiguity + // (which OutboxStore wins) an undocumented nested-@Configuration processing-order + // race — which is exactly what issue #90 found going wrong in practice: MySQL won + // silently, its DDL/SQL got applied to the Postgres database, and the app looked + // healthy at startup while every processor tick failed afterward. The fix + // (@Order(1)/@Order(2) on PostgresStoreConfiguration/MysqlStoreConfiguration in + // OutboxAutoConfiguration) makes Postgres win deterministically instead. This test + // proves it end-to-end: PostgresOutboxStore wins, okapiPostgresLiquibase is the only + // Liquibase bean, and it successfully migrates this real Postgres database. // // No FilteredClassLoader here — both `okapi-postgres` and `okapi-mysql` are visible // on the runtime classpath, mirroring a real consumer that pulls in both modules @@ -112,9 +121,13 @@ class LiquibaseE2ETest : FunSpec({ ) .run { ctx -> ctx.startupFailure.shouldBeNull() - val activeLiquibase = listOf("okapiPostgresLiquibase", "okapiMysqlLiquibase") - .filter { ctx.containsBean(it) } - activeLiquibase.size shouldBe 1 + ctx.getBean(OutboxStore::class.java).shouldBeInstanceOf() + ctx.containsBean("okapiPostgresLiquibase") shouldBe true + ctx.containsBean("okapiMysqlLiquibase") shouldBe false + + val tables = listTables(ds) + tables shouldContain "okapi_databasechangelog" + tables shouldContain "okapi_outbox" } }