Skip to content

Move duplicated quoting DB logic for OB and autopilot into shared - #4939

Merged
AryanGodara merged 4 commits into
mainfrom
aryan/be-284-moveduplicated-quoting-logic-to-shared
Sep 23, 2026
Merged

AryanGodara merged 4 commits into
mainfrom
aryan/be-284-moveduplicated-quoting-logic-to-shared

Conversation

@AryanGodara

Copy link
Copy Markdown
Member

Description

The QuoteStoring save/get/find impls were duplicated for orderbook and autopilot. Moves them into shared/quote_storage.rs, where the fast-path save_quote_competition logic already live, so both flows share singular implementation.

Changes

  • Add save_quote, get_quote, find_quote to shared::quote_storage, each taking &PgPool and owning its own connection/transaction.
  • Make save_quote_competition private. its only caller is now save_quote.
  • Both QuoteStoring for Postgres impls just start a metrics timer and delegate; the timer stays per-crate (separate Metrics types).
  • Add a database::PgPool type alias so shared names the pool type without a direct sqlx dependency.

How to test

Existing unit and e2e tests (no expected behaviour chagnes)

Closes BE-284

Signed-off-by: Aryan Godara <aryangodara03@gmail.com>
@AryanGodara AryanGodara self-assigned this Sep 18, 2026
@linear-code

linear-code Bot commented Sep 18, 2026

Copy link
Copy Markdown

BE-284

…icated-quoting-logic-to-shared

Signed-off-by: Aryan Godara <aryangodara03@gmail.com>
…icated-quoting-logic-to-shared

Signed-off-by: Aryan Godara <aryangodara03@gmail.com>
@AryanGodara
AryanGodara marked this pull request as ready for review September 21, 2026 08:22
@AryanGodara
AryanGodara requested a review from a team as a code owner September 21, 2026 08:22
@claude

claude Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Claude finished @AryanGodara's task in 1m 13s —— View job


PR Review: Move duplicated quoting DB logic into shared

Reviewed the refactor. This is a clean, behavior-preserving move — no blocking issues found. ✅

What I verified:

  • The save/get/find QuoteStoring impls were byte-for-byte identical between orderbook and autopilot on origin/main, so consolidating them into shared::quote_storage is safe. The moved bodies (transaction handling, connection acquisition, create_db_search_parameters, error context) match the originals exactly.
  • Both call sites correctly keep their per-crate metrics timer and just delegate — the separate Metrics types stay intact.
  • save_quote_competition is now private with save_quote as its only caller; no other references remain (the persistence/mod.rs:1216 hit is just a doc comment).
  • Removed imports (create_db_search_parameters) are genuinely unused now, and Context is still used in both files, so no dangling/unused imports.
  • The database::PgPool alias is a reasonable way to let shared name the pool type at the crate boundary.

Minor (non-blocking) nit: the module doc at crates/shared/src/quote_storage.rs:1-3 is now slightly stale — it describes the helpers as shared between the QuoteStoring::save implementations, but the module now also owns get_quote/find_quote, which aren't specific to quote competitions. Consider broadening the wording, e.g.:

Persistence helpers for quotes and quote competitions. Shared between the orderbook and autopilot QuoteStoring implementations…

(Couldn't attach this inline since those lines aren't part of the diff.)
· Branch: aryan/be-284-moveduplicated-quoting-logic-to-shared

…icated-quoting-logic-to-shared

Signed-off-by: Aryan Godara <aryangodara03@gmail.com>

# Conflicts:
#	crates/shared/src/quote_storage.rs
@AryanGodara
AryanGodara added this pull request to the merge queue Sep 23, 2026
Merged via the queue into main with commit 4c9a625 Sep 23, 2026
24 checks passed
@AryanGodara
AryanGodara deleted the aryan/be-284-moveduplicated-quoting-logic-to-shared branch September 23, 2026 07:55
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.

2 participants