test(payment): cover ErrOrderNotFound sentinel contract
Service layer (payment_fulfillment_order_not_found_test.go): - TestHandlePaymentNotification_UnknownOrder_ReturnsSentinel: in-memory sqlite ent client, query for a non-existent out_trade_no → errors.Is must recognise ErrOrderNotFound (handler relies on this to ack 200). - TestHandlePaymentNotification_NonSuccessStatus_Skips: non-success notification short-circuits before DB lookup → nil error. - TestErrOrderNotFound_DistinctFromOtherErrors: generic errors must not match the sentinel (prevents silently swallowing DB failures). Handler layer (payment_webhook_handler_test.go): - TestUnknownOrderWebhookAcksWithSuccess: locks the two ingredients the handleNotify ack path depends on — fmt.Errorf %w wrapping preserves errors.Is recognition, and writeSuccessResponse(stripe) returns an empty 200 body that Stripe treats as acknowledged.
This commit is contained in:
@@ -6,11 +6,13 @@ import (
|
|||||||
"context"
|
"context"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"errors"
|
"errors"
|
||||||
|
"fmt"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/Wei-Shaw/sub2api/internal/payment"
|
"github.com/Wei-Shaw/sub2api/internal/payment"
|
||||||
|
"github.com/Wei-Shaw/sub2api/internal/service"
|
||||||
"github.com/gin-gonic/gin"
|
"github.com/gin-gonic/gin"
|
||||||
"github.com/stretchr/testify/assert"
|
"github.com/stretchr/testify/assert"
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
@@ -91,6 +93,43 @@ func TestWriteSuccessResponse(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestUnknownOrderWebhookAcksWithSuccess exercises the response contract that
|
||||||
|
// handleNotify relies on when HandlePaymentNotification returns ErrOrderNotFound:
|
||||||
|
// we still need to emit the provider-specific 2xx so the provider stops
|
||||||
|
// retrying. We can't easily drive handleNotify end-to-end without mocking the
|
||||||
|
// concrete *service.PaymentService, so this test locks down the two ingredients
|
||||||
|
// the fix depends on:
|
||||||
|
// 1. errors.Is recognises the sentinel through fmt.Errorf %w wrapping (which
|
||||||
|
// is how service layer wraps it with the out_trade_no context).
|
||||||
|
// 2. writeSuccessResponse produces the provider-specific body for Stripe
|
||||||
|
// (empty 200) — matching what handleNotify calls on the ack path.
|
||||||
|
//
|
||||||
|
// If either contract breaks, the Stripe "unknown order → 500 loop" regresses.
|
||||||
|
func TestUnknownOrderWebhookAcksWithSuccess(t *testing.T) {
|
||||||
|
gin.SetMode(gin.TestMode)
|
||||||
|
|
||||||
|
// 1) Sentinel recognition through wrapping.
|
||||||
|
wrapped := fmt.Errorf("%w: out_trade_no=sub2_missing_42", service.ErrOrderNotFound)
|
||||||
|
require.True(t, errors.Is(wrapped, service.ErrOrderNotFound),
|
||||||
|
"handleNotify uses errors.Is on the wrapped service error; regression here "+
|
||||||
|
"would mean unknown-order webhooks go back to returning 500 and looping forever")
|
||||||
|
|
||||||
|
// A distinct error must NOT match — otherwise a DB failure would be silently
|
||||||
|
// swallowed as an ack.
|
||||||
|
other := errors.New("lookup order failed: connection refused")
|
||||||
|
require.False(t, errors.Is(other, service.ErrOrderNotFound))
|
||||||
|
|
||||||
|
// 2) Provider-specific success body is what handleNotify emits on the
|
||||||
|
// ack path. Asserted again here because this is the shape Stripe expects
|
||||||
|
// to consider the webhook acknowledged.
|
||||||
|
w := httptest.NewRecorder()
|
||||||
|
c, _ := gin.CreateTestContext(w)
|
||||||
|
writeSuccessResponse(c, payment.TypeStripe)
|
||||||
|
require.Equal(t, http.StatusOK, w.Code,
|
||||||
|
"Stripe requires 2xx to stop retrying; anything else restarts the retry loop")
|
||||||
|
require.Empty(t, w.Body.String(), "Stripe expects an empty body on the ack path")
|
||||||
|
}
|
||||||
|
|
||||||
func TestWebhookConstants(t *testing.T) {
|
func TestWebhookConstants(t *testing.T) {
|
||||||
t.Run("maxWebhookBodySize is 1MB", func(t *testing.T) {
|
t.Run("maxWebhookBodySize is 1MB", func(t *testing.T) {
|
||||||
assert.Equal(t, int64(1<<20), int64(maxWebhookBodySize))
|
assert.Equal(t, int64(1<<20), int64(maxWebhookBodySize))
|
||||||
|
|||||||
@@ -0,0 +1,106 @@
|
|||||||
|
//go:build unit
|
||||||
|
|
||||||
|
package service
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"database/sql"
|
||||||
|
"errors"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"entgo.io/ent/dialect"
|
||||||
|
entsql "entgo.io/ent/dialect/sql"
|
||||||
|
_ "modernc.org/sqlite"
|
||||||
|
|
||||||
|
dbent "github.com/Wei-Shaw/sub2api/ent"
|
||||||
|
"github.com/Wei-Shaw/sub2api/ent/enttest"
|
||||||
|
"github.com/Wei-Shaw/sub2api/internal/payment"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
)
|
||||||
|
|
||||||
|
// newOrderNotFoundTestClient wires an in-memory sqlite-backed ent.Client so
|
||||||
|
// tests can exercise HandlePaymentNotification's real DB lookup path without
|
||||||
|
// standing up a service stack.
|
||||||
|
func newOrderNotFoundTestClient(t *testing.T) *dbent.Client {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
db, err := sql.Open("sqlite", "file:payment_order_not_found?mode=memory&cache=shared&_fk=1")
|
||||||
|
require.NoError(t, err)
|
||||||
|
t.Cleanup(func() { _ = db.Close() })
|
||||||
|
|
||||||
|
_, err = db.Exec("PRAGMA foreign_keys = ON")
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
drv := entsql.OpenDB(dialect.SQLite, db)
|
||||||
|
client := enttest.NewClient(t, enttest.WithOptions(dbent.Driver(drv)))
|
||||||
|
t.Cleanup(func() { _ = client.Close() })
|
||||||
|
return client
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandlePaymentNotification_UnknownOrder_ReturnsSentinel exercises the
|
||||||
|
// happy-path of the webhook 404 fix: when the notification references an
|
||||||
|
// out_trade_no that does not exist in our DB, HandlePaymentNotification must
|
||||||
|
// return an error that errors.Is(err, ErrOrderNotFound) recognizes. The
|
||||||
|
// webhook handler relies on that contract to ack with a 2xx so the provider
|
||||||
|
// stops retrying.
|
||||||
|
func TestHandlePaymentNotification_UnknownOrder_ReturnsSentinel(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
client := newOrderNotFoundTestClient(t)
|
||||||
|
|
||||||
|
svc := &PaymentService{
|
||||||
|
entClient: client,
|
||||||
|
providersLoaded: true,
|
||||||
|
}
|
||||||
|
|
||||||
|
notification := &payment.PaymentNotification{
|
||||||
|
OrderID: "sub2_does_not_exist_12345",
|
||||||
|
TradeNo: "stripe_evt_test_xyz",
|
||||||
|
Status: payment.NotificationStatusSuccess,
|
||||||
|
Amount: 1000,
|
||||||
|
}
|
||||||
|
|
||||||
|
err := svc.HandlePaymentNotification(ctx, notification, payment.TypeStripe)
|
||||||
|
require.Error(t, err, "unknown out_trade_no should surface an error")
|
||||||
|
require.ErrorIs(t, err, ErrOrderNotFound,
|
||||||
|
"webhook handler relies on errors.Is(err, ErrOrderNotFound) to downgrade to 200")
|
||||||
|
|
||||||
|
// Sanity: the wrapped error message should still include the out_trade_no
|
||||||
|
// for operator diagnostics.
|
||||||
|
require.Contains(t, err.Error(), notification.OrderID)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestHandlePaymentNotification_NonSuccessStatus_Skips documents the
|
||||||
|
// short-circuit that precedes the DB lookup: when the notification is not a
|
||||||
|
// success event (e.g. Stripe non-payment events that reach us via the webhook
|
||||||
|
// route), we return nil without touching the DB and the handler responds 200.
|
||||||
|
func TestHandlePaymentNotification_NonSuccessStatus_Skips(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
client := newOrderNotFoundTestClient(t)
|
||||||
|
|
||||||
|
svc := &PaymentService{
|
||||||
|
entClient: client,
|
||||||
|
providersLoaded: true,
|
||||||
|
}
|
||||||
|
|
||||||
|
notification := &payment.PaymentNotification{
|
||||||
|
OrderID: "sub2_does_not_exist_12345",
|
||||||
|
Status: "failed", // any value other than NotificationStatusSuccess
|
||||||
|
}
|
||||||
|
|
||||||
|
err := svc.HandlePaymentNotification(ctx, notification, payment.TypeStripe)
|
||||||
|
require.NoError(t, err,
|
||||||
|
"non-success notifications must short-circuit before the DB lookup")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestErrOrderNotFound_DistinctFromOtherErrors guards against an accidental
|
||||||
|
// collapse where a generic wrapped error would start matching ErrOrderNotFound
|
||||||
|
// (which would silently mask real DB failures).
|
||||||
|
func TestErrOrderNotFound_DistinctFromOtherErrors(t *testing.T) {
|
||||||
|
genericErr := errors.New("some other failure")
|
||||||
|
require.False(t, errors.Is(genericErr, ErrOrderNotFound))
|
||||||
|
require.False(t, errors.Is(ErrOrderNotFound, genericErr))
|
||||||
|
|
||||||
|
wrappedLookupErr := errors.New("lookup order failed for out_trade_no sub2_42: connection refused")
|
||||||
|
require.False(t, errors.Is(wrappedLookupErr, ErrOrderNotFound),
|
||||||
|
"DB connection failures must not masquerade as order-not-found")
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user