diff --git a/credit_overage_serialization_test.go b/credit_overage_serialization_test.go new file mode 100644 index 0000000..8c9fc64 --- /dev/null +++ b/credit_overage_serialization_test.go @@ -0,0 +1,93 @@ +package rulesengine_test + +import ( + "encoding/json" + "testing" + + "github.com/schematichq/rulesengine" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The overage map leans on a distinction JSON can express but is easy to lose in +// a hand-written client: a key present with a null value (overage on, uncapped) +// against the key being absent (overage off). Collapsing those turns "gate at +// zero" into "never gate", which is why it is asserted rather than assumed. +func TestCreditOverageSerialization(t *testing.T) { + const creditID = "test-credit-id" + + t.Run("decodes the three states distinctly", func(t *testing.T) { + for _, tc := range []struct { + name string + raw string + overageOn bool + uncapped bool + cap float64 + }{ + {name: "field absent", raw: `{}`}, + {name: "field null", raw: `{"credit_overage":null}`}, + {name: "map empty", raw: `{"credit_overage":{}}`}, + {name: "value null is on and uncapped", raw: `{"credit_overage":{"test-credit-id":null}}`, overageOn: true, uncapped: true}, + {name: "value set is on and capped", raw: `{"credit_overage":{"test-credit-id":100}}`, overageOn: true, cap: 100}, + } { + t.Run(tc.name, func(t *testing.T) { + var company rulesengine.Company + require.NoError(t, json.Unmarshal([]byte(tc.raw), &company)) + + overageCap, overageOn := company.CreditOverage[creditID] + require.Equal(t, tc.overageOn, overageOn, "presence in the map is the opt-in") + + if !tc.overageOn { + return + } + + if tc.uncapped { + assert.Nil(t, overageCap, "a null value must stay nil, not become zero") + return + } + + require.NotNil(t, overageCap) + assert.Equal(t, tc.cap, *overageCap) + }) + } + }) + + // Round-tripping must not quietly promote nil to 0. A zero cap denies every + // draw past the balance, which is the opposite of what nil means. + t.Run("round-trips without collapsing nil to zero", func(t *testing.T) { + limit := float64(100) + for name, overage := range map[string]map[string]*float64{ + "uncapped": {creditID: nil}, + "capped": {creditID: &limit}, + "empty": {}, + "nil map": nil, + } { + t.Run(name, func(t *testing.T) { + encoded, err := json.Marshal(rulesengine.Company{CreditOverage: overage}) + require.NoError(t, err) + + var decoded rulesengine.Company + require.NoError(t, json.Unmarshal(encoded, &decoded)) + + assert.Len(t, decoded.CreditOverage, len(overage)) + + want, wantOn := overage[creditID] + got, gotOn := decoded.CreditOverage[creditID] + require.Equal(t, wantOn, gotOn) + + if !wantOn { + return + } + + if want == nil { + assert.Nil(t, got, "nil cap must not decode as zero") + return + } + + require.NotNil(t, got) + assert.Equal(t, *want, *got) + }) + } + }) +} diff --git a/credit_overage_test.go b/credit_overage_test.go index 21a633f..31591db 100644 --- a/credit_overage_test.go +++ b/credit_overage_test.go @@ -22,15 +22,26 @@ func TestCreditOverage(t *testing.T) { const creditID = "test-credit-id" + // overage: nil leaves the credit out of the map entirely (off); non-nil puts + // it in, with the pointed-at value as the cap — nil cap meaning uncapped. companyWith := func(balance float64, overage *bool) *rulesengine.Company { company := createTestCompany() company.CreditBalances = map[string]float64{creditID: balance} - if overage != nil { - company.CreditOverageEnabled = map[string]bool{creditID: *overage} + if overage != nil && *overage { + company.CreditOverage = map[string]*float64{creditID: nil} + } else if overage != nil { + company.CreditOverage = map[string]*float64{} } return company } + companyWithCap := func(balance float64, limit float64) *rulesengine.Company { + enabled := true + company := companyWith(balance, &enabled) + company.CreditOverage = map[string]*float64{creditID: &limit} + return company + } + creditRule := func() *rulesengine.Rule { rule := createTestRule() condition := createTestCondition(rulesengine.ConditionTypeCredit) @@ -116,4 +127,80 @@ func TestCreditOverage(t *testing.T) { require.NoError(t, err) assert.True(t, result.Value) }) + + // SCHX-582 cap: the floor moves from zero to -cap rather than disappearing. + t.Run("allows while inside the cap", func(t *testing.T) { + assert.True(t, matches(t, companyWithCap(-40, 100))) + }) + + t.Run("denies once the cap is spent", func(t *testing.T) { + assert.False(t, matches(t, companyWithCap(-100, 100))) + }) + + t.Run("denies past the cap", func(t *testing.T) { + assert.False(t, matches(t, companyWithCap(-140, 100))) + }) + + // A cap on one credit must not bound a different one. + t.Run("cap does not leak across credits", func(t *testing.T) { + company := companyWithCap(-140, 100) + otherCap := 100.0 + company.CreditOverage = map[string]*float64{ + creditID: nil, + "other-credit": &otherCap, + } + assert.True(t, matches(t, company)) + }) + + // Absent cap keeps the uncapped behaviour that shipped first. + t.Run("no cap means uncapped", func(t *testing.T) { + enabled := true + assert.True(t, matches(t, companyWith(-10_000, &enabled))) + }) + + // The cap has to bound the balance *after* this call, not before it. + // Checking the balance alone enforces the cap only to within one call's + // cost: 5 credits short of the cap, a call costing 50 would pass and land 45 + // past it. + matchesWithCost := func(t *testing.T, company *rulesengine.Company, cost float64) bool { + t.Helper() + + flag := createTestFlag() + // createTestFlag randomizes DefaultValue and CheckFlag falls back to it + // when no rule matches, so a true default would pass without the rule. + flag.DefaultValue = false + flag.Rules = []*rulesengine.Rule{creditRule()} + + result, err := rulesengine.CheckFlag( + ctx, company, nil, flag, rulesengine.WithCreditCost(creditID, cost), + ) + require.NoError(t, err) + return result.Value + } + + t.Run("denies a cost that would carry past the cap", func(t *testing.T) { + assert.False(t, matchesWithCost(t, companyWithCap(-95, 100), 50)) + }) + + t.Run("allows a cost that fits inside the cap", func(t *testing.T) { + assert.True(t, matchesWithCost(t, companyWithCap(-40, 100), 50)) + }) + + // Exactly reaching the cap is allowed; the next credit past it is not. + t.Run("allows a cost landing exactly on the cap", func(t *testing.T) { + assert.True(t, matchesWithCost(t, companyWithCap(-50, 100), 50)) + }) + + // Uncapped ignores the cost entirely — there is no floor to measure against. + t.Run("uncapped allows any cost", func(t *testing.T) { + enabled := true + assert.True(t, matchesWithCost(t, companyWith(-10_000, &enabled), 5_000)) + }) + + // Without overage the allowance is zero, so this stays the balance >= cost + // check that has always applied. + t.Run("without overage a cost still gates on balance", func(t *testing.T) { + assert.False(t, matchesWithCost(t, companyWith(10, nil), 50)) + assert.True(t, matchesWithCost(t, companyWith(60, nil), 50)) + }) } diff --git a/models.go b/models.go index 2ec50eb..f85e9c7 100644 --- a/models.go +++ b/models.go @@ -172,21 +172,30 @@ type Company struct { BasePlanID *string `json:"base_plan_id"` BillingProductIDs JSONSlice[string] `json:"billing_product_ids"` CreditBalances map[string]float64 `json:"credit_balances"` - // CreditOverageEnabled is the per-credit overage opt-in (SCHX-582), keyed by - // billing credit ID — the same key CreditBalances uses. When true for a - // credit, consumption continues past a zero balance and accrues against an - // overage rate rather than being denied, so the balance stops gating the - // check. Absent or false is the historical behaviour, so a caller that does - // not send the field keeps hard-stopping at zero. - CreditOverageEnabled map[string]bool `json:"credit_overage_enabled"` - Entitlements JSONSlice[*FeatureEntitlement] `json:"entitlements,omitempty"` - Keys map[string]string `json:"keys"` - Metrics CompanyMetricCollection `json:"metrics"` - PlanIDs JSONSlice[string] `json:"plan_ids"` - PlanVersionIDs JSONSlice[string] `json:"plan_version_ids"` - Rules JSONSlice[*Rule] `json:"rules"` - Subscription *Subscription `json:"subscription"` - Traits JSONSlice[*Trait] `json:"traits"` + // CreditOverage is per-credit overage config (SCHX-582), keyed by billing + // credit ID — the same key CreditBalances uses. + // + // Three states, which is why the value is nullable rather than this being a + // map of caps or a map of bools: + // + // key absent -> overage off; an exhausted balance denies, as it always has + // value nil -> overage on, uncapped; the balance stops gating the check + // value set -> overage on, capped; the balance may run down to -cap + // + // One map rather than an enabled-set plus a cap-map because those two can + // disagree — a cap on a credit that is not enabled, or vice versa — and + // neither state means anything. + // + // A caller that does not send the field keeps hard-stopping at zero. + CreditOverage map[string]*float64 `json:"credit_overage"` + Entitlements JSONSlice[*FeatureEntitlement] `json:"entitlements,omitempty"` + Keys map[string]string `json:"keys"` + Metrics CompanyMetricCollection `json:"metrics"` + PlanIDs JSONSlice[string] `json:"plan_ids"` + PlanVersionIDs JSONSlice[string] `json:"plan_version_ids"` + Rules JSONSlice[*Rule] `json:"rules"` + Subscription *Subscription `json:"subscription"` + Traits JSONSlice[*Trait] `json:"traits"` mu sync.Mutex `json:"-"` // mutex for thread safety } diff --git a/rulecheck.go b/rulecheck.go index 5363a59..a5f3f41 100644 --- a/rulecheck.go +++ b/rulecheck.go @@ -147,42 +147,45 @@ func (s *RuleCheckService) checkCreditBalanceCondition(ctx context.Context, scop } } - // SCHX-582: with overage enabled the balance no longer gates the check — - // consumption continues past zero and accrues at the configured rate, so - // every branch below (all of which compare against the balance) would ask - // the wrong question. There is no cap in the current design, so once overage - // is on there is nothing further to compare against. - // - // Mirrors check_credit_balance_condition in rulesengine-rust; the two must - // agree (see SCHY-515) until the Go engine is retired. - if scope.Company.CreditOverageEnabled[*condition.CreditID] { - return true, nil - } - - // Precedence on credit-balance conditions, most specific first. No - // options supplied falls through to the legacy single-unit check. - // 1. creditCost[credit_id]: caller-supplied per-call cost in credits; - // gate on balance >= cost. + // What this call costs, most specific source first. No options supplied + // falls through to the legacy single-unit check. + // 1. creditCost[credit_id]: caller-supplied per-call cost in credits. // 2. eventUsage, when its event_subtype matches the condition's: - // simulated quantity for this specific event; gate on - // balance >= quantity × consumption_rate. - // 3. usage: generic quantity (no event disambiguation); gate on - // balance >= quantity × consumption_rate. - // 4. Legacy: balance >= consumption_rate (single unit). - if cost, ok := scope.creditCost[*condition.CreditID]; ok { - return creditBalance >= cost, nil - } - - if eu := scope.eventUsage; eu != nil && condition.EventSubtype != nil && + // simulated quantity for this specific event. + // 3. usage: generic quantity (no event disambiguation). + // 4. Legacy: a single unit at the consumption rate. + cost := consumptionRate + if creditCost, ok := scope.creditCost[*condition.CreditID]; ok { + cost = creditCost + } else if eu := scope.eventUsage; eu != nil && condition.EventSubtype != nil && eu.eventSubtype == *condition.EventSubtype && eu.quantity > 0 { - return creditBalance >= float64(eu.quantity)*consumptionRate, nil + cost = float64(eu.quantity) * consumptionRate + } else if scope.usage != nil && *scope.usage > 0 { + cost = float64(*scope.usage) * consumptionRate } - if scope.usage != nil && *scope.usage > 0 { - return creditBalance >= float64(*scope.usage)*consumptionRate, nil + // SCHX-582: overage moves the floor the cost is measured against, rather + // than skipping the comparison. Checking the balance alone would enforce the + // cap only to within one call: a company at -95 against a cap of 100 would + // pass a call costing 50 and land at -145. + // + // An uncapped grant returns before the comparison — there is no floor to + // measure against, and the company is free to run the balance as negative as + // it likes. With no overage the allowance is zero, which reduces this to the + // balance >= cost check that has always applied. + // + // Mirrors check_credit_balance_condition in rulesengine-rust; the two must + // agree (see SCHY-515) until the Go engine is retired. + var overageAllowance float64 + if overageCap, overageOn := scope.Company.CreditOverage[*condition.CreditID]; overageOn { + if overageCap == nil { + return true, nil + } + + overageAllowance = *overageCap } - return creditBalance >= consumptionRate, nil + return creditBalance+overageAllowance >= cost, nil } func (s *RuleCheckService) checkBillingProductCondition(ctx context.Context, company *Company, condition *Condition) (bool, error) {