diff --git a/docs/database-locks.md b/docs/database-locks.md index 78801db6..fd4a9660 100644 --- a/docs/database-locks.md +++ b/docs/database-locks.md @@ -39,7 +39,7 @@ advisory lock, `LOCK TABLE`, or an explicit `FOR SHARE`. | Lock | Row | Mode | Taken by | Serializes | |---|---|---|---|---| -| Pool row | `core.resource_pools` row of one pool | `FOR UPDATE` | the functions `core.confer`, `core.end_conferral`, `core.align_conferral_shape`, `core.update_conferral_bounds` and `core.settle_obligation`; `LockPools` (through the `LockResourcePool` query), called by `RevokeGrantTx`, `ManageGrantTx`, `ResumeReplacedGrantsTx`, `settlePool`, `EnactDefault` and `RemoveTier`; and `LockPoolOfGrant` (through the `LockGrantPool` query), called by `ExpireGrantTx` and `ExtendWaitingGrantTx` | Everything that changes a pool's positions or entitlements: conferral, endings, grant acts, tier removal, a default change, rule-change settlement and grant resumption. | +| Pool row | `core.resource_pools` row of one pool | `FOR UPDATE` | the functions `core.confer`, `core.end_conferral`, `core.align_conferral_shape`, `core.update_conferral_bounds` and `core.settle_obligation`; `LockPools` (through the `LockResourcePool` query), called by `RevokeGrantTx`, `ManageGrantTx`, `ResumeReplacedGrantsTx`, `settlePool`, `EnactDefault`, `runPlanAct` (`AddTier`) and `RemoveTier`; and `LockPoolOfGrant` (through the `LockGrantPool` query), called by `ExpireGrantTx` and `ExtendWaitingGrantTx` | Everything that changes a pool's positions or entitlements: conferral, endings, grant acts, tier removal, a default change, rule-change settlement and grant resumption. | | Entitlement set row and rule row | `core.entitlement_sets` row and `core.entitlement_set_rules` row | `FOR UPDATE` | `core.commit_rule_change` (the set row also by the `LockEntitlementSet` query, once per batch) | A rule change against a writer that changes the set or the rule without holding the rendezvous. | | Change obligation row | `core.entitlement_set_change_obligations` row of one (change, pool) | `FOR UPDATE` | `core.settle_obligation` | Two passes settling one owed pool, so each effect is written once. | | Grant lineage head | the `core.grants` row no other grant extends | `FOR UPDATE OF g` | the `GetGrantLineageHeadForUpdate` query, run by `RevokeGrantTx`, `ExtendWaitingGrantTx` and `ManageGrantTx` | Acts on one lineage, so an extension that committed while another act waited is visible to it. | @@ -166,11 +166,20 @@ ladder's ranks and a product's shape in separate statements, so no conferral may run while one of them commits (#213). The default change and each organization's enactment run in transactions from `BeginMaterializing`. -- **Tier add** (`entitlements.AddTier`): the exclusive rendezvous, the tier - insert, then for each live provision of the product in creation order - `core.align_conferral_shape` (the pool row of that provision) and - materialization. The pools are locked in provision order, not in `pool_id` - order. +- **Tier add** (`entitlements.AddTier`): the exclusive rendezvous, then the + reads that decide the add, inside the transaction: the product (a product no + row names, or one not published, refuses the add) and the count of the + organizations holding a live provision of it (above `planActHolderCap` the + add is refused), then the pool row of every pool holding a live provision + of the product through `LockPools`, which sorts and deduplicates them, so + they lock in ascending `pool_id` order. The provisions are read again under + the locks, then the ladder's name, the act row and its detail row + (`core.record_plan_act`), the tier insert, and the collision read + (`ListTierAddCollisions`), which refuses a pool that already holds a live + position on the ladder or holds two live provisions of the product before + any alignment. Then, for each pool in the same order, + `core.align_conferral_shape` for each of its provisions in creation order and + one materialization. It runs through `runPlanAct`. - **Tier removal with holders** (`entitlements.RemoveTier`): the exclusive rendezvous, then the pool row of every holder through `LockPools`, which sorts and deduplicates them, so they lock in ascending @@ -320,8 +329,8 @@ that allocates or releases the name. differ"). The five conferral and settlement functions lock the row themselves, so a caller that goes straight to one of them still holds it. 3. A transaction that locks several pools takes them in ascending `pool_id` - order, through `LockPools`. The rule change and the tier removal do; tier add does not (see "Where - the paths differ"). The deferred drain locks one pool per transaction. + order, through `LockPools`. The rule change, the tier add and the tier + removal do. The deferred drain locks one pool per transaction. 4. The grant lineage head row is locked after the pool row, never before. 5. A transaction that holds a site row and the usage row takes the site row first. @@ -344,8 +353,6 @@ that allocates or releases the name. conventions above. - **The suspended branch of the reconcile materializes without a pool row lock** (#193). It holds the shared rendezvous and the subscription lock only. -- **Tier add locks pools in provision order**, where tier removal sorts them - (#194). - **Recording a failed drain locks the obligation row before the pool row** (#196), where the drain and the commit take the pool first. - **The FedWiki usage counter has no explicit lock in the request paths.** The diff --git a/docs/plan-management.md b/docs/plan-management.md index a1bee081..b4bfe02f 100644 --- a/docs/plan-management.md +++ b/docs/plan-management.md @@ -68,6 +68,13 @@ The panel also flags the **invisible-limbo** case: a published product that is n 2. Select any **published product** (Doc 41: display category never gates tier membership). 3. Click **Add Tier** — the tier appends at the bottom (rank self-assigns) and you drag it into position. If the product already carries live provisions, the induction backfill aligns them to the grown shape in the same transaction. +The add decides inside the one transaction that inserts the tier: + +- **The product.** It reads the product there and refuses one that is not published; a product no row names answers "Product not found.". +- **The holder limit.** It counts the organizations holding a live provision of the product and refuses above 200, the limit set from the timed cost of reconciling one holder, so a tier change's exclusive hold on the materialization lock stays near half a second. The add reconciles a holder faster than the removal, so at the limit it holds about 0.15 s; the removal sets the shared limit. The refusal names the count and the limit. When more than half of those organizations hold the product through their org type's default on a ladder, it adds that changing that default with Migrate moves them in the background. A default-sourced provision that a Keep removal left off every ladder counts toward the limit and not toward that sentence, because changing the default would not move it. +- **A held rung.** After the tier insert and before any alignment, it reads the holders' pools it has locked. A pool that already holds a live position on the ladder refuses the add, naming the organization and the product it holds; so does a pool that holds the product in more than one live provision, such as an operator grant and a subscription, because an organization holds one position per ladder. The first such pool in ascending pool order is the one named. +- **The record.** It writes one act (cause `tier_added`) naming the ladder and the product, and the alignments record the ladder's name in their reason. A lock wait or a deadlock at any step answers "Another change collided with this one; try again."; every other failure answers "Failed to add tier. Reference: {request id}.", and the log line carries the same request id and the step that failed. + Tiers within a ladder are ordered by rank. Lower ranks are "lower" tiers; higher ranks are "higher" tiers. The typical pattern is: - Rank 0 — Public / Free tier diff --git a/internal/entitlements/cap_timing_test.go b/internal/entitlements/cap_timing_test.go index 74cc2b9f..9e9f264b 100644 --- a/internal/entitlements/cap_timing_test.go +++ b/internal/entitlements/cap_timing_test.go @@ -7,6 +7,7 @@ import ( "context" "database/sql" "fmt" + "math" "os" "slices" "strings" @@ -1096,6 +1097,7 @@ func timeAddTier(t *testing.T, w *world.World, owner world.Person, holders int) start := time.Now() err := entitlements.AddTier(w.Ctx, w.DB, entitlements.AddTierInput{ LadderID: target.ID, ProductID: held.ID, OperatorPersonID: owner.ID, + Acts: entitlements.PlanActConfig{HolderCap: math.MaxInt64}, }) took := time.Since(start) if err != nil { diff --git a/internal/entitlements/constants.go b/internal/entitlements/constants.go index 9e9606cf..b21db046 100644 --- a/internal/entitlements/constants.go +++ b/internal/entitlements/constants.go @@ -16,6 +16,18 @@ const ( // lock_timeout a waiting materializer tolerates on a slower host. rematerializeSyncCap = 250 + // planActHolderCap is how many organizations may hold a tier being + // removed, or a product being added as a tier, before the act is refused. + // A tier change reconciles every holder inside its own transaction, which + // holds the exclusive rendezvous. Set from the timed run of + // cap_timing_test.go (TestTimedPlanActCaps, PostgreSQL 18.1, one run per + // holder count from 50 to 3,200, two runs, 2026-10-19): a removal with + // migrate costs 2.2 to 2.6 ms per holder and an add 0.7 to 1.0 ms. One + // cap serves both, so the slower binds: 200 holders at 2.6 ms hold the + // rendezvous for about half a second here, well inside the 5 s + // lock_timeout a waiting materializer tolerates on a slower host. + planActHolderCap = 200 + // recomputeDrainPollInterval is the poller's cadence. The commit starts // its own drain workflow, so the poller only catches a failed start or a // dead process; five seconds bounds that gap without the Temporal churn diff --git a/internal/entitlements/ladder_acts.go b/internal/entitlements/ladder_acts.go index 6e32cc2c..8d54d0ed 100644 --- a/internal/entitlements/ladder_acts.go +++ b/internal/entitlements/ladder_acts.go @@ -6,6 +6,7 @@ package entitlements import ( "context" "database/sql" + "errors" "fmt" "slices" "time" @@ -47,24 +48,28 @@ type PlanActStep string // The steps of the plan-wide acts, in the order the acts run them. const ( - StepBegin PlanActStep = "begin" - StepCreateTier PlanActStep = "create the tier" - StepReadProvisions PlanActStep = "read the product's live provisions" - StepReadHolders PlanActStep = "read the tier's holders" - StepReadTiers PlanActStep = "read the ladder's tiers" - StepReadOrgTypes PlanActStep = "read the organization types" - StepReadNames PlanActStep = "read the ladder's and the product's names" - StepRecordAct PlanActStep = "record the act" - StepLockPools PlanActStep = "lock the holders' pools" - StepRereadHolders PlanActStep = "read the tier's holders under the pool locks" - StepDeleteTier PlanActStep = "delete the tier" - StepListTiers PlanActStep = "list the remaining tiers" - StepRenumber PlanActStep = "renumber the tiers" - StepEndIncumbents PlanActStep = "end the default-sourced positions" - StepAlign PlanActStep = "align a position to its product's shape" - StepRestore PlanActStep = "restore the organization type's default" - StepMaterialize PlanActStep = "materialize the pool" - StepCommit PlanActStep = "commit" + StepBegin PlanActStep = "begin" + StepReadProduct PlanActStep = "read the product" + StepCountHolders PlanActStep = "count the product's holders" + StepCreateTier PlanActStep = "create the tier" + StepReadProvisions PlanActStep = "read the product's live provisions" + StepRereadProvisions PlanActStep = "read the product's live provisions under the pool locks" + StepReadCollisions PlanActStep = "read the pools that cannot take the tier" + StepReadHolders PlanActStep = "read the tier's holders" + StepReadTiers PlanActStep = "read the ladder's tiers" + StepReadOrgTypes PlanActStep = "read the organization types" + StepReadNames PlanActStep = "read the ladder's and the product's names" + StepRecordAct PlanActStep = "record the act" + StepLockPools PlanActStep = "lock the holders' pools" + StepRereadHolders PlanActStep = "read the tier's holders under the pool locks" + StepDeleteTier PlanActStep = "delete the tier" + StepListTiers PlanActStep = "list the remaining tiers" + StepRenumber PlanActStep = "renumber the tiers" + StepEndIncumbents PlanActStep = "end the default-sourced positions" + StepAlign PlanActStep = "align a position to its product's shape" + StepRestore PlanActStep = "restore the organization type's default" + StepMaterialize PlanActStep = "materialize the pool" + StepCommit PlanActStep = "commit" ) // PlanActError is a plan-wide act's failure: the step that failed, the pool, @@ -112,6 +117,47 @@ type LastTierError struct { func (e *LastTierError) Error() string { return e.Message } +// HolderLimitError is a tier add refused because more organizations hold the +// product than planActHolderCap. Message is HolderLimitRefusal's sentences. +// The act wrote nothing. +type HolderLimitError struct { + Message string +} + +func (e *HolderLimitError) Error() string { return e.Message } + +// ErrTierProductNotFound is a tier add of a product no row names. +var ErrTierProductNotFound = errors.New("product not found") + +// ProductNotPublishedError is a tier add of a product whose lifecycle status +// is Status, not published. The add wrote nothing. +type ProductNotPublishedError struct { + Status string +} + +func (e *ProductNotPublishedError) Error() string { + return "product is " + e.Status + ", not published" +} + +// TierAddCollisionError is a tier add refused because the pool of +// Organization cannot take the product onto the ladder: it already holds a +// live position on the ladder, which delivers Product, or, when MoreThanOnce, +// it holds Product, the product being added, in more than one live +// provision, so the add would place both. Both names were read inside the +// add's transaction. The add wrote nothing. +type TierAddCollisionError struct { + Organization string + Product string + MoreThanOnce bool +} + +func (e *TierAddCollisionError) Error() string { + if e.MoreThanOnce { + return e.Organization + " holds " + e.Product + " more than once" + } + return e.Organization + " already holds " + e.Product + " on the ladder" +} + // operatorActor is the operator acting with the given reason, as every // transition and conferral of a plan-wide act records it. func operatorActor(operatorPersonID, reason string) Actor { @@ -315,61 +361,175 @@ func removeFromPool(ctx context.Context, tx *sql.Tx, qtx *Queries, poolID string } // AddTierInput is ProductID added as the last tier of LadderID by the -// operator. +// operator. RequestID is the request's identifier, which the act stores for +// correlating it with log lines; Acts says where the act builds its queriers. type AddTierInput struct { LadderID string ProductID string OperatorPersonID string + RequestID string + Acts PlanActConfig } // AddTier appends the product to the ladder (its rank is MAX+1, assigned // inside the insert) and aligns every live provision of the product to the // grown shape, in one transaction: adding the tier grew the product's -// conferral shape, so existing deliveries gain the new rung atomically. Each -// provision is aligned and its pool materialized in the provisions' creation -// order; the alignment locks each provision's pool. A held rung refuses the -// add with ErrConferralShapeCollision under StepAlign. +// conferral shape, so existing deliveries gain the new rung atomically. It +// reads the product inside and refuses one that is not published, and counts +// the organizations holding a live provision of it and refuses above +// planActHolderCap (HolderLimitRefusal). It locks the provisions' pools in +// ascending order before any alignment, reads the provisions again under the +// locks, records the act, inserts the tier, and reads the pools that cannot +// take the tier: a pool that already holds a live position on the ladder, or +// holds two live provisions of the product, refuses the add with a +// *TierAddCollisionError before any alignment, because the second provision's +// alignment would meet the first's rung and the rollback would leave no +// position to name. Then each pool, in the same order, aligns its provisions +// and materializes once. func AddTier(ctx context.Context, b db.Beginner, in AddTierInput) error { - tx, err := BeginRuleChange(ctx, b) - if err != nil { - return &PlanActError{Step: StepBegin, Err: err} - } - defer tx.Rollback() - entQ := New(tx) + t := &tierAdd{in: in} + return runPlanAct(ctx, b, planAct{ + Kind: RuleChange, + Queriers: in.Acts, + Find: t.find, + Reread: t.reread, + Act: t.act, + }) +} - if _, err := billing.New(tx).CreatePlanLadderTier(ctx, billing.CreatePlanLadderTierParams{ - PlanLadderID: in.LadderID, - ProductID: in.ProductID, +// tierAdd is one tier add's state across the runner's steps: the product it +// read, the pools it locked and the provisions of each, in creation order. +type tierAdd struct { + in AddTierInput + product billing.Product + locked []string + byPool map[string][]PoolProvision +} + +// find reads the product, refuses it when it is not published or too many +// organizations hold it, and names the pools of its live provisions. +func (t *tierAdd) find(ctx context.Context, q planActQueriers) ([]string, error) { + product, err := q.Billing.GetProductByID(ctx, t.in.ProductID) + if errors.Is(err, sql.ErrNoRows) { + return nil, ErrTierProductNotFound + } + if err != nil { + return nil, &PlanActError{Step: StepReadProduct, Err: err} + } + if product.LifecycleStatus != "published" { + return nil, &ProductNotPublishedError{Status: product.LifecycleStatus} + } + t.product = product + counts, err := q.Ent.CountLiveProductHolders(ctx, t.in.ProductID) + if err != nil { + return nil, &PlanActError{Step: StepCountHolders, Err: err} + } + if refusal := HolderLimitRefusal(HolderLimitAdd, counts.Holders, counts.DefaultSourced, t.in.Acts.holderCap(), false); refusal != "" { + return nil, &HolderLimitError{Message: refusal} + } + provisions, err := q.Ent.GetLivePoolProvisionsByProductID(ctx, t.in.ProductID) + if err != nil { + return nil, &PlanActError{Step: StepReadProvisions, Err: err} + } + for _, p := range provisions { + t.locked = append(t.locked, p.PoolID) + } + slices.Sort(t.locked) + t.locked = slices.Compact(t.locked) + return t.locked, nil +} + +// reread reads the provisions again under the pool locks and groups them by +// pool. Under the exclusive rendezvous no conferral adds a provision between +// the reads, so a pool the first read did not name is a defect. +func (t *tierAdd) reread(ctx context.Context, q planActQueriers) error { + provisions, err := q.Ent.GetLivePoolProvisionsByProductID(ctx, t.in.ProductID) + if err != nil { + return &PlanActError{Step: StepRereadProvisions, Err: err} + } + t.byPool = make(map[string][]PoolProvision) + for _, p := range provisions { + if !slices.Contains(t.locked, p.PoolID) { + return &PlanActError{Step: StepRereadProvisions, PoolID: p.PoolID, + Err: errors.New("a provision of a pool the first read did not name")} + } + t.byPool[p.PoolID] = append(t.byPool[p.PoolID], p) + } + return nil +} + +// act records the act, inserts the tier, refuses a collision and aligns each +// pool. +func (t *tierAdd) act(ctx context.Context, q planActQueriers) error { + ladder, err := q.Billing.GetPlanLadderByID(ctx, t.in.LadderID) + if err != nil { + return &PlanActError{Step: StepReadNames, Err: err} + } + record := planActRecord{ + Cause: ActTierAdded, + ActorPersonID: t.in.OperatorPersonID, + RequestID: t.in.RequestID, + EffectiveAt: time.Now(), + PlanLadderID: t.in.LadderID, + PlanLadderName: ladder.Name, + ProductID: t.in.ProductID, + ProductName: t.product.Name, + } + if err := writePlanAct(ctx, q.Ent, record); err != nil { + return &PlanActError{Step: StepRecordAct, Err: err} + } + if _, err := q.Billing.CreatePlanLadderTier(ctx, billing.CreatePlanLadderTierParams{ + PlanLadderID: t.in.LadderID, + ProductID: t.in.ProductID, }); err != nil { return &PlanActError{Step: StepCreateTier, Err: err} } - provisions, err := entQ.GetLivePoolProvisionsByProductID(ctx, in.ProductID) - if err != nil { - return &PlanActError{Step: StepReadProvisions, Err: err} + if err := t.refuseCollision(ctx, q); err != nil { + return err } - actorID := uuid.NullUUID{UUID: uuid.MustParse(in.OperatorPersonID), Valid: true} - for _, p := range provisions { - // The operator-actor transition CHECK requires actor_id; omitting it - // made every provision-bearing induction roll back (found 2026-07-12 - // by the tier-reorder walkthrough). - if _, err := entQ.AlignConferralShape(ctx, AlignConferralShapeParams{ - ProvisionID: p.ProvisionID, - ActorType: "operator", - ActorID: actorID, - Reason: sql.NullString{String: "tier added to ladder " + in.LadderID, Valid: true}, - }); err != nil { - return &PlanActError{Step: StepAlign, ProvisionID: p.ProvisionID, Err: err} + actor := operatorActor(t.in.OperatorPersonID, PlanActReason(record)) + for _, poolID := range t.locked { + for _, p := range t.byPool[poolID] { + // The operator-actor transition CHECK requires actor_id; omitting + // it made every provision-bearing induction roll back (found + // 2026-07-12 by the tier-reorder walkthrough). + if _, err := q.Conferral.AlignConferralShape(ctx, AlignConferralShapeParams{ + ProvisionID: p.ProvisionID, + ActorType: actor.ActorType, + ActorID: actor.ActorID, + Reason: sql.NullString{String: actor.Reason, Valid: true}, + }); err != nil { + return &PlanActError{Step: StepAlign, ProvisionID: p.ProvisionID, Err: err} + } } - if err := MaterializePoolEntitlements(ctx, entQ, p.PoolID); err != nil { - return &PlanActError{Step: StepMaterialize, PoolID: p.PoolID, Err: err} + if err := MaterializePoolEntitlements(ctx, q.Conferral, poolID); err != nil { + return &PlanActError{Step: StepMaterialize, PoolID: poolID, Err: err} } } - if err := tx.Commit(); err != nil { - return &PlanActError{Step: StepCommit, Err: err} - } return nil } +// refuseCollision reads the locked pools that cannot take the tier and +// refuses with the first in ascending pool order. +func (t *tierAdd) refuseCollision(ctx context.Context, q planActQueriers) error { + collisions, err := q.Ent.ListTierAddCollisions(ctx, ListTierAddCollisionsParams{ + PlanLadderID: t.in.LadderID, + ProductID: t.in.ProductID, + PoolIds: t.locked, + }) + if err != nil { + return &PlanActError{Step: StepReadCollisions, Err: err} + } + if len(collisions) == 0 { + return nil + } + c := collisions[0] + if c.HeldTwice { + return &TierAddCollisionError{Organization: c.OrgName, Product: t.product.Name, MoreThanOnce: true} + } + return &TierAddCollisionError{Organization: c.OrgName, Product: c.HeldProductName} +} + // DeleteTierInput is the delete of ProductID's tier from LadderID by the // operator. RequestID is the request's identifier, which the act stores for // correlating it with log lines; Acts says where the act builds its queriers. @@ -441,7 +601,7 @@ func deleteTierAct(ctx context.Context, q planActQueriers, in DeleteTierInput) e if err != nil { return &PlanActError{Step: StepReadNames, Err: err} } - if _, err := writePlanAct(ctx, q.Ent, planActRecord{ + if err := writePlanAct(ctx, q.Ent, planActRecord{ Cause: ActTierRemoved, ActorPersonID: in.OperatorPersonID, RequestID: in.RequestID, diff --git a/internal/entitlements/plan_act_runner.go b/internal/entitlements/plan_act_runner.go index 18a1af0f..444d33f7 100644 --- a/internal/entitlements/plan_act_runner.go +++ b/internal/entitlements/plan_act_runner.go @@ -27,6 +27,10 @@ type PlanActConfig struct { Entitlements func(DBTX) Querier Billing func(billing.DBTX) billing.Querier Organization func(organization.DBTX) organization.Querier + // HolderCap replaces planActHolderCap, the holder limit of a tier add; + // zero means the constant. A test sets it so a scenario can cross the + // limit with a few organizations. + HolderCap int64 } // planActQueriers are the queriers of one plan act, all on its transaction. @@ -34,11 +38,14 @@ type planActQueriers struct { Ent Querier Billing billing.Querier Org organization.Querier + // Conferral is the generated querier on the act's transaction, for the + // conferral and materialization functions, which take the generated type. + Conferral *Queries } // bind builds the act's queriers on tx. func (c PlanActConfig) bind(tx *sql.Tx) planActQueriers { - q := planActQueriers{Ent: New(tx), Billing: billing.New(tx), Org: organization.New(tx)} + q := planActQueriers{Ent: New(tx), Billing: billing.New(tx), Org: organization.New(tx), Conferral: New(tx)} if c.Entitlements != nil { q.Ent = c.Entitlements(tx) } @@ -129,11 +136,11 @@ type planActRecord struct { RetriedActID string } -// writePlanAct writes the act through core.record_plan_act and returns its -// id. The caller's transaction must hold the exclusive rendezvous (the -// shared one for a Retry), which the function asserts. The actor is a person: -// every plan act today is an operator's. -func writePlanAct(ctx context.Context, q Querier, r planActRecord) (string, error) { +// writePlanAct writes the act through core.record_plan_act. The caller's +// transaction must hold the exclusive rendezvous (the shared one for a Retry), +// which the function asserts. The actor is a person: every plan act today is an +// operator's. +func writePlanAct(ctx context.Context, q Querier, r planActRecord) error { var bad error id := func(what, s string) uuid.NullUUID { parsed, err := nullUUID(s) @@ -163,9 +170,10 @@ func writePlanAct(ctx context.Context, q Querier, r planActRecord) (string, erro RetriedActID: id("retried act", r.RetriedActID), } if bad != nil { - return "", bad + return bad } - return q.RecordPlanAct(ctx, params) + _, err := q.RecordPlanAct(ctx, params) + return err } func nullString(s string) sql.NullString { @@ -203,3 +211,68 @@ func orgTypesDefaultingTo(types []organization.OrgType, ladderID string) []strin } return names } + +// holderCap is the holder limit a tier add enforces. +func (c PlanActConfig) holderCap() int64 { + if c.HolderCap > 0 { + return c.HolderCap + } + return planActHolderCap +} + +// HolderLimitAct says whose limit sentences HolderLimitRefusal words: a tier +// removal's or a tier add's. +type HolderLimitAct int + +// The acts that count their holders against planActHolderCap. +const ( + HolderLimitRemoval HolderLimitAct = iota + HolderLimitAdd +) + +// HolderLimitRefusal is the guard that bounds a tier change's exclusive hold. +// It returns the sentences that refuse the act when holders, the organizations +// holding the tier (a removal) or a live provision of the product (an add), +// exceed limit; otherwise it returns "". The first sentence names the count +// and the limit. When more than half of the holders hold it through a +// default-sourced position on a ladder (defaultSourced of them), a second +// sentence names the way through: changing the default with Migrate, and, for +// a removal of the rank-0 tier of a ladder their type defaults to +// (rankZeroOfTheirDefault), reordering the ladder with Migrate as well. It +// reads no row; the tier add applies it, and the removal's wording is here +// for the act that will apply the same rule. +func HolderLimitRefusal(act HolderLimitAct, holders, defaultSourced, limit int64, rankZeroOfTheirDefault bool) string { + if holders <= limit { + return "" + } + first := formatCount(holders) + " organizations hold this tier; a removal is refused above " + formatCount(limit) + "." + if act == HolderLimitAdd { + first = formatCount(holders) + " organizations hold this product; adding it as a tier is refused above " + formatCount(limit) + "." + } + if defaultSourced*2 <= holders { + return first + } + way := "changing that default with Migrate moves them in the background." + if act == HolderLimitRemoval && rankZeroOfTheirDefault { + way = "changing that default, or reordering this ladder, with Migrate moves them in the background." + } + return first + " " + formatCount(defaultSourced) + " of them hold it through their org type's default; " + way +} + +// PlanActReason is the reason every transition of a plan act records, built +// from the act's detail row so the wording lives in one place. The prefixes +// are the ones the Tier changes list recognizes. A cause that moves no +// position (a Retry) has none. +func PlanActReason(detail planActRecord) string { + switch detail.Cause { + case ActTierAdded: + return "tier added to ladder " + detail.PlanLadderName + case ActTierRemoved: + return fmt.Sprintf("tier removal (%s from %s)", detail.ProductName, detail.PlanLadderName) + case ActTiersReordered: + return fmt.Sprintf("plan-ladder tier reorder (%s)", detail.PlanLadderName) + case ActOrgTypeDefaultChanged: + return fmt.Sprintf("org-type default change (%s)", detail.OrgType) + } + return "" +} diff --git a/internal/entitlements/plan_act_runner_test.go b/internal/entitlements/plan_act_runner_test.go index b41a4954..df97763d 100644 --- a/internal/entitlements/plan_act_runner_test.go +++ b/internal/entitlements/plan_act_runner_test.go @@ -176,3 +176,63 @@ func TestRunPlanActNamesAFailedBegin(t *testing.T) { t.Errorf("steps ran after a failed begin: %v", events) } } + +// HolderLimitRefusal refuses only above the limit, names the way through only +// when more than half of the holders hold the tier or product through their +// type's default, and names the reorder only for a removal of a rank-0 tier. +func TestHolderLimitRefusal(t *testing.T) { + const ( + removal = "1,001 organizations hold this tier; a removal is refused above 1,000." + add = "1,001 organizations hold this product; adding it as a tier is refused above 1,000." + viaType = " 501 of them hold it through their org type's default; " + change = "changing that default with Migrate moves them in the background." + reorder = "changing that default, or reordering this ladder, with Migrate moves them in the background." + ) + for _, tc := range []struct { + name string + act HolderLimitAct + holders int64 + defaultSourced int64 + rankZero bool + want string + }{ + {"a removal below the limit", HolderLimitRemoval, 999, 999, true, ""}, + {"a removal at the limit", HolderLimitRemoval, 1000, 1000, true, ""}, + {"a removal above the limit, few default-sourced", HolderLimitRemoval, 1001, 500, false, removal}, + {"a removal above the limit, none default-sourced", HolderLimitRemoval, 1001, 0, false, removal}, + {"a removal above the limit, more than half default-sourced", HolderLimitRemoval, 1001, 501, false, removal + viaType + change}, + {"the same, the tier being rank 0", HolderLimitRemoval, 1001, 501, true, removal + viaType + reorder}, + {"a removal of rank 0, few default-sourced", HolderLimitRemoval, 1001, 500, true, removal}, + {"an add below the limit", HolderLimitAdd, 999, 999, false, ""}, + {"an add at the limit", HolderLimitAdd, 1000, 1000, false, ""}, + {"an add above the limit, few default-sourced", HolderLimitAdd, 1001, 500, false, add}, + {"an add above the limit, more than half default-sourced", HolderLimitAdd, 1001, 501, false, add + viaType + change}, + {"an add with exactly half default-sourced", HolderLimitAdd, 1002, 501, false, "1,002 organizations hold this product; adding it as a tier is refused above 1,000."}, + {"an add with just above half default-sourced", HolderLimitAdd, 1002, 502, false, "1,002 organizations hold this product; adding it as a tier is refused above 1,000." + + " 502 of them hold it through their org type's default; " + change}, + {"an add never names the reorder", HolderLimitAdd, 1001, 501, true, add + viaType + change}, + } { + if got := HolderLimitRefusal(tc.act, tc.holders, tc.defaultSourced, 1000, tc.rankZero); got != tc.want { + t.Errorf("%s: HolderLimitRefusal = %q, want %q", tc.name, got, tc.want) + } + } +} + +// PlanActReason words each cause's reason from the detail row's snapshots, in +// the prefixes the Tier changes list recognizes. +func TestPlanActReason(t *testing.T) { + for _, tc := range []struct { + detail planActRecord + want string + }{ + {planActRecord{Cause: ActTierAdded, PlanLadderName: "Main"}, "tier added to ladder Main"}, + {planActRecord{Cause: ActTierRemoved, PlanLadderName: "Main", ProductName: "Basic"}, "tier removal (Basic from Main)"}, + {planActRecord{Cause: ActTiersReordered, PlanLadderName: "Main"}, "plan-ladder tier reorder (Main)"}, + {planActRecord{Cause: ActOrgTypeDefaultChanged, OrgType: "team"}, "org-type default change (team)"}, + {planActRecord{Cause: ActPlanActRetried}, ""}, + } { + if got := PlanActReason(tc.detail); got != tc.want { + t.Errorf("%s: PlanActReason = %q, want %q", tc.detail.Cause, got, tc.want) + } + } +} diff --git a/internal/entitlements/pool_provision_ladders.sql.go b/internal/entitlements/pool_provision_ladders.sql.go index 2b72715e..833162aa 100644 --- a/internal/entitlements/pool_provision_ladders.sql.go +++ b/internal/entitlements/pool_provision_ladders.sql.go @@ -630,3 +630,82 @@ func (q *Queries) ListPlanPoolBucketsByOrgType(ctx context.Context, arg ListPlan } return items, nil } + +const listTierAddCollisions = `-- name: ListTierAddCollisions :many +SELECT rp.pool_id, + o.name AS org_name, + COALESCE(rung.product_name, '')::varchar AS held_product_name, + (rung.pool_id IS NULL)::boolean AS held_twice +FROM core.resource_pools rp +JOIN core.organizations o ON o.org_id = rp.org_id +LEFT JOIN ( + SELECT DISTINCT ON (l.pool_id) l.pool_id, pr.name AS product_name + FROM core.pool_provision_ladders l + JOIN core.products pr ON pr.product_id = l.product_id + WHERE l.plan_ladder_id = $1 + AND l.pool_id = ANY($2::uuid[]) + AND l.status IN ('active', 'suspended') + ORDER BY l.pool_id, l.provision_ladder_id +) rung ON rung.pool_id = rp.pool_id +LEFT JOIN ( + SELECT pp.pool_id + FROM core.pool_provisions pp + WHERE pp.product_id = $3 + AND pp.pool_id = ANY($2::uuid[]) + AND pp.status <> 'ended' + GROUP BY pp.pool_id + HAVING COUNT(*) > 1 +) twice ON twice.pool_id = rp.pool_id +WHERE rp.pool_id = ANY($2::uuid[]) + AND (rung.pool_id IS NOT NULL OR twice.pool_id IS NOT NULL) +ORDER BY rp.pool_id +` + +type ListTierAddCollisionsParams struct { + PlanLadderID string `json:"plan_ladder_id"` + PoolIds []string `json:"pool_ids"` + ProductID string `json:"product_id"` +} + +type ListTierAddCollisionsRow struct { + PoolID string `json:"pool_id"` + OrgName string `json:"org_name"` + HeldProductName string `json:"held_product_name"` + HeldTwice bool `json:"held_twice"` +} + +// The pools among pool_ids (the pools the tier add has locked) that the add +// cannot align the product's provisions in, ascending by pool_id: a pool that +// already holds a live position on the ladder (held_product_name is the +// product that position delivers), and a pool that holds more than one live +// provision of the product, whose second would meet the first's rung +// (held_twice). A pool matching both is listed for the held rung. Run after +// the tier insert; a product already on the ladder is refused as already a +// tier before it (design D15). +func (q *Queries) ListTierAddCollisions(ctx context.Context, arg ListTierAddCollisionsParams) ([]ListTierAddCollisionsRow, error) { + rows, err := q.db.QueryContext(ctx, listTierAddCollisions, arg.PlanLadderID, pq.Array(arg.PoolIds), arg.ProductID) + if err != nil { + return nil, err + } + defer rows.Close() + items := []ListTierAddCollisionsRow{} + for rows.Next() { + var i ListTierAddCollisionsRow + if err := rows.Scan( + &i.PoolID, + &i.OrgName, + &i.HeldProductName, + &i.HeldTwice, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} diff --git a/internal/entitlements/pool_provisions.sql.go b/internal/entitlements/pool_provisions.sql.go index 5767f795..6c6b45cf 100644 --- a/internal/entitlements/pool_provisions.sql.go +++ b/internal/entitlements/pool_provisions.sql.go @@ -14,6 +14,37 @@ import ( "github.com/google/uuid" ) +const countLiveProductHolders = `-- name: CountLiveProductHolders :one +SELECT COUNT(DISTINCT rp.org_id) AS holders, + COUNT(DISTINCT rp.org_id) FILTER (WHERE EXISTS ( + SELECT 1 FROM core.pool_provision_ladders l + WHERE l.provision_id = pp.provision_id + AND l.default_sourced + AND l.status IN ('active', 'suspended') + )) AS default_sourced +FROM core.pool_provisions pp +JOIN core.resource_pools rp ON rp.pool_id = pp.pool_id +WHERE pp.product_id = $1 AND pp.status <> 'ended' +` + +type CountLiveProductHoldersRow struct { + Holders int64 `json:"holders"` + DefaultSourced int64 `json:"default_sourced"` +} + +// The organizations holding a live (non-ended) provision of a product, and how +// many of them hold it through a live default-sourced position on some ladder. +// A default-sourced provision with no position on any ladder (one a keep +// removal left off) counts as a holder and not as default-sourced: changing +// the default would not move it. The tier add refuses above its holder limit +// on the first count and words the way through by the second. +func (q *Queries) CountLiveProductHolders(ctx context.Context, productID string) (CountLiveProductHoldersRow, error) { + row := q.db.QueryRowContext(ctx, countLiveProductHolders, productID) + var i CountLiveProductHoldersRow + err := row.Scan(&i.Holders, &i.DefaultSourced) + return i, err +} + const countPoolsCarryingSet = `-- name: CountPoolsCarryingSet :one SELECT COUNT(DISTINCT pp.pool_id)::BIGINT AS pool_count, COUNT(DISTINCT rp.org_id)::BIGINT AS org_count diff --git a/internal/entitlements/querier.go b/internal/entitlements/querier.go index efb3ab1b..17810010 100644 --- a/internal/entitlements/querier.go +++ b/internal/entitlements/querier.go @@ -56,6 +56,13 @@ type Querier interface { // The tier's live holders, positions that are active or suspended: the // population a removal reconciles, and the count a plain delete refuses on. CountLiveAttachmentsByTier(ctx context.Context, arg CountLiveAttachmentsByTierParams) (int64, error) + // The organizations holding a live (non-ended) provision of a product, and how + // many of them hold it through a live default-sourced position on some ladder. + // A default-sourced provision with no position on any ladder (one a keep + // removal left off) counts as a holder and not as default-sourced: changing + // the default would not move it. The tier add refuses above its holder limit + // on the first count and words the way through by the second. + CountLiveProductHolders(ctx context.Context, productID string) (CountLiveProductHoldersRow, error) // The drain's own reads, beside the ledger queries in // entitlement_set_changes.sql. Neither writes: one counts what a change still // owes, the other names the pools whose last attempt left an error so the @@ -480,6 +487,15 @@ type Querier interface { // definition leaves no two of them sharing a ladder, so their conferrals // never block each other. The caller holds the pool lock. ListResumableGrantsHeldBy(ctx context.Context, arg ListResumableGrantsHeldByParams) ([]ListResumableGrantsHeldByRow, error) + // The pools among pool_ids (the pools the tier add has locked) that the add + // cannot align the product's provisions in, ascending by pool_id: a pool that + // already holds a live position on the ladder (held_product_name is the + // product that position delivers), and a pool that holds more than one live + // provision of the product, whose second would meet the first's rung + // (held_twice). A pool matching both is listed for the held rung. Run after + // the tier insert; a product already on the ladder is refused as already a + // tier before it (design D15). + ListTierAddCollisions(ctx context.Context, arg ListTierAddCollisionsParams) ([]ListTierAddCollisionsRow, error) // Writes to core.pool_provision_transitions go through the conferral functions // (queries/conferral.sql) alone; core_writer holds no direct DML on this // table (migration 00005 enclosure). Only read paths live here. diff --git a/internal/entitlements/queries/pool_provision_ladders.sql b/internal/entitlements/queries/pool_provision_ladders.sql index 161d3eae..f4cb323c 100644 --- a/internal/entitlements/queries/pool_provision_ladders.sql +++ b/internal/entitlements/queries/pool_provision_ladders.sql @@ -206,3 +206,40 @@ LEFT JOIN core.products tp ON tp.product_id = pr.product_id WHERE l.pool_id = $1 AND l.status = 'active' ORDER BY l.activated_at DESC, l.plan_ladder_id; + +-- name: ListTierAddCollisions :many +-- The pools among pool_ids (the pools the tier add has locked) that the add +-- cannot align the product's provisions in, ascending by pool_id: a pool that +-- already holds a live position on the ladder (held_product_name is the +-- product that position delivers), and a pool that holds more than one live +-- provision of the product, whose second would meet the first's rung +-- (held_twice). A pool matching both is listed for the held rung. Run after +-- the tier insert; a product already on the ladder is refused as already a +-- tier before it (design D15). +SELECT rp.pool_id, + o.name AS org_name, + COALESCE(rung.product_name, '')::varchar AS held_product_name, + (rung.pool_id IS NULL)::boolean AS held_twice +FROM core.resource_pools rp +JOIN core.organizations o ON o.org_id = rp.org_id +LEFT JOIN ( + SELECT DISTINCT ON (l.pool_id) l.pool_id, pr.name AS product_name + FROM core.pool_provision_ladders l + JOIN core.products pr ON pr.product_id = l.product_id + WHERE l.plan_ladder_id = sqlc.arg(plan_ladder_id) + AND l.pool_id = ANY(sqlc.arg(pool_ids)::uuid[]) + AND l.status IN ('active', 'suspended') + ORDER BY l.pool_id, l.provision_ladder_id +) rung ON rung.pool_id = rp.pool_id +LEFT JOIN ( + SELECT pp.pool_id + FROM core.pool_provisions pp + WHERE pp.product_id = sqlc.arg(product_id) + AND pp.pool_id = ANY(sqlc.arg(pool_ids)::uuid[]) + AND pp.status <> 'ended' + GROUP BY pp.pool_id + HAVING COUNT(*) > 1 +) twice ON twice.pool_id = rp.pool_id +WHERE rp.pool_id = ANY(sqlc.arg(pool_ids)::uuid[]) + AND (rung.pool_id IS NOT NULL OR twice.pool_id IS NOT NULL) +ORDER BY rp.pool_id; diff --git a/internal/entitlements/queries/pool_provisions.sql b/internal/entitlements/queries/pool_provisions.sql index 839e5111..70f44aea 100644 --- a/internal/entitlements/queries/pool_provisions.sql +++ b/internal/entitlements/queries/pool_provisions.sql @@ -42,3 +42,21 @@ SELECT COUNT(DISTINCT pp.pool_id)::BIGINT AS pool_count, FROM core.pool_provisions pp JOIN core.resource_pools rp ON rp.pool_id = pp.pool_id WHERE pp.entitlement_set_id = $1 AND pp.status = 'active'; + +-- name: CountLiveProductHolders :one +-- The organizations holding a live (non-ended) provision of a product, and how +-- many of them hold it through a live default-sourced position on some ladder. +-- A default-sourced provision with no position on any ladder (one a keep +-- removal left off) counts as a holder and not as default-sourced: changing +-- the default would not move it. The tier add refuses above its holder limit +-- on the first count and words the way through by the second. +SELECT COUNT(DISTINCT rp.org_id) AS holders, + COUNT(DISTINCT rp.org_id) FILTER (WHERE EXISTS ( + SELECT 1 FROM core.pool_provision_ladders l + WHERE l.provision_id = pp.provision_id + AND l.default_sourced + AND l.status IN ('active', 'suspended') + )) AS default_sourced +FROM core.pool_provisions pp +JOIN core.resource_pools rp ON rp.pool_id = pp.pool_id +WHERE pp.product_id = $1 AND pp.status <> 'ended'; diff --git a/internal/entitlements/tier_add_db_test.go b/internal/entitlements/tier_add_db_test.go new file mode 100644 index 00000000..7c301987 --- /dev/null +++ b/internal/entitlements/tier_add_db_test.go @@ -0,0 +1,231 @@ +// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Commercial +// SPDX-FileCopyrightText: 2025-2026 Christian Galo + +package entitlements_test + +// The tier add on the runner (openspec change plan-act-obligations, task 5): +// the order it locks pools in, the pools it refuses before any alignment, the +// holders it counts against its limit, and the product it reads. + +import ( + "context" + "errors" + "slices" + "strings" + "testing" + + "github.com/google/uuid" + + "git.coopcloud.tech/wiki-cafe/member-console/internal/entitlements" + "git.coopcloud.tech/wiki-cafe/member-console/internal/testkit/dbtest" + "git.coopcloud.tech/wiki-cafe/member-console/internal/testkit/world" +) + +// poolLocks records the pool rows the act locks through its querier, in order. +type poolLocks struct { + entitlements.Querier + locked *[]string +} + +func (r poolLocks) LockResourcePool(ctx context.Context, poolID string) (string, error) { + *r.locked = append(*r.locked, poolID) + return r.Querier.LockResourcePool(ctx, poolID) +} + +// A tier add locks the pools of the product's provisions in ascending pool id +// order, whatever order the provisions were created in, and aligns them in +// that order (#194). +func TestAddTierLocksThePoolsInAscendingOrder(t *testing.T) { + w := world.New(t, dbtest.Open(t)) + operator := w.Person("Operator") + basic, extra := w.Product("Basic"), w.Product("Extra") + main := w.Ladder("Main", basic) + orgType := w.OrgType("lock") + first, second := w.Org("First", orgType, operator), w.Org("Second", orgType, operator) + low, high := first, second + if low.PoolID > high.PoolID { + low, high = high, low + } + // The provision of the higher pool is created first. + w.Issue(high, extra, world.IssueOpts{By: operator}) + w.Issue(low, extra, world.IssueOpts{By: operator}) + + var locked []string + err := entitlements.AddTier(w.Ctx, w.DB, entitlements.AddTierInput{ + LadderID: main.ID, ProductID: extra.ID, OperatorPersonID: operator.ID, + Acts: entitlements.PlanActConfig{Entitlements: func(d entitlements.DBTX) entitlements.Querier { + return poolLocks{Querier: entitlements.New(d), locked: &locked} + }}, + }) + if err != nil { + t.Fatalf("AddTier: %v", err) + } + if want := []string{low.PoolID, high.PoolID}; !slices.Equal(locked, want) { + t.Errorf("pools locked as %v, want %v", locked, want) + } + aligned := dbtest.On(t, w.DB).Strings(`SELECT pool_id::text FROM core.pool_provision_transitions + WHERE plan_ladder_id = $1 ORDER BY transition_id`, main.ID) + if want := []string{low.PoolID, high.PoolID}; !slices.Equal(aligned, want) { + t.Errorf("pools aligned as %v, want %v", aligned, want) + } +} + +// writes is what a tier add writes: the ladder's tiers, its acts and the +// transitions of the pools given. +func writes(t *testing.T, w *world.World, ladderID string, pools ...string) []int64 { + t.Helper() + q := dbtest.On(t, w.DB) + return []int64{ + q.Int(`SELECT count(*) FROM core.plan_ladder_tiers WHERE plan_ladder_id = $1`, ladderID), + q.Int(`SELECT count(*) FROM core.plan_act_details WHERE plan_ladder_id = $1`, ladderID), + q.Int(`SELECT count(*) FROM core.pool_provision_transitions WHERE pool_id = ANY($1::uuid[])`, "{"+strings.Join(pools, ",")+"}"), + } +} + +// A pool that already holds a live position on the ladder, or that holds two +// live provisions of the product, refuses the add before any alignment and +// writes nothing: the first such pool in ascending pool order is the one +// named, with the names read inside the transaction. +func TestAddTierRefusesAPoolThatCannotTakeTheTier(t *testing.T) { + for _, tc := range []struct { + name string + build func(w *world.World, operator world.Person) (ladder world.Ladder, product world.Product, pools []world.Org, want entitlements.TierAddCollisionError) + }{ + {"a pool already holding a position on the ladder", func(w *world.World, operator world.Person) (world.Ladder, world.Product, []world.Org, entitlements.TierAddCollisionError) { + basic, extra := w.Product("Basic"), w.Product("Extra") + main := w.Ladder("Main", basic) + orgType := w.OrgType("rung") + w.DefaultLadder(orgType, main) + holder := w.Org("Holder", orgType, operator) + w.Floor(holder) + w.Issue(holder, extra, world.IssueOpts{By: operator}) + return main, extra, []world.Org{holder}, entitlements.TierAddCollisionError{Organization: holder.Name, Product: basic.Name} + }}, + {"a pool holding a position on the ladder and the product twice", func(w *world.World, operator world.Person) (world.Ladder, world.Product, []world.Org, entitlements.TierAddCollisionError) { + basic, extra := w.Product("Basic"), w.Product("Extra") + w.Price(extra, 200) + main := w.Ladder("Main", basic) + orgType := w.OrgType("both") + w.DefaultLadder(orgType, main) + holder := w.Org("Holder", orgType, operator) + w.Floor(holder) + w.Issue(holder, extra, world.IssueOpts{By: operator}) + w.Subscribe(holder, world.SubOpts{}, extra) + return main, extra, []world.Org{holder}, entitlements.TierAddCollisionError{Organization: holder.Name, Product: basic.Name} + }}, + {"a pool holding the product twice", func(w *world.World, operator world.Person) (world.Ladder, world.Product, []world.Org, entitlements.TierAddCollisionError) { + basic, extra := w.Product("Basic"), w.Product("Extra") + w.Price(extra, 200) + main := w.Ladder("Main", basic) + holder := w.Org("Holder", w.OrgType("twice"), operator) + w.Issue(holder, extra, world.IssueOpts{By: operator}) + w.Subscribe(holder, world.SubOpts{}, extra) + return main, extra, []world.Org{holder}, entitlements.TierAddCollisionError{Organization: holder.Name, Product: extra.Name, MoreThanOnce: true} + }}, + {"the first of two such pools", func(w *world.World, operator world.Person) (world.Ladder, world.Product, []world.Org, entitlements.TierAddCollisionError) { + basic, extra := w.Product("Basic"), w.Product("Extra") + w.Price(extra, 200) + main := w.Ladder("Main", basic) + orgType := w.OrgType("both") + one, two := w.Org("One", orgType, operator), w.Org("Two", orgType, operator) + for _, o := range []world.Org{one, two} { + w.Issue(o, extra, world.IssueOpts{By: operator}) + w.Subscribe(o, world.SubOpts{}, extra) + } + first := one + if two.PoolID < one.PoolID { + first = two + } + return main, extra, []world.Org{one, two}, entitlements.TierAddCollisionError{Organization: first.Name, Product: extra.Name, MoreThanOnce: true} + }}, + } { + t.Run(tc.name, func(t *testing.T) { + w := world.New(t, dbtest.Open(t)) + operator := w.Person("Operator") + ladder, product, orgs, want := tc.build(w, operator) + pools := make([]string, len(orgs)) + for i, o := range orgs { + pools[i] = o.PoolID + } + before := writes(t, w, ladder.ID, pools...) + + err := entitlements.AddTier(w.Ctx, w.DB, entitlements.AddTierInput{ + LadderID: ladder.ID, ProductID: product.ID, OperatorPersonID: operator.ID, + }) + var got *entitlements.TierAddCollisionError + if !errors.As(err, &got) || *got != want { + t.Fatalf("AddTier error = %v, want %+v", err, want) + } + if after := writes(t, w, ladder.ID, pools...); !slices.Equal(after, before) { + t.Errorf("the refused add wrote: tiers, acts and transitions went from %v to %v", before, after) + } + }) + } +} + +// A default-sourced provision a keep removal left off every ladder counts +// toward the limit, because the add aligns it, and not toward the way +// through: changing the default would not end it, so the holder count would +// not fall. +func TestAddTierCountsAKeptProvisionOnlyTowardTheLimit(t *testing.T) { + w := world.New(t, dbtest.Open(t)) + operator := w.Person("Operator") + basic, plus := w.Product("Basic"), w.Product("Plus") + main, other := w.Ladder("Main", basic, plus), w.Ladder("Other") + orgType := w.OrgType("kept") + w.DefaultLadder(orgType, main) + for _, name := range []string{"First", "Second"} { + w.Floor(w.Org(name, orgType, operator)) + } + q := entitlements.New(w.DB) + counts, err := q.CountLiveProductHolders(w.Ctx, basic.ID) + if err != nil || counts.Holders != 2 || counts.DefaultSourced != 2 { + t.Fatalf("holders before the removal = %+v, %v; want 2 holders, 2 default-sourced", counts, err) + } + + if _, err := entitlements.RemoveTier(w.Ctx, w.DB, entitlements.RemoveTierInput{ + LadderID: main.ID, ProductID: basic.ID, ProductName: basic.Name, LadderLabel: main.Name, + Disposition: "keep", OperatorPersonID: operator.ID, + }); err != nil { + t.Fatalf("RemoveTier: %v", err) + } + counts, err = q.CountLiveProductHolders(w.Ctx, basic.ID) + if err != nil || counts.Holders != 2 || counts.DefaultSourced != 0 { + t.Fatalf("holders after the removal = %+v, %v; want 2 holders, none default-sourced", counts, err) + } + + err = entitlements.AddTier(w.Ctx, w.DB, entitlements.AddTierInput{ + LadderID: other.ID, ProductID: basic.ID, OperatorPersonID: operator.ID, + Acts: entitlements.PlanActConfig{HolderCap: 1}, + }) + var limit *entitlements.HolderLimitError + if !errors.As(err, &limit) { + t.Fatalf("AddTier error = %v, want a *HolderLimitError", err) + } + if want := "2 organizations hold this product; adding it as a tier is refused above 1."; limit.Message != want { + t.Errorf("refusal = %q, want %q", limit.Message, want) + } +} + +// A product no row names and a product that is not published are refused by +// the add's own read of the product, before any lock. +func TestAddTierRefusesAProductThatIsNotAPublishedTier(t *testing.T) { + w := world.New(t, dbtest.Open(t)) + operator := w.Person("Operator") + basic, draft := w.Product("Basic"), w.Product("Draft") + main := w.Ladder("Main", basic) + w.Draft(draft) + add := func(productID string) error { + return entitlements.AddTier(w.Ctx, w.DB, entitlements.AddTierInput{ + LadderID: main.ID, ProductID: productID, OperatorPersonID: operator.ID, + }) + } + + if err := add(uuid.NewString()); !errors.Is(err, entitlements.ErrTierProductNotFound) { + t.Errorf("a product no row names: error = %v, want ErrTierProductNotFound", err) + } + var notPublished *entitlements.ProductNotPublishedError + if err := add(draft.ID); !errors.As(err, ¬Published) || notPublished.Status != "draft" { + t.Errorf("a draft product: error = %v, want a *ProductNotPublishedError for draft", err) + } +} diff --git a/internal/server/operator_plan_ladder_tier_add_test.go b/internal/server/operator_plan_ladder_tier_add_test.go new file mode 100644 index 00000000..80f8fd86 --- /dev/null +++ b/internal/server/operator_plan_ladder_tier_add_test.go @@ -0,0 +1,242 @@ +// SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Commercial +// SPDX-FileCopyrightText: 2025-2026 Christian Galo + +package server_test + +// The tier add's failures as the operator reads them (openspec change +// plan-act-obligations, task 5.3): every failure that is not a refusal shows +// the request's reference, and its log line carries the same request id and +// the step that failed. + +import ( + "context" + "errors" + "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "slices" + "strings" + "testing" + + "git.coopcloud.tech/wiki-cafe/member-console/internal/billing" + "git.coopcloud.tech/wiki-cafe/member-console/internal/entitlements" + "git.coopcloud.tech/wiki-cafe/member-console/internal/logging" + "git.coopcloud.tech/wiki-cafe/member-console/internal/middleware" + "git.coopcloud.tech/wiki-cafe/member-console/internal/testkit/dbtest" + "git.coopcloud.tech/wiki-cafe/member-console/internal/testkit/world" +) + +// addTierInRequest sends the tier add for productID under the request id, with +// the request's logger, as the server wraps a handler. +func (h *otcHarness) addTierInRequest(requestID, ladderID, productID string) (int, string) { + h.t.Helper() + handler := middleware.RequestID()(middleware.Logging()(http.HandlerFunc(h.handler.CreatePlanLadderTier))) + ctx := logging.WithContext(h.ctx, slog.New(slog.NewTextHandler(h.logs, nil))) + req := httptest.NewRequestWithContext(ctx, http.MethodPost, "/", strings.NewReader(url.Values{"product_id": {productID}}.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set(middleware.RequestIDHeader, requestID) + req.SetPathValue("ladderID", ladderID) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + return rec.Code, rec.Body.String() +} + +// addRefusals is the entitlements querier whose named statement fails, built on +// the act's own transaction. A collision read that is set to find nothing +// returns no row, as a read that missed a collision would. +type addRefusals struct { + entitlements.Querier + fail string + blindCollisions bool + provisionReads *int +} + +var errAddRefused = errors.New("statement refused") + +func (r addRefusals) CountLiveProductHolders(ctx context.Context, productID string) (entitlements.CountLiveProductHoldersRow, error) { + if r.fail == "holders" { + return entitlements.CountLiveProductHoldersRow{}, errAddRefused + } + return r.Querier.CountLiveProductHolders(ctx, productID) +} + +func (r addRefusals) GetLivePoolProvisionsByProductID(ctx context.Context, productID string) ([]entitlements.PoolProvision, error) { + if r.fail == "provisions" { + return nil, errAddRefused + } + if r.fail == "reread" { + *r.provisionReads++ + if *r.provisionReads == 2 { + return nil, errAddRefused + } + } + return r.Querier.GetLivePoolProvisionsByProductID(ctx, productID) +} + +func (r addRefusals) LockResourcePool(ctx context.Context, poolID string) (string, error) { + if r.fail == "locks" { + return "", errAddRefused + } + return r.Querier.LockResourcePool(ctx, poolID) +} + +func (r addRefusals) RecordPlanAct(ctx context.Context, arg entitlements.RecordPlanActParams) (string, error) { + if r.fail == "act" { + return "", errAddRefused + } + return r.Querier.RecordPlanAct(ctx, arg) +} + +func (r addRefusals) ListTierAddCollisions(ctx context.Context, arg entitlements.ListTierAddCollisionsParams) ([]entitlements.ListTierAddCollisionsRow, error) { + if r.fail == "collisions" { + return nil, errAddRefused + } + if r.blindCollisions { + return nil, nil + } + return r.Querier.ListTierAddCollisions(ctx, arg) +} + +// productRefused is the billing querier whose product read fails. +type productRefused struct{ billing.Querier } + +func (productRefused) GetProductByID(context.Context, string) (billing.Product, error) { + return billing.Product{}, errAddRefused +} + +// ladderRefused is the billing querier whose named statement fails: the +// ladder read that names the act, or the tier insert with an error that is not +// a constraint violation. +type ladderRefused struct { + billing.Querier + fail string +} + +func (r ladderRefused) GetPlanLadderByID(ctx context.Context, id string) (billing.PlanLadder, error) { + if r.fail == "ladder" { + return billing.PlanLadder{}, errAddRefused + } + return r.Querier.GetPlanLadderByID(ctx, id) +} + +func (r ladderRefused) CreatePlanLadderTier(ctx context.Context, arg billing.CreatePlanLadderTierParams) (billing.PlanLadderTier, error) { + if r.fail == "insert" { + return billing.PlanLadderTier{}, errAddRefused + } + return r.Querier.CreatePlanLadderTier(ctx, arg) +} + +// A world with one organization holding Extra, a ladder of Basic and a type +// that defaults to the ladder: the add of Extra reaches every step. +func addTierWorld(t *testing.T) (w *world.World, operator world.Person, main world.Ladder, extra world.Product, holder world.Org) { + t.Helper() + w = world.New(t, dbtest.Open(t)) + operator = w.Person("Operator") + basic := w.Product("Basic") + extra = w.Product("Extra") + main = w.Ladder("Main", basic) + orgType := w.OrgType("add") + w.DefaultLadder(orgType, main) + holder = w.Org("Holder", orgType, operator) + w.Issue(holder, extra, world.IssueOpts{By: operator}) + return w, operator, main, extra, holder +} + +// A failure of the add that is not a refusal answers with the failure sentence +// and the request's identifier, and logs the step that failed under the same +// identifier; the tier is not added. +func TestCreatePlanLadderTier_EveryOtherFailureShowsTheReference(t *testing.T) { + for _, tc := range []struct { + name string + acts func() entitlements.PlanActConfig + step string + }{ + {"the product read", func() entitlements.PlanActConfig { + return entitlements.PlanActConfig{Billing: func(d billing.DBTX) billing.Querier { return productRefused{billing.New(d)} }} + }, `step="read the product"`}, + {"the holder count", refusing("holders"), `step="count the product's holders"`}, + {"the first provision read", refusing("provisions"), `step="read the product's live provisions"`}, + {"the second provision read", refusing("reread"), `step="read the product's live provisions under the pool locks"`}, + {"the ladder's name read", func() entitlements.PlanActConfig { + return entitlements.PlanActConfig{Billing: func(d billing.DBTX) billing.Querier { return ladderRefused{billing.New(d), "ladder"} }} + }, `step="read the ladder's and the product's names"`}, + {"the tier insert", func() entitlements.PlanActConfig { + return entitlements.PlanActConfig{Billing: func(d billing.DBTX) billing.Querier { return ladderRefused{billing.New(d), "insert"} }} + }, `step="create the tier"`}, + {"the pool locks", refusing("locks"), `step="lock the holders' pools"`}, + {"the act record", refusing("act"), `step="record the act"`}, + {"the collision read", refusing("collisions"), `step="read the pools that cannot take the tier"`}, + } { + t.Run(tc.name, func(t *testing.T) { + w, operator, main, extra, _ := addTierWorld(t) + h := newOtcHarnessActs(t, w.DB, operator.ID, tc.acts()) + + code, body := h.addTierInRequest("ref-31", main.ID, extra.ID) + if code != http.StatusOK { + t.Fatalf("status=%d body=%s", code, body) + } + if !strings.Contains(body, "Failed to add tier. Reference: ref-31.") { + t.Errorf("the answer lacks the failure sentence with the request id; body:\n%s", body) + } + wantInLine(t, h.logs.String(), "tier add failed", "request_id=ref-31", tc.step, "statement refused") + if n := dbtest.On(t, w.DB).Int(`SELECT count(*) FROM core.plan_ladder_tiers WHERE plan_ladder_id = $1`, main.ID); n != 1 { + t.Errorf("the ladder has %d tiers after the failed add, want 1", n) + } + }) + } +} + +// wantInLine fails the test unless one line of logs holds every part: the +// request's own access log carries the request id too, so the parts must meet +// in the failure's line. +func wantInLine(t *testing.T, logs string, parts ...string) { + t.Helper() + for line := range strings.Lines(logs) { + if strings.Contains(line, parts[0]) && !slices.ContainsFunc(parts, func(p string) bool { return !strings.Contains(line, p) }) { + return + } + } + t.Errorf("no log line holds all of %q:\n%s", parts, logs) +} + +// refusing is an act configuration whose entitlements querier fails the +// statement named. +func refusing(statement string) func() entitlements.PlanActConfig { + return func() entitlements.PlanActConfig { + return entitlements.PlanActConfig{Entitlements: func(d entitlements.DBTX) entitlements.Querier { + return addRefusals{Querier: entitlements.New(d), fail: statement, provisionReads: new(int)} + }} + } +} + +// A conflict the alignment raises although the collision read should have +// found it (a defect) answers with the failure sentence and the request's +// identifier. The driver text is in the log line under the same identifier and +// is not shown. +func TestCreatePlanLadderTier_ACollisionTheAlignmentRaisesIsLoggedNotShown(t *testing.T) { + w, operator, main, extra, holder := addTierWorld(t) + w.Floor(holder) // the pool now holds Basic on Main, the rung Extra needs + h := newOtcHarnessActs(t, w.DB, operator.ID, entitlements.PlanActConfig{ + Entitlements: func(d entitlements.DBTX) entitlements.Querier { + return addRefusals{Querier: entitlements.New(d), blindCollisions: true} + }, + }) + + code, body := h.addTierInRequest("ref-32", main.ID, extra.ID) + if code != http.StatusOK { + t.Fatalf("status=%d body=%s", code, body) + } + if !strings.Contains(body, "Failed to add tier. Reference: ref-32.") { + t.Errorf("the answer lacks the failure sentence with the request id; body:\n%s", body) + } + for _, shown := range []string{"align_conferral_shape", "rung", "SQLSTATE"} { + if strings.Contains(body, shown) { + t.Errorf("the answer shows driver text %q:\n%s", shown, body) + } + } + wantInLine(t, h.logs.String(), "tier add failed", "request_id=ref-32", `step="align a position to its product's shape"`, "alignment cannot supersede") + if n := dbtest.On(t, w.DB).Int(`SELECT count(*) FROM core.plan_ladder_tiers WHERE plan_ladder_id = $1`, main.ID); n != 1 { + t.Errorf("the ladder has %d tiers after the failed add, want 1", n) + } +} diff --git a/internal/server/operator_plan_ladders.go b/internal/server/operator_plan_ladders.go index 3bf867c6..1d448179 100644 --- a/internal/server/operator_plan_ladders.go +++ b/internal/server/operator_plan_ladders.go @@ -393,82 +393,95 @@ func (h *OperatorPartialsHandler) CreatePlanLadderTier(w http.ResponseWriter, r productID := values.String("product_id") - // Any published product may be a tier (Doc 41: display_category is - // presentation-only and never gates ladder membership). - product, err := h.BillingQ.GetProductByID(r.Context(), productID) - if err != nil { - errs.Field("product_id", "Product not found.") - h.renderPlanLadderTierAddRefusal(w, r, ladderID, values, errs) - return - } - if product.LifecycleStatus != "published" { - errs.Field("product_id", "Only published products can be tiers. This product is "+product.LifecycleStatus+".") - h.renderPlanLadderTierAddRefusal(w, r, ladderID, values, errs) - return - } - - // Tier creation and the shape-alignment of every live provision of this - // product run in one transaction (entitlements.AddTier). Rank - // self-assigns to MAX+1 inside the INSERT — new tiers append at the - // bottom; the operator drags them into position (that is also the only - // way the rank-0 provisioning default changes, which is the right - // affordance). - err = entitlements.AddTier(r.Context(), h.Database, entitlements.AddTierInput{ + // The product's publication, the holder limit, the tier creation and the + // shape-alignment of every live provision of the product run in one + // transaction (entitlements.AddTier). Rank self-assigns to MAX+1 inside + // the INSERT: new tiers append at the bottom; the operator drags them + // into position (that is also the only way the rank-0 provisioning + // default changes, which is the right affordance). + err := entitlements.AddTier(r.Context(), h.Database, entitlements.AddTierInput{ LadderID: ladderID, ProductID: productID, OperatorPersonID: session.PersonID, + RequestID: middleware.GetRequestID(r.Context()), + Acts: h.PlanActs, }) if err == nil { // Signal dependent tabs (Org Types) to re-fetch — they show plan ladder dropdowns h.renderPlanLadderTiersPage(w, r, ladderID, "Tier added at the bottom; drag it into position.", "") return } - if h.refuseTierAdd(errs, err) { - h.renderPlanLadderTierAddRefusal(w, r, ladderID, values, errs) + if alert := h.refuseTierAdd(r, ladderID, productID, errs, err); alert != "" { + h.renderPlanLadderTiersPage(w, r, ladderID, "", alert) return } - h.renderPlanLadderTiersPage(w, r, ladderID, "", "Failed to add tier; see server logs.") + h.renderPlanLadderTierAddRefusal(w, r, ladderID, values, errs) } -// refuseTierAdd puts a tier add the database refused under the product -// field and reports that it did: the product already a tier, a rank -// collision with a concurrent add, or a rung the grown shape needs already -// held (CF003, named by the alignment's error). Any other failure is logged -// as the step that failed is logged and reports false. -func (h *OperatorPartialsHandler) refuseTierAdd(errs *forms.Errors, err error) bool { +// refuseTierAdd answers a tier add that did not commit. A refusal of the +// add's own guards, or a database refusal the operator can act on (the +// product already a tier, a rank collision with a concurrent add), goes under +// the product field and the result is "". Any other failure is logged with +// the step that failed, through the request's logger so the line carries the +// request id, and the result is the sentence for the page's alert: a lock +// wait or a deadlock reads as the contention refusal, the rest as the failure +// sentence with that request id, so an operator who cannot read the logs can +// hand it to the person who can. That includes a CF003 from the alignment, +// which the add's collision read should have prevented: its driver text is in +// the log line, not on the page. +func (h *OperatorPartialsHandler) refuseTierAdd(r *http.Request, ladderID, productID string, errs *forms.Errors, err error) string { + var published *entitlements.ProductNotPublishedError + var limit *entitlements.HolderLimitError + var collision *entitlements.TierAddCollisionError + switch { + case errors.Is(err, entitlements.ErrTierProductNotFound): + errs.Field("product_id", "Product not found.") + return "" + case errors.As(err, &published): + errs.Field("product_id", "Only published products can be tiers. This product is "+published.Status+".") + return "" + case errors.As(err, &limit): + errs.Field("product_id", limit.Message) + return "" + case errors.As(err, &collision): + errs.Field("product_id", tierAddCollisionSentence(collision)) + return "" + } + + step := entitlements.PlanActStep("") var failed *entitlements.PlanActError - if !errors.As(err, &failed) { - return false - } - cause := failed.Err - switch failed.Step { - case entitlements.StepBegin: - h.Logger.Error("failed to begin add-tier tx", slog.Any("error", cause)) - case entitlements.StepCreateTier: - if fe, ok := web.FieldErrorsFromDB(cause, web.ConstraintMessages{ - "plan_ladder_tiers_pkey": {Field: "product_id", Message: "This product is already a tier in this ladder."}, - "plan_ladder_tiers_plan_ladder_id_rank_key": {Field: "product_id", Message: "Another change collided with this one; try again."}, - }); ok { - for field, msg := range fe { - errs.Field(field, msg) + if errors.As(err, &failed) { + step = failed.Step + if step == entitlements.StepCreateTier { + if fe, ok := web.FieldErrorsFromDB(failed.Err, web.ConstraintMessages{ + "plan_ladder_tiers_pkey": {Field: "product_id", Message: "This product is already a tier in this ladder."}, + "plan_ladder_tiers_plan_ladder_id_rank_key": {Field: "product_id", Message: entitlements.MessageInProgress}, + }); ok { + for field, msg := range fe { + errs.Field(field, msg) + } + return "" } - return true } - h.Logger.Error("failed to create plan ladder tier", slog.Any("error", cause)) - case entitlements.StepReadProvisions: - h.Logger.Error("failed to list live provisions for tier product", slog.Any("error", cause)) - case entitlements.StepAlign: - if errors.Is(cause, entitlements.ErrConferralShapeCollision) { - errs.Field("product_id", "Cannot add this tier; a rung is already held: "+cause.Error()) - return true - } - h.Logger.Error("failed to align provision to new tier shape", slog.Any("error", cause), slog.String("provision_id", failed.ProvisionID)) - case entitlements.StepMaterialize: - h.Logger.Error("failed to materialize after tier align", slog.Any("error", cause)) - case entitlements.StepCommit: - h.Logger.Error("failed to commit add-tier tx", slog.Any("error", cause)) } - return false + logging.FromContext(r.Context()).Error("tier add failed", + slog.String("step", string(step)), + slog.String("ladder_id", ladderID), + slog.String("product_id", productID), + slog.Any("error", err)) + if db.IsLockContention(err) { + return entitlements.MessageInProgress + } + return "Failed to add tier. Reference: " + middleware.GetRequestID(r.Context()) + "." +} + +// tierAddCollisionSentence is the product field's error for a pool the tier +// add cannot align, with the names the add read inside its transaction. +func tierAddCollisionSentence(c *entitlements.TierAddCollisionError) string { + if c.MoreThanOnce { + return "Cannot add this tier; " + c.Organization + " holds " + c.Product + " more than once, and an organization holds one position per ladder." + } + return "Cannot add this tier; " + c.Organization + " already holds " + c.Product + " on this ladder." } // reconcileTierOrder merges a submitted product order with the ladder's diff --git a/internal/server/plan_acts_golden_db_test.go b/internal/server/plan_acts_golden_db_test.go index a1058206..c265d730 100644 --- a/internal/server/plan_acts_golden_db_test.go +++ b/internal/server/plan_acts_golden_db_test.go @@ -64,6 +64,8 @@ type actFixture struct { // refuse names the statements the handler's queriers and the act's own // refuse. refuse actRefusals + // holderCap replaces the holder limit of a tier add when it is set. + holderCap int64 // namedActs are the acts the normalizer already names. namedActs map[string]bool } @@ -303,6 +305,7 @@ func (f *actFixture) prepare(act func(*OperatorPartialsHandler) http.HandlerFunc // The act builds its own queriers on its transaction. They refuse the // statements the scenario names, as the handler's do. PlanActs: entitlements.PlanActConfig{ + HolderCap: f.holderCap, Billing: func(d billing.DBTX) billing.Querier { return actBillingQ{Querier: billing.New(d), refuse: f.refuse} }, @@ -694,6 +697,41 @@ var planActScenarios = []struct { f.w.Issue(o, extra, world.IssueOpts{By: f.operator}) return f.act(actAddTier, url.Values{"product_id": {extra.ID}}, authtest.PathValue("ladderID", main.ID)) }}, + {"add-above-the-limit", func(f *actFixture) actGolden { + basic, extra := f.product("Basic", 1), f.product("Extra", 7) + main := f.ladder("Main", basic) + u := f.orgType("U", nil) + f.w.Issue(f.org("First", u), extra, world.IssueOpts{By: f.operator}) + f.w.Issue(f.org("Second", u), extra, world.IssueOpts{By: f.operator}) + f.holderCap = 1 + return f.act(actAddTier, url.Values{"product_id": {extra.ID}}, authtest.PathValue("ladderID", main.ID)) + }}, + {"add-above-the-limit-default-sourced", func(f *actFixture) actGolden { + basic := f.product("Basic", 1) + main := f.ladder("Main", basic) + other := f.ladder("Other") + t := f.orgType("T", &main) + f.floored("First", t) + f.floored("Second", t) + f.holderCap = 1 + return f.act(actAddTier, url.Values{"product_id": {basic.ID}}, authtest.PathValue("ladderID", other.ID)) + }}, + {"add-product-unread", func(f *actFixture) actGolden { + basic, extra := f.product("Basic", 1), f.product("Extra", 7) + main := f.ladder("Main", basic) + f.w.Issue(f.org("Granted", f.orgType("U", nil)), extra, world.IssueOpts{By: f.operator}) + f.refuse.productName = true + return f.act(actAddTier, url.Values{"product_id": {extra.ID}}, authtest.PathValue("ladderID", main.ID)) + }}, + {"add-product-held-twice", func(f *actFixture) actGolden { + basic, extra := f.product("Basic", 1), f.product("Extra", 7) + f.w.Price(extra, 200) + main := f.ladder("Main", basic) + o := f.org("Holder", f.orgType("U", nil)) + f.w.Issue(o, extra, world.IssueOpts{By: f.operator}) + f.w.Subscribe(o, world.SubOpts{}, extra) + return f.act(actAddTier, url.Values{"product_id": {extra.ID}}, authtest.PathValue("ladderID", main.ID)) + }}, {"add-begin-fails", func(f *actFixture) actGolden { basic, extra := f.product("Basic", 1), f.product("Extra", 7) main := f.ladder("Main", basic) diff --git a/internal/server/testdata/plan_acts/add-above-the-limit-default-sourced.json b/internal/server/testdata/plan_acts/add-above-the-limit-default-sourced.json new file mode 100644 index 00000000..b019c2a1 --- /dev/null +++ b/internal/server/testdata/plan_acts/add-above-the-limit-default-sourced.json @@ -0,0 +1,55 @@ +{ + "answer": [ + "status 422", + ".invalid-feedback: 2 organizations hold this product; adding it as a tier is refused above 1. 2 of them hold it through their org type's default; changing that default with Migrate moves them in the background." + ], + "checked": [], + "logs": [], + "statements": [ + "begin", + "SET LOCAL lock_timeout = '5s'", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", + "rollback" + ], + "rows": { + "tiers": [ + "ladder | rank | product", + " | 0 | " + ], + "defaults": [ + "org_type | default_ladder", + " | " + ], + "grants": [ + "grant_id | org | product | grant_reason | granted_by | extends | description | status", + " | | | default | none | none | | active", + " | | | default | none | none | | active" + ], + "provisions": [ + "provision | pool | grant_id | status", + " | | | active", + " | | | active" + ], + "positions": [ + "provision | ladder | product | status | ended", + " | | | active | false", + " | | | active | false" + ], + "transitions": [ + "pool | provision | ladder | from_rank | to_rank | transition_type | actor_type | actor | reason", + " | | | - | 0 | initiate | system | none | test fixture", + " | | | - | 0 | initiate | system | none | test fixture" + ], + "entitlements": [ + "pool | resource_key | resource_limit", + " | plan_golden_seats | 1", + " | plan_golden_seats | 1" + ], + "acts": [ + "act | cause | actor_type | actor | note | request_id" + ], + "act_details": [ + "act | cause | ladder | ladder_name | product | product_name | previous_product | previous_product_name | org_type | org_type_name | previous_ladder | previous_ladder_name | disposition | retried" + ] + } +} diff --git a/internal/server/testdata/plan_acts/add-above-the-limit.json b/internal/server/testdata/plan_acts/add-above-the-limit.json new file mode 100644 index 00000000..561c709c --- /dev/null +++ b/internal/server/testdata/plan_acts/add-above-the-limit.json @@ -0,0 +1,51 @@ +{ + "answer": [ + "status 422", + ".invalid-feedback: 2 organizations hold this product; adding it as a tier is refused above 1." + ], + "checked": [], + "logs": [], + "statements": [ + "begin", + "SET LOCAL lock_timeout = '5s'", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", + "rollback" + ], + "rows": { + "tiers": [ + "ladder | rank | product", + " | 0 | " + ], + "defaults": [ + "org_type | default_ladder", + " | none" + ], + "grants": [ + "grant_id | org | product | grant_reason | granted_by | extends | description | status", + " | | | complimentary | | none | | active", + " | | | complimentary | | none | | active" + ], + "provisions": [ + "provision | pool | grant_id | status", + " | | | active", + " | | | active" + ], + "positions": [ + "provision | ladder | product | status | ended" + ], + "transitions": [ + "pool | provision | ladder | from_rank | to_rank | transition_type | actor_type | actor | reason" + ], + "entitlements": [ + "pool | resource_key | resource_limit", + " | plan_golden_seats | 7", + " | plan_golden_seats | 7" + ], + "acts": [ + "act | cause | actor_type | actor | note | request_id" + ], + "act_details": [ + "act | cause | ladder | ladder_name | product | product_name | previous_product | previous_product_name | org_type | org_type_name | previous_ladder | previous_ladder_name | disposition | retried" + ] + } +} diff --git a/internal/server/testdata/plan_acts/add-aligns-a-live-provision.json b/internal/server/testdata/plan_acts/add-aligns-a-live-provision.json index 44dbd276..ca164c0b 100644 --- a/internal/server/testdata/plan_acts/add-aligns-a-live-provision.json +++ b/internal/server/testdata/plan_acts/add-aligns-a-live-provision.json @@ -9,6 +9,8 @@ "begin", "SET LOCAL lock_timeout = '5s'", "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", + "LockResourcePool", + "RecordPlanAct", "CreatePlanLadderTier", "SELECT plan_ladder_id, action FROM core.align_conferral_shape($1, $2, $3, $4, $5)", "SELECT core.assert_rendezvous(false)", @@ -41,17 +43,19 @@ ], "transitions": [ "pool | provision | ladder | from_rank | to_rank | transition_type | actor_type | actor | reason", - " | | | - | 1 | initiate | operator | | tier added to ladder " + " | | | - | 1 | initiate | operator | | tier added to ladder Main" ], "entitlements": [ "pool | resource_key | resource_limit", " | plan_golden_seats | 7" ], "acts": [ - "act | cause | actor_type | actor | note | request_id" + "act | cause | actor_type | actor | note | request_id", + " | tier_added | person | | none | " ], "act_details": [ - "act | cause | ladder | ladder_name | product | product_name | previous_product | previous_product_name | org_type | org_type_name | previous_ladder | previous_ladder_name | disposition | retried" + "act | cause | ladder | ladder_name | product | product_name | previous_product | previous_product_name | org_type | org_type_name | previous_ladder | previous_ladder_name | disposition | retried", + " | tier_added | | Main | | Extra | none | none | none | none | none | none | none | none" ] } } diff --git a/internal/server/testdata/plan_acts/add-begin-fails.json b/internal/server/testdata/plan_acts/add-begin-fails.json index 069f0d1f..08955952 100644 --- a/internal/server/testdata/plan_acts/add-begin-fails.json +++ b/internal/server/testdata/plan_acts/add-begin-fails.json @@ -1,12 +1,12 @@ { "answer": [ "status 200", - "Hx-Trigger showErrorToast: Failed to add tier; see server logs.", - "[role=\"alert\"]: Failed to add tier; see server logs." + "Hx-Trigger showErrorToast: Failed to add tier. Reference: .", + "[role=\"alert\"]: Failed to add tier. Reference: ." ], "checked": [], "logs": [ - "level=ERROR msg=\"failed to begin add-tier tx\" error=\"sql: database is closed\"" + "level=ERROR msg=\"tier add failed\" request_id= step=begin ladder_id= product_id= error=\"begin: sql: database is closed\"" ], "statements": [], "rows": { diff --git a/internal/server/testdata/plan_acts/add-collides-with-a-held-rung.json b/internal/server/testdata/plan_acts/add-collides-with-a-held-rung.json index b2c56fe9..4c2de664 100644 --- a/internal/server/testdata/plan_acts/add-collides-with-a-held-rung.json +++ b/internal/server/testdata/plan_acts/add-collides-with-a-held-rung.json @@ -1,7 +1,7 @@ { "answer": [ "status 422", - ".invalid-feedback: Cannot add this tier; a rung is already held: conferral shape alignment collides with a rung held by another provision: align_conferral_shape: rung on ladder is held by provision (active); alignment cannot supersede" + ".invalid-feedback: Cannot add this tier; Holder already holds Basic on this ladder." ], "checked": [], "logs": [], @@ -9,20 +9,26 @@ "begin", "SET LOCAL lock_timeout = '5s'", "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", + "LockResourcePool", + "RecordPlanAct", "CreatePlanLadderTier", - "SELECT plan_ladder_id, action FROM core.align_conferral_shape($1, $2, $3, $4, $5)", "rollback" ], "statements_with_reads": [ "ListActiveProducts", "ListTiersByLadder", - "GetProductByID", "begin", "SET LOCAL lock_timeout = '5s'", "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", - "CreatePlanLadderTier", + "GetProductByID", + "CountLiveProductHolders", "GetLivePoolProvisionsByProductID", - "SELECT plan_ladder_id, action FROM core.align_conferral_shape($1, $2, $3, $4, $5)", + "LockResourcePool", + "GetLivePoolProvisionsByProductID", + "GetPlanLadderByID", + "RecordPlanAct", + "CreatePlanLadderTier", + "ListTierAddCollisions", "rollback", "GetPlanLadderByID", "GetPlanLadderByID", @@ -44,21 +50,21 @@ ], "grants": [ "grant_id | org | product | grant_reason | granted_by | extends | description | status", - " | | | default | none | none | | active", - " | | | complimentary | | none | | active" + " | | | default | none | none | | active", + " | | | complimentary | | none | | active" ], "provisions": [ "provision | pool | grant_id | status", - " | | | active", - " | | | active" + " | | | active", + " | | | active" ], "positions": [ "provision | ladder | product | status | ended", - " | | | active | false" + " | | | active | false" ], "transitions": [ "pool | provision | ladder | from_rank | to_rank | transition_type | actor_type | actor | reason", - " | | | - | 0 | initiate | system | none | test fixture" + " | | | - | 0 | initiate | system | none | test fixture" ], "entitlements": [ "pool | resource_key | resource_limit", diff --git a/internal/server/testdata/plan_acts/add-product-held-twice.json b/internal/server/testdata/plan_acts/add-product-held-twice.json new file mode 100644 index 00000000..f991ebd1 --- /dev/null +++ b/internal/server/testdata/plan_acts/add-product-held-twice.json @@ -0,0 +1,52 @@ +{ + "answer": [ + "status 422", + ".invalid-feedback: Cannot add this tier; Holder holds Extra more than once, and an organization holds one position per ladder." + ], + "checked": [], + "logs": [], + "statements": [ + "begin", + "SET LOCAL lock_timeout = '5s'", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", + "LockResourcePool", + "RecordPlanAct", + "CreatePlanLadderTier", + "rollback" + ], + "rows": { + "tiers": [ + "ladder | rank | product", + " | 0 | " + ], + "defaults": [ + "org_type | default_ladder", + " | none" + ], + "grants": [ + "grant_id | org | product | grant_reason | granted_by | extends | description | status", + " | | | complimentary | | none | | active" + ], + "provisions": [ + "provision | pool | grant_id | status", + " | | | active", + " | | none | active" + ], + "positions": [ + "provision | ladder | product | status | ended" + ], + "transitions": [ + "pool | provision | ladder | from_rank | to_rank | transition_type | actor_type | actor | reason" + ], + "entitlements": [ + "pool | resource_key | resource_limit", + " | plan_golden_seats | 14" + ], + "acts": [ + "act | cause | actor_type | actor | note | request_id" + ], + "act_details": [ + "act | cause | ladder | ladder_name | product | product_name | previous_product | previous_product_name | org_type | org_type_name | previous_ladder | previous_ladder_name | disposition | retried" + ] + } +} diff --git a/internal/server/testdata/plan_acts/add-product-unread.json b/internal/server/testdata/plan_acts/add-product-unread.json new file mode 100644 index 00000000..610cfcc1 --- /dev/null +++ b/internal/server/testdata/plan_acts/add-product-unread.json @@ -0,0 +1,51 @@ +{ + "answer": [ + "status 200", + "Hx-Trigger showErrorToast: Failed to add tier. Reference: .", + "[role=\"alert\"]: Failed to add tier. Reference: ." + ], + "checked": [], + "logs": [ + "level=ERROR msg=\"tier add failed\" request_id= step=\"read the product\" ladder_id= product_id= error=\"read the product: read failed\"" + ], + "statements": [ + "begin", + "SET LOCAL lock_timeout = '5s'", + "SELECT pg_advisory_xact_lock(hashtextextended($1, 0))", + "rollback" + ], + "rows": { + "tiers": [ + "ladder | rank | product", + " | 0 | " + ], + "defaults": [ + "org_type | default_ladder", + " | none" + ], + "grants": [ + "grant_id | org | product | grant_reason | granted_by | extends | description | status", + " | | | complimentary | | none | | active" + ], + "provisions": [ + "provision | pool | grant_id | status", + " | | | active" + ], + "positions": [ + "provision | ladder | product | status | ended" + ], + "transitions": [ + "pool | provision | ladder | from_rank | to_rank | transition_type | actor_type | actor | reason" + ], + "entitlements": [ + "pool | resource_key | resource_limit", + " | plan_golden_seats | 7" + ], + "acts": [ + "act | cause | actor_type | actor | note | request_id" + ], + "act_details": [ + "act | cause | ladder | ladder_name | product | product_name | previous_product | previous_product_name | org_type | org_type_name | previous_ladder | previous_ladder_name | disposition | retried" + ] + } +} diff --git a/internal/server/testdata/plan_acts/contention.json b/internal/server/testdata/plan_acts/contention.json index a36c9bf3..43420828 100644 --- a/internal/server/testdata/plan_acts/contention.json +++ b/internal/server/testdata/plan_acts/contention.json @@ -3,12 +3,12 @@ "add": { "answer": [ "status 200", - "Hx-Trigger showErrorToast: Failed to add tier; see server logs.", - "[role=\"alert\"]: Failed to add tier; see server logs." + "Hx-Trigger showErrorToast: Another change collided with this one; try again.", + "[role=\"alert\"]: Another change collided with this one; try again." ], "checked": [], "logs": [ - "level=ERROR msg=\"failed to begin add-tier tx\" error=\"take materialization rendezvous: ERROR: canceling statement due to lock timeout (SQLSTATE 55P03)\"" + "level=ERROR msg=\"tier add failed\" request_id= step=begin ladder_id= product_id= error=\"begin: take materialization rendezvous: ERROR: canceling statement due to lock timeout (SQLSTATE 55P03)\"" ] }, "default": { @@ -28,7 +28,7 @@ ], "checked": [], "logs": [ - "level=ERROR msg=\"tier delete failed\" request_id= step=begin ladder_id= product_id= error=\"begin: take materialization rendezvous: ERROR: canceling statement due to lock timeout (SQLSTATE 55P03)\"" + "level=ERROR msg=\"tier delete failed\" request_id= step=begin ladder_id= product_id= error=\"begin: take materialization rendezvous: ERROR: canceling statement due to lock timeout (SQLSTATE 55P03)\"" ] }, "remove": { @@ -69,19 +69,19 @@ ], "grants": [ "grant_id | org | product | grant_reason | granted_by | extends | description | status", - " | | | default | none | none | | active" + " | | | default | none | none | | active" ], "provisions": [ "provision | pool | grant_id | status", - " | | | active" + " | | | active" ], "positions": [ "provision | ladder | product | status | ended", - " | | | active | false" + " | | | active | false" ], "transitions": [ "pool | provision | ladder | from_rank | to_rank | transition_type | actor_type | actor | reason", - " | | | - | 0 | initiate | system | none | test fixture" + " | | | - | 0 | initiate | system | none | test fixture" ], "entitlements": [ "pool | resource_key | resource_limit", diff --git a/internal/server/testdata/plan_acts/tier-change-during-conferral/a-tier-add.json b/internal/server/testdata/plan_acts/tier-change-during-conferral/a-tier-add.json index ce9782ac..8c2d0bb4 100644 --- a/internal/server/testdata/plan_acts/tier-change-during-conferral/a-tier-add.json +++ b/internal/server/testdata/plan_acts/tier-change-during-conferral/a-tier-add.json @@ -47,17 +47,19 @@ " | | | - | 0 | initiate | system | none | test fixture", " | | | 0 | - | end | operator | | org-type default change (): grandfathered as legacy", " | | | 0 | 0 | transfer | operator | | org-type default change (): grandfathered as legacy", - " | | | - | 0 | initiate | operator | | tier added to ladder " + " | | | - | 0 | initiate | operator | | tier added to ladder Other" ], "entitlements": [ "pool | resource_key | resource_limit", " | plan_golden_seats | 1" ], "acts": [ - "act | cause | actor_type | actor | note | request_id" + "act | cause | actor_type | actor | note | request_id", + " | tier_added | person | | none | " ], "act_details": [ - "act | cause | ladder | ladder_name | product | product_name | previous_product | previous_product_name | org_type | org_type_name | previous_ladder | previous_ladder_name | disposition | retried" + "act | cause | ladder | ladder_name | product | product_name | previous_product | previous_product_name | org_type | org_type_name | previous_ladder | previous_ladder_name | disposition | retried", + " | tier_added | | Other | | Basic | none | none | none | none | none | none | none | none" ] } } diff --git a/openspec/changes/plan-act-obligations/tasks.md b/openspec/changes/plan-act-obligations/tasks.md index b57b0278..6a607e9c 100644 --- a/openspec/changes/plan-act-obligations/tasks.md +++ b/openspec/changes/plan-act-obligations/tasks.md @@ -50,12 +50,12 @@ Each numbered section below is one commit, and its items are that commit's parts ## 5. The tier add on the runner -- [ ] 5.1 Add `planActHolderCap` to `internal/entitlements/constants.go`, set from task 1's per-holder cost of the add and the removal, with the measurement and the rule in its comment (an add's or a removal's exclusive hold near half a second on the measuring host). Add `HolderLimitRefusal`, the limit and more-than-half rule as a function that reads no row, returning the sentences of copy 15 to 18 or nothing. -- [ ] 5.2 Add `CountLiveProductHolders` (organizations holding a live provision of the product, and how many hold it through a live default-sourced position on some ladder) and the reason builder `PlanActReason` (design D19). Rewrite `AddTier` on the runner: read the product inside and refuse one that is not published (no row answers "Product not found."; any other error the generic failure), count the holders and refuse above `planActHolderCap`, lock the provisions' pools in ascending order before any alignment (#194), read the provisions again, record the act (`tier_added`, names read inside), insert the tier, read `ListTierAddCollisions` for the locked pools and refuse the first collision in ascending pool id order with a `*TierAddCollisionError` carrying the organization's and the product's names (design D15), then align each pool's provisions and materialize each pool once, with the reason naming the ladder. -- [ ] 5.3 `CreatePlanLadderTier` only parses, calls and renders: its product read goes; a lock wait or deadlock at any step answers the new refusal (#216); every failure is logged with its step. Ride-along: `refuseTierAdd` answers a `*TierAddCollisionError` under the product field with copy 43 (a held rung) or copy 48 (the product held more than once), and answers a CF003 from the alignment, which the collision read should have prevented, with "Failed to add tier. Reference: {request id}." and logs the driver text through `logging.FromContext(r.Context())`; it no longer shows the driver text. Ride-along (copy 50): every other "Failed to add tier; see server logs." in `CreatePlanLadderTier` (`internal/server/operator_plan_ladders.go:425`) takes the same reference form. -- [ ] 5.4 Tests: ascending lock order with provisions created in descending pool order; a table test of `HolderLimitRefusal` (below, at and above the limit; with and without more than half default-sourced; the rank-0 variant); a default-sourced provision a `keep` removal left off every ladder counts toward the limit and not toward the second sentence; the product read failing inside answers the generic failure; the collision read refuses, before any alignment and writing nothing, a pool already holding a live position on the ladder (copy 43, naming the organization and the product it holds) and a pool holding two live provisions of the product, an operator grant and a subscription, neither on any ladder (copy 48); a `refuseTierAdd` test that a CF003 alignment error answers "Failed to add tier. Reference: {request id}.", logs the driver text with the same `request_id` and shows none; a test that each other failure path of the add shows the reference and its log line carries that id; `TestTierChangesWaitForAConferral`'s tier add records the ladder's name. -- [ ] 5.5 Goldens: `add-aligns-a-live-provision.json` (the act row; the reason names the ladder; pools lock in ascending order and materialize once), `add-begin-fails.json` (the log line; the toast and the alert read the reference form), `add-collides-with-a-held-rung.json` (refused by the collision read, so no alignment statement runs; the field error naming the organization and the product), `contention.json` (the add answers the new refusal; the tier delete's and the removal's "see server logs" lines in this file take the reference form with tasks 4 and 8), `tier-change-during-conferral/a-tier-add.json` (the act row and the reason). New: `add-above-the-limit.json`, `add-above-the-limit-default-sourced.json`, `add-product-unread.json`, `add-product-held-twice.json` (copy 48; nothing written). -- [ ] 5.6 Docs: the tier add's ascending lock order in `docs/database-locks.md` (rule 3 without the tier add exception; "Where the paths differ" without the #194 entry) and the add's holder limit in `docs/plan-management.md`. +- [x] 5.1 Add `planActHolderCap` to `internal/entitlements/constants.go`, set from task 1's per-holder cost of the add and the removal, with the measurement and the rule in its comment (an add's or a removal's exclusive hold near half a second on the measuring host). Add `HolderLimitRefusal`, the limit and more-than-half rule as a function that reads no row, returning the sentences of copy 15 to 18 or nothing. +- [x] 5.2 Add `CountLiveProductHolders` (organizations holding a live provision of the product, and how many hold it through a live default-sourced position on some ladder) and the reason builder `PlanActReason` (design D19). Rewrite `AddTier` on the runner: read the product inside and refuse one that is not published (no row answers "Product not found."; any other error the generic failure), count the holders and refuse above `planActHolderCap`, lock the provisions' pools in ascending order before any alignment (#194), read the provisions again, record the act (`tier_added`, names read inside), insert the tier, read `ListTierAddCollisions` for the locked pools and refuse the first collision in ascending pool id order with a `*TierAddCollisionError` carrying the organization's and the product's names (design D15), then align each pool's provisions and materialize each pool once, with the reason naming the ladder. +- [x] 5.3 `CreatePlanLadderTier` only parses, calls and renders: its product read goes; a lock wait or deadlock at any step answers the new refusal (#216); every failure is logged with its step. Ride-along: `refuseTierAdd` answers a `*TierAddCollisionError` under the product field with copy 43 (a held rung) or copy 48 (the product held more than once), and answers a CF003 from the alignment, which the collision read should have prevented, with "Failed to add tier. Reference: {request id}." and logs the driver text through `logging.FromContext(r.Context())`; it no longer shows the driver text. Ride-along (copy 50): every other "Failed to add tier; see server logs." in `CreatePlanLadderTier` (`internal/server/operator_plan_ladders.go:425`) takes the same reference form. +- [x] 5.4 Tests: ascending lock order with provisions created in descending pool order; a table test of `HolderLimitRefusal` (below, at and above the limit; with and without more than half default-sourced; the rank-0 variant); a default-sourced provision a `keep` removal left off every ladder counts toward the limit and not toward the second sentence; the product read failing inside answers the generic failure; the collision read refuses, before any alignment and writing nothing, a pool already holding a live position on the ladder (copy 43, naming the organization and the product it holds) and a pool holding two live provisions of the product, an operator grant and a subscription, neither on any ladder (copy 48); a `refuseTierAdd` test that a CF003 alignment error answers "Failed to add tier. Reference: {request id}.", logs the driver text with the same `request_id` and shows none; a test that each other failure path of the add shows the reference and its log line carries that id; `TestTierChangesWaitForAConferral`'s tier add records the ladder's name. +- [x] 5.5 Goldens: `add-aligns-a-live-provision.json` (the act row; the reason names the ladder; pools lock in ascending order and materialize once), `add-begin-fails.json` (the log line; the toast and the alert read the reference form), `add-collides-with-a-held-rung.json` (refused by the collision read, so no alignment statement runs; the field error naming the organization and the product), `contention.json` (the add answers the new refusal; the tier delete's and the removal's "see server logs" lines in this file take the reference form with tasks 4 and 8), `tier-change-during-conferral/a-tier-add.json` (the act row and the reason). New: `add-above-the-limit.json`, `add-above-the-limit-default-sourced.json`, `add-product-unread.json`, `add-product-held-twice.json` (copy 48; nothing written). +- [x] 5.6 Docs: the tier add's ascending lock order in `docs/database-locks.md` (rule 3 without the tier add exception; "Where the paths differ" without the #194 entry) and the add's holder limit in `docs/plan-management.md`. ## 6. The default change on obligations, its drain, the card's lines and Retry