From c0da265ef535aa932704f65d2bc751786e62b3fe Mon Sep 17 00:00:00 2001 From: rolosp <6114043+rolosp@users.noreply.github.com> Date: Tue, 30 Jun 2026 16:55:02 +0200 Subject: [PATCH 1/6] feat: apply unit config in rating via mutator --- app/common/billing.go | 6 +- app/common/config.go | 2 + cmd/billing-worker/wire_gen.go | 3 +- cmd/jobs/internal/wire_gen.go | 3 +- cmd/server/wire_gen.go | 3 +- .../billing/charges/service/base_test.go | 13 +- .../charges/service/unitconfig_rating_test.go | 164 ++++++++++++++++++ .../billing/charges/testutils/service.go | 6 +- .../billing/charges/usagebased/rating.go | 8 + .../service/rating/delta/base_test.go | 2 +- .../rating/periodpreserving/engine_test.go | 6 +- .../usagebased/service/rating/service_test.go | 4 +- .../service/rating/subtract/subtract_test.go | 2 +- openmeter/billing/rating/line.go | 3 + .../billing/rating/service/billableperiod.go | 2 +- .../billing/rating/service/detailedline.go | 2 +- .../rating/service/mutator/unitconfig.go | 54 ++++++ .../rating/service/mutator/unitconfig_test.go | 159 +++++++++++++++++ openmeter/billing/rating/service/pricer.go | 15 +- .../service/rate/unitconfig_rating_test.go | 155 +++++++++++++++++ openmeter/billing/rating/service/service.go | 17 +- .../rating/service/testutil/ubptest.go | 29 +++- .../service/unitconfig_registration_test.go | 48 +++++ openmeter/billing/stdinvoiceline.go | 16 ++ .../ledger/customerbalance/testenv_test.go | 4 +- openmeter/productcatalog/price.go | 10 ++ openmeter/productcatalog/price_test.go | 40 +++++ openmeter/productcatalog/ratecard.go | 10 +- test/app/testenv.go | 2 +- test/billing/suite.go | 2 +- test/customer/testenv.go | 2 +- test/subscription/framework_test.go | 2 +- 32 files changed, 752 insertions(+), 42 deletions(-) create mode 100644 openmeter/billing/charges/service/unitconfig_rating_test.go create mode 100644 openmeter/billing/rating/service/mutator/unitconfig.go create mode 100644 openmeter/billing/rating/service/mutator/unitconfig_test.go create mode 100644 openmeter/billing/rating/service/rate/unitconfig_rating_test.go create mode 100644 openmeter/billing/rating/service/unitconfig_registration_test.go diff --git a/app/common/billing.go b/app/common/billing.go index e80a84c425..3db932de21 100644 --- a/app/common/billing.go +++ b/app/common/billing.go @@ -242,8 +242,10 @@ func NewBillingCustomerOverrideService(billingRegistry BillingRegistry) billing. return billingRegistry.Billing } -func NewBillingRatingService() rating.Service { - return billingratingservice.New() +func NewBillingRatingService(unitConfig config.UnitConfigConfiguration) rating.Service { + return billingratingservice.New(billingratingservice.Config{ + UnitConfigEnabled: unitConfig.Enabled, + }) } func NewBillingAutoAdvancer(logger *slog.Logger, billingRegistry BillingRegistry) (*billingworkerautoadvance.AutoAdvancer, error) { diff --git a/app/common/config.go b/app/common/config.go index 4728a535a5..f761c70da8 100644 --- a/app/common/config.go +++ b/app/common/config.go @@ -52,6 +52,8 @@ var Config = wire.NewSet( wire.FieldsOf(new(config.Configuration), "ReservedEventTypes"), // Svix wire.FieldsOf(new(config.Configuration), "Svix"), + // UnitConfig + wire.FieldsOf(new(config.Configuration), "UnitConfig"), // Telemetry wire.FieldsOf(new(config.Configuration), "Telemetry"), wire.FieldsOf(new(config.TelemetryConfig), "Metrics"), diff --git a/cmd/billing-worker/wire_gen.go b/cmd/billing-worker/wire_gen.go index dcd11d4390..d13e704fd8 100644 --- a/cmd/billing-worker/wire_gen.go +++ b/cmd/billing-worker/wire_gen.go @@ -153,7 +153,8 @@ func initializeApplication(ctx context.Context, conf config.Configuration) (Appl cleanup() return Application{}, nil, err } - ratingService := common.NewBillingRatingService() + unitConfigConfiguration := conf.UnitConfig + ratingService := common.NewBillingRatingService(unitConfigConfiguration) customerService, err := common.NewCustomerService(logger, client, eventbusPublisher) if err != nil { cleanup6() diff --git a/cmd/jobs/internal/wire_gen.go b/cmd/jobs/internal/wire_gen.go index c5b19bf0af..4c9c7da5f7 100644 --- a/cmd/jobs/internal/wire_gen.go +++ b/cmd/jobs/internal/wire_gen.go @@ -170,7 +170,8 @@ func initializeApplication(ctx context.Context, conf config.Configuration) (Appl cleanup() return Application{}, nil, err } - ratingService := common.NewBillingRatingService() + unitConfigConfiguration := conf.UnitConfig + ratingService := common.NewBillingRatingService(unitConfigConfiguration) adapterAdapter, err := common.NewMeterAdapter(logger, client) if err != nil { cleanup6() diff --git a/cmd/server/wire_gen.go b/cmd/server/wire_gen.go index 9975d2d12d..a631c9d7b2 100644 --- a/cmd/server/wire_gen.go +++ b/cmd/server/wire_gen.go @@ -223,7 +223,8 @@ func initializeApplication(ctx context.Context, conf config.Configuration) (Appl cleanup() return Application{}, nil, err } - ratingService := common.NewBillingRatingService() + unitConfigConfiguration := conf.UnitConfig + ratingService := common.NewBillingRatingService(unitConfigConfiguration) customerService, err := common.NewCustomerService(logger, client, eventbusPublisher) if err != nil { cleanup6() diff --git a/openmeter/billing/charges/service/base_test.go b/openmeter/billing/charges/service/base_test.go index 426ab563ba..6afbd80cc4 100644 --- a/openmeter/billing/charges/service/base_test.go +++ b/openmeter/billing/charges/service/base_test.go @@ -44,6 +44,11 @@ const USD = currencyx.Code(currency.USD) type BaseSuite struct { billingtest.BaseSuite + // UnitConfigEnabled toggles the unitConfig.enabled rating flag for the charges + // stack the suite builds. Defaults to false; a derived suite sets it in its own + // SetupSuite (before calling BaseSuite.SetupSuite) to exercise unit_config rating. + UnitConfigEnabled bool + Charges *service UsageBasedService usagebased.Service FlatFeeTestHandler *flatFeeTestHandler @@ -92,7 +97,7 @@ func (s *BaseSuite) SetupSuite() { Lineage: lineageService, MetaAdapter: metaAdapter, Locker: locker, - RatingService: billingratingservice.New(), + RatingService: billingratingservice.New(billingratingservice.Config{UnitConfigEnabled: s.UnitConfigEnabled}), }) s.NoError(err) @@ -114,7 +119,7 @@ func (s *BaseSuite) SetupSuite() { MetaAdapter: metaAdapter, CustomerOverrideService: s.BillingService, FeatureService: s.FeatureService, - RatingService: billingratingservice.New(), + RatingService: billingratingservice.New(billingratingservice.Config{UnitConfigEnabled: s.UnitConfigEnabled}), StreamingConnector: s.MockStreamingConnector, }) s.NoError(err) @@ -139,7 +144,7 @@ func (s *BaseSuite) SetupSuite() { s.NoError(err) creditPurchaseLineEngine, err := creditpurchaselineengine.New(creditpurchaselineengine.Config{ - RatingService: billingratingservice.New(), + RatingService: billingratingservice.New(billingratingservice.Config{UnitConfigEnabled: s.UnitConfigEnabled}), }) s.NoError(err) @@ -194,6 +199,7 @@ type createMockChargeIntentInput struct { currency currencyx.Code servicePeriod timeutil.ClosedPeriod price *productcatalog.Price + unitConfig *productcatalog.UnitConfig featureKey string name string settlementMode productcatalog.SettlementMode @@ -283,6 +289,7 @@ func (s *BaseSuite) createMockChargeIntent(input createMockChargeIntentInput) ch IntentMutableFields: usagebased.IntentMutableFields{ IntentMutableFields: intentMutableFields, Price: lo.FromPtr(input.price), + UnitConfig: input.unitConfig, InvoiceAt: invoiceAt, }, SettlementMode: lo.CoalesceOrEmpty(input.settlementMode, productcatalog.CreditThenInvoiceSettlementMode), diff --git a/openmeter/billing/charges/service/unitconfig_rating_test.go b/openmeter/billing/charges/service/unitconfig_rating_test.go new file mode 100644 index 0000000000..987374c724 --- /dev/null +++ b/openmeter/billing/charges/service/unitconfig_rating_test.go @@ -0,0 +1,164 @@ +package service + +import ( + "testing" + "time" + + "github.com/alpacahq/alpacadecimal" + "github.com/samber/lo" + "github.com/stretchr/testify/suite" + + "github.com/openmeterio/openmeter/openmeter/billing" + "github.com/openmeterio/openmeter/openmeter/billing/charges" + "github.com/openmeterio/openmeter/openmeter/productcatalog" + "github.com/openmeterio/openmeter/pkg/clock" + "github.com/openmeterio/openmeter/pkg/datetime" + "github.com/openmeterio/openmeter/pkg/timeutil" + billingtest "github.com/openmeterio/openmeter/test/billing" +) + +// These suites drive the REAL charges path end-to-end: a usage-based charge whose +// intent carries a unit_config is created, usage is seeded, and the charge is +// invoiced — exercising rating.GenerateDetailedLines via RateableIntent.GetUnitConfig. +// The enabled/disabled split proves the unitConfig.enabled flag actually gates the +// converted amount through the production wiring (not just the in-memory mutator). + +func TestUsageBasedUnitConfigRatingEnabled(t *testing.T) { + suite.Run(t, new(unitConfigRatingEnabledSuite)) +} + +type unitConfigRatingEnabledSuite struct { + BaseSuite +} + +func (s *unitConfigRatingEnabledSuite) SetupSuite() { + s.UnitConfigEnabled = true + s.BaseSuite.SetupSuite() +} + +func (s *unitConfigRatingEnabledSuite) TearDownTest() { + s.BaseSuite.TearDownTest() +} + +func (s *unitConfigRatingEnabledSuite) TestRatesConvertedQuantity() { + // flag on: 7400 raw / 1000, ceiling => 8 billed units * $1 = $8. + s.runUnitConfigChargesScenario(8) +} + +func TestUsageBasedUnitConfigRatingDisabled(t *testing.T) { + suite.Run(t, new(unitConfigRatingDisabledSuite)) +} + +type unitConfigRatingDisabledSuite struct { + BaseSuite +} + +func (s *unitConfigRatingDisabledSuite) SetupSuite() { + // UnitConfigEnabled defaults to false: the intent still carries a unit_config, + // but the mutator is not registered, so rating must bill the raw quantity. + s.BaseSuite.SetupSuite() +} + +func (s *unitConfigRatingDisabledSuite) TearDownTest() { + s.BaseSuite.TearDownTest() +} + +func (s *unitConfigRatingDisabledSuite) TestRatesRawQuantityWhenFlagOff() { + // flag off: unit_config ignored, 7400 raw units * $1 = $7400 (parity with today). + s.runUnitConfigChargesScenario(7400) +} + +// runUnitConfigChargesScenario creates a usage-based charge carrying a divide-by-1000 +// ceiling unit_config, seeds 7400 raw units, invoices mid-period, and asserts the +// rated line amount matches expectedAmount. +func (s *BaseSuite) runUnitConfigChargesScenario(expectedAmount float64) { + s.T().Helper() + + ctx := s.T().Context() + ns := s.GetUniqueNamespace("charges-service-unit-config-rating") + s.ProvisionDefaultTaxCodes(ctx, ns) + + customInvoicing := s.SetupCustomInvoicing(ns) + + cust := s.CreateTestCustomer(ns, "test-subject") + s.NotEmpty(cust.ID) + + _ = s.ProvisionBillingProfile(ctx, ns, customInvoicing.App.GetID(), + billingtest.WithProgressiveBilling(), + billingtest.WithCollectionInterval(datetime.MustParseDuration(s.T(), "P2D")), + billingtest.WithManualApproval(), + ) + + createAt := datetime.MustParseTimeInLocation(s.T(), "2025-12-01T00:00:00Z", time.UTC).AsTime() + servicePeriod := timeutil.ClosedPeriod{ + From: datetime.MustParseTimeInLocation(s.T(), "2026-01-01T00:00:00Z", time.UTC).AsTime(), + To: datetime.MustParseTimeInLocation(s.T(), "2026-02-01T00:00:00Z", time.UTC).AsTime(), + } + invoiceAt := datetime.MustParseTimeInLocation(s.T(), "2026-01-16T00:00:00Z", time.UTC).AsTime() + + apiRequestsTotal := s.SetupApiRequestsTotalFeature(ctx, ns) + meterSlug := apiRequestsTotal.Feature.Key + + clock.FreezeTime(createAt) + defer clock.UnFreeze() + defer s.UsageBasedTestHandler.Reset() + + // Cap credit-only accrual at 0 so the full amount is invoiced (no credits). + s.UsageBasedTestHandler.onCreditsOnlyUsageAccrued, _ = newCappedCreditAllocator(0) + + // Meter is in raw units, bill in thousands: divide by 1000, round up. + unitConfig := &productcatalog.UnitConfig{ + Operation: productcatalog.UnitConfigOperationDivide, + ConversionFactor: alpacadecimal.NewFromInt(1000), + Rounding: productcatalog.UnitConfigRoundingModeCeiling, + } + + res, err := s.Charges.Create(ctx, charges.CreateInput{ + Namespace: ns, + Intents: []charges.ChargeIntent{ + s.createMockChargeIntent(createMockChargeIntentInput{ + customer: cust.GetID(), + currency: USD, + servicePeriod: servicePeriod, + settlementMode: productcatalog.CreditThenInvoiceSettlementMode, + price: productcatalog.NewPriceFrom(productcatalog.UnitPrice{Amount: alpacadecimal.NewFromFloat(1)}), + unitConfig: unitConfig, + name: "usage-based-unit-config", + managedBy: billing.SubscriptionManagedLine, + uniqueReferenceID: "usage-based-unit-config", + featureKey: meterSlug, + }), + }, + }) + s.Require().NoError(err) + s.Require().Len(res, 1) + + // 7400 raw units. Flag on: ceil(7400/1000) = 8 billed units. + s.MockStreamingConnector.AddSimpleEvent( + meterSlug, + 7400, + datetime.MustParseTimeInLocation(s.T(), "2026-01-15T00:00:00Z", time.UTC).AsTime(), + ) + clock.FreezeTime(invoiceAt) + + invoices, err := s.BillingService.InvoicePendingLines(ctx, billing.InvoicePendingLinesInput{ + Customer: cust.GetID(), + AsOf: lo.ToPtr(invoiceAt), + }) + s.Require().NoError(err) + s.Require().Len(invoices, 1) + s.Require().Len(invoices[0].Lines.OrEmpty(), 1) + + stdLine := invoices[0].Lines.OrEmpty()[0] + + // The raw metered quantity is always 7400; unit_config only changes the priced + // amount in this ticket. (The displayed UsageBased.Quantity staying in raw units + // until the charges line-mapper also converts is a separate, later scope item.) + s.Require().NotNil(stdLine.UsageBased.MeteredQuantity) + s.Equal(float64(7400), lo.FromPtr(stdLine.UsageBased.MeteredQuantity).InexactFloat64()) + + s.RequireTotals(billingtest.ExpectedTotals{ + Amount: expectedAmount, + Total: expectedAmount, + }, stdLine.Totals) +} diff --git a/openmeter/billing/charges/testutils/service.go b/openmeter/billing/charges/testutils/service.go index 533455e7cc..ec226e42f4 100644 --- a/openmeter/billing/charges/testutils/service.go +++ b/openmeter/billing/charges/testutils/service.go @@ -153,7 +153,7 @@ func NewServices(t testing.TB, config Config) (*Services, error) { Lineage: lineageService, MetaAdapter: metaAdapter, Locker: locker, - RatingService: billingratingservice.New(), + RatingService: billingratingservice.New(billingratingservice.Config{}), }) if err != nil { return nil, fmt.Errorf("creating flat fee service: %w", err) @@ -180,7 +180,7 @@ func NewServices(t testing.TB, config Config) (*Services, error) { MetaAdapter: metaAdapter, CustomerOverrideService: config.BillingService, FeatureService: config.FeatureService, - RatingService: billingratingservice.New(), + RatingService: billingratingservice.New(billingratingservice.Config{}), StreamingConnector: config.StreamingConnector, }) if err != nil { @@ -211,7 +211,7 @@ func NewServices(t testing.TB, config Config) (*Services, error) { } creditPurchaseLineEngine, err := creditpurchaselineengine.New(creditpurchaselineengine.Config{ - RatingService: billingratingservice.New(), + RatingService: billingratingservice.New(billingratingservice.Config{}), }) if err != nil { return nil, fmt.Errorf("creating credit purchase line engine: %w", err) diff --git a/openmeter/billing/charges/usagebased/rating.go b/openmeter/billing/charges/usagebased/rating.go index aba7713e9f..9b50cbd61a 100644 --- a/openmeter/billing/charges/usagebased/rating.go +++ b/openmeter/billing/charges/usagebased/rating.go @@ -53,6 +53,14 @@ func (r RateableIntent) GetRateCardDiscounts() billing.Discounts { return r.Discounts.Clone() } +func (r RateableIntent) GetUnitConfig() *productcatalog.UnitConfig { + if r.UnitConfig == nil { + return nil + } + + return lo.ToPtr(r.UnitConfig.Clone()) +} + func (r RateableIntent) GetStandardLineDiscounts() billing.StandardLineDiscounts { // A charge is never associated with user defined line discounts return billing.StandardLineDiscounts{} diff --git a/openmeter/billing/charges/usagebased/service/rating/delta/base_test.go b/openmeter/billing/charges/usagebased/service/rating/delta/base_test.go index d5ff04ce42..aa534e1a0a 100644 --- a/openmeter/billing/charges/usagebased/service/rating/delta/base_test.go +++ b/openmeter/billing/charges/usagebased/service/rating/delta/base_test.go @@ -45,7 +45,7 @@ func runDeltaRatingTestCase(t *testing.T, tc deltaRatingTestCase) { } intent := ratingtestutils.NewIntentForTest(t, fullServicePeriod, tc.price, tc.discounts) - engine := New(billingratingservice.New()) + engine := New(billingratingservice.New(billingratingservice.Config{})) bookedDetailedLinesByPhase := make([]usagebased.DetailedLines, len(tc.phases)) for phaseIdx, phase := range tc.phases { diff --git a/openmeter/billing/charges/usagebased/service/rating/periodpreserving/engine_test.go b/openmeter/billing/charges/usagebased/service/rating/periodpreserving/engine_test.go index fbb1060b01..352a293df9 100644 --- a/openmeter/billing/charges/usagebased/service/rating/periodpreserving/engine_test.go +++ b/openmeter/billing/charges/usagebased/service/rating/periodpreserving/engine_test.go @@ -1173,7 +1173,7 @@ func TestRateRejectsOverlappingPriorPeriods(t *testing.T) { productcatalog.Discounts{}, ) - _, err := New(billingratingservice.New()).Rate(t.Context(), Input{ + _, err := New(billingratingservice.New(billingratingservice.Config{})).Rate(t.Context(), Input{ Intent: intent, PriorPeriods: []PriorPeriod{ { @@ -1210,7 +1210,7 @@ func TestRateRejectsPriorPeriodThatIsEmptyAtMinimumStreamingWindowSize(t *testin productcatalog.Discounts{}, ) - _, err := New(billingratingservice.New()).Rate(t.Context(), Input{ + _, err := New(billingratingservice.New(billingratingservice.Config{})).Rate(t.Context(), Input{ Intent: intent, PriorPeriods: []PriorPeriod{ { @@ -1242,7 +1242,7 @@ func runLateEventRatingTestCase(t *testing.T, tc lateEventRatingTestCase) { intent := ratingtestutils.NewIntentForTest(t, fullServicePeriod, tc.price, tc.discounts) - engine := New(billingratingservice.New()) + engine := New(billingratingservice.New(billingratingservice.Config{})) bookedDetailedLinesByPhase := make([]usagebased.DetailedLines, len(tc.phases)) phaseRunIDs := make([]usagebased.RealizationRunID, len(tc.phases)) diff --git a/openmeter/billing/charges/usagebased/service/rating/service_test.go b/openmeter/billing/charges/usagebased/service/rating/service_test.go index 28b272b566..bab26665d8 100644 --- a/openmeter/billing/charges/usagebased/service/rating/service_test.go +++ b/openmeter/billing/charges/usagebased/service/rating/service_test.go @@ -192,7 +192,7 @@ func TestGetDetailedRatingForUsageUsesPeriodPreservingRatingEngine(t *testing.T) svc, err := New(Config{ StreamingConnector: streamingConnector, - RatingService: billingratingservice.New(), + RatingService: billingratingservice.New(billingratingservice.Config{}), DetailedLinesFetcher: passthroughDetailedLinesFetcher, }) require.NoError(t, err) @@ -391,7 +391,7 @@ func TestGetTotalsForUsageMinimumCommitment(t *testing.T) { svc, err := New(Config{ StreamingConnector: streamingConnector, - RatingService: billingratingservice.New(), + RatingService: billingratingservice.New(billingratingservice.Config{}), DetailedLinesFetcher: passthroughDetailedLinesFetcher, }) require.NoError(t, err) diff --git a/openmeter/billing/charges/usagebased/service/rating/subtract/subtract_test.go b/openmeter/billing/charges/usagebased/service/rating/subtract/subtract_test.go index c443622471..9ad157c9f3 100644 --- a/openmeter/billing/charges/usagebased/service/rating/subtract/subtract_test.go +++ b/openmeter/billing/charges/usagebased/service/rating/subtract/subtract_test.go @@ -1252,7 +1252,7 @@ func runSubtractRatedRunDetailsTestCases( ) { t.Helper() - ratingService := billingratingservice.New() + ratingService := billingratingservice.New(billingratingservice.Config{}) for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { diff --git a/openmeter/billing/rating/line.go b/openmeter/billing/rating/line.go index 6e9fa8af38..de7caf0973 100644 --- a/openmeter/billing/rating/line.go +++ b/openmeter/billing/rating/line.go @@ -39,6 +39,9 @@ type StandardLineAccessor interface { GetName() string // GetRateCardDiscounts returns the rate card discounts for the line GetRateCardDiscounts() billing.Discounts + // GetUnitConfig returns the optional unit conversion to apply to the raw metered + // quantity before pricing. Nil means no conversion (rating is unchanged). + GetUnitConfig() *productcatalog.UnitConfig // GetStandardLineDiscounts returns the standard line discounts for the line GetStandardLineDiscounts() billing.StandardLineDiscounts diff --git a/openmeter/billing/rating/service/billableperiod.go b/openmeter/billing/rating/service/billableperiod.go index 47aab789e7..0faa013c82 100644 --- a/openmeter/billing/rating/service/billableperiod.go +++ b/openmeter/billing/rating/service/billableperiod.go @@ -14,7 +14,7 @@ func (s *service) ResolveBillablePeriod(in rating.ResolveBillablePeriodInput) (* return nil, err } - linePricer, err := getPricerFor(in.Line, rating.NewGenerateDetailedLinesOptions()) + linePricer, err := getPricerFor(in.Line, rating.NewGenerateDetailedLinesOptions(), s.unitConfigEnabled) if err != nil { return nil, err } diff --git a/openmeter/billing/rating/service/detailedline.go b/openmeter/billing/rating/service/detailedline.go index 4683f24416..e7f0b740c3 100644 --- a/openmeter/billing/rating/service/detailedline.go +++ b/openmeter/billing/rating/service/detailedline.go @@ -50,7 +50,7 @@ func (s *service) GenerateDetailedLines(in rating.StandardLineAccessor, opts ... generateOpts := rating.NewGenerateDetailedLinesOptions(opts...) - linePricer, err := getPricerFor(in, generateOpts) + linePricer, err := getPricerFor(in, generateOpts, s.unitConfigEnabled) if err != nil { return rating.GenerateDetailedLinesResult{}, fmt.Errorf("creating pricer: %w", err) } diff --git a/openmeter/billing/rating/service/mutator/unitconfig.go b/openmeter/billing/rating/service/mutator/unitconfig.go new file mode 100644 index 0000000000..d124ae2f42 --- /dev/null +++ b/openmeter/billing/rating/service/mutator/unitconfig.go @@ -0,0 +1,54 @@ +package mutator + +import ( + "github.com/openmeterio/openmeter/openmeter/billing/rating/service/rate" +) + +// UnitConfig is a pre-calculation mutator that applies the rate card's unit_config +// conversion to the raw metered quantity before the pricer runs. It converts and +// rounds on the cumulative endpoints (start = pre-line-period quantity, end = pre + +// current) and writes the rounded billed quantity back as the per-line delta, so the +// downstream pricer sees converted units without ever knowing a conversion happened. +// +// Rounding is applied on the cumulative boundary rather than per line on purpose: +// rounding is non-linear, so rounding each line independently would double-bill +// across progressive splits (⌈1.4⌉ + ⌈1.3⌉ = 4 vs the correct ⌈2.7⌉ = 3). Computing +// the diff here also matters because the Unit pricer ignores PreLinePeriodQuantity. +// +// Unlike DiscountUsage this mutator stays idempotent: it only rewrites the in-memory +// Usage (rebuilt from the raw metered quantity on every run), not a persisted field, +// so re-rating reconverts from raw instead of double-converting. +type UnitConfig struct{} + +var _ PreCalculationMutator = (*UnitConfig)(nil) + +func (m *UnitConfig) Mutate(l rate.PricerCalculateInput) (rate.PricerCalculateInput, error) { + unitConfig := l.GetUnitConfig() + if unitConfig == nil { + return l, nil + } + + // Defensive: the authoring validator forbids a unit_config on price types that + // cannot convert (flat/package/dynamic), so reaching the mutator with an + // unsupported price means inconsistent data. Skip conversion rather than + // mis-price, leaving the pricer to bill the raw quantity as it does today. + price := l.GetPrice() + if price == nil || !price.SupportsUnitConfig() { + return l, nil + } + + usage, err := l.GetUsage() + if err != nil { + return l, err + } + + _, start := unitConfig.Apply(usage.PreLinePeriodQuantity) + _, end := unitConfig.Apply(usage.PreLinePeriodQuantity.Add(usage.Quantity)) + + usage.PreLinePeriodQuantity = start + usage.Quantity = end.Sub(start) + + l.Usage = &usage + + return l, nil +} diff --git a/openmeter/billing/rating/service/mutator/unitconfig_test.go b/openmeter/billing/rating/service/mutator/unitconfig_test.go new file mode 100644 index 0000000000..3fd49ffcd2 --- /dev/null +++ b/openmeter/billing/rating/service/mutator/unitconfig_test.go @@ -0,0 +1,159 @@ +package mutator + +import ( + "testing" + + "github.com/alpacahq/alpacadecimal" + "github.com/stretchr/testify/require" + + "github.com/openmeterio/openmeter/openmeter/billing" + "github.com/openmeterio/openmeter/openmeter/billing/rating" + "github.com/openmeterio/openmeter/openmeter/billing/rating/service/rate" + "github.com/openmeterio/openmeter/openmeter/productcatalog" +) + +const ( + opMultiply = productcatalog.UnitConfigOperationMultiply + opDivide = productcatalog.UnitConfigOperationDivide + + roundCeiling = productcatalog.UnitConfigRoundingModeCeiling + roundFloor = productcatalog.UnitConfigRoundingModeFloor + roundHalfUp = productcatalog.UnitConfigRoundingModeHalfUp + roundNone = productcatalog.UnitConfigRoundingModeNone +) + +// unitConfigTestLine is a StandardLineAccessor whose price and unit_config are +// overridable; the embedded StandardLine satisfies the rest of the interface. We +// use it (rather than a real RateableIntent) so a single fixture can exercise the +// Pre-populated cumulative case, which the charges path never produces at rating time. +type unitConfigTestLine struct { + *billing.StandardLine + + price *productcatalog.Price + unitConfig *productcatalog.UnitConfig +} + +func (l unitConfigTestLine) GetPrice() *productcatalog.Price { return l.price } +func (l unitConfigTestLine) GetUnitConfig() *productcatalog.UnitConfig { return l.unitConfig } + +func newUnitConfig(op productcatalog.UnitConfigOperation, factor float64, rounding productcatalog.UnitConfigRoundingMode) *productcatalog.UnitConfig { + return &productcatalog.UnitConfig{ + Operation: op, + ConversionFactor: alpacadecimal.NewFromFloat(factor), + Rounding: rounding, + } +} + +func mutateUnitConfig(t *testing.T, price *productcatalog.Price, unitConfig *productcatalog.UnitConfig, quantity, preLinePeriod float64) rating.Usage { + t.Helper() + + input := rate.PricerCalculateInput{ + StandardLineAccessor: unitConfigTestLine{ + StandardLine: &billing.StandardLine{}, + price: price, + unitConfig: unitConfig, + }, + Usage: &rating.Usage{ + Quantity: alpacadecimal.NewFromFloat(quantity), + PreLinePeriodQuantity: alpacadecimal.NewFromFloat(preLinePeriod), + }, + } + + out, err := (&UnitConfig{}).Mutate(input) + require.NoError(t, err) + + usage, err := out.GetUsage() + require.NoError(t, err) + + return usage +} + +func TestUnitConfigMutator(t *testing.T) { + unitPrice := productcatalog.NewPriceFrom(productcatalog.UnitPrice{Amount: alpacadecimal.NewFromInt(1)}) + + t.Run("no unit_config is a no-op", func(t *testing.T) { + usage := mutateUnitConfig(t, unitPrice, nil, 1400, 0) + require.Equal(t, float64(1400), usage.Quantity.InexactFloat64()) + require.Equal(t, float64(0), usage.PreLinePeriodQuantity.InexactFloat64()) + }) + + t.Run("conversion and rounding modes", func(t *testing.T) { + // given a single-period line (Pre = 0), the billed quantity is round(convert(Quantity)). + cases := []struct { + name string + cfg *productcatalog.UnitConfig + quantity float64 + expectQuantity float64 + }{ + {"multiply, no rounding", newUnitConfig(opMultiply, 1.2, roundNone), 10, 12}, + {"divide, ceiling rounds up", newUnitConfig(opDivide, 1000, roundCeiling), 1400, 2}, + {"divide, floor rounds down", newUnitConfig(opDivide, 1000, roundFloor), 1900, 1}, + {"divide, half_up rounds half away from zero", newUnitConfig(opDivide, 1000, roundHalfUp), 1500, 2}, + {"divide, half_up below half rounds down", newUnitConfig(opDivide, 1000, roundHalfUp), 1400, 1}, + {"divide, none keeps precision", newUnitConfig(opDivide, 1000, roundNone), 1400, 1.4}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + usage := mutateUnitConfig(t, unitPrice, tc.cfg, tc.quantity, 0) + require.Equal(t, tc.expectQuantity, usage.Quantity.InexactFloat64()) + require.Equal(t, float64(0), usage.PreLinePeriodQuantity.InexactFloat64()) + }) + } + }) + + t.Run("rounds on cumulative endpoints across a progressive split", func(t *testing.T) { + // given: + // - the second line of a split, cumulative 1400 -> 2700, divide by 1000, ceiling. + // when: + // - the mutator converts and rounds the cumulative endpoints. + // then: + // - start' = ceil(1.4) = 2, end' = ceil(2.7) = 3, so the billed delta is 1. + // Rounding per line would wrongly bill ceil(1.4) + ceil(1.3) = 4. + usage := mutateUnitConfig(t, unitPrice, newUnitConfig(opDivide, 1000, roundCeiling), 1300, 1400) + require.Equal(t, float64(1), usage.Quantity.InexactFloat64()) + require.Equal(t, float64(2), usage.PreLinePeriodQuantity.InexactFloat64()) + }) + + t.Run("unsupported price type is a no-op", func(t *testing.T) { + // A package price cannot carry a unit_config (the validator blocks it); if one + // slips through, the mutator must leave the raw quantity untouched. + packagePrice := productcatalog.NewPriceFrom(productcatalog.PackagePrice{ + Amount: alpacadecimal.NewFromInt(10), + QuantityPerPackage: alpacadecimal.NewFromInt(1000), + }) + usage := mutateUnitConfig(t, packagePrice, newUnitConfig(opDivide, 1000, roundCeiling), 1400, 0) + require.Equal(t, float64(1400), usage.Quantity.InexactFloat64()) + require.Equal(t, float64(0), usage.PreLinePeriodQuantity.InexactFloat64()) + }) + + t.Run("does not mutate the caller's raw usage when re-rated", func(t *testing.T) { + // Re-rating reads from the raw metered quantity each run; the mutator must not + // double-convert, which it guarantees by never mutating the input usage in place. + input := rate.PricerCalculateInput{ + StandardLineAccessor: unitConfigTestLine{ + StandardLine: &billing.StandardLine{}, + price: unitPrice, + unitConfig: newUnitConfig(opDivide, 1000, roundCeiling), + }, + Usage: &rating.Usage{ + Quantity: alpacadecimal.NewFromFloat(1400), + PreLinePeriodQuantity: alpacadecimal.NewFromFloat(0), + }, + } + + out1, err := (&UnitConfig{}).Mutate(input) + require.NoError(t, err) + out2, err := (&UnitConfig{}).Mutate(input) + require.NoError(t, err) + + u1, err := out1.GetUsage() + require.NoError(t, err) + u2, err := out2.GetUsage() + require.NoError(t, err) + + require.Equal(t, float64(2), u1.Quantity.InexactFloat64()) + require.Equal(t, u1.Quantity.InexactFloat64(), u2.Quantity.InexactFloat64()) + require.Equal(t, float64(1400), input.Usage.Quantity.InexactFloat64(), "caller's raw usage stays untouched") + }) +} diff --git a/openmeter/billing/rating/service/pricer.go b/openmeter/billing/rating/service/pricer.go index 140f08e8c1..71c5cdc7e6 100644 --- a/openmeter/billing/rating/service/pricer.go +++ b/openmeter/billing/rating/service/pricer.go @@ -11,7 +11,7 @@ import ( "github.com/openmeterio/openmeter/pkg/timeutil" ) -func getPricerFor(line rating.PriceAccessor, opts rating.GenerateDetailedLinesOptions) (*priceMutator, error) { +func getPricerFor(line rating.PriceAccessor, opts rating.GenerateDetailedLinesOptions, unitConfigEnabled bool) (*priceMutator, error) { if line == nil { return nil, errors.New("line is nil") } @@ -70,11 +70,18 @@ func getPricerFor(line rating.PriceAccessor, opts rating.GenerateDetailedLinesOp postCalculationMutators = append(postCalculationMutators, &mutator.Credits{}) } + // UnitConfig converts the raw metered quantity into billed units and must run + // before DiscountUsage so the usage discount applies to the converted, rounded + // quantity (convert → round → discount). + preCalculationMutators := make([]mutator.PreCalculationMutator, 0, 2) + if unitConfigEnabled { + preCalculationMutators = append(preCalculationMutators, &mutator.UnitConfig{}) + } + preCalculationMutators = append(preCalculationMutators, &mutator.DiscountUsage{}) + // This priceMutator captures the calculation flow for discounts and commitments: return &priceMutator{ - PreCalculation: []mutator.PreCalculationMutator{ - &mutator.DiscountUsage{}, - }, + PreCalculation: preCalculationMutators, Pricer: basePricer, PostCalculation: postCalculationMutators, }, nil diff --git a/openmeter/billing/rating/service/rate/unitconfig_rating_test.go b/openmeter/billing/rating/service/rate/unitconfig_rating_test.go new file mode 100644 index 0000000000..4c31bab8b6 --- /dev/null +++ b/openmeter/billing/rating/service/rate/unitconfig_rating_test.go @@ -0,0 +1,155 @@ +package rate_test + +import ( + "testing" + + "github.com/alpacahq/alpacadecimal" + "github.com/samber/lo" + + "github.com/openmeterio/openmeter/openmeter/billing" + "github.com/openmeterio/openmeter/openmeter/billing/models/totals" + "github.com/openmeterio/openmeter/openmeter/billing/rating" + "github.com/openmeterio/openmeter/openmeter/billing/rating/service/testutil" + "github.com/openmeterio/openmeter/openmeter/productcatalog" +) + +// TestUnitConfigRating exercises the unit_config conversion end-to-end through the +// real rating service and unit pricer. +func TestUnitConfigRating(t *testing.T) { + unitPrice := *productcatalog.NewPriceFrom(productcatalog.UnitPrice{Amount: alpacadecimal.NewFromFloat(10)}) + + divideCeiling := &productcatalog.UnitConfig{ + Operation: productcatalog.UnitConfigOperationDivide, + ConversionFactor: alpacadecimal.NewFromInt(1000), + Rounding: productcatalog.UnitConfigRoundingModeCeiling, + } + + t.Run("flag on bills the converted, rounded quantity", func(t *testing.T) { + // 1400 raw / 1000 = 1.4, ceiling -> 2 billed units at 10 = 20. + testutil.RunCalculationTestCase(t, testutil.CalculationTestCase{ + Price: unitPrice, + UnitConfig: divideCeiling, + UnitConfigEnabled: true, + LineMode: testutil.SinglePerPeriodLineMode, + Usage: testutil.FeatureUsageResponse{LinePeriodQty: alpacadecimal.NewFromFloat(1400)}, + Expect: rating.DetailedLines{ + { + Name: "feature: usage in period", + PerUnitAmount: alpacadecimal.NewFromFloat(10), + Quantity: alpacadecimal.NewFromFloat(2), + ChildUniqueReferenceID: rating.UnitPriceUsageChildUniqueReferenceID, + PaymentTerm: productcatalog.InArrearsPaymentTerm, + Totals: totals.Totals{ + Amount: alpacadecimal.NewFromFloat(20), + Total: alpacadecimal.NewFromFloat(20), + }, + }, + }, + }) + }) + + t.Run("flag off bills the raw quantity (parity with today)", func(t *testing.T) { + testutil.RunCalculationTestCase(t, testutil.CalculationTestCase{ + Price: unitPrice, + UnitConfig: divideCeiling, + UnitConfigEnabled: false, + LineMode: testutil.SinglePerPeriodLineMode, + Usage: testutil.FeatureUsageResponse{LinePeriodQty: alpacadecimal.NewFromFloat(1400)}, + Expect: rating.DetailedLines{ + { + Name: "feature: usage in period", + PerUnitAmount: alpacadecimal.NewFromFloat(10), + Quantity: alpacadecimal.NewFromFloat(1400), + ChildUniqueReferenceID: rating.UnitPriceUsageChildUniqueReferenceID, + PaymentTerm: productcatalog.InArrearsPaymentTerm, + Totals: totals.Totals{ + Amount: alpacadecimal.NewFromFloat(14000), + Total: alpacadecimal.NewFromFloat(14000), + }, + }, + }, + }) + }) + + t.Run("graduated tiers operate in converted units", func(t *testing.T) { + // Meter is in bytes, tiers are authored in GB. 7400 bytes / 1000 = 7.4 -> ceil 8 GB. + // Graduated over [0, 8]: tier 1 [0,5] @ 1 = 5, tier 2 (open) [5,8] @ 0.5 = 1.5. + gradPrice := *productcatalog.NewPriceFrom(productcatalog.TieredPrice{ + Mode: productcatalog.GraduatedTieredPrice, + Tiers: []productcatalog.PriceTier{ + { + UpToAmount: lo.ToPtr(alpacadecimal.NewFromFloat(5)), + UnitPrice: &productcatalog.PriceTierUnitPrice{Amount: alpacadecimal.NewFromFloat(1)}, + }, + { + UnitPrice: &productcatalog.PriceTierUnitPrice{Amount: alpacadecimal.NewFromFloat(0.5)}, + }, + }, + }) + + testutil.RunCalculationTestCase(t, testutil.CalculationTestCase{ + Price: gradPrice, + UnitConfig: divideCeiling, + UnitConfigEnabled: true, + LineMode: testutil.SinglePerPeriodLineMode, + Usage: testutil.FeatureUsageResponse{LinePeriodQty: alpacadecimal.NewFromFloat(7400)}, + Expect: rating.DetailedLines{ + { + Name: "feature: usage price for tier 1", + PerUnitAmount: alpacadecimal.NewFromFloat(1), + Quantity: alpacadecimal.NewFromFloat(5), + ChildUniqueReferenceID: "graduated-tiered-1-price-usage", + PaymentTerm: productcatalog.InArrearsPaymentTerm, + Totals: totals.Totals{ + Amount: alpacadecimal.NewFromFloat(5), + Total: alpacadecimal.NewFromFloat(5), + }, + }, + { + Name: "feature: usage price for tier 2", + PerUnitAmount: alpacadecimal.NewFromFloat(0.5), + Quantity: alpacadecimal.NewFromFloat(3), + ChildUniqueReferenceID: "graduated-tiered-2-price-usage", + PaymentTerm: productcatalog.InArrearsPaymentTerm, + Totals: totals.Totals{ + Amount: alpacadecimal.NewFromFloat(1.5), + Total: alpacadecimal.NewFromFloat(1.5), + }, + }, + }, + }) + }) + + t.Run("convert then round then discount: usage discount applies to the rounded quantity", func(t *testing.T) { + // 1400 / 1000 = 1.4 -> ceil 2; a 0.5 (converted-unit) usage discount -> 1.5 at 10 = 15. + // Deliberately not ceil(1.4 - 0.5) = 1: the discount is against the rounded, displayed quantity. + testutil.RunCalculationTestCase(t, testutil.CalculationTestCase{ + Price: unitPrice, + UnitConfig: divideCeiling, + UnitConfigEnabled: true, + LineMode: testutil.SinglePerPeriodLineMode, + Usage: testutil.FeatureUsageResponse{LinePeriodQty: alpacadecimal.NewFromFloat(1400)}, + Discounts: billing.Discounts{ + Usage: &billing.UsageDiscount{ + UsageDiscount: productcatalog.UsageDiscount{ + Quantity: alpacadecimal.NewFromFloat(0.5), + }, + CorrelationID: "01ARZ3NDEKTSV4RRFFQ69G5FAV", + }, + }, + Expect: rating.DetailedLines{ + { + Name: "feature: usage in period", + PerUnitAmount: alpacadecimal.NewFromFloat(10), + Quantity: alpacadecimal.NewFromFloat(1.5), + ChildUniqueReferenceID: rating.UnitPriceUsageChildUniqueReferenceID, + PaymentTerm: productcatalog.InArrearsPaymentTerm, + Totals: totals.Totals{ + Amount: alpacadecimal.NewFromFloat(15), + Total: alpacadecimal.NewFromFloat(15), + }, + }, + }, + }) + }) +} diff --git a/openmeter/billing/rating/service/service.go b/openmeter/billing/rating/service/service.go index 16aa71972b..6864b9d386 100644 --- a/openmeter/billing/rating/service/service.go +++ b/openmeter/billing/rating/service/service.go @@ -2,8 +2,19 @@ package service import "github.com/openmeterio/openmeter/openmeter/billing/rating" -type service struct{} +// Config carries the deploy-wide rating configuration. +type Config struct { + // UnitConfigEnabled gates the unit_config pre-calculation mutator. When false, + // rating output is byte-identical to having no unit_config on the rate card. + UnitConfigEnabled bool +} + +type service struct { + unitConfigEnabled bool +} -func New() rating.Service { - return &service{} +func New(cfg Config) rating.Service { + return &service{ + unitConfigEnabled: cfg.UnitConfigEnabled, + } } diff --git a/openmeter/billing/rating/service/testutil/ubptest.go b/openmeter/billing/rating/service/testutil/ubptest.go index 888de645d9..ed5961fb11 100644 --- a/openmeter/billing/rating/service/testutil/ubptest.go +++ b/openmeter/billing/rating/service/testutil/ubptest.go @@ -46,6 +46,26 @@ type CalculationTestCase struct { PreviousBilledAmount alpacadecimal.Decimal CreditsApplied billing.CreditsApplied Options []rating.GenerateDetailedLinesOption + + // UnitConfig, when set, is exposed on the rated line via GetUnitConfig. Standard + // invoice lines do not carry a unit_config in production (only the charges path + // does), so the harness injects it through a wrapper accessor for testing. + UnitConfig *productcatalog.UnitConfig + // UnitConfigEnabled toggles the deploy-wide unitConfig.enabled flag on the rating + // service under test. + UnitConfigEnabled bool +} + +// unitConfigLineAccessor wraps a StandardLine to expose a unit_config, which the +// StandardLine type itself never carries. +type unitConfigLineAccessor struct { + *billing.StandardLine + + unitConfig *productcatalog.UnitConfig +} + +func (l unitConfigLineAccessor) GetUnitConfig() *productcatalog.UnitConfig { + return l.unitConfig } type Service interface { @@ -123,9 +143,14 @@ func RunCalculationTestCase(t *testing.T, tc CalculationTestCase) { line.UsageBased.PreLinePeriodQuantity = &tc.Usage.PreLinePeriodQty line.UsageBased.MeteredPreLinePeriodQuantity = &tc.Usage.PreLinePeriodQty - service := service.New() + var accessor rating.StandardLineAccessor = line + if tc.UnitConfig != nil { + accessor = unitConfigLineAccessor{StandardLine: line, unitConfig: tc.UnitConfig} + } + + svc := service.New(service.Config{UnitConfigEnabled: tc.UnitConfigEnabled}) - res, err := service.GenerateDetailedLines(line, tc.Options...) + res, err := svc.GenerateDetailedLines(accessor, tc.Options...) if err != nil { if tc.ExpectErrorIs != nil { require.ErrorIs(t, err, tc.ExpectErrorIs) diff --git a/openmeter/billing/rating/service/unitconfig_registration_test.go b/openmeter/billing/rating/service/unitconfig_registration_test.go new file mode 100644 index 0000000000..7d77f9afd3 --- /dev/null +++ b/openmeter/billing/rating/service/unitconfig_registration_test.go @@ -0,0 +1,48 @@ +package service + +import ( + "testing" + + "github.com/alpacahq/alpacadecimal" + "github.com/stretchr/testify/require" + + "github.com/openmeterio/openmeter/openmeter/billing" + "github.com/openmeterio/openmeter/openmeter/billing/rating" + "github.com/openmeterio/openmeter/openmeter/billing/rating/service/mutator" + "github.com/openmeterio/openmeter/openmeter/productcatalog" +) + +func TestGetPricerForUnitConfigRegistration(t *testing.T) { + unitLine := &billing.StandardLine{ + UsageBased: &billing.UsageBasedLine{ + Price: productcatalog.NewPriceFrom(productcatalog.UnitPrice{Amount: alpacadecimal.NewFromInt(1)}), + }, + } + + t.Run("flag off keeps only DiscountUsage (parity with today)", func(t *testing.T) { + pm, err := getPricerFor(unitLine, rating.NewGenerateDetailedLinesOptions(), false) + require.NoError(t, err) + require.Len(t, pm.PreCalculation, 1) + require.IsType(t, &mutator.DiscountUsage{}, pm.PreCalculation[0]) + }) + + t.Run("flag on registers UnitConfig before DiscountUsage", func(t *testing.T) { + pm, err := getPricerFor(unitLine, rating.NewGenerateDetailedLinesOptions(), true) + require.NoError(t, err) + require.Len(t, pm.PreCalculation, 2) + require.IsType(t, &mutator.UnitConfig{}, pm.PreCalculation[0]) + require.IsType(t, &mutator.DiscountUsage{}, pm.PreCalculation[1]) + }) + + t.Run("flat price has no pre-calculation mutators regardless of flag", func(t *testing.T) { + flatLine := &billing.StandardLine{ + UsageBased: &billing.UsageBasedLine{ + Price: productcatalog.NewPriceFrom(productcatalog.FlatPrice{Amount: alpacadecimal.NewFromInt(1)}), + }, + } + + pm, err := getPricerFor(flatLine, rating.NewGenerateDetailedLinesOptions(), true) + require.NoError(t, err) + require.Empty(t, pm.PreCalculation) + }) +} diff --git a/openmeter/billing/stdinvoiceline.go b/openmeter/billing/stdinvoiceline.go index b0e803e00d..9dcfca11a6 100644 --- a/openmeter/billing/stdinvoiceline.go +++ b/openmeter/billing/stdinvoiceline.go @@ -588,6 +588,22 @@ func (i StandardLine) GetRateCardDiscounts() Discounts { return i.RateCardDiscounts } +// GetUnitConfig returns nil for standard invoice lines today because the standard +// line does not yet carry a unit_config snapshot. OM-395 persisted unit_config only +// on the shared rate card and on the charge intent, so at rating time the conversion +// reaches us solely via the charges path (RateableIntent). The design intends the +// conversion to apply on the legacy/standard-line path too (the rating mutator already +// computes the Pre-populated cumulative case for it), but the source field on this line +// is a separate, later ticket. +// +// TODO(unit-config seam ④ / W4 — invoice-line applied_unit_config snapshot): when +// UsageBasedLine gains the write-once AppliedUnitConfig snapshot, return it here instead +// of nil. Until then a unit_config rate card must not be billed through the legacy path, +// or rating silently bills raw quantities (overbilling by the conversion factor). +func (i StandardLine) GetUnitConfig() *productcatalog.UnitConfig { + return nil +} + func (i StandardLine) GetServicePeriod() timeutil.ClosedPeriod { return timeutil.ClosedPeriod{ From: i.Period.From, diff --git a/openmeter/ledger/customerbalance/testenv_test.go b/openmeter/ledger/customerbalance/testenv_test.go index 0d6e711c94..2fb7f2764f 100644 --- a/openmeter/ledger/customerbalance/testenv_test.go +++ b/openmeter/ledger/customerbalance/testenv_test.go @@ -229,7 +229,7 @@ func newTestEnv(t *testing.T) *testEnv { Lineage: lineageService, MetaAdapter: metaAdapter, Locker: locker, - RatingService: billingratingservice.New(), + RatingService: billingratingservice.New(billingratingservice.Config{}), }) require.NoError(t, err) @@ -249,7 +249,7 @@ func newTestEnv(t *testing.T) *testEnv { MetaAdapter: metaAdapter, CustomerOverrideService: billingService, FeatureService: featureService, - RatingService: billingratingservice.New(), + RatingService: billingratingservice.New(billingratingservice.Config{}), StreamingConnector: streaming, }) require.NoError(t, err) diff --git a/openmeter/productcatalog/price.go b/openmeter/productcatalog/price.go index bb3fc0b682..84a1f76550 100644 --- a/openmeter/productcatalog/price.go +++ b/openmeter/productcatalog/price.go @@ -289,6 +289,16 @@ func (p *Price) Type() PriceType { return p.t } +// SupportsUnitConfig reports whether a unit_config conversion can be applied to +// this price. unit_config is a per-quantity conversion applied to the raw metered +// quantity before rating, so it is only meaningful on per-unit prices (unit and +// the graduated/volume tiered modes). Flat prices are not per-quantity, and +// package/dynamic prices already carry their own conversion that unit_config would +// double up on. +func (p *Price) SupportsUnitConfig() bool { + return p.t == UnitPriceType || p.t == TieredPriceType +} + func (p *Price) AsFlat() (FlatPrice, error) { if p.t == "" || p.flat == nil { return FlatPrice{}, errors.New("invalid FlatPrice: not initialized") diff --git a/openmeter/productcatalog/price_test.go b/openmeter/productcatalog/price_test.go index 2538b4027f..b0760036a6 100644 --- a/openmeter/productcatalog/price_test.go +++ b/openmeter/productcatalog/price_test.go @@ -81,6 +81,46 @@ func TestPrice_JSON(t *testing.T) { } } +func TestPriceSupportsUnitConfig(t *testing.T) { + tests := []struct { + Name string + Price *Price + Expected bool + }{ + { + Name: "unit price supports unit config", + Price: NewPriceFrom(UnitPrice{Amount: decimal.NewFromInt(1)}), + Expected: true, + }, + { + Name: "tiered price supports unit config", + Price: NewPriceFrom(TieredPrice{Mode: VolumeTieredPrice}), + Expected: true, + }, + { + Name: "flat price does not support unit config", + Price: NewPriceFrom(FlatPrice{Amount: decimal.NewFromInt(1)}), + Expected: false, + }, + { + Name: "package price does not support unit config", + Price: NewPriceFrom(PackagePrice{Amount: decimal.NewFromInt(1), QuantityPerPackage: decimal.NewFromInt(1000)}), + Expected: false, + }, + { + Name: "dynamic price does not support unit config", + Price: NewPriceFrom(DynamicPrice{Multiplier: decimal.NewFromFloat(1.2)}), + Expected: false, + }, + } + + for _, test := range tests { + t.Run(test.Name, func(t *testing.T) { + assert.Equal(t, test.Expected, test.Price.SupportsUnitConfig()) + }) + } +} + func TestFlatPrice(t *testing.T) { t.Run("Validate", func(t *testing.T) { tests := []struct { diff --git a/openmeter/productcatalog/ratecard.go b/openmeter/productcatalog/ratecard.go index 609f92ebdd..d3b690f833 100644 --- a/openmeter/productcatalog/ratecard.go +++ b/openmeter/productcatalog/ratecard.go @@ -297,13 +297,9 @@ func (r RateCardMeta) Validate() error { )) } - // A unit_config is a per-quantity conversion applied before rating, so it is - // only valid on a unit or tiered (graduated/volume) price. It is rejected on - // flat prices (not per-quantity) and on package/dynamic prices (which already - // carry their own conversion and would otherwise convert twice). - priceAllowsUnitConfig := r.Price != nil && - (r.Price.Type() == UnitPriceType || r.Price.Type() == TieredPriceType) - if !priceAllowsUnitConfig { + // A unit_config is only valid on a price that supports a per-quantity + // conversion (unit or tiered); Price.SupportsUnitConfig owns that rule. + if r.Price == nil || !r.Price.SupportsUnitConfig() { errs = append(errs, ErrRateCardUnitConfigRequiresUsageBasedPrice) } } diff --git a/test/app/testenv.go b/test/app/testenv.go index 80962c2f65..9184e5a293 100644 --- a/test/app/testenv.go +++ b/test/app/testenv.go @@ -274,7 +274,7 @@ func InitBillingService(t *testing.T, ctx context.Context, in InitBillingService }) require.NoError(t, err) - billingRatingService := billingratingservice.New() + billingRatingService := billingratingservice.New(billingratingservice.Config{}) return billingservice.New(billingservice.Config{ Adapter: billingAdapter, diff --git a/test/billing/suite.go b/test/billing/suite.go index fc0f29155e..0e2caa6627 100644 --- a/test/billing/suite.go +++ b/test/billing/suite.go @@ -240,7 +240,7 @@ func (s *BaseSuite) setupSuite(opts SetupSuiteOptions) { billingService, err := billingservice.New(billingservice.Config{ Adapter: billingAdapter, - RatingService: billingratingservice.New(), + RatingService: billingratingservice.New(billingratingservice.Config{}), CustomerService: s.CustomerService, AppService: s.AppService, Logger: slog.Default(), diff --git a/test/customer/testenv.go b/test/customer/testenv.go index 79ac67a840..9e4ca2820a 100644 --- a/test/customer/testenv.go +++ b/test/customer/testenv.go @@ -407,7 +407,7 @@ func NewTestEnv(t *testing.T, ctx context.Context) (TestEnv, error) { billingService, err := billingservice.New(billingservice.Config{ Adapter: billingAdapter, - RatingService: billingratingservice.New(), + RatingService: billingratingservice.New(billingratingservice.Config{}), CustomerService: customerService, AppService: appService, Logger: logger.WithGroup("billing"), diff --git a/test/subscription/framework_test.go b/test/subscription/framework_test.go index 2971f7a30e..7fea90227b 100644 --- a/test/subscription/framework_test.go +++ b/test/subscription/framework_test.go @@ -99,7 +99,7 @@ func setup(t *testing.T, _ setupConfig) testDeps { billingService, err := billingservice.New(billingservice.Config{ Adapter: billingAdapter, - RatingService: billingratingservice.New(), + RatingService: billingratingservice.New(billingratingservice.Config{}), CustomerService: deps.CustomerService, AppService: appService, Logger: slog.Default(), From c21482a184462458eaf0c8787c364a5df737a6af Mon Sep 17 00:00:00 2001 From: rolosp <6114043+rolosp@users.noreply.github.com> Date: Tue, 30 Jun 2026 19:06:17 +0200 Subject: [PATCH 2/6] test: cover progressive split in unit config rating --- .../service/rate/unitconfig_rating_test.go | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/openmeter/billing/rating/service/rate/unitconfig_rating_test.go b/openmeter/billing/rating/service/rate/unitconfig_rating_test.go index 4c31bab8b6..b8a6fd20ba 100644 --- a/openmeter/billing/rating/service/rate/unitconfig_rating_test.go +++ b/openmeter/billing/rating/service/rate/unitconfig_rating_test.go @@ -48,6 +48,35 @@ func TestUnitConfigRating(t *testing.T) { }) }) + t.Run("progressive split rounds on cumulative endpoints", func(t *testing.T) { + // Mid-period split with a non-zero pre-line quantity: cumulative 1400 -> 2700, + // divide by 1000, ceiling. start' = ceil(1.4) = 2, end' = ceil(2.7) = 3, so the + // billed delta is 1 -- NOT per-line ceil(1300/1000) = 2 -- at $10 = $10. + testutil.RunCalculationTestCase(t, testutil.CalculationTestCase{ + Price: unitPrice, + UnitConfig: divideCeiling, + UnitConfigEnabled: true, + LineMode: testutil.MidPeriodSplitLineMode, + Usage: testutil.FeatureUsageResponse{ + PreLinePeriodQty: alpacadecimal.NewFromFloat(1400), + LinePeriodQty: alpacadecimal.NewFromFloat(1300), + }, + Expect: rating.DetailedLines{ + { + Name: "feature: usage in period", + PerUnitAmount: alpacadecimal.NewFromFloat(10), + Quantity: alpacadecimal.NewFromFloat(1), + ChildUniqueReferenceID: rating.UnitPriceUsageChildUniqueReferenceID, + PaymentTerm: productcatalog.InArrearsPaymentTerm, + Totals: totals.Totals{ + Amount: alpacadecimal.NewFromFloat(10), + Total: alpacadecimal.NewFromFloat(10), + }, + }, + }, + }) + }) + t.Run("flag off bills the raw quantity (parity with today)", func(t *testing.T) { testutil.RunCalculationTestCase(t, testutil.CalculationTestCase{ Price: unitPrice, From d3e00c8f9ffad99796d6d68fcd126b120e4c6ff3 Mon Sep 17 00:00:00 2001 From: rolosp <6114043+rolosp@users.noreply.github.com> Date: Wed, 1 Jul 2026 15:59:44 +0200 Subject: [PATCH 3/6] fix: apply unit_config to subscription charges and fail on inconsistencies --- .../charges/service/unitconfig_rating_test.go | 66 +++++++++++-------- .../billing/charges/testutils/service.go | 6 +- .../billing/rating/service/mutator/errors.go | 16 +++++ .../service/mutator/forbidunitconfig.go | 24 +++++++ .../service/mutator/forbidunitconfig_test.go | 47 +++++++++++++ .../rating/service/mutator/unitconfig.go | 51 ++++++++------ .../rating/service/mutator/unitconfig_test.go | 38 +++++++---- openmeter/billing/rating/service/pricer.go | 6 +- .../service/rate/unitconfig_rating_test.go | 32 ++++++--- .../service/unitconfig_registration_test.go | 7 +- openmeter/billing/stdinvoiceline.go | 5 +- .../reconciler/patchchargeusagebased.go | 18 +++-- .../reconciler/patchchargeusagebased_test.go | 50 ++++++++++++++ .../ledger/customerbalance/testenv_test.go | 4 +- test/app/testenv.go | 6 +- test/billing/suite.go | 2 +- test/customer/testenv.go | 2 +- test/subscription/framework_test.go | 2 +- 18 files changed, 291 insertions(+), 91 deletions(-) create mode 100644 openmeter/billing/rating/service/mutator/errors.go create mode 100644 openmeter/billing/rating/service/mutator/forbidunitconfig.go create mode 100644 openmeter/billing/rating/service/mutator/forbidunitconfig_test.go create mode 100644 openmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeusagebased_test.go diff --git a/openmeter/billing/charges/service/unitconfig_rating_test.go b/openmeter/billing/charges/service/unitconfig_rating_test.go index 987374c724..a0165bba39 100644 --- a/openmeter/billing/charges/service/unitconfig_rating_test.go +++ b/openmeter/billing/charges/service/unitconfig_rating_test.go @@ -10,6 +10,7 @@ import ( "github.com/openmeterio/openmeter/openmeter/billing" "github.com/openmeterio/openmeter/openmeter/billing/charges" + "github.com/openmeterio/openmeter/openmeter/billing/rating/service/mutator" "github.com/openmeterio/openmeter/openmeter/productcatalog" "github.com/openmeterio/openmeter/pkg/clock" "github.com/openmeterio/openmeter/pkg/datetime" @@ -42,7 +43,27 @@ func (s *unitConfigRatingEnabledSuite) TearDownTest() { func (s *unitConfigRatingEnabledSuite) TestRatesConvertedQuantity() { // flag on: 7400 raw / 1000, ceiling => 8 billed units * $1 = $8. - s.runUnitConfigChargesScenario(8) + invoices, err := s.invoiceUnitConfigChargesScenario() + s.Require().NoError(err) + s.Require().Len(invoices, 1) + s.Require().Len(invoices[0].Lines.OrEmpty(), 1) + + stdLine := invoices[0].Lines.OrEmpty()[0] + + // MeteredQuantity records the raw metered value and stays 7400: the unit_config + // conversion changes the priced amount (asserted below), not the recorded metered + // quantity. + // + // TODO: once the charges line-mapper converts+rounds the displayed UsageBased.Quantity, + // extend this test to also assert the converted displayed quantity. + // MeteredQuantity stays raw (7400) as the audit value. + s.Require().NotNil(stdLine.UsageBased.MeteredQuantity) + s.Equal(float64(7400), lo.FromPtr(stdLine.UsageBased.MeteredQuantity).InexactFloat64()) + + s.RequireTotals(billingtest.ExpectedTotals{ + Amount: 8, + Total: 8, + }, stdLine.Totals) } func TestUsageBasedUnitConfigRatingDisabled(t *testing.T) { @@ -54,8 +75,8 @@ type unitConfigRatingDisabledSuite struct { } func (s *unitConfigRatingDisabledSuite) SetupSuite() { - // UnitConfigEnabled defaults to false: the intent still carries a unit_config, - // but the mutator is not registered, so rating must bill the raw quantity. + // UnitConfigEnabled defaults to false: the intent still carries a unit_config, so + // ForbidUnitConfig must reject rating rather than silently bill the raw quantity. s.BaseSuite.SetupSuite() } @@ -63,15 +84,19 @@ func (s *unitConfigRatingDisabledSuite) TearDownTest() { s.BaseSuite.TearDownTest() } -func (s *unitConfigRatingDisabledSuite) TestRatesRawQuantityWhenFlagOff() { - // flag off: unit_config ignored, 7400 raw units * $1 = $7400 (parity with today). - s.runUnitConfigChargesScenario(7400) +func (s *unitConfigRatingDisabledSuite) TestErrorsWhenFlagOff() { + // flag off + a unit_config on the charge: ForbidUnitConfig surfaces the + // inconsistency instead of silently billing the raw quantity. + _, err := s.invoiceUnitConfigChargesScenario() + s.Require().Error(err) + s.Require().ErrorIs(err, mutator.ErrUnitConfigDisabled) } -// runUnitConfigChargesScenario creates a usage-based charge carrying a divide-by-1000 -// ceiling unit_config, seeds 7400 raw units, invoices mid-period, and asserts the -// rated line amount matches expectedAmount. -func (s *BaseSuite) runUnitConfigChargesScenario(expectedAmount float64) { +// invoiceUnitConfigChargesScenario creates a usage-based charge carrying a divide-by-1000 +// ceiling unit_config, seeds 7400 raw units, and invoices mid-period. It returns the +// InvoicePendingLines result so each suite asserts its own outcome (converted amount +// when the flag is on, a ForbidUnitConfig error when it is off). +func (s *BaseSuite) invoiceUnitConfigChargesScenario() ([]billing.StandardInvoice, error) { s.T().Helper() ctx := s.T().Context() @@ -83,7 +108,8 @@ func (s *BaseSuite) runUnitConfigChargesScenario(expectedAmount float64) { cust := s.CreateTestCustomer(ns, "test-subject") s.NotEmpty(cust.ID) - _ = s.ProvisionBillingProfile(ctx, ns, customInvoicing.App.GetID(), + _ = s.ProvisionBillingProfile( + ctx, ns, customInvoicing.App.GetID(), billingtest.WithProgressiveBilling(), billingtest.WithCollectionInterval(datetime.MustParseDuration(s.T(), "P2D")), billingtest.WithManualApproval(), @@ -141,24 +167,8 @@ func (s *BaseSuite) runUnitConfigChargesScenario(expectedAmount float64) { ) clock.FreezeTime(invoiceAt) - invoices, err := s.BillingService.InvoicePendingLines(ctx, billing.InvoicePendingLinesInput{ + return s.BillingService.InvoicePendingLines(ctx, billing.InvoicePendingLinesInput{ Customer: cust.GetID(), AsOf: lo.ToPtr(invoiceAt), }) - s.Require().NoError(err) - s.Require().Len(invoices, 1) - s.Require().Len(invoices[0].Lines.OrEmpty(), 1) - - stdLine := invoices[0].Lines.OrEmpty()[0] - - // The raw metered quantity is always 7400; unit_config only changes the priced - // amount in this ticket. (The displayed UsageBased.Quantity staying in raw units - // until the charges line-mapper also converts is a separate, later scope item.) - s.Require().NotNil(stdLine.UsageBased.MeteredQuantity) - s.Equal(float64(7400), lo.FromPtr(stdLine.UsageBased.MeteredQuantity).InexactFloat64()) - - s.RequireTotals(billingtest.ExpectedTotals{ - Amount: expectedAmount, - Total: expectedAmount, - }, stdLine.Totals) } diff --git a/openmeter/billing/charges/testutils/service.go b/openmeter/billing/charges/testutils/service.go index ec226e42f4..8fc2bbe95c 100644 --- a/openmeter/billing/charges/testutils/service.go +++ b/openmeter/billing/charges/testutils/service.go @@ -153,7 +153,7 @@ func NewServices(t testing.TB, config Config) (*Services, error) { Lineage: lineageService, MetaAdapter: metaAdapter, Locker: locker, - RatingService: billingratingservice.New(billingratingservice.Config{}), + RatingService: billingratingservice.New(billingratingservice.Config{UnitConfigEnabled: true}), }) if err != nil { return nil, fmt.Errorf("creating flat fee service: %w", err) @@ -180,7 +180,7 @@ func NewServices(t testing.TB, config Config) (*Services, error) { MetaAdapter: metaAdapter, CustomerOverrideService: config.BillingService, FeatureService: config.FeatureService, - RatingService: billingratingservice.New(billingratingservice.Config{}), + RatingService: billingratingservice.New(billingratingservice.Config{UnitConfigEnabled: true}), StreamingConnector: config.StreamingConnector, }) if err != nil { @@ -211,7 +211,7 @@ func NewServices(t testing.TB, config Config) (*Services, error) { } creditPurchaseLineEngine, err := creditpurchaselineengine.New(creditpurchaselineengine.Config{ - RatingService: billingratingservice.New(billingratingservice.Config{}), + RatingService: billingratingservice.New(billingratingservice.Config{UnitConfigEnabled: true}), }) if err != nil { return nil, fmt.Errorf("creating credit purchase line engine: %w", err) diff --git a/openmeter/billing/rating/service/mutator/errors.go b/openmeter/billing/rating/service/mutator/errors.go new file mode 100644 index 0000000000..944be3c163 --- /dev/null +++ b/openmeter/billing/rating/service/mutator/errors.go @@ -0,0 +1,16 @@ +package mutator + +import "errors" + +var ( + // ErrUnitConfigDisabled is returned by ForbidUnitConfig when a line carries a + // unit_config while the unitConfig feature is disabled. Rating the raw quantity + // would silently under/over-bill, so the inconsistency is surfaced instead. + ErrUnitConfigDisabled = errors.New("unit_config is set on the line but the unitConfig feature is disabled") + + // ErrUnitConfigUnsupportedPrice is returned by UnitConfig when a unit_config rides + // on a price type that cannot convert (flat/package/dynamic) or on a line with no + // price. The authoring validator blocks this, so reaching rating means inconsistent + // data that must not be billed as raw. + ErrUnitConfigUnsupportedPrice = errors.New("unit_config is set on a price that does not support unit conversion") +) diff --git a/openmeter/billing/rating/service/mutator/forbidunitconfig.go b/openmeter/billing/rating/service/mutator/forbidunitconfig.go new file mode 100644 index 0000000000..6853fa213b --- /dev/null +++ b/openmeter/billing/rating/service/mutator/forbidunitconfig.go @@ -0,0 +1,24 @@ +package mutator + +import ( + "fmt" + + "github.com/openmeterio/openmeter/openmeter/billing/rating/service/rate" +) + +// ForbidUnitConfig is the flag-off counterpart to UnitConfig: it is registered in +// the pre-calculation pipeline when unitConfig.enabled is false. For the common case +// (no unit_config on the line) it is a no-op, but it errors when a line does carry a +// unit_config, so a config that reaches rating while the feature is disabled surfaces +// as a hard failure instead of silently billing the raw metered quantity. +type ForbidUnitConfig struct{} + +var _ PreCalculationMutator = (*ForbidUnitConfig)(nil) + +func (m *ForbidUnitConfig) Mutate(l rate.PricerCalculateInput) (rate.PricerCalculateInput, error) { + if l.GetUnitConfig() != nil { + return l, fmt.Errorf("refusing to bill raw quantity: %w", ErrUnitConfigDisabled) + } + + return l, nil +} diff --git a/openmeter/billing/rating/service/mutator/forbidunitconfig_test.go b/openmeter/billing/rating/service/mutator/forbidunitconfig_test.go new file mode 100644 index 0000000000..fb33348693 --- /dev/null +++ b/openmeter/billing/rating/service/mutator/forbidunitconfig_test.go @@ -0,0 +1,47 @@ +package mutator + +import ( + "testing" + + "github.com/alpacahq/alpacadecimal" + "github.com/stretchr/testify/require" + + "github.com/openmeterio/openmeter/openmeter/billing" + "github.com/openmeterio/openmeter/openmeter/billing/rating" + "github.com/openmeterio/openmeter/openmeter/billing/rating/service/rate" + "github.com/openmeterio/openmeter/openmeter/productcatalog" +) + +func TestForbidUnitConfigMutator(t *testing.T) { + unitPrice := productcatalog.NewPriceFrom(productcatalog.UnitPrice{Amount: alpacadecimal.NewFromInt(1)}) + + newInput := func(unitConfig *productcatalog.UnitConfig) rate.PricerCalculateInput { + return rate.PricerCalculateInput{ + StandardLineAccessor: unitConfigTestLine{ + StandardLine: &billing.StandardLine{}, + price: unitPrice, + unitConfig: unitConfig, + }, + Usage: &rating.Usage{ + Quantity: alpacadecimal.NewFromFloat(1400), + PreLinePeriodQuantity: alpacadecimal.NewFromFloat(0), + }, + } + } + + t.Run("no unit_config is a no-op", func(t *testing.T) { + out, err := (&ForbidUnitConfig{}).Mutate(newInput(nil)) + require.NoError(t, err) + + usage, err := out.GetUsage() + require.NoError(t, err) + require.Equal(t, float64(1400), usage.Quantity.InexactFloat64()) + }) + + t.Run("unit_config present errors instead of billing raw", func(t *testing.T) { + // With the feature disabled a config must never be silently dropped: rating + // the raw quantity would under/over-bill, so surface it as an error. + _, err := (&ForbidUnitConfig{}).Mutate(newInput(newUnitConfig(opDivide, 1000, roundCeiling))) + require.ErrorIs(t, err, ErrUnitConfigDisabled) + }) +} diff --git a/openmeter/billing/rating/service/mutator/unitconfig.go b/openmeter/billing/rating/service/mutator/unitconfig.go index d124ae2f42..13060f3688 100644 --- a/openmeter/billing/rating/service/mutator/unitconfig.go +++ b/openmeter/billing/rating/service/mutator/unitconfig.go @@ -1,23 +1,29 @@ package mutator import ( + "fmt" + "github.com/openmeterio/openmeter/openmeter/billing/rating/service/rate" ) // UnitConfig is a pre-calculation mutator that applies the rate card's unit_config -// conversion to the raw metered quantity before the pricer runs. It converts and -// rounds on the cumulative endpoints (start = pre-line-period quantity, end = pre + -// current) and writes the rounded billed quantity back as the per-line delta, so the -// downstream pricer sees converted units without ever knowing a conversion happened. +// conversion to the raw metered quantity before the pricer runs, so the downstream +// pricer sees billing units (e.g. GB) instead of raw metered units (e.g. bytes) +// without ever knowing a conversion happened. // -// Rounding is applied on the cumulative boundary rather than per line on purpose: -// rounding is non-linear, so rounding each line independently would double-bill -// across progressive splits (⌈1.4⌉ + ⌈1.3⌉ = 4 vs the correct ⌈2.7⌉ = 3). Computing -// the diff here also matters because the Unit pricer ignores PreLinePeriodQuantity. +// It converts the current-line quantity and the pre-line-period quantity +// independently, each through UnitConfig.Apply, and writes back the rounded +// (invoiced) result. Both are converted so tiered/graduated pricers evaluate their +// tier boundaries in converted units. It deliberately does NOT reconstruct a +// cumulative total as pre-line-period + current: that identity does not hold for +// every meter aggregation (e.g. MAX/UNIQUE_COUNT) or for the charges path (where +// pre-line-period is always zero), so making split lines sum correctly under +// non-linear rounding is left to the invoice-line layer where the raw metered pair +// is available. // -// Unlike DiscountUsage this mutator stays idempotent: it only rewrites the in-memory -// Usage (rebuilt from the raw metered quantity on every run), not a persisted field, -// so re-rating reconverts from raw instead of double-converting. +// Idempotent: it only rewrites the in-memory Usage (rebuilt from the raw metered +// quantity on every run), never a persisted field, so re-rating reconverts from raw +// instead of double-converting. type UnitConfig struct{} var _ PreCalculationMutator = (*UnitConfig)(nil) @@ -28,13 +34,16 @@ func (m *UnitConfig) Mutate(l rate.PricerCalculateInput) (rate.PricerCalculateIn return l, nil } - // Defensive: the authoring validator forbids a unit_config on price types that - // cannot convert (flat/package/dynamic), so reaching the mutator with an - // unsupported price means inconsistent data. Skip conversion rather than - // mis-price, leaving the pricer to bill the raw quantity as it does today. + // The authoring validator forbids a unit_config on price types that cannot + // convert (flat/package/dynamic), so reaching the mutator with an unsupported + // price means inconsistent data. Surface it as an error rather than silently + // billing the raw quantity — a dropped conversion would under/over-bill. price := l.GetPrice() - if price == nil || !price.SupportsUnitConfig() { - return l, nil + if price == nil { + return l, fmt.Errorf("line has no price: %w", ErrUnitConfigUnsupportedPrice) + } + if !price.SupportsUnitConfig() { + return l, fmt.Errorf("price type %q: %w", price.Type(), ErrUnitConfigUnsupportedPrice) } usage, err := l.GetUsage() @@ -42,11 +51,11 @@ func (m *UnitConfig) Mutate(l rate.PricerCalculateInput) (rate.PricerCalculateIn return l, err } - _, start := unitConfig.Apply(usage.PreLinePeriodQuantity) - _, end := unitConfig.Apply(usage.PreLinePeriodQuantity.Add(usage.Quantity)) + _, quantity := unitConfig.Apply(usage.Quantity) + _, preLinePeriodQuantity := unitConfig.Apply(usage.PreLinePeriodQuantity) - usage.PreLinePeriodQuantity = start - usage.Quantity = end.Sub(start) + usage.Quantity = quantity + usage.PreLinePeriodQuantity = preLinePeriodQuantity l.Usage = &usage diff --git a/openmeter/billing/rating/service/mutator/unitconfig_test.go b/openmeter/billing/rating/service/mutator/unitconfig_test.go index 3fd49ffcd2..813caad11a 100644 --- a/openmeter/billing/rating/service/mutator/unitconfig_test.go +++ b/openmeter/billing/rating/service/mutator/unitconfig_test.go @@ -102,29 +102,45 @@ func TestUnitConfigMutator(t *testing.T) { } }) - t.Run("rounds on cumulative endpoints across a progressive split", func(t *testing.T) { + t.Run("converts quantity and pre-line-period independently", func(t *testing.T) { // given: - // - the second line of a split, cumulative 1400 -> 2700, divide by 1000, ceiling. + // - a split line carrying a non-zero pre-line-period, divide by 1000, ceiling. // when: - // - the mutator converts and rounds the cumulative endpoints. + // - the mutator converts each endpoint on its own (no cumulative delta). // then: - // - start' = ceil(1.4) = 2, end' = ceil(2.7) = 3, so the billed delta is 1. - // Rounding per line would wrongly bill ceil(1.4) + ceil(1.3) = 4. + // - Quantity = ceil(1300/1000) = 2 and PreLinePeriodQuantity = ceil(1400/1000) = 2, + // both in converted units so tiered pricers see converted tier boundaries. The + // retired cumulative-endpoint approach would instead have billed ceil(2.7)-ceil(1.4)=1; + // making split lines sum correctly under non-linear rounding is now the invoice + // layer's job, not the mutator's. usage := mutateUnitConfig(t, unitPrice, newUnitConfig(opDivide, 1000, roundCeiling), 1300, 1400) - require.Equal(t, float64(1), usage.Quantity.InexactFloat64()) + require.Equal(t, float64(2), usage.Quantity.InexactFloat64()) require.Equal(t, float64(2), usage.PreLinePeriodQuantity.InexactFloat64()) }) - t.Run("unsupported price type is a no-op", func(t *testing.T) { + t.Run("unsupported price type errors instead of billing raw", func(t *testing.T) { // A package price cannot carry a unit_config (the validator blocks it); if one - // slips through, the mutator must leave the raw quantity untouched. + // slips through, the mutator must surface the inconsistency rather than silently + // bill the raw quantity. packagePrice := productcatalog.NewPriceFrom(productcatalog.PackagePrice{ Amount: alpacadecimal.NewFromInt(10), QuantityPerPackage: alpacadecimal.NewFromInt(1000), }) - usage := mutateUnitConfig(t, packagePrice, newUnitConfig(opDivide, 1000, roundCeiling), 1400, 0) - require.Equal(t, float64(1400), usage.Quantity.InexactFloat64()) - require.Equal(t, float64(0), usage.PreLinePeriodQuantity.InexactFloat64()) + + input := rate.PricerCalculateInput{ + StandardLineAccessor: unitConfigTestLine{ + StandardLine: &billing.StandardLine{}, + price: packagePrice, + unitConfig: newUnitConfig(opDivide, 1000, roundCeiling), + }, + Usage: &rating.Usage{ + Quantity: alpacadecimal.NewFromFloat(1400), + PreLinePeriodQuantity: alpacadecimal.NewFromFloat(0), + }, + } + + _, err := (&UnitConfig{}).Mutate(input) + require.ErrorIs(t, err, ErrUnitConfigUnsupportedPrice) }) t.Run("does not mutate the caller's raw usage when re-rated", func(t *testing.T) { diff --git a/openmeter/billing/rating/service/pricer.go b/openmeter/billing/rating/service/pricer.go index 71c5cdc7e6..e9c51e9a05 100644 --- a/openmeter/billing/rating/service/pricer.go +++ b/openmeter/billing/rating/service/pricer.go @@ -72,10 +72,14 @@ func getPricerFor(line rating.PriceAccessor, opts rating.GenerateDetailedLinesOp // UnitConfig converts the raw metered quantity into billed units and must run // before DiscountUsage so the usage discount applies to the converted, rounded - // quantity (convert → round → discount). + // quantity (convert → round → discount). When the feature is disabled, + // ForbidUnitConfig takes its place and errors if a line unexpectedly carries a + // unit_config, so a dropped conversion surfaces instead of silently billing raw. preCalculationMutators := make([]mutator.PreCalculationMutator, 0, 2) if unitConfigEnabled { preCalculationMutators = append(preCalculationMutators, &mutator.UnitConfig{}) + } else { + preCalculationMutators = append(preCalculationMutators, &mutator.ForbidUnitConfig{}) } preCalculationMutators = append(preCalculationMutators, &mutator.DiscountUsage{}) diff --git a/openmeter/billing/rating/service/rate/unitconfig_rating_test.go b/openmeter/billing/rating/service/rate/unitconfig_rating_test.go index b8a6fd20ba..c29c98ccf6 100644 --- a/openmeter/billing/rating/service/rate/unitconfig_rating_test.go +++ b/openmeter/billing/rating/service/rate/unitconfig_rating_test.go @@ -9,6 +9,7 @@ import ( "github.com/openmeterio/openmeter/openmeter/billing" "github.com/openmeterio/openmeter/openmeter/billing/models/totals" "github.com/openmeterio/openmeter/openmeter/billing/rating" + "github.com/openmeterio/openmeter/openmeter/billing/rating/service/mutator" "github.com/openmeterio/openmeter/openmeter/billing/rating/service/testutil" "github.com/openmeterio/openmeter/openmeter/productcatalog" ) @@ -48,10 +49,11 @@ func TestUnitConfigRating(t *testing.T) { }) }) - t.Run("progressive split rounds on cumulative endpoints", func(t *testing.T) { - // Mid-period split with a non-zero pre-line quantity: cumulative 1400 -> 2700, - // divide by 1000, ceiling. start' = ceil(1.4) = 2, end' = ceil(2.7) = 3, so the - // billed delta is 1 -- NOT per-line ceil(1300/1000) = 2 -- at $10 = $10. + t.Run("progressive split converts each endpoint independently", func(t *testing.T) { + // Mid-period split with a non-zero pre-line quantity, divide by 1000, ceiling. + // The mutator converts this line's quantity on its own: ceil(1300/1000) = 2 at + // $10 = $20. It deliberately does NOT reconstruct the cumulative (which would give + // ceil(2.7)-ceil(1.4)=1); split-line rounding correctness is the invoice layer's job. testutil.RunCalculationTestCase(t, testutil.CalculationTestCase{ Price: unitPrice, UnitConfig: divideCeiling, @@ -65,22 +67,21 @@ func TestUnitConfigRating(t *testing.T) { { Name: "feature: usage in period", PerUnitAmount: alpacadecimal.NewFromFloat(10), - Quantity: alpacadecimal.NewFromFloat(1), + Quantity: alpacadecimal.NewFromFloat(2), ChildUniqueReferenceID: rating.UnitPriceUsageChildUniqueReferenceID, PaymentTerm: productcatalog.InArrearsPaymentTerm, Totals: totals.Totals{ - Amount: alpacadecimal.NewFromFloat(10), - Total: alpacadecimal.NewFromFloat(10), + Amount: alpacadecimal.NewFromFloat(20), + Total: alpacadecimal.NewFromFloat(20), }, }, }, }) }) - t.Run("flag off bills the raw quantity (parity with today)", func(t *testing.T) { + t.Run("flag off without a unit_config bills the raw quantity (parity with today)", func(t *testing.T) { testutil.RunCalculationTestCase(t, testutil.CalculationTestCase{ Price: unitPrice, - UnitConfig: divideCeiling, UnitConfigEnabled: false, LineMode: testutil.SinglePerPeriodLineMode, Usage: testutil.FeatureUsageResponse{LinePeriodQty: alpacadecimal.NewFromFloat(1400)}, @@ -100,6 +101,19 @@ func TestUnitConfigRating(t *testing.T) { }) }) + t.Run("flag off with a unit_config errors instead of billing raw", func(t *testing.T) { + // ForbidUnitConfig guards the disabled path: a line carrying a unit_config while + // the feature is off must surface rather than silently bill the raw quantity. + testutil.RunCalculationTestCase(t, testutil.CalculationTestCase{ + Price: unitPrice, + UnitConfig: divideCeiling, + UnitConfigEnabled: false, + LineMode: testutil.SinglePerPeriodLineMode, + Usage: testutil.FeatureUsageResponse{LinePeriodQty: alpacadecimal.NewFromFloat(1400)}, + ExpectErrorIs: mutator.ErrUnitConfigDisabled, + }) + }) + t.Run("graduated tiers operate in converted units", func(t *testing.T) { // Meter is in bytes, tiers are authored in GB. 7400 bytes / 1000 = 7.4 -> ceil 8 GB. // Graduated over [0, 8]: tier 1 [0,5] @ 1 = 5, tier 2 (open) [5,8] @ 0.5 = 1.5. diff --git a/openmeter/billing/rating/service/unitconfig_registration_test.go b/openmeter/billing/rating/service/unitconfig_registration_test.go index 7d77f9afd3..d672df8647 100644 --- a/openmeter/billing/rating/service/unitconfig_registration_test.go +++ b/openmeter/billing/rating/service/unitconfig_registration_test.go @@ -19,11 +19,12 @@ func TestGetPricerForUnitConfigRegistration(t *testing.T) { }, } - t.Run("flag off keeps only DiscountUsage (parity with today)", func(t *testing.T) { + t.Run("flag off registers ForbidUnitConfig before DiscountUsage", func(t *testing.T) { pm, err := getPricerFor(unitLine, rating.NewGenerateDetailedLinesOptions(), false) require.NoError(t, err) - require.Len(t, pm.PreCalculation, 1) - require.IsType(t, &mutator.DiscountUsage{}, pm.PreCalculation[0]) + require.Len(t, pm.PreCalculation, 2) + require.IsType(t, &mutator.ForbidUnitConfig{}, pm.PreCalculation[0]) + require.IsType(t, &mutator.DiscountUsage{}, pm.PreCalculation[1]) }) t.Run("flag on registers UnitConfig before DiscountUsage", func(t *testing.T) { diff --git a/openmeter/billing/stdinvoiceline.go b/openmeter/billing/stdinvoiceline.go index 9dcfca11a6..902a77b5af 100644 --- a/openmeter/billing/stdinvoiceline.go +++ b/openmeter/billing/stdinvoiceline.go @@ -592,9 +592,8 @@ func (i StandardLine) GetRateCardDiscounts() Discounts { // line does not yet carry a unit_config snapshot. OM-395 persisted unit_config only // on the shared rate card and on the charge intent, so at rating time the conversion // reaches us solely via the charges path (RateableIntent). The design intends the -// conversion to apply on the legacy/standard-line path too (the rating mutator already -// computes the Pre-populated cumulative case for it), but the source field on this line -// is a separate, later ticket. +// conversion to apply on the legacy/standard-line path too, but the source field on +// this line is a separate, later ticket. // // TODO(unit-config seam ④ / W4 — invoice-line applied_unit_config snapshot): when // UsageBasedLine gains the write-once AppliedUnitConfig snapshot, return it here instead diff --git a/openmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeusagebased.go b/openmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeusagebased.go index 2ecb21b95d..354afdb0d8 100644 --- a/openmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeusagebased.go +++ b/openmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeusagebased.go @@ -83,6 +83,14 @@ func newUsageBasedChargeIntent(target targetstate.StateItem) (charges.ChargeInte return charges.ChargeIntent{}, fmt.Errorf("price is required for usage based charge") } + // Copy unit_config too, not just price/discounts: if it is dropped here a + // subscription-created charge silently rates the raw metered quantity. Clone so the + // intent does not alias the spec. + var unitConfig *productcatalog.UnitConfig + if rateCardMeta.UnitConfig != nil { + unitConfig = lo.ToPtr(rateCardMeta.UnitConfig.Clone()) + } + annotations, err := target.SubscriptionItem.Annotations.Clone() if err != nil { return charges.ChargeIntent{}, err @@ -117,12 +125,10 @@ func newUsageBasedChargeIntent(target targetstate.StateItem) (charges.ChargeInte To: target.BillingPeriod.To, }, }, - InvoiceAt: target.GetInvoiceAt(), - Price: *price, - Discounts: billing.DiscountsFromProductCatalog(rateCardMeta.Discounts).UpsertCorrelationIDs(), - // TODO(unit-config): copy rateCardMeta.UnitConfig here (with a - // round-trip test) when the "snapshot unit_config onto subscriptions" - // ticket lands; the charge adapter already persists Intent.UnitConfig. + InvoiceAt: target.GetInvoiceAt(), + Price: *price, + Discounts: billing.DiscountsFromProductCatalog(rateCardMeta.Discounts).UpsertCorrelationIDs(), + UnitConfig: unitConfig, }, SettlementMode: target.Subscription.SettlementMode, FeatureKey: lo.FromPtr(rateCardMeta.FeatureKey), diff --git a/openmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeusagebased_test.go b/openmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeusagebased_test.go new file mode 100644 index 0000000000..98c13e0081 --- /dev/null +++ b/openmeter/billing/worker/subscriptionsync/service/reconciler/patchchargeusagebased_test.go @@ -0,0 +1,50 @@ +package reconciler + +import ( + "testing" + + "github.com/alpacahq/alpacadecimal" + "github.com/stretchr/testify/require" + + "github.com/openmeterio/openmeter/openmeter/productcatalog" +) + +// A subscription-created usage-based charge must snapshot the rate card's unit_config +// onto its intent; otherwise a plan using unit_config would rate the raw metered +// quantity (only direct charge-intent tests would exercise conversion). The intent → +// converted invoice amount is covered by the charges service integration suite. +func TestUsageBasedChargeIntentSnapshotsUnitConfig(t *testing.T) { + unitConfig := &productcatalog.UnitConfig{ + Operation: productcatalog.UnitConfigOperationDivide, + ConversionFactor: alpacadecimal.NewFromInt(1000), + Rounding: productcatalog.UnitConfigRoundingModeCeiling, + } + + t.Run("copies the rate card unit_config onto the usage-based intent", func(t *testing.T) { + rateCard := newChargePatchTestUsageRateCard() + rateCard.(*productcatalog.UsageBasedRateCard).UnitConfig = unitConfig + + target := newChargePatchTestTarget(t, productcatalog.CreditThenInvoiceSettlementMode, rateCard) + + intent, err := newUsageBasedChargeIntent(target) + require.NoError(t, err) + + ubIntent, err := intent.AsUsageBasedIntent() + require.NoError(t, err) + require.NotNil(t, ubIntent.UnitConfig) + require.True(t, unitConfig.Equal(ubIntent.UnitConfig)) + // Cloned, not aliased to the rate card's config. + require.NotSame(t, unitConfig, ubIntent.UnitConfig) + }) + + t.Run("leaves the intent unit_config nil when the rate card has none", func(t *testing.T) { + target := newChargePatchTestTarget(t, productcatalog.CreditThenInvoiceSettlementMode, newChargePatchTestUsageRateCard()) + + intent, err := newUsageBasedChargeIntent(target) + require.NoError(t, err) + + ubIntent, err := intent.AsUsageBasedIntent() + require.NoError(t, err) + require.Nil(t, ubIntent.UnitConfig) + }) +} diff --git a/openmeter/ledger/customerbalance/testenv_test.go b/openmeter/ledger/customerbalance/testenv_test.go index 2fb7f2764f..8a328cc76c 100644 --- a/openmeter/ledger/customerbalance/testenv_test.go +++ b/openmeter/ledger/customerbalance/testenv_test.go @@ -229,7 +229,7 @@ func newTestEnv(t *testing.T) *testEnv { Lineage: lineageService, MetaAdapter: metaAdapter, Locker: locker, - RatingService: billingratingservice.New(billingratingservice.Config{}), + RatingService: billingratingservice.New(billingratingservice.Config{UnitConfigEnabled: true}), }) require.NoError(t, err) @@ -249,7 +249,7 @@ func newTestEnv(t *testing.T) *testEnv { MetaAdapter: metaAdapter, CustomerOverrideService: billingService, FeatureService: featureService, - RatingService: billingratingservice.New(billingratingservice.Config{}), + RatingService: billingratingservice.New(billingratingservice.Config{UnitConfigEnabled: true}), StreamingConnector: streaming, }) require.NoError(t, err) diff --git a/test/app/testenv.go b/test/app/testenv.go index 9184e5a293..3f95632c75 100644 --- a/test/app/testenv.go +++ b/test/app/testenv.go @@ -274,7 +274,11 @@ func InitBillingService(t *testing.T, ctx context.Context, in InitBillingService }) require.NoError(t, err) - billingRatingService := billingratingservice.New(billingratingservice.Config{}) + // Enable unitConfig rating across the shared test env so the suite validates + // there is no regression from the flag being on (config-less lines must rate + // identically). Lines that carry a unit_config are exercised by the dedicated + // unit_config suites. + billingRatingService := billingratingservice.New(billingratingservice.Config{UnitConfigEnabled: true}) return billingservice.New(billingservice.Config{ Adapter: billingAdapter, diff --git a/test/billing/suite.go b/test/billing/suite.go index 0e2caa6627..7e4450a1cb 100644 --- a/test/billing/suite.go +++ b/test/billing/suite.go @@ -240,7 +240,7 @@ func (s *BaseSuite) setupSuite(opts SetupSuiteOptions) { billingService, err := billingservice.New(billingservice.Config{ Adapter: billingAdapter, - RatingService: billingratingservice.New(billingratingservice.Config{}), + RatingService: billingratingservice.New(billingratingservice.Config{UnitConfigEnabled: true}), CustomerService: s.CustomerService, AppService: s.AppService, Logger: slog.Default(), diff --git a/test/customer/testenv.go b/test/customer/testenv.go index 9e4ca2820a..e6a34350d0 100644 --- a/test/customer/testenv.go +++ b/test/customer/testenv.go @@ -407,7 +407,7 @@ func NewTestEnv(t *testing.T, ctx context.Context) (TestEnv, error) { billingService, err := billingservice.New(billingservice.Config{ Adapter: billingAdapter, - RatingService: billingratingservice.New(billingratingservice.Config{}), + RatingService: billingratingservice.New(billingratingservice.Config{UnitConfigEnabled: true}), CustomerService: customerService, AppService: appService, Logger: logger.WithGroup("billing"), diff --git a/test/subscription/framework_test.go b/test/subscription/framework_test.go index 7fea90227b..16908eaf35 100644 --- a/test/subscription/framework_test.go +++ b/test/subscription/framework_test.go @@ -99,7 +99,7 @@ func setup(t *testing.T, _ setupConfig) testDeps { billingService, err := billingservice.New(billingservice.Config{ Adapter: billingAdapter, - RatingService: billingratingservice.New(billingratingservice.Config{}), + RatingService: billingratingservice.New(billingratingservice.Config{UnitConfigEnabled: true}), CustomerService: deps.CustomerService, AppService: appService, Logger: slog.Default(), From 70db41af059cae278fb6256b14a1b5c6ea79b258 Mon Sep 17 00:00:00 2001 From: rolosp <6114043+rolosp@users.noreply.github.com> Date: Thu, 2 Jul 2026 15:12:19 +0200 Subject: [PATCH 4/6] fix: convert displayed quantity in charges line-mapper --- .../charges/service/unitconfig_rating_test.go | 14 +++++----- .../billing/charges/usagebased/charge.go | 16 +++++++++++ .../charges/usagebased/service/lineengine.go | 6 ++-- .../charges/usagebased/service/linemapper.go | 21 +++++++++----- .../usagebased/service/linemapper_test.go | 6 ++-- .../rating/service/mutator/unitconfig.go | 28 ++++++++++++++----- 6 files changed, 64 insertions(+), 27 deletions(-) diff --git a/openmeter/billing/charges/service/unitconfig_rating_test.go b/openmeter/billing/charges/service/unitconfig_rating_test.go index a0165bba39..633e96d179 100644 --- a/openmeter/billing/charges/service/unitconfig_rating_test.go +++ b/openmeter/billing/charges/service/unitconfig_rating_test.go @@ -50,16 +50,16 @@ func (s *unitConfigRatingEnabledSuite) TestRatesConvertedQuantity() { stdLine := invoices[0].Lines.OrEmpty()[0] - // MeteredQuantity records the raw metered value and stays 7400: the unit_config - // conversion changes the priced amount (asserted below), not the recorded metered - // quantity. - // - // TODO: once the charges line-mapper converts+rounds the displayed UsageBased.Quantity, - // extend this test to also assert the converted displayed quantity. - // MeteredQuantity stays raw (7400) as the audit value. + // The line is internally consistent: MeteredQuantity is the raw audit value (7400), + // while the customer-facing billable Quantity is the converted ceil(7400/1000) = 8, + // matching the priced amount. The line-mapper converts the displayed quantity through + // the same unit_config as rating. s.Require().NotNil(stdLine.UsageBased.MeteredQuantity) s.Equal(float64(7400), lo.FromPtr(stdLine.UsageBased.MeteredQuantity).InexactFloat64()) + s.Require().NotNil(stdLine.UsageBased.Quantity) + s.Equal(float64(8), lo.FromPtr(stdLine.UsageBased.Quantity).InexactFloat64()) + s.RequireTotals(billingtest.ExpectedTotals{ Amount: 8, Total: 8, diff --git a/openmeter/billing/charges/usagebased/charge.go b/openmeter/billing/charges/usagebased/charge.go index 933c6eba48..c58c67644c 100644 --- a/openmeter/billing/charges/usagebased/charge.go +++ b/openmeter/billing/charges/usagebased/charge.go @@ -387,6 +387,22 @@ func (i OverridableIntent) GetEffectiveDiscounts() billing.Discounts { return i.baseLayer.Discounts.Clone() } +// GetEffectiveUnitConfig returns the cloned unit_config from the active mutable +// layer, preferring the override layer when it is present. Nil when the rate card +// carries no unit_config. +func (i OverridableIntent) GetEffectiveUnitConfig() *productcatalog.UnitConfig { + unitConfig := i.baseLayer.UnitConfig + if i.overrideLayer != nil { + unitConfig = i.overrideLayer.UnitConfig + } + + if unitConfig == nil { + return nil + } + + return lo.ToPtr(unitConfig.Clone()) +} + // GetTaxConfig returns the immutable tax config from the base intent. // Override layers cannot change tax attribution. func (i OverridableIntent) GetTaxConfig() productcatalog.TaxCodeConfig { diff --git a/openmeter/billing/charges/usagebased/service/lineengine.go b/openmeter/billing/charges/usagebased/service/lineengine.go index 29e82d16cf..3314d67bfe 100644 --- a/openmeter/billing/charges/usagebased/service/lineengine.go +++ b/openmeter/billing/charges/usagebased/service/lineengine.go @@ -150,7 +150,7 @@ func (e *LineEngine) BuildStandardLinesForGatheringPreview(ctx context.Context, return nil, fmt.Errorf("building gathering preview run for line[%s]: %w", stdLine.ID, err) } - if err := populateUsageBasedStandardLineFromRun(stdLine, previewResult.Run, previewResult.Runs); err != nil { + if err := populateUsageBasedStandardLineFromRun(stdLine, previewResult.Run, previewResult.Runs, charge.Intent.GetEffectiveUnitConfig()); err != nil { return nil, fmt.Errorf("populating gathering preview line[%s] from run: %w", stdLine.ID, err) } @@ -250,7 +250,7 @@ func (e *LineEngine) OnStandardInvoiceCreated(ctx context.Context, input billing return nil, fmt.Errorf("getting current realization run for charge[%s]: %w", charge.ID, err) } - if err := populateUsageBasedStandardLineFromRun(stdLine, currentRun, charge.Realizations); err != nil { + if err := populateUsageBasedStandardLineFromRun(stdLine, currentRun, charge.Realizations, charge.Intent.GetEffectiveUnitConfig()); err != nil { return nil, fmt.Errorf("populating standard line from run for charge[%s]: %w", charge.ID, err) } @@ -301,7 +301,7 @@ func (e *LineEngine) OnCollectionCompleted(ctx context.Context, input billing.On return nil, fmt.Errorf("getting current realization run for charge[%s]: %w", charge.ID, err) } - if err := populateUsageBasedStandardLineFromRun(stdLine, currentRun, charge.Realizations); err != nil { + if err := populateUsageBasedStandardLineFromRun(stdLine, currentRun, charge.Realizations, charge.Intent.GetEffectiveUnitConfig()); err != nil { return nil, fmt.Errorf("populating standard line from run for charge[%s]: %w", charge.ID, err) } diff --git a/openmeter/billing/charges/usagebased/service/linemapper.go b/openmeter/billing/charges/usagebased/service/linemapper.go index 690292f6a0..964ff3cb0b 100644 --- a/openmeter/billing/charges/usagebased/service/linemapper.go +++ b/openmeter/billing/charges/usagebased/service/linemapper.go @@ -10,10 +10,11 @@ import ( "github.com/openmeterio/openmeter/openmeter/billing/charges/usagebased" billingrating "github.com/openmeterio/openmeter/openmeter/billing/rating" "github.com/openmeterio/openmeter/openmeter/billing/rating/service/mutator" + "github.com/openmeterio/openmeter/openmeter/productcatalog" "github.com/openmeterio/openmeter/pkg/currencyx" ) -func populateUsageBasedStandardLineFromRun(stdLine *billing.StandardLine, run usagebased.RealizationRun, runs usagebased.RealizationRuns) error { +func populateUsageBasedStandardLineFromRun(stdLine *billing.StandardLine, run usagebased.RealizationRun, runs usagebased.RealizationRuns, unitConfig *productcatalog.UnitConfig) error { if stdLine.UsageBased == nil { stdLine.UsageBased = &billing.UsageBasedLine{} } @@ -33,13 +34,19 @@ func populateUsageBasedStandardLineFromRun(stdLine *billing.StandardLine, run us stdLine.UsageBased.MeteredPreLinePeriodQuantity = lo.ToPtr(billingMeteredQuantity.PreLinePeriod) // Charge runs store cumulative raw metered quantity. Billing lines expose the raw - // metered values separately from net billable quantities and consumed usage discounts, - // so reuse the standard billing usage-discount mutator contract here. + // metered values (MeteredQuantity above) separately from net billable quantities and + // consumed usage discounts. Convert the raw quantity through the rate card's + // unit_config before the discount — mirroring the rating pipeline's + // [UnitConfig, DiscountUsage] order — so the displayed billable Quantity matches the + // priced amount rather than staying in raw metered units. A nil unitConfig is the + // identity, so non-unit_config lines are unchanged. + billableUsage := mutator.ApplyUnitConfig(billingrating.Usage{ + Quantity: billingMeteredQuantity.LinePeriod, + PreLinePeriodQuantity: billingMeteredQuantity.PreLinePeriod, + }, unitConfig) + discountedUsage, err := mutator.ApplyUsageDiscount(mutator.ApplyUsageDiscountInput{ - Usage: billingrating.Usage{ - Quantity: billingMeteredQuantity.LinePeriod, - PreLinePeriodQuantity: billingMeteredQuantity.PreLinePeriod, - }, + Usage: billableUsage, RateCardDiscounts: stdLine.RateCardDiscounts, StandardLineDiscounts: stdLine.Discounts, }) diff --git a/openmeter/billing/charges/usagebased/service/linemapper_test.go b/openmeter/billing/charges/usagebased/service/linemapper_test.go index c40c0700de..771617ef60 100644 --- a/openmeter/billing/charges/usagebased/service/linemapper_test.go +++ b/openmeter/billing/charges/usagebased/service/linemapper_test.go @@ -97,7 +97,7 @@ func TestPopulateUsageBasedStandardLineFromRunProjectsDetailsAndCredits(t *testi }, } - err := populateUsageBasedStandardLineFromRun(line, run, usagebased.RealizationRuns{priorRun, run}) + err := populateUsageBasedStandardLineFromRun(line, run, usagebased.RealizationRuns{priorRun, run}, nil) require.NoError(t, err) require.Len(t, line.DetailedLines, 2) @@ -163,7 +163,7 @@ func TestPopulateUsageBasedStandardLineFromRunAppliesUsageDiscount(t *testing.T) }, } - err := populateUsageBasedStandardLineFromRun(line, run, usagebased.RealizationRuns{priorRun, run}) + err := populateUsageBasedStandardLineFromRun(line, run, usagebased.RealizationRuns{priorRun, run}, nil) require.NoError(t, err) require.Equal(t, float64(10), lo.FromPtr(line.UsageBased.Quantity).InexactFloat64()) @@ -206,7 +206,7 @@ func TestPopulateUsageBasedStandardLineFromRunRequiresExpandedDetails(t *testing }, } - err := populateUsageBasedStandardLineFromRun(line, run, usagebased.RealizationRuns{run}) + err := populateUsageBasedStandardLineFromRun(line, run, usagebased.RealizationRuns{run}, nil) require.ErrorContains(t, err, "detailed lines must be expanded") } diff --git a/openmeter/billing/rating/service/mutator/unitconfig.go b/openmeter/billing/rating/service/mutator/unitconfig.go index 13060f3688..41d9568810 100644 --- a/openmeter/billing/rating/service/mutator/unitconfig.go +++ b/openmeter/billing/rating/service/mutator/unitconfig.go @@ -3,7 +3,9 @@ package mutator import ( "fmt" + "github.com/openmeterio/openmeter/openmeter/billing/rating" "github.com/openmeterio/openmeter/openmeter/billing/rating/service/rate" + "github.com/openmeterio/openmeter/openmeter/productcatalog" ) // UnitConfig is a pre-calculation mutator that applies the rate card's unit_config @@ -48,16 +50,28 @@ func (m *UnitConfig) Mutate(l rate.PricerCalculateInput) (rate.PricerCalculateIn usage, err := l.GetUsage() if err != nil { - return l, err + return l, fmt.Errorf("getting usage: %w", err) } - _, quantity := unitConfig.Apply(usage.Quantity) - _, preLinePeriodQuantity := unitConfig.Apply(usage.PreLinePeriodQuantity) - - usage.Quantity = quantity - usage.PreLinePeriodQuantity = preLinePeriodQuantity - + usage = ApplyUnitConfig(usage, unitConfig) l.Usage = &usage return l, nil } + +// ApplyUnitConfig converts the billable quantities of usage through the unit_config, +// applying the conversion (and its rounding) INDEPENDENTLY to Quantity and +// PreLinePeriodQuantity. It is the shared contract used by both the rating +// PreCalculation mutator and the charges line-mapper, so the priced amount and the +// displayed billable quantity convert through identical logic and cannot drift. A nil +// unitConfig is the identity. +func ApplyUnitConfig(usage rating.Usage, unitConfig *productcatalog.UnitConfig) rating.Usage { + if unitConfig == nil { + return usage + } + + _, usage.Quantity = unitConfig.Apply(usage.Quantity) + _, usage.PreLinePeriodQuantity = unitConfig.Apply(usage.PreLinePeriodQuantity) + + return usage +} From 943753c152a6285ded3aae28e64a4b8e82ce0fb9 Mon Sep 17 00:00:00 2001 From: rolosp <6114043+rolosp@users.noreply.github.com> Date: Thu, 2 Jul 2026 15:40:28 +0200 Subject: [PATCH 5/6] test: cover unit_config override inheritance --- .../billing/charges/usagebased/charge.go | 3 ++ .../billing/charges/usagebased/charge_test.go | 53 +++++++++++++++++++ 2 files changed, 56 insertions(+) create mode 100644 openmeter/billing/charges/usagebased/charge_test.go diff --git a/openmeter/billing/charges/usagebased/charge.go b/openmeter/billing/charges/usagebased/charge.go index c58c67644c..8a734283db 100644 --- a/openmeter/billing/charges/usagebased/charge.go +++ b/openmeter/billing/charges/usagebased/charge.go @@ -390,6 +390,9 @@ func (i OverridableIntent) GetEffectiveDiscounts() billing.Discounts { // GetEffectiveUnitConfig returns the cloned unit_config from the active mutable // layer, preferring the override layer when it is present. Nil when the rate card // carries no unit_config. +// +// The override layer is a full snapshot of the effective fields (like Price/Discounts), +// so a nil override value means the config was cleared, not inherited from the base. func (i OverridableIntent) GetEffectiveUnitConfig() *productcatalog.UnitConfig { unitConfig := i.baseLayer.UnitConfig if i.overrideLayer != nil { diff --git a/openmeter/billing/charges/usagebased/charge_test.go b/openmeter/billing/charges/usagebased/charge_test.go new file mode 100644 index 0000000000..e4f68c276c --- /dev/null +++ b/openmeter/billing/charges/usagebased/charge_test.go @@ -0,0 +1,53 @@ +package usagebased + +import ( + "testing" + + "github.com/alpacahq/alpacadecimal" + "github.com/stretchr/testify/require" + + "github.com/openmeterio/openmeter/openmeter/productcatalog" +) + +// TestOverridableIntentGetEffectiveUnitConfig locks in that the override layer is a +// full snapshot of the effective mutable fields: an override created from the effective +// intent inherits the base unit_config, so an unrelated edit does not drop the +// conversion, while an explicit nil is a genuine cleared state. +func TestOverridableIntentGetEffectiveUnitConfig(t *testing.T) { + unitConfig := &productcatalog.UnitConfig{ + Operation: productcatalog.UnitConfigOperationDivide, + ConversionFactor: alpacadecimal.NewFromInt(1000), + Rounding: productcatalog.UnitConfigRoundingModeCeiling, + } + + base := Intent{ + IntentMutableFields: IntentMutableFields{ + Price: *productcatalog.NewPriceFrom(productcatalog.UnitPrice{Amount: alpacadecimal.NewFromInt(1)}), + UnitConfig: unitConfig, + }, + FeatureKey: "f", + } + + oi := base.AsOverridableIntent() + + t.Run("override with an unrelated edit inherits the base unit_config", func(t *testing.T) { + // Mirror how the state machine creates the first override: snapshot the full + // effective mutable fields, then change one unrelated field only. + overrideFields := oi.GetEffectiveIntent().IntentMutableFields + overrideFields.Name = "unrelated override edit" + + withOverride := NewOverridableIntent(base, &overrideFields) + + got := withOverride.GetEffectiveUnitConfig() + require.NotNil(t, got, "unrelated override must not drop the base unit_config") + require.True(t, unitConfig.Equal(got)) + }) + + t.Run("explicit nil on the override is a cleared state, not inherited", func(t *testing.T) { + clearedFields := oi.GetEffectiveIntent().IntentMutableFields + clearedFields.UnitConfig = nil + + cleared := NewOverridableIntent(base, &clearedFields) + require.Nil(t, cleared.GetEffectiveUnitConfig()) + }) +} From 9762fb207206614f8a97e1a1442870f2dd0f36e0 Mon Sep 17 00:00:00 2001 From: rolosp <6114043+rolosp@users.noreply.github.com> Date: Thu, 2 Jul 2026 16:15:06 +0200 Subject: [PATCH 6/6] refactor: use a struct for populateStandardLineFromRun for improved readability --- .../charges/usagebased/service/lineengine.go | 18 +++++++++++--- .../charges/usagebased/service/linemapper.go | 24 ++++++++++++------- .../usagebased/service/linemapper_test.go | 15 +++++++++--- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/openmeter/billing/charges/usagebased/service/lineengine.go b/openmeter/billing/charges/usagebased/service/lineengine.go index 3314d67bfe..368d2a5baa 100644 --- a/openmeter/billing/charges/usagebased/service/lineengine.go +++ b/openmeter/billing/charges/usagebased/service/lineengine.go @@ -150,7 +150,11 @@ func (e *LineEngine) BuildStandardLinesForGatheringPreview(ctx context.Context, return nil, fmt.Errorf("building gathering preview run for line[%s]: %w", stdLine.ID, err) } - if err := populateUsageBasedStandardLineFromRun(stdLine, previewResult.Run, previewResult.Runs, charge.Intent.GetEffectiveUnitConfig()); err != nil { + if err := populateStandardLineFromRun(stdLine, populateStandardLineFromRunInput{ + Run: previewResult.Run, + Runs: previewResult.Runs, + UnitConfig: charge.Intent.GetEffectiveUnitConfig(), + }); err != nil { return nil, fmt.Errorf("populating gathering preview line[%s] from run: %w", stdLine.ID, err) } @@ -250,7 +254,11 @@ func (e *LineEngine) OnStandardInvoiceCreated(ctx context.Context, input billing return nil, fmt.Errorf("getting current realization run for charge[%s]: %w", charge.ID, err) } - if err := populateUsageBasedStandardLineFromRun(stdLine, currentRun, charge.Realizations, charge.Intent.GetEffectiveUnitConfig()); err != nil { + if err := populateStandardLineFromRun(stdLine, populateStandardLineFromRunInput{ + Run: currentRun, + Runs: charge.Realizations, + UnitConfig: charge.Intent.GetEffectiveUnitConfig(), + }); err != nil { return nil, fmt.Errorf("populating standard line from run for charge[%s]: %w", charge.ID, err) } @@ -301,7 +309,11 @@ func (e *LineEngine) OnCollectionCompleted(ctx context.Context, input billing.On return nil, fmt.Errorf("getting current realization run for charge[%s]: %w", charge.ID, err) } - if err := populateUsageBasedStandardLineFromRun(stdLine, currentRun, charge.Realizations, charge.Intent.GetEffectiveUnitConfig()); err != nil { + if err := populateStandardLineFromRun(stdLine, populateStandardLineFromRunInput{ + Run: currentRun, + Runs: charge.Realizations, + UnitConfig: charge.Intent.GetEffectiveUnitConfig(), + }); err != nil { return nil, fmt.Errorf("populating standard line from run for charge[%s]: %w", charge.ID, err) } diff --git a/openmeter/billing/charges/usagebased/service/linemapper.go b/openmeter/billing/charges/usagebased/service/linemapper.go index 964ff3cb0b..f3cbe53f08 100644 --- a/openmeter/billing/charges/usagebased/service/linemapper.go +++ b/openmeter/billing/charges/usagebased/service/linemapper.go @@ -14,7 +14,13 @@ import ( "github.com/openmeterio/openmeter/pkg/currencyx" ) -func populateUsageBasedStandardLineFromRun(stdLine *billing.StandardLine, run usagebased.RealizationRun, runs usagebased.RealizationRuns, unitConfig *productcatalog.UnitConfig) error { +type populateStandardLineFromRunInput struct { + Run usagebased.RealizationRun + Runs usagebased.RealizationRuns + UnitConfig *productcatalog.UnitConfig +} + +func populateStandardLineFromRun(stdLine *billing.StandardLine, input populateStandardLineFromRunInput) error { if stdLine.UsageBased == nil { stdLine.UsageBased = &billing.UsageBasedLine{} } @@ -24,12 +30,12 @@ func populateUsageBasedStandardLineFromRun(stdLine *billing.StandardLine, run us return fmt.Errorf("creating currency calculator: %w", err) } - billingMeteredQuantity, err := runs.MapToBillingMeteredQuantity(run) + billingMeteredQuantity, err := input.Runs.MapToBillingMeteredQuantity(input.Run) if err != nil { return fmt.Errorf("mapping run metered quantity to billing: %w", err) } - stdLine.OverrideCollectionPeriodEnd = lo.ToPtr(run.StoredAtLT.Add(usagebased.InternalCollectionPeriod)) + stdLine.OverrideCollectionPeriodEnd = lo.ToPtr(input.Run.StoredAtLT.Add(usagebased.InternalCollectionPeriod)) stdLine.UsageBased.MeteredQuantity = lo.ToPtr(billingMeteredQuantity.LinePeriod) stdLine.UsageBased.MeteredPreLinePeriodQuantity = lo.ToPtr(billingMeteredQuantity.PreLinePeriod) @@ -38,12 +44,12 @@ func populateUsageBasedStandardLineFromRun(stdLine *billing.StandardLine, run us // consumed usage discounts. Convert the raw quantity through the rate card's // unit_config before the discount — mirroring the rating pipeline's // [UnitConfig, DiscountUsage] order — so the displayed billable Quantity matches the - // priced amount rather than staying in raw metered units. A nil unitConfig is the + // priced amount rather than staying in raw metered units. A nil unit_config is the // identity, so non-unit_config lines are unchanged. billableUsage := mutator.ApplyUnitConfig(billingrating.Usage{ Quantity: billingMeteredQuantity.LinePeriod, PreLinePeriodQuantity: billingMeteredQuantity.PreLinePeriod, - }, unitConfig) + }, input.UnitConfig) discountedUsage, err := mutator.ApplyUsageDiscount(mutator.ApplyUsageDiscountInput{ Usage: billableUsage, @@ -58,14 +64,14 @@ func populateUsageBasedStandardLineFromRun(stdLine *billing.StandardLine, run us stdLine.UsageBased.PreLinePeriodQuantity = lo.ToPtr(discountedUsage.Usage.PreLinePeriodQuantity) stdLine.Discounts = discountedUsage.StandardLineDiscounts - creditsApplied, err := run.CreditsAllocated.AsCreditsApplied() + creditsApplied, err := input.Run.CreditsAllocated.AsCreditsApplied() if err != nil { return err } stdLine.CreditsApplied = creditsApplied - mappedDetailedLines, err := mapUsageBasedDetailedLines(stdLine, run, currencyCalculator) + mappedDetailedLines, err := mapUsageBasedDetailedLines(stdLine, input.Run, currencyCalculator) if err != nil { return fmt.Errorf("mapping run detailed lines: %w", err) } @@ -73,10 +79,10 @@ func populateUsageBasedStandardLineFromRun(stdLine *billing.StandardLine, run us stdLine.DetailedLines = stdLine.DetailedLinesWithIDReuse(mappedDetailedLines) stdLine.Totals = stdLine.DetailedLines.SumTotals().RoundToPrecision(currencyCalculator) - expectedTotals := run.Totals.RoundToPrecision(currencyCalculator) + expectedTotals := input.Run.Totals.RoundToPrecision(currencyCalculator) if !stdLine.Totals.Equal(expectedTotals) { return fmt.Errorf("mapped line totals do not match run totals [line_id=%s run_id=%s line_total=%s run_total=%s]", - stdLine.ID, run.ID.ID, stdLine.Totals.Total.String(), expectedTotals.Total.String()) + stdLine.ID, input.Run.ID.ID, stdLine.Totals.Total.String(), expectedTotals.Total.String()) } return nil diff --git a/openmeter/billing/charges/usagebased/service/linemapper_test.go b/openmeter/billing/charges/usagebased/service/linemapper_test.go index 771617ef60..b86d2fb1a6 100644 --- a/openmeter/billing/charges/usagebased/service/linemapper_test.go +++ b/openmeter/billing/charges/usagebased/service/linemapper_test.go @@ -97,7 +97,10 @@ func TestPopulateUsageBasedStandardLineFromRunProjectsDetailsAndCredits(t *testi }, } - err := populateUsageBasedStandardLineFromRun(line, run, usagebased.RealizationRuns{priorRun, run}, nil) + err := populateStandardLineFromRun(line, populateStandardLineFromRunInput{ + Run: run, + Runs: usagebased.RealizationRuns{priorRun, run}, + }) require.NoError(t, err) require.Len(t, line.DetailedLines, 2) @@ -163,7 +166,10 @@ func TestPopulateUsageBasedStandardLineFromRunAppliesUsageDiscount(t *testing.T) }, } - err := populateUsageBasedStandardLineFromRun(line, run, usagebased.RealizationRuns{priorRun, run}, nil) + err := populateStandardLineFromRun(line, populateStandardLineFromRunInput{ + Run: run, + Runs: usagebased.RealizationRuns{priorRun, run}, + }) require.NoError(t, err) require.Equal(t, float64(10), lo.FromPtr(line.UsageBased.Quantity).InexactFloat64()) @@ -206,7 +212,10 @@ func TestPopulateUsageBasedStandardLineFromRunRequiresExpandedDetails(t *testing }, } - err := populateUsageBasedStandardLineFromRun(line, run, usagebased.RealizationRuns{run}, nil) + err := populateStandardLineFromRun(line, populateStandardLineFromRunInput{ + Run: run, + Runs: usagebased.RealizationRuns{run}, + }) require.ErrorContains(t, err, "detailed lines must be expanded") }