Skip to content

refactor(aws): use the shared purchase guard in the MemoryDB purchase path - #594

Merged
cristim merged 1 commit into
mainfrom
refactor/10-purchaseguard-memorydb
Oct 20, 2026
Merged

cristim merged 1 commit into
mainfrom
refactor/10-purchaseguard-memorydb

Conversation

@cristim

@cristim cristim commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

Summary

  • The MemoryDB purchase path now calls the shared purchaseguard.Guard and purchaseguard.Recover instead of its own copy of the issue #641 guard, mirroring ElastiCache (refactor(aws): share the purchase idempotency guard, starting with ElastiCache #507, providers/aws/services/elasticache/client.go idempotencyGuard and recoverAlreadyExists).
  • The guard still runs before the purchase call. Valid purchases behave as before.
  • New purchase_guard_test.go covers each guard rule at this path with bounded mocks.

Verification

Part of #10

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved MemoryDB reservation purchases by preventing duplicate purchases when an idempotency token is reused.
    • Added recovery for reservations that already exist, while preserving valid purchase behavior.
    • Improved error handling when reservation lookups fail or return no matching reservation.
    • Purchases without an idempotency token continue without unnecessary reservation lookups.
  • Documentation

    • Updated the changelog to document the MemoryDB purchase behavior changes.

@coderabbitai

coderabbitai Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Essentials
  • Run ID: fe322d45-d8df-4214-883e-2dc486d68600
📥 Commits

Reviewing files that changed from the base of the PR and between 03dcf7a and 652fbcd.

📒 Files selected for processing: 3
  • CHANGELOG.md
  • providers/aws/services/memorydb/client.go
  • providers/aws/services/memorydb/purchase_guard_test.go

Included review availability: This review used your included allowance. 0 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

📝 Walkthrough

MemoryDB now uses the shared purchaseguard helpers for idempotency checks and AlreadyExists recovery. New tests cover lookup ordering, lookup failures, recovery outcomes, and purchases without an idempotency token.

Changes: MemoryDB purchase guard

Layer / File(s) Summary
Shared guard integration
providers/aws/services/memorydb/client.go, CHANGELOG.md
MemoryDB delegates idempotency checks and ReservedNodeAlreadyExistsFault recovery to shared guard helpers. The changelog records this adoption and the lookup-failure wording.
Purchase guard behavior tests
providers/aws/services/memorydb/purchase_guard_test.go
Tests cover empty tokens, lookup-before-purchase ordering, lookup errors, recovery lookup results, and non-recoverable purchase errors.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 652fb

MemoryDB purchases now use the shared idempotency guard and duplicate-recovery helpers. Valid purchases behave as before, and only the lookup-failure wording changes. No merge-blocking risk was found.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: adopting the shared purchase guard in the MemoryDB purchase path.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 77.78% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

… path

Replace the MemoryDB-local copy of the issue #641 idempotency guard and
AlreadyExists recovery with purchaseguard.Guard and Recover, as ElastiCache
already does (#507). The lookup and fault type stay service-specific.

Add purchase_guard_test.go covering each guard rule at this path: no
lookup without a token, lookup before purchase, fail-loud lookup error, and
AlreadyExists recovery limits (re-lookup error or miss, no token, other
purchase errors). Mocks are bounded with Test(t).

Part of #10

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@cristim

cristim commented Oct 20, 2026

Copy link
Copy Markdown
Member Author

Gate note (claude-cc-libs)

Rebased onto main after the overlapping PRs merged. The only conflict was CHANGELOG.md, resolved by keeping both entries. Excluding CHANGELOG.md, git diff origin/main 652fbcd18765a60d577301a0eaba71456c632734 is identical to the reviewed ccbacfbdf9c051945fba507164879073fdec57e7 diff apart from hunk line numbers. Re-verified on the combined tree: vet and go test -count=1 pass.

@cristim
cristim force-pushed the refactor/10-purchaseguard-memorydb branch from ccbacfb to 652fbcd Compare October 20, 2026 16:19
@cristim

cristim commented Oct 20, 2026

Copy link
Copy Markdown
Member Author

Merge gate verdict (claude-cc-libs)

Every gate holds at head 652fbcd:

  • Two independent reviews approve the reviewed head 652fbcd. The branch was then updated from main, and the PR diff is byte-identical to the reviewed diff (compared mechanically).
  • CI is green on 652fbcd, which is the reviewed change on top of current main.
  • 0 unresolved threads; MERGEABLE/CLEAN.

CodeRabbit is waived as a merge gate under the 2026-10-19 decision. Squash-merging.

@cristim
cristim merged commit 00ff5cf into main Oct 20, 2026
15 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