Skip to content

Commit 2323906

Browse files
rolosprolosp
committed
feat: don't allow listing or getting unit config equipped plans on v1 api
1 parent e16545b commit 2323906

13 files changed

Lines changed: 456 additions & 8 deletions

File tree

‎e2e/v1readsurface_unitconfig_test.go‎

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
package e2e
2+
3+
import (
4+
"net/http"
5+
"testing"
6+
7+
"github.com/samber/lo"
8+
"github.com/stretchr/testify/assert"
9+
"github.com/stretchr/testify/require"
10+
11+
api "github.com/openmeterio/openmeter/api/client/go"
12+
apiv3 "github.com/openmeterio/openmeter/api/v3"
13+
)
14+
15+
const unitConfigNotRepresentableCode = "unit_config_not_representable"
16+
17+
// Flow:
18+
// - v1 plan GET on a unit_config plan → 400 with the typed code (not a stripped 200)
19+
// - v1 plan GET on a plain plan → 200 (exclusion is content-derived, not blanket)
20+
// - v1 plan LIST omits the unit_config plan, keeps the plain one, and reports an exact TotalCount
21+
// - v1 subscribe-by-plan-key from the unit_config plan still succeeds (read ≠ subscribe; OM-399)
22+
// - v1 subscription GET on that subscription → 400 with the same typed code
23+
func TestV1ReadSurfaceExcludesUnitConfig(t *testing.T) {
24+
c := newV3Client(t)
25+
v1 := initClient(t)
26+
27+
uniq := uniqueKey("ucread")
28+
meterKey := "ucread_meter_" + uniq
29+
eventType := "ucread_event_" + uniq
30+
featureKey := "ucread_feature_" + uniq
31+
customerKey := "ucread_customer_" + uniq
32+
subjectKey := "ucread_subject_" + uniq
33+
ucPlanKey := "ucread_uc_plan_" + uniq
34+
plainPlanKey := "ucread_plain_plan_" + uniq
35+
36+
var ucPlan *apiv3.BillingPlan
37+
var plainPlan *apiv3.BillingPlan
38+
39+
// given:
40+
// - a unit_config plan (usage-based rate card, divide-by-1000 ceiling) and a plain plan, both
41+
// authored and published via v3 (v1 cannot author unit_config).
42+
runRequired(t, "creates a unit_config plan and a plain plan", func(t *testing.T) {
43+
status, meter, problem := c.CreateMeter(apiv3.CreateMeterRequest{
44+
Key: meterKey,
45+
Name: "UC Read Meter " + uniq,
46+
Aggregation: apiv3.MeterAggregationSum,
47+
EventType: eventType,
48+
ValueProperty: lo.ToPtr("$.value"),
49+
})
50+
require.Equal(t, http.StatusCreated, status, "problem: %+v", problem)
51+
require.NotNil(t, meter)
52+
53+
status, feature, problem := c.CreateFeature(apiv3.CreateFeatureRequest{
54+
Key: featureKey,
55+
Name: "UC Read Feature " + uniq,
56+
Meter: &apiv3.FeatureMeterReference{Id: meter.Id},
57+
})
58+
require.Equal(t, http.StatusCreated, status, "problem: %+v", problem)
59+
require.NotNil(t, feature)
60+
61+
cadence := apiv3.ISO8601Duration("P1M")
62+
term := apiv3.BillingPricePaymentTermInArrears
63+
price := apiv3.BillingPrice{}
64+
require.NoError(t, price.FromBillingPriceUnit(apiv3.BillingPriceUnit{
65+
Type: apiv3.BillingPriceUnitTypeUnit,
66+
Amount: "0.10",
67+
}))
68+
ucRateCard := apiv3.BillingRateCard{
69+
Key: feature.Key,
70+
Name: "UC Read Rate Card " + uniq,
71+
Price: price,
72+
BillingCadence: &cadence,
73+
PaymentTerm: &term,
74+
Feature: &apiv3.FeatureReference{Id: feature.Id},
75+
UnitConfig: &apiv3.BillingUnitConfig{
76+
Operation: apiv3.BillingUnitConfigOperationDivide,
77+
ConversionFactor: "1000",
78+
Rounding: lo.ToPtr(apiv3.BillingUnitConfigRoundingModeCeiling),
79+
Precision: lo.ToPtr(0),
80+
},
81+
}
82+
83+
status, createdUC, problem := c.CreatePlan(apiv3.CreatePlanRequest{
84+
Key: ucPlanKey,
85+
Name: "UC Read Plan " + uniq,
86+
Currency: "USD",
87+
BillingCadence: apiv3.ISO8601Duration("P1M"),
88+
Phases: []apiv3.BillingPlanPhase{{
89+
Key: "phase_1",
90+
Name: "UC Phase",
91+
RateCards: []apiv3.BillingRateCard{ucRateCard},
92+
}},
93+
})
94+
require.Equal(t, http.StatusCreated, status, "problem: %+v", problem)
95+
require.NotNil(t, createdUC)
96+
status, ucPlan, problem = c.PublishPlan(createdUC.Id)
97+
require.Equal(t, http.StatusOK, status, "problem: %+v", problem)
98+
require.NotNil(t, ucPlan)
99+
100+
status, createdPlain, problem := c.CreatePlan(apiv3.CreatePlanRequest{
101+
Key: plainPlanKey,
102+
Name: "Plain Read Plan " + uniq,
103+
Currency: "USD",
104+
BillingCadence: apiv3.ISO8601Duration("P1M"),
105+
Phases: []apiv3.BillingPlanPhase{validPlanPhase("plain_phase", true /* isLast */)},
106+
})
107+
require.Equal(t, http.StatusCreated, status, "problem: %+v", problem)
108+
require.NotNil(t, createdPlain)
109+
status, plainPlan, problem = c.PublishPlan(createdPlain.Id)
110+
require.Equal(t, http.StatusOK, status, "problem: %+v", problem)
111+
require.NotNil(t, plainPlan)
112+
})
113+
114+
// then:
115+
// - v1 GET on the unit_config plan is rejected with the typed code (not a silently-stripped 200).
116+
runRequired(t, "v1 plan GET rejects the unit_config plan", func(t *testing.T) {
117+
resp, err := v1.GetPlanWithResponse(t.Context(), ucPlan.Id, nil)
118+
require.NoError(t, err)
119+
require.Equal(t, http.StatusBadRequest, resp.StatusCode(), "body: %s", string(resp.Body))
120+
require.NotNil(t, resp.ApplicationproblemJSON400)
121+
assert.Contains(t, string(resp.Body), unitConfigNotRepresentableCode, "the 400 must carry the typed unit_config code")
122+
})
123+
124+
// and:
125+
// - the plain plan is still gettable via v1 (the exclusion is content-derived, not a blanket block).
126+
runRequired(t, "v1 plan GET returns the plain plan", func(t *testing.T) {
127+
resp, err := v1.GetPlanWithResponse(t.Context(), plainPlan.Id, nil)
128+
require.NoError(t, err)
129+
require.Equal(t, http.StatusOK, resp.StatusCode(), "body: %s", string(resp.Body))
130+
require.NotNil(t, resp.JSON200)
131+
assert.Equal(t, plainPlanKey, resp.JSON200.Key)
132+
})
133+
134+
// and:
135+
// - v1 LIST omits the unit_config plan, keeps the plain one, and TotalCount reflects the filtered set
136+
// (the exclusion runs at the query layer, before the COUNT — so the count stays exact).
137+
runRequired(t, "v1 plan LIST excludes the unit_config plan with an exact TotalCount", func(t *testing.T) {
138+
resp, err := v1.ListPlansWithResponse(t.Context(), &api.ListPlansParams{
139+
Key: &[]string{ucPlanKey, plainPlanKey},
140+
PageSize: lo.ToPtr(api.PaginationPageSize(1000)),
141+
})
142+
require.NoError(t, err)
143+
require.Equal(t, http.StatusOK, resp.StatusCode(), "body: %s", string(resp.Body))
144+
require.NotNil(t, resp.JSON200)
145+
146+
keys := lo.Map(resp.JSON200.Items, func(p api.Plan, _ int) string { return p.Key })
147+
assert.NotContains(t, keys, ucPlanKey, "unit_config plan must be excluded from the v1 list")
148+
assert.Contains(t, keys, plainPlanKey, "plain plan must remain in the v1 list")
149+
assert.Equal(t, 1, resp.JSON200.TotalCount, "TotalCount must reflect the filtered set, not the raw count")
150+
})
151+
152+
// and:
153+
// - a v1 subscription created from the unit_config plan BY KEY still succeeds (read ≠ subscribe; the
154+
// server rates it correctly per OM-399, only the read surfaces are restricted).
155+
var subscriptionID string
156+
runRequired(t, "v1 subscribe-by-plan-key from the unit_config plan succeeds", func(t *testing.T) {
157+
customer := CreateCustomerWithSubject(t, v1, customerKey, subjectKey)
158+
require.NotNil(t, customer)
159+
160+
timing := &api.SubscriptionTiming{}
161+
require.NoError(t, timing.FromSubscriptionTimingEnum(api.SubscriptionTimingEnumImmediate))
162+
163+
body := api.SubscriptionCreate{}
164+
require.NoError(t, body.FromPlanSubscriptionCreate(api.PlanSubscriptionCreate{
165+
Timing: timing,
166+
CustomerId: lo.ToPtr(customer.Id),
167+
Plan: api.PlanReferenceInput{
168+
Key: ucPlanKey,
169+
Version: lo.ToPtr(1),
170+
},
171+
}))
172+
173+
resp, err := v1.CreateSubscriptionWithResponse(t.Context(), body)
174+
require.NoError(t, err)
175+
require.Equal(t, http.StatusCreated, resp.StatusCode(), "body: %s", string(resp.Body))
176+
require.NotNil(t, resp.JSON201)
177+
subscriptionID = resp.JSON201.Id
178+
})
179+
180+
// then:
181+
// - v1 GET on that subscription is rejected with the same typed code (its items carry the unit_config).
182+
runRequired(t, "v1 subscription GET rejects the unit_config subscription", func(t *testing.T) {
183+
require.NotEmpty(t, subscriptionID)
184+
resp, err := v1.GetSubscriptionWithResponse(t.Context(), subscriptionID, nil)
185+
require.NoError(t, err)
186+
require.Equal(t, http.StatusBadRequest, resp.StatusCode(), "body: %s", string(resp.Body))
187+
require.NotNil(t, resp.ApplicationproblemJSON400)
188+
assert.Contains(t, string(resp.Body), unitConfigNotRepresentableCode, "the 400 must carry the typed unit_config code")
189+
})
190+
}

‎openmeter/productcatalog/errors.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,15 @@ var ErrRateCardUnitConfigRequiresUsageBasedPrice = models.NewValidationIssue(
400400
commonhttp.WithHTTPStatusCodeAttribute(http.StatusBadRequest),
401401
)
402402

403+
const ErrCodeUnitConfigNotRepresentable models.ErrorCode = "unit_config_not_representable"
404+
405+
var ErrUnitConfigNotRepresentable = models.NewValidationIssue(
406+
ErrCodeUnitConfigNotRepresentable,
407+
"this resource uses unit_config and is only available via the v3 API",
408+
models.WithCriticalSeverity(),
409+
commonhttp.WithHTTPStatusCodeAttribute(http.StatusBadRequest),
410+
)
411+
403412
const ErrCodeRateCardUsageBasedPriceWithFeatureAndNoMeter models.ErrorCode = "usage_based_price_with_feature_and_no_meter"
404413

405414
var ErrRateCardUsageBasedPriceWithFeatureAndNoMeter = models.NewValidationIssue(

‎openmeter/productcatalog/plan/adapter/adapter_test.go‎

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,8 @@ func TestPostgresAdapter(t *testing.T) {
177177
})
178178

179179
t.Run("CreateWithCreditOnlySettlement", func(t *testing.T) {
180-
input := pctestutils.NewTestPlan(t, namespace,
180+
input := pctestutils.NewTestPlan(
181+
t, namespace,
181182
pctestutils.WithPlanPhases(planPhases...),
182183
func(t *testing.T, p *productcatalog.Plan) {
183184
t.Helper()
@@ -420,6 +421,80 @@ func TestFromAddonRateCardRowMapsUnitConfig(t *testing.T) {
420421
"UnitConfig must survive the plan adapter's linked add-on mapper")
421422
}
422423

424+
func TestListPlansExcludeUnitConfig(t *testing.T) {
425+
env := pctestutils.NewTestEnv(t)
426+
t.Cleanup(func() { env.Close(t) })
427+
env.DBSchemaMigrate(t)
428+
429+
namespace := pctestutils.NewTestNamespace(t)
430+
431+
// A usage-based rate card carrying a unit_config needs a feature (usage-based price requires one).
432+
require.NoError(t, env.Meter.ReplaceMeters(t.Context(), pctestutils.NewTestMeters(t, namespace)),
433+
"replacing meters must not fail")
434+
435+
meters, err := env.Meter.ListMeters(t.Context(), meter.ListMetersParams{
436+
Page: pagination.Page{PageSize: 1000, PageNumber: 1},
437+
Namespace: namespace,
438+
})
439+
require.NoError(t, err, "listing meters must not fail")
440+
require.NotEmpty(t, meters.Items, "list of meters must not be empty")
441+
442+
feat, err := env.Feature.CreateFeature(t.Context(), pctestutils.NewTestFeatureFromMeter(t, &meters.Items[0]))
443+
require.NoError(t, err, "creating feature must not fail")
444+
445+
// Plain plan: default flat-fee rate card, no unit_config → v1-representable.
446+
_, err = env.Plan.CreatePlan(t.Context(), pctestutils.NewTestPlan(t, namespace, pctestutils.WithPlanKey("plain")))
447+
require.NoError(t, err, "creating plain plan must not fail")
448+
449+
// unit_config plan: usage-based rate card carrying a unit_config → not v1-representable.
450+
ucPhases := []productcatalog.Phase{
451+
{
452+
PhaseMeta: productcatalog.PhaseMeta{Key: "pro", Name: "Pro"},
453+
RateCards: []productcatalog.RateCard{
454+
&productcatalog.UsageBasedRateCard{
455+
RateCardMeta: productcatalog.RateCardMeta{
456+
Key: feat.Key,
457+
Name: "Pro RateCard",
458+
FeatureKey: &feat.Key,
459+
Price: productcatalog.NewPriceFrom(productcatalog.UnitPrice{Amount: decimal.NewFromInt(1)}),
460+
UnitConfig: &productcatalog.UnitConfig{
461+
Operation: productcatalog.UnitConfigOperationDivide,
462+
ConversionFactor: decimal.NewFromInt(1000),
463+
},
464+
},
465+
BillingCadence: pctestutils.MonthPeriod,
466+
},
467+
},
468+
},
469+
}
470+
_, err = env.Plan.CreatePlan(t.Context(), pctestutils.NewTestPlan(t, namespace,
471+
pctestutils.WithPlanKey("with-uc"), pctestutils.WithPlanPhases(ucPhases...)))
472+
require.NoError(t, err, "creating unit_config plan must not fail")
473+
474+
t.Run("included when ExcludeUnitConfig is false", func(t *testing.T) {
475+
list, err := env.PlanRepository.ListPlans(t.Context(), plan.ListPlansInput{
476+
Namespaces: []string{namespace},
477+
})
478+
require.NoError(t, err, "listing plans must not fail")
479+
480+
keys := lo.Map(list.Items, func(p plan.Plan, _ int) string { return p.Key })
481+
require.ElementsMatch(t, []string{"plain", "with-uc"}, keys)
482+
require.Equal(t, 2, list.TotalCount, "TotalCount must count both plans")
483+
})
484+
485+
t.Run("excluded when ExcludeUnitConfig is true, TotalCount stays consistent", func(t *testing.T) {
486+
list, err := env.PlanRepository.ListPlans(t.Context(), plan.ListPlansInput{
487+
Namespaces: []string{namespace},
488+
ExcludeUnitConfig: true,
489+
})
490+
require.NoError(t, err, "listing plans must not fail")
491+
492+
keys := lo.Map(list.Items, func(p plan.Plan, _ int) string { return p.Key })
493+
require.ElementsMatch(t, []string{"plain"}, keys)
494+
require.Equal(t, 1, list.TotalCount, "TotalCount must exclude the unit_config plan, not just the page slice")
495+
})
496+
}
497+
423498
type createPlanVersionInput struct {
424499
Namespace string
425500
Version int

‎openmeter/productcatalog/plan/adapter/plan.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,19 @@ func (a *adapter) ListPlans(ctx context.Context, params plan.ListPlansInput) (pa
6060
query = filter.ApplyToQuery(query, params.Name, plandb.FieldName)
6161
query = filter.ApplyToQuery(query, params.Currency, plandb.FieldCurrency)
6262

63+
if params.ExcludeUnitConfig {
64+
// The v1 API cannot represent a unit_config, so exclude any plan whose active rate cards
65+
// carry one. Filtering here (before Paginate's COUNT) keeps TotalCount and the page slice
66+
// consistent; DeletedAtIsNil scopes it to the active plan definition so a stale deleted
67+
// rate card cannot hide an otherwise-representable plan.
68+
query = query.Where(plandb.Not(plandb.HasPhasesWith(
69+
phasedb.HasRatecardsWith(
70+
ratecarddb.UnitConfigNotNil(),
71+
ratecarddb.DeletedAtIsNil(),
72+
),
73+
)))
74+
}
75+
6376
if !params.IncludeDeleted {
6477
query = query.Where(plandb.DeletedAtIsNil())
6578
}

‎openmeter/productcatalog/plan/httpdriver/plan.go‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,14 @@ func (h *handler) ListPlans() ListPlansHandler {
5252
PageSize: defaultx.WithDefault(params.PageSize, notification.DefaultPageSize),
5353
PageNumber: defaultx.WithDefault(params.Page, notification.DefaultPageNumber),
5454
},
55-
Namespaces: []string{ns},
56-
IDs: lo.FromPtr(params.Id),
57-
Keys: lo.FromPtr(params.Key),
58-
KeyVersions: lo.FromPtr(params.KeyVersion),
59-
IncludeDeleted: lo.FromPtr(params.IncludeDeleted),
60-
Currencies: lo.FromPtr(params.Currency),
61-
Status: statusFilter,
55+
Namespaces: []string{ns},
56+
IDs: lo.FromPtr(params.Id),
57+
Keys: lo.FromPtr(params.Key),
58+
KeyVersions: lo.FromPtr(params.KeyVersion),
59+
IncludeDeleted: lo.FromPtr(params.IncludeDeleted),
60+
Currencies: lo.FromPtr(params.Currency),
61+
Status: statusFilter,
62+
ExcludeUnitConfig: true,
6263
}
6364

6465
return req, nil
@@ -291,6 +292,10 @@ func (h *handler) GetPlan() GetPlanHandler {
291292
return GetPlanResponse{}, fmt.Errorf("failed to get plan: %w", err)
292293
}
293294

295+
if p.HasUnitConfig() {
296+
return GetPlanResponse{}, productcatalog.ErrUnitConfigNotRepresentable
297+
}
298+
294299
return FromPlan(*p)
295300
},
296301
commonhttp.JSONResponseEncoderWithStatus[GetPlanResponse](http.StatusOK),

‎openmeter/productcatalog/plan/plan.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,12 @@ func (p Plan) AsProductCatalogPlan() productcatalog.Plan {
4747
}
4848
}
4949

50+
func (p Plan) HasUnitConfig() bool {
51+
return lo.SomeBy(p.Phases, func(ph Phase) bool {
52+
return ph.RateCards.HasUnitConfig()
53+
})
54+
}
55+
5056
func ValidatePlanMeta() models.ValidatorFunc[Plan] {
5157
return func(p Plan) error {
5258
return p.PlanMeta.Validate()

0 commit comments

Comments
 (0)