max / makenotwork
4 files changed,
+116 insertions,
-2 deletions
| @@ -0,0 +1,22 @@ | |||
| 1 | + | -- Per-invoice idempotency ledger for the Fan+ monthly $5 platform credit. | |
| 2 | + | -- | |
| 3 | + | -- `invoice.payment_succeeded` is deduplicated at the webhook layer by a | |
| 4 | + | -- check-then-act read (read processed-marker, run handler, mark after success), | |
| 5 | + | -- which does NOT serialize two concurrent deliveries of the same event: both | |
| 6 | + | -- pass the read and both run the handler. Every other money-moving webhook | |
| 7 | + | -- side-effect is idempotent by a DB-level write; the Fan+ credit was not — | |
| 8 | + | -- `create_platform_promo_code` minted a fresh code per call, so a duplicate | |
| 9 | + | -- delivery double-issued the credit. | |
| 10 | + | -- | |
| 11 | + | -- This table makes credit issuance idempotent on `(stripe_sub_id, period_end)`: | |
| 12 | + | -- a renewal for a given subscription period mints at most one credit. The | |
| 13 | + | -- handler inserts ON CONFLICT DO NOTHING and only generates+emails the code | |
| 14 | + | -- when its insert wins; the loser short-circuits. `period_end` is the Stripe | |
| 15 | + | -- invoice period-end unix timestamp, the same value the credit's expiry is | |
| 16 | + | -- derived from, so it is stable across redeliveries of one renewal. | |
| 17 | + | CREATE TABLE IF NOT EXISTS fan_plus_credit_issuance ( | |
| 18 | + | stripe_sub_id TEXT NOT NULL, | |
| 19 | + | period_end BIGINT NOT NULL, | |
| 20 | + | issued_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), | |
| 21 | + | PRIMARY KEY (stripe_sub_id, period_end) | |
| 22 | + | ); |
| @@ -432,6 +432,36 @@ pub async fn create_platform_promo_code( | |||
| 432 | 432 | Ok(promo_code) | |
| 433 | 433 | } | |
| 434 | 434 | ||
| 435 | + | /// Claim the once-per-renewal slot for a Fan+ monthly credit. | |
| 436 | + | /// | |
| 437 | + | /// Returns `true` if this `(stripe_sub_id, period_end)` was not yet claimed (row | |
| 438 | + | /// inserted — the caller should mint and email the credit), `false` if a prior | |
| 439 | + | /// delivery of the same renewal already claimed it (the caller must do nothing). | |
| 440 | + | /// | |
| 441 | + | /// This is the DB-level idempotency guard that closes the duplicate-webhook | |
| 442 | + | /// double-credit race: `invoice.payment_succeeded` dedup at the webhook layer is | |
| 443 | + | /// a check-then-act read that two concurrent deliveries both pass, so the | |
| 444 | + | /// money-moving side-effect must serialize on its own atomic write. The | |
| 445 | + | /// `ON CONFLICT DO NOTHING` against the `(stripe_sub_id, period_end)` primary key | |
| 446 | + | /// makes exactly one of N concurrent deliveries win. | |
| 447 | + | #[tracing::instrument(skip_all)] | |
| 448 | + | pub async fn try_claim_fan_plus_credit( | |
| 449 | + | pool: &PgPool, | |
| 450 | + | stripe_sub_id: &str, | |
| 451 | + | period_end: i64, | |
| 452 | + | ) -> Result<bool> { | |
| 453 | + | let result = sqlx::query( | |
| 454 | + | "INSERT INTO fan_plus_credit_issuance (stripe_sub_id, period_end) \ | |
| 455 | + | VALUES ($1, $2) ON CONFLICT DO NOTHING", | |
| 456 | + | ) | |
| 457 | + | .bind(stripe_sub_id) | |
| 458 | + | .bind(period_end) | |
| 459 | + | .execute(pool) | |
| 460 | + | .await?; | |
| 461 | + | ||
| 462 | + | Ok(result.rows_affected() == 1) | |
| 463 | + | } | |
| 464 | + | ||
| 435 | 465 | /// Look up a platform-wide promo code by user ID and code string (case-insensitive). | |
| 436 | 466 | /// | |
| 437 | 467 | /// Used at checkout to validate Fan+ credits: the buyer owns the code, and it |
| @@ -75,8 +75,23 @@ pub(super) async fn handle_invoice_payment_succeeded( | |||
| 75 | 75 | // Stripe period to the sealed writer, which drops a non-positive end. | |
| 76 | 76 | db::fan_plus::apply_stripe_update(&state.db, &stripe_sub_id, None, Some((invoice.period_start, invoice.period_end))).await.context("refresh fan+ period")?; | |
| 77 | 77 | ||
| 78 | - | // On renewal, generate a $5 platform-wide promo code and email it | |
| 79 | - | if is_renewal { | |
| 78 | + | // On renewal, generate a $5 platform-wide promo code and email it. | |
| 79 | + | // | |
| 80 | + | // Idempotency: webhook dedup (`webhook/mod.rs`) is a check-then-act read | |
| 81 | + | // that two concurrent deliveries of the same `invoice.payment_succeeded` | |
| 82 | + | // both pass, so the credit — a money-moving side-effect — serializes on | |
| 83 | + | // its own atomic write here. `try_claim_fan_plus_credit` inserts a | |
| 84 | + | // `(stripe_sub_id, period_end)` row ON CONFLICT DO NOTHING; only the | |
| 85 | + | // delivery that wins the insert mints and emails the code. A redelivery | |
| 86 | + | // (or duplicate concurrent delivery) finds the slot taken and skips, | |
| 87 | + | // so a renewal issues at most one $5 credit. | |
| 88 | + | if is_renewal | |
| 89 | + | && db::promo_codes::try_claim_fan_plus_credit( | |
| 90 | + | &state.db, &stripe_sub_id, invoice.period_end, | |
| 91 | + | ) | |
| 92 | + | .await | |
| 93 | + | .context("claim fan+ credit issuance slot")? | |
| 94 | + | { | |
| 80 | 95 | let period_end = chrono::DateTime::from_timestamp(invoice.period_end, 0); | |
| 81 | 96 | ||
| 82 | 97 | // Uniqueness of the generated code is enforced by the DB-level |
| @@ -1848,3 +1848,50 @@ async fn webhook_invoice_paid_cannot_refresh_period_on_canceled_fan_plus() { | |||
| 1848 | 1848 | ).bind(stripe_sub_id).fetch_one(&h.db).await.unwrap(); | |
| 1849 | 1849 | assert_eq!(after.timestamp(), 1000000000, "period must stay frozen on a canceled Fan+ sub, not jump to the invoice's period_end"); | |
| 1850 | 1850 | } | |
| 1851 | + | ||
| 1852 | + | #[tokio::test] | |
| 1853 | + | async fn fan_plus_renewal_credit_issued_once_across_duplicate_deliveries() { | |
| 1854 | + | // Regression for the Run #22 SERIOUS: webhook event-dedup is a check-then-act | |
| 1855 | + | // read keyed on event id; a duplicate `invoice.payment_succeeded` delivery | |
| 1856 | + | // that escapes it (distinct event id, same invoice/period) must NOT mint a | |
| 1857 | + | // second $5 Fan+ credit. The per-(stripe_sub_id, period_end) idempotency | |
| 1858 | + | // guard makes the renewal credit a single DB-level write. | |
| 1859 | + | let mut h = TestHarness::with_stripe().await; | |
| 1860 | + | let user_id = h.signup("fpcredit", "fpcredit@test.com", "password123").await; | |
| 1861 | + | let stripe_sub_id = "sub_fp_credit_idem"; | |
| 1862 | + | sqlx::query( | |
| 1863 | + | r#"INSERT INTO fan_plus_subscriptions | |
| 1864 | + | (user_id, stripe_subscription_id, stripe_customer_id, status, current_period_end) | |
| 1865 | + | VALUES ($1, $2, 'cus_fp_credit', 'active', to_timestamp(1700000000))"#, | |
| 1866 | + | ) | |
| 1867 | + | .bind(user_id).bind(stripe_sub_id).execute(&h.db).await.unwrap(); | |
| 1868 | + | ||
| 1869 | + | // The platform-wide $5 credit codes minted for this user. | |
| 1870 | + | async fn credit_count(h: &TestHarness, user_id: UserId) -> i64 { | |
| 1871 | + | sqlx::query_scalar( | |
| 1872 | + | "SELECT COUNT(*) FROM promo_codes \ | |
| 1873 | + | WHERE creator_id = $1 AND is_platform_wide = true AND discount_value = 500", | |
| 1874 | + | ) | |
| 1875 | + | .bind(user_id).fetch_one(&h.db).await.unwrap() | |
| 1876 | + | } | |
| 1877 | + | ||
| 1878 | + | // First renewal delivery mints exactly one credit. | |
| 1879 | + | let resp = post_event_json_with_id( | |
| 1880 | + | &mut h, "evt_fpcredit_1", "invoice.payment_succeeded", | |
| 1881 | + | make_invoice(stripe_sub_id, "subscription_cycle"), | |
| 1882 | + | ).await; | |
| 1883 | + | assert_eq!(resp.status.as_u16(), 200, "first renewal failed: {}", resp.text); | |
| 1884 | + | assert_eq!(credit_count(&h, user_id).await, 1, "first renewal must mint one credit"); | |
| 1885 | + | ||
| 1886 | + | // A duplicate delivery of the SAME renewal period under a different event id | |
| 1887 | + | // (so event-dedup does not catch it) must not mint a second credit. | |
| 1888 | + | let resp = post_event_json_with_id( | |
| 1889 | + | &mut h, "evt_fpcredit_2", "invoice.payment_succeeded", | |
| 1890 | + | make_invoice(stripe_sub_id, "subscription_cycle"), | |
| 1891 | + | ).await; | |
| 1892 | + | assert_eq!(resp.status.as_u16(), 200, "duplicate renewal failed: {}", resp.text); | |
| 1893 | + | assert_eq!( | |
| 1894 | + | credit_count(&h, user_id).await, 1, | |
| 1895 | + | "a duplicate delivery of the same renewal must not double-issue the credit" | |
| 1896 | + | ); | |
| 1897 | + | } |