From 62404033959800cb82828b65ccc6dfc7b9e3f839 Mon Sep 17 00:00:00 2001 From: Ben Papillon Date: Fri, 11 Sep 2026 10:09:27 -0700 Subject: [PATCH 1/2] nest credit postpaid config per credit A nullable number as the map value does not survive SDKs that strip null map values or generators that type the value as non-nullable, so an unbounded grant read as off. Presence of the key is now the opt-in and the overdraft limit is an optional member of the value. --- credit_entitlement_test.go | 8 +- credit_postpaid_serialization_test.go | 108 ++++++++++++-------------- credit_postpaid_test.go | 22 +++--- models.go | 48 ++++++------ rulecheck.go | 6 +- 5 files changed, 94 insertions(+), 98 deletions(-) diff --git a/credit_entitlement_test.go b/credit_entitlement_test.go index d33372e..b71471b 100644 --- a/credit_entitlement_test.go +++ b/credit_entitlement_test.go @@ -105,7 +105,7 @@ func TestCreditEntitlementExceededRule(t *testing.T) { t.Run("exceeded rule does not fire with unbounded postpaid", func(t *testing.T) { flag, entitled, _ := entitlementFlag() company := companyWith(0) - company.CreditPostpaidLimit = map[string]*float64{creditID: nil} + company.CreditPostpaid = map[string]rulesengine.CreditPostpaidConfig{creditID: {}} result, err := rulesengine.CheckFlag(ctx, company, nil, flag) @@ -121,15 +121,17 @@ func TestCreditEntitlementExceededRule(t *testing.T) { flag, entitled, exceeded := entitlementFlag() overdraftLimit := 10.0 + postpaid := map[string]rulesengine.CreditPostpaidConfig{creditID: {OverdraftLimit: &overdraftLimit}} + company := companyWith(-5) - company.CreditPostpaidLimit = map[string]*float64{creditID: &overdraftLimit} + company.CreditPostpaid = postpaid 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.CreditPostpaidLimit = map[string]*float64{creditID: &overdraftLimit} + company.CreditPostpaid = postpaid 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_postpaid_serialization_test.go b/credit_postpaid_serialization_test.go index 4c07263..56872fe 100644 --- a/credit_postpaid_serialization_test.go +++ b/credit_postpaid_serialization_test.go @@ -10,83 +10,65 @@ import ( "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) { +// Presence of a credit's key is the postpaid opt-in, and null never means +// anything on its own: a null map is an empty map, a null config is {}, and a +// null limit is no limit. An earlier shape used a nullable number as the map +// value, and SDKs that strip null map values read "unbounded" as "off". +// +// Mirrors credit_postpaid_serialization_test.rs in rulesengine-rust. +func TestCreditPostpaidSerialization(t *testing.T) { const creditID = "test-credit-id" - t.Run("decodes the three states distinctly", func(t *testing.T) { + t.Run("decodes each encoding", func(t *testing.T) { + hundred := 100.0 for _, tc := range []struct { name string raw string postpaidOn bool - unbounded bool - limit float64 + 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}, + {name: "field null", raw: `{"credit_postpaid":null}`}, + {name: "credit absent", raw: `{"credit_postpaid":{}}`}, + {name: "empty config is on and unbounded", raw: `{"credit_postpaid":{"test-credit-id":{}}}`, postpaidOn: true}, + {name: "null limit is on and unbounded", raw: `{"credit_postpaid":{"test-credit-id":{"overdraft_limit":null}}}`, postpaidOn: true}, + {name: "null config is on and unbounded", raw: `{"credit_postpaid":{"test-credit-id":null}}`, postpaidOn: true}, + {name: "numeric limit is on and capped", raw: `{"credit_postpaid":{"test-credit-id":{"overdraft_limit":100}}}`, postpaidOn: true, limit: &hundred}, } { 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] + postpaid, postpaidOn := company.CreditPostpaid[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) + assert.Equal(t, tc.limit, postpaid.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}, + // Round-tripping must not turn "no limit" into a zero limit, which would + // deny every draw past the balance. + t.Run("round-trips", func(t *testing.T) { + limit := 100.0 + for name, postpaid := range map[string]map[string]rulesengine.CreditPostpaidConfig{ + "unbounded": {creditID: {}}, + "capped": {creditID: {OverdraftLimit: &limit}}, "empty": {}, "nil map": nil, } { t.Run(name, func(t *testing.T) { - encoded, err := json.Marshal(rulesengine.Company{CreditPostpaidLimit: postpaid}) + encoded, err := json.Marshal(rulesengine.Company{CreditPostpaid: postpaid}) require.NoError(t, err) var decoded rulesengine.Company require.NoError(t, json.Unmarshal(encoded, &decoded)) - assert.Len(t, decoded.CreditPostpaidLimit, len(postpaid)) + assert.Len(t, decoded.CreditPostpaid, len(postpaid)) want, wantOn := postpaid[creditID] - got, gotOn := decoded.CreditPostpaidLimit[creditID] + got, gotOn := decoded.CreditPostpaid[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) + assert.Equal(t, want, got) }) } }) @@ -94,30 +76,36 @@ func TestCreditPostpaidLimitSerialization(t *testing.T) { // 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{ +// strictly; if an empty map serialised as "credit_postpaid":{} the key would be +// an unknown property to that client. A company with no postpaid grant is the +// common case, so this is the shape most payloads take. +func TestCreditPostpaidWireShape(t *testing.T) { + for name, postpaid := range map[string]map[string]rulesengine.CreditPostpaidConfig{ "nil map": nil, "empty map": {}, } { t.Run(name, func(t *testing.T) { - encoded, err := json.Marshal(rulesengine.Company{CreditPostpaidLimit: postpaid}) + encoded, err := json.Marshal(rulesengine.Company{CreditPostpaid: postpaid}) require.NoError(t, err) - assert.NotContains(t, string(encoded), "credit_postpaid_limit", + assert.NotContains(t, string(encoded), "credit_postpaid", "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) + t.Run("an unbounded credit sends an empty config", func(t *testing.T) { + encoded, err := json.Marshal(rulesengine.Company{ + CreditPostpaid: map[string]rulesengine.CreditPostpaidConfig{"c": {}}, + }) + require.NoError(t, err) + assert.Contains(t, string(encoded), `"credit_postpaid":{"c":{}}`) + }) + + t.Run("a capped credit sends its limit", func(t *testing.T) { + limit := 100.0 encoded, err := json.Marshal(rulesengine.Company{ - CreditPostpaidLimit: map[string]*float64{"c": &limit}, + CreditPostpaid: map[string]rulesengine.CreditPostpaidConfig{"c": {OverdraftLimit: &limit}}, }) require.NoError(t, err) - assert.Contains(t, string(encoded), `"credit_postpaid_limit":{"c":100}`) + assert.Contains(t, string(encoded), `"credit_postpaid":{"c":{"overdraft_limit":100}}`) }) } diff --git a/credit_postpaid_test.go b/credit_postpaid_test.go index f89c2a1..9ebc9da 100644 --- a/credit_postpaid_test.go +++ b/credit_postpaid_test.go @@ -15,22 +15,22 @@ import ( // continues past a zero balance and accrues at a configured rate, so the balance // stops gating the check. // -// These mirror credit_postpaid_limit_tests in rulesengine-rust; the two engines must +// These mirror credit_postpaid_tests in rulesengine-rust; the two engines must // agree (SCHY-515) for as long as both are in use. -func TestCreditPostpaidLimit(t *testing.T) { +func TestCreditPostpaid(t *testing.T) { ctx := context.Background() const creditID = "test-credit-id" - // 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. + // postpaid: nil leaves the credit out of the map entirely (off); true puts it + // in with no cap (unbounded); false is an explicit empty map (off). companyWith := func(balance float64, postpaid *bool) *rulesengine.Company { company := createTestCompany() company.CreditBalances = map[string]float64{creditID: balance} if postpaid != nil && *postpaid { - company.CreditPostpaidLimit = map[string]*float64{creditID: nil} + company.CreditPostpaid = map[string]rulesengine.CreditPostpaidConfig{creditID: {}} } else if postpaid != nil { - company.CreditPostpaidLimit = map[string]*float64{} + company.CreditPostpaid = map[string]rulesengine.CreditPostpaidConfig{} } return company } @@ -38,7 +38,9 @@ func TestCreditPostpaidLimit(t *testing.T) { companyWithCap := func(balance float64, limit float64) *rulesengine.Company { enabled := true company := companyWith(balance, &enabled) - company.CreditPostpaidLimit = map[string]*float64{creditID: &limit} + company.CreditPostpaid = map[string]rulesengine.CreditPostpaidConfig{ + creditID: {OverdraftLimit: &limit}, + } return company } @@ -145,9 +147,9 @@ func TestCreditPostpaidLimit(t *testing.T) { t.Run("cap does not leak across credits", func(t *testing.T) { company := companyWithCap(-140, 100) otherCap := 100.0 - company.CreditPostpaidLimit = map[string]*float64{ - creditID: nil, - "other-credit": &otherCap, + company.CreditPostpaid = map[string]rulesengine.CreditPostpaidConfig{ + creditID: {}, + "other-credit": {OverdraftLimit: &otherCap}, } assert.True(t, matches(t, company)) }) diff --git a/models.go b/models.go index 4940986..b938dfc 100644 --- a/models.go +++ b/models.go @@ -172,39 +172,43 @@ type Company struct { BasePlanID *string `json:"base_plan_id"` BillingProductIDs JSONSlice[string] `json:"billing_product_ids"` CreditBalances map[string]float64 `json:"credit_balances"` - // 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. + // CreditPostpaid 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. // - // Three states, which is why the value is nullable rather than this being a - // map of limits or a map of bools: + // key absent -> postpaid off; an exhausted balance denies + // key present, no limit -> postpaid on, unbounded + // key present, limit set -> postpaid on; the balance may run down to -limit // - // 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 limit-map because those two can - // disagree — a limit on a credit that is not enabled, or vice versa — and - // neither state means anything. + // Presence of the key is the opt-in, and null never carries a meaning of its + // own: a null map is an empty map, a null config is {}, and a null limit is + // no limit. An earlier shape put the limit directly in the map as a nullable + // number, and SDKs that strip null map values turned "unbounded" into "off". // // 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"` + CreditPostpaid map[string]CreditPostpaidConfig `json:"credit_postpaid,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 } +// CreditPostpaidConfig is one credit's entry in Company.CreditPostpaid. +type CreditPostpaidConfig struct { + // OverdraftLimit is how far below zero the balance may run. Nil means no + // limit. + OverdraftLimit *float64 `json:"overdraft_limit,omitempty"` +} + func (c *Company) getTraitByDefinitionID(traitDefinitionID string) *Trait { if c == nil { return nil diff --git a/rulecheck.go b/rulecheck.go index fb640ef..c7ba27b 100644 --- a/rulecheck.go +++ b/rulecheck.go @@ -196,12 +196,12 @@ func (s *RuleCheckService) creditBalanceCoversCost(scope *CheckScope, condition // Mirrors check_credit_balance_condition in rulesengine-rust; the two must // agree (see SCHY-515) until the Go engine is retired. var overdraftAllowance float64 - if overdraftLimit, postpaidOn := scope.Company.CreditPostpaidLimit[*condition.CreditID]; postpaidOn { - if overdraftLimit == nil { + if postpaid, postpaidOn := scope.Company.CreditPostpaid[*condition.CreditID]; postpaidOn { + if postpaid.OverdraftLimit == nil { return true } - overdraftAllowance = *overdraftLimit + overdraftAllowance = *postpaid.OverdraftLimit } return creditBalance+overdraftAllowance >= cost From 7b3888b2d1d71d6db3c8e2e4b0640406eab5557f Mon Sep 17 00:00:00 2001 From: Ben Papillon Date: Fri, 11 Sep 2026 10:32:14 -0700 Subject: [PATCH 2/2] drop null credit entries from credit_postpaid SDKs that strip null map values already read {"cred_1": null} as off, so the engine drops the entry too and a malformed payload fails closed. --- credit_postpaid_serialization_test.go | 9 +++-- models.go | 56 ++++++++++++++++++++------- 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/credit_postpaid_serialization_test.go b/credit_postpaid_serialization_test.go index 56872fe..e499a1b 100644 --- a/credit_postpaid_serialization_test.go +++ b/credit_postpaid_serialization_test.go @@ -10,9 +10,9 @@ import ( "github.com/stretchr/testify/require" ) -// Presence of a credit's key is the postpaid opt-in, and null never means -// anything on its own: a null map is an empty map, a null config is {}, and a -// null limit is no limit. An earlier shape used a nullable number as the map +// Presence of a credit's key is the postpaid opt-in, and null always reads as +// absent: a null map is an empty map, a null config drops the credit (off), and +// a null limit is no limit. An earlier shape used a nullable number as the map // value, and SDKs that strip null map values read "unbounded" as "off". // // Mirrors credit_postpaid_serialization_test.rs in rulesengine-rust. @@ -32,7 +32,8 @@ func TestCreditPostpaidSerialization(t *testing.T) { {name: "credit absent", raw: `{"credit_postpaid":{}}`}, {name: "empty config is on and unbounded", raw: `{"credit_postpaid":{"test-credit-id":{}}}`, postpaidOn: true}, {name: "null limit is on and unbounded", raw: `{"credit_postpaid":{"test-credit-id":{"overdraft_limit":null}}}`, postpaidOn: true}, - {name: "null config is on and unbounded", raw: `{"credit_postpaid":{"test-credit-id":null}}`, postpaidOn: true}, + {name: "null config is off", raw: `{"credit_postpaid":{"test-credit-id":null}}`}, + {name: "null config drops only its own credit", raw: `{"credit_postpaid":{"test-credit-id":{},"other-credit-id":null}}`, postpaidOn: true}, {name: "numeric limit is on and capped", raw: `{"credit_postpaid":{"test-credit-id":{"overdraft_limit":100}}}`, postpaidOn: true, limit: &hundred}, } { t.Run(tc.name, func(t *testing.T) { diff --git a/models.go b/models.go index b938dfc..fe3f025 100644 --- a/models.go +++ b/models.go @@ -180,24 +180,25 @@ type Company struct { // key present, no limit -> postpaid on, unbounded // key present, limit set -> postpaid on; the balance may run down to -limit // - // Presence of the key is the opt-in, and null never carries a meaning of its - // own: a null map is an empty map, a null config is {}, and a null limit is - // no limit. An earlier shape put the limit directly in the map as a nullable - // number, and SDKs that strip null map values turned "unbounded" into "off". + // Presence of the key is the opt-in, and null always reads as absent: a null + // map is an empty map, a null config drops the credit (postpaid off), and a + // null limit is no limit. An earlier shape put the limit directly in the map + // as a nullable number, and SDKs that strip null map values turned + // "unbounded" into "off". // // 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. - CreditPostpaid map[string]CreditPostpaidConfig `json:"credit_postpaid,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"` + CreditPostpaid CreditPostpaidMap `json:"credit_postpaid,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 } @@ -209,6 +210,35 @@ type CreditPostpaidConfig struct { OverdraftLimit *float64 `json:"overdraft_limit,omitempty"` } +// CreditPostpaidMap is Company.CreditPostpaid, keyed by billing credit ID. +type CreditPostpaidMap map[string]CreditPostpaidConfig + +// UnmarshalJSON drops every credit whose value is null. Plain decoding would +// keep the key with a zero config, which reads as postpaid on with no limit; +// SDKs that strip null map values before calling the engine read the same +// payload as off, and a malformed entry should fail closed. +func (m *CreditPostpaidMap) UnmarshalJSON(data []byte) error { + var raw map[string]*CreditPostpaidConfig + if err := json.Unmarshal(data, &raw); err != nil { + return err + } + + if raw == nil { + *m = nil + return nil + } + + decoded := make(CreditPostpaidMap, len(raw)) + for creditID, postpaid := range raw { + if postpaid != nil { + decoded[creditID] = *postpaid + } + } + + *m = decoded + return nil +} + func (c *Company) getTraitByDefinitionID(traitDefinitionID string) *Trait { if c == nil { return nil