Files
Crussell/backend/handlers/payments/concurrency_test.go
T
popertots 9ff591fa4e Fix payment review round 2: refund idempotency, pending-resume safety, terminal-completion lock
Refund idempotency (P2):
- RefundRequest gains an optional client idempotency_key: two DISTINCT equal
  partial refunds of one payment no longer collide on the amount-derived key
  (the second was silently swallowed as a dedup)
- Extract resumeManualPendingRefund: resumes a pending refund with the row's
  OWN stored key, so Square's key dedup returns the original refund if the
  prior attempt completed — never issues a second
- (payment, amount) pending fallback: when the exact-key lookup misses (admin
  reopened the modal, new UUID), resume the matching pending row instead of
  creating a second pending row the sweep would double-process
- 409 in-flight guard: if a pending refund exists for the payment but no
  same-amount row matches, reject a different-amount refund (money state at
  Square is unknown — no new refund is safe until it resolves)
- Frontend (EditBookingModal): UUID per refund attempt, reused on retry,
  mirroring the tip flow

Terminal completion (P3):
- GetCheckoutStatus serializes on pg_advisory_lock('crussell:terminal:' ||
  SquarePayID) on a pinned connection — concurrent polls of the same checkout
  can no longer both pass the dedup SELECT and race the UNIQUE constraint

Card-on-file / doc-only:
- Document why CreateCardOnFile is NOT rolled back on payment failure
  (deterministic sha256 retry returns the same card; deletion breaks it)
- Document HasCompletedPayment's deliberate 'tip' exclusion

Regression tests:
- TestRefund_TwoEqualPartialRefunds_ClientKeyDisambiguates
- TestRefund_PendingResume_NewKeyAfterModalReopen (proves stored-key resume)
- TestRefund_PendingResume_DifferentAmountRejected (409 + no second row)
- TestRefund_GuardCountsPendingRefunds updated: 400 -> 409 (in-flight guard
  fires first — strictly safer, blocks before any Square attempt)
- TestGetCheckoutStatus_ConcurrentPolls_SingleRecord (real two-goroutine race)
2026-08-22 00:34:49 +01:00

422 lines
14 KiB
Go
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
//go:build test && dev
package payments
import (
"context"
"net/http"
"net/http/httptest"
"sync"
"testing"
"time"
"crussell/db"
"crussell/internal/square"
"crussell/testutils"
"crussell/testutils/fixtures"
"crussell/testutils/jwt"
"github.com/go-chi/chi/v5"
)
// slowCreatePaymentClient delays the Square charge so each handler holds its
// advisory lock long enough that a concurrent same-key request would race it
// without the lock (two goroutines both reading "no record", both charging).
type slowCreatePaymentClient struct {
square.SquareClient
delay time.Duration
}
func (c *slowCreatePaymentClient) CreatePayment(ctx context.Context, req square.CreatePaymentReq) (*square.PaymentResult, error) {
time.Sleep(c.delay)
return c.SquareClient.CreatePayment(ctx, req)
}
// cleanupConcurrentTestRows deletes the rows a concurrency test committed at
// pool level. These tests must COMMIT their setup so the advisory locks work
// across independent connections, which leaves committed rows in the shared
// test DB — without cleanup they leak into parallel tests (e.g. GetGiftCards
// counts gift_cards/user_giftcard_balances globally). FK-safe deletion order.
func cleanupConcurrentTestRows(t *testing.T, pool context.Context, userID, bookingID string) {
t.Helper()
t.Cleanup(func() {
var gcIDs []string
rows, err := db.Conn.Query(pool, `SELECT id FROM gift_cards WHERE created_by = $1`, userID)
if err == nil {
for rows.Next() {
var id string
if rows.Scan(&id) == nil {
gcIDs = append(gcIDs, id)
}
}
rows.Close()
}
for _, gcID := range gcIDs {
_, _ = db.Conn.Exec(pool, `DELETE FROM gift_card_transactions WHERE gift_card_id = $1`, gcID)
}
_, _ = db.Conn.Exec(pool, `DELETE FROM user_giftcard_balances WHERE user_id = $1`, userID)
if bookingID != "" {
_, _ = db.Conn.Exec(pool, `DELETE FROM payments WHERE booking_id = $1`, bookingID)
_, _ = db.Conn.Exec(pool, `DELETE FROM booking_services WHERE booking_id = $1`, bookingID)
_, _ = db.Conn.Exec(pool, `DELETE FROM bookings WHERE id = $1`, bookingID)
}
_, _ = db.Conn.Exec(pool, `DELETE FROM payments WHERE created_by = $1 OR idempotency_key LIKE 'concurrent-%'`, userID)
for _, gcID := range gcIDs {
_, _ = db.Conn.Exec(pool, `DELETE FROM gift_cards WHERE id = $1`, gcID)
}
_, _ = db.Conn.Exec(pool, `DELETE FROM user_saved_cards WHERE user_id = $1`, userID)
_, _ = db.Conn.Exec(pool, `DELETE FROM users WHERE id = $1`, userID)
})
}
// TestBuyGiftCard_ConcurrentSameKey_SingleRecord proves the BuyGiftCard
// advisory lock (giftcards.go): two goroutines POSTing the same idempotency key
// must produce exactly ONE payment record and ONE funded gift card — never
// 2× value for 1 charge. Without the lock, both goroutines pass the
// idempotency check, both reuse/insert pending records, and both fund the card.
func TestBuyGiftCard_ConcurrentSameKey_SingleRecord(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, err := fixtures.CreateTestUser(tx)
if err != nil {
t.Fatalf("failed to create user: %v", err)
}
token := jwt.GenerateUserToken(userID)
cleanupConcurrentTestRows(t, context.Background(), userID, "")
// Commit the setup so both goroutines operate at pool level — the advisory
// locks only serialize across independent connections, and a per-test tx
// would route both sides through a single shared connection.
innerTx := db.TxFromContext(ctx)
if innerTx == nil {
t.Fatal("no transaction in context")
}
if err := innerTx.Commit(ctx); err != nil {
t.Fatalf("failed to commit setup tx: %v", err)
}
origClient := SquareClient
slow := &slowCreatePaymentClient{SquareClient: square.NewDevClient(), delay: 300 * time.Millisecond}
SquareClient = slow
defer func() { SquareClient = origClient }()
pool := context.Background()
key := "buy-gc-concurrent-same-key"
reqBody := map[string]interface{}{
"amount": 2000,
"recipient_type": "self",
"new_card_token": "cnon:concurrent-card",
"idempotency_key": key,
}
var wg sync.WaitGroup
startBoth := make(chan struct{})
recs := make([]*httptest.ResponseRecorder, 2)
for i := 0; i < 2; i++ {
wg.Add(1)
go func(idx int) {
defer wg.Done()
<-startBoth
recs[idx] = makePaymentRequest(BuyGiftCard, "POST", "/api/user/giftcards/buy", reqBody, token, pool)
}(i)
}
close(startBoth)
wg.Wait()
// Both requests must succeed — the lock serializes them and the second
// finds the completed record (idempotent dedup, HTTP 200), so neither
// double-charges nor errors.
for i, rec := range recs {
if rec.Code != http.StatusCreated && rec.Code != http.StatusOK {
t.Errorf("request %d expected 201 (create) or 200 (dedup), got %d. body: %s", i, rec.Code, rec.Body.String())
}
}
// Exactly one payment record for this key.
var payCount int
err = db.Conn.QueryRow(pool,
`SELECT COUNT(*) FROM payments WHERE idempotency_key = $1`, key).Scan(&payCount)
if err != nil {
t.Fatalf("failed to count payments: %v", err)
}
if payCount != 1 {
t.Errorf("expected exactly 1 payment record, got %d (double-charge!)", payCount)
}
// Exactly one funded gift card for this user's self-purchase.
var gcCount int
err = db.Conn.QueryRow(pool,
`SELECT COUNT(*) FROM gift_cards WHERE total_funds_added = 20.00 AND created_by = $1`, userID).Scan(&gcCount)
if err != nil {
t.Fatalf("failed to count gift cards: %v", err)
}
if gcCount != 1 {
t.Errorf("expected exactly 1 funded gift card, got %d (2× value!)", gcCount)
}
// User balance credited exactly once.
var balance float64
err = db.Conn.QueryRow(pool,
`SELECT balance FROM user_giftcard_balances WHERE user_id = $1`, userID).Scan(&balance)
if err != nil {
t.Fatalf("failed to query balance: %v", err)
}
if balance != 20.00 {
t.Errorf("expected balance 20.00, got %.2f (double-credit!)", balance)
}
}
// TestTipPayment_ConcurrentSameKey_SingleRecord proves the tip advisory lock
// (handlers.go): two goroutines POSTing the same booking with the same key must
// produce exactly ONE tip payment record — never two charges for one booking.
func TestTipPayment_ConcurrentSameKey_SingleRecord(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, err := fixtures.CreateTestUser(tx)
if err != nil {
t.Fatalf("failed to create user: %v", err)
}
serviceID, err := fixtures.CreateTestService(tx)
if err != nil {
t.Fatalf("failed to create service: %v", err)
}
start := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)
bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, start)
if err != nil {
t.Fatalf("failed to create booking: %v", err)
}
if _, err := fixtures.CreateTestPayment(tx, bookingID, 5000.00, "online_square", "full", "completed"); err != nil {
t.Fatalf("failed to create prior payment: %v", err)
}
token := jwt.GenerateUserToken(userID)
cleanupConcurrentTestRows(t, context.Background(), userID, bookingID)
innerTx := db.TxFromContext(ctx)
if innerTx == nil {
t.Fatal("no transaction in context")
}
if err := innerTx.Commit(ctx); err != nil {
t.Fatalf("failed to commit setup tx: %v", err)
}
origClient := SquareClient
slow := &slowCreatePaymentClient{SquareClient: square.NewDevClient(), delay: 300 * time.Millisecond}
SquareClient = slow
defer func() { SquareClient = origClient }()
pool := context.Background()
key := "tip-concurrent-same-key"
cardToken := "cnon:concurrent-tip-card"
reqBody := CreateTipPaymentRequest{
Amount: 500,
NewCardToken: &cardToken,
IdempotencyKey: key,
}
var wg sync.WaitGroup
startBoth := make(chan struct{})
recs := make([]*httptest.ResponseRecorder, 2)
for i := 0; i < 2; i++ {
wg.Add(1)
go func(idx int) {
defer wg.Done()
<-startBoth
recs[idx] = makePaymentRequest(CreateTipPayment, "POST", "/api/bookings/"+bookingID+"/tip", reqBody, token, pool)
}(i)
}
close(startBoth)
wg.Wait()
for i, rec := range recs {
if rec.Code != http.StatusOK {
t.Errorf("request %d expected 200, got %d. body: %s", i, rec.Code, rec.Body.String())
}
}
var tipCount int
err = db.Conn.QueryRow(pool,
`SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND payment_type = 'tip' AND idempotency_key = $2`, bookingID, key).Scan(&tipCount)
if err != nil {
t.Fatalf("failed to count tip payments: %v", err)
}
if tipCount != 1 {
t.Errorf("expected exactly 1 tip payment record, got %d (double-charge!)", tipCount)
}
}
// TestBookingPayment_ConcurrentSameKey_SingleRecord proves the booking-payment
// advisory lock (handlers.go): two goroutines paying the same booking with the
// same key must produce exactly ONE payment record.
func TestBookingPayment_ConcurrentSameKey_SingleRecord(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, bookingID, _ := setupTestDataPast(t, ctx, tx)
token := jwt.GenerateUserToken(userID)
cleanupConcurrentTestRows(t, context.Background(), userID, bookingID)
innerTx := db.TxFromContext(ctx)
if innerTx == nil {
t.Fatal("no transaction in context")
}
if err := innerTx.Commit(ctx); err != nil {
t.Fatalf("failed to commit setup tx: %v", err)
}
origClient := SquareClient
slow := &slowCreatePaymentClient{SquareClient: square.NewDevClient(), delay: 300 * time.Millisecond}
SquareClient = slow
defer func() { SquareClient = origClient }()
pool := context.Background()
key := "booking-pay-concurrent-same-key"
cardToken := "cnon:concurrent-booking-card"
reqBody := CreateBookingPaymentRequest{
Amount: 5000,
PaymentType: "full",
NewCardToken: &cardToken,
IdempotencyKey: key,
}
var wg sync.WaitGroup
startBoth := make(chan struct{})
recs := make([]*httptest.ResponseRecorder, 2)
for i := 0; i < 2; i++ {
wg.Add(1)
go func(idx int) {
defer wg.Done()
<-startBoth
recs[idx] = makePaymentRequest(CreateBookingPayment, "POST", "/api/bookings/"+bookingID+"/payment", reqBody, token, pool)
}(i)
}
close(startBoth)
wg.Wait()
for i, rec := range recs {
if rec.Code != http.StatusOK {
t.Errorf("request %d expected 200, got %d. body: %s", i, rec.Code, rec.Body.String())
}
}
var payCount int
err := db.Conn.QueryRow(pool,
`SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND idempotency_key = $2`, bookingID, key).Scan(&payCount)
if err != nil {
t.Fatalf("failed to count payments: %v", err)
}
if payCount != 1 {
t.Errorf("expected exactly 1 payment record, got %d (double-charge!)", payCount)
}
}
// completedCheckoutClient forces GetCheckout to return a fixed COMPLETED
// payment for any checkout id, deterministically exercising the terminal
// completion dedup+insert path (the mock's real async goroutine would be
// non-deterministic in a race test).
type completedCheckoutClient struct {
square.SquareClient
result *square.PaymentResult
}
func (c *completedCheckoutClient) GetCheckout(ctx context.Context, checkoutID string) (*square.PaymentResult, error) {
return c.result, nil
}
// TestGetCheckoutStatus_ConcurrentPolls_SingleRecord proves the P3 fix: the
// terminal-completion path serializes on a per-payment advisory lock, so two
// concurrent polls of the same completed checkout produce exactly ONE payment
// record. Without the lock, both goroutines pass the dedup SELECT, both INSERT,
// and the second dies on the idempotency_key UNIQUE constraint after the
// customer already paid.
func TestGetCheckoutStatus_ConcurrentPolls_SingleRecord(t *testing.T) {
ctx, tx := testutils.SetupTestTx(t)
userID, err := fixtures.CreateTestUser(tx)
if err != nil {
t.Fatalf("failed to create user: %v", err)
}
serviceID, err := fixtures.CreateTestService(tx)
if err != nil {
t.Fatalf("failed to create service: %v", err)
}
start := time.Date(2099, 12, 31, 10, 0, 0, 0, time.UTC)
bookingID, err := fixtures.CreateTestBookingAtTime(tx, userID, serviceID, start)
if err != nil {
t.Fatalf("failed to create booking: %v", err)
}
cleanupConcurrentTestRows(t, context.Background(), userID, bookingID)
innerTx := db.TxFromContext(ctx)
if innerTx == nil {
t.Fatal("no transaction in context")
}
if err := innerTx.Commit(ctx); err != nil {
t.Fatalf("failed to commit setup tx: %v", err)
}
origClient := SquareClient
SquareClient = &completedCheckoutClient{
SquareClient: square.NewDevClient(),
result: &square.PaymentResult{
ID: "pay_terminal_race",
Status: "COMPLETED",
Amount: 5000,
Fees: 88,
SquarePayID: "pay_terminal_race",
CardBrand: "VISA",
CardLast4: "4242",
ReceiptURL: "https://receipt.example/pay_terminal_race",
EntryMethod: "EMV",
LocationID: "loc",
ReferenceID: bookingID,
CreatedAt: "2026-07-31T00:00:00Z",
UpdatedAt: "2026-07-31T00:00:00Z",
},
}
defer func() { SquareClient = origClient }()
pool := context.Background()
checkoutID := "abcd1234ef56" // 12 hex chars, passes the checkout-id validation
var wg sync.WaitGroup
startBoth := make(chan struct{})
recs := make([]*httptest.ResponseRecorder, 2)
for i := 0; i < 2; i++ {
wg.Add(1)
go func(idx int) {
defer wg.Done()
<-startBoth
req := httptest.NewRequest("GET", "/api/checkout/"+checkoutID+"/status?booking_id="+bookingID, nil)
rctx := chi.NewRouteContext()
rctx.URLParams.Add("checkout_id", checkoutID)
req = req.WithContext(context.WithValue(req.Context(), chi.RouteCtxKey, rctx))
w := httptest.NewRecorder()
GetCheckoutStatus(w, req)
recs[idx] = w
}(i)
}
close(startBoth)
wg.Wait()
for i, rec := range recs {
if rec.Code != http.StatusOK {
t.Errorf("request %d expected 200, got %d. body: %s", i, rec.Code, rec.Body.String())
}
}
// Exactly one completed terminal payment record for this booking.
var payCount int
err = db.Conn.QueryRow(pool,
`SELECT COUNT(*) FROM payments WHERE booking_id = $1 AND payment_method = 'in_person_card' AND square_payment_id = $2`,
bookingID, "pay_terminal_race").Scan(&payCount)
if err != nil {
t.Fatalf("failed to count terminal payments: %v", err)
}
if payCount != 1 {
t.Errorf("expected exactly 1 terminal payment record, got %d (double-record race!)", payCount)
}
}