Skip to content

feat(core): require TransactionRunner in OutboxScheduler/OutboxPurger (breaking)#54

Open
endrju19 wants to merge 1 commit into
mainfrom
kojak-68-core-require-tx-runner
Open

feat(core): require TransactionRunner in OutboxScheduler/OutboxPurger (breaking)#54
endrju19 wants to merge 1 commit into
mainfrom
kojak-68-core-require-tx-runner

Conversation

@endrju19
Copy link
Copy Markdown
Collaborator

Summary

Moves the "claim+deliver runs in a transaction" invariant down into okapi-core, where it belongs. OutboxScheduler and OutboxPurger now require a non-null TransactionRunner constructor parameter; the nullable default and the silent non-transactional fallback in tick() are gone.

Under JDBC auto-commit, FOR UPDATE SKIP LOCKED releases its row lock at the end of the claim SELECT. Concurrent processor instances then see overlapping result sets and silently deliver the same entry multiple times — no log, no exception, no metric. This PR removes that footgun from the core API. Companion to #49, which closed the same hole at the Spring Boot autoconfig layer; this PR completes the story for non-Spring consumers (Ktor, manual Spring wiring, plain Java/Kotlin).

Breaking change — pre-1.0, permitted by the project's stated versioning policy.

Changes

  • OutboxScheduler and OutboxPurger: transactionRunner: TransactionRunner? = nulltransactionRunner: TransactionRunner (required); drop the ?: fallback in tick(). @JvmOverloads kept (still useful for remaining default params config / clock).
  • KDoc on OutboxScheduler documents the FOR UPDATE SKIP LOCKED rationale — the why is non-obvious from the signature alone and would otherwise tempt a future reader to "simplify" back to nullable.
  • Test updates: 5 scheduler + 7 purger construction sites supplied with a noOpTransactionRunner() helper; deleted "tick runs without transactionRunner" (exercised the removed null-fallback); added new test "transactionRunner wraps each batch delete" on the purger — asserts the runner is invoked per batch, not just present (closes a behavioral gap the type system cannot enforce; analogous to the existing scheduler test).
  • Renamed "transactionRunner wraps tick when provided""transactionRunner wraps tick" (always provided now).
  • CHANGELOG.md: new ### Changed (BREAKING) entry under [Unreleased]. Migration example uses anonymous-object syntax, not lambda — TransactionRunner is intentionally a plain interface, not fun interface, because Kotlin forbids SAM conversion on generic abstract methods.

What this does NOT change

  • Spring Boot autoconfig — already provides a TransactionRunner after feat: require TransactionRunner in okapi-spring-boot autoconfig (KOJAK-67) #49; no further changes needed.
  • The TransactionRunner interface itself — unchanged.
  • Existing concrete impls (SpringTransactionRunner, ExposedTransactionRunner).
  • Logging / metrics for the schedulers — out of scope; one related logging improvement for OutboxPurger.tick() was identified during review and deferred to a separate follow-up.

Test plan

  • ./gradlew :okapi-core:test — green
  • ./gradlew :okapi-spring-boot:test — green (adapter still happy)
  • ./gradlew ktlintCheck — clean
  • ./gradlew build — full build green
  • Grep verified: every OutboxScheduler( / OutboxPurger( construction site in the repo now passes an explicit transactionRunner.

Closes #51.

… (breaking)

Drop the nullable default and the silent non-transactional fallback in tick().
Under JDBC auto-commit, FOR UPDATE SKIP LOCKED releases its row lock at the
end of the claim SELECT and concurrent processor instances deliver the same
entry multiple times. Make the invariant explicit at the type level in
okapi-core, complementing #49's adapter-layer fix.

Closes #51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

core: require non-null TransactionRunner in OutboxScheduler / OutboxPurger (breaking)

1 participant