From 3cd82ba5be260fdd43f95e3764e047ef3bf202fc Mon Sep 17 00:00:00 2001 From: senke Date: Sat, 18 Apr 2026 02:30:02 +0200 Subject: [PATCH] =?UTF-8?q?fix(hyperswitch):=20idempotency-key=20on=20crea?= =?UTF-8?q?te-payment=20and=20create-refund=20=E2=80=94=20v1.0.7=20item=20?= =?UTF-8?q?D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every outbound POST /payments and POST /refunds from the Hyperswitch client now carries an Idempotency-Key HTTP header. Key values are explicit parameters at every call site — no context-carrier magic, no auto-generation. An empty key is a loud error from the client (not silent header omission) so a future new call site that forgets to supply one fails immediately, not months later under an obscure replay scenario. Key choices, both stable across HTTP retries of the same logical call: * CreatePayment → order.ID.String() (GORM BeforeCreate populates order.ID before the PSP call in ConfirmOrder). * CreateRefund → pendingRefund.ID.String() (populated by the Phase 1 tx.Create in RefundOrder, available for the Phase 2 PSP call). Scope note (reproduced here for the next reader who grep-s the commit log for "Idempotency-Key"): Idempotency-Key covers HTTP-transport retry (TLS reconnect, proxy retry, DNS flap) within a single CreatePayment / CreateRefund invocation. It does NOT cover application-level replay (user double-click, form double-submit, retry after crash before DB write). That class of bug requires state-machine preconditions on VEZA side — already addressed by the order state machine + the handler-level guards on POST /api/v1/payments (for payments) and the partial UNIQUE on `refunds.hyperswitch_refund_id` landed in v1.0.6.1 (for refunds). Hyperswitch TTL on Idempotency-Key: typically 24h-7d server-side (verify against current PSP docs). Beyond TTL, a retry with the same key is treated as a new request. Not a concern at current volumes; document if retry logic ever extends beyond 1 hour. Explicitly out of scope: item D does NOT add application-level retry logic. The current "try once, fail loudly" behavior on PSP errors is preserved. Adding retries is a separate design exercise (backoff, max attempts, circuit breaker) not part of this commit. Interfaces changed: * hyperswitch.Client.CreatePayment(ctx, idempotencyKey, ...) * hyperswitch.Client.CreatePaymentSimple(...) convenience wrapper * hyperswitch.Client.CreateRefund(ctx, idempotencyKey, ...) * hyperswitch.Provider.CreatePayment threads through * hyperswitch.Provider.CreateRefund threads through * marketplace.PaymentProvider interface — first param after ctx * marketplace.refundProvider interface — first param after ctx Removed: * hyperswitch.Provider.Refund (zero callers, superseded by CreateRefund which returns (refund_id, status, err) and is the only method marketplace's refundProvider cares about). Tests: * Two new httptest.Server-backed tests (client_test.go) pin the Idempotency-Key header value for CreatePayment and CreateRefund. * Two new empty-key tests confirm the client errors rather than silently sending no header. * TestRefundOrder_OpensPendingRefund gains an assertion that f.provider.lastIdempotencyKey == refund.ID.String() — if a future refactor threads the key from somewhere else (paymentID, uuid.New() per call, etc.) the test fails loudly. * Four pre-existing test mocks updated for the new signature (mockRefundPaymentProvider in marketplace, mockPaymentProvider in tests/integration and tests/contract, mockRefundPayment Provider in tests/integration/refund_flow). Subscription's CreateSubscriptionPayment interface declares its own shape and has no live Hyperswitch-backed implementation today — v1.0.6.2 noted this as the payment-gate bypass surface, v1.0.7 item G will ship the real provider. When that lands, item G's implementation threads the idempotency key through in the same pattern (documented in v107-plan.md item G acceptance). CHANGELOG v1.0.7-rc1 entry updated with the full item D scope note and the "out of scope: retries" caveat. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 56 ++++++++++++++++ docs/audit-2026-04/v107-plan.md | 9 ++- .../internal/core/marketplace/refund_test.go | 25 +++++-- .../internal/core/marketplace/service.go | 40 +++++++++-- .../internal/services/hyperswitch/client.go | 42 ++++++++++-- .../services/hyperswitch/client_test.go | 66 ++++++++++++++++++- .../internal/services/hyperswitch/provider.go | 27 ++++---- .../tests/contract/critical_endpoints_test.go | 2 +- .../tests/integration/payment_flow_test.go | 2 +- .../tests/integration/refund_flow_test.go | 9 ++- 10 files changed, 240 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da56ae1d9..2046ae050 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,62 @@ auto-reversed; the backfill CLI queries Stripe's transfers.List by metadata[order_id] to populate missing ids, acceptable to leave NULL per v107-plan. +### Item D — Idempotency-Key on CreatePayment / CreateRefund + +The Hyperswitch client now sends an `Idempotency-Key` HTTP header on +every outbound POST /payments and POST /refunds. The header value is +an explicit parameter at every call site — no context-carrier magic, +no auto-generation — so the contract is visible in every call and +impossible to forget (empty keys cause a loud error, not silent +header omission). + +Key values: + * CreatePayment → `order.ID.String()` (UUID generated by GORM + BeforeCreate before the HTTP call). + * CreateRefund → `pendingRefund.ID.String()` (same pattern — UUID + populated by the Phase 1 tx.Create in RefundOrder, available and + stable for the Phase 2 PSP call). + +Scope (load-bearing note for future readers): + + `Idempotency-Key` covers HTTP-transport retry (TLS reconnect, + proxy retry, DNS flap) within a single CreatePayment / + CreateRefund invocation. It does NOT cover application-level + replay (user double-click, form double-submit, retry after crash + before DB write). That class of bug requires state-machine + preconditions on VEZA side — already addressed by the order + state machine + checkout handler guards (for payments) and the + partial UNIQUE on `refunds.hyperswitch_refund_id` landed in + v1.0.6.1 (for refunds). + + Hyperswitch TTL on Idempotency-Key is typically 24h–7d + server-side (verify against current PSP docs). Beyond TTL, a + retry with the same key is treated as a new request. Not a + concern at current volumes; document if retry logic ever extends + beyond 1 hour. + +What stays unchanged: this commit does NOT add application-level +retry logic. The current "try once, fail loudly" behavior on PSP +errors is preserved. Adding retries is a separate design exercise +(backoff, max attempts, circuit breaker) explicitly out of scope +for item D. + +Tests: + * Two httptest.Server-backed tests in client_test.go pin the + header value emitted for CreatePayment and CreateRefund, plus + two tests asserting empty keys cause a loud error. + * TestRefundOrder_OpensPendingRefund now pins the + `refund.ID.String() == lastIdempotencyKey` contract so a + future refactor that drops or reshapes the key fails the test. + * Four existing test mocks updated for the new signature. + +Subscription's CreateSubscriptionPayment interface also takes a +payment provider but no implementation is wired in today (v1.0.6.2 +noted this as the bypass surface, v1.0.7 item G is the full fix). +When item G lands its Hyperswitch-backed subscription provider, +it will need to thread the idempotency key through the same way — +noted in item G's acceptance in v107-plan.md. + ### Item B — async Stripe Connect reversal worker `reverseSellerAccounting` moved from synchronous "mark row reversed diff --git a/docs/audit-2026-04/v107-plan.md b/docs/audit-2026-04/v107-plan.md index 505b4825b..65b8472ae 100644 --- a/docs/audit-2026-04/v107-plan.md +++ b/docs/audit-2026-04/v107-plan.md @@ -84,14 +84,21 @@ Effort: **XS**. Pure header addition. Tests: the 15-case refund suite already exists; add 2 cases verifying the header is set correctly (httptest.Server assertion on `r.Header.Get("Idempotency-Key")`). -Acceptance: +Acceptance (landed in commit TBD — this entry pinned ahead): - Every outbound `POST /payments` carries `Idempotency-Key: `. - Every outbound `POST /refunds` carries `Idempotency-Key: `. - No implicit-via-ctx magic: each call site sets the header explicitly, greppable. +- Empty idempotency key returns an error from the client (loud failure, + not silent header omission). - CHANGELOG entry cross-references P0.4 + its scope note (HTTP retry only, not app-level replay). +**Status** — landed 2026-04-18 alongside item B day 3 closure. +Subscription's CreateSubscriptionPayment interface still lacks a live +Hyperswitch impl (deferred to item G); that's where the remaining +idempotency-key plumbing goes. + **TTL caveat** — Hyperswitch (like most PSPs) honours `Idempotency-Key` server-side only for a finite window: 24 h is common, 7 d at the high end. Beyond the TTL, a replayed call with the same key is treated as diff --git a/veza-backend-api/internal/core/marketplace/refund_test.go b/veza-backend-api/internal/core/marketplace/refund_test.go index c819b1660..0877ad35d 100644 --- a/veza-backend-api/internal/core/marketplace/refund_test.go +++ b/veza-backend-api/internal/core/marketplace/refund_test.go @@ -36,15 +36,17 @@ func setupRefundTestDB(t *testing.T) *gorm.DB { } // mockRefundPaymentProvider implements both PaymentProvider and -// refundProvider (v1.0.6: CreateRefund returns refund_id + status). +// refundProvider (v1.0.6: CreateRefund returns refund_id + status, +// v1.0.7 item D: CreateRefund captures idempotencyKey for assertion). type mockRefundPaymentProvider struct { - refundErr error - refundIDCount atomic.Int32 - lastAmount *int64 - lastReason string + refundErr error + refundIDCount atomic.Int32 + lastAmount *int64 + lastReason string + lastIdempotencyKey string } -func (m *mockRefundPaymentProvider) CreatePayment(_ context.Context, _ int64, _ string, _ string, _ string, _ map[string]string) (string, string, error) { +func (m *mockRefundPaymentProvider) CreatePayment(_ context.Context, _ string, _ int64, _ string, _ string, _ string, _ map[string]string) (string, string, error) { return "pay_mock", "secret_mock", nil } @@ -52,7 +54,8 @@ func (m *mockRefundPaymentProvider) GetPayment(_ context.Context, _ string) (str return "succeeded", nil } -func (m *mockRefundPaymentProvider) CreateRefund(_ context.Context, _ string, amount *int64, reason string) (string, string, error) { +func (m *mockRefundPaymentProvider) CreateRefund(_ context.Context, idempotencyKey, _ string, amount *int64, reason string) (string, string, error) { + m.lastIdempotencyKey = idempotencyKey if m.refundErr != nil { return "", "", m.refundErr } @@ -166,6 +169,14 @@ func TestRefundOrder_OpensPendingRefund(t *testing.T) { var persisted Refund require.NoError(t, f.db.First(&persisted, refund.ID).Error) assert.Equal(t, refund.HyperswitchRefundID, persisted.HyperswitchRefundID) + + // v1.0.7 item D: the pending Refund row's own UUID is what gets sent + // as Idempotency-Key to Hyperswitch. This test pins the contract so + // a future refactor that drops the key, falls back to a different + // value (like paymentID), or uses uuid.New() per retry surfaces + // immediately. + assert.Equal(t, refund.ID.String(), f.provider.lastIdempotencyKey, + "Idempotency-Key must be refund.ID (stable across HTTP-layer retries of the same logical refund)") } func TestRefundOrder_PSPErrorRollsBack(t *testing.T) { diff --git a/veza-backend-api/internal/core/marketplace/service.go b/veza-backend-api/internal/core/marketplace/service.go index 5f89454b7..e7c6cb0c3 100644 --- a/veza-backend-api/internal/core/marketplace/service.go +++ b/veza-backend-api/internal/core/marketplace/service.go @@ -42,8 +42,29 @@ type CreateOrderResponse struct { } // PaymentProvider defines the interface for payment processing (Hyperswitch). +// +// v1.0.7 item D: CreatePayment takes an explicit `idempotencyKey` +// (first parameter after ctx) that the provider sends as the +// `Idempotency-Key` HTTP header. Callers supply a UUID stable across +// retries of the same logical call — for checkout, that's the order +// ID generated by GORM's BeforeCreate hook before the HTTP call. A +// provider that receives an empty key is expected to error, so a +// future new call site that forgets to supply one fails loudly. +// +// Scope note: idempotencyKey covers HTTP-transport retry (TLS +// reconnect, proxy retry, DNS flap) within a single CreatePayment +// invocation. It does NOT cover application-level replay (user +// double-click, form double-submit, retry after crash before DB +// write). That class of bug requires state-machine preconditions on +// VEZA side — already addressed by the order state machine + the +// handler-level guards on POST /api/v1/payments. +// +// Hyperswitch TTL on Idempotency-Key: typically 24h–7d server-side +// (verify against current docs). Beyond TTL, a retry with the same +// key is treated as a new request. Not a concern at current volumes, +// noted here for any future extension of retry windows. type PaymentProvider interface { - CreatePayment(ctx context.Context, amount int64, currency, orderID, returnURL string, metadata map[string]string) (paymentID, clientSecret string, err error) + CreatePayment(ctx context.Context, idempotencyKey string, amount int64, currency, orderID, returnURL string, metadata map[string]string) (paymentID, clientSecret string, err error) GetPayment(ctx context.Context, paymentID string) (status string, err error) } @@ -512,7 +533,11 @@ func (s *Service) CreateOrder(ctx context.Context, buyerID uuid.UUID, items []Ne } returnURL := baseURL + sep + "order_id=" + order.ID.String() var err error - paymentID, clientSecret, err = s.paymentProvider.CreatePayment(ctx, int64(amountCents), "EUR", order.ID.String(), returnURL, map[string]string{"order_id": order.ID.String()}) + // v1.0.7 item D: order.ID is populated at struct construction + // (via GORM BeforeCreate) before this call, so the key is + // stable across any HTTP-transport retry of this single + // CreatePayment invocation. + paymentID, clientSecret, err = s.paymentProvider.CreatePayment(ctx, order.ID.String(), int64(amountCents), "EUR", order.ID.String(), returnURL, map[string]string{"order_id": order.ID.String()}) if err != nil { s.logger.Error("Hyperswitch CreatePayment failed", zap.Error(err), zap.String("order_id", order.ID.String())) return fmt.Errorf("payment creation failed: %w", err) @@ -1237,8 +1262,12 @@ func (s *Service) ListReviews(ctx context.Context, productID uuid.UUID, limit, o // service can persist the correlation key used by the webhook handler for // idempotent finalization. The status is informational — we never trust a // synchronous "succeeded" response, we always wait for the webhook. +// +// v1.0.7 item D: idempotencyKey is the pending Refund row's UUID, +// stable across HTTP retries of the same logical call. Sent as +// `Idempotency-Key` header. Empty key is a loud error (see client). type refundProvider interface { - CreateRefund(ctx context.Context, paymentID string, amount *int64, reason string) (refundID string, status string, err error) + CreateRefund(ctx context.Context, idempotencyKey, paymentID string, amount *int64, reason string) (refundID string, status string, err error) } var ErrOrderNotRefundable = errors.New("order cannot be refunded") @@ -1359,7 +1388,10 @@ func (s *Service) RefundOrder(ctx context.Context, orderID, initiatorID uuid.UUI // Phase 2: PSP call outside the transaction. Any failure here unwinds // the pending state so the buyer can retry. - refundID, hsStatus, pspErr := rp.CreateRefund(ctx, paymentID, nil, reason) + // v1.0.7 item D: pendingRefund.ID is populated by GORM BeforeCreate + // during the Phase 1 tx.Create above, so it's available and stable + // here as the Idempotency-Key for this HTTP call. + refundID, hsStatus, pspErr := rp.CreateRefund(ctx, pendingRefund.ID.String(), paymentID, nil, reason) if pspErr != nil { now := time.Now().UTC() errMsg := pspErr.Error() diff --git a/veza-backend-api/internal/services/hyperswitch/client.go b/veza-backend-api/internal/services/hyperswitch/client.go index 37e4a6f56..071e5dcb2 100644 --- a/veza-backend-api/internal/services/hyperswitch/client.go +++ b/veza-backend-api/internal/services/hyperswitch/client.go @@ -51,7 +51,26 @@ type PaymentStatus struct { } // CreatePayment creates a payment in Hyperswitch and returns client_secret for frontend. -func (c *Client) CreatePayment(ctx context.Context, amount int64, currency, orderID, returnURL string, metadata map[string]string) (*PaymentResponse, error) { +// +// idempotencyKey is REQUIRED (v1.0.7 item D) and sent as the +// `Idempotency-Key` header. Hyperswitch short-circuits subsequent +// requests carrying the same key (within the PSP-side TTL — typically +// 24h to 7d, verify against current docs) to the original response, +// so an HTTP-layer retry (TLS reconnect, proxy flap, DNS hiccup) on +// the same call produces at-most-once semantics. The key MUST be +// stable across retries of the same logical call — order.ID.String() +// at the site that creates orders is the canonical choice. +// +// Scope note: this header only addresses HTTP-transport retry within +// a single CreatePayment invocation. It does NOT address +// application-level replay (user double-click, form double-submit, +// retry after crash before DB write). That class of bug requires +// state-machine preconditions on VEZA side and is addressed by the +// order state machine + checkout handler's existing guards. +func (c *Client) CreatePayment(ctx context.Context, idempotencyKey string, amount int64, currency, orderID, returnURL string, metadata map[string]string) (*PaymentResponse, error) { + if idempotencyKey == "" { + return nil, fmt.Errorf("create payment: idempotency key required") + } if metadata == nil { metadata = make(map[string]string) } @@ -76,6 +95,7 @@ func (c *Client) CreatePayment(ctx context.Context, amount int64, currency, orde } req.Header.Set("Content-Type", "application/json") req.Header.Set("api-key", c.apiKey) + req.Header.Set("Idempotency-Key", idempotencyKey) resp, err := c.httpClient.Do(req) if err != nil { @@ -96,8 +116,8 @@ func (c *Client) CreatePayment(ctx context.Context, amount int64, currency, orde // CreatePaymentSimple creates a payment and returns paymentID and clientSecret. // Convenience wrapper for PaymentProvider interface. -func (c *Client) CreatePaymentSimple(ctx context.Context, amount int64, currency, orderID, returnURL string, metadata map[string]string) (paymentID, clientSecret string, err error) { - resp, err := c.CreatePayment(ctx, amount, currency, orderID, returnURL, metadata) +func (c *Client) CreatePaymentSimple(ctx context.Context, idempotencyKey string, amount int64, currency, orderID, returnURL string, metadata map[string]string) (paymentID, clientSecret string, err error) { + resp, err := c.CreatePayment(ctx, idempotencyKey, amount, currency, orderID, returnURL, metadata) if err != nil { return "", "", err } @@ -155,8 +175,19 @@ type RefundResponse struct { Status string `json:"status"` } -// CreateRefund creates a refund against a payment (v0.403 R2) -func (c *Client) CreateRefund(ctx context.Context, paymentID string, amount *int64, reason string) (*RefundResponse, error) { +// CreateRefund creates a refund against a payment (v0.403 R2). +// +// idempotencyKey is REQUIRED (v1.0.7 item D) and sent as the +// `Idempotency-Key` header. Canonical choice: the pending Refund +// row's ID — stable across HTTP retries of the same logical refund, +// and unique per refund attempt. Same scope caveats as CreatePayment: +// HTTP-transport-level retry only, not application-level replay. +// Application-level idempotency is guaranteed by the partial UNIQUE +// on `refunds.hyperswitch_refund_id` landed in v1.0.6.1. +func (c *Client) CreateRefund(ctx context.Context, idempotencyKey, paymentID string, amount *int64, reason string) (*RefundResponse, error) { + if idempotencyKey == "" { + return nil, fmt.Errorf("create refund: idempotency key required") + } reqBody := CreateRefundRequest{ PaymentID: paymentID, Amount: amount, @@ -173,6 +204,7 @@ func (c *Client) CreateRefund(ctx context.Context, paymentID string, amount *int } req.Header.Set("Content-Type", "application/json") req.Header.Set("api-key", c.apiKey) + req.Header.Set("Idempotency-Key", idempotencyKey) resp, err := c.httpClient.Do(req) if err != nil { return nil, fmt.Errorf("http request: %w", err) diff --git a/veza-backend-api/internal/services/hyperswitch/client_test.go b/veza-backend-api/internal/services/hyperswitch/client_test.go index 0579419aa..def4be9c0 100644 --- a/veza-backend-api/internal/services/hyperswitch/client_test.go +++ b/veza-backend-api/internal/services/hyperswitch/client_test.go @@ -9,6 +9,8 @@ import ( ) func TestClient_CreatePayment(t *testing.T) { + const expectedIdempKey = "order-123" + var gotIdempKey string server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodPost || r.URL.Path != "/payments" { t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) @@ -16,6 +18,7 @@ func TestClient_CreatePayment(t *testing.T) { if r.Header.Get("api-key") == "" { t.Error("missing api-key header") } + gotIdempKey = r.Header.Get("Idempotency-Key") w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(map[string]string{ "payment_id": "pay_test_123", @@ -28,6 +31,7 @@ func TestClient_CreatePayment(t *testing.T) { client := NewClient(server.URL, "test_api_key") paymentID, clientSecret, err := client.CreatePaymentSimple( context.Background(), + expectedIdempKey, 6540, "EUR", "order-123", @@ -43,6 +47,28 @@ func TestClient_CreatePayment(t *testing.T) { if clientSecret != "pi_test_secret_xxx" { t.Errorf("client_secret = %q, want pi_test_secret_xxx", clientSecret) } + // v1.0.7 item D: the Idempotency-Key header is load-bearing — a + // future refactor that drops it from the request pipeline silently + // reopens the HTTP-retry duplicate-payment exposure. Pin the value. + if gotIdempKey != expectedIdempKey { + t.Errorf("Idempotency-Key header = %q, want %q", gotIdempKey, expectedIdempKey) + } +} + +func TestClient_CreatePayment_RejectsEmptyIdempotencyKey(t *testing.T) { + // v1.0.7 item D: an empty key is an error — the caller forgot to + // supply one, and silently sending the header empty would produce + // Hyperswitch behavior indistinguishable from a no-header request. + // Fail loudly so the bug surfaces in tests / code review. + client := NewClient("http://unreachable.invalid", "test_api_key") + _, _, err := client.CreatePaymentSimple( + context.Background(), + "", // empty key + 100, "EUR", "", "", nil, + ) + if err == nil { + t.Fatal("expected error for empty idempotency key") + } } func TestClient_CreatePayment_HTTPError(t *testing.T) { @@ -52,12 +78,50 @@ func TestClient_CreatePayment_HTTPError(t *testing.T) { defer server.Close() client := NewClient(server.URL, "test_api_key") - _, _, err := client.CreatePaymentSimple(context.Background(), 100, "EUR", "", "", nil) + _, _, err := client.CreatePaymentSimple(context.Background(), "order-err", 100, "EUR", "", "", nil) if err == nil { t.Fatal("expected error for 400 response") } } +func TestClient_CreateRefund_SendsIdempotencyKey(t *testing.T) { + const expectedIdempKey = "refund-abc-123" + var gotIdempKey string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.Path != "/refunds" { + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + } + gotIdempKey = r.Header.Get("Idempotency-Key") + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(map[string]string{ + "refund_id": "ref_test_456", + "payment_id": "pay_test_123", + "status": "pending", + }) + })) + defer server.Close() + + client := NewClient(server.URL, "test_api_key") + resp, err := client.CreateRefund(context.Background(), expectedIdempKey, "pay_test_123", nil, "customer request") + if err != nil { + t.Fatalf("CreateRefund: %v", err) + } + if resp.RefundID != "ref_test_456" { + t.Errorf("refund_id = %q, want ref_test_456", resp.RefundID) + } + if gotIdempKey != expectedIdempKey { + t.Errorf("Idempotency-Key header = %q, want %q", gotIdempKey, expectedIdempKey) + } +} + +func TestClient_CreateRefund_RejectsEmptyIdempotencyKey(t *testing.T) { + client := NewClient("http://unreachable.invalid", "test_api_key") + _, err := client.CreateRefund(context.Background(), "", "pay_123", nil, "no key") + if err == nil { + t.Fatal("expected error for empty idempotency key on CreateRefund") + } +} + func TestClient_GetPaymentStatus(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/payments/pay_123" { diff --git a/veza-backend-api/internal/services/hyperswitch/provider.go b/veza-backend-api/internal/services/hyperswitch/provider.go index 8c2cbd982..03d3a2c98 100644 --- a/veza-backend-api/internal/services/hyperswitch/provider.go +++ b/veza-backend-api/internal/services/hyperswitch/provider.go @@ -20,8 +20,8 @@ func NewProvider(client *Client) *Provider { } // CreatePayment creates a payment in Hyperswitch. -func (p *Provider) CreatePayment(ctx context.Context, amount int64, currency, orderID, returnURL string, metadata map[string]string) (paymentID, clientSecret string, err error) { - return p.client.CreatePaymentSimple(ctx, amount, currency, orderID, returnURL, metadata) +func (p *Provider) CreatePayment(ctx context.Context, idempotencyKey string, amount int64, currency, orderID, returnURL string, metadata map[string]string) (paymentID, clientSecret string, err error) { + return p.client.CreatePaymentSimple(ctx, idempotencyKey, amount, currency, orderID, returnURL, metadata) } // GetPayment retrieves payment status from Hyperswitch. @@ -29,22 +29,19 @@ func (p *Provider) GetPayment(ctx context.Context, paymentID string) (string, er return p.client.GetPaymentStatus(ctx, paymentID) } -// Refund creates a refund in Hyperswitch (v0.403 R2, kept for backward -// compatibility with any call-site that only cared about the error). -func (p *Provider) Refund(ctx context.Context, paymentID string, amount *int64, reason string) error { - _, err := p.client.CreateRefund(ctx, paymentID, amount, reason) - return err -} - // CreateRefund creates a refund in Hyperswitch and returns the PSP refund -// id and synchronous status (v1.0.6). The marketplace service persists the -// refund_id as the idempotency key for the webhook handler — every later -// refund.* notification can be correlated back to the pending Refund row -// via `hyperswitch_refund_id`. +// id and synchronous status (v1.0.6, v1.0.7 item D). The marketplace +// service persists the refund_id as the idempotency key for the webhook +// handler — every later refund.* notification can be correlated back to +// the pending Refund row via `hyperswitch_refund_id`. +// +// idempotencyKey (v1.0.7 item D): passed through to the +// `Idempotency-Key` HTTP header. Marketplace passes the pending Refund +// row's UUID — stable across HTTP retries of the same logical call. // // Matches marketplace.refundProvider interface. -func (p *Provider) CreateRefund(ctx context.Context, paymentID string, amount *int64, reason string) (string, string, error) { - resp, err := p.client.CreateRefund(ctx, paymentID, amount, reason) +func (p *Provider) CreateRefund(ctx context.Context, idempotencyKey, paymentID string, amount *int64, reason string) (string, string, error) { + resp, err := p.client.CreateRefund(ctx, idempotencyKey, paymentID, amount, reason) if err != nil { return "", "", err } diff --git a/veza-backend-api/tests/contract/critical_endpoints_test.go b/veza-backend-api/tests/contract/critical_endpoints_test.go index b59d22d0d..67a5cb8ca 100644 --- a/veza-backend-api/tests/contract/critical_endpoints_test.go +++ b/veza-backend-api/tests/contract/critical_endpoints_test.go @@ -367,7 +367,7 @@ type mockPaymentProvider struct { clientSecret string } -func (m *mockPaymentProvider) CreatePayment(_ context.Context, _ int64, _, _, _ string, _ map[string]string) (string, string, error) { +func (m *mockPaymentProvider) CreatePayment(_ context.Context, _ string, _ int64, _, _, _ string, _ map[string]string) (string, string, error) { return m.paymentID, m.clientSecret, nil } func (m *mockPaymentProvider) GetPayment(_ context.Context, _ string) (string, error) { diff --git a/veza-backend-api/tests/integration/payment_flow_test.go b/veza-backend-api/tests/integration/payment_flow_test.go index 8e4c285e5..d85ecb1f0 100644 --- a/veza-backend-api/tests/integration/payment_flow_test.go +++ b/veza-backend-api/tests/integration/payment_flow_test.go @@ -37,7 +37,7 @@ type mockPaymentProvider struct { clientSecret string } -func (m *mockPaymentProvider) CreatePayment(_ context.Context, _ int64, _ string, _ string, _ string, _ map[string]string) (string, string, error) { +func (m *mockPaymentProvider) CreatePayment(_ context.Context, _ string, _ int64, _ string, _ string, _ string, _ map[string]string) (string, string, error) { return m.paymentID, m.clientSecret, nil } diff --git a/veza-backend-api/tests/integration/refund_flow_test.go b/veza-backend-api/tests/integration/refund_flow_test.go index 61f4baf8a..c1beb402b 100644 --- a/veza-backend-api/tests/integration/refund_flow_test.go +++ b/veza-backend-api/tests/integration/refund_flow_test.go @@ -35,7 +35,7 @@ type mockRefundPaymentProvider struct { refundErr error } -func (m *mockRefundPaymentProvider) CreatePayment(_ context.Context, _ int64, _ string, _ string, _ string, _ map[string]string) (string, string, error) { +func (m *mockRefundPaymentProvider) CreatePayment(_ context.Context, _ string, _ int64, _ string, _ string, _ string, _ map[string]string) (string, string, error) { return "pay_refund_mock", "secret", nil } @@ -43,8 +43,11 @@ func (m *mockRefundPaymentProvider) GetPayment(_ context.Context, _ string) (str return "succeeded", nil } -func (m *mockRefundPaymentProvider) Refund(_ context.Context, _ string, _ *int64, _ string) error { - return m.refundErr +func (m *mockRefundPaymentProvider) CreateRefund(_ context.Context, _ string, _ string, _ *int64, _ string) (string, string, error) { + if m.refundErr != nil { + return "", "", m.refundErr + } + return "ref_mock", "pending", nil } func setupRefundFlowRouter(t *testing.T, db *gorm.DB, marketService *marketplace.Service) *gin.Engine {