refactor(queries): extract shared bucket-pagination helper for get_ca… - #717
Conversation
…mpaigns_by_category and get_creator_campaigns (Iris-IV#663) Introduce a private helper that encapsulates the identical bucket-traversal algorithm previously duplicated in and . The helper is parameterised by: - total count (derived by each caller from its own domain counter) - bucket size (CATEGORY_CAMPAIGNS_BUCKET_SIZE / CREATOR_CAMPAIGNS_BUCKET_SIZE) - a bucket getter closure (each caller supplies its own storage lookup) Algorithm (unchanged): 1. Jump to the bucket containing the page offset. 2. Walk entries within that bucket starting at the requested position. 3. Collect up to campaigns (capped at LIST_MAX_LIMIT). 4. When the bucket is exhausted, advance position past the bucket boundary and repeat from step 1 with the next bucket. Behaviour preservation: - Public function signatures unchanged — backwards-compatible. - Only src/queries.rs modified. - All 400 workspace tests run; only 2 pre-existing failures remain (test_campaign_update — unrelated Soroban host vector unpacking). - 26 query/bucket/benchmark tests pass with zero regressions. - cargo fmt --check and cargo clippy --all-targets --features testutils both pass cleanly. Closes Iris-IV#663
|
@Nife-tanny Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
good refactor. i diffed the extracted two small notes, neither blocking:
on CI: the two failing tests are test_campaign_update payload assertions that main already fixed (4-tuple event), your branch just predates that. not this PR's fault. gate: resolve the conflicts with main (branch shows DIRTY), CI should go green on its own after that, then this is good to merge. |
) The extraction itself already landed on main via Iris-IV#734; this PR's remaining contribution is the algorithm documentation for the shared helper, plus a code comment pinning the intentional `if let Some` sparse-bucket behavior (a sparse bucket skips instead of panicking - the safer of the two forms, adopted on purpose).
davidmaronio
left a comment
davidmaronio
left a comment
There was a problem hiding this comment.
both notes addressed, the doc comment on the helper and the if-let rationale read well. extraction is still faithful after the rebase. merging.
2 participants
Summary
Extract the identical bucket-traversal algorithm previously duplicated in
get_campaigns_by_categoryandget_creator_campaignsinto a single private helper, parameterised by total count, bucket size, and a bucket-getter closure.Update after review: the extraction itself has since landed on main via #734 (as
get_campaigns_from_buckets, covering both callers). This PR's incremental contribution is now:if let Somesparse-bucket behaviour.Algorithm (unchanged)
limitcampaigns (capped atLIST_MAX_LIMIT).Behaviour notes (reviewer follow-ups)
if let Some(campaign_id) = bucket.get(idx_in_bucket)(the safer form the creator path already used) rather thanunwrap(). A sparse bucket — one whose entry is absent — now skips the slot instead of panicking. This is deliberate so both paths behave identically.get_campaigns_by_categorynow reads the category total before thelimit == 0early return — one extra storage read on a degenerate call, understood and accepted.Validation
src/queries.rsmodified.cargo fmt --checkandcargo clippy --all-targets --features testutils -- -D warningspass cleanly.test_campaign_updatepayload assertions.Closes #663