diff --git a/test/stripe-integration/env.go b/test/stripe-integration/env.go index 1167b55..fb81c24 100644 --- a/test/stripe-integration/env.go +++ b/test/stripe-integration/env.go @@ -120,6 +120,29 @@ func cleanupCustomerByDID(t *testing.T, userDID string) { }) } +// newBareCustomer creates a real sandbox customer carrying no user_did +// metadata, and returns its ID. +// +// This exists because "customer.Get fails" and "customer has no user_did" are +// different conclusions that getCustomerDID used to collapse into the same +// empty string. Now that a failed lookup is an error (and retryable), a test +// that wants the no-DID branch has to name a customer that actually resolves. +// A literal "cus_fake" reaches the error branch instead and no longer proves +// anything about dispatch. +func newBareCustomer(t *testing.T) string { + t.Helper() + cust, err := customer.New(&stripe.CustomerParams{}) + if err != nil { + t.Fatalf("create bare customer: %v", err) + } + t.Cleanup(func() { + if _, err := customer.Del(cust.ID, nil); err != nil { + t.Logf("cleanup: delete customer %s: %v", cust.ID, err) + } + }) + return cust.ID +} + func getenvDefault(key, def string) string { if v := os.Getenv(key); v != "" { return v diff --git a/test/stripe-integration/manager_test.go b/test/stripe-integration/manager_test.go index 662852e..1c10b89 100644 --- a/test/stripe-integration/manager_test.go +++ b/test/stripe-integration/manager_test.go @@ -3,10 +3,12 @@ package stripeintegration import ( + "database/sql" "encoding/json" "errors" "net/http" "net/http/httptest" + "path/filepath" "strings" "testing" "time" @@ -15,6 +17,7 @@ import ( "github.com/stripe/stripe-go/v84/customer" "github.com/stripe/stripe-go/v84/webhook" + appdb "atcr.io/pkg/appview/db" "atcr.io/pkg/billing" ) @@ -25,6 +28,30 @@ import ( // by the harness E2E test in this same package. func newManager(t *testing.T, env stripeEnv) *billing.Manager { t.Helper() + m, _ := newManagerWithDB(t, env) + return m +} + +// newManagerWithDB builds the same Manager and hands back its database. +// +// The database is the point. This suite used to pass nil, which switched off +// every `m.db != nil` branch in HandleWebhook — the idempotency check, the +// per-customer ordering guard, and the processed-event record. It ran against +// real Stripe and exercised none of the code those guards live in, so it read +// as much broader coverage than it was. +// +// A file under t.TempDir rather than ":memory:": libsql scopes an in-memory +// database to a single connection, so the pool's second connection sees no +// tables. +func newManagerWithDB(t *testing.T, env stripeEnv) (*billing.Manager, *sql.DB) { + t.Helper() + + database, err := appdb.InitDB(filepath.Join(t.TempDir(), "ui.db"), appdb.LibsqlConfig{}) + if err != nil { + t.Fatalf("init db: %v", err) + } + t.Cleanup(func() { _ = database.Close() }) + cfg := &billing.Config{ StripeSecretKey: env.SecretKey, WebhookSecret: env.WebhookSecret, @@ -44,7 +71,7 @@ func newManager(t *testing.T, env stripeEnv) *billing.Manager { }, }, } - return billing.New(cfg, nil, "did:web:test-appview.local", nil, "http://test-appview.local", nil) + return billing.New(cfg, nil, "did:web:test-appview.local", nil, "http://test-appview.local", database), database } func TestManagerEnabled(t *testing.T) { @@ -211,6 +238,8 @@ func TestHandleWebhookValidSignature(t *testing.T) { env := requireStripeEnv(t) m := newManager(t, env) + // handleCheckoutCompleted only logs, and never resolves the customer, so + // an ID that does not exist in the sandbox is fine here. payload := buildEventPayload("evt_test_checkout", "checkout.session.completed", map[string]any{ "id": "cs_test_x", "object": "checkout.session", @@ -337,9 +366,19 @@ func TestHandleWebhookSubscriptionCreated(t *testing.T) { // TestHandleWebhookSubscriptionDeleted. func TestHandleWebhookAllSubscribedEvents(t *testing.T) { env := requireStripeEnv(t) + stripe.Key = env.SecretKey m := newManager(t, env) cap := newLogCapture(t) + // A customer that resolves but carries no user_did. The subscription + // handlers are then expected to stop at "No user DID found", which is a + // conclusion rather than a failure. This used to be the literal string + // "cus_fake"; once a failed customer.Get became a retryable error rather + // than an empty DID, that reached the error branch instead and the whole + // dispatch assertion stopped holding. + customerID := newBareCustomer(t) + created := time.Now().Unix() + // Distinctive substring each handler emits. If a switch case is dropped, // the event hits the default branch ("Ignoring Stripe event") and these // substrings won't appear in the captured output. @@ -358,7 +397,12 @@ func TestHandleWebhookAllSubscribedEvents(t *testing.T) { t.Run(eventType, func(t *testing.T) { cap.reset() - payload := buildEventPayload("evt_test_"+eventType, eventType, payloadFor(eventType, env.PriceMonthly)) + // Distinct, increasing created values: these subtests share one + // customer, and equal-or-older timestamps would let the ordering + // guard drop later events before they reach their handler. + created++ + payload := buildEventPayloadAt("evt_test_"+eventType, eventType, created, + payloadFor(eventType, env.PriceMonthly, customerID)) req := signedWebhookRequest(t, payload, env.WebhookSecret, time.Now()) if err := m.HandleWebhook(req); err != nil { @@ -464,6 +508,104 @@ func TestTierResolutionByPriceID(t *testing.T) { } } +// TestHandleWebhook_RedeliveryIsSkipped covers the idempotency guard against +// the real Stripe backend and a real database — the pair this suite could not +// reach while its Manager was built with nil. +// +// The assertion that carries the weight is the second one: that redelivery did +// not reach a handler. A row count cannot show this, because RecordStripeEvent +// is an idempotent upsert and deleting the guard entirely leaves the table +// looking exactly the same. +func TestHandleWebhook_RedeliveryIsSkipped(t *testing.T) { + env := requireStripeEnv(t) + stripe.Key = env.SecretKey + m, database := newManagerWithDB(t, env) + cap := newLogCapture(t) + + customerID := newBareCustomer(t) + payload := buildEventPayloadAt("evt_test_redelivery", billing.EventSubscriptionUpdated, + time.Now().Unix(), payloadFor(billing.EventSubscriptionUpdated, env.PriceMonthly, customerID)) + + req := signedWebhookRequest(t, payload, env.WebhookSecret, time.Now()) + if err := m.HandleWebhook(req); err != nil { + t.Fatalf("first delivery: %v", err) + } + if !cap.contains("No user DID found") { + t.Fatalf("first delivery did not reach the subscription handler:\n%s", cap.String()) + } + + cap.reset() + req = signedWebhookRequest(t, payload, env.WebhookSecret, time.Now()) + if err := m.HandleWebhook(req); err != nil { + t.Fatalf("redelivery: %v", err) + } + if !cap.contains("Ignoring already-processed Stripe event") { + t.Errorf("redelivery was not skipped:\n%s", cap.String()) + } + if cap.contains("No user DID found") { + t.Errorf("redelivery reached the subscription handler again:\n%s", cap.String()) + } + if n := processedEventCount(t, database, "evt_test_redelivery"); n != 1 { + t.Errorf("stripe_processed_events holds %d rows for the event, want 1", n) + } +} + +// TestHandleWebhook_StaleEventIsIgnored covers the per-customer ordering guard. +// +// Stripe does not promise delivery order, so a stale "active" arriving after a +// cancellation could otherwise re-grant a tier the customer no longer holds. +// The two events here are identical but for created, so a tier push on the +// second one can only mean the guard did not fire. +func TestHandleWebhook_StaleEventIsIgnored(t *testing.T) { + env := requireStripeEnv(t) + stripe.Key = env.SecretKey + m := newManager(t, env) + cap := newLogCapture(t) + + // A user_did here, unlike the redelivery test: it carries the handler past + // the DID lookup to the tier push, which is the loudest available signal + // for "the guard let this through". + userDID := uniqueDID(t) + cust, err := customer.New(&stripe.CustomerParams{ + Params: stripe.Params{Metadata: map[string]string{"user_did": userDID}}, + }) + if err != nil { + t.Fatalf("create customer: %v", err) + } + t.Cleanup(func() { + if _, err := customer.Del(cust.ID, nil); err != nil { + t.Logf("cleanup: delete customer %s: %v", cust.ID, err) + } + }) + + now := time.Now().Unix() + data := payloadFor(billing.EventSubscriptionUpdated, env.PriceMonthly, cust.ID) + + req := signedWebhookRequest(t, + buildEventPayloadAt("evt_test_order_fresh", billing.EventSubscriptionUpdated, now, data), + env.WebhookSecret, time.Now()) + if err := m.HandleWebhook(req); err != nil { + t.Fatalf("fresh event: %v", err) + } + if !cap.contains("Pushing tier update to managed holds") { + t.Fatalf("fresh event did not apply a tier:\n%s", cap.String()) + } + + cap.reset() + req = signedWebhookRequest(t, + buildEventPayloadAt("evt_test_order_stale", billing.EventSubscriptionUpdated, now-3600, data), + env.WebhookSecret, time.Now()) + if err := m.HandleWebhook(req); err != nil { + t.Fatalf("stale event: %v", err) + } + if !cap.contains("Ignoring stale out-of-order subscription event") { + t.Errorf("stale event was not recognised as out of order:\n%s", cap.String()) + } + if cap.contains("Pushing tier update to managed holds") { + t.Errorf("stale event applied a tier:\n%s", cap.String()) + } +} + // --- helpers --------------------------------------------------------------- // buildEventPayload assembles a Stripe Event envelope with api_version set @@ -471,10 +613,23 @@ func TestTierResolutionByPriceID(t *testing.T) { // version mismatch. The handler reads event.Type, event.Data.Raw, and not // much else, so the surrounding fields are minimal. func buildEventPayload(id, eventType string, dataObject map[string]any) []byte { + return buildEventPayloadAt(id, eventType, time.Now().Unix(), dataObject) +} + +// buildEventPayloadAt is buildEventPayload with an explicit created timestamp, +// for the per-customer ordering guard — the only way to build an event that is +// unseen and yet older than one already applied. +// +// created must be set even when a test does not care about ordering. Left +// absent it unmarshals as 0, and the guard reads any later event for the same +// customer as older than what it has already applied, so the second event for +// a customer silently becomes a no-op. +func buildEventPayloadAt(id, eventType string, created int64, dataObject map[string]any) []byte { envelope := map[string]any{ "id": id, "object": "event", "api_version": stripe.APIVersion, + "created": created, "type": eventType, "data": map[string]any{"object": dataObject}, } @@ -489,14 +644,15 @@ func buildEventPayload(id, eventType string, dataObject map[string]any) []byte { // payloadFor returns a minimal but well-shaped data.object for the given // event type — just enough that the handler's json.Unmarshal succeeds and // the function runs to its first observable log line. priceID is the price -// ID stamped into subscription payloads (unused for non-subscription events). -func payloadFor(eventType, priceID string) map[string]any { +// ID stamped into subscription payloads (unused for non-subscription events), +// and customerID must name a customer that really exists in the sandbox. +func payloadFor(eventType, priceID, customerID string) map[string]any { switch eventType { case "checkout.session.completed": return map[string]any{ "id": "cs_test_x", "object": "checkout.session", - "customer": "cus_fake", + "customer": customerID, "subscription": "sub_fake", } case "customer.subscription.created", @@ -508,7 +664,7 @@ func payloadFor(eventType, priceID string) map[string]any { "id": "sub_test_fake", "object": "subscription", "status": "active", - "customer": "cus_fake", + "customer": customerID, "items": map[string]any{ "object": "list", "data": []map[string]any{{ @@ -526,7 +682,7 @@ func payloadFor(eventType, priceID string) map[string]any { return map[string]any{ "id": "in_test_fake", "object": "invoice", - "customer": "cus_fake", + "customer": customerID, "amount_due": 100, "currency": "usd", "attempt_count": 1, @@ -544,7 +700,7 @@ func payloadFor(eventType, priceID string) map[string]any { "charge": map[string]any{ "id": "ch_test_fake", "object": "charge", - "customer": "cus_fake", + "customer": customerID, }, } } @@ -569,3 +725,15 @@ func signedWebhookRequest(t *testing.T, payload []byte, secret string, ts time.T return req } +// processedEventCount reports how many rows stripe_processed_events holds for +// one event ID. +func processedEventCount(t *testing.T, database *sql.DB, eventID string) int { + t.Helper() + var n int + if err := database.QueryRow( + "SELECT COUNT(*) FROM stripe_processed_events WHERE event_id = ?", eventID, + ).Scan(&n); err != nil { + t.Fatalf("count processed events: %v", err) + } + return n +}