Write the item pass's conferrals in product id order
convergeConferrals ranged over the desired-products map, so a reconcile that confers several products wrote them in Go's random map order. The trace kit sorted conferral statements to hide that. The item pass now ranges over sortedProducts(desired), ascending product id, so two reconciles that touch the same products write them in one order, which the displacement rule's writes rely on (design, D6). The trace kit's sortConferralRuns and conferralWrites are deleted: a scan of every golden found no run of conferral statements longer than one line, so the sort changed nothing they record, and no break can show its removal. Ride-along, listed in the proposal: GetLivePoolProvisionsBySubscriptionID orders by created_at and then provision_id. Provisions conferred in one transaction share created_at, so the reconcile's end loop walked them in no stated order. No golden or test changed. reconcile_trace/three_ladders_first_sight pins the order for three ladders seen for the first time in one reconcile (A, B, C). TestSortedProducts feeds maps built forward, backward and in one fixed shuffle, fifty calls each. Breaks (scripts/mutation-check, 2 of 2 CAUGHT): - sortedProducts returns map order (TestSortedProducts; the golden sees it on about seven runs in eight, since a small map can iterate in order, so it carries no record of its own for this break) - item pass walks the ids in reverse (three_ladders_first_sight) The suite on reset databases passes, 4622 tests, 15 skipped. make lint passes; openspec validate --strict passes.
This commit is contained in:
@@ -164,7 +164,7 @@ func (q *Queries) GetLivePoolProvisionsByProductID(ctx context.Context, productI
|
||||
const getLivePoolProvisionsBySubscriptionID = `-- name: GetLivePoolProvisionsBySubscriptionID :many
|
||||
SELECT provision_id, pool_id, billing_account_id, subscription_id, purchase_id, grant_id, quantity, status, activated_at, suspended_at, ended_at, created_at, updated_at, entitlement_set_id, product_id FROM core.pool_provisions
|
||||
WHERE subscription_id = $1 AND status <> 'ended'
|
||||
ORDER BY created_at ASC
|
||||
ORDER BY created_at ASC, provision_id ASC
|
||||
`
|
||||
|
||||
// Live (non-ended) provisions of a subscription, one per product. Reconcile
|
||||
|
||||
@@ -16,7 +16,7 @@ WHERE provision_id = $1;
|
||||
-- diffs these against the subscription's desired-by-product line items.
|
||||
SELECT * FROM core.pool_provisions
|
||||
WHERE subscription_id = $1 AND status <> 'ended'
|
||||
ORDER BY created_at ASC;
|
||||
ORDER BY created_at ASC, provision_id ASC;
|
||||
|
||||
-- name: GetLivePoolProvisionsByProductID :many
|
||||
-- Live (non-ended) provisions delivering a product. Tier add/remove aligns
|
||||
|
||||
@@ -20,6 +20,8 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"maps"
|
||||
"slices"
|
||||
"time"
|
||||
|
||||
"git.coopcloud.tech/wiki-cafe/member-console/internal/billing"
|
||||
@@ -653,6 +655,14 @@ func endReplacedItems(
|
||||
return nil
|
||||
}
|
||||
|
||||
// sortedProducts returns the desired product ids in ascending order. A loop
|
||||
// that writes several rows walks a stated order (docs/go-conventions.md,
|
||||
// "Output order"), and Stripe's item order can change on an edit; the ids are
|
||||
// UUIDv7, so the order is the products' creation order.
|
||||
func sortedProducts(desired map[string]int32) []string {
|
||||
return slices.Sorted(maps.Keys(desired))
|
||||
}
|
||||
|
||||
// convergeConferrals brings the subscription's live provisions to desired:
|
||||
// a missing product is conferred, a quantity-only delta updates the bounds,
|
||||
// and a provision no longer desired is ended. It reports whether it wrote
|
||||
@@ -677,8 +687,8 @@ func convergeConferrals(
|
||||
|
||||
// Missing product -> confer; quantity-only delta -> update_conferral_bounds.
|
||||
changed := false
|
||||
for productID, qty := range desired {
|
||||
wrote, err := conferOrBound(ctx, entQ, liveByProduct, productID, qty, subUUID, poolID, actor)
|
||||
for _, productID := range sortedProducts(desired) {
|
||||
wrote, err := conferOrBound(ctx, entQ, liveByProduct, productID, desired[productID], subUUID, poolID, actor)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Commercial
|
||||
// SPDX-FileCopyrightText: 2025-2026 Christian Galo
|
||||
|
||||
package fulfillment
|
||||
|
||||
import (
|
||||
"slices"
|
||||
"testing"
|
||||
|
||||
"github.com/google/uuid"
|
||||
)
|
||||
|
||||
// TestSortedProducts pins that sortedProducts returns the desired products in
|
||||
// id order whatever order the map was built in.
|
||||
func TestSortedProducts(t *testing.T) {
|
||||
ids := make([]string, 5)
|
||||
for i := range ids {
|
||||
id, err := uuid.NewV7()
|
||||
if err != nil {
|
||||
t.Fatalf("generate id: %v", err)
|
||||
}
|
||||
ids[i] = id.String()
|
||||
}
|
||||
orders := []struct {
|
||||
name string
|
||||
order []int
|
||||
}{
|
||||
{"forward", []int{0, 1, 2, 3, 4}},
|
||||
{"backward", []int{4, 3, 2, 1, 0}},
|
||||
{"shuffled", []int{3, 0, 4, 2, 1}},
|
||||
}
|
||||
for _, o := range orders {
|
||||
name, order := o.name, o.order
|
||||
desired := make(map[string]int32, len(ids))
|
||||
for _, i := range order {
|
||||
desired[ids[i]] = int32(i + 1)
|
||||
}
|
||||
for range 50 {
|
||||
if got := sortedProducts(desired); !slices.Equal(got, ids) {
|
||||
t.Fatalf("%s: sortedProducts returned the products out of id order: %v", name, got)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -227,6 +227,23 @@ var reconcileCases = []traceCase{
|
||||
e.start()
|
||||
return e.reconcile("stripe:customer.subscription.updated:evt_3")
|
||||
}},
|
||||
// A subscription whose items Stripe lists as C, A, B first meets three
|
||||
// tiers, each alone on a ladder of its own: the item pass writes them in
|
||||
// product id order, which is the order the tiers were created in.
|
||||
{"three_ladders_first_sight", func(e *traceEnv) string {
|
||||
a, b, c := e.tier("A", 1, 1000), e.tier("B", 1, 2000), e.tier("C", 1, 3000)
|
||||
for _, t := range []struct {
|
||||
name string
|
||||
tier tier
|
||||
}{{"A", a}, {"B", b}, {"C", c}} {
|
||||
l := e.w.Ladder("Ladder "+t.name, t.tier.product)
|
||||
e.norm.Name("ladder", t.name, l.ID)
|
||||
}
|
||||
e.mock.Sub = e.sub(stripe.SubscriptionStatusActive,
|
||||
e.item("si_c", c, 1), e.item("si_a", a, 1), e.item("si_b", b, 1))
|
||||
e.start()
|
||||
return e.reconcile("stripe:customer.subscription.created:evt_1")
|
||||
}},
|
||||
// #212: a reconcile confers the subscription's tier again over the
|
||||
// operator's grant that replaced it. The fix flips this golden.
|
||||
{"grant_over_subscription", func(e *traceEnv) string {
|
||||
|
||||
+74
@@ -0,0 +1,74 @@
|
||||
== result
|
||||
nil
|
||||
== trace
|
||||
GetSubscriptionMappingByStripeID
|
||||
stripe GET /v1/subscriptions/<stripe-subscription> expand[0]=items.data.price&expand[1]=schedule
|
||||
begin
|
||||
SET LOCAL lock_timeout = '5s'
|
||||
SELECT pg_advisory_xact_lock_shared(hashtextextended($1, 0))
|
||||
SELECT pg_advisory_xact_lock(hashtext($1)::bigint)
|
||||
GetCustomerMappingByStripeCustomerID
|
||||
GetBillingAccountByID
|
||||
GetDefaultPoolByOrgID
|
||||
GetSubscriptionMappingByStripeID
|
||||
CreateSubscription
|
||||
UpsertSubscriptionMapping
|
||||
CreateSubscriptionChange
|
||||
LockResourcePool
|
||||
SELECT core.sync_source_status($1, $2, $3, $4, $5)
|
||||
GetPriceMappingByStripeID
|
||||
GetPrice
|
||||
GetProductByID
|
||||
GetSubscriptionItemMappingByStripeID
|
||||
CreateSubscriptionItem
|
||||
UpsertSubscriptionItemMapping
|
||||
GetPriceMappingByStripeID
|
||||
GetPrice
|
||||
GetProductByID
|
||||
GetSubscriptionItemMappingByStripeID
|
||||
CreateSubscriptionItem
|
||||
UpsertSubscriptionItemMapping
|
||||
GetPriceMappingByStripeID
|
||||
GetPrice
|
||||
GetProductByID
|
||||
GetSubscriptionItemMappingByStripeID
|
||||
CreateSubscriptionItem
|
||||
UpsertSubscriptionItemMapping
|
||||
GetSubscriptionItemsBySubscriptionID
|
||||
GetLivePoolProvisionsBySubscriptionID
|
||||
SELECT provision_id, outcome FROM core.confer($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11)
|
||||
SELECT provision_id, outcome FROM core.confer($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11)
|
||||
SELECT provision_id, outcome FROM core.confer($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11)
|
||||
GetLivePoolProvisionsBySubscriptionID
|
||||
<materialize>
|
||||
ListActiveScheduledChangesBySubscription
|
||||
GetPriceMappingByStripeID
|
||||
GetPriceMappingByStripeID
|
||||
GetPriceMappingByStripeID
|
||||
commit
|
||||
== logs
|
||||
== rows
|
||||
-- subscriptions
|
||||
{"status" : "active", "cancel_at_period_end" : false, "period" : "4102444800-4105123200", "canceled" : false, "ended" : false, "stripe_id" : "<stripe-subscription>", "sync_status" : "synced", "livemode" : false}
|
||||
-- subscription_changes
|
||||
{"from" : null, "to" : "active", "reason" : "stripe:customer.subscription.created:evt_1"}
|
||||
-- subscription_items
|
||||
{"stripe_id" : "si_a", "product" : "<product:A>", "price" : "<price:A>", "quantity" : 1, "ended" : false, "sync_status" : "synced", "livemode" : false}
|
||||
{"stripe_id" : "si_b", "product" : "<product:B>", "price" : "<price:B>", "quantity" : 1, "ended" : false, "sync_status" : "synced", "livemode" : false}
|
||||
{"stripe_id" : "si_c", "product" : "<product:C>", "price" : "<price:C>", "quantity" : 1, "ended" : false, "sync_status" : "synced", "livemode" : false}
|
||||
-- scheduled_changes
|
||||
-- provisions
|
||||
{"product" : "<product:A>", "set" : "<set:A>", "source" : "subscription", "quantity" : 1, "status" : "active", "ended" : false}
|
||||
{"product" : "<product:B>", "set" : "<set:B>", "source" : "subscription", "quantity" : 1, "status" : "active", "ended" : false}
|
||||
{"product" : "<product:C>", "set" : "<set:C>", "source" : "subscription", "quantity" : 1, "status" : "active", "ended" : false}
|
||||
-- ladder_positions
|
||||
{"ladder" : "<ladder:A>", "product" : "<product:A>", "status" : "active"}
|
||||
{"ladder" : "<ladder:B>", "product" : "<product:B>", "status" : "active"}
|
||||
{"ladder" : "<ladder:C>", "product" : "<product:C>", "status" : "active"}
|
||||
-- transitions
|
||||
{"type" : "initiate", "actor" : "webhook", "reason" : "stripe:customer.subscription.created:evt_1", "from_rank" : null, "to_rank" : 0, "ladder" : "<ladder:A>", "product" : "<product:A>"}
|
||||
{"type" : "initiate", "actor" : "webhook", "reason" : "stripe:customer.subscription.created:evt_1", "from_rank" : null, "to_rank" : 0, "ladder" : "<ladder:B>", "product" : "<product:B>"}
|
||||
{"type" : "initiate", "actor" : "webhook", "reason" : "stripe:customer.subscription.created:evt_1", "from_rank" : null, "to_rank" : 0, "ladder" : "<ladder:C>", "product" : "<product:C>"}
|
||||
-- numeric_entitlements
|
||||
{"key" : "fedwiki_sites", "type" : "limit", "limit" : 3}
|
||||
-- boolean_entitlements
|
||||
@@ -277,10 +277,9 @@ func result(err error) string {
|
||||
return "nil"
|
||||
}
|
||||
|
||||
// lines folds each materialization into one line and sorts each run of
|
||||
// conferral writes, whose order is Go's map order.
|
||||
// lines folds each materialization into one line.
|
||||
func (e *traceEnv) lines() []string {
|
||||
return sortConferralRuns(foldMaterialize(e.stmts.Names()))
|
||||
return foldMaterialize(e.stmts.Names())
|
||||
}
|
||||
|
||||
// foldMaterialize replaces each run of materialization statements with one
|
||||
@@ -300,35 +299,6 @@ func foldMaterialize(in []string) []string {
|
||||
return out
|
||||
}
|
||||
|
||||
// conferralWrites are the first lines of the statements the desired loop
|
||||
// issues: Queries.Confer and Queries.UpdateConferralBounds run raw SQL
|
||||
// (conferral.go), so a trace names them by their text.
|
||||
var conferralWrites = []string{
|
||||
"SELECT provision_id, outcome FROM core.confer(",
|
||||
"SELECT core.update_conferral_bounds(",
|
||||
}
|
||||
|
||||
// sortConferralRuns sorts each run of conferral writes in place.
|
||||
func sortConferralRuns(in []string) []string {
|
||||
conferral := func(s string) bool {
|
||||
return strings.HasPrefix(s, conferralWrites[0]) || strings.HasPrefix(s, conferralWrites[1])
|
||||
}
|
||||
out := slices.Clone(in)
|
||||
for i := 0; i < len(out); {
|
||||
j := i
|
||||
for j < len(out) && conferral(out[j]) {
|
||||
j++
|
||||
}
|
||||
if j > i {
|
||||
slices.Sort(out[i:j])
|
||||
i = j
|
||||
continue
|
||||
}
|
||||
i++
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// nameWritten names the rows the act wrote whose ids a log line or an error
|
||||
// prints: the core subscription, its items and its scheduled changes.
|
||||
func (e *traceEnv) nameWritten() {
|
||||
|
||||
@@ -15,6 +15,7 @@ The same root cause has two more forms. A subscription that carries two items fo
|
||||
- **Recovery from past due goes through the same rule.** No branch of its own: the resume finds nothing and the item pass leaves the ladder alone.
|
||||
- **A subscription that pays for two tiers of one ladder confers the higher.** The rule is one Go function that reads no row: conflicting products are walked by their highest rank, then product id, and a product is kept unless a kept one holds one of its ladders. The product left out is logged as a warning, since the member pays for it.
|
||||
- **The item pass writes in ascending product id order**, never in Stripe's item order or Go's map order.
|
||||
- **Ride-along: the live-provisions query orders by `provision_id` after `created_at`.** `GetLivePoolProvisionsBySubscriptionID` ordered by `created_at` alone; provisions conferred in one transaction share it, so `endSurplusConferrals` ended surplus provisions in no stated order (`docs/go-conventions.md`, "Output order"). No behaviour a caller can name changes.
|
||||
- **One statement of a supersession pair.** A view, `core.supersession_pairs`, pairs an `end` row with the typed row of the conferral that caused it, which `core.grant_replacements` already does for grants alone. `core.grant_replacements` is re-expressed over it with its rows unchanged, and the displacement function reads it.
|
||||
- **The organization page says so.** The Billing card's Subscriptions row carries one muted line when a subscription of the account delivers nothing while another plan holds its ladder, read through the reconcile's own choice over the subscription's current items, for example "Delivers nothing on Hosting, which a grant holds" (design, Copy table). A failed read shows the section's error in the line's place, never an absent line.
|
||||
|
||||
|
||||
@@ -19,13 +19,15 @@ This change starts from the tree that `reconcile-takes-pool-lock` (#193) leaves,
|
||||
|
||||
## 2. The item pass writes in product id order
|
||||
|
||||
- [ ] 2.1 `internal/fulfillment/reconcile.go` (design D6): `sortedProducts(desired map[string]int32) []string` returns `slices.Sorted(maps.Keys(desired))`; `convergeConferrals` ranges over it and reads each quantity from `desired`. The doc comment says why: a loop that writes several rows walks a stated order (`docs/go-conventions.md`, "Output order"), and Stripe's item order can change on an edit; the ids are UUIDv7, so the order is the products' creation order.
|
||||
- [ ] 2.2 `internal/fulfillment/trace_db_test.go`: `sortConferralRuns` and `conferralWrites` are deleted, since the order they hid is now fixed; `lines()` returns `foldMaterialize(e.stmts.Names())` and its comment says it folds each materialization into one line. No golden changes: no committed golden holds a run of two conferral statements.
|
||||
- [ ] 2.3 The trace case `three_ladders_first_sight` (`reconcile_trace_db_test.go`): tiers A, B and C created in that order, so their product ids ascend, each the one tier of a ladder of its own (normalized `<ladder:A>`, `<ladder:B>`, `<ladder:C>`), and a first reconcile of a subscription whose items Stripe lists as C, A, B. Its golden, written with `-update`, lists the three `initiate` rows in the order A, B, C.
|
||||
- [ ] 2.4 Test. `TestSortedProducts` (a new `internal/fulfillment/reconcile_order_test.go`, package `fulfillment`, no database): five UUIDv7 ids generated in order, maps built by inserting them forward, last to first, and in one fixed shuffle, each called fifty times and each result compared with the ids in generated order; a failure reads `sortedProducts returned the products out of id order: %v`.
|
||||
- [ ] 2.5 Breaks:
|
||||
- the map's order (`slices.Sorted(maps.Keys(desired))` replaced by `slices.Collect(maps.Keys(desired))`): caught by `TestSortedProducts`, `want_new` `sortedProducts returned the products out of id order`. `three_ladders_first_sight` sees the same break on most runs but not on every run, because one of the map's orders is the sorted one, so it carries no record; the commit message says so;
|
||||
- [x] 2.1 `internal/fulfillment/reconcile.go` (design D6): `sortedProducts(desired map[string]int32) []string` returns `slices.Sorted(maps.Keys(desired))`; `convergeConferrals` ranges over it and reads each quantity from `desired`. The doc comment says why: a loop that writes several rows walks a stated order (`docs/go-conventions.md`, "Output order"), and Stripe's item order can change on an edit; the ids are UUIDv7, so the order is the products' creation order.
|
||||
- [x] 2.2 `internal/fulfillment/trace_db_test.go`: `sortConferralRuns` and `conferralWrites` are deleted, since the order they hid is now fixed; `lines()` returns `foldMaterialize(e.stmts.Names())` and its comment says it folds each materialization into one line. No golden changes: no committed golden holds a run of two conferral statements.
|
||||
- [x] 2.3 The trace case `three_ladders_first_sight` (`reconcile_trace_db_test.go`): tiers A, B and C created in that order, so their product ids ascend, each the one tier of a ladder of its own (normalized `<ladder:A>`, `<ladder:B>`, `<ladder:C>`), and a first reconcile of a subscription whose items Stripe lists as C, A, B. Its golden, written with `-update`, lists the three `initiate` rows in the order A, B, C.
|
||||
- [x] 2.4 Test. `TestSortedProducts` (a new `internal/fulfillment/reconcile_order_test.go`, package `fulfillment`, no database): five UUIDv7 ids generated in order, maps built by inserting them forward, last to first, and in one fixed shuffle, each called fifty times and each result compared with the ids in generated order; a failure reads `sortedProducts returned the products out of id order: %v`.
|
||||
- [x] 2.5 Breaks:
|
||||
- the map's order (`slices.Sorted(maps.Keys(desired))` replaced by `slices.Collect(maps.Keys(desired))`): caught by `TestSortedProducts`, `want_new` `sortedProducts returned the products out of id order`. `three_ladders_first_sight` sees the same break on most runs but not on every run, because one of the map's orders is the sorted one, so it carries no record of that break; the commit message says so;
|
||||
- the item pass walks the ids in reverse (`range sortedProducts(desired)` replaced by `range slices.Backward(sortedProducts(desired))`): caught by `three_ladders_first_sight` on every run, `run_new` `^TestReconcileTrace$/^three_ladders_first_sight$`, `want_new` `testdata/reconcile_trace/three_ladders_first_sight.golden differs from what the code produced`; it is the break for the call site, which `TestSortedProducts` cannot see;
|
||||
- the deletion of `sortConferralRuns` has no break: it removes test code whose effect no golden held, which the unchanged goldens show.
|
||||
- [x] 2.6 Ride-along (listed in the proposal): `GetLivePoolProvisionsBySubscriptionID` (`internal/entitlements/queries/pool_provisions.sql`) orders by `created_at ASC, provision_id ASC`, and `make sqlc-generate` regenerates its output. Provisions conferred in one transaction share `created_at`, so `endSurplusConferrals` ended surplus provisions in no stated order.
|
||||
|
||||
## 3. One statement of a supersession pair
|
||||
|
||||
|
||||
Reference in New Issue
Block a user