Skip to content

refactor: reduce duplicated source code#91

Merged
patrickleet merged 4 commits into
mainfrom
codex/reduce-loc
Jun 13, 2026
Merged

refactor: reduce duplicated source code#91
patrickleet merged 4 commits into
mainfrom
codex/reduce-loc

Conversation

@patrickleet

@patrickleet patrickleet commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • reduce Rust source LOC from 58,241 to 57,591 (net -650)
  • remove the sync outbox store API, rename the async-only trait to OutboxStore, and drop _async method suffixes
  • make OutboxPublisher::publish, OutboxWorker::process_message, and OutboxWorker::process_batch async
  • align repository/transport docs with the async-only API surface and remove stale sync-path references
  • deduplicate shared read-model and outbox handling across backends
  • extract repeated test helpers and trim redundant test-only comments

Tests

  • cargo fmt --check
  • git diff --check
  • cargo test --lib --all-features outbox_worker
  • cargo test --test todos --all-features
  • cargo test --test bomberman --all-features
  • cargo test --test microsvc --all-features
  • cargo test --workspace --all-features

Summary by CodeRabbit

Release Notes

  • New Features

    • Unified async-first outbox store API, consolidating separate async and synchronous trait variants into a single, async-only contract.
    • Async outbox publisher interface for consistent message publication behavior.
  • Documentation

    • Clarified async-only transport architecture and persistence traits across all documentation.
    • Updated outbox storage and worker guidance to reflect new async-first design.

@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@patrickleet, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 45 minutes and 46 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: db71db80-0d26-4b2c-80a2-7035f82b8d4c

📥 Commits

Reviewing files that changed from the base of the PR and between 647b12f and 3d86ed0.

📒 Files selected for processing (2)
  • src/outbox_worker/mod.rs
  • src/outbox_worker/worker.rs
📝 Walkthrough

Walkthrough

This PR consolidates the outbox worker into a fully async-first design by converting the OutboxStore trait and OutboxPublisher/OutboxWorker APIs from synchronous to async futures, removing the parallel AsyncOutboxStore abstraction, updating all backend implementations and downstream components, and refactoring read-model helpers into shared consolidation functions.

Changes

Outbox async-first consolidation

Layer / File(s) Summary
Store trait async redesign and implementations
src/outbox_worker/store.rs, src/postgres_repo/mod.rs, src/sqlite_repo/mod.rs
OutboxStore trait methods (messages_by_status, pending, claim, complete, release, fail, record_failure) now return impl Future and are implemented via async move blocks. Default async implementations provided for pending and record_failure. All three concrete stores (HashMapOutboxStore, PostgresOutboxStore, SqliteOutboxStore) updated to implement the async trait while preserving claim/complete/release/fail settlement semantics. AsyncOutboxStore trait removed.
Publisher and Worker async conversion
src/outbox_worker/publisher.rs, src/outbox_worker/worker.rs
OutboxPublisher::publish now returns impl Future<Output = Result<(), Self::Error>> + 'a with explicit lifetime bounds instead of direct Result. LogPublisher and LocalEmitterPublisher implementations return async futures using async move. OutboxWorker::process_message and process_batch converted to pub async fn, awaiting publisher.publish(...) and driving message state (complete/release/fail) based on publish outcome.
Dispatcher, source, and hook component updates
src/outbox_worker/outbox_dispatch.rs, src/outbox_worker/outbox_source.rs, src/outbox_worker/publish_hook.rs
OutboxDispatcher, OutboxSource, and BusOutboxPublishHook updated to require S: OutboxStore instead of AsyncOutboxStore. All components switch from *_async method calls (claim_async, complete_async, etc.) to non-async names (claim, complete, etc.) while keeping the same await control flow. Documentation and tests updated.
Module exports and dependency trait wiring
src/lib.rs, src/outbox_worker/mod.rs, src/microsvc/dependencies.rs
AsyncOutboxStore removed from crate-root re-exports in src/lib.rs. outbox_worker module documentation and public re-exports updated to remove AsyncOutboxStore while keeping OutboxStore and related types. HasOutboxStore dependency trait now requires OutboxStore instead of AsyncOutboxStore.
Read-model helper consolidation and backend integration
src/sqlx_repo/read_model.rs, src/postgres_repo/mod.rs, src/sqlite_repo/mod.rs, src/read_model/session.rs
Introduces commit_read_model_write_plan and load_read_model_graph shared helpers in the SQLx layer to consolidate relational read-model transactional and graph-loading logic. PostgreSQL and SQLite repositories now delegate read-model writes and graph loading to these helpers instead of inlining transaction/schema-resolution procedures. ReadModelMutation refactored to use shared helpers for table name and key derivation.
Documentation updates for async-first design
README.md, docs/async-transports.md, docs/postgres-event-store.md, docs/repositories.md, docs/research-and-roadmap.md
README clarified that blocking primitives remain internal to async transport methods and updated "Outbox publishing" row to reference OutboxStore plus async publishers with LogPublisher as dev/test default. Async-transports doc reframed to describe async-only transport layer. Postgres event store now requires async-first access and forbids blocking adapters. Repositories doc updated to reference OutboxStore as the durable outbox API. Research roadmap clarified that competing frameworks are async-first and SQL backends directly implement async traits.
Test suite updates across all test modules
tests/bomberman/*, tests/distributed_read_model/*, tests/distributed_read_model_board/*, tests/durable_enqueue_sqlite/*, tests/microsvc/*, tests/persistent_repository_conformance/*, tests/postgres_repository/*, tests/postgres_transport/*, tests/repository_api/*, tests/sagas/*, tests/sourced_snapshot/*, tests/sql_lock_manager/*, tests/sqlite_repository/*, tests/todos/*, tests/transport_conformance/*
All tests updated to await store operations (pending(), claim(), complete(), record_failure(), etc.), removing *_async variants. Synchronous test assertions converted to async (#[tokio::test]). Test helpers refactored to centralize repeated logic (e.g., initialized_todo, claim_and_process, assert_checkout_screen in tests/todos/main.rs and tests/distributed_read_model/main.rs). Matrix harness tests migrated to use run_matrix_cell helper to reduce repetition.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

  • hops-ops/distributed#40: Introduces the OutboxStore/AsyncOutboxStore handle-based worker store model; this PR builds on that by removing the dual API and consolidating to OutboxStore only.

  • hops-ops/distributed#48: Updates the transport × persistence matrix test harness around publish_pending_outbox and outbox claiming/completion; overlaps with this PR's refactoring of the same test flow.

  • hops-ops/distributed#89: Refactors shared src/sqlx_repo/read_model.rs helpers (commit_read_model_write_plan, load_read_model_graph) that this PR introduces to consolidate read-model loading.

🐰 A store once split in two,
Now unified, forever true,
Async all the way, no sync in sight,
The outbox worker shines so bright!

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/reduce-loc

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/outbox_worker/mod.rs`:
- Around line 21-31: The example shows calling OutboxClaimRef::from_message(...)
and then outbox.complete(...) without any publish step; update the snippet to
include the publish call used by the real dispatch path (i.e., invoke
publisher.publish(...).await and only call outbox.complete(&claim).await after
that publish succeeds) or replace the short example with a brief note pointing
readers to OutboxDispatcher/OutboxWorker (which perform
publisher.publish(...).await then complete) so examples do not acknowledge
messages before publishing.

In `@src/outbox_worker/worker.rs`:
- Around line 130-145: The failure path uses message.attempts compared to
self.max_attempts before counting the current failure, causing an off-by-one
extra retry; change the condition to check if message.attempts + 1 >=
self.max_attempts (or otherwise increment the attempt count before deciding) so
that when the current failure would reach max_attempts the code calls
message.fail(error_msg)? instead of message.release(error_msg) (affecting the
same branch that returns ProcessOneResult with failed/released flags).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5b8ea03f-4dea-45ea-8535-9eded29c2a86

📥 Commits

Reviewing files that changed from the base of the PR and between 9219e95 and 647b12f.

📒 Files selected for processing (36)
  • README.md
  • docs/async-transports.md
  • docs/postgres-event-store.md
  • docs/repositories.md
  • docs/research-and-roadmap.md
  • src/lib.rs
  • src/microsvc/dependencies.rs
  • src/microsvc/runtime.rs
  • src/outbox/commit.rs
  • src/outbox_worker/mod.rs
  • src/outbox_worker/outbox_dispatch.rs
  • src/outbox_worker/outbox_source.rs
  • src/outbox_worker/publish_hook.rs
  • src/outbox_worker/publisher.rs
  • src/outbox_worker/store.rs
  • src/outbox_worker/worker.rs
  • src/postgres_repo/mod.rs
  • src/read_model/session.rs
  • src/sqlite_repo/mod.rs
  • src/sqlx_repo/read_model.rs
  • tests/bomberman/main.rs
  • tests/distributed_read_model/main.rs
  • tests/distributed_read_model_board/main.rs
  • tests/durable_enqueue_sqlite/main.rs
  • tests/microsvc/convention.rs
  • tests/persistent_repository_conformance/inbox.rs
  • tests/persistent_repository_conformance/outbox.rs
  • tests/postgres_repository/main.rs
  • tests/postgres_transport/main.rs
  • tests/repository_api/main.rs
  • tests/sagas/microsvc_saga.rs
  • tests/sourced_snapshot/main.rs
  • tests/sql_lock_manager/main.rs
  • tests/sqlite_repository/main.rs
  • tests/todos/main.rs
  • tests/transport_conformance/mod.rs
💤 Files with no reviewable changes (1)
  • src/lib.rs

Comment thread src/outbox_worker/mod.rs Outdated
Comment thread src/outbox_worker/worker.rs
@patrickleet patrickleet merged commit ff5cb35 into main Jun 13, 2026
7 checks passed
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.

1 participant