feat: require TransactionRunner in okapi-spring-boot autoconfig (KOJAK-67)#49
Open
endrju19 wants to merge 2 commits into
Open
feat: require TransactionRunner in okapi-spring-boot autoconfig (KOJAK-67)#49endrju19 wants to merge 2 commits into
endrju19 wants to merge 2 commits into
Conversation
…K-67) 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.
…qualifier safety
Address review-round findings on top of the KOJAK-67 baseline:
- okapiTransactionRunner factory now reads ObjectProvider<TransactionTemplate>
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes the silent JDBC auto-commit fallback in
OutboxAutoConfigurationthat allowed duplicate delivery across processor instances when noPlatformTransactionManagerwas on the classpath. Replaces it with a fail-fast bean factory plus a structured safety net for the multi-DataSource / multi-PTM / proxy edge cases discovered during review.Why this matters
Under the prior code path, scheduler ticks ran in JDBC auto-commit when no PTM was registered.
FOR UPDATE SKIP LOCKEDreleased its row lock as soon as the SELECT statement returned — beforeupdateAfterProcessingfinished — so a second processor instance could claim the same entry and deliver it again. Zero error, zero log, invisible until horizontal scaling.What changed
Production (
okapi-spring-boot):@Bean okapiTransactionRunnerfactory — derivesSpringTransactionRunnerfromPlatformTransactionManager(qualifier > getIfUnique > distinct error); honours a user-supplied@Bean TransactionTemplatewhen present; gated by@ConditionalOnExpressionso publish-only deployments (both schedulers disabled) start without requiring a PTM.okapi.transaction-manager-qualifierproperty (mirror ofokapi.datasource-qualifier), includingspring-configuration-metadata.jsonentry for IDE autocomplete.validatePtmDataSourceMatch— fails fast when an RTM-based PTM is bound to a differentDataSourcethan okapi's outbox; logs an actionable WARN (with the actualresourceFactoryclass) for PTMs that don't expose a JDBCDataSource(JPA'sEntityManagerFactory, Hibernate'sSessionFactory, Exposed bridge, JTA); emits an INFO breadcrumb in single-DataSource non-RTM setups so future multi-DS migrations have a grep target; rewrapsBeanNotOfRequiredTypeExceptionfrom a mis-typed qualifier with okapi-specific context.unwrapDataSource— iterative with visited-set; safely unwrapsTransactionAwareDataSourceProxy/LazyConnectionDataSourceProxychains before reference comparison; cannot hang on a self-referencingDelegatingDataSource.OutboxProcessorScheduler/OutboxPurgerSchedulerconstructors take a non-nullTransactionRunner(was nullableTransactionTemplate).Tests:
OutboxAutoConfigurationTransactionRunnerTest— 19 slice tests (multi-PTM, qualifier resolution, RTM/non-RTM/JPA-RTM validation, property binding, publish-only mode, user-template honoured, etc.) + 6 unit tests forunwrapDataSource(cycle / null target / nested chain).SpringObjectProviderSemanticsAssumptionsTest— pins Spring 7ObjectProvider.getIfUnique/ResourceTransactionManagercontract so framework upgrades surface loudly.ExposedSpringBridgeEndToEndTest— Postgres testcontainer + ExposedSpringTransactionManagerbridge, including a multi-instance amplification test (5 concurrent processors × 50 entries, mutation-verified to fail under the auto-commit regression) and a purger E2E pod the bridged PTM.TransactionRunnerwhere they ran without a PTM, plus content-asserted WARN/INFO log assertions.Build / infrastructure:
okapi-spring-boot: opt-in PIT mutation testing via-PenableMutationTesting=true.okapi-integration-tests:spring7-transaction+assertj+spring-boot-testfor the Exposed bridge end-to-end coverage.Docs:
Test plan
./gradlew checkpasses (Postgres + MySQL + Kafka testcontainers, all modules)okapi.transaction-manager-qualifierYAML contractokapi_databasechangelogtables to isolate library migration state #37 / KOJAK-80) still pass post-rebased9aa3be)Out of scope (separate follow-up tickets)
These pre-existing concerns surfaced during review but are NOT part of this PR's scope — they live in
okapi-coreor predate this change:OutboxScheduler.transactionRunner: TransactionRunner? = nullis still nullable (Ktor / direct-core users can still hit the original auto-commit bug). Closing this is a breaking API change in core and warrants its own design discussion.OutboxScheduler.tick()/OutboxPurger.tick()swallow-and-retry-forever — log+retry on every exception with no escalation or listener callback for consecutive failures.OutboxStatus.from()throws on a corrupt status string, halting the entire pipeline.SpringTransactionRunner.runInTransactionuses!!— NPE risk for nullable generic types (currently unreachable, future-fragile).Each of the above is a self-contained problem worth its own ticket and dedicated test coverage.