diff --git a/e2e/v1readsurface_unitconfig_test.go b/e2e/v1readsurface_unitconfig_test.go new file mode 100644 index 0000000000..6054249f27 --- /dev/null +++ b/e2e/v1readsurface_unitconfig_test.go @@ -0,0 +1,219 @@ +package e2e + +import ( + "net/http" + "testing" + + "github.com/samber/lo" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + api "github.com/openmeterio/openmeter/api/client/go" + v3sdk "github.com/openmeterio/openmeter/api/v3/client" +) + +const unitConfigNotRepresentableCode = "unit_config_not_representable" + +// Flow: +// - v1 plan GET on a unit_config plan → 400 with the typed code (not a stripped 200) +// - v1 plan GET on a plain plan → 200 (exclusion is content-derived, not blanket) +// - v1 plan LIST omits the unit_config plan, keeps the plain one, and reports an exact TotalCount +// - v1 subscribe-by-plan-key from the unit_config plan still succeeds (read ≠ subscribe; OM-399) +// - v1 subscription GET on that subscription → 400 with the same typed code +func TestV1ReadSurfaceExcludesUnitConfig(t *testing.T) { + c := newV3Client(t) + v1 := initClient(t) + + uniq := uniqueKey("ucread") + meterKey := "ucread_meter_" + uniq + eventType := "ucread_event_" + uniq + featureKey := "ucread_feature_" + uniq + customerKey := "ucread_customer_" + uniq + subjectKey := "ucread_subject_" + uniq + ucPlanKey := "ucread_uc_plan_" + uniq + plainPlanKey := "ucread_plain_plan_" + uniq + + var ucPlan *v3sdk.Plan + var plainPlan *v3sdk.Plan + + // given: + // - a unit_config plan (usage-based rate card, divide-by-1000 ceiling) and a plain plan, both + // authored and published via v3 (v1 cannot author unit_config). + runRequired(t, "creates a unit_config plan and a plain plan", func(t *testing.T) { + meter, err := c.Meters.Create(t.Context(), v3sdk.CreateMeterRequest{ + Key: meterKey, + Name: "UC Read Meter " + uniq, + Aggregation: v3sdk.MeterAggregationSum, + EventType: eventType, + ValueProperty: lo.ToPtr("$.value"), + }) + c.requireStatus(http.StatusCreated, err) + require.NotNil(t, meter) + + feature, err := c.Features.Create(t.Context(), v3sdk.CreateFeatureRequest{ + Key: featureKey, + Name: "UC Read Feature " + uniq, + Meter: &v3sdk.FeatureMeterReferenceInput{ID: meter.ID}, + }) + c.requireStatus(http.StatusCreated, err) + require.NotNil(t, feature) + + cadence := "P1M" + term := v3sdk.PricePaymentTermInArrears + price := lo.Must(v3sdk.PriceFromPriceUnit(v3sdk.PriceUnit{ + Amount: "0.10", + })) + ucRateCard := v3sdk.RateCardInput{ + Key: feature.Key, + Name: "UC Read Rate Card " + uniq, + Price: price, + BillingCadence: &cadence, + PaymentTerm: &term, + Feature: &v3sdk.FeatureReference{ID: feature.ID}, + UnitConfig: &v3sdk.UnitConfig{ + Operation: v3sdk.UnitConfigOperationDivide, + ConversionFactor: "1000", + Rounding: lo.ToPtr(v3sdk.UnitConfigRoundingModeCeiling), + Precision: lo.ToPtr(int64(0)), + }, + } + + createdUC, err := c.Plans.Create(t.Context(), v3sdk.CreatePlanRequest{ + Key: ucPlanKey, + Name: "UC Read Plan " + uniq, + Currency: "USD", + BillingCadence: "P1M", + Phases: []v3sdk.PlanPhaseInput{{ + Key: "phase_1", + Name: "UC Phase", + RateCards: []v3sdk.RateCardInput{ucRateCard}, + }}, + }) + c.requireStatus(http.StatusCreated, err) + require.NotNil(t, createdUC) + ucPlan, err = c.Plans.Publish(t.Context(), createdUC.ID) + c.requireStatus(http.StatusOK, err) + require.NotNil(t, ucPlan) + + createdPlain, err := c.Plans.Create(t.Context(), v3sdk.CreatePlanRequest{ + Key: plainPlanKey, + Name: "Plain Read Plan " + uniq, + Currency: "USD", + BillingCadence: "P1M", + Phases: []v3sdk.PlanPhaseInput{validPlanPhase("plain_phase", true /* isLast */)}, + }) + c.requireStatus(http.StatusCreated, err) + require.NotNil(t, createdPlain) + plainPlan, err = c.Plans.Publish(t.Context(), createdPlain.ID) + c.requireStatus(http.StatusOK, err) + require.NotNil(t, plainPlan) + }) + + // then: + // - v1 GET on the unit_config plan is rejected with the typed code (not a silently-stripped 200). + runRequired(t, "v1 plan GET rejects the unit_config plan", func(t *testing.T) { + resp, err := v1.GetPlanWithResponse(t.Context(), ucPlan.ID, nil) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, resp.StatusCode(), "body: %s", string(resp.Body)) + require.NotNil(t, resp.ApplicationproblemJSON400) + assert.Contains(t, string(resp.Body), unitConfigNotRepresentableCode, "the 400 must carry the typed unit_config code") + }) + + // and: + // - the plain plan is still gettable via v1 (the exclusion is content-derived, not a blanket block). + runRequired(t, "v1 plan GET returns the plain plan", func(t *testing.T) { + resp, err := v1.GetPlanWithResponse(t.Context(), plainPlan.ID, nil) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode(), "body: %s", string(resp.Body)) + require.NotNil(t, resp.JSON200) + assert.Equal(t, plainPlanKey, resp.JSON200.Key) + }) + + // and: + // - v1 LIST omits the unit_config plan, keeps the plain one, and TotalCount reflects the filtered set + // (the exclusion runs at the query layer, before the COUNT — so the count stays exact). + runRequired(t, "v1 plan LIST excludes the unit_config plan with an exact TotalCount", func(t *testing.T) { + resp, err := v1.ListPlansWithResponse(t.Context(), &api.ListPlansParams{ + Key: &[]string{ucPlanKey, plainPlanKey}, + PageSize: lo.ToPtr(api.PaginationPageSize(1000)), + }) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode(), "body: %s", string(resp.Body)) + require.NotNil(t, resp.JSON200) + + keys := lo.Map(resp.JSON200.Items, func(p api.Plan, _ int) string { return p.Key }) + assert.NotContains(t, keys, ucPlanKey, "unit_config plan must be excluded from the v1 list") + assert.Contains(t, keys, plainPlanKey, "plain plan must remain in the v1 list") + assert.Equal(t, 1, resp.JSON200.TotalCount, "TotalCount must reflect the filtered set, not the raw count") + }) + + // and: + // - v1 mutations on the unit_config plan are rejected BEFORE any side effect. The guard runs + // pre-commit in the service (not in the response mapper), so a 400 means nothing committed — + // the plan is left exactly as it was. This is the core OM-409 follow-up: no commit-then-400. + runRequired(t, "v1 plan mutations reject the unit_config plan with no side effect", func(t *testing.T) { + // archive: the guard runs before the active-status check, so an active plan is still rejected. + archiveResp, err := v1.ArchivePlanWithResponse(t.Context(), ucPlan.ID) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, archiveResp.StatusCode(), "body: %s", string(archiveResp.Body)) + assert.Contains(t, string(archiveResp.Body), unitConfigNotRepresentableCode, "archive 400 must carry the typed unit_config code") + + // publish: the guard runs before the publishable-status check. + publishResp, err := v1.PublishPlanWithResponse(t.Context(), ucPlan.ID) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, publishResp.StatusCode(), "body: %s", string(publishResp.Body)) + assert.Contains(t, string(publishResp.Body), unitConfigNotRepresentableCode, "publish 400 must carry the typed unit_config code") + + // next: the guard runs before the next draft version is created. + nextResp, err := v1.NextPlanWithResponse(t.Context(), ucPlan.ID) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, nextResp.StatusCode(), "body: %s", string(nextResp.Body)) + assert.Contains(t, string(nextResp.Body), unitConfigNotRepresentableCode, "next 400 must carry the typed unit_config code") + + // no side effect: after three rejected mutations the plan is unchanged — still the single + // active version it was published as (a commit-then-400 mapper would have left it archived). + after, err := c.Plans.Get(t.Context(), ucPlan.ID) + c.requireStatus(http.StatusOK, err) + require.NotNil(t, after) + assert.Equal(t, v3sdk.PlanStatusActive, after.Status, "the plan must remain active — no mutation may have committed") + }) + + // and: + // - a v1 subscription created from the unit_config plan BY KEY still succeeds (read ≠ subscribe; the + // server rates it correctly per OM-399, only the read surfaces are restricted). + var subscriptionID string + runRequired(t, "v1 subscribe-by-plan-key from the unit_config plan succeeds", func(t *testing.T) { + customer := CreateCustomerWithSubject(t, v1, customerKey, subjectKey) + require.NotNil(t, customer) + + timing := &api.SubscriptionTiming{} + require.NoError(t, timing.FromSubscriptionTimingEnum(api.SubscriptionTimingEnumImmediate)) + + body := api.SubscriptionCreate{} + require.NoError(t, body.FromPlanSubscriptionCreate(api.PlanSubscriptionCreate{ + Timing: timing, + CustomerId: lo.ToPtr(customer.Id), + Plan: api.PlanReferenceInput{ + Key: ucPlanKey, + Version: lo.ToPtr(1), + }, + })) + + resp, err := v1.CreateSubscriptionWithResponse(t.Context(), body) + require.NoError(t, err) + require.Equal(t, http.StatusCreated, resp.StatusCode(), "body: %s", string(resp.Body)) + require.NotNil(t, resp.JSON201) + subscriptionID = resp.JSON201.Id + }) + + // then: + // - v1 GET on that subscription is rejected with the same typed code (its items carry the unit_config). + runRequired(t, "v1 subscription GET rejects the unit_config subscription", func(t *testing.T) { + require.NotEmpty(t, subscriptionID) + resp, err := v1.GetSubscriptionWithResponse(t.Context(), subscriptionID, nil) + require.NoError(t, err) + require.Equal(t, http.StatusBadRequest, resp.StatusCode(), "body: %s", string(resp.Body)) + require.NotNil(t, resp.ApplicationproblemJSON400) + assert.Contains(t, string(resp.Body), unitConfigNotRepresentableCode, "the 400 must carry the typed unit_config code") + }) +} diff --git a/openmeter/productcatalog/addon.go b/openmeter/productcatalog/addon.go index c79f9544a0..0282d58908 100644 --- a/openmeter/productcatalog/addon.go +++ b/openmeter/productcatalog/addon.go @@ -184,6 +184,10 @@ func (a Addon) ValidateWith(validators ...models.ValidatorFunc[Addon]) error { return models.Validate(a, validators...) } +func (a Addon) HasUnitConfig() bool { + return a.RateCards.HasUnitConfig() +} + // ValidationErrors returns a list of possible validation errors for the add-on. // It returns nil if the add-on has no validation issues. func (a Addon) ValidationErrors() (models.ValidationIssues, error) { diff --git a/openmeter/productcatalog/addon/adapter/adapter_test.go b/openmeter/productcatalog/addon/adapter/adapter_test.go index a03b237578..07a8908965 100644 --- a/openmeter/productcatalog/addon/adapter/adapter_test.go +++ b/openmeter/productcatalog/addon/adapter/adapter_test.go @@ -450,6 +450,78 @@ func TestPostgresAdapter(t *testing.T) { }) } +func TestListAddonsExcludeUnitConfig(t *testing.T) { + ctx := context.Background() + + env := pctestutils.NewTestEnv(t) + t.Cleanup(func() { env.Close(t) }) + env.DBSchemaMigrate(t) + + namespace := pctestutils.NewTestNamespace(t) + + require.NoError(t, env.Meter.ReplaceMeters(ctx, pctestutils.NewTestMeters(t, namespace)), + "replacing meters must not fail") + meters, err := env.Meter.ListMeters(ctx, meter.ListMetersParams{ + Page: pagination.Page{PageSize: 1000, PageNumber: 1}, + Namespace: namespace, + }) + require.NoError(t, err, "listing meters must not fail") + require.NotEmpty(t, meters.Items, "list of meters must not be empty") + + feat, err := env.Feature.CreateFeature(ctx, pctestutils.NewTestFeatureFromMeter(t, &meters.Items[0])) + require.NoError(t, err, "creating feature must not fail") + + // Plain add-on: flat rate card, no unit_config → v1-representable. + plainInput := pctestutils.NewTestAddon(t, namespace, &productcatalog.FlatFeeRateCard{ + RateCardMeta: productcatalog.RateCardMeta{Key: "flat", Name: "Flat"}, + }) + plainInput.Addon.Key = "plain" + _, err = env.AddonRepository.CreateAddon(ctx, plainInput) + require.NoError(t, err, "creating plain add-on must not fail") + + // unit_config add-on: usage-based rate card carrying a unit_config → not v1-representable. + ucInput := pctestutils.NewTestAddon(t, namespace, &productcatalog.UsageBasedRateCard{ + RateCardMeta: productcatalog.RateCardMeta{ + Key: feat.Key, + Name: "UC RateCard", + FeatureKey: lo.ToPtr(feat.Key), + FeatureID: lo.ToPtr(feat.ID), + Price: productcatalog.NewPriceFrom(productcatalog.UnitPrice{Amount: decimal.NewFromInt(1)}), + UnitConfig: &productcatalog.UnitConfig{ + Operation: productcatalog.UnitConfigOperationDivide, + ConversionFactor: decimal.NewFromInt(1000), + }, + }, + BillingCadence: MonthPeriod, + }) + ucInput.Addon.Key = "with-uc" + _, err = env.AddonRepository.CreateAddon(ctx, ucInput) + require.NoError(t, err, "creating unit_config add-on must not fail") + + t.Run("included when ExcludeUnitConfig is false", func(t *testing.T) { + list, err := env.AddonRepository.ListAddons(ctx, addon.ListAddonsInput{ + Namespaces: []string{namespace}, + }) + require.NoError(t, err, "listing add-ons must not fail") + + keys := lo.Map(list.Items, func(a addon.Addon, _ int) string { return a.Key }) + require.ElementsMatch(t, []string{"plain", "with-uc"}, keys) + require.Equal(t, 2, list.TotalCount, "TotalCount must count both add-ons") + }) + + t.Run("excluded when ExcludeUnitConfig is true, TotalCount stays consistent", func(t *testing.T) { + list, err := env.AddonRepository.ListAddons(ctx, addon.ListAddonsInput{ + Namespaces: []string{namespace}, + ExcludeUnitConfig: true, + }) + require.NoError(t, err, "listing add-ons must not fail") + + keys := lo.Map(list.Items, func(a addon.Addon, _ int) string { return a.Key }) + require.ElementsMatch(t, []string{"plain"}, keys) + require.Equal(t, 1, list.TotalCount, "TotalCount must exclude the unit_config add-on, not just the page slice") + }) +} + // TestFromPlanRateCardRowMapsUnitConfig guards the cross-package mapper used when an // add-on is loaded with expanded linked plans // (FromAddonRow → FromPlanAddonRow → FromPlanRow → FromPlanPhaseRow → FromPlanRateCardRow). diff --git a/openmeter/productcatalog/addon/adapter/addon.go b/openmeter/productcatalog/addon/adapter/addon.go index 77e5f666a6..6265203811 100644 --- a/openmeter/productcatalog/addon/adapter/addon.go +++ b/openmeter/productcatalog/addon/adapter/addon.go @@ -50,6 +50,13 @@ func (a *adapter) ListAddons(ctx context.Context, params addon.ListAddonsInput) query = filter.ApplyToQuery(query, params.Name, addondb.FieldName) query = filter.ApplyToQuery(query, params.Currency, addondb.FieldCurrency) + if params.ExcludeUnitConfig { + query = query.Where(addondb.Not(addondb.HasRatecardsWith( + addonratecarddb.UnitConfigNotNil(), + addonratecarddb.DeletedAtIsNil(), + ))) + } + if !params.IncludeDeleted { query = query.Where(addondb.DeletedAtIsNil()) } diff --git a/openmeter/productcatalog/addon/httpdriver/addon.go b/openmeter/productcatalog/addon/httpdriver/addon.go index e628ac52ed..9efae4e04e 100644 --- a/openmeter/productcatalog/addon/httpdriver/addon.go +++ b/openmeter/productcatalog/addon/httpdriver/addon.go @@ -56,6 +56,8 @@ func (h *handler) ListAddons() ListAddonsHandler { KeyVersions: lo.FromPtr(params.KeyVersion), IncludeDeleted: lo.FromPtr(params.IncludeDeleted), Status: statusFilter, + // The v1 API cannot represent unit_config; exclude such add-ons from the v1 list at the query layer. + ExcludeUnitConfig: true, } if params.Id != nil { @@ -185,6 +187,8 @@ func (h *handler) UpdateAddon() UpdateAddonHandler { req.IgnoreNonCriticalIssues = true + req.RejectUnitConfig = true + return req, nil }, func(ctx context.Context, request UpdateAddonRequest) (UpdateAddonResponse, error) { @@ -287,6 +291,10 @@ func (h *handler) GetAddon() GetAddonHandler { return GetAddonResponse{}, fmt.Errorf("failed to get add-on [namespace=%s key=%s id=%s]: %w", request.Namespace, request.Key, request.ID, err) } + if a.AsProductCatalogAddon().HasUnitConfig() { + return GetAddonResponse{}, productcatalog.ErrUnitConfigNotRepresentable + } + return FromAddon(*a) }, commonhttp.JSONResponseEncoderWithStatus[GetAddonResponse](http.StatusOK), @@ -322,6 +330,7 @@ func (h *handler) PublishAddon() PublishAddonHandler { EffectivePeriod: productcatalog.EffectivePeriod{ EffectiveFrom: lo.ToPtr(clock.Now()), }, + RejectUnitConfig: true, } return req, nil @@ -365,6 +374,8 @@ func (h *handler) ArchiveAddon() ArchiveAddonHandler { ID: addonID, }, EffectiveTo: clock.Now(), + // The v1 API cannot represent unit_config; reject archiving such an add-on. + RejectUnitConfig: true, } return req, nil diff --git a/openmeter/productcatalog/addon/service.go b/openmeter/productcatalog/addon/service.go index a236e43780..2448273e98 100644 --- a/openmeter/productcatalog/addon/service.go +++ b/openmeter/productcatalog/addon/service.go @@ -95,6 +95,9 @@ type ListAddonsInput struct { Key *filter.FilterString Name *filter.FilterString Currency *filter.FilterString + + // ExcludeUnitConfig omits add-ons carrying a unit_config conversion on any of their rate cards. + ExcludeUnitConfig bool } func (i ListAddonsInput) Validate() error { @@ -208,6 +211,12 @@ type UpdateAddonInput struct { // RateCards RateCards *productcatalog.RateCards `json:"rateCards,omitempty"` + // RejectUnitConfig makes mutation validation reject an add-on that carries a unit_config + // conversion on any rate card. The v1 API cannot represent unit_config, and v1 update + // rewrites rate cards from a body that has no such field, so proceeding would silently + // drop the conversion. v1 handlers set this; v3 leaves it false. + RejectUnitConfig bool + inputOptions } @@ -350,6 +359,10 @@ type PublishAddonInput struct { // AddonEffectivePeriod productcatalog.EffectivePeriod + + // RejectUnitConfig rejects the operation when the target add-on carries a unit_config + // conversion. The v1 API cannot represent it, so v1 handlers set this; v3 leaves it false. + RejectUnitConfig bool } func (i PublishAddonInput) Validate() error { @@ -398,6 +411,10 @@ type ArchiveAddonInput struct { // EffectiveFrom defines the time from the Addon is going to be unpublished. EffectiveTo time.Time `json:"effectiveTo,omitempty"` + + // RejectUnitConfig rejects the operation when the target add-on carries a unit_config + // conversion. The v1 API cannot represent it, so v1 handlers set this; v3 leaves it false. + RejectUnitConfig bool } func (i ArchiveAddonInput) Validate() error { diff --git a/openmeter/productcatalog/addon/service/addon.go b/openmeter/productcatalog/addon/service/addon.go index dfd4837ab7..bc9b3702c9 100644 --- a/openmeter/productcatalog/addon/service/addon.go +++ b/openmeter/productcatalog/addon/service/addon.go @@ -342,6 +342,10 @@ func (s service) UpdateAddon(ctx context.Context, params addon.UpdateAddonInput) return nil, fmt.Errorf("failed to get add-on: %w", err) } + if params.RejectUnitConfig && add.AsProductCatalogAddon().HasUnitConfig() { + return nil, productcatalog.ErrUnitConfigNotRepresentable + } + // Run validations prior updating add-on. if err = add.AsProductCatalogAddon().ValidateWith( productcatalog.ValidateAddonWithStatus(productcatalog.AddonStatusDraft), @@ -404,6 +408,10 @@ func (s service) PublishAddon(ctx context.Context, params addon.PublishAddonInpu ) } + if params.RejectUnitConfig && add.AsProductCatalogAddon().HasUnitConfig() { + return nil, productcatalog.ErrUnitConfigNotRepresentable + } + pa := add.AsProductCatalogAddon() // Run validations prior publishing add-on. @@ -452,7 +460,8 @@ func (s service) PublishAddon(ctx context.Context, params addon.PublishAddonInpu Namespace: activeAddon.Namespace, ID: activeAddon.ID, }, - EffectiveTo: lo.FromPtr(params.EffectiveFrom), + EffectiveTo: lo.FromPtr(params.EffectiveFrom), + RejectUnitConfig: params.RejectUnitConfig, }) if err != nil { return nil, fmt.Errorf("failed to archive add-on with active status: %w", err) @@ -523,6 +532,10 @@ func (s service) ArchiveAddon(ctx context.Context, params addon.ArchiveAddonInpu ) } + if params.RejectUnitConfig && add.AsProductCatalogAddon().HasUnitConfig() { + return nil, productcatalog.ErrUnitConfigNotRepresentable + } + // Run validations prior archiving add-on. if err = add.AsProductCatalogAddon().ValidateWith( productcatalog.ValidateAddonWithStatus(productcatalog.AddonStatusActive), diff --git a/openmeter/productcatalog/errors.go b/openmeter/productcatalog/errors.go index f314300076..7a8b335de8 100644 --- a/openmeter/productcatalog/errors.go +++ b/openmeter/productcatalog/errors.go @@ -400,6 +400,15 @@ var ErrRateCardUnitConfigRequiresUsageBasedPrice = models.NewValidationIssue( commonhttp.WithHTTPStatusCodeAttribute(http.StatusBadRequest), ) +const ErrCodeUnitConfigNotRepresentable models.ErrorCode = "unit_config_not_representable" + +var ErrUnitConfigNotRepresentable = models.NewValidationIssue( + ErrCodeUnitConfigNotRepresentable, + "this resource uses unit_config and is only available via the v3 API", + models.WithCriticalSeverity(), + commonhttp.WithHTTPStatusCodeAttribute(http.StatusBadRequest), +) + const ErrCodeRateCardUsageBasedPriceWithFeatureAndNoMeter models.ErrorCode = "usage_based_price_with_feature_and_no_meter" var ErrRateCardUsageBasedPriceWithFeatureAndNoMeter = models.NewValidationIssue( diff --git a/openmeter/productcatalog/http/mapping.go b/openmeter/productcatalog/http/mapping.go index e6c8b26fa9..434b545407 100644 --- a/openmeter/productcatalog/http/mapping.go +++ b/openmeter/productcatalog/http/mapping.go @@ -923,7 +923,7 @@ func FromAnnotations(annotations models.Annotations) *api.Annotations { return nil } - return lo.ToPtr((api.Annotations)(annotations)) + return lo.ToPtr(api.Annotations(annotations)) } func FromValidationErrors(issues models.ValidationIssues) *[]api.ValidationError { diff --git a/openmeter/productcatalog/plan.go b/openmeter/productcatalog/plan.go index 2b19b299dc..8a4a528088 100644 --- a/openmeter/productcatalog/plan.go +++ b/openmeter/productcatalog/plan.go @@ -63,6 +63,15 @@ func (p Plan) ValidateWith(validators ...models.ValidatorFunc[Plan]) error { return models.Validate(p, validators...) } +// HasUnitConfig reports whether any rate card in any phase carries a unit_config +// conversion. The v1 API cannot represent unit_config, so v1 read and mutation +// surfaces use this to reject such plans instead of silently stripping the field. +func (p Plan) HasUnitConfig() bool { + return lo.SomeBy(p.Phases, func(ph Phase) bool { + return ph.RateCards.HasUnitConfig() + }) +} + func ValidatePlanMeta() models.ValidatorFunc[Plan] { return func(p Plan) error { return p.PlanMeta.Validate() diff --git a/openmeter/productcatalog/plan/adapter/adapter_test.go b/openmeter/productcatalog/plan/adapter/adapter_test.go index 4364c9feda..1355304d22 100644 --- a/openmeter/productcatalog/plan/adapter/adapter_test.go +++ b/openmeter/productcatalog/plan/adapter/adapter_test.go @@ -177,7 +177,8 @@ func TestPostgresAdapter(t *testing.T) { }) t.Run("CreateWithCreditOnlySettlement", func(t *testing.T) { - input := pctestutils.NewTestPlan(t, namespace, + input := pctestutils.NewTestPlan( + t, namespace, pctestutils.WithPlanPhases(planPhases...), func(t *testing.T, p *productcatalog.Plan) { t.Helper() @@ -420,6 +421,80 @@ func TestFromAddonRateCardRowMapsUnitConfig(t *testing.T) { "UnitConfig must survive the plan adapter's linked add-on mapper") } +func TestListPlansExcludeUnitConfig(t *testing.T) { + env := pctestutils.NewTestEnv(t) + t.Cleanup(func() { env.Close(t) }) + env.DBSchemaMigrate(t) + + namespace := pctestutils.NewTestNamespace(t) + + // A usage-based rate card carrying a unit_config needs a feature (usage-based price requires one). + require.NoError(t, env.Meter.ReplaceMeters(t.Context(), pctestutils.NewTestMeters(t, namespace)), + "replacing meters must not fail") + + meters, err := env.Meter.ListMeters(t.Context(), meter.ListMetersParams{ + Page: pagination.Page{PageSize: 1000, PageNumber: 1}, + Namespace: namespace, + }) + require.NoError(t, err, "listing meters must not fail") + require.NotEmpty(t, meters.Items, "list of meters must not be empty") + + feat, err := env.Feature.CreateFeature(t.Context(), pctestutils.NewTestFeatureFromMeter(t, &meters.Items[0])) + require.NoError(t, err, "creating feature must not fail") + + // Plain plan: default flat-fee rate card, no unit_config → v1-representable. + _, err = env.Plan.CreatePlan(t.Context(), pctestutils.NewTestPlan(t, namespace, pctestutils.WithPlanKey("plain"))) + require.NoError(t, err, "creating plain plan must not fail") + + // unit_config plan: usage-based rate card carrying a unit_config → not v1-representable. + ucPhases := []productcatalog.Phase{ + { + PhaseMeta: productcatalog.PhaseMeta{Key: "pro", Name: "Pro"}, + RateCards: []productcatalog.RateCard{ + &productcatalog.UsageBasedRateCard{ + RateCardMeta: productcatalog.RateCardMeta{ + Key: feat.Key, + Name: "Pro RateCard", + FeatureKey: &feat.Key, + Price: productcatalog.NewPriceFrom(productcatalog.UnitPrice{Amount: decimal.NewFromInt(1)}), + UnitConfig: &productcatalog.UnitConfig{ + Operation: productcatalog.UnitConfigOperationDivide, + ConversionFactor: decimal.NewFromInt(1000), + }, + }, + BillingCadence: pctestutils.MonthPeriod, + }, + }, + }, + } + _, err = env.Plan.CreatePlan(t.Context(), pctestutils.NewTestPlan(t, namespace, + pctestutils.WithPlanKey("with-uc"), pctestutils.WithPlanPhases(ucPhases...))) + require.NoError(t, err, "creating unit_config plan must not fail") + + t.Run("included when ExcludeUnitConfig is false", func(t *testing.T) { + list, err := env.PlanRepository.ListPlans(t.Context(), plan.ListPlansInput{ + Namespaces: []string{namespace}, + }) + require.NoError(t, err, "listing plans must not fail") + + keys := lo.Map(list.Items, func(p plan.Plan, _ int) string { return p.Key }) + require.ElementsMatch(t, []string{"plain", "with-uc"}, keys) + require.Equal(t, 2, list.TotalCount, "TotalCount must count both plans") + }) + + t.Run("excluded when ExcludeUnitConfig is true, TotalCount stays consistent", func(t *testing.T) { + list, err := env.PlanRepository.ListPlans(t.Context(), plan.ListPlansInput{ + Namespaces: []string{namespace}, + ExcludeUnitConfig: true, + }) + require.NoError(t, err, "listing plans must not fail") + + keys := lo.Map(list.Items, func(p plan.Plan, _ int) string { return p.Key }) + require.ElementsMatch(t, []string{"plain"}, keys) + require.Equal(t, 1, list.TotalCount, "TotalCount must exclude the unit_config plan, not just the page slice") + }) +} + type createPlanVersionInput struct { Namespace string Version int diff --git a/openmeter/productcatalog/plan/adapter/plan.go b/openmeter/productcatalog/plan/adapter/plan.go index 0d26f32d66..1acbc8a1b8 100644 --- a/openmeter/productcatalog/plan/adapter/plan.go +++ b/openmeter/productcatalog/plan/adapter/plan.go @@ -60,6 +60,19 @@ func (a *adapter) ListPlans(ctx context.Context, params plan.ListPlansInput) (pa query = filter.ApplyToQuery(query, params.Name, plandb.FieldName) query = filter.ApplyToQuery(query, params.Currency, plandb.FieldCurrency) + if params.ExcludeUnitConfig { + // The v1 API cannot represent a unit_config, so exclude any plan whose active rate cards + // carry one. Filtering here (before Paginate's COUNT) keeps TotalCount and the page slice + // consistent; DeletedAtIsNil scopes it to the active plan definition so a stale deleted + // rate card cannot hide an otherwise-representable plan. + query = query.Where(plandb.Not(plandb.HasPhasesWith( + phasedb.HasRatecardsWith( + ratecarddb.UnitConfigNotNil(), + ratecarddb.DeletedAtIsNil(), + ), + ))) + } + if !params.IncludeDeleted { query = query.Where(plandb.DeletedAtIsNil()) } diff --git a/openmeter/productcatalog/plan/httpdriver/plan.go b/openmeter/productcatalog/plan/httpdriver/plan.go index 781bc8cfb5..58e846edc2 100644 --- a/openmeter/productcatalog/plan/httpdriver/plan.go +++ b/openmeter/productcatalog/plan/httpdriver/plan.go @@ -52,13 +52,14 @@ func (h *handler) ListPlans() ListPlansHandler { PageSize: defaultx.WithDefault(params.PageSize, notification.DefaultPageSize), PageNumber: defaultx.WithDefault(params.Page, notification.DefaultPageNumber), }, - Namespaces: []string{ns}, - IDs: lo.FromPtr(params.Id), - Keys: lo.FromPtr(params.Key), - KeyVersions: lo.FromPtr(params.KeyVersion), - IncludeDeleted: lo.FromPtr(params.IncludeDeleted), - Currencies: lo.FromPtr(params.Currency), - Status: statusFilter, + Namespaces: []string{ns}, + IDs: lo.FromPtr(params.Id), + Keys: lo.FromPtr(params.Key), + KeyVersions: lo.FromPtr(params.KeyVersion), + IncludeDeleted: lo.FromPtr(params.IncludeDeleted), + Currencies: lo.FromPtr(params.Currency), + Status: statusFilter, + ExcludeUnitConfig: true, } return req, nil @@ -182,6 +183,7 @@ func (h *handler) UpdatePlan() UpdatePlanHandler { } req.IgnoreNonCriticalIssues = true + req.RejectUnitConfig = true return req, nil }, @@ -291,6 +293,10 @@ func (h *handler) GetPlan() GetPlanHandler { return GetPlanResponse{}, fmt.Errorf("failed to get plan: %w", err) } + if p.HasUnitConfig() { + return GetPlanResponse{}, productcatalog.ErrUnitConfigNotRepresentable + } + return FromPlan(*p) }, commonhttp.JSONResponseEncoderWithStatus[GetPlanResponse](http.StatusOK), @@ -326,6 +332,7 @@ func (h *handler) PublishPlan() PublishPlanHandler { EffectivePeriod: productcatalog.EffectivePeriod{ EffectiveFrom: lo.ToPtr(clock.Now()), }, + RejectUnitConfig: true, } return req, nil @@ -368,7 +375,8 @@ func (h *handler) ArchivePlan() ArchivePlanHandler { Namespace: ns, ID: planID, }, - EffectiveTo: clock.Now(), + EffectiveTo: clock.Now(), + RejectUnitConfig: true, } return req, nil @@ -414,8 +422,9 @@ func (h *handler) NextPlan() NextPlanHandler { Namespace: ns, ID: idOrKey.ID, }, - Key: idOrKey.Key, - Version: 0, + Key: idOrKey.Key, + Version: 0, + RejectUnitConfig: true, } return req, nil diff --git a/openmeter/productcatalog/plan/plan.go b/openmeter/productcatalog/plan/plan.go index 3ea2008863..dd3fbdee38 100644 --- a/openmeter/productcatalog/plan/plan.go +++ b/openmeter/productcatalog/plan/plan.go @@ -47,6 +47,12 @@ func (p Plan) AsProductCatalogPlan() productcatalog.Plan { } } +func (p Plan) HasUnitConfig() bool { + return lo.SomeBy(p.Phases, func(ph Phase) bool { + return ph.RateCards.HasUnitConfig() + }) +} + func ValidatePlanMeta() models.ValidatorFunc[Plan] { return func(p Plan) error { return p.PlanMeta.Validate() diff --git a/openmeter/productcatalog/plan/plan_test.go b/openmeter/productcatalog/plan/plan_test.go new file mode 100644 index 0000000000..595fed6640 --- /dev/null +++ b/openmeter/productcatalog/plan/plan_test.go @@ -0,0 +1,43 @@ +package plan + +import ( + "testing" + + decimal "github.com/alpacahq/alpacadecimal" + "github.com/samber/lo" + "github.com/stretchr/testify/assert" + + "github.com/openmeterio/openmeter/openmeter/productcatalog" +) + +func TestPlanHasUnitConfig(t *testing.T) { + card := func(uc *productcatalog.UnitConfig) productcatalog.RateCard { + return &productcatalog.UsageBasedRateCard{ + RateCardMeta: productcatalog.RateCardMeta{ + Key: "feat-1", + Name: "Feature 1", + FeatureKey: lo.ToPtr("feat-1"), + Price: productcatalog.NewPriceFrom(productcatalog.UnitPrice{Amount: decimal.NewFromInt(1)}), + UnitConfig: uc, + }, + } + } + phase := func(cards ...productcatalog.RateCard) Phase { + return Phase{Phase: productcatalog.Phase{RateCards: cards}} + } + divide := &productcatalog.UnitConfig{Operation: productcatalog.UnitConfigOperationDivide, ConversionFactor: decimal.NewFromInt(1000)} + + t.Run("plan with no phases has none", func(t *testing.T) { + assert.False(t, Plan{}.HasUnitConfig()) + }) + + t.Run("no phase carries unit_config", func(t *testing.T) { + p := Plan{Phases: []Phase{phase(card(nil)), phase(card(nil))}} + assert.False(t, p.HasUnitConfig()) + }) + + t.Run("a later phase carrying unit_config is detected", func(t *testing.T) { + p := Plan{Phases: []Phase{phase(card(nil)), phase(card(nil), card(divide))}} + assert.True(t, p.HasUnitConfig()) + }) +} diff --git a/openmeter/productcatalog/plan/service.go b/openmeter/productcatalog/plan/service.go index 5ea12e67b6..3692baa18e 100644 --- a/openmeter/productcatalog/plan/service.go +++ b/openmeter/productcatalog/plan/service.go @@ -106,6 +106,9 @@ type ListPlansInput struct { // Currency filters plans by their currency field (AND semantics, supports eq/neq/contains/oeq). Currency *filter.FilterString + + // ExcludeUnitConfig omits plans carrying a unit_config conversion on any of their rate cards. (v1 can't represent it) + ExcludeUnitConfig bool } func (i ListPlansInput) Validate() error { @@ -214,6 +217,9 @@ type UpdatePlanInput struct { // Phases Phases *[]productcatalog.Phase `json:"phases"` + // RejectUnitConfig makes mutation validation reject a plan that carries a unit_config conversion on any rate card. + RejectUnitConfig bool + inputOptions } @@ -319,6 +325,10 @@ func (i UpdatePlanInput) Validate() error { } func (i UpdatePlanInput) ValidateWithPlan(p productcatalog.Plan) error { + if i.RejectUnitConfig && p.HasUnitConfig() { + return productcatalog.ErrUnitConfigNotRepresentable + } + var errs []error if i.Name != nil { @@ -420,6 +430,10 @@ type PublishPlanInput struct { // EffectivePeriod productcatalog.EffectivePeriod + + // RejectUnitConfig rejects the operation when the target plan carries a unit_config + // conversion. The v1 API cannot represent it, so v1 handlers set this; v3 leaves it false. + RejectUnitConfig bool } func (i PublishPlanInput) Validate() error { @@ -468,6 +482,10 @@ type ArchivePlanInput struct { // EffectiveFrom defines the time from the Plan is going to be unpublished. EffectiveTo time.Time `json:"effectiveTo,omitempty"` + + // RejectUnitConfig rejects the operation when the target plan carries a unit_config + // conversion. The v1 API cannot represent it, so v1 handlers set this; v3 leaves it false. + RejectUnitConfig bool } func (i ArchivePlanInput) Validate() error { @@ -503,6 +521,10 @@ type NextPlanInput struct { // Version is the version of the Plan. // If not set the latest version is assumed. Version int `json:"version,omitempty"` + + // RejectUnitConfig rejects the operation when the target plan carries a unit_config + // conversion. The v1 API cannot represent it, so v1 handlers set this; v3 leaves it false. + RejectUnitConfig bool } func (i NextPlanInput) Validate() error { diff --git a/openmeter/productcatalog/plan/service/plan.go b/openmeter/productcatalog/plan/service/plan.go index 5a1b484b72..006795d822 100644 --- a/openmeter/productcatalog/plan/service/plan.go +++ b/openmeter/productcatalog/plan/service/plan.go @@ -392,6 +392,11 @@ func (s service) PublishPlan(ctx context.Context, params plan.PublishPlanInput) // Validate the plan before publishing it // + // The v1 API cannot represent unit_config; reject before publishing + if params.RejectUnitConfig && p.HasUnitConfig() { + return nil, productcatalog.ErrUnitConfigNotRepresentable + } + // Check if the plan is already deleted if p.DeletedAt != nil { @@ -489,7 +494,8 @@ func (s service) PublishPlan(ctx context.Context, params plan.PublishPlanInput) Namespace: activePlan.Namespace, ID: activePlan.ID, }, - EffectiveTo: lo.FromPtr(params.EffectiveFrom), + EffectiveTo: lo.FromPtr(params.EffectiveFrom), + RejectUnitConfig: params.RejectUnitConfig, }) if err != nil { return nil, fmt.Errorf("failed to archive plan with active status: %w", err) @@ -552,6 +558,11 @@ func (s service) ArchivePlan(ctx context.Context, params plan.ArchivePlanInput) return nil, fmt.Errorf("failed to get Plan: %w", err) } + // The v1 API cannot represent unit_config; reject before archiving + if params.RejectUnitConfig && p.HasUnitConfig() { + return nil, productcatalog.ErrUnitConfigNotRepresentable + } + activeStatuses := []productcatalog.PlanStatus{productcatalog.PlanStatusActive} status := p.Status() if !lo.Contains(activeStatuses, status) { @@ -691,6 +702,11 @@ func (s service) NextPlan(ctx context.Context, params plan.NextPlanInput) (*plan ) } + // The v1 API cannot represent unit_config; reject before creating the next draft + if params.RejectUnitConfig && sourcePlan.HasUnitConfig() { + return nil, productcatalog.ErrUnitConfigNotRepresentable + } + nextPlan, err := s.adapter.CreatePlan(ctx, plan.CreatePlanInput{ NamespacedModel: models.NamespacedModel{ Namespace: sourcePlan.Namespace, diff --git a/openmeter/productcatalog/plan/service_test.go b/openmeter/productcatalog/plan/service_test.go new file mode 100644 index 0000000000..073200b7be --- /dev/null +++ b/openmeter/productcatalog/plan/service_test.go @@ -0,0 +1,71 @@ +package plan + +import ( + "slices" + "testing" + + decimal "github.com/alpacahq/alpacadecimal" + "github.com/samber/lo" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/openmeterio/openmeter/openmeter/productcatalog" + "github.com/openmeterio/openmeter/pkg/models" +) + +// TestUpdatePlanInputValidateWithPlanRejectsUnitConfig covers the v1 write-loss guard: a v1 update +// replaces rate cards from a body that has no unit_config field, so the guard must reject based on +// the STORED plan (the argument), before the incoming fields are merged, or the conversion would be +// silently dropped. The flag is opt-in so the v3 path (flag off) is unaffected. +func TestUpdatePlanInputValidateWithPlanRejectsUnitConfig(t *testing.T) { + card := func(uc *productcatalog.UnitConfig) productcatalog.RateCard { + return &productcatalog.UsageBasedRateCard{ + RateCardMeta: productcatalog.RateCardMeta{ + Key: "feat-1", + Name: "Feature 1", + FeatureKey: lo.ToPtr("feat-1"), + Price: productcatalog.NewPriceFrom(productcatalog.UnitPrice{Amount: decimal.NewFromInt(1)}), + UnitConfig: uc, + }, + } + } + planWith := func(cards ...productcatalog.RateCard) productcatalog.Plan { + return productcatalog.Plan{Phases: []productcatalog.Phase{{RateCards: cards}}} + } + divide := &productcatalog.UnitConfig{Operation: productcatalog.UnitConfigOperationDivide, ConversionFactor: decimal.NewFromInt(1000)} + + rejectedForUnitConfig := func(err error) bool { + if err == nil { + return false + } + issues, convErr := models.AsValidationIssues(err) + if convErr != nil { + return false + } + return slices.ContainsFunc(issues, func(i models.ValidationIssue) bool { + return i.Code() == productcatalog.ErrCodeUnitConfigNotRepresentable + }) + } + + t.Run("flag on + stored plan has unit_config is rejected even though the update body carries none", func(t *testing.T) { + // The input intentionally carries no phases (a v1 body cannot express unit_config); the + // rejection must come from the stored plan, proving the check runs before the merge. + in := UpdatePlanInput{RejectUnitConfig: true} + + err := in.ValidateWithPlan(planWith(card(divide))) + require.Error(t, err) + assert.True(t, rejectedForUnitConfig(err), "expected unit_config rejection, got %v", err) + }) + + t.Run("flag off does not reject a stored unit_config plan", func(t *testing.T) { + in := UpdatePlanInput{} + + assert.False(t, rejectedForUnitConfig(in.ValidateWithPlan(planWith(card(divide))))) + }) + + t.Run("flag on does not reject a plan without unit_config", func(t *testing.T) { + in := UpdatePlanInput{RejectUnitConfig: true} + + assert.False(t, rejectedForUnitConfig(in.ValidateWithPlan(planWith(card(nil))))) + }) +} diff --git a/openmeter/productcatalog/plan_test.go b/openmeter/productcatalog/plan_test.go index a2a9b80cf7..d2e74ce3cd 100644 --- a/openmeter/productcatalog/plan_test.go +++ b/openmeter/productcatalog/plan_test.go @@ -342,3 +342,35 @@ func TestAlignmentEnforcement(t *testing.T) { assert.ErrorContains(t, err, "ratecards with prices must have compatible billing cadence") }) } + +func TestPlanHasUnitConfig(t *testing.T) { + card := func(uc *productcatalog.UnitConfig) productcatalog.RateCard { + return &productcatalog.UsageBasedRateCard{ + RateCardMeta: productcatalog.RateCardMeta{ + Key: "feat-1", + Name: "Feature 1", + FeatureKey: lo.ToPtr("feat-1"), + Price: productcatalog.NewPriceFrom(productcatalog.UnitPrice{Amount: alpacadecimal.NewFromInt(1)}), + UnitConfig: uc, + }, + } + } + phase := func(cards ...productcatalog.RateCard) productcatalog.Phase { + return productcatalog.Phase{RateCards: cards} + } + divide := &productcatalog.UnitConfig{Operation: productcatalog.UnitConfigOperationDivide, ConversionFactor: alpacadecimal.NewFromInt(1000)} + + t.Run("plan with no phases has none", func(t *testing.T) { + assert.False(t, productcatalog.Plan{}.HasUnitConfig()) + }) + + t.Run("no phase carries unit_config", func(t *testing.T) { + p := productcatalog.Plan{Phases: []productcatalog.Phase{phase(card(nil)), phase(card(nil))}} + assert.False(t, p.HasUnitConfig()) + }) + + t.Run("unit_config in any later phase is detected", func(t *testing.T) { + p := productcatalog.Plan{Phases: []productcatalog.Phase{phase(card(nil)), phase(card(nil), card(divide))}} + assert.True(t, p.HasUnitConfig()) + }) +} diff --git a/openmeter/productcatalog/planaddon/httpdriver/planaddon.go b/openmeter/productcatalog/planaddon/httpdriver/planaddon.go index 5ae1cfe90e..6769e722e1 100644 --- a/openmeter/productcatalog/planaddon/httpdriver/planaddon.go +++ b/openmeter/productcatalog/planaddon/httpdriver/planaddon.go @@ -8,6 +8,7 @@ import ( "github.com/samber/lo" "github.com/openmeterio/openmeter/api" + "github.com/openmeterio/openmeter/openmeter/productcatalog" productcataloghttp "github.com/openmeterio/openmeter/openmeter/productcatalog/http" "github.com/openmeterio/openmeter/openmeter/productcatalog/planaddon" "github.com/openmeterio/openmeter/pkg/framework/commonhttp" @@ -71,6 +72,10 @@ func (h *handler) ListPlanAddons() ListPlanAddonsHandler { for _, a := range resp.Items { var item api.PlanAddon + if a.Plan.HasUnitConfig() || a.Addon.AsProductCatalogAddon().HasUnitConfig() { + return ListPlanAddonsResponse{}, productcatalog.ErrUnitConfigNotRepresentable + } + item, err = FromPlanAddon(a) if err != nil { return ListPlanAddonsResponse{}, fmt.Errorf("failed to cast plan add-on assignment [namespace=%s plan.id=%s addon.id=%s]: %w", @@ -120,6 +125,7 @@ func (h *handler) CreatePlanAddon() CreatePlanAddonHandler { return CreatePlanAddonRequest{}, fmt.Errorf("failed to parse create plan add-on assignment request [namespace=%s plan.id=%s]: %w", ns, planID, err) } + req.RejectUnitConfig = true return req, nil }, @@ -169,6 +175,7 @@ func (h *handler) UpdatePlanAddon() UpdatePlanAddonHandler { return UpdatePlanAddonRequest{}, fmt.Errorf("failed to parse update plan add-on assignment request [namespace=%s plan.id=%s addon.id=%s]: %w", ns, params.PlanID, params.AddonID, err) } + req.RejectUnitConfig = true return req, nil }, @@ -267,6 +274,10 @@ func (h *handler) GetPlanAddon() GetPlanAddonHandler { request.Namespace, request.PlanIDOrKey, request.AddonIDOrKey, err) } + if a.Plan.HasUnitConfig() || a.Addon.AsProductCatalogAddon().HasUnitConfig() { + return GetPlanAddonResponse{}, productcatalog.ErrUnitConfigNotRepresentable + } + return FromPlanAddon(*a) }, commonhttp.JSONResponseEncoderWithStatus[GetPlanAddonResponse](http.StatusOK), diff --git a/openmeter/productcatalog/planaddon/service.go b/openmeter/productcatalog/planaddon/service.go index d3143081ae..6b7e151130 100644 --- a/openmeter/productcatalog/planaddon/service.go +++ b/openmeter/productcatalog/planaddon/service.go @@ -105,6 +105,9 @@ type CreatePlanAddonInput struct { // MaxQuantity MaxQuantity *int `json:"maxQuantity"` + + // RejectUnitConfig rejects the operation when the referenced plan carries a unit_config conversion + RejectUnitConfig bool } func (i CreatePlanAddonInput) Validate() error { @@ -153,6 +156,9 @@ type UpdatePlanAddonInput struct { // MaxQuantity MaxQuantity *int `json:"maxQuantity"` + + // RejectUnitConfig rejects the operation when the referenced plan carries a unit_config conversion + RejectUnitConfig bool } func (i UpdatePlanAddonInput) Equal(p PlanAddon) bool { diff --git a/openmeter/productcatalog/planaddon/service/planaddon.go b/openmeter/productcatalog/planaddon/service/planaddon.go index c351e2a138..929671e536 100644 --- a/openmeter/productcatalog/planaddon/service/planaddon.go +++ b/openmeter/productcatalog/planaddon/service/planaddon.go @@ -125,6 +125,10 @@ func (s service) CreatePlanAddon(ctx context.Context, params planaddon.CreatePla ) } + if params.RejectUnitConfig && (p.HasUnitConfig() || a.AsProductCatalogAddon().HasUnitConfig()) { + return nil, productcatalog.ErrUnitConfigNotRepresentable + } + // // Create plan add-on assignment // @@ -322,6 +326,11 @@ func (s service) UpdatePlanAddon(ctx context.Context, params planaddon.UpdatePla ) } + if params.RejectUnitConfig && + (planAddon.Plan.HasUnitConfig() || planAddon.Addon.AsProductCatalogAddon().HasUnitConfig()) { + return nil, productcatalog.ErrUnitConfigNotRepresentable + } + // // Validate plan add-on assignment compatibility after applying the patch // diff --git a/openmeter/productcatalog/ratecard.go b/openmeter/productcatalog/ratecard.go index 6c9696f11e..2ba01fe22a 100644 --- a/openmeter/productcatalog/ratecard.go +++ b/openmeter/productcatalog/ratecard.go @@ -5,6 +5,7 @@ import ( "encoding/json" "errors" "fmt" + "slices" "github.com/samber/lo" @@ -611,6 +612,12 @@ func (c RateCards) At(idx int) RateCard { return c[idx] } +func (c RateCards) HasUnitConfig() bool { + return slices.ContainsFunc(c, func(rc RateCard) bool { + return rc.AsMeta().UnitConfig != nil + }) +} + func (c RateCards) Billables() RateCards { var billables RateCards for _, rc := range c { diff --git a/openmeter/productcatalog/ratecard_test.go b/openmeter/productcatalog/ratecard_test.go index 44c69c12f1..6f3c6e3770 100644 --- a/openmeter/productcatalog/ratecard_test.go +++ b/openmeter/productcatalog/ratecard_test.go @@ -1370,6 +1370,33 @@ func TestRateCardMetaUnitConfig(t *testing.T) { }) } +func TestRateCardsHasUnitConfig(t *testing.T) { + card := func(uc *UnitConfig) RateCard { + return &UsageBasedRateCard{ + RateCardMeta: RateCardMeta{ + Key: "feat-1", + Name: "Feature 1", + FeatureKey: lo.ToPtr("feat-1"), + Price: NewPriceFrom(UnitPrice{Amount: decimal.NewFromInt(1)}), + UnitConfig: uc, + }, + } + } + divide := &UnitConfig{Operation: UnitConfigOperationDivide, ConversionFactor: decimal.NewFromInt(1000)} + + t.Run("empty collection has none", func(t *testing.T) { + assert.False(t, RateCards{}.HasUnitConfig()) + }) + + t.Run("no rate card carries unit_config", func(t *testing.T) { + assert.False(t, RateCards{card(nil), card(nil)}.HasUnitConfig()) + }) + + t.Run("any rate card carrying unit_config is detected", func(t *testing.T) { + assert.True(t, RateCards{card(nil), card(divide)}.HasUnitConfig()) + }) +} + func TestValidateRateCardsHaveCompatibleUnitConfig(t *testing.T) { card := func(uc *UnitConfig) RateCard { return &UsageBasedRateCard{ diff --git a/openmeter/productcatalog/subscription/http/change.go b/openmeter/productcatalog/subscription/http/change.go index c8e55a6af3..fe0b5abc59 100644 --- a/openmeter/productcatalog/subscription/http/change.go +++ b/openmeter/productcatalog/subscription/http/change.go @@ -90,6 +90,7 @@ func (h *handler) ChangeSubscription() ChangeSubscriptionHandler { }, BillingAnchor: parsedBody.BillingAnchor, }, + RejectUnitConfig: true, }, nil } else { // Changing to a Plan @@ -126,8 +127,9 @@ func (h *handler) ChangeSubscription() ChangeSubscriptionHandler { Description: parsedBody.Description, BillingAnchor: parsedBody.BillingAnchor, }, - StartingPhase: parsedBody.StartingPhase, - SettlementMode: settlementMode, + StartingPhase: parsedBody.StartingPhase, + SettlementMode: settlementMode, + RejectUnitConfig: true, }, nil } }, diff --git a/openmeter/productcatalog/subscription/http/get.go b/openmeter/productcatalog/subscription/http/get.go index 6743191b0b..54391cde73 100644 --- a/openmeter/productcatalog/subscription/http/get.go +++ b/openmeter/productcatalog/subscription/http/get.go @@ -9,6 +9,7 @@ import ( "github.com/openmeterio/openmeter/api" "github.com/openmeterio/openmeter/openmeter/customer" + "github.com/openmeterio/openmeter/openmeter/productcatalog" "github.com/openmeterio/openmeter/openmeter/subscription" "github.com/openmeterio/openmeter/pkg/filter" "github.com/openmeterio/openmeter/pkg/framework/commonhttp" @@ -59,6 +60,10 @@ func (h *handler) GetSubscription() GetSubscriptionHandler { return def, err } + if view.Spec.HasUnitConfig() { + return def, productcatalog.ErrUnitConfigNotRepresentable + } + return MapSubscriptionViewToAPI(view) }, commonhttp.JSONResponseEncoderWithStatus[GetSubscriptionResponse](http.StatusOK), diff --git a/openmeter/productcatalog/subscription/http/migrate.go b/openmeter/productcatalog/subscription/http/migrate.go index 1b8dd5b500..44f55de506 100644 --- a/openmeter/productcatalog/subscription/http/migrate.go +++ b/openmeter/productcatalog/subscription/http/migrate.go @@ -51,10 +51,11 @@ func (h *handler) MigrateSubscription() MigrateSubscriptionHandler { Namespace: ns, ID: params.ID, }, - TargetVersion: body.TargetVersion, - StartingPhase: body.StartingPhase, - Timing: timing, - BillingAnchor: body.BillingAnchor, + TargetVersion: body.TargetVersion, + StartingPhase: body.StartingPhase, + Timing: timing, + BillingAnchor: body.BillingAnchor, + RejectUnitConfig: true, }, nil }, func(ctx context.Context, request MigrateSubscriptionRequest) (MigrateSubscriptionResponse, error) { diff --git a/openmeter/productcatalog/subscription/service.go b/openmeter/productcatalog/subscription/service.go index a546616925..a4116e9fba 100644 --- a/openmeter/productcatalog/subscription/service.go +++ b/openmeter/productcatalog/subscription/service.go @@ -23,11 +23,12 @@ type SubscriptionChangeResponse struct { } type MigrateSubscriptionRequest struct { - ID models.NamespacedID - TargetVersion *int - StartingPhase *string - Timing *subscription.Timing - BillingAnchor *time.Time + ID models.NamespacedID + TargetVersion *int + StartingPhase *string + Timing *subscription.Timing + BillingAnchor *time.Time + RejectUnitConfig bool } type ChangeSubscriptionRequest struct { @@ -38,6 +39,8 @@ type ChangeSubscriptionRequest struct { // Only used if existing plan is provided StartingPhase *string SettlementMode *productcatalog.SettlementMode + + RejectUnitConfig bool } type CreateSubscriptionRequest struct { diff --git a/openmeter/productcatalog/subscription/service/change.go b/openmeter/productcatalog/subscription/service/change.go index 0e5cc6377c..3d031ade0b 100644 --- a/openmeter/productcatalog/subscription/service/change.go +++ b/openmeter/productcatalog/subscription/service/change.go @@ -30,6 +30,10 @@ func (s *service) Change(ctx context.Context, request plansubscription.ChangeSub return def, err } + if request.RejectUnitConfig && planInput.Plan.HasUnitConfig() { + return def, productcatalog.ErrUnitConfigNotRepresentable + } + plan = p } else if request.PlanInput.AsRef() != nil { p, err := s.getPlanByVersion(ctx, request.ID.Namespace, *request.PlanInput.AsRef()) @@ -61,6 +65,10 @@ func (s *service) Change(ctx context.Context, request plansubscription.ChangeSub p.SettlementMode = *request.SettlementMode } + if request.RejectUnitConfig && p.HasUnitConfig() { + return def, productcatalog.ErrUnitConfigNotRepresentable + } + plan = PlanFromPlan(*p) } else { return def, fmt.Errorf("plan or plan reference must be provided, input should already be validated") diff --git a/openmeter/productcatalog/subscription/service/migrate.go b/openmeter/productcatalog/subscription/service/migrate.go index 345a069035..f9f46539dd 100644 --- a/openmeter/productcatalog/subscription/service/migrate.go +++ b/openmeter/productcatalog/subscription/service/migrate.go @@ -73,6 +73,10 @@ func (s *service) Migrate(ctx context.Context, request plansubscription.MigrateS } } + if request.RejectUnitConfig && p.HasUnitConfig() { + return def, productcatalog.ErrUnitConfigNotRepresentable + } + pp := PlanFromPlan(*p) var timing subscription.Timing diff --git a/openmeter/server/router/router.go b/openmeter/server/router/router.go index 75f5cf5e1c..b20dfc950e 100644 --- a/openmeter/server/router/router.go +++ b/openmeter/server/router/router.go @@ -498,6 +498,7 @@ func NewRouter(config Config) (*Router, error) { SubscriptionAddonService: config.SubscriptionAddonService, SubscriptionWorkflowService: config.SubscriptionWorkflowService, SubscriptionService: config.SubscriptionService, + AddonService: config.Addon, NamespaceDecoder: config.NamespaceDecoder, Logger: config.Logger, }, diff --git a/openmeter/subscription/addon/http/create.go b/openmeter/subscription/addon/http/create.go index 7ebb1ca2ba..36a2e32b16 100644 --- a/openmeter/subscription/addon/http/create.go +++ b/openmeter/subscription/addon/http/create.go @@ -8,6 +8,8 @@ import ( "github.com/samber/lo" "github.com/openmeterio/openmeter/api" + "github.com/openmeterio/openmeter/openmeter/productcatalog" + "github.com/openmeterio/openmeter/openmeter/productcatalog/addon" "github.com/openmeterio/openmeter/openmeter/subscription" subscriptionaddon "github.com/openmeterio/openmeter/openmeter/subscription/addon" subscriptionworkflow "github.com/openmeterio/openmeter/openmeter/subscription/workflow" @@ -58,6 +60,23 @@ func (h *handler) CreateSubscriptionAddon() CreateSubscriptionAddonHandler { func(ctx context.Context, req CreateSubscriptionAddonRequest) (CreateSubscriptionAddonResponse, error) { var def CreateSubscriptionAddonResponse + // The add-on's own rate cards are serialized in the response, so reject a unit_config + // add-on (the v1 shape cannot represent it) before it lands on the subscription. We do + // NOT reject based on the subscription's plan + addonToAdd, err := h.AddonService.GetAddon(ctx, addon.GetAddonInput{ + NamespacedID: models.NamespacedID{ + Namespace: req.SubscriptionID.Namespace, + ID: req.AddonInput.AddonID, + }, + }) + if err != nil { + return def, err + } + + if addonToAdd.AsProductCatalogAddon().HasUnitConfig() { + return def, productcatalog.ErrUnitConfigNotRepresentable + } + subsAdds, err := h.SubscriptionAddonService.List(ctx, req.SubscriptionID.Namespace, subscriptionaddon.ListSubscriptionAddonsInput{ SubscriptionID: req.SubscriptionID.ID, }) diff --git a/openmeter/subscription/addon/http/handler.go b/openmeter/subscription/addon/http/handler.go index 1a8e7a7c2e..dbb932bdea 100644 --- a/openmeter/subscription/addon/http/handler.go +++ b/openmeter/subscription/addon/http/handler.go @@ -7,6 +7,7 @@ import ( "net/http" "github.com/openmeterio/openmeter/openmeter/namespace/namespacedriver" + "github.com/openmeterio/openmeter/openmeter/productcatalog/addon" "github.com/openmeterio/openmeter/openmeter/subscription" subscriptionaddon "github.com/openmeterio/openmeter/openmeter/subscription/addon" subscriptionworkflow "github.com/openmeterio/openmeter/openmeter/subscription/workflow" @@ -25,6 +26,7 @@ type HandlerConfig struct { SubscriptionAddonService subscriptionaddon.Service SubscriptionWorkflowService subscriptionworkflow.Service SubscriptionService subscription.Service + AddonService addon.Service NamespaceDecoder namespacedriver.NamespaceDecoder Logger *slog.Logger } diff --git a/openmeter/subscription/subscriptionspec.go b/openmeter/subscription/subscriptionspec.go index d5fef2537d..3d46dc0b99 100644 --- a/openmeter/subscription/subscriptionspec.go +++ b/openmeter/subscription/subscriptionspec.go @@ -203,6 +203,12 @@ func (s *SubscriptionSpec) HasMeteredBillables() bool { }) } +func (s *SubscriptionSpec) HasUnitConfig() bool { + return lo.SomeBy(lo.Values(s.Phases), func(p *SubscriptionPhaseSpec) bool { + return p.HasUnitConfig() + }) +} + // For a phase in an Aligned subscription, there's a single aligned BillingPeriod for all items in that phase. // The period starts with the phase and iterates every subscription.BillingCadence duration, but can be reanchored to the time of an edit. func (s *SubscriptionSpec) GetAlignedBillingPeriodAt(at time.Time) (timeutil.ClosedPeriod, error) { @@ -479,6 +485,12 @@ func (s SubscriptionPhaseSpec) HasMeteredBillables() bool { }) } +func (s SubscriptionPhaseSpec) HasUnitConfig() bool { + return lo.SomeBy(lo.Flatten(lo.Values(s.ItemsByKey)), func(item *SubscriptionItemSpec) bool { + return item.RateCard.AsMeta().UnitConfig != nil + }) +} + func (s SubscriptionPhaseSpec) HasBillables() bool { return len(s.GetBillableItemsByKey()) > 0 } diff --git a/openmeter/subscription/subscriptionspec_test.go b/openmeter/subscription/subscriptionspec_test.go index c277417184..b3e6db97f0 100644 --- a/openmeter/subscription/subscriptionspec_test.go +++ b/openmeter/subscription/subscriptionspec_test.go @@ -5,9 +5,12 @@ import ( "testing" "time" + decimal "github.com/alpacahq/alpacadecimal" "github.com/samber/lo" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/openmeterio/openmeter/openmeter/productcatalog" "github.com/openmeterio/openmeter/openmeter/subscription" "github.com/openmeterio/openmeter/pkg/clock" "github.com/openmeterio/openmeter/pkg/models" @@ -134,3 +137,53 @@ func TestGetFullServicePeriodAtInputValidate(t *testing.T) { }) } } + +func TestSubscriptionSpecHasUnitConfig(t *testing.T) { + card := func(uc *productcatalog.UnitConfig) productcatalog.RateCard { + return &productcatalog.UsageBasedRateCard{ + RateCardMeta: productcatalog.RateCardMeta{ + Key: "feat-1", + Name: "Feature 1", + FeatureKey: lo.ToPtr("feat-1"), + Price: productcatalog.NewPriceFrom(productcatalog.UnitPrice{Amount: decimal.NewFromInt(1)}), + UnitConfig: uc, + }, + } + } + item := func(uc *productcatalog.UnitConfig) *subscription.SubscriptionItemSpec { + return &subscription.SubscriptionItemSpec{ + CreateSubscriptionItemInput: subscription.CreateSubscriptionItemInput{ + CreateSubscriptionItemPlanInput: subscription.CreateSubscriptionItemPlanInput{ + PhaseKey: "phase-1", + ItemKey: "item-1", + RateCard: card(uc), + }, + }, + } + } + specWith := func(items ...*subscription.SubscriptionItemSpec) subscription.SubscriptionSpec { + return subscription.SubscriptionSpec{ + Phases: map[string]*subscription.SubscriptionPhaseSpec{ + "phase-1": { + ItemsByKey: map[string][]*subscription.SubscriptionItemSpec{"item-1": items}, + }, + }, + } + } + divide := &productcatalog.UnitConfig{Operation: productcatalog.UnitConfigOperationDivide, ConversionFactor: decimal.NewFromInt(1000)} + + t.Run("empty spec has none", func(t *testing.T) { + s := subscription.SubscriptionSpec{} + assert.False(t, s.HasUnitConfig()) + }) + + t.Run("no item carries unit_config", func(t *testing.T) { + s := specWith(item(nil)) + assert.False(t, s.HasUnitConfig()) + }) + + t.Run("an item carrying unit_config is detected", func(t *testing.T) { + s := specWith(item(nil), item(divide)) + assert.True(t, s.HasUnitConfig()) + }) +}