diff --git a/backend/internal/handler/payment_webhook_handler_test.go b/backend/internal/handler/payment_webhook_handler_test.go index 88221b5c..7551fc83 100644 --- a/backend/internal/handler/payment_webhook_handler_test.go +++ b/backend/internal/handler/payment_webhook_handler_test.go @@ -6,11 +6,13 @@ import ( "context" "encoding/json" "errors" + "fmt" "net/http" "net/http/httptest" "testing" "github.com/Wei-Shaw/sub2api/internal/payment" + "github.com/Wei-Shaw/sub2api/internal/service" "github.com/gin-gonic/gin" "github.com/stretchr/testify/assert" "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) { t.Run("maxWebhookBodySize is 1MB", func(t *testing.T) { assert.Equal(t, int64(1<<20), int64(maxWebhookBodySize)) diff --git a/backend/internal/service/payment_fulfillment_order_not_found_test.go b/backend/internal/service/payment_fulfillment_order_not_found_test.go new file mode 100644 index 00000000..f6787e29 --- /dev/null +++ b/backend/internal/service/payment_fulfillment_order_not_found_test.go @@ -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") +}