From dbc978e394aca4ad0b282e78ead8d4daae295409 Mon Sep 17 00:00:00 2001 From: Peter Turi Date: Mon, 26 Oct 2026 13:15:11 +0100 Subject: [PATCH] fix(billing): resolve persisted split history during invoice simulation --- openmeter/billing/README.md | 5 +- .../billing/httpdriver/invoiceline_test.go | 35 ++++++ openmeter/billing/lineengine/rating.go | 33 +++--- openmeter/billing/lineengine/rating_test.go | 87 ++++++--------- openmeter/billing/service/invoice.go | 17 +-- openmeter/billing/stdinvoice.go | 4 - test/billing/invoice_test.go | 100 ++++++++---------- test/billing/legacycalculation_test.go | 90 +++++++++++++--- 8 files changed, 215 insertions(+), 156 deletions(-) diff --git a/openmeter/billing/README.md b/openmeter/billing/README.md index b9ddea4c3b..024705ab7d 100644 --- a/openmeter/billing/README.md +++ b/openmeter/billing/README.md @@ -180,8 +180,6 @@ context. Persisted engine operations resolve complete history in the caller's transaction so materialization, gathering previews, collection, and mutable updates account for previously billed amounts consistently. Snapshot-only immutable comparisons use the group's period without rating historical amounts. -Simulation supplies transient context by split-group ID within its namespace, -independently of generated line IDs, without reading persisted groups. Calculation never replaces pending line edits with historical children. Rating returns amounts before credit application. Legacy invoice lines do not @@ -232,6 +230,9 @@ reject the whole edit, including mixed-engine writes. Gathering edits and invoice-deletion cleanup keep their rejection semantics for every callback error. Simulation validates feature/meter references and explicitly rates supplied legacy quantities on clones without persisting the transient result. +Simulation inputs with split-group IDs resolve persisted history through the +legacy engine while retaining the supplied quantities and pending line edits. +Missing groups fail without falling back to ordinary rating. Charge simulation is unsupported; supplied charge projections are not rerated. `ValidationIssueRecorder.Record` collects validation issues of any severity; diff --git a/openmeter/billing/httpdriver/invoiceline_test.go b/openmeter/billing/httpdriver/invoiceline_test.go index 1a0e5e4ab2..3a384cbb7d 100644 --- a/openmeter/billing/httpdriver/invoiceline_test.go +++ b/openmeter/billing/httpdriver/invoiceline_test.go @@ -16,6 +16,41 @@ import ( "github.com/openmeterio/openmeter/pkg/timeutil" ) +func TestMapSimulationLinePreservesPriorUsageAndSuppliedIdentity(t *testing.T) { + // Given ordinary HTTP simulation input with a prior-usage offset and supplied ID. + price := api.RateCardUsageBasedPrice{} + require.NoError(t, price.FromUnitPriceWithCommitments(api.UnitPriceWithCommitments{Amount: "1"})) + period := invoiceLineTestPeriod() + input := api.InvoiceSimulationLine{ + Id: lo.ToPtr("supplied-line-id"), + Name: "simulated usage", + Period: api.Period(period), + InvoiceAt: period.To, + Quantity: "20", + PreLinePeriodQuantity: lo.ToPtr("40"), + RateCard: &api.InvoiceUsageBasedRateCard{ + Price: &price, + FeatureKey: lo.ToPtr("api-requests"), + }, + } + + // When the HTTP mapper builds the supplied snapshot. + line, err := mapSimulationLineToEntity(input) + require.NoError(t, err) + + // Then raw and displayed quantities survive without implying a persisted group. + require.Equal(t, "supplied-line-id", line.ID) + require.Equal(t, 20.0, line.UsageBased.MeteredQuantity.InexactFloat64()) + require.Equal(t, 20.0, line.UsageBased.Quantity.InexactFloat64()) + require.Equal(t, 40.0, line.UsageBased.MeteredPreLinePeriodQuantity.InexactFloat64()) + require.Equal(t, 40.0, line.UsageBased.PreLinePeriodQuantity.InexactFloat64()) + require.Nil(t, line.SplitLineGroupID) + + input.PreLinePeriodQuantity = lo.ToPtr("invalid") + _, err = mapSimulationLineToEntity(input) + require.ErrorContains(t, err, "failed to parse preLinePeriodQuantity") +} + func TestMergeStandardLineFromInvoiceLineReplaceUpdateOverwritesTaxConfig(t *testing.T) { price := api.RateCardUsageBasedPrice{} require.NoError(t, price.FromUnitPriceWithCommitments(api.UnitPriceWithCommitments{ diff --git a/openmeter/billing/lineengine/rating.go b/openmeter/billing/lineengine/rating.go index c8b2dc9f02..f18873b984 100644 --- a/openmeter/billing/lineengine/rating.go +++ b/openmeter/billing/lineengine/rating.go @@ -66,13 +66,17 @@ func (e *Engine) RecalculateStandardLine(ctx context.Context, input RecalculateS return lines[0], err } -type RateStandardLinesWithSplitLineHierarchiesInput struct { - Lines billing.StandardLines - Hierarchies map[billing.LineID]*billing.SplitLineHierarchy +type RateStandardLinesInput struct { + Namespace string + Lines billing.StandardLines } -func (i RateStandardLinesWithSplitLineHierarchiesInput) Validate() error { +func (i RateStandardLinesInput) Validate() error { var errs []error + if i.Namespace == "" { + errs = append(errs, errors.New("namespace is required")) + } + if len(i.Lines) == 0 { errs = append(errs, errors.New("lines are required")) } @@ -91,22 +95,21 @@ func (i RateStandardLinesWithSplitLineHierarchiesInput) Validate() error { return models.NewNillableGenericValidationError(errors.Join(errs...)) } -// RateStandardLinesWithSplitLineHierarchies rates transient snapshots with supplied -// read-only history keyed by namespaced line ID. It performs no group loading, -// snapshotting or persistence and preserves calculated lines alongside rating issues. -func (e *Engine) RateStandardLinesWithSplitLineHierarchies(input RateStandardLinesWithSplitLineHierarchiesInput) (billing.StandardLines, error) { +// RateStandardLines rates supplied snapshots on clones, resolving persisted +// history for split-group IDs. Supplied quantities, including prior-period usage +// offsets, remain authoritative; it performs no snapshotting or persistence and +// preserves calculated lines alongside rating issues. +func (e *Engine) RateStandardLines(ctx context.Context, input RateStandardLinesInput) (billing.StandardLines, error) { if err := input.Validate(); err != nil { return nil, err } - return e.rateStandardLines(lo.Map(input.Lines, func(line *billing.StandardLine, _ int) lineWithHierarchy { - var hierarchy *billing.SplitLineHierarchy - if line.SplitLineGroupID != nil { - hierarchy = input.Hierarchies[line.GetLineID()] - } + resolvedLines, err := e.resolveSplitLineHierarchies(ctx, input.Namespace, input.Lines) + if err != nil { + return nil, fmt.Errorf("resolving standard line hierarchies: %w", err) + } - return lineWithHierarchy{StandardLine: line, hierarchy: hierarchy} - })) + return e.rateStandardLines(resolvedLines) } func (e *Engine) rateStandardLines(lines linesWithHierarchy) (billing.StandardLines, error) { diff --git a/openmeter/billing/lineengine/rating_test.go b/openmeter/billing/lineengine/rating_test.go index 593d2b04b0..260073121a 100644 --- a/openmeter/billing/lineengine/rating_test.go +++ b/openmeter/billing/lineengine/rating_test.go @@ -198,7 +198,10 @@ func TestRateStandardLinesUsesMeteredUsageAndPreservesIdentities(t *testing.T) { require.NoError(t, err) // When the supplied snapshots are rated without an invoice or persistence dependency. - lines, err := engine.RateStandardLinesWithSplitLineHierarchies(RateStandardLinesWithSplitLineHierarchiesInput{Lines: input}) + lines, err := engine.RateStandardLines(t.Context(), RateStandardLinesInput{ + Namespace: line.Namespace, + Lines: input, + }) require.NoError(t, err) // Then normalization and rating affect only the result, using raw rather than net usage. @@ -240,7 +243,10 @@ func TestRateStandardLinesUsesMeteredUsageAndPreservesIdentities(t *testing.T) { require.NoError(t, err) // When the edited state is rated again. - rerated, err := engine.RateStandardLinesWithSplitLineHierarchies(RateStandardLinesWithSplitLineHierarchiesInput{Lines: input}) + rerated, err := engine.RateStandardLines(t.Context(), RateStandardLinesInput{ + Namespace: line.Namespace, + Lines: input, + }) require.NoError(t, err) // Then discounts are applied once and matching persisted identities survive. @@ -263,17 +269,19 @@ func TestRateStandardLinesUsesMeteredUsageAndPreservesIdentities(t *testing.T) { require.True(t, detail.AmountDiscounts[0].UpdatedAt.IsZero()) // The adapter owns the update timestamp. require.Equal(t, 6.25, detail.AmountDiscounts[0].Amount.InexactFloat64()) - repeated, err := engine.RateStandardLinesWithSplitLineHierarchies(RateStandardLinesWithSplitLineHierarchiesInput{Lines: rerated}) + repeated, err := engine.RateStandardLines(t.Context(), RateStandardLinesInput{ + Namespace: line.Namespace, + Lines: rerated, + }) require.NoError(t, err) require.Equal(t, rerated, repeated) } func TestRateStandardLinesFailuresLeaveTheWholeInputUnchanged(t *testing.T) { for _, tc := range []struct { - name string - invalidate func(*billing.StandardLine) - errorText string - hierarchies map[billing.LineID]*billing.SplitLineHierarchy + name string + invalidate func(*billing.StandardLine) + errorText string }{ { name: "rating failure", @@ -289,18 +297,6 @@ func TestRateStandardLinesFailuresLeaveTheWholeInputUnchanged(t *testing.T) { }, errorText: "split line hierarchy is required", }, - { - name: "history from another namespace", - invalidate: func(line *billing.StandardLine) { - line.SplitLineGroupID = lo.ToPtr("missing-group") - }, - hierarchies: map[billing.LineID]*billing.SplitLineHierarchy{ - {Namespace: "another-namespace", ID: "second-line"}: { - Group: billing.SplitLineGroup{SplitLineGroupMutableFields: billing.SplitLineGroupMutableFields{ServicePeriod: lineEngineOverrideTestPeriod()}}, - }, - }, - errorText: "split line hierarchy is required", - }, { name: "mapping failure", invalidate: func(line *billing.StandardLine) { @@ -335,10 +331,9 @@ func TestRateStandardLinesFailuresLeaveTheWholeInputUnchanged(t *testing.T) { // When rating the batch fails on the second line. engine := &Engine{ratingService: ratingservice.New(ratingservice.Config{})} - out, err := engine.RateStandardLinesWithSplitLineHierarchies(RateStandardLinesWithSplitLineHierarchiesInput{ - Lines: lines, - Hierarchies: tc.hierarchies, - }) + out, err := engine.rateStandardLines(lo.Map(lines, func(line *billing.StandardLine, _ int) lineWithHierarchy { + return lineWithHierarchy{StandardLine: line} + })) // Then neither normalization nor earlier successful rating leaks into the input. require.ErrorContains(t, err, tc.errorText) @@ -455,7 +450,6 @@ func TestRateStandardLinesProgressiveCommitments(t *testing.T) { hierarchy.Lines = append(hierarchy.Lines, child) } - hierarchies := map[billing.LineID]*billing.SplitLineHierarchy{line.GetLineID(): hierarchy} historyBefore, err := json.Marshal(hierarchy) require.NoError(t, err) before, err := json.Marshal(line) @@ -463,10 +457,7 @@ func TestRateStandardLinesProgressiveCommitments(t *testing.T) { engine := &Engine{ratingService: ratingservice.New(ratingservice.Config{})} // When supplied history rates the current snapshot. - lines, err := engine.RateStandardLinesWithSplitLineHierarchies(RateStandardLinesWithSplitLineHierarchiesInput{ - Lines: billing.StandardLines{line}, - Hierarchies: hierarchies, - }) + lines, err := engine.rateStandardLines(linesWithHierarchy{{StandardLine: line, hierarchy: hierarchy}}) require.NoError(t, err) // Then deleted history and later periods do not contribute to commitments. @@ -488,10 +479,7 @@ func TestRateStandardLinesProgressiveCommitments(t *testing.T) { rated.DetailedLines[0].AmountDiscounts[0].ID = "max-spend-discount-id" } - rerated, err := engine.RateStandardLinesWithSplitLineHierarchies(RateStandardLinesWithSplitLineHierarchiesInput{ - Lines: lines, - Hierarchies: hierarchies, - }) + rerated, err := engine.rateStandardLines(linesWithHierarchy{{StandardLine: rated, hierarchy: hierarchy}}) require.NoError(t, err) require.Equal(t, rated.Totals, rerated[0].Totals) require.Len(t, rerated[0].DetailedLines, len(rated.DetailedLines)) @@ -511,29 +499,17 @@ func TestRateStandardLinesProgressiveCommitments(t *testing.T) { require.NoError(t, err) require.JSONEq(t, string(historyBefore), string(historyAfter)) - // Ordinary rating has no history. Header-only progressive context also - // retains the existing zero-history behavior. + // Ordinary rating has no prior monetary history. ordinaryLine, err := line.Clone() require.NoError(t, err) ordinaryLine.SplitLineGroupID = nil - ordinaryLines, err := engine.RateStandardLinesWithSplitLineHierarchies(RateStandardLinesWithSplitLineHierarchiesInput{Lines: billing.StandardLines{ordinaryLine}}) + ordinaryLines, err := engine.RateStandardLines(t.Context(), RateStandardLinesInput{ + Namespace: ordinaryLine.Namespace, + Lines: billing.StandardLines{ordinaryLine}, + }) require.NoError(t, err) require.Equal(t, tc.ordinaryCharges, ordinaryLines[0].Totals.ChargesTotal.InexactFloat64()) require.Equal(t, tc.ordinaryTotal, ordinaryLines[0].Totals.Total.InexactFloat64()) - - headerOnlyLine, err := line.Clone() - require.NoError(t, err) - headerOnlyLine.ID = "header-only-line" - mixedHistoryLines, err := engine.RateStandardLinesWithSplitLineHierarchies(RateStandardLinesWithSplitLineHierarchiesInput{ - Lines: billing.StandardLines{line, headerOnlyLine}, - Hierarchies: map[billing.LineID]*billing.SplitLineHierarchy{ - line.GetLineID(): hierarchy, - headerOnlyLine.GetLineID(): {Group: hierarchy.Group}, - }, - }) - require.NoError(t, err) - require.Equal(t, rated.Totals, mixedHistoryLines[0].Totals) - require.Equal(t, ordinaryLines[0].Totals, mixedHistoryLines[1].Totals) }) } } @@ -550,7 +526,10 @@ func TestRateStandardLinesPreservesCalculatedResultsWithCriticalIssues(t *testin ratingService.On("GenerateLegacyProgressiveBilledLines", mock.Anything, mock.Anything). Return(result, billing.ErrInvoiceLineVolumeSplitNotSupported).Once() engine := &Engine{ratingService: ratingService} - lines, err := engine.RateStandardLinesWithSplitLineHierarchies(RateStandardLinesWithSplitLineHierarchiesInput{Lines: billing.StandardLines{line}}) + lines, err := engine.RateStandardLines(t.Context(), RateStandardLinesInput{ + Namespace: line.Namespace, + Lines: billing.StandardLines{line}, + }) // Then calculated state is returned with the critical issue and line attribution. issues, systemErr := billing.ToValidationIssues(err) @@ -584,7 +563,10 @@ func TestRateStandardLinesPreservesResultsWithValidationWarnings(t *testing.T) { )).Once() engine := &Engine{ratingService: ratingService} - lines, err := engine.RateStandardLinesWithSplitLineHierarchies(RateStandardLinesWithSplitLineHierarchiesInput{Lines: billing.StandardLines{line}}) + lines, err := engine.RateStandardLines(t.Context(), RateStandardLinesInput{ + Namespace: line.Namespace, + Lines: billing.StandardLines{line}, + }) issues, systemErr := billing.ToValidationIssues(err) require.NoError(t, systemErr) @@ -614,7 +596,8 @@ func TestRateStandardLinesRejectsSystemErrors(t *testing.T) { Return(rating.GenerateDetailedLinesResult{}, ratingErr).Once() engine := &Engine{ratingService: ratingService} - lines, err := engine.RateStandardLinesWithSplitLineHierarchies(RateStandardLinesWithSplitLineHierarchiesInput{ + lines, err := engine.RateStandardLines(t.Context(), RateStandardLinesInput{ + Namespace: "namespace", Lines: billing.StandardLines{ standardLineForLineEngineOverrideTest(t, lineEngineOverrideTestPeriod()), }, diff --git a/openmeter/billing/service/invoice.go b/openmeter/billing/service/invoice.go index 3f76c88674..73c8565cdb 100644 --- a/openmeter/billing/service/invoice.go +++ b/openmeter/billing/service/invoice.go @@ -933,20 +933,9 @@ func (s Service) SimulateInvoice(ctx context.Context, input billing.SimulateInvo continue } - // Simulation uses only supplied transient history, without resolving persisted groups. - splitLineGroups := lo.FilterSliceToMap(grouped.Lines, func(line *billing.StandardLine) (billing.LineID, *billing.SplitLineHierarchy, bool) { - if line.SplitLineGroupID == nil { - return billing.LineID{}, nil, false - } - - hierarchy := input.SplitLineHierarchies[*line.SplitLineGroupID] - - return line.GetLineID(), hierarchy, hierarchy != nil - }) - - ratedLines, ratingErr := legacyEngine.RateStandardLinesWithSplitLineHierarchies(lineengine.RateStandardLinesWithSplitLineHierarchiesInput{ - Lines: grouped.Lines, - Hierarchies: splitLineGroups, + ratedLines, ratingErr := legacyEngine.RateStandardLines(ctx, lineengine.RateStandardLinesInput{ + Namespace: input.Namespace, + Lines: grouped.Lines, }) if err := invoice.MergeValidationIssues(billing.NewLineEngineValidationError(legacyEngine, ratingErr), billing.LineEngineValidationComponent(legacyEngine.GetLineEngineType())); err != nil { return billing.StandardInvoice{}, fmt.Errorf("rating simulation lines: %w", err) diff --git a/openmeter/billing/stdinvoice.go b/openmeter/billing/stdinvoice.go index 8e088549e9..a6611353c6 100644 --- a/openmeter/billing/stdinvoice.go +++ b/openmeter/billing/stdinvoice.go @@ -977,10 +977,6 @@ type SimulateInvoiceInput struct { Number *string Currency currencyx.FiatCode Lines StandardInvoiceLines - - // SplitLineHierarchies supplies read-only transient history keyed by split-group - // ID within Namespace. Association survives generated line IDs and normalization. - SplitLineHierarchies map[string]*SplitLineHierarchy } func (i SimulateInvoiceInput) Validate() error { diff --git a/test/billing/invoice_test.go b/test/billing/invoice_test.go index 83631b8f95..d863dee864 100644 --- a/test/billing/invoice_test.go +++ b/test/billing/invoice_test.go @@ -191,113 +191,101 @@ func (s *InvoicingTestSuite) TestSimulateInvoiceFeatureMeterValidation() { }) } -func (s *InvoicingTestSuite) TestSimulateInvoiceUsesTransientSplitHistory() { - // Given a transient final child with 20 of usage and 40 previously billed toward a 100 minimum. +func (s *InvoicingTestSuite) TestSimulateInvoiceSuppliedSnapshots() { + // Given supplied usage whose prior quantity crosses a graduated tier boundary. ctx := s.T().Context() - namespace := s.GetUniqueNamespace("transient-split-history") + namespace := s.GetUniqueNamespace("simulate-supplied-snapshots") sandboxApp := s.InstallSandboxApp(s.T(), namespace) s.ProvisionBillingProfile(ctx, namespace, sandboxApp.GetID()) customerEntity := s.CreateTestCustomer(namespace, "test-customer") testFeature := s.SetupApiRequestsTotalFeature(ctx, namespace) defer testFeature.Cleanup() - fullPeriod := timeutil.ClosedPeriod{ + period := timeutil.ClosedPeriod{ From: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC), To: time.Date(2026, 2, 1, 0, 0, 0, 0, time.UTC), } - boundary := fullPeriod.From.Add(fullPeriod.Duration() / 2) line := billing.NewFlatFeeLine(billing.NewFlatFeeLineInput{ - Namespace: namespace, - Period: timeutil.ClosedPeriod{From: boundary, To: fullPeriod.To}, - InvoiceAt: fullPeriod.To, + Period: period, + InvoiceAt: period.To, Name: "transient usage", PerUnitAmount: alpacadecimal.NewFromInt(1), PaymentTerm: productcatalog.InArrearsPaymentTerm, ManagedBy: billing.ManuallyManagedLine, }, billing.WithFeatureKey(testFeature.Feature.Key)) - line.UsageBased.Price = productcatalog.NewPriceFrom(productcatalog.UnitPrice{ - Amount: alpacadecimal.NewFromInt(1), - Commitments: productcatalog.Commitments{ - MinimumAmount: lo.ToPtr(alpacadecimal.NewFromInt(100)), + line.UsageBased.Price = productcatalog.NewPriceFrom(productcatalog.TieredPrice{ + Mode: productcatalog.GraduatedTieredPrice, + Tiers: []productcatalog.PriceTier{ + { + UpToAmount: lo.ToPtr(alpacadecimal.NewFromInt(50)), + UnitPrice: &productcatalog.PriceTierUnitPrice{Amount: alpacadecimal.NewFromInt(1)}, + }, + {UnitPrice: &productcatalog.PriceTierUnitPrice{Amount: alpacadecimal.NewFromInt(2)}}, }, }) line.UsageBased.MeteredQuantity = lo.ToPtr(alpacadecimal.NewFromInt(20)) line.UsageBased.MeteredPreLinePeriodQuantity = lo.ToPtr(alpacadecimal.NewFromInt(40)) - line.SplitLineGroupID = lo.ToPtr("unpersisted-group") - hierarchy := &billing.SplitLineHierarchy{ - Group: billing.SplitLineGroup{ - NamespacedID: models.NamespacedID{Namespace: namespace, ID: *line.SplitLineGroupID}, - SplitLineGroupMutableFields: billing.SplitLineGroupMutableFields{ServicePeriod: fullPeriod}, - }, - Lines: []billing.SplitLineHierarchyLine{{ - ServicePeriod: timeutil.ClosedPeriod{From: fullPeriod.From, To: boundary}, - Amount: alpacadecimal.NewFromInt(40), - Invoice: billing.SplitLineHierarchyInvoice{Type: billing.InvoiceTypeStandard}, - }}, - } - line.Namespace = "" otherLine, err := line.Clone() s.Require().NoError(err) otherLine.Namespace = "caller-namespace" otherLine.ID = "supplied-line-id" - otherLine.SplitLineGroupID = lo.ToPtr("another-unpersisted-group") otherLine.UsageBased.MeteredPreLinePeriodQuantity = lo.ToPtr(alpacadecimal.NewFromInt(10)) - otherHierarchy, err := hierarchy.Clone() - s.Require().NoError(err) - otherHierarchy.Group.ID = *otherLine.SplitLineGroupID - otherHierarchy.Lines[0].Amount = alpacadecimal.NewFromInt(10) input := billing.SimulateInvoiceInput{ Namespace: namespace, CustomerID: &customerEntity.ID, Currency: currencyx.FiatCode(currency.USD), Lines: billing.NewStandardInvoiceLines(billing.StandardLines{line, otherLine}), - SplitLineHierarchies: map[string]*billing.SplitLineHierarchy{ - *line.SplitLineGroupID: hierarchy, - *otherLine.SplitLineGroupID: &otherHierarchy, - }, } before, err := json.Marshal(input) s.Require().NoError(err) - // When simulation rates supplied quantities and history for a group absent from the database. + // When simulation rates snapshots without reading split groups or metered usage. historyQueries := &splitLineHistoryQueryCounter{} s.DBClient.BillingInvoiceSplitLineGroup.Intercept(historyQueries) invoice, err := s.BillingService.SimulateInvoice(ctx, input) s.Require().NoError(err) - // Then supplied history produces the 40 adjustment without persistence or source mutation. + // Then prior usage affects pricing, generated and supplied identities survive, + // and the input and database remain unchanged. s.Require().Len(invoice.Lines.OrEmpty(), 2) - s.Equal(40.0, invoice.Lines.OrEmpty()[0].Totals.ChargesTotal.InexactFloat64()) + s.Equal(30.0, invoice.Lines.OrEmpty()[0].Totals.Total.InexactFloat64()) + s.Equal(40.0, invoice.Lines.OrEmpty()[0].UsageBased.PreLinePeriodQuantity.InexactFloat64()) s.NotEmpty(invoice.Lines.OrEmpty()[0].ID) s.Equal("supplied-line-id", invoice.Lines.OrEmpty()[1].ID) s.Equal(namespace, invoice.Lines.OrEmpty()[0].Namespace) s.Equal(namespace, invoice.Lines.OrEmpty()[1].Namespace) - s.Equal(60.0, invoice.Lines.OrEmpty()[0].Totals.Total.InexactFloat64()) - s.Equal(70.0, invoice.Lines.OrEmpty()[1].Totals.ChargesTotal.InexactFloat64()) - s.Equal(90.0, invoice.Lines.OrEmpty()[1].Totals.Total.InexactFloat64()) - s.Equal(150.0, invoice.Totals.Total.InexactFloat64()) + s.Equal(20.0, invoice.Lines.OrEmpty()[1].Totals.Total.InexactFloat64()) + s.Equal(50.0, invoice.Totals.Total.InexactFloat64()) s.Zero(historyQueries.count) s.Empty(invoice.ValidationIssues) - // Missing supplied history retains the legacy error without a DB fallback. - withoutHistory := input - withoutHistory.SplitLineHierarchies = nil - _, err = s.BillingService.SimulateInvoice(ctx, withoutHistory) - s.ErrorContains(err, "split line hierarchy is required") - s.Zero(historyQueries.count) - after, err := json.Marshal(input) s.Require().NoError(err) s.JSONEq(string(before), string(after)) - histories, err := s.BillingAdapter.GetSplitLineGroups(ctx, billing.GetSplitLineGroupsInput{ - Namespace: namespace, - SplitLineGroupIDs: []string{*line.SplitLineGroupID, *otherLine.SplitLineGroupID}, - }) - s.Require().NoError(err) - s.Empty(histories) _, err = s.BillingAdapter.GetStandardInvoiceById(ctx, billing.GetStandardInvoiceByIdInput{ Invoice: invoice.GetInvoiceID(), }) s.ErrorIs(err, billing.ErrInvoiceNotFound) + + s.Run("missing split group fails without a fallback", func() { + // Given a clone of an otherwise valid supplied line carrying a group ID. + splitLine, err := line.Clone() + s.Require().NoError(err) + splitLine.SplitLineGroupID = lo.ToPtr("unpersisted-group") + before, err := json.Marshal(splitLine) + s.Require().NoError(err) + splitInput := input + splitInput.Lines = billing.NewStandardInvoiceLines(billing.StandardLines{splitLine}) + + // When simulation resolves a group absent from the namespace. + _, err = s.BillingService.SimulateInvoice(ctx, splitInput) + + // Then missing history fails explicitly and the caller's input is untouched. + s.ErrorContains(err, "split line group[unpersisted-group]") + s.Equal(1, historyQueries.count) + after, err := json.Marshal(splitLine) + s.Require().NoError(err) + s.JSONEq(string(before), string(after)) + }) } func (s *InvoicingTestSuite) TestPendingLineCreation() { @@ -5619,8 +5607,8 @@ func (s *InvoicingTestSuite) TestUpdateInvoice() { }) } -// Counts persisted history reads independently of returned data: an unnecessary -// query must not silently turn simulation or ordinary reads into DB consumers. +// Counts group-history reads so ordinary simulation and invoice reads remain +// independent of history, while explicit split-group calculations can load it. type splitLineHistoryQueryCounter struct { count int } diff --git a/test/billing/legacycalculation_test.go b/test/billing/legacycalculation_test.go index e89b38a18b..f1b9eed708 100644 --- a/test/billing/legacycalculation_test.go +++ b/test/billing/legacycalculation_test.go @@ -102,26 +102,90 @@ func (s *BillingAdapterTestSuite) TestLegacyCalculationInvariants() { before, err := json.Marshal(current) s.Require().NoError(err) - // When the final slice is rated against the persisted billing history. - persistedHistories, err := s.BillingAdapter.GetSplitLineGroups(ctx, billing.GetSplitLineGroupsInput{ - Namespace: namespace, - SplitLineGroupIDs: []string{group.ID}, + // When mutable recalculation resolves and rates the persisted billing history. + ratedSnapshot, err := s.LegacyBillingLineEngine.RecalculateStandardLine(ctx, lineengine.RecalculateStandardLineInput{ + Invoice: invoice, + Line: current, }) s.Require().NoError(err) - s.Require().Len(persistedHistories, 1) s.Equal(1, historyQueries.count) - ratedSnapshots, err := s.LegacyBillingLineEngine.RateStandardLinesWithSplitLineHierarchies(lineengine.RateStandardLinesWithSplitLineHierarchiesInput{ - Lines: billing.StandardLines{current}, - Hierarchies: map[billing.LineID]*billing.SplitLineHierarchy{current.GetLineID(): &persistedHistories[0]}, - }) - s.Require().NoError(err) - s.Require().Len(ratedSnapshots, 1) - expectedTotals := ratedSnapshots[0].Totals + expectedTotals := ratedSnapshot.Totals // Then prior billing plus the final slice meets the period minimum exactly once. + s.Equal(40.0, expectedTotals.ChargesTotal.InexactFloat64()) + s.Equal(60.0, expectedTotals.Total.InexactFloat64()) s.Equal(100.0, prior.Totals.Amount.Add(expectedTotals.Total).InexactFloat64()) - s.Run("materialization and preview agree with snapshot rating", func() { + s.Run("simulation resolves history while retaining supplied snapshots", func() { + // Given a hypothetical price and quantity different from the persisted group and metered usage. + simulatedLine, err := current.Clone() + s.Require().NoError(err) + simulatedLine.Namespace = "caller-namespace" + simulatedLine.ID = "" + simulatedLine.UsageBased.Price = productcatalog.NewPriceFrom(productcatalog.UnitPrice{ + Amount: alpacadecimal.NewFromInt(2), + Commitments: productcatalog.Commitments{MinimumAmount: lo.ToPtr(alpacadecimal.NewFromInt(100))}, + }) + simulatedLine.UsageBased.MeteredQuantity = lo.ToPtr(alpacadecimal.NewFromInt(15)) + before, err := json.Marshal(simulatedLine) + s.Require().NoError(err) + queriesBefore := historyQueries.count + + // When simulation loads history using the group ID after normalizing the supplied line. + simulatedInvoice, err := s.BillingService.SimulateInvoice(ctx, billing.SimulateInvoiceInput{ + Namespace: namespace, + CustomerID: &invoice.Customer.CustomerID, + Currency: invoice.Currency, + Lines: billing.NewStandardInvoiceLines(billing.StandardLines{simulatedLine}), + }) + s.Require().NoError(err) + + // Then prior billing of 40 leaves 60 due, using the supplied 30 of usage and 30 adjustment. + s.Require().Len(simulatedInvoice.Lines.OrEmpty(), 1) + ratedLine := simulatedInvoice.Lines.OrEmpty()[0] + s.NotEmpty(ratedLine.ID) + s.Equal(namespace, ratedLine.Namespace) + s.Equal(current.Name, ratedLine.Name) + s.Equal(group.ID, *ratedLine.SplitLineGroupID) + s.Equal(15.0, ratedLine.UsageBased.MeteredQuantity.InexactFloat64()) + s.Equal(40.0, ratedLine.UsageBased.MeteredPreLinePeriodQuantity.InexactFloat64()) + s.Equal(30.0, ratedLine.Totals.Amount.InexactFloat64()) + s.Equal(30.0, ratedLine.Totals.ChargesTotal.InexactFloat64()) + s.Equal(60.0, simulatedInvoice.Totals.Total.InexactFloat64()) + s.Equal(queriesBefore+1, historyQueries.count) + s.Empty(simulatedInvoice.ValidationIssues) + after, err := json.Marshal(simulatedLine) + s.Require().NoError(err) + s.JSONEq(string(before), string(after)) + _, err = s.BillingAdapter.GetStandardInvoiceById(ctx, billing.GetStandardInvoiceByIdInput{ + Invoice: simulatedInvoice.GetInvoiceID(), + }) + s.ErrorIs(err, billing.ErrInvoiceNotFound) + }) + + s.Run("supplied snapshot rating cannot load another namespace", func() { + // Given a supplied line referencing a group belonging to a different namespace. + foreignLine, err := current.Clone() + s.Require().NoError(err) + foreignLine.Namespace = s.GetUniqueNamespace("foreign-simulation") + before, err := json.Marshal(foreignLine) + s.Require().NoError(err) + + // When rating resolves history in the supplied namespace. + lines, err := s.LegacyBillingLineEngine.RateStandardLines(ctx, lineengine.RateStandardLinesInput{ + Namespace: foreignLine.Namespace, + Lines: billing.StandardLines{foreignLine}, + }) + + // Then the group remains missing and the failed calculation leaves its input intact. + s.ErrorContains(err, "split line group["+group.ID+"] for line["+foreignLine.ID+"] not found") + s.Nil(lines) + after, err := json.Marshal(foreignLine) + s.Require().NoError(err) + s.JSONEq(string(before), string(after)) + }) + + s.Run("materialization and preview agree with mutable recalculation", func() { // When the same final slice is materialized and previewed. gatheringBase, err := current.ToGatheringLineBase() s.Require().NoError(err)