abono: grace counts from the unpaid period; the sandbox finishes cancellations and retries deliveries
Summary
R21: abono_subscriptions gains current_period_start (in 0001_init.sql), stripeapi reads it (items[].current_period_start, top-level for older versions; PeriodStart()), sync stores it and treats a moved start as a change, and the shared grants rule anchors past_due on it: grants while now < start+30d, Backstop once now >= start, Describe's failing date start+30d; no start grants with Backstop false. Holding gains PeriodStart. The fake renders current_period_start on items. Review fixes: the fake ends a subscription set to cancel at its period end whatever its status, so a past_due one cancelled in the portal ends; and events are queued, sent in order, re-signed per attempt and kept on failure. A failed in-process (sandbox) delivery makes the page action answer 500, and the next action, page view or Advance sends what waits first; paying an already-paid session redirects to the success URL again instead of 410, so that retry has a path.
Agent notes
frozenAbonoChecksum re-recorded (2e452a38... -> a634c00c...) for the new column in 0001_init.sql, allowed by R21 and the constant's own rule while v0.1.0 is untagged. DeliveryErrors now reports why events are still waiting and clears once the queue empties; webhook (abonotest) failures still do not fail the page action, as Stripe's pages do not.
15 files changed,
+481
−97
describe.go+4 −3describe_test.go+17 −6holdings.go+22 −13holdings_test.go+20 −11internal/fakestripe/fake.go+95 −35internal/fakestripe/fake_test.go+137 −7internal/fakestripe/pages.go+8 −2internal/stripeapi/client.go+25 −6internal/stripeapi/client_test.go+8 −3migrations/0001_init.sql+1 −0sandbox_test.go+80 −0schema_test.go+1 −1stripe_double_test.go+2 −2sync.go+13 −8sync_test.go+48 −0
CI passed — run details
CI log
=== vet === go vet ./... === fmt === === test === go test ./... -count=1 ok amadan.net/rastrillo/abono 0.568s ok amadan.net/rastrillo/abono/abonotest 0.049s ok amadan.net/rastrillo/abono/internal/fakestripe 0.018s ok amadan.net/rastrillo/abono/internal/stripeapi 0.005s === race === CGO_ENABLED=1 go test ./... -race -count=1 ok amadan.net/rastrillo/abono 4.777s ok amadan.net/rastrillo/abono/abonotest 1.356s ok amadan.net/rastrillo/abono/internal/fakestripe 1.063s ok amadan.net/rastrillo/abono/internal/stripeapi 1.020s === browser === browser: no browser test file yet, nothing to run
Intent
diff --git a/describe.go b/describe.go| index 89cd966..82fabe3 100644 |
| --- a/describe.go |
| +++ b/describe.go |
| @@ -28,7 +28,8 @@ type Status struct { |
| // active grants, no date known |
| // renews active or trialing, with a period end, not cancelling: Date is the period end |
| // cancelling cancel_at_period_end and granting: Date is the period end |
| -// failing past_due and granting: Date is the day access stops, the period end plus 30 days |
| +// failing past_due and granting: Date is the day access stops, 30 days after the |
| +// unpaid period started (no date when the start is unknown) |
| // ended does not grant |
| // comp a comp with no end |
| // comp_until a comp with an end: Date is ends_at |
| @@ -50,8 +51,8 @@ func Describe(h Holding, now time.Time) Status { |
| switch { |
| case h.Status == "past_due": |
| var stops *time.Time |
| - if h.Until != nil { |
| - t := h.Until.Add(backstopWindow) |
| + if h.PeriodStart != nil { |
| + t := h.PeriodStart.Add(backstopWindow) |
| stops = &t |
| } |
| return Status{Key: StatusFailing, Tone: "warning", Date: stops} |
diff --git a/describe_test.go b/describe_test.go| index 14dd830..21e6077 100644 |
| --- a/describe_test.go |
| +++ b/describe_test.go |
| @@ -32,15 +32,26 @@ func TestDescribe(t *testing.T) { |
| {"cancelling", |
| abono.Holding{Source: abono.SourceSubscription, Status: "active", Until: &end, CancelAtPeriodEnd: true}, |
| "abono.status.cancelling", "warning", &end}, |
| - {"failing, before the period end", |
| - abono.Holding{Source: abono.SourceSubscription, Status: "past_due", Until: &end}, |
| + // failing (ruling R21): access stops 30 days after the unpaid |
| + // period started, whatever its end says. |
| + {"failing, before the period start", |
| + abono.Holding{Source: abono.SourceSubscription, Status: "past_due", PeriodStart: &end, Until: ptr(end.AddDate(1, 0, 0))}, |
| "abono.status.failing", "warning", ptr(end.Add(30 * day))}, |
| {"failing, held by the backstop", |
| - abono.Holding{Source: abono.SourceSubscription, Status: "past_due", Until: &past}, |
| + abono.Holding{Source: abono.SourceSubscription, Status: "past_due", PeriodStart: &past, Until: ptr(past.AddDate(1, 0, 0))}, |
| "abono.status.failing", "warning", ptr(past.Add(30 * day))}, |
| {"failing and cancelling reads as failing", |
| - abono.Holding{Source: abono.SourceSubscription, Status: "past_due", Until: &end, CancelAtPeriodEnd: true}, |
| - "abono.status.failing", "warning", ptr(end.Add(30 * day))}, |
| + abono.Holding{Source: abono.SourceSubscription, Status: "past_due", PeriodStart: &past, Until: ptr(past.AddDate(1, 0, 0)), CancelAtPeriodEnd: true}, |
| + "abono.status.failing", "warning", ptr(past.Add(30 * day))}, |
| + {"failing with no period start has no date", |
| + abono.Holding{Source: abono.SourceSubscription, Status: "past_due", Until: &end}, |
| + "abono.status.failing", "warning", nil}, |
| + {"failing one second before start+30d", |
| + abono.Holding{Source: abono.SourceSubscription, Status: "past_due", PeriodStart: ptr(now.Add(-30*day + time.Second)), Until: ptr(now.AddDate(1, 0, 0))}, |
| + "abono.status.failing", "warning", ptr(now.Add(time.Second))}, |
| + {"ended, past_due at exactly start+30d", |
| + abono.Holding{Source: abono.SourceSubscription, Status: "past_due", PeriodStart: ptr(now.Add(-30 * day)), Until: ptr(now.AddDate(1, 0, 0))}, |
| + "abono.status.ended", "neutral", nil}, |
| {"ended, cancelling and its period is over", |
| abono.Holding{Source: abono.SourceSubscription, Status: "active", Until: &past, CancelAtPeriodEnd: true}, |
| "abono.status.ended", "neutral", nil}, |
| @@ -51,7 +62,7 @@ func TestDescribe(t *testing.T) { |
| abono.Holding{Source: abono.SourceSubscription, Status: "canceled", Until: &past}, |
| "abono.status.ended", "neutral", nil}, |
| {"ended, past_due beyond the backstop", |
| - abono.Holding{Source: abono.SourceSubscription, Status: "past_due", Until: ptr(now.Add(-31 * day))}, |
| + abono.Holding{Source: abono.SourceSubscription, Status: "past_due", PeriodStart: ptr(now.Add(-31 * day)), Until: ptr(now.AddDate(1, 0, 0))}, |
| "abono.status.ended", "neutral", nil}, |
| {"ended, a comp past its end", |
| abono.Holding{Source: abono.SourceComp, Status: "active", Until: &past}, |
diff --git a/holdings.go b/holdings.go| index 35ddf22..2a9e2f9 100644 |
| --- a/holdings.go |
| +++ b/holdings.go |
| @@ -9,7 +9,9 @@ import ( |
| // backstopWindow is how long past a paid period's end a past_due subscription |
| // still grants: Stripe lets an account leave a subscription past_due for |
| // good once its retries end, so abono stops holding it 30 days after the |
| -// period it was paid to, whatever the account's setting. |
| +// period it was paid to, whatever the account's setting. The paid period's |
| +// end is the unpaid one's current_period_start: a failed renewal has |
| +// already moved current_period_end a whole period on (ruling R21). |
| const backstopWindow = 30 * 24 * time.Hour |
| // Source says where a holding came from. |
| @@ -31,9 +33,13 @@ type Holding struct { |
| // Status is Stripe's subscription status, or a comp's: active or |
| // canceled. |
| Status string |
| - // Until is the paid period's end for a subscription, a comp's ends_at; |
| - // nil when there is none. |
| - Until *time.Time |
| + // Until is the current period's end for a subscription, a comp's |
| + // ends_at; nil when there is none. |
| + Until *time.Time |
| + // PeriodStart is a subscription's current period's start, nil when |
| + // unknown. For a past_due one it is where the unpaid period began, |
| + // which the 30 days count from. |
| + PeriodStart *time.Time |
| CancelAtPeriodEnd bool |
| // Grants is whether this holding gives access at the moment Holdings |
| // was asked. |
| @@ -52,14 +58,14 @@ type Holding struct { |
| // |
| // active, trialing grant, unless cancelling: then only |
| // while now < periodEnd |
| -// past_due grants while now < periodEnd + 30 days |
| +// past_due grants while now < periodStart + 30 days |
| // canceled, unpaid, incomplete, |
| // incomplete_expired, paused, anything |
| // Stripe adds later does not |
| // |
| // backstop is true for a past_due subscription held only by the 30-day |
| -// window, that is once now has reached periodEnd. |
| -func grants(status string, periodEnd *time.Time, cancelAtPeriodEnd bool, now time.Time) (grants, backstop bool) { |
| +// window, that is once now has reached periodStart. |
| +func grants(status string, periodStart, periodEnd *time.Time, cancelAtPeriodEnd bool, now time.Time) (grants, backstop bool) { |
| switch status { |
| case "active", "trialing": |
| // A subscription set to end with its period grants until the period |
| @@ -71,17 +77,19 @@ func grants(status string, periodEnd *time.Time, cancelAtPeriodEnd bool, now tim |
| } |
| return true, false |
| case "past_due": |
| - if periodEnd == nil { |
| - // Stripe is retrying the card and we have no period end to count |
| + // Counted from the unpaid period's start, never its end: Stripe |
| + // moved the end a whole period on when the renewal failed. |
| + if periodStart == nil { |
| + // Stripe is retrying the card and we have no period start to count |
| // the 30 days from. Nothing says the access should have stopped, so |
| // it grants, and it is not flagged as held by the backstop because |
| // there is no backstop to hold it. |
| return true, false |
| } |
| - if !now.Before(periodEnd.Add(backstopWindow)) { |
| + if !now.Before(periodStart.Add(backstopWindow)) { |
| return false, false |
| } |
| - return true, !now.Before(*periodEnd) |
| + return true, !now.Before(*periodStart) |
| } |
| return false, false |
| } |
| @@ -100,7 +108,7 @@ func (h Holding) evaluate(now time.Time) (ok, held bool) { |
| if h.Source == SourceComp { |
| return compGrants(h.Status, h.Until, now), false |
| } |
| - return grants(h.Status, h.Until, h.CancelAtPeriodEnd, now) |
| + return grants(h.Status, h.PeriodStart, h.Until, h.CancelAtPeriodEnd, now) |
| } |
| type subscriptionRow struct { |
| @@ -108,6 +116,7 @@ type subscriptionRow struct { |
| Subject string |
| Plan string |
| Status string |
| + CurrentPeriodStart *time.Time |
| CurrentPeriodEnd *time.Time |
| CancelAtPeriodEnd bool |
| CreatedAt time.Time |
| @@ -142,7 +151,7 @@ func (s *Service) Holdings(ctx context.Context, subject string) ([]Holding, erro |
| for _, r := range subs { |
| h := Holding{ |
| Plan: r.Plan, Source: SourceSubscription, Status: r.Status, |
| - Until: utc(r.CurrentPeriodEnd), CancelAtPeriodEnd: r.CancelAtPeriodEnd, |
| + Until: utc(r.CurrentPeriodEnd), PeriodStart: utc(r.CurrentPeriodStart), CancelAtPeriodEnd: r.CancelAtPeriodEnd, |
| SubscriptionID: r.StripeSubscriptionID, |
| } |
| h.Grants, h.Backstop = h.evaluate(now) |
diff --git a/holdings_test.go b/holdings_test.go| index 4ff6f5a..8a92188 100644 |
| --- a/holdings_test.go |
| +++ b/holdings_test.go |
| @@ -23,6 +23,7 @@ func TestHoldingTable(t *testing.T) { |
| source abono.Source |
| status string |
| end *time.Time // current_period_end, or a comp's ends_at |
| + start *time.Time // current_period_start |
| cancel bool |
| now time.Time |
| grants bool |
| @@ -43,14 +44,19 @@ func TestHoldingTable(t *testing.T) { |
| {name: "trialing, cancelling, one second after the period end", status: "trialing", end: ptr(end), cancel: true, now: at(time.Second), grants: false}, |
| {name: "trialing, cancelling, a day after the period end", status: "trialing", end: ptr(end), cancel: true, now: at(day), grants: false}, |
| {name: "active, cancelling, no period end", status: "active", cancel: true, now: at(400 * day), grants: true}, |
| - {name: "past_due, cancelling, keeps the past_due rule", status: "past_due", end: ptr(end), cancel: true, now: at(10 * day), grants: true, backstop: true}, |
| - {name: "past_due before the period end", status: "past_due", end: ptr(end), now: at(-time.Second), grants: true}, |
| - {name: "past_due at the period end", status: "past_due", end: ptr(end), now: at(0), grants: true, backstop: true}, |
| - {name: "past_due, 29 days past", status: "past_due", end: ptr(end), now: at(29 * day), grants: true, backstop: true}, |
| - {name: "past_due, one second before the backstop ends", status: "past_due", end: ptr(end), now: at(30*day - time.Second), grants: true, backstop: true}, |
| - {name: "past_due at the backstop", status: "past_due", end: ptr(end), now: at(30 * day), grants: false}, |
| - {name: "past_due past the backstop", status: "past_due", end: ptr(end), now: at(31 * day), grants: false}, |
| - {name: "past_due with no period end", status: "past_due", now: at(400 * day), grants: true}, |
| + // past_due (ruling R21): a failed renewal has already moved the |
| + // period on, so current_period_end is the unpaid period's end and |
| + // current_period_start the paid one's. The 30 days count from the |
| + // start; the end, a year off here, does not matter. |
| + {name: "past_due, cancelling, keeps the past_due rule", status: "past_due", start: ptr(end), end: ptr(end.AddDate(1, 0, 0)), cancel: true, now: at(10 * day), grants: true, backstop: true}, |
| + {name: "past_due before the period start", status: "past_due", start: ptr(end), end: ptr(end.AddDate(1, 0, 0)), now: at(-time.Second), grants: true}, |
| + {name: "past_due at the period start", status: "past_due", start: ptr(end), end: ptr(end.AddDate(1, 0, 0)), now: at(0), grants: true, backstop: true}, |
| + {name: "past_due, 29 days past the start", status: "past_due", start: ptr(end), end: ptr(end.AddDate(1, 0, 0)), now: at(29 * day), grants: true, backstop: true}, |
| + {name: "past_due, one second before start+30d", status: "past_due", start: ptr(end), end: ptr(end.AddDate(1, 0, 0)), now: at(30*day - time.Second), grants: true, backstop: true}, |
| + {name: "past_due at exactly start+30d", status: "past_due", start: ptr(end), end: ptr(end.AddDate(1, 0, 0)), now: at(30 * day), grants: false}, |
| + {name: "past_due past start+30d, the unpaid period's end still months off", status: "past_due", start: ptr(end), end: ptr(end.AddDate(1, 0, 0)), now: at(31 * day), grants: false}, |
| + {name: "past_due with no period start", status: "past_due", end: ptr(end), now: at(400 * day), grants: true}, |
| + {name: "past_due with no period at all", status: "past_due", now: at(400 * day), grants: true}, |
| {name: "canceled", status: "canceled", end: ptr(end), now: at(-day), grants: false}, |
| {name: "unpaid", status: "unpaid", end: ptr(end), now: at(-day), grants: false}, |
| {name: "incomplete", status: "incomplete", end: ptr(end), now: at(-day), grants: false}, |
| @@ -76,9 +82,9 @@ func TestHoldingTable(t *testing.T) { |
| db.exec(`INSERT INTO abono_comps (subject, plan, status, ends_at, created_at, updated_at) VALUES (?, 'annual', ?, ?, ?, ?)`, |
| sub, c.status, c.end, t0, t0) |
| } else { |
| - db.exec(`INSERT INTO abono_subscriptions (stripe_subscription_id, stripe_customer_id, subject, plan, stripe_price_id, status, current_period_end, cancel_at_period_end, created_at, updated_at) |
| - VALUES ('sub_1', 'cus_1', ?, 'annual', 'price_A', ?, ?, ?, ?, ?)`, |
| - sub, c.status, c.end, c.cancel, t0, t0) |
| + db.exec(`INSERT INTO abono_subscriptions (stripe_subscription_id, stripe_customer_id, subject, plan, stripe_price_id, status, current_period_start, current_period_end, cancel_at_period_end, created_at, updated_at) |
| + VALUES ('sub_1', 'cus_1', ?, 'annual', 'price_A', ?, ?, ?, ?, ?, ?)`, |
| + sub, c.status, c.start, c.end, c.cancel, t0, t0) |
| } |
| hs, err := s.Holdings(ctx, sub) |
| @@ -102,6 +108,9 @@ func TestHoldingTable(t *testing.T) { |
| if (h.Until == nil) != (c.end == nil) || (h.Until != nil && !h.Until.Equal(*c.end)) { |
| t.Errorf("Until = %v, want %v", h.Until, c.end) |
| } |
| + if (h.PeriodStart == nil) != (c.start == nil) || (h.PeriodStart != nil && !h.PeriodStart.Equal(*c.start)) { |
| + t.Errorf("PeriodStart = %v, want %v", h.PeriodStart, c.start) |
| + } |
| if h.Source == abono.SourceSubscription && h.SubscriptionID != "sub_1" || h.Source == abono.SourceComp && h.CompID == 0 { |
| t.Errorf("ids missing: %+v", h) |
| } |
diff --git a/internal/fakestripe/fake.go b/internal/fakestripe/fake.go| index 1aba31e..28a7549 100644 |
| --- a/internal/fakestripe/fake.go |
| +++ b/internal/fakestripe/fake.go |
| @@ -70,10 +70,10 @@ type Subscription struct { |
| Price string |
| // Interval is the price's when empty, and "month" for a price nobody |
| // added. |
| - Interval string |
| - PeriodEnd time.Time |
| - CancelAtPeriodEnd bool |
| - Metadata map[string]string |
| + Interval string |
| + PeriodStart, PeriodEnd time.Time |
| + CancelAtPeriodEnd bool |
| + Metadata map[string]string |
| } |
| // Session is what a test sets of a Checkout session. |
| @@ -102,7 +102,13 @@ type Fake struct { |
| portalForms []url.Values |
| priceFetches map[string]int |
| onFetch func() |
| - failures []error |
| + |
| + // queue is the events not yet delivered, oldest first; failures says |
| + // why the first of them was not. sending is held while a queue is |
| + // being sent, so events go out one at a time and in order. |
| + queue [][]byte |
| + failures []error |
| + sending sync.Mutex |
| } |
| type price struct { |
| @@ -200,7 +206,7 @@ func (f *Fake) PutSubscription(s Subscription) string { |
| } |
| f.subs[s.ID] = &subscription{ |
| id: s.ID, customer: s.Customer, status: s.Status, price: s.Price, interval: s.Interval, |
| - periodEnd: s.PeriodEnd, cancel: s.CancelAtPeriodEnd, metadata: s.Metadata, |
| + periodStart: s.PeriodStart, periodEnd: s.PeriodEnd, cancel: s.CancelAtPeriodEnd, metadata: s.Metadata, |
| } |
| return s.ID |
| } |
| @@ -274,8 +280,9 @@ func (f *Fake) OnSubscriptionFetch(fn func()) { |
| f.onFetch = fn |
| } |
| -// DeliveryErrors is every event that could not be delivered: the |
| -// endpoint was unreachable or did not answer 2xx. |
| +// DeliveryErrors is why events are still waiting to be delivered: each |
| +// failed attempt since the queue was last empty. A delivery that later |
| +// succeeds clears them. |
| func (f *Fake) DeliveryErrors() []error { |
| f.mu.Lock() |
| defer f.mu.Unlock() |
| @@ -298,23 +305,37 @@ func (f *Fake) Advance(d time.Duration) { |
| f.Tick() |
| } |
| -// Tick does what is due at the fake's now, and sends its events. A |
| -// subscription whose period has ended renews, paid, and moves to the next |
| +// Tick sends any events still waiting, then does what is due at the |
| +// fake's now and sends its events. A subscription set to cancel at its |
| +// period end ends then, whatever its status. Otherwise an active or |
| +// trialing one whose period has ended renews, paid, and moves to the next |
| // period; with FailNextRenewal it goes past_due instead (the period still |
| -// moves on, as Stripe's does: the failed invoice is for the new one); set |
| -// to cancel at the period end, it ends. A past_due subscription is left |
| -// alone: Stripe's retries and what follows them are not modelled. |
| -func (f *Fake) Tick() { |
| +// moves on, as Stripe's does: the failed invoice is for the new one, and |
| +// current_period_start is where it began). A past_due subscription is not |
| +// renewed again: Stripe's retries and what follows them are not modelled. |
| +// A delivery that fails stays queued for the next Tick or page action; |
| +// DeliveryErrors says why. |
| +func (f *Fake) Tick() { _ = f.tick(context.Background()) } |
| + |
| +func (f *Fake) tick(ctx context.Context) error { |
| + if err := f.flush(ctx); err != nil { |
| + return err |
| + } |
| f.mu.Lock() |
| now := f.now() |
| var events [][]byte |
| for _, id := range f.order { |
| s := f.subs[id] |
| - for (s.status == "active" || s.status == "trialing") && !s.periodEnd.IsZero() && !now.Before(s.periodEnd) { |
| - switch { |
| - case s.cancel: |
| + for live(s.status) && !s.periodEnd.IsZero() && !now.Before(s.periodEnd) { |
| + if s.cancel { |
| s.status = "canceled" |
| events = append(events, f.event("customer.subscription.deleted", f.subObject(s, false))) |
| + break |
| + } |
| + if s.status == "past_due" { |
| + break |
| + } |
| + switch { |
| case f.failNext[id]: |
| delete(f.failNext, id) |
| s.status = "past_due" |
| @@ -330,7 +351,7 @@ func (f *Fake) Tick() { |
| } |
| } |
| f.mu.Unlock() |
| - f.deliver(context.Background(), events) |
| + return f.deliver(ctx, events) |
| } |
| // SendEvent sends an event of typ about subscription subID as it is now, |
| @@ -344,7 +365,7 @@ func (f *Fake) SendEvent(typ, subID string) { |
| } |
| e := f.event(typ, f.subObject(s, false)) |
| f.mu.Unlock() |
| - f.deliver(context.Background(), [][]byte{e}) |
| + _ = f.deliver(context.Background(), [][]byte{e}) |
| } |
| // Handler serves the API (/v1/...) and the pages (/checkout/{id}, |
| @@ -557,9 +578,19 @@ func (f *Fake) promotionCodes(w http.ResponseWriter, r *http.Request) { |
| // customer (the one Checkout was given, else a new one with email), an |
| // active subscription on the session's price for one period, and the |
| // session complete and paid. It returns where Checkout sends the buyer. |
| +// Paying a session already paid buys nothing more and returns the same |
| +// place: the retry after a delivery that failed. |
| func (f *Fake) pay(ctx context.Context, id, email string) (string, error) { |
| + if err := f.flush(ctx); err != nil { |
| + return "", err |
| + } |
| f.mu.Lock() |
| s := f.sessions[id] |
| + if s != nil && s.Status == "complete" && s.successURL != "" { |
| + back := strings.ReplaceAll(s.successURL, "{CHECKOUT_SESSION_ID}", s.ID) |
| + f.mu.Unlock() |
| + return back, nil |
| + } |
| if err := usable(s); err != nil { |
| f.mu.Unlock() |
| return "", err |
| @@ -597,13 +628,15 @@ func (f *Fake) pay(ctx context.Context, id, email string) (string, error) { |
| } |
| back := strings.ReplaceAll(s.successURL, "{CHECKOUT_SESSION_ID}", s.ID) |
| f.mu.Unlock() |
| - f.deliver(ctx, events) |
| - return back, nil |
| + return back, f.deliver(ctx, events) |
| } |
| // decline ends session id with nothing bought, and returns where Checkout |
| // sends the buyer: a payment that fails the sandbox's way. |
| func (f *Fake) decline(ctx context.Context, id string) (string, error) { |
| + if err := f.flush(ctx); err != nil { |
| + return "", err |
| + } |
| f.mu.Lock() |
| s := f.sessions[id] |
| if err := usable(s); err != nil { |
| @@ -614,12 +647,16 @@ func (f *Fake) decline(ctx context.Context, id string) (string, error) { |
| e := f.event("checkout.session.expired", f.sessionObject(s)) |
| back := s.cancelURL |
| f.mu.Unlock() |
| - f.deliver(ctx, [][]byte{e}) |
| - return back, nil |
| + return back, f.deliver(ctx, [][]byte{e}) |
| } |
| // cancel sets customer's subscription subID to end with its period. |
| +// Cancelling one already cancelling changes nothing, but still sends |
| +// whatever is waiting: the retry after a delivery that failed. |
| func (f *Fake) cancel(ctx context.Context, customer, subID string) error { |
| + if err := f.flush(ctx); err != nil { |
| + return err |
| + } |
| f.mu.Lock() |
| s := f.subs[subID] |
| if s == nil || s.customer != customer { |
| @@ -633,8 +670,7 @@ func (f *Fake) cancel(ctx context.Context, customer, subID string) error { |
| s.cancel = true |
| e := f.event("customer.subscription.updated", f.subObject(s, false)) |
| f.mu.Unlock() |
| - f.deliver(ctx, [][]byte{e}) |
| - return nil |
| + return f.deliver(ctx, [][]byte{e}) |
| } |
| var ( |
| @@ -687,28 +723,52 @@ func (f *Fake) event(typ string, object map[string]any) []byte { |
| return b |
| } |
| -// deliver sends events in order. Call it without f.mu: the receiver |
| -// usually calls the API back. |
| -func (f *Fake) deliver(ctx context.Context, events [][]byte) { |
| +// deliver queues events behind any still waiting and sends the queue. |
| +// Call it without f.mu: the receiver usually calls the API back. |
| +func (f *Fake) deliver(ctx context.Context, events [][]byte) error { |
| f.mu.Lock() |
| - o := f.opts |
| - now := f.now() |
| + f.queue = append(f.queue, events...) |
| f.mu.Unlock() |
| - for _, e := range events { |
| + return f.flush(ctx) |
| +} |
| + |
| +// flush sends the queued events in order, stopping at the first that |
| +// fails, which stays at the head of the queue with the rest behind it. |
| +// Each attempt is signed afresh, so a retry is never stale. A failure to |
| +// hand an event to Options.Deliver is returned, so the sandbox's page |
| +// action fails rather than pretend; a failed webhook POST is not, since |
| +// Stripe's own pages carry on when a webhook fails. |
| +func (f *Fake) flush(ctx context.Context) error { |
| + f.sending.Lock() |
| + defer f.sending.Unlock() |
| + for { |
| + f.mu.Lock() |
| + if len(f.queue) == 0 { |
| + f.failures = nil |
| + f.mu.Unlock() |
| + return nil |
| + } |
| + e, o, now := f.queue[0], f.opts, f.now() |
| + f.mu.Unlock() |
| var err error |
| switch { |
| case o.Deliver != nil: |
| err = o.Deliver(ctx, e) |
| case o.WebhookURL != "": |
| err = post(ctx, o.WebhookURL, e, sign(o.WebhookSecret, e, now)) |
| - default: |
| - continue |
| } |
| + f.mu.Lock() |
| if err != nil { |
| - f.mu.Lock() |
| - f.failures = append(f.failures, fmt.Errorf("fakestripe: delivering %s: %w", e, err)) |
| + err = fmt.Errorf("fakestripe: delivering %s: %w", e, err) |
| + f.failures = append(f.failures, err) |
| f.mu.Unlock() |
| + if o.Deliver != nil { |
| + return err |
| + } |
| + return nil |
| } |
| + f.queue = f.queue[1:] |
| + f.mu.Unlock() |
| } |
| } |
diff --git a/internal/fakestripe/fake_test.go b/internal/fakestripe/fake_test.go| index c529189..e55e90d 100644 |
| --- a/internal/fakestripe/fake_test.go |
| +++ b/internal/fakestripe/fake_test.go |
| @@ -3,6 +3,7 @@ package fakestripe_test |
| import ( |
| "context" |
| "encoding/json" |
| + "errors" |
| "io" |
| "net/http" |
| "net/http/httptest" |
| @@ -224,6 +225,9 @@ func TestFakeCheckoutPayCreatesAnActiveSubscriptionAndSignsAnEvent(t *testing.T) |
| t.Errorf("subscription = %+v", sub) |
| } |
| // 2025-03-31.basil: the period end is on the items, not the subscription. |
| + if !sub.PeriodStart().Equal(t0) { |
| + t.Errorf("period start = %v, want %v", sub.PeriodStart(), t0) |
| + } |
| if sub.CurrentPeriodEnd != 0 || !sub.PeriodEnd().Equal(t0.AddDate(1, 0, 0)) { |
| t.Errorf("period end: top level %d, items %v; want only the items, a year on", sub.CurrentPeriodEnd, sub.PeriodEnd()) |
| } |
| @@ -240,9 +244,14 @@ func TestFakeCheckoutPayCreatesAnActiveSubscriptionAndSignsAnEvent(t *testing.T) |
| t.Errorf("events = %v, want %v", got, want) |
| } |
| - // Paid is paid: the page and its buttons are gone. |
| - if code, _ := post(t, cs.URL+"/pay", nil); code != http.StatusGone { |
| - t.Errorf("paying twice: status %d, want 410", code) |
| + // Paid is paid: paying again (a resubmitted form, or a retry after a |
| + // failed delivery) goes to the success page again and buys nothing, |
| + // and the page and its buttons are gone. |
| + if code, loc := post(t, cs.URL+"/pay", nil); code != http.StatusSeeOther || loc != "https://site.example/done?session_id=cs_1" { |
| + t.Errorf("paying twice: %d to %q, want 303 to the success URL", code, loc) |
| + } |
| + if _, err := r.client.GetSubscription(ctx, "sub_2"); !stripeapi.NotFound(err) { |
| + t.Errorf("paying twice made a second subscription: %v", err) |
| } |
| code, page = get(t, cs.URL) |
| if code != http.StatusGone { |
| @@ -303,6 +312,11 @@ func TestFakeRenewalFailureGoesPastDue(t *testing.T) { |
| if sub.Status != "past_due" || !sub.PeriodEnd().Equal(t0.AddDate(3, 0, 0)) { |
| t.Errorf("after a failed renewal: %s until %v, want past_due until %v", sub.Status, sub.PeriodEnd(), t0.AddDate(3, 0, 0)) |
| } |
| + // The unpaid period starts where the paid one ended: abono counts its |
| + // 30 days from here (ruling R21). |
| + if !sub.PeriodStart().Equal(t0.AddDate(2, 0, 0)) { |
| + t.Errorf("after a failed renewal the period starts %v, want %v", sub.PeriodStart(), t0.AddDate(2, 0, 0)) |
| + } |
| if got, want := types(t, r.hook.take(), now), []string{"invoice.payment_failed", "customer.subscription.updated"}; !reflect.DeepEqual(got, want) { |
| t.Errorf("failure events = %v, want %v", got, want) |
| } |
| @@ -423,13 +437,129 @@ func TestFakeAPIIsStripes(t *testing.T) { |
| } |
| } |
| -func TestFakeRecordsAFailedDelivery(t *testing.T) { |
| +func TestFakeKeepsAFailedWebhookDeliveryForLater(t *testing.T) { |
| r := start(t) |
| r.hook.mu.Lock() |
| r.hook.code = http.StatusInternalServerError |
| r.hook.mu.Unlock() |
| - r.subscribe(t) |
| - if errs := r.f.DeliveryErrors(); len(errs) != 2 { |
| - t.Errorf("delivery errors = %v, want one for each of the two events", errs) |
| + |
| + // Like Stripe, Checkout carries on when the webhook fails; the event |
| + // waits, and the rest wait behind it. |
| + cs := r.checkout(t) |
| + if code, _ := post(t, cs.URL+"/pay", nil); code != http.StatusSeeOther { |
| + t.Fatalf("Pay with a failing webhook: status %d, want 303", code) |
| + } |
| + if errs := r.f.DeliveryErrors(); len(errs) != 1 { |
| + t.Errorf("delivery errors = %v, want one", errs) |
| + } |
| + if got := len(r.hook.take()); got != 1 { |
| + t.Errorf("attempts = %d, want 1: the second event waits behind the first", got) |
| + } |
| + |
| + r.hook.mu.Lock() |
| + r.hook.code = 0 |
| + r.hook.mu.Unlock() |
| + r.f.Tick() |
| + if got, want := types(t, r.hook.take(), t0), []string{"customer.subscription.created", "checkout.session.completed"}; !reflect.DeepEqual(got, want) { |
| + t.Errorf("redelivered %v, want %v in order", got, want) |
| + } |
| + if errs := r.f.DeliveryErrors(); len(errs) != 0 { |
| + t.Errorf("delivery errors after redelivery = %v, want none", errs) |
| + } |
| +} |
| + |
| +// A sandbox's in-process delivery that fails makes the action fail, and |
| +// the event waits: the next action sends it first, without redoing the |
| +// action. |
| +func TestFakeRetriesAFailedInProcessDelivery(t *testing.T) { |
| + var mu sync.Mutex |
| + var got []string |
| + failNext := false |
| + deliver := func(_ context.Context, body []byte) error { |
| + var e struct { |
| + Type string `json:"type"` |
| + } |
| + _ = json.Unmarshal(body, &e) |
| + mu.Lock() |
| + defer mu.Unlock() |
| + if failNext { |
| + failNext = false |
| + return errors.New("the app's OnChange failed") |
| + } |
| + got = append(got, e.Type) |
| + return nil |
| + } |
| + srv := httptest.NewUnstartedServer(nil) |
| + base := "http://" + srv.Listener.Addr().String() |
| + f := fakestripe.New(fakestripe.Options{Deliver: deliver, PageBase: base, Now: func() time.Time { return t0 }}) |
| + srv.Config.Handler = f.Handler() |
| + srv.Start() |
| + t.Cleanup(srv.Close) |
| + f.AddPrice("price_A", "annual", 6000, "eur", "year") |
| + r := &rig{f: f, base: base, client: &stripeapi.Client{Key: "rk_test_fake", Base: base}, hook: &hook{}} |
| + id := r.subscribe(t) |
| + u, err := r.client.PortalURL(context.Background(), "cus_1", "https://site.example/account") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + mu.Lock() |
| + got, failNext = nil, true |
| + mu.Unlock() |
| + |
| + if code, _ := post(t, u+"/cancel", url.Values{"subscription": {id}}); code != http.StatusInternalServerError { |
| + t.Fatalf("Cancel whose delivery failed: status %d, want 500", code) |
| + } |
| + if len(f.DeliveryErrors()) != 1 { |
| + t.Errorf("delivery errors = %v, want one", f.DeliveryErrors()) |
| + } |
| + code, loc := post(t, u+"/cancel", url.Values{"subscription": {id}}) |
| + if code != http.StatusSeeOther || loc != u { |
| + t.Fatalf("Cancel again: %d to %q, want 303 back to the portal", code, loc) |
| + } |
| + mu.Lock() |
| + defer mu.Unlock() |
| + if want := []string{"customer.subscription.updated"}; !reflect.DeepEqual(got, want) { |
| + t.Errorf("delivered %v, want the one queued event once", got) |
| + } |
| + if sub := r.sub(t, id); sub.Status != "active" || !sub.CancelAtPeriodEnd { |
| + t.Errorf("subscription = %s, cancelling %v", sub.Status, sub.CancelAtPeriodEnd) |
| + } |
| + var ids []string |
| + _ = r.client.ListSubscriptions(context.Background(), func(s stripeapi.Subscription) error { ids = append(ids, s.ID); return nil }) |
| + if !reflect.DeepEqual(ids, []string{id}) { |
| + t.Errorf("subscriptions = %v, want only %s", ids, id) |
| + } |
| +} |
| + |
| +// A subscription cancelled while past_due ends at its period end too. |
| +func TestFakeCancelAfterAFailedRenewalEnds(t *testing.T) { |
| + r := start(t) |
| + id := r.subscribe(t) |
| + r.f.FailNextRenewal(id) |
| + r.f.Advance(366 * 24 * time.Hour) |
| + if sub := r.sub(t, id); sub.Status != "past_due" { |
| + t.Fatalf("after a failed renewal: %s", sub.Status) |
| + } |
| + r.hook.take() |
| + |
| + u, err := r.client.PortalURL(context.Background(), "cus_1", "https://site.example/account") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if code, _ := post(t, u+"/cancel", url.Values{"subscription": {id}}); code != http.StatusSeeOther { |
| + t.Fatalf("Cancel: status %d", code) |
| + } |
| + if sub := r.sub(t, id); sub.Status != "past_due" || !sub.CancelAtPeriodEnd { |
| + t.Errorf("after cancelling: %s, cancelling %v", sub.Status, sub.CancelAtPeriodEnd) |
| + } |
| + if got, want := types(t, r.hook.take(), t0.Add(366*24*time.Hour)), []string{"customer.subscription.updated"}; !reflect.DeepEqual(got, want) { |
| + t.Errorf("events = %v, want %v", got, want) |
| + } |
| + r.f.Advance(366 * 24 * time.Hour) |
| + if sub := r.sub(t, id); sub.Status != "canceled" { |
| + t.Errorf("past the period end: %s, want canceled", sub.Status) |
| + } |
| + if got, want := types(t, r.hook.take(), t0.Add(2*366*24*time.Hour)), []string{"customer.subscription.deleted"}; !reflect.DeepEqual(got, want) { |
| + t.Errorf("events = %v, want %v", got, want) |
| } |
| } |
diff --git a/internal/fakestripe/pages.go b/internal/fakestripe/pages.go| index d1e9484..d7d4a2a 100644 |
| --- a/internal/fakestripe/pages.go |
| +++ b/internal/fakestripe/pages.go |
| @@ -125,7 +125,10 @@ func (f *Fake) action(kind, id, verb string) string { |
| func (f *Fake) checkoutPage(w http.ResponseWriter, r *http.Request) { |
| // The page is where the reader sees time pass in the sandbox. |
| - f.Tick() |
| + if err := f.tick(r.Context()); err != nil { |
| + http.Error(w, err.Error(), http.StatusInternalServerError) |
| + return |
| + } |
| id := r.PathValue("id") |
| f.mu.Lock() |
| s := f.sessions[id] |
| @@ -167,7 +170,10 @@ func (f *Fake) priceLine(p *price) string { |
| } |
| func (f *Fake) portalPage(w http.ResponseWriter, r *http.Request) { |
| - f.Tick() |
| + if err := f.tick(r.Context()); err != nil { |
| + http.Error(w, err.Error(), http.StatusInternalServerError) |
| + return |
| + } |
| customer := r.PathValue("customer") |
| f.mu.Lock() |
| var items []portalItem |
diff --git a/internal/stripeapi/client.go b/internal/stripeapi/client.go| index d3b4d50..cbf91b1 100644 |
| --- a/internal/stripeapi/client.go |
| +++ b/internal/stripeapi/client.go |
| @@ -39,18 +39,21 @@ type Subscription struct { |
| ID string `json:"id"` |
| Status string `json:"status"` |
| CancelAtPeriodEnd bool `json:"cancel_at_period_end"` |
| - // CurrentPeriodEnd moved onto the items in 2025-03-31.basil; the |
| - // top-level one is read for an older account version's webhooks. |
| - CurrentPeriodEnd int64 `json:"current_period_end"` |
| - Customer struct { |
| + // CurrentPeriodStart and CurrentPeriodEnd moved onto the items in |
| + // 2025-03-31.basil; the top-level ones are read for an older account |
| + // version's webhooks. |
| + CurrentPeriodStart int64 `json:"current_period_start"` |
| + CurrentPeriodEnd int64 `json:"current_period_end"` |
| + Customer struct { |
| ID string `json:"id"` |
| Email string `json:"email"` |
| Name string `json:"name"` |
| } `json:"customer"` |
| Items struct { |
| Data []struct { |
| - CurrentPeriodEnd int64 `json:"current_period_end"` |
| - Price struct { |
| + CurrentPeriodStart int64 `json:"current_period_start"` |
| + CurrentPeriodEnd int64 `json:"current_period_end"` |
| + Price struct { |
| ID string `json:"id"` |
| Recurring struct { |
| Interval string `json:"interval"` |
| @@ -75,6 +78,22 @@ func (s Subscription) PeriodEnd() time.Time { |
| return time.Unix(end, 0).UTC() |
| } |
| +// PeriodStart is when the current period began, or the zero time. After |
| +// a failed renewal Stripe has already moved the period on, so this is |
| +// where the unpaid period starts: the end of the one last paid for. |
| +func (s Subscription) PeriodStart() time.Time { |
| + start := s.CurrentPeriodStart |
| + for _, it := range s.Items.Data { |
| + if it.CurrentPeriodStart > start { |
| + start = it.CurrentPeriodStart |
| + } |
| + } |
| + if start == 0 { |
| + return time.Time{} |
| + } |
| + return time.Unix(start, 0).UTC() |
| +} |
| + |
| // PriceID is the first item's price. |
| func (s Subscription) PriceID() (id, interval string) { |
| if len(s.Items.Data) == 0 { |
diff --git a/internal/stripeapi/client_test.go b/internal/stripeapi/client_test.go| index 3dae21c..f5be0ae 100644 |
| --- a/internal/stripeapi/client_test.go |
| +++ b/internal/stripeapi/client_test.go |
| @@ -17,8 +17,8 @@ import ( |
| func TestSubscriptionWire(t *testing.T) { |
| t.Parallel() |
| for name, raw := range map[string]string{ |
| - "basil, expanded": `{"id":"sub_1","status":"active","customer":{"id":"cus_1","email":"a@example.com"},"items":{"data":[{"current_period_end":1800000000,"price":{"id":"price_A","recurring":{"interval":"year"}}}]}}`, |
| - "older, id only": `{"id":"sub_1","status":"active","customer":"cus_1","current_period_end":1800000000,"items":{"data":[{"price":{"id":"price_A","recurring":{"interval":"year"}}}]}}`, |
| + "basil, expanded": `{"id":"sub_1","status":"active","customer":{"id":"cus_1","email":"a@example.com"},"items":{"data":[{"current_period_start":1700000000,"current_period_end":1800000000,"price":{"id":"price_A","recurring":{"interval":"year"}}}]}}`, |
| + "older, id only": `{"id":"sub_1","status":"active","customer":"cus_1","current_period_start":1700000000,"current_period_end":1800000000,"items":{"data":[{"price":{"id":"price_A","recurring":{"interval":"year"}}}]}}`, |
| } { |
| var w subscriptionWire |
| if err := json.Unmarshal([]byte(raw), &w); err != nil { |
| @@ -29,10 +29,15 @@ func TestSubscriptionWire(t *testing.T) { |
| t.Fatal(err) |
| } |
| price, interval := s.PriceID() |
| - if s.Customer.ID != "cus_1" || price != "price_A" || interval != "year" || !s.PeriodEnd().Equal(time.Unix(1_800_000_000, 0)) { |
| + if s.Customer.ID != "cus_1" || price != "price_A" || interval != "year" || !s.PeriodEnd().Equal(time.Unix(1_800_000_000, 0)) || |
| + !s.PeriodStart().Equal(time.Unix(1_700_000_000, 0)) { |
| t.Errorf("%s: %+v", name, s) |
| } |
| } |
| + var none Subscription |
| + if !none.PeriodStart().IsZero() || !none.PeriodEnd().IsZero() { |
| + t.Error("a subscription with no period has a start or an end") |
| + } |
| } |
| // fake serves h and returns a Client pointed at it with key "k". |
diff --git a/migrations/0001_init.sql b/migrations/0001_init.sql| index afb317a..00bee34 100644 |
| --- a/migrations/0001_init.sql |
| +++ b/migrations/0001_init.sql |
| @@ -44,6 +44,7 @@ CREATE TABLE abono_subscriptions ( |
| plan TEXT NOT NULL DEFAULT '', |
| stripe_price_id TEXT NOT NULL DEFAULT '', |
| status TEXT NOT NULL, |
| + current_period_start DATETIME, |
| current_period_end DATETIME, |
| cancel_at_period_end INTEGER NOT NULL DEFAULT 0, |
| email TEXT NOT NULL DEFAULT '', |
diff --git a/sandbox_test.go b/sandbox_test.go| index ca458be..3ef56d2 100644 |
| --- a/sandbox_test.go |
| +++ b/sandbox_test.go |
| @@ -3,6 +3,7 @@ package abono_test |
| import ( |
| "bytes" |
| "context" |
| + "errors" |
| "io" |
| "log/slog" |
| "net/http" |
| @@ -13,6 +14,8 @@ import ( |
| "testing" |
| "time" |
| + "gorm.io/gorm" |
| + |
| "amadan.net/rastrillo/rastrillo/csrf" |
| "amadan.net/rastrillo/rastrillo/migrate" |
| "amadan.net/rastrillo/rastrillo/sessions" |
| @@ -241,3 +244,80 @@ func TestSandboxSurvivesARestart(t *testing.T) { |
| t.Errorf("subscriptions are held by %v, want ana and bo", subs) |
| } |
| } |
| + |
| +// TestSandboxRetriesADeliveryThatFailed: when the app fails an event |
| +// (OnChange errors), the sandbox's page answers 500 rather than pretend, |
| +// and keeps the event; doing it again sends it, without a second |
| +// subscription. |
| +func TestSandboxRetriesADeliveryThatFailed(t *testing.T) { |
| + ctx := context.Background() |
| + d := openDB(t) |
| + if _, err := migrate.Apply(ctx, d, migrate.Merge(sessions.Schema, abono.Schema)); err != nil { |
| + t.Fatalf("migrate.Apply: %v", err) |
| + } |
| + const origin = "http://localhost:8080" |
| + var mu sync.Mutex |
| + fail := false |
| + s, err := abono.New(abono.Config{ |
| + DB: d.G, Origin: origin, Sandbox: true, Now: func() time.Time { return t0 }, |
| + Holder: func(context.Context, *gorm.DB, string, string) (string, error) { return "ana", nil }, |
| + OnChange: func(context.Context, *gorm.DB, string) error { |
| + mu.Lock() |
| + defer mu.Unlock() |
| + if fail { |
| + fail = false |
| + return errors.New("the app is down") |
| + } |
| + return nil |
| + }, |
| + Logger: slog.New(slog.NewTextHandler(&bytes.Buffer{}, nil)), |
| + }) |
| + if err != nil { |
| + t.Fatalf("New: %v", err) |
| + } |
| + if err := s.SeedPlans(ctx, []abono.PlanSpec{{Plan: "annual", Sold: "price_annual"}}); err != nil { |
| + t.Fatalf("SeedPlans: %v", err) |
| + } |
| + post := func(u string, form url.Values) int { |
| + t.Helper() |
| + r := httptest.NewRequest(http.MethodPost, strings.TrimPrefix(u, origin), strings.NewReader(form.Encode())) |
| + r.Header.Set("Content-Type", "application/x-www-form-urlencoded") |
| + w := httptest.NewRecorder() |
| + s.SandboxHandler().ServeHTTP(w, r) |
| + return w.Code |
| + } |
| + |
| + u, err := s.Checkout(ctx, abono.CheckoutRequest{Plan: "annual", Email: "ana@example.com", SuccessPath: "/done"}) |
| + if err != nil { |
| + t.Fatalf("Checkout: %v", err) |
| + } |
| + if code := post(u+"/pay", nil); code != http.StatusSeeOther { |
| + t.Fatalf("Pay: status %d", code) |
| + } |
| + hs, _ := s.Holdings(ctx, "ana") |
| + if len(hs) != 1 { |
| + t.Fatalf("Holdings = %+v", hs) |
| + } |
| + pu, err := s.Portal(ctx, "ana", "/account") |
| + if err != nil { |
| + t.Fatalf("Portal: %v", err) |
| + } |
| + |
| + mu.Lock() |
| + fail = true |
| + mu.Unlock() |
| + form := url.Values{"subscription": {hs[0].SubscriptionID}} |
| + if code := post(pu+"/cancel", form); code != http.StatusInternalServerError { |
| + t.Fatalf("Cancel while the app fails: status %d, want 500", code) |
| + } |
| + if hs, _ = s.Holdings(ctx, "ana"); hs[0].CancelAtPeriodEnd { |
| + t.Fatal("abono recorded the cancellation the app refused") |
| + } |
| + if code := post(pu+"/cancel", form); code != http.StatusSeeOther { |
| + t.Fatalf("Cancel again: status %d, want 303", code) |
| + } |
| + hs, _ = s.Holdings(ctx, "ana") |
| + if len(hs) != 1 || abono.Describe(hs[0], t0).Key != abono.StatusCancelling { |
| + t.Errorf("after the retry: %+v, want one subscription, cancelling", hs) |
| + } |
| +} |
diff --git a/schema_test.go b/schema_test.go| index b21c55f..d7c49cc 100644 |
| --- a/schema_test.go |
| +++ b/schema_test.go |
| @@ -140,7 +140,7 @@ func TestSchemaTimestampsDecode(t *testing.T) { |
| // migration file beside it, with a new ID and a new entry here. See |
| // rastrillo's own migrate/frozen_checksums_test.go, which this test is |
| // deliberately shaped after. |
| -const frozenAbonoChecksum = "2e452a3832ec97d35318b289716e3c39f3a114d8c127d775d73e57cbc77ab96c" |
| +const frozenAbonoChecksum = "a634c00c6cdd9170b094c55a9a77658142a48c33652821238b2aa25ee9f53185" |
| func TestSchemaFrozenChecksum(t *testing.T) { |
| for _, m := range abono.Schema.All() { |
diff --git a/stripe_double_test.go b/stripe_double_test.go| index 2cbbc7b..cb7f79d 100644 |
| --- a/stripe_double_test.go |
| +++ b/stripe_double_test.go |
| @@ -64,7 +64,7 @@ func (d *stripeDouble) service(t *testing.T, now *time.Time, cfg abono.Config) ( |
| type subSpec struct { |
| ID, Status, Price, Interval string |
| Customer, Email, Name string |
| - End time.Time |
| + Start, End time.Time |
| Cancel bool |
| Metadata map[string]string |
| } |
| @@ -78,7 +78,7 @@ func (d *stripeDouble) put(t *testing.T, sp subSpec) stripeapi.Subscription { |
| } |
| d.PutSubscription(fakestripe.Subscription{ |
| ID: sp.ID, Customer: sp.Customer, Email: sp.Email, Name: sp.Name, Status: sp.Status, |
| - Price: sp.Price, Interval: sp.Interval, PeriodEnd: sp.End, CancelAtPeriodEnd: sp.Cancel, Metadata: sp.Metadata, |
| + Price: sp.Price, Interval: sp.Interval, PeriodStart: sp.Start, PeriodEnd: sp.End, CancelAtPeriodEnd: sp.Cancel, Metadata: sp.Metadata, |
| }) |
| return d.sub(t, sp.ID) |
| } |
diff --git a/sync.go b/sync.go| index 53e1bd7..bd28a21 100644 |
| --- a/sync.go |
| +++ b/sync.go |
| @@ -30,6 +30,7 @@ type subRecord struct { |
| Plan string |
| StripePriceID string |
| Status string |
| + CurrentPeriodStart *time.Time |
| CurrentPeriodEnd *time.Time |
| CancelAtPeriodEnd bool |
| Email string |
| @@ -95,12 +96,15 @@ func (s *Service) syncTx(ctx context.Context, tx *gorm.DB, sub stripeapi.Subscri |
| CreatedAt: now, |
| UpdatedAt: now, |
| } |
| + if start := sub.PeriodStart(); !start.IsZero() { |
| + row.CurrentPeriodStart = &start |
| + } |
| if end := sub.PeriodEnd(); !end.IsZero() { |
| row.CurrentPeriodEnd = &end |
| } |
| if len(have) == 0 { |
| - ok, _ := grants(row.Status, row.CurrentPeriodEnd, row.CancelAtPeriodEnd, now) |
| + ok, _ := grants(row.Status, row.CurrentPeriodStart, row.CurrentPeriodEnd, row.CancelAtPeriodEnd, now) |
| if !ok && !adopt { |
| // An ended subscription first seen now (a reconcile reading the |
| // account's history): recording it could only take away access |
| @@ -113,10 +117,10 @@ func (s *Service) syncTx(ctx context.Context, tx *gorm.DB, sub stripeapi.Subscri |
| } |
| if err := tx.Exec(`INSERT INTO abono_subscriptions |
| (stripe_subscription_id, stripe_customer_id, subject, plan, stripe_price_id, status, |
| - current_period_end, cancel_at_period_end, email, created_at, updated_at) |
| - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, |
| + current_period_start, current_period_end, cancel_at_period_end, email, created_at, updated_at) |
| + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, |
| row.StripeSubscriptionID, row.StripeCustomerID, row.Subject, row.Plan, row.StripePriceID, row.Status, |
| - row.CurrentPeriodEnd, row.CancelAtPeriodEnd, row.Email, row.CreatedAt, row.UpdatedAt).Error; err != nil { |
| + row.CurrentPeriodStart, row.CurrentPeriodEnd, row.CancelAtPeriodEnd, row.Email, row.CreatedAt, row.UpdatedAt).Error; err != nil { |
| return "", err |
| } |
| if row.Plan != "" { |
| @@ -155,16 +159,17 @@ func (s *Service) syncTx(ctx context.Context, tx *gorm.DB, sub stripeapi.Subscri |
| } |
| moved := row.Subject != old.Subject || row.Plan != old.Plan || row.Status != old.Status || |
| - row.CancelAtPeriodEnd != old.CancelAtPeriodEnd || !sameTime(row.CurrentPeriodEnd, old.CurrentPeriodEnd) |
| + row.CancelAtPeriodEnd != old.CancelAtPeriodEnd || !sameTime(row.CurrentPeriodEnd, old.CurrentPeriodEnd) || |
| + !sameTime(row.CurrentPeriodStart, old.CurrentPeriodStart) |
| if !moved && row.StripePriceID == old.StripePriceID && row.StripeCustomerID == old.StripeCustomerID && row.Email == old.Email { |
| return row.Subject, nil |
| } |
| if err := tx.Exec(`UPDATE abono_subscriptions SET |
| stripe_customer_id = ?, subject = ?, plan = ?, stripe_price_id = ?, status = ?, |
| - current_period_end = ?, cancel_at_period_end = ?, email = ?, updated_at = ? |
| + current_period_start = ?, current_period_end = ?, cancel_at_period_end = ?, email = ?, updated_at = ? |
| WHERE stripe_subscription_id = ?`, |
| row.StripeCustomerID, row.Subject, row.Plan, row.StripePriceID, row.Status, |
| - row.CurrentPeriodEnd, row.CancelAtPeriodEnd, row.Email, now, row.StripeSubscriptionID).Error; err != nil { |
| + row.CurrentPeriodStart, row.CurrentPeriodEnd, row.CancelAtPeriodEnd, row.Email, now, row.StripeSubscriptionID).Error; err != nil { |
| return "", err |
| } |
| if !moved { |
| @@ -283,7 +288,7 @@ func (s *Service) Unrecognised(ctx context.Context) ([]Unrecognised, error) { |
| now := s.now() |
| var out []Unrecognised |
| for _, r := range rows { |
| - if ok, _ := grants(r.Status, utc(r.CurrentPeriodEnd), r.CancelAtPeriodEnd, now); !ok { |
| + if ok, _ := grants(r.Status, utc(r.CurrentPeriodStart), utc(r.CurrentPeriodEnd), r.CancelAtPeriodEnd, now); !ok { |
| continue |
| } |
| out = append(out, Unrecognised{ |
diff --git a/sync_test.go b/sync_test.go| index 77bc1f1..52abe5b 100644 |
| --- a/sync_test.go |
| +++ b/sync_test.go |
| @@ -555,3 +555,51 @@ func TestUnclaimedThenClaim(t *testing.T) { |
| t.Errorf("subject after a later sync = %q, want 5", got) |
| } |
| } |
| + |
| +// TestSyncKeepsThePeriodStartForThePastDueWindow is ruling R21: a failed |
| +// renewal has moved Stripe's period on, so the 30 days of past_due count |
| +// from the unpaid period's start, which sync records, not from its end. |
| +func TestSyncKeepsThePeriodStartForThePastDueWindow(t *testing.T) { |
| + now := t0 |
| + d := newStripeDouble(t) |
| + rec := &recorder{} |
| + s, db := d.service(t, &now, rec.config()) |
| + seedPlans(t, s) |
| + ctx := context.Background() |
| + |
| + start := t0.Add(-10 * day) // the paid year ended, the renewal failed |
| + sub := d.put(t, subSpec{ID: "sub_1", Status: "past_due", Price: "price_A", Customer: "cus_1", Email: "ana@example.com", |
| + Start: start, End: start.AddDate(1, 0, 0)}) |
| + if err := s.Sync(ctx, sub, abono.Hint{Subject: "ana"}); err != nil { |
| + t.Fatalf("Sync: %v", err) |
| + } |
| + if got := db.dump(`SELECT current_period_start IS NOT NULL FROM abono_subscriptions`); got != "1\n" { |
| + t.Fatalf("current_period_start stored = %q, want set", got) |
| + } |
| + hs, err := s.Holdings(ctx, "ana") |
| + if err != nil || len(hs) != 1 || hs[0].PeriodStart == nil || !hs[0].PeriodStart.Equal(start) { |
| + t.Fatalf("Holdings = %+v, %v; want the period start %v", hs, err, start) |
| + } |
| + if !hs[0].Grants || !hs[0].Backstop { |
| + t.Errorf("10 days into the unpaid period: Grants %v, Backstop %v; want both", hs[0].Grants, hs[0].Backstop) |
| + } |
| + if got := abono.Describe(hs[0], now).Date; got == nil || !got.Equal(start.Add(30*day)) { |
| + t.Errorf("failing date = %v, want %v", got, start.Add(30*day)) |
| + } |
| + |
| + now = start.Add(30 * day) |
| + if hasPlan(t, s, "ana", "annual") { |
| + t.Error("past_due still grants 30 days after the unpaid period started, its end months away") |
| + } |
| + |
| + // A new start (paid at last, the next period) is a change the app hears of. |
| + rec.take() |
| + sub = d.put(t, subSpec{ID: "sub_1", Status: "active", Price: "price_A", Customer: "cus_1", Email: "ana@example.com", |
| + Start: start.AddDate(1, 0, 0), End: start.AddDate(2, 0, 0)}) |
| + if err := s.Sync(ctx, sub, abono.Hint{}); err != nil { |
| + t.Fatalf("Sync: %v", err) |
| + } |
| + if _, changes := rec.take(); len(changes) != 1 { |
| + t.Errorf("OnChange after the period moved: %v, want once", changes) |
| + } |
| +} |