From d17650de071e3405074a7f414600fb524545a53b Mon Sep 17 00:00:00 2001 From: Ben Papillon Date: Tue, 8 Sep 2026 17:56:59 -0700 Subject: [PATCH] rename CreditOverage to CreditPostpaidLimit "Overage" already means something else in the platform: a soft limit of free units followed by a rate, on an entitlement. This field is consumption past a zero credit balance, where there is no free allowance because the allowance was the grant. The API side renamed to postpaid for the arrangement and overdraft for the negative balance, so the field is the postpaid grant's overdraft limit. The wire name gains omitempty. Absent has to be a legal payload: a client generated from a spec predating this field decodes strictly and treats every property it knows as required, so an empty map serialised as "credit_postpaid_limit":{} is an unknown property to it, and a required field missing from a payload fails validation outright. Omitting the key when the map is empty keeps the payload legal either way, and no postpaid grant is the common case. A new test asserts it, and fails without the tag. Nothing needs the old spelling accepted on decode. Go ignores unknown fields, so a payload still carrying credit_overage leaves the map empty and the company hard-stops at zero, which is the pre-feature behavior. The feature is unreleased, so there is no arrangement in the field to preserve. --- credit_entitlement_test.go | 18 +-- credit_overage_serialization_test.go | 93 ------------- credit_postpaid_serialization_test.go | 123 ++++++++++++++++++ ...overage_test.go => credit_postpaid_test.go | 48 +++---- models.go | 41 +++--- rulecheck.go | 24 ++-- 6 files changed, 191 insertions(+), 156 deletions(-) delete mode 100644 credit_overage_serialization_test.go create mode 100644 credit_postpaid_serialization_test.go rename credit_overage_test.go => credit_postpaid_test.go (78%) diff --git a/credit_entitlement_test.go b/credit_entitlement_test.go index 60c5298..d33372e 100644 --- a/credit_entitlement_test.go +++ b/credit_entitlement_test.go @@ -100,12 +100,12 @@ func TestCreditEntitlementExceededRule(t *testing.T) { assert.Equal(t, &exceeded.ID, result.RuleID) }) - // Uncapped overage removes the balance gate, so the exceeded rule must stay quiet + // Unbounded postpaid removes the balance gate, so the exceeded rule must stay quiet // even at zero. - t.Run("exceeded rule does not fire with uncapped overage", func(t *testing.T) { + t.Run("exceeded rule does not fire with unbounded postpaid", func(t *testing.T) { flag, entitled, _ := entitlementFlag() company := companyWith(0) - company.CreditOverage = map[string]*float64{creditID: nil} + company.CreditPostpaidLimit = map[string]*float64{creditID: nil} result, err := rulesengine.CheckFlag(ctx, company, nil, flag) @@ -114,22 +114,22 @@ func TestCreditEntitlementExceededRule(t *testing.T) { assert.Equal(t, &entitled.ID, result.RuleID) }) - // A capped overage moves the floor to -cap rather than removing it, so the + // A capped postpaid moves the floor to -cap rather than removing it, so the // exceeded rule fires once the cap is spent, the same as a drained balance - // with overage off. - t.Run("exceeded rule fires once a capped overage is spent", func(t *testing.T) { + // with postpaid off. + t.Run("exceeded rule fires once a capped postpaid is spent", func(t *testing.T) { flag, entitled, exceeded := entitlementFlag() - overageCap := 10.0 + overdraftLimit := 10.0 company := companyWith(-5) - company.CreditOverage = map[string]*float64{creditID: &overageCap} + company.CreditPostpaidLimit = map[string]*float64{creditID: &overdraftLimit} result, err := rulesengine.CheckFlag(ctx, company, nil, flag) require.NoError(t, err) assert.True(t, result.Value, "still inside the cap") assert.Equal(t, &entitled.ID, result.RuleID) company = companyWith(-10) - company.CreditOverage = map[string]*float64{creditID: &overageCap} + company.CreditPostpaidLimit = map[string]*float64{creditID: &overdraftLimit} result, err = rulesengine.CheckFlag(ctx, company, nil, flag) require.NoError(t, err) assert.False(t, result.Value, "the cap is spent") diff --git a/credit_overage_serialization_test.go b/credit_overage_serialization_test.go deleted file mode 100644 index 8c9fc64..0000000 --- a/credit_overage_serialization_test.go +++ /dev/null @@ -1,93 +0,0 @@ -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_postpaid_serialization_test.go b/credit_postpaid_serialization_test.go new file mode 100644 index 0000000..4c07263 --- /dev/null +++ b/credit_postpaid_serialization_test.go @@ -0,0 +1,123 @@ +package rulesengine_test + +import ( + "encoding/json" + "testing" + + "github.com/schematichq/rulesengine" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The postpaid 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 (postpaid on, unbounded) +// against the key being absent (postpaid off). Collapsing those turns "gate at +// zero" into "never gate", which is why it is asserted rather than assumed. +func TestCreditPostpaidLimitSerialization(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 + postpaidOn bool + unbounded bool + limit float64 + }{ + {name: "field absent", raw: `{}`}, + {name: "field null", raw: `{"credit_postpaid_limit":null}`}, + {name: "map empty", raw: `{"credit_postpaid_limit":{}}`}, + {name: "value null is on and unbounded", raw: `{"credit_postpaid_limit":{"test-credit-id":null}}`, postpaidOn: true, unbounded: true}, + {name: "value set is on and capped", raw: `{"credit_postpaid_limit":{"test-credit-id":100}}`, postpaidOn: true, limit: 100}, + } { + t.Run(tc.name, func(t *testing.T) { + var company rulesengine.Company + require.NoError(t, json.Unmarshal([]byte(tc.raw), &company)) + + overdraftLimit, postpaidOn := company.CreditPostpaidLimit[creditID] + require.Equal(t, tc.postpaidOn, postpaidOn, "presence in the map is the opt-in") + + if !tc.postpaidOn { + return + } + + if tc.unbounded { + assert.Nil(t, overdraftLimit, "a null value must stay nil, not become zero") + return + } + + require.NotNil(t, overdraftLimit) + assert.Equal(t, tc.limit, *overdraftLimit) + }) + } + }) + + // 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, postpaid := range map[string]map[string]*float64{ + "unbounded": {creditID: nil}, + "capped": {creditID: &limit}, + "empty": {}, + "nil map": nil, + } { + t.Run(name, func(t *testing.T) { + encoded, err := json.Marshal(rulesengine.Company{CreditPostpaidLimit: postpaid}) + require.NoError(t, err) + + var decoded rulesengine.Company + require.NoError(t, json.Unmarshal(encoded, &decoded)) + + assert.Len(t, decoded.CreditPostpaidLimit, len(postpaid)) + + want, wantOn := postpaid[creditID] + got, gotOn := decoded.CreditPostpaidLimit[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) + }) + } + }) +} + +// omitempty is load-bearing rather than cosmetic. A client generated from a spec +// that predates this field marks every property it knows as required and decodes +// strictly; if an empty map serialised as "credit_postpaid_limit":{} the key +// would be an unknown property to that client, and if the field were required a +// payload without it would fail validation. Omitting it when empty keeps the +// payload legal for both, and a company with no postpaid grant is the common +// case, so this is the shape most payloads take. +func TestCreditPostpaidLimitOmittedWhenEmpty(t *testing.T) { + for name, postpaid := range map[string]map[string]*float64{ + "nil map": nil, + "empty map": {}, + } { + t.Run(name, func(t *testing.T) { + encoded, err := json.Marshal(rulesengine.Company{CreditPostpaidLimit: postpaid}) + require.NoError(t, err) + assert.NotContains(t, string(encoded), "credit_postpaid_limit", + "an empty map must not put the key on the wire") + }) + } + + t.Run("a populated map is still sent", func(t *testing.T) { + limit := float64(100) + encoded, err := json.Marshal(rulesengine.Company{ + CreditPostpaidLimit: map[string]*float64{"c": &limit}, + }) + require.NoError(t, err) + assert.Contains(t, string(encoded), `"credit_postpaid_limit":{"c":100}`) + }) +} diff --git a/credit_overage_test.go b/credit_postpaid_test.go similarity index 78% rename from credit_overage_test.go rename to credit_postpaid_test.go index 31591db..f89c2a1 100644 --- a/credit_overage_test.go +++ b/credit_postpaid_test.go @@ -11,26 +11,26 @@ import ( "github.com/stretchr/testify/require" ) -// SCHX-582: credit overage. With overage enabled on a credit, consumption +// SCHX-582: credit postpaid. With postpaid enabled on a credit, consumption // continues past a zero balance and accrues at a configured rate, so the balance // stops gating the check. // -// These mirror credit_overage_tests in rulesengine-rust; the two engines must +// These mirror credit_postpaid_limit_tests in rulesengine-rust; the two engines must // agree (SCHY-515) for as long as both are in use. -func TestCreditOverage(t *testing.T) { +func TestCreditPostpaidLimit(t *testing.T) { ctx := context.Background() 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 { + // postpaid: 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 unbounded. + companyWith := func(balance float64, postpaid *bool) *rulesengine.Company { company := createTestCompany() company.CreditBalances = map[string]float64{creditID: balance} - if overage != nil && *overage { - company.CreditOverage = map[string]*float64{creditID: nil} - } else if overage != nil { - company.CreditOverage = map[string]*float64{} + if postpaid != nil && *postpaid { + company.CreditPostpaidLimit = map[string]*float64{creditID: nil} + } else if postpaid != nil { + company.CreditPostpaidLimit = map[string]*float64{} } return company } @@ -38,7 +38,7 @@ func TestCreditOverage(t *testing.T) { companyWithCap := func(balance float64, limit float64) *rulesengine.Company { enabled := true company := companyWith(balance, &enabled) - company.CreditOverage = map[string]*float64{creditID: &limit} + company.CreditPostpaidLimit = map[string]*float64{creditID: &limit} return company } @@ -63,7 +63,7 @@ func TestCreditOverage(t *testing.T) { } // The existing hard stop, unchanged when nobody has opted in. - t.Run("denies at zero without overage", func(t *testing.T) { + t.Run("denies at zero without postpaid", func(t *testing.T) { assert.False(t, matches(t, companyWith(0, nil))) }) @@ -72,11 +72,11 @@ func TestCreditOverage(t *testing.T) { }) // The point of the feature. - t.Run("allows past zero with overage enabled", func(t *testing.T) { + t.Run("allows past zero with postpaid enabled", func(t *testing.T) { assert.True(t, matches(t, companyWith(0, null.Nullable(true)))) }) - // A negative balance is legal (SCH-5103); overage is what makes it billable, + // A negative balance is legal (SCH-5103); postpaid is what makes it billable, // and an already-overdrafted company must keep working. t.Run("allows when already negative", func(t *testing.T) { assert.True(t, matches(t, companyWith(-40, null.Nullable(true)))) @@ -88,7 +88,7 @@ func TestCreditOverage(t *testing.T) { assert.False(t, matches(t, companyWith(0, null.Nullable(false)))) }) - // Overage is per credit: enabling it on one must not unblock another. + // Postpaid is per credit: enabling it on one must not unblock another. t.Run("does not leak across credits", func(t *testing.T) { company := companyWith(0, null.Nullable(true)) company.CreditBalances["other-credit-id"] = 0 @@ -104,10 +104,10 @@ func TestCreditOverage(t *testing.T) { Rule: rule, }) require.NoError(t, err) - assert.False(t, result.Match, "the other credit has no overage and no balance") + assert.False(t, result.Match, "the other credit has no postpaid and no balance") }) - // Overage has to beat the more specific branches too. A caller-supplied + // Postpaid has to beat the more specific branches too. A caller-supplied // credit cost would otherwise re-impose the balance gate it short-circuits. t.Run("overrides the credit cost option", func(t *testing.T) { flag := createTestFlag() @@ -145,15 +145,15 @@ func TestCreditOverage(t *testing.T) { t.Run("cap does not leak across credits", func(t *testing.T) { company := companyWithCap(-140, 100) otherCap := 100.0 - company.CreditOverage = map[string]*float64{ + company.CreditPostpaidLimit = 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) { + // Absent cap keeps the unbounded behaviour that shipped first. + t.Run("no cap means unbounded", func(t *testing.T) { enabled := true assert.True(t, matches(t, companyWith(-10_000, &enabled))) }) @@ -191,15 +191,15 @@ func TestCreditOverage(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) { + // Unbounded ignores the cost entirely — there is no floor to measure against. + t.Run("unbounded 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 + // Without postpaid 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) { + t.Run("without postpaid 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 f85e9c7..4940986 100644 --- a/models.go +++ b/models.go @@ -172,30 +172,35 @@ type Company struct { BasePlanID *string `json:"base_plan_id"` BillingProductIDs JSONSlice[string] `json:"billing_product_ids"` CreditBalances map[string]float64 `json:"credit_balances"` - // CreditOverage is per-credit overage config (SCHX-582), keyed by billing - // credit ID — the same key CreditBalances uses. + // CreditPostpaidLimit is per-credit postpaid config, keyed by billing credit + // ID — the same key CreditBalances uses. A postpaid grant lets consumption + // continue past a zero balance; the negative portion is an overdraft, and the + // value here is how far it may run. // // Three states, which is why the value is nullable rather than this being a - // map of caps or a map of bools: + // map of limits 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 + // key absent -> postpaid off; an exhausted balance denies, as it always has + // value nil -> postpaid on, unbounded; the balance stops gating the check + // value set -> postpaid on, bounded; the balance may run down to -limit // - // 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 + // One map rather than an enabled-set plus a limit-map because those two can + // disagree — a limit 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"` + // omitempty on purpose: absent must be a legal payload, so that a client + // generated against a spec that predates this field cannot fail a + // required-property check on it. A caller that does not send it keeps + // hard-stopping at zero. + CreditPostpaidLimit map[string]*float64 `json:"credit_postpaid_limit,omitempty"` + 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 f78d003..fb640ef 100644 --- a/rulecheck.go +++ b/rulecheck.go @@ -183,28 +183,28 @@ func (s *RuleCheckService) creditBalanceCoversCost(scope *CheckScope, condition cost = float64(*scope.usage) * consumptionRate } - // 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. + // A postpaid grant moves the floor the cost is measured against, rather than + // skipping the comparison. Checking the balance alone would enforce the + // overdraft limit only to within one call: a company at -95 against a limit + // 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 + // An unbounded grant returns before the comparison — there is no floor to + // measure against, and the company is free to run the overdraft as deep as it + // likes. With postpaid off 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 { + var overdraftAllowance float64 + if overdraftLimit, postpaidOn := scope.Company.CreditPostpaidLimit[*condition.CreditID]; postpaidOn { + if overdraftLimit == nil { return true } - overageAllowance = *overageCap + overdraftAllowance = *overdraftLimit } - return creditBalance+overageAllowance >= cost + return creditBalance+overdraftAllowance >= cost } func (s *RuleCheckService) checkBillingProductCondition(ctx context.Context, company *Company, condition *Condition) (bool, error) {