From 8c3517fade6602734bd76f4bf30e84bf98fa8e0c Mon Sep 17 00:00:00 2001 From: Alex Goth <64845621+GAlexIHU@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:41:51 +0200 Subject: [PATCH 1/5] fix(ledger): remove overage correction transaction ordering --- .../chargeadapter/customcurrencyoverage.go | 31 +++-- .../flatfee_customcurrency_test.go | 125 ++++++++++++++++++ 2 files changed, 143 insertions(+), 13 deletions(-) diff --git a/openmeter/ledger/chargeadapter/customcurrencyoverage.go b/openmeter/ledger/chargeadapter/customcurrencyoverage.go index 7edcf1be01..091c48f99c 100644 --- a/openmeter/ledger/chargeadapter/customcurrencyoverage.go +++ b/openmeter/ledger/chargeadapter/customcurrencyoverage.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "slices" "time" "github.com/alpacahq/alpacadecimal" @@ -211,8 +212,8 @@ func (h *customCurrencyOverageHandler) book(ctx context.Context, input bookCusto }, nil } -// correct reverses a complete custom-currency overage booking in dependency -// order: currency conversion, immediate consumption, then credit issuance. +// correct reverses every accounting leg of a custom-currency overage booking +// in one atomic group. func (h *customCurrencyOverageHandler) correct(ctx context.Context, input correctCustomCurrencyOverageInput) error { if err := input.Validate(); err != nil { return err @@ -233,19 +234,20 @@ func (h *customCurrencyOverageHandler) correct(ctx context.Context, input correc return fmt.Errorf("get original transaction group: %w", err) } - expectedForwardOrder := []string{ + expectedTemplateCodes := []string{ transactions.TemplateCode(transactions.IssueCustomerReceivableTemplate{}), transactions.TemplateCode(transactions.TransferCustomerFBOAdvanceToAccruedTemplate{}), transactions.TemplateCode(transactions.ConvertCurrencyTemplate{}), } originalTransactions := originalGroup.Transactions() - if len(originalTransactions) != len(expectedForwardOrder) { - return fmt.Errorf("custom-currency overage group has %d transactions, expected %d", len(originalTransactions), len(expectedForwardOrder)) + if len(originalTransactions) != len(expectedTemplateCodes) { + return fmt.Errorf("custom-currency overage group has %d transactions, expected %d", len(originalTransactions), len(expectedTemplateCodes)) } - // The booking owns this transaction order, and transaction groups reload in - // creation order. Validate that contract before unwinding it in reverse. - for idx, transaction := range originalTransactions { + // Creation timestamps and IDs do not define the accounting legs' order. + // Require exactly one of each template before resolving any corrections. + seenTemplates := make(map[string]struct{}, len(expectedTemplateCodes)) + for _, transaction := range originalTransactions { direction, err := ledger.TransactionDirectionFromAnnotations(transaction.Annotations()) if err != nil { return fmt.Errorf("get original transaction direction: %w", err) @@ -260,14 +262,17 @@ func (h *customCurrencyOverageHandler) correct(ctx context.Context, input correc return fmt.Errorf("get original transaction template code: %w", err) } - if templateCode != expectedForwardOrder[idx] { - return fmt.Errorf("unexpected transaction template %s at index %d in custom-currency overage group, expected %s", templateCode, idx, expectedForwardOrder[idx]) + if !slices.Contains(expectedTemplateCodes, templateCode) { + return fmt.Errorf("unexpected transaction template %s in custom-currency overage group", templateCode) } + if _, seen := seenTemplates[templateCode]; seen { + return fmt.Errorf("duplicate transaction template %s in custom-currency overage group", templateCode) + } + seenTemplates[templateCode] = struct{}{} } correctionInputs := make([]ledger.TransactionInput, 0, len(originalTransactions)) - for idx := len(originalTransactions) - 1; idx >= 0; idx-- { - originalTransaction := originalTransactions[idx] + for _, originalTransaction := range originalTransactions { resolved, err := transactions.CorrectTransaction(ctx, h.deps, transactions.CorrectionInput{ At: input.BookedAt, Amount: input.CustomAmount, @@ -276,7 +281,7 @@ func (h *customCurrencyOverageHandler) correct(ctx context.Context, input correc OriginalGroup: originalGroup, }) if err != nil { - return fmt.Errorf("correct transaction template %s: %w", expectedForwardOrder[idx], err) + return fmt.Errorf("correct original transaction %s: %w", originalTransaction.ID().ID, err) } for _, correctionInput := range resolved { diff --git a/openmeter/ledger/chargeadapter/flatfee_customcurrency_test.go b/openmeter/ledger/chargeadapter/flatfee_customcurrency_test.go index 312e2e86a0..9902418572 100644 --- a/openmeter/ledger/chargeadapter/flatfee_customcurrency_test.go +++ b/openmeter/ledger/chargeadapter/flatfee_customcurrency_test.go @@ -16,6 +16,7 @@ import ( "github.com/openmeterio/openmeter/openmeter/billing/charges/models/costbasis" "github.com/openmeterio/openmeter/openmeter/billing/charges/models/creditrealization" "github.com/openmeterio/openmeter/openmeter/billing/charges/models/invoicedusage" + "github.com/openmeterio/openmeter/openmeter/billing/charges/models/ledgertransaction" "github.com/openmeterio/openmeter/openmeter/billing/models/totals" "github.com/openmeterio/openmeter/openmeter/currencies" currenciestestutils "github.com/openmeterio/openmeter/openmeter/currencies/testutils" @@ -201,6 +202,130 @@ func TestOnFlatFeeCustomCurrencyOverageAccruedCorrection(t *testing.T) { }, templateCodes) } +func TestOnFlatFeeCustomCurrencyOverageAccruedCorrection_TransactionOrder(t *testing.T) { + const ( + issuance = iota + consumption + conversion + ) + + for _, tc := range []struct { + name string + order [3]int + expectedError string + }{ + {name: "issuance consumption conversion", order: [3]int{issuance, consumption, conversion}}, + {name: "issuance conversion consumption", order: [3]int{issuance, conversion, consumption}}, + {name: "consumption issuance conversion", order: [3]int{consumption, issuance, conversion}}, + {name: "consumption conversion issuance", order: [3]int{consumption, conversion, issuance}}, + {name: "conversion issuance consumption", order: [3]int{conversion, issuance, consumption}}, + {name: "conversion consumption issuance", order: [3]int{conversion, consumption, issuance}}, + {name: "duplicate issuance", order: [3]int{issuance, issuance, conversion}, expectedError: "duplicate transaction template"}, + } { + t.Run(tc.name, func(t *testing.T) { + env := newFlatFeeHandlerTestEnv(t) + customCurrencyValue := currenciestestutils.NewCustomCurrency(t, "ACME", 2) + customCurrencyIdentity := customCurrencyValue.Reference() + settlementCurrency := currencyx.Code("USD") + costBasis := alpacadecimal.NewFromFloat(0.25) + charge := env.newCustomCurrencyCreditThenInvoiceCharge(t, customCurrencyValue, costBasis) + run := env.newCustomOverageRun(totals.Totals{ + Amount: alpacadecimal.NewFromInt(40), + Total: alpacadecimal.NewFromInt(40), + }) + bookedAt := flatfee.UsageBookedAt(charge.Intent.GetEffectivePaymentTerm(), run.ServicePeriod) + + // given: the overage's accounting legs are persisted in a different order + inputs, err := transactions.ResolveTransactions(t.Context(), transactions.ResolverDependencies{ + AccountService: env.Deps.ResolversService, + AccountCatalog: env.Deps.AccountService, + BalanceQuerier: env.Deps.HistoricalLedger, + }, transactions.ResolutionScope{CustomerID: env.CustomerID, Namespace: env.Namespace}, + transactions.IssueCustomerReceivableTemplate{ + At: bookedAt, + Amount: alpacadecimal.NewFromInt(40), + Currency: customCurrencyIdentity, + CostBasisCurrency: &settlementCurrency, + CostBasis: &costBasis, + SourceChargeID: &charge.ID, + }, + transactions.TransferCustomerFBOAdvanceToAccruedTemplate{ + At: bookedAt, + Amount: alpacadecimal.NewFromInt(40), + Currency: customCurrencyIdentity, + TaxCode: lo.ToPtr(testChargeTaxCodeID), + CostBasisCurrency: &settlementCurrency, + CostBasis: &costBasis, + SourceChargeID: &charge.ID, + SpendChargeID: &charge.ID, + }, + transactions.ConvertCurrencyTemplate{ + At: bookedAt, + SourceAmount: alpacadecimal.NewFromInt(10), + TargetAmount: alpacadecimal.NewFromInt(40), + CostBasis: costBasis, + SourceCurrency: currencies.NewCurrencyReference(settlementCurrency), + TargetCurrency: customCurrencyIdentity, + SourceChargeID: &charge.ID, + }, + ) + require.NoError(t, err) + orderedInputs := make([]ledger.TransactionInput, len(tc.order)) + for idx, originalIdx := range tc.order { + orderedInputs[idx] = inputs[originalIdx] + } + group, err := env.Deps.HistoricalLedger.CommitGroup(t.Context(), transactions.GroupInputs(env.Namespace, nil, orderedInputs...)) + require.NoError(t, err) + group, err = env.Deps.HistoricalLedger.GetTransactionGroup(t.Context(), group.ID()) + require.NoError(t, err) + require.Len(t, group.Transactions(), len(orderedInputs)) + for idx, transaction := range group.Transactions() { + require.Equal(t, orderedInputs[idx].Annotations()[ledger.AnnotationTransactionTemplateCode], transaction.Annotations()[ledger.AnnotationTransactionTemplateCode]) + } + run.AccruedUsage = &invoicedusage.AccruedUsage{ + ServicePeriod: run.ServicePeriod, + Totals: totals.Totals{Amount: alpacadecimal.NewFromInt(10), Total: alpacadecimal.NewFromInt(10)}, + LedgerTransaction: &ledgertransaction.GroupReference{ + TransactionGroupID: group.ID().ID, + }, + } + + // when: cleanup corrects the reloaded group + err = env.handler.OnCustomCurrencyOverageAccruedCorrection(t.Context(), flatfee.OnCustomCurrencyOverageAccruedCorrectionInput{ + Charge: charge, + Run: run, + }) + if tc.expectedError != "" { + // then: an incomplete set of accounting legs cannot be corrected + require.ErrorContains(t, err, tc.expectedError) + corrections, err := env.Deps.HistoricalLedger.ListTransactions(t.Context(), ledger.ListTransactionsInput{ + Namespace: env.Namespace, + Limit: 10, + AnnotationFilters: map[string]string{ + ledger.AnnotationTransactionDirection: string(ledger.TransactionDirectionCorrection), + }, + }) + require.NoError(t, err) + require.Empty(t, corrections.Items) + return + } + require.NoError(t, err) + + // then: every native and fiat leg is reversed regardless of enumeration order + for _, subAccount := range []ledger.SubAccount{ + env.FBOSubAccountForCurrency(t, customCurrencyIdentity, &settlementCurrency, costBasis), + env.ReceivableSubAccountForCurrency(t, customCurrencyIdentity, &settlementCurrency, costBasis), + env.AccruedSubAccountForCurrency(t, customCurrencyIdentity, &settlementCurrency, &costBasis, lo.ToPtr(testChargeTaxCodeID)), + env.ReceivableSubAccountWithCostBasis(t, &costBasis), + env.BrokerageSubAccountForCurrency(t, currencies.NewCurrencyReference(settlementCurrency), nil, costBasis), + env.BrokerageSubAccountForCurrency(t, customCurrencyIdentity, &settlementCurrency, costBasis), + } { + require.Equal(t, 0.0, env.sumBalance(t, subAccount).InexactFloat64()) + } + }) + } +} + func TestOnFlatFeeCustomCurrencyOverageAccruedCorrection_NoLedgerTransaction(t *testing.T) { env := newFlatFeeHandlerTestEnv(t) customCurrencyValue := currenciestestutils.NewCustomCurrency(t, "ACME", 2) From e5570ca2a8e4f6fcd076f26886b1fb318c301d98 Mon Sep 17 00:00:00 2001 From: Alex Goth <64845621+GAlexIHU@users.noreply.github.com> Date: Mon, 5 Oct 2026 13:27:46 +0200 Subject: [PATCH 2/5] feat(ledger): support preassigned posting IDs --- openmeter/ledger/README.md | 2 + openmeter/ledger/entry_identity_test.go | 4 + openmeter/ledger/historical/adapter/ledger.go | 40 ++- openmeter/ledger/historical/entry.go | 4 + openmeter/ledger/historical/ledger.go | 15 +- openmeter/ledger/historical/repo.go | 14 + openmeter/ledger/ids.go | 115 +++++++ openmeter/ledger/impact_test.go | 4 + openmeter/ledger/ledger_test.go | 4 + openmeter/ledger/primitives.go | 7 + openmeter/ledger/transactions/input.go | 19 ++ openmeter/ledger/transactions/input_test.go | 293 ++++++++++++++++++ openmeter/ledger/transactions/resolve_test.go | 42 +++ .../transactions/testutils/anytransaction.go | 15 + openmeter/ledger/validations.go | 74 +++++ openmeter/ledger/validations_test.go | 195 ++++++++++++ 16 files changed, 827 insertions(+), 20 deletions(-) create mode 100644 openmeter/ledger/ids.go create mode 100644 openmeter/ledger/transactions/input_test.go diff --git a/openmeter/ledger/README.md b/openmeter/ledger/README.md index a5fd20dff2..f67d14a9d7 100644 --- a/openmeter/ledger/README.md +++ b/openmeter/ledger/README.md @@ -51,6 +51,8 @@ facts stored independently from the journal. currency's precision. - `CommitGroup` validates the whole input, locks every affected parent account, and books the group atomically in the caller's database transaction. +- IDs can be assigned to resolved postings before commit; `ledger.PreassignIDs` + generates missing transaction and entry IDs without posting. - Default routing rules constrain allowed account-type combinations, flow direction, authorization stages, route compatibility, and dimension scope. A balanced transaction can still be invalid. diff --git a/openmeter/ledger/entry_identity_test.go b/openmeter/ledger/entry_identity_test.go index 94771cf5a2..cac0cda5b7 100644 --- a/openmeter/ledger/entry_identity_test.go +++ b/openmeter/ledger/entry_identity_test.go @@ -239,6 +239,10 @@ type validationEntryInput struct { collectionOriginID *string } +func (validationEntryInput) AssignedID() string { + return "" +} + func (e validationEntryInput) PostingAddress() ledger.PostingAddress { return e.address } diff --git a/openmeter/ledger/historical/adapter/ledger.go b/openmeter/ledger/historical/adapter/ledger.go index cbf20dda9c..b6b7d10348 100644 --- a/openmeter/ledger/historical/adapter/ledger.go +++ b/openmeter/ledger/historical/adapter/ledger.go @@ -109,14 +109,20 @@ func (r *repo) BookTransaction(ctx context.Context, groupID models.NamespacedID, if input == nil { return nil, ledger.ErrTransactionInputRequired } + if err := ledger.ValidateAssignedID(input.AssignedID()); err != nil { + return nil, fmt.Errorf("transaction ID: %w", err) + } return entutils.TransactingRepo(ctx, r, func(ctx context.Context, tx *repo) (*ledgerhistorical.Transaction, error) { - entity, err := tx.db.LedgerTransaction.Create(). + create := tx.db.LedgerTransaction.Create(). SetNamespace(groupID.Namespace). SetGroupID(groupID.ID). SetAnnotations(input.Annotations()). - SetBookedAt(input.BookedAt()). - Save(ctx) + SetBookedAt(input.BookedAt()) + if id := input.AssignedID(); id != "" { + create.SetID(id) + } + entity, err := create.Save(ctx) if err != nil { return nil, fmt.Errorf("failed to create ledger transaction: %w", err) } @@ -128,7 +134,13 @@ func (r *repo) BookTransaction(ctx context.Context, groupID models.NamespacedID, routeKeyVersionBySubAccountID := make(map[string]ledger.RoutingKeyVersion, len(entryInputs)) routeBySubAccountID := make(map[string]ledger.Route, len(entryInputs)) createInputs := make([]*db.LedgerEntryCreate, 0, len(entryInputs)) - for _, entryInput := range entryInputs { + for idx, entryInput := range entryInputs { + if entryInput == nil { + return nil, fmt.Errorf("entries[%d]: entry is required", idx) + } + if err := ledger.ValidateAssignedID(entryInput.AssignedID()); err != nil { + return nil, fmt.Errorf("entries[%d] ID: %w", idx, err) + } subAccountID := entryInput.PostingAddress().SubAccountID() route := entryInput.PostingAddress().Route() accountTypesBySubAccountID[subAccountID] = entryInput.PostingAddress().AccountType() @@ -137,7 +149,7 @@ func (r *repo) BookTransaction(ctx context.Context, groupID models.NamespacedID, routeKeyVersionBySubAccountID[subAccountID] = route.RoutingKey().Version() routeBySubAccountID[subAccountID] = route.Route() - createInputs = append(createInputs, tx.db.LedgerEntry.Create(). + create := tx.db.LedgerEntry.Create(). SetNamespace(groupID.Namespace). SetSubAccountID(subAccountID). SetIdentityKey(entryInput.IdentityKey()). @@ -147,7 +159,11 @@ func (r *repo) BookTransaction(ctx context.Context, groupID models.NamespacedID, SetNillableCollectionOriginID(entryInput.Provenance().CollectionOriginID). SetAnnotations(entryInput.Annotations()). SetAmount(entryInput.Amount()). - SetTransactionID(entity.ID)) + SetTransactionID(entity.ID) + if id := entryInput.AssignedID(); id != "" { + create.SetID(id) + } + createInputs = append(createInputs, create) } createdEntries := make([]*db.LedgerEntry, 0, len(createInputs)) @@ -201,11 +217,17 @@ func (r *repo) BookTransaction(ctx context.Context, groupID models.NamespacedID, } func (r *repo) CreateTransactionGroup(ctx context.Context, transactionGroup ledgerhistorical.CreateTransactionGroupInput) (ledgerhistorical.TransactionGroupData, error) { + if err := transactionGroup.Validate(); err != nil { + return ledgerhistorical.TransactionGroupData{}, err + } return entutils.TransactingRepo(ctx, r, func(ctx context.Context, tx *repo) (ledgerhistorical.TransactionGroupData, error) { - entity, err := tx.db.LedgerTransactionGroup.Create(). + create := tx.db.LedgerTransactionGroup.Create(). SetNamespace(transactionGroup.Namespace). - SetAnnotations(transactionGroup.Annotations). - Save(ctx) + SetAnnotations(transactionGroup.Annotations) + if transactionGroup.ID != "" { + create.SetID(transactionGroup.ID) + } + entity, err := create.Save(ctx) if err != nil { return ledgerhistorical.TransactionGroupData{}, fmt.Errorf("failed to create transaction group: %w", err) } diff --git a/openmeter/ledger/historical/entry.go b/openmeter/ledger/historical/entry.go index 022fa607eb..45a9d8971a 100644 --- a/openmeter/ledger/historical/entry.go +++ b/openmeter/ledger/historical/entry.go @@ -91,6 +91,10 @@ func (e *Entry) ID() models.NamespacedID { return models.NamespacedID{Namespace: e.data.Namespace, ID: e.data.ID} } +func (e *Entry) AssignedID() string { + return e.data.ID +} + func (e *Entry) TransactionID() models.NamespacedID { return models.NamespacedID{Namespace: e.data.Namespace, ID: e.data.TransactionID} } diff --git a/openmeter/ledger/historical/ledger.go b/openmeter/ledger/historical/ledger.go index 88c5afa9a1..7ca7154094 100644 --- a/openmeter/ledger/historical/ledger.go +++ b/openmeter/ledger/historical/ledger.go @@ -60,18 +60,10 @@ func (l *Ledger) GetTransactionGroup(ctx context.Context, id models.NamespacedID } func (l *Ledger) CommitGroup(ctx context.Context, group ledger.TransactionGroupInput) (ledger.TransactionGroup, error) { - txInputs := group.Transactions() - - if len(txInputs) == 0 { - return nil, ledger.ErrTransactionGroupEmpty - } - - // 1. Validate each transaction sequentially - for idx, txInput := range txInputs { - if err := ledger.ValidateTransactionInputWith(ctx, txInput, l.routingValidator); err != nil { - return nil, fmt.Errorf("failed to validate transaction at index %d in group: %w", idx, err) - } + if err := ledger.ValidateTransactionGroupInputWith(ctx, group, l.routingValidator); err != nil { + return nil, fmt.Errorf("failed to validate transaction group: %w", err) } + txInputs := group.Transactions() return transaction.Run(ctx, l.repo, func(ctx context.Context) (*TransactionGroup, error) { // 1.1 (lock everything preemptively, not by sub-txs) @@ -88,6 +80,7 @@ func (l *Ledger) CommitGroup(ctx context.Context, group ledger.TransactionGroupI // 3. Create the transactions & the group txG, err := l.repo.CreateTransactionGroup(ctx, CreateTransactionGroupInput{ + ID: group.AssignedID(), Namespace: group.Namespace(), Annotations: group.Annotations(), }) diff --git a/openmeter/ledger/historical/repo.go b/openmeter/ledger/historical/repo.go index 138ae185de..7cd554cbd4 100644 --- a/openmeter/ledger/historical/repo.go +++ b/openmeter/ledger/historical/repo.go @@ -2,6 +2,8 @@ package historical import ( "context" + "errors" + "fmt" "time" "github.com/alpacahq/alpacadecimal" @@ -65,11 +67,23 @@ type CreateTransactionInput struct { } type CreateTransactionGroupInput struct { + ID string Namespace string Annotations models.Annotations } +func (i CreateTransactionGroupInput) Validate() error { + var errs []error + if i.Namespace == "" { + errs = append(errs, errors.New("namespace is required")) + } + if err := ledger.ValidateAssignedID(i.ID); err != nil { + errs = append(errs, fmt.Errorf("ID: %w", err)) + } + return models.NewNillableGenericValidationError(errors.Join(errs...)) +} + type TransactionGroupData struct { ID string Namespace string diff --git a/openmeter/ledger/ids.go b/openmeter/ledger/ids.go new file mode 100644 index 0000000000..2698a23d0d --- /dev/null +++ b/openmeter/ledger/ids.go @@ -0,0 +1,115 @@ +package ledger + +import ( + "fmt" + "slices" + + "github.com/oklog/ulid/v2" + + "github.com/openmeterio/openmeter/pkg/models" +) + +func WithGroupID(input TransactionGroupInput, id string) TransactionGroupInput { + return &identifiedGroupInput{TransactionGroupInput: input, id: id, transactions: slices.Clone(input.Transactions())} +} + +func WithTransactionID(input TransactionInput, id string) TransactionInput { + return &identifiedTransactionInput{TransactionInput: input, id: id, entries: slices.Clone(input.EntryInputs())} +} + +func WithEntryID(input EntryInput, id string) EntryInput { + return &identifiedEntryInput{EntryInput: input, id: id} +} + +// WithEntryInputs replaces the concrete postings while retaining the transaction's +// ID and metadata. It lets callers assign entry IDs after template resolution. +func WithEntryInputs(input TransactionInput, entries ...EntryInput) TransactionInput { + return &identifiedTransactionInput{TransactionInput: input, id: input.AssignedID(), entries: slices.Clone(entries)} +} + +// PreassignIDs materializes missing transaction and entry IDs without posting or +// mutating the input. Supplied IDs are preserved and validated, so returned entry +// IDs can be referenced before the transaction is committed. +func PreassignIDs(input TransactionInput) (TransactionInput, error) { + if input == nil { + return nil, ErrTransactionInputRequired + } + if err := ValidateAssignedID(input.AssignedID()); err != nil { + return nil, fmt.Errorf("transaction ID: %w", err) + } + + entries := input.EntryInputs() + identifiedEntries := make([]EntryInput, len(entries)) + for idx, entry := range entries { + if entry == nil { + return nil, fmt.Errorf("entries[%d]: entry is required", idx) + } + if err := ValidateAssignedID(entry.AssignedID()); err != nil { + return nil, fmt.Errorf("entries[%d] ID: %w", idx, err) + } + identifiedEntries[idx] = entry + if entry.AssignedID() == "" { + identifiedEntries[idx] = WithEntryID(entry, ulid.Make().String()) + } + } + + id := input.AssignedID() + if id == "" { + id = ulid.Make().String() + } + return &identifiedTransactionInput{TransactionInput: input, id: id, entries: identifiedEntries}, nil +} + +type identifiedGroupInput struct { + TransactionGroupInput + id string + transactions []TransactionInput +} + +var _ TransactionGroupInput = (*identifiedGroupInput)(nil) + +func (i *identifiedGroupInput) AssignedID() string { + return i.id +} + +func (i *identifiedGroupInput) Transactions() []TransactionInput { + return slices.Clone(i.transactions) +} + +type identifiedTransactionInput struct { + TransactionInput + id string + entries []EntryInput +} + +var _ TransactionInput = (*identifiedTransactionInput)(nil) + +func (i *identifiedTransactionInput) AssignedID() string { + return i.id +} + +func (i *identifiedTransactionInput) EntryInputs() []EntryInput { + return slices.Clone(i.entries) +} + +func (i *identifiedTransactionInput) AsGroupInput(namespace string, annotations models.Annotations) TransactionGroupInput { + // Reuse the underlying group metadata, but retain this wrapper as its + // transaction so grouping cannot discard assigned IDs or entry overrides. + group := i.TransactionInput.AsGroupInput(namespace, annotations) + return &identifiedGroupInput{ + TransactionGroupInput: group, + id: group.AssignedID(), + transactions: []TransactionInput{i}, + } +} + +type identifiedEntryInput struct { + EntryInput + id string +} + +var _ EntryInput = (*identifiedEntryInput)(nil) + +func (i *identifiedEntryInput) AssignedID() string { + return i.id +} diff --git a/openmeter/ledger/impact_test.go b/openmeter/ledger/impact_test.go index 8df53c9f91..2efd5b3b70 100644 --- a/openmeter/ledger/impact_test.go +++ b/openmeter/ledger/impact_test.go @@ -390,6 +390,10 @@ func (e impactTestEntry) ID() models.NamespacedID { return e.id } +func (e impactTestEntry) AssignedID() string { + return e.id.ID +} + func (e impactTestEntry) TransactionID() models.NamespacedID { return e.txID } diff --git a/openmeter/ledger/ledger_test.go b/openmeter/ledger/ledger_test.go index bb87af7d0d..3bfc9378b8 100644 --- a/openmeter/ledger/ledger_test.go +++ b/openmeter/ledger/ledger_test.go @@ -21,6 +21,10 @@ type exampleEntryInput struct { amount alpacadecimal.Decimal } +func (exampleEntryInput) AssignedID() string { + return "" +} + func (e exampleEntryInput) PostingAddress() ledger.PostingAddress { return e.account } diff --git a/openmeter/ledger/primitives.go b/openmeter/ledger/primitives.go index 1bd38e0afa..bd19931f0b 100644 --- a/openmeter/ledger/primitives.go +++ b/openmeter/ledger/primitives.go @@ -80,6 +80,9 @@ type Account interface { // ---------------------------------------------------------------------------- type EntryInput interface { + // AssignedID is an optional database ID, distinct from accounting identity. + // Empty IDs are generated when the entry is committed. + AssignedID() string PostingAddress() PostingAddress Amount() alpacadecimal.Decimal IdentityKey() string @@ -103,6 +106,8 @@ type Entry interface { } type TransactionInput interface { + // AssignedID is preserved on commit; empty IDs are generated at persistence. + AssignedID() string BookedAt() time.Time EntryInputs() []EntryInput Annotations() models.Annotations @@ -169,6 +174,8 @@ func (c TransactionCursor) Validate() error { } type TransactionGroupInput interface { + // AssignedID is preserved on commit; empty IDs are generated at persistence. + AssignedID() string Namespace() string Transactions() []TransactionInput Annotations() models.Annotations diff --git a/openmeter/ledger/transactions/input.go b/openmeter/ledger/transactions/input.go index f45a6767f2..0e30499efc 100644 --- a/openmeter/ledger/transactions/input.go +++ b/openmeter/ledger/transactions/input.go @@ -11,6 +11,7 @@ import ( ) type EntryInput struct { + id string amount alpacadecimal.Decimal address ledger.PostingAddress identity ledger.EntryIdentityParts @@ -23,6 +24,10 @@ type EntryInput struct { var _ ledger.EntryInput = (*EntryInput)(nil) +func (e *EntryInput) AssignedID() string { + return e.id +} + func (e *EntryInput) PostingAddress() ledger.PostingAddress { return e.address } @@ -53,6 +58,7 @@ func (e *EntryInput) Annotations() models.Annotations { } type TransactionInput struct { + id string bookedAt time.Time entryInputs []*EntryInput annotations models.Annotations @@ -64,6 +70,10 @@ type TransactionInput struct { var _ ledger.TransactionInput = (*TransactionInput)(nil) +func (t *TransactionInput) AssignedID() string { + return t.id +} + func (t *TransactionInput) BookedAt() time.Time { return t.bookedAt } @@ -122,7 +132,12 @@ func (a *annotatedTransactionInput) Annotations() models.Annotations { return a.annotations } +func (a *annotatedTransactionInput) AsGroupInput(namespace string, annotations models.Annotations) ledger.TransactionGroupInput { + return GroupInputs(namespace, annotations, a) +} + type TransactionGroupInput struct { + id string namespace string transactions []ledger.TransactionInput annotations models.Annotations @@ -130,6 +145,10 @@ type TransactionGroupInput struct { var _ ledger.TransactionGroupInput = (*TransactionGroupInput)(nil) +func (t *TransactionGroupInput) AssignedID() string { + return t.id +} + // ---------------------------------------------------------------------------- // Let's implement ledger.TransactionGroupInput interface // ---------------------------------------------------------------------------- diff --git a/openmeter/ledger/transactions/input_test.go b/openmeter/ledger/transactions/input_test.go new file mode 100644 index 0000000000..d49b4b8712 --- /dev/null +++ b/openmeter/ledger/transactions/input_test.go @@ -0,0 +1,293 @@ +package transactions + +import ( + "testing" + + "github.com/alpacahq/alpacadecimal" + "github.com/oklog/ulid/v2" + "github.com/stretchr/testify/require" + + "github.com/openmeterio/openmeter/openmeter/ledger" + "github.com/openmeterio/openmeter/pkg/models" +) + +func TestCommitGroup_AssignedIDs(t *testing.T) { + t.Run("generated at commit", func(t *testing.T) { + env := newTransactionsTestEnv(t) + + // given: a resolved credit issuance without assigned IDs + input := env.creditIssuanceInput(t, 10) + + // when/then: committing generates IDs without changing the original input + env.requireCommittedIDs(t, input, "") + requireUnassignedIDs(t, input) + }) + + t.Run("preassigned before commit", func(t *testing.T) { + env := newTransactionsTestEnv(t) + + // given: every posting is identified before it is committed + original := env.creditIssuanceInput(t, 10) + input, err := ledger.PreassignIDs(original) + require.NoError(t, err) + requireStablePreassignedIDs(t, input) + + // when/then: committing preserves the preassigned references + env.requireCommittedIDs(t, input, "") + requireUnassignedIDs(t, original) + }) + + t.Run("mixed supplied and generated", func(t *testing.T) { + env := newTransactionsTestEnv(t) + + // given: the group, transaction and one entry have caller-supplied IDs + original := env.creditIssuanceInput(t, 10) + input := ledger.WithTransactionID(original, ulid.Make().String()) + entries := input.EntryInputs() + entries[0] = ledger.WithEntryID(entries[0], ulid.Make().String()) + input = ledger.WithEntryInputs(input, entries...) + groupID := ulid.Make().String() + + // when/then: committing preserves supplied IDs and generates the missing one + env.requireCommittedIDs(t, input, groupID) + requireUnassignedIDs(t, original) + }) + + t.Run("supplied IDs survive preassignment", func(t *testing.T) { + env := newTransactionsTestEnv(t) + + // given: preassignment fills only the missing posting IDs + original := env.creditIssuanceInput(t, 10) + transactionID := ulid.Make().String() + entryID := ulid.Make().String() + input := ledger.WithTransactionID(original, transactionID) + entries := input.EntryInputs() + entries[0] = ledger.WithEntryID(entries[0], entryID) + input = ledger.WithEntryInputs(input, entries...) + + input, err := ledger.PreassignIDs(input) + require.NoError(t, err) + require.Equal(t, transactionID, input.AssignedID()) + require.Equal(t, entryID, input.EntryInputs()[0].AssignedID()) + requireStablePreassignedIDs(t, input) + + // when/then: the prepared posting retains every ID through persistence + env.requireCommittedIDs(t, input, ulid.Make().String()) + requireUnassignedIDs(t, original) + }) +} + +func (e *transactionsTestEnv) creditIssuanceInput(t *testing.T, amount int64) ledger.TransactionInput { + t.Helper() + + return e.resolve(t, IssueCustomerReceivableTemplate{ + At: e.Now(), + Amount: alpacadecimal.NewFromInt(amount), + Currency: e.CurrencyReference(), + })[0] +} + +func (e *transactionsTestEnv) requireCommittedIDs(t *testing.T, input ledger.TransactionInput, groupID string) { + t.Helper() + + input = WithAnnotations(input, models.Annotations{"test": "assigned IDs"}) + groupInput := ledger.WithGroupID(input.AsGroupInput(e.Namespace, models.Annotations{"group": "assigned IDs"}), groupID) + + group, err := e.Deps.HistoricalLedger.CommitGroup(t.Context(), groupInput) + require.NoError(t, err) + + group, err = e.Deps.HistoricalLedger.GetTransactionGroup(t.Context(), group.ID()) + require.NoError(t, err) + + if groupID != "" { + require.Equal(t, groupID, group.ID().ID) + } + require.NotEmpty(t, group.ID().ID) + require.NoError(t, ledger.ValidateAssignedID(group.ID().ID)) + require.Equal(t, groupInput.Annotations(), group.Annotations()) + require.Len(t, group.Transactions(), 1) + + committed := group.Transactions()[0] + if input.AssignedID() != "" { + require.Equal(t, input.AssignedID(), committed.ID().ID) + } + require.NotEmpty(t, committed.ID().ID) + require.NoError(t, ledger.ValidateAssignedID(committed.ID().ID)) + require.Equal(t, input.Annotations(), committed.Annotations()) + require.Len(t, committed.Entries(), len(input.EntryInputs())) + + for _, entryInput := range input.EntryInputs() { + var matched ledger.Entry + for _, entry := range committed.Entries() { + if entry.PostingAddress().SubAccountID() == entryInput.PostingAddress().SubAccountID() { + matched = entry + break + } + } + + require.NotNil(t, matched) + if entryInput.AssignedID() != "" { + require.Equal(t, entryInput.AssignedID(), matched.ID().ID) + } + require.NotEmpty(t, matched.ID().ID) + require.NoError(t, ledger.ValidateAssignedID(matched.ID().ID)) + require.Equal(t, entryInput.Amount().InexactFloat64(), matched.Amount().InexactFloat64()) + require.Equal(t, entryInput.IdentityKey(), matched.IdentityKey()) + } + + require.Equal(t, 10.0, e.SumBalance(t, e.FBOSubAccount(t, ledger.DefaultCustomerFBOPriority)).InexactFloat64()) + require.Equal(t, -10.0, e.SumBalance(t, e.ReceivableSubAccount(t)).InexactFloat64()) +} + +func requireUnassignedIDs(t *testing.T, input ledger.TransactionInput) { + t.Helper() + + require.Empty(t, input.AssignedID()) + for _, entry := range input.EntryInputs() { + require.Empty(t, entry.AssignedID()) + } +} + +func requireStablePreassignedIDs(t *testing.T, input ledger.TransactionInput) { + t.Helper() + + again, err := ledger.PreassignIDs(input) + require.NoError(t, err) + require.NotEmpty(t, input.AssignedID()) + require.Equal(t, input.AssignedID(), again.AssignedID()) + + for idx, entry := range input.EntryInputs() { + require.NotEmpty(t, entry.AssignedID()) + require.Equal(t, entry.AssignedID(), again.EntryInputs()[idx].AssignedID()) + } +} + +func TestCommitGroup_RejectsDuplicateEntryIDs(t *testing.T) { + env := newTransactionsTestEnv(t) + + // given: two balanced transactions reuse the same entry primary key + first := env.creditIssuanceInput(t, 10) + second := env.creditIssuanceInput(t, 5) + id := ulid.Make().String() + firstEntries := first.EntryInputs() + firstEntries[0] = ledger.WithEntryID(firstEntries[0], id) + first = ledger.WithEntryInputs(first, firstEntries...) + secondEntries := second.EntryInputs() + secondEntries[0] = ledger.WithEntryID(secondEntries[0], id) + second = ledger.WithEntryInputs(second, secondEntries...) + + // when: the group is submitted to the ledger + _, err := env.Deps.HistoricalLedger.CommitGroup(t.Context(), GroupInputs(env.Namespace, nil, first, second)) + require.ErrorContains(t, err, "duplicate entry ID") + + // then: group validation prevents any accounting rows from being persisted + groups, err := env.DB.LedgerTransactionGroup.Query().Count(t.Context()) + require.NoError(t, err) + require.Zero(t, groups) + + entries, err := env.DB.LedgerEntry.Query().Count(t.Context()) + require.NoError(t, err) + require.Zero(t, entries) +} + +func TestCommitGroup_IDCollisionRollsBack(t *testing.T) { + setup := func(t *testing.T) (*transactionsTestEnv, ledger.TransactionGroup, []ledger.TransactionInput) { + t.Helper() + + env := newTransactionsTestEnv(t) + original := env.creditIssuanceInput(t, 10) + committed, err := env.Deps.HistoricalLedger.CommitGroup(t.Context(), original.AsGroupInput(env.Namespace, nil)) + require.NoError(t, err) + + inputs := []ledger.TransactionInput{ + env.creditIssuanceInput(t, 7), + env.creditIssuanceInput(t, 7), + } + + return env, committed, inputs + } + + requireRollback := func(t *testing.T, env *transactionsTestEnv, group ledger.TransactionGroupInput) { + t.Helper() + + _, err := env.Deps.HistoricalLedger.CommitGroup(t.Context(), group) + require.Error(t, err) + + groups, err := env.DB.LedgerTransactionGroup.Query().Count(t.Context()) + require.NoError(t, err) + require.Equal(t, 1, groups) + + txs, err := env.DB.LedgerTransaction.Query().Count(t.Context()) + require.NoError(t, err) + require.Equal(t, 1, txs) + + entries, err := env.DB.LedgerEntry.Query().Count(t.Context()) + require.NoError(t, err) + require.Equal(t, 2, entries) + + require.Equal(t, 10.0, env.SumBalance(t, env.FBOSubAccount(t, ledger.DefaultCustomerFBOPriority)).InexactFloat64()) + require.Equal(t, -10.0, env.SumBalance(t, env.ReceivableSubAccount(t)).InexactFloat64()) + } + + t.Run("group collision", func(t *testing.T) { + // given: an existing posting owns the requested group ID + env, committed, inputs := setup(t) + group := ledger.WithGroupID(GroupInputs(env.Namespace, nil, inputs...), committed.ID().ID) + + // when/then: the collision leaves only the original posting + requireRollback(t, env, group) + }) + + t.Run("transaction collision after an earlier transaction posts", func(t *testing.T) { + // given: the second transaction reuses an existing transaction ID + env, committed, inputs := setup(t) + inputs[1] = ledger.WithTransactionID(inputs[1], committed.Transactions()[0].ID().ID) + group := ledger.WithGroupID(GroupInputs(env.Namespace, nil, inputs...), ulid.Make().String()) + + // when/then: the collision rolls back the first transaction as well + requireRollback(t, env, group) + }) + + t.Run("entry collision after an earlier transaction posts", func(t *testing.T) { + // given: the second transaction reuses an existing entry ID + env, committed, inputs := setup(t) + entries := inputs[1].EntryInputs() + entries[0] = ledger.WithEntryID(entries[0], committed.Transactions()[0].Entries()[0].ID().ID) + inputs[1] = ledger.WithEntryInputs(inputs[1], entries...) + group := ledger.WithGroupID(GroupInputs(env.Namespace, nil, inputs...), ulid.Make().String()) + + // when/then: the collision rolls back all entries from the new group + requireRollback(t, env, group) + }) +} + +func TestPreassignIDs_RejectsInvalidInput(t *testing.T) { + for _, tc := range []struct { + name string + input ledger.TransactionInput + }{ + { + name: "missing transaction", + }, + { + name: "invalid transaction ID", + input: &TransactionInput{id: "invalid"}, + }, + { + name: "invalid entry ID", + input: &TransactionInput{ + entryInputs: []*EntryInput{{id: "invalid"}}, + }, + }, + { + name: "missing entry", + input: ledger.WithEntryInputs(&TransactionInput{}, nil), + }, + } { + t.Run(tc.name, func(t *testing.T) { + input, err := ledger.PreassignIDs(tc.input) + require.Error(t, err) + require.Nil(t, input) + }) + } +} diff --git a/openmeter/ledger/transactions/resolve_test.go b/openmeter/ledger/transactions/resolve_test.go index 8b83766847..c3938254e9 100644 --- a/openmeter/ledger/transactions/resolve_test.go +++ b/openmeter/ledger/transactions/resolve_test.go @@ -4,10 +4,12 @@ import ( "context" "testing" + "github.com/oklog/ulid/v2" "github.com/stretchr/testify/require" "github.com/openmeterio/openmeter/openmeter/customer" "github.com/openmeterio/openmeter/openmeter/ledger" + "github.com/openmeterio/openmeter/pkg/models" ) // spyCustomerTemplate implements CustomerTransactionTemplate and records Validate calls. @@ -65,3 +67,43 @@ func TestResolveTransactions_addsTemplateAnnotations(t *testing.T) { require.Equal(t, string(TemplateCodeIssueCustomerReceivable), input.Annotations()[ledger.AnnotationTransactionTemplateCode]) require.Equal(t, string(ledger.TransactionDirectionForward), input.Annotations()[ledger.AnnotationTransactionDirection]) } + +func TestDecoratedTransactionInput_AsGroupInput(t *testing.T) { + id := ulid.Make().String() + entryID := ulid.Make().String() + original := &TransactionInput{entryInputs: []*EntryInput{{}}} + input := ledger.WithEntryInputs(original, ledger.WithEntryID(original.EntryInputs()[0], entryID)) + annotations := models.Annotations{"source": "decorated input"} + groupAnnotations := models.Annotations{"group": "decorated group"} + + requireDecoratedGroup := func(t *testing.T, input ledger.TransactionInput) { + t.Helper() + + group := input.AsGroupInput("ns-test", groupAnnotations) + require.Equal(t, "ns-test", group.Namespace()) + require.Equal(t, groupAnnotations, group.Annotations()) + require.Len(t, group.Transactions(), 1) + + transaction := group.Transactions()[0] + require.Equal(t, id, transaction.AssignedID()) + require.Equal(t, annotations, transaction.Annotations()) + require.Len(t, transaction.EntryInputs(), 1) + require.Equal(t, entryID, transaction.EntryInputs()[0].AssignedID()) + } + + t.Run("IDs before annotations", func(t *testing.T) { + // given: annotations wrap an already identified transaction + decorated := WithAnnotations(ledger.WithTransactionID(input, id), annotations) + + // when/then: grouping retains both wrappers' data + requireDecoratedGroup(t, decorated) + }) + + t.Run("annotations before IDs", func(t *testing.T) { + // given: ID assignment wraps an already annotated transaction + decorated := ledger.WithTransactionID(WithAnnotations(input, annotations), id) + + // when/then: grouping retains both wrappers' data + requireDecoratedGroup(t, decorated) + }) +} diff --git a/openmeter/ledger/transactions/testutils/anytransaction.go b/openmeter/ledger/transactions/testutils/anytransaction.go index e7fa81aea0..d6f4895d07 100644 --- a/openmeter/ledger/transactions/testutils/anytransaction.go +++ b/openmeter/ledger/transactions/testutils/anytransaction.go @@ -11,6 +11,7 @@ import ( ) type AnyEntryInput struct { + IDValue string Address ledger.PostingAddress AmountValue alpacadecimal.Decimal IdentityKeyValue string @@ -23,6 +24,10 @@ type AnyEntryInput struct { var _ ledger.EntryInput = (*AnyEntryInput)(nil) +func (a *AnyEntryInput) AssignedID() string { + return a.IDValue +} + func (a *AnyEntryInput) PostingAddress() ledger.PostingAddress { return a.Address } @@ -56,6 +61,7 @@ func (a *AnyEntryInput) Annotations() models.Annotations { } type AnyTransactionInput struct { + IDValue string BookedAtValue time.Time EntryInputsValues []*AnyEntryInput AnnotationsValue models.Annotations @@ -63,6 +69,10 @@ type AnyTransactionInput struct { var _ ledger.TransactionInput = (*AnyTransactionInput)(nil) +func (a *AnyTransactionInput) AssignedID() string { + return a.IDValue +} + func (a *AnyTransactionInput) BookedAt() time.Time { return a.BookedAtValue } @@ -82,6 +92,7 @@ func (a *AnyTransactionInput) AsGroupInput(namespace string, annotations models. } type AnyTransactionGroupInput struct { + IDValue string NamespaceValue string TransactionsValues []*AnyTransactionInput AnnotationsValue models.Annotations @@ -89,6 +100,10 @@ type AnyTransactionGroupInput struct { var _ ledger.TransactionGroupInput = (*AnyTransactionGroupInput)(nil) +func (a *AnyTransactionGroupInput) AssignedID() string { + return a.IDValue +} + func (a *AnyTransactionGroupInput) Namespace() string { return a.NamespaceValue } diff --git a/openmeter/ledger/validations.go b/openmeter/ledger/validations.go index d430a55223..d22f055c41 100644 --- a/openmeter/ledger/validations.go +++ b/openmeter/ledger/validations.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/alpacahq/alpacadecimal" + "github.com/oklog/ulid/v2" "github.com/samber/lo" "github.com/openmeterio/openmeter/openmeter/currencies" @@ -61,6 +62,13 @@ func ValidateEntryInput(ctx context.Context, entry EntryInput) error { }) } + if err := ValidateAssignedID(entry.AssignedID()); err != nil { + return ErrEntryInvalid.WithAttrs(models.Attributes{ + "reason": "invalid_id", + "error": err, + }) + } + // Let's validate the address if err := ValidateAddress(ctx, entry.PostingAddress()); err != nil { return ErrEntryInvalid.WithAttrs(models.Attributes{ @@ -149,6 +157,10 @@ func ValidateTransactionInputWith(ctx context.Context, transaction TransactionIn return ErrTransactionInputRequired } + if err := ValidateAssignedID(transaction.AssignedID()); err != nil { + return models.NewGenericValidationError(fmt.Errorf("transaction ID: %w", err)) + } + entries := lo.Map(transaction.EntryInputs(), func(e EntryInput, _ int) EntryInput { return e }) @@ -176,6 +188,68 @@ func ValidateTransactionInputWith(ctx context.Context, transaction TransactionIn return nil } +// ValidateAssignedID permits omission for generated IDs and rejects supplied +// values that cannot represent the ledger's ULID primary keys. +func ValidateAssignedID(id string) error { + if id == "" { + return nil + } + if _, err := ulid.ParseStrict(id); err != nil { + return fmt.Errorf("invalid ledger ID: %w", err) + } + return nil +} + +// ValidateTransactionGroupInputWith validates all postings and supplied IDs before +// persistence. IDs are unique per entity type, so duplicate entry IDs are checked +// across transactions as well as within each transaction. +func ValidateTransactionGroupInputWith(ctx context.Context, group TransactionGroupInput, routingValidator RoutingValidator) error { + if group == nil { + return models.NewGenericValidationError(errors.New("transaction group is required")) + } + + var errs []error + if group.Namespace() == "" { + errs = append(errs, errors.New("namespace is required")) + } + if err := ValidateAssignedID(group.AssignedID()); err != nil { + errs = append(errs, fmt.Errorf("group ID: %w", err)) + } + + inputs := group.Transactions() + if len(inputs) == 0 { + errs = append(errs, ErrTransactionGroupEmpty) + } + transactionIDs := make(map[string]struct{}, len(inputs)) + entryIDs := make(map[string]struct{}) + for idx, input := range inputs { + if err := ValidateTransactionInputWith(ctx, input, routingValidator); err != nil { + errs = append(errs, fmt.Errorf("transactions[%d]: %w", idx, err)) + } + if input == nil { + continue + } + if id := input.AssignedID(); id != "" { + if _, exists := transactionIDs[id]; exists { + errs = append(errs, fmt.Errorf("transactions[%d]: duplicate transaction ID %s", idx, id)) + } + transactionIDs[id] = struct{}{} + } + for entryIdx, entry := range input.EntryInputs() { + if entry == nil { + continue + } + if id := entry.AssignedID(); id != "" { + if _, exists := entryIDs[id]; exists { + errs = append(errs, fmt.Errorf("transactions[%d].entries[%d]: duplicate entry ID %s", idx, entryIdx, id)) + } + entryIDs[id] = struct{}{} + } + } + } + return models.NewNillableGenericValidationError(errors.Join(errs...)) +} + // ValidateOriginProvenance prevents a balanced transaction from silently moving // value between collection occurrences or dropping its immutable spend identity. // Cost-basis translation may add/remove source attribution on the same account diff --git a/openmeter/ledger/validations_test.go b/openmeter/ledger/validations_test.go index 777f66d3ea..f1ccdfd968 100644 --- a/openmeter/ledger/validations_test.go +++ b/openmeter/ledger/validations_test.go @@ -1,11 +1,13 @@ package ledger_test import ( + "errors" "fmt" "testing" "time" "github.com/alpacahq/alpacadecimal" + "github.com/oklog/ulid/v2" "github.com/samber/lo" "github.com/samber/mo" "github.com/stretchr/testify/require" @@ -13,10 +15,203 @@ import ( "github.com/openmeterio/openmeter/openmeter/currencies" "github.com/openmeterio/openmeter/openmeter/ledger" ledgeraccount "github.com/openmeterio/openmeter/openmeter/ledger/account" + "github.com/openmeterio/openmeter/openmeter/ledger/transactions" "github.com/openmeterio/openmeter/openmeter/ledger/transactions/testutils" "github.com/openmeterio/openmeter/pkg/currencyx" ) +func TestValidateTransactionGroupInputWith(t *testing.T) { + address := mustPostingAddress(t, currencyx.Code("USD")) + at := time.Date(2026, time.January, 1, 0, 0, 0, 0, time.UTC) + id := ulid.Make().String() + + newTransaction := func(id string, entryIDs ...string) *testutils.AnyTransactionInput { + input := &testutils.AnyTransactionInput{ + IDValue: id, + BookedAtValue: at, + EntryInputsValues: []*testutils.AnyEntryInput{ + { + Address: address, + AmountValue: alpacadecimal.NewFromInt(10), + }, + { + Address: address, + AmountValue: alpacadecimal.NewFromInt(-10), + }, + }, + } + + for idx, id := range entryIDs { + input.EntryInputsValues[idx].IDValue = id + } + + return input + } + + unbalanced := newTransaction("") + unbalanced.EntryInputsValues[1].AmountValue = alpacadecimal.NewFromInt(-9) + + for _, tc := range []struct { + name string + input ledger.TransactionGroupInput + expectedError error + expectedMessages []string + }{ + { + name: "accepts omitted IDs across multiple transactions", + input: transactions.GroupInputs("ns-test", nil, newTransaction(""), newTransaction("")), + }, + { + name: "accepts the same ID across different entity types", + input: ledger.WithGroupID( + transactions.GroupInputs("ns-test", nil, newTransaction(id, id, ulid.Make().String())), + id, + ), + }, + { + name: "accepts mixed supplied and omitted IDs", + input: transactions.GroupInputs("ns-test", nil, newTransaction(id, ulid.Make().String()), newTransaction("")), + }, + { + name: "missing group", + expectedMessages: []string{"transaction group is required"}, + }, + { + name: "missing namespace", + input: transactions.GroupInputs("", nil, newTransaction("")), + expectedMessages: []string{"namespace is required"}, + }, + { + name: "empty group", + input: transactions.GroupInputs("ns-test", nil), + expectedError: ledger.ErrTransactionGroupEmpty, + }, + { + name: "invalid group ID", + input: ledger.WithGroupID( + transactions.GroupInputs("ns-test", nil, newTransaction("")), + "invalid", + ), + expectedMessages: []string{"group ID"}, + }, + { + name: "invalid transaction ID", + input: transactions.GroupInputs("ns-test", nil, newTransaction("invalid")), + expectedMessages: []string{"transactions[0]:", "transaction ID"}, + }, + { + name: "invalid entry ID", + input: transactions.GroupInputs("ns-test", nil, newTransaction("", "invalid")), + expectedError: ledger.ErrEntryInvalid, + }, + { + name: "duplicate transaction IDs", + input: transactions.GroupInputs("ns-test", nil, newTransaction(id), newTransaction(id)), + expectedMessages: []string{"transactions[1]: duplicate transaction ID"}, + }, + { + name: "duplicate entry IDs within a transaction", + input: transactions.GroupInputs("ns-test", nil, newTransaction("", id, id)), + expectedMessages: []string{"transactions[0].entries[1]: duplicate entry ID"}, + }, + { + name: "duplicate entry IDs across transactions", + input: transactions.GroupInputs("ns-test", nil, newTransaction("", id), newTransaction("", id)), + expectedMessages: []string{"transactions[1].entries[0]: duplicate entry ID"}, + }, + { + name: "missing transaction", + input: transactions.GroupInputs("ns-test", nil, nil), + expectedError: ledger.ErrTransactionInputRequired, + }, + { + name: "missing entry", + input: transactions.GroupInputs("ns-test", nil, + ledger.WithEntryInputs(newTransaction(""), nil), + ), + expectedError: ledger.ErrEntryInvalid, + }, + { + name: "unbalanced transaction", + input: transactions.GroupInputs("ns-test", nil, unbalanced), + expectedError: ledger.ErrInvalidTransactionTotal, + }, + { + name: "reports independent scope and posting errors together", + input: ledger.WithGroupID( + transactions.GroupInputs("", nil, newTransaction("invalid"), newTransaction("", "invalid")), + "invalid", + ), + expectedMessages: []string{ + "namespace is required", + "group ID", + "transactions[0]:", + "transaction ID", + "transactions[1]: ledger entry is invalid", + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + err := ledger.ValidateTransactionGroupInputWith(t.Context(), tc.input, nil) + if tc.expectedError == nil && len(tc.expectedMessages) == 0 { + require.NoError(t, err) + + return + } + + require.Error(t, err) + if tc.expectedError != nil { + require.ErrorIs(t, err, tc.expectedError) + } + + for _, message := range tc.expectedMessages { + require.ErrorContains(t, err, message) + } + }) + } + + t.Run("validates routing for every transaction", func(t *testing.T) { + // given: a group with two independently balanced transactions + first, second := newTransaction(""), newTransaction("") + group := transactions.GroupInputs("ns-test", nil, first, second) + validator := &groupValidationRoutingValidator{} + + // when: the group is validated with an injected routing policy + err := ledger.ValidateTransactionGroupInputWith(t.Context(), group, validator) + require.NoError(t, err) + + // then: routing sees each transaction's complete postings + require.Equal(t, [][]ledger.EntryInput{first.EntryInputs(), second.EntryInputs()}, validator.entries) + }) + + t.Run("propagates routing rejection", func(t *testing.T) { + // given: the routing policy rejects a balanced posting + group := transactions.GroupInputs("ns-test", nil, newTransaction("")) + routingError := errors.New("routing rejected") + validator := &groupValidationRoutingValidator{err: routingError} + + // when: the group is validated + err := ledger.ValidateTransactionGroupInputWith(t.Context(), group, validator) + + // then: the policy's error is retained with transaction context + require.ErrorIs(t, err, routingError) + require.ErrorContains(t, err, "transactions[0]") + }) +} + +type groupValidationRoutingValidator struct { + entries [][]ledger.EntryInput + err error +} + +var _ ledger.RoutingValidator = (*groupValidationRoutingValidator)(nil) + +func (v *groupValidationRoutingValidator) ValidateEntries(entries []ledger.EntryInput) error { + v.entries = append(v.entries, entries) + + return v.err +} + func TestValidateTransactionInputCurrencyAccounting(t *testing.T) { for _, testCase := range []struct { name string From 2d3138d4048649fb0699a2d211db7852bd1e7877 Mon Sep 17 00:00:00 2001 From: Alex Goth <64845621+GAlexIHU@users.noreply.github.com> Date: Mon, 5 Oct 2026 16:13:47 +0200 Subject: [PATCH 3/5] refactor(ledger): persist breakage before posting --- atlas.hcl | 6 +- .../ent/schema/ledger_breakage_record.go | 4 +- openmeter/ledger/advance/README.md | 5 +- openmeter/ledger/advance/correction.go | 33 +- .../ledger/advance/service/correction.go | 5 +- .../advance/service/legacy_correction.go | 16 +- openmeter/ledger/breakage/README.md | 10 +- openmeter/ledger/breakage/noop.go | 16 +- openmeter/ledger/breakage/service.go | 243 +++++++-------- openmeter/ledger/breakage/service_test.go | 288 ++++++++++++++++++ openmeter/ledger/breakage/types.go | 34 ++- .../ledger/chargeadapter/creditpurchase.go | 17 +- openmeter/ledger/collector/collect.go | 241 +++++++++------ .../ledger/collector/collection_fbo_test.go | 46 ++- .../ledger/collector/correction/breakage.go | 19 +- .../ledger/collector/correction/correct.go | 32 +- .../ledger/collector/correction/input.go | 2 + .../collector/correction/legacy_correction.go | 7 +- .../collector/correction/origin_correction.go | 5 +- .../ledger/collector/correction/prepare.go | 7 +- openmeter/ledger/creditvoid/service.go | 14 +- .../customerbalance/expired_loader_test.go | 81 +++-- tools/migrate/README.md | 5 + tools/migrate/deferred_constraints.sql | 8 + tools/migrate/entschema/main.go | 52 ++++ .../ledger_breakage_references_test.go | 70 +++++ ..._defer_breakage_ledger_references.down.sql | 2 + ...27_defer_breakage_ledger_references.up.sql | 2 + tools/migrate/migrations/atlas.sum | 3 +- 29 files changed, 879 insertions(+), 394 deletions(-) create mode 100644 openmeter/ledger/breakage/service_test.go create mode 100644 tools/migrate/deferred_constraints.sql create mode 100644 tools/migrate/entschema/main.go create mode 100644 tools/migrate/ledger_breakage_references_test.go create mode 100644 tools/migrate/migrations/20261005135427_defer_breakage_ledger_references.down.sql create mode 100644 tools/migrate/migrations/20261005135427_defer_breakage_ledger_references.up.sql diff --git a/atlas.hcl b/atlas.hcl index a3d6fa24cd..b3c4c63f86 100644 --- a/atlas.hcl +++ b/atlas.hcl @@ -1,3 +1,7 @@ +data "external_schema" "ent" { + program = ["go", "run", "-mod=readonly", "./tools/migrate/entschema"] +} + env "local" { src = "${local.schema_src}" @@ -45,7 +49,7 @@ env "ci" { locals { // Define the directory where the schema definition resides. - schema_src = "ent://openmeter/ent/schema" + schema_src = data.external_schema.ent.url // Define the initial migration timestamp init_migration_ts = "20240826120919" // Define the directory where the migrations are stored. diff --git a/openmeter/ent/schema/ledger_breakage_record.go b/openmeter/ent/schema/ledger_breakage_record.go index f89550c494..3e787ce7ef 100644 --- a/openmeter/ent/schema/ledger_breakage_record.go +++ b/openmeter/ent/schema/ledger_breakage_record.go @@ -18,8 +18,8 @@ type LedgerBreakageRecord struct { ent.Schema } -// Breakage records intentionally keep ledger references as plain IDs: they are -// projection rows, not accounting source of truth. See ledger/breakage/README.md. +// Ledger-reference FKs are deferred by tools/migrate/deferred_constraints.sql: +// bookkeeping is written before the journal group in the same transaction. func (LedgerBreakageRecord) Mixin() []ent.Mixin { return []ent.Mixin{ entutils.IDMixin{}, diff --git a/openmeter/ledger/advance/README.md b/openmeter/ledger/advance/README.md index 024eb6be47..79fe328c02 100644 --- a/openmeter/ledger/advance/README.md +++ b/openmeter/ledger/advance/README.md @@ -57,5 +57,6 @@ the purchase was fully consumed by backfill and issued no ordinary FBO credit. `PlanLegacyCorrection` uses original transaction groups for legacy histories. It returns deferred template corrections so the collector can merge selections -against the same transaction before resolving them. Both paths return breakage -records for the caller to persist with the committed group; neither writes lineage. +against the same transaction before resolving them. Both paths write breakage +bookkeeping in the caller's transaction and return its postings for the same group; +neither writes lineage. diff --git a/openmeter/ledger/advance/correction.go b/openmeter/ledger/advance/correction.go index bb9936fff0..af21feb8e2 100644 --- a/openmeter/ledger/advance/correction.go +++ b/openmeter/ledger/advance/correction.go @@ -44,12 +44,13 @@ type BackfillCorrection struct { } type CorrectionInput struct { - CustomerID customer.CustomerID - At time.Time - Amount alpacadecimal.Decimal - Collection CorrectionSource - Issue CorrectionSource - Backfills []BackfillCorrection + BreakagePosting breakage.PostingInput + CustomerID customer.CustomerID + At time.Time + Amount alpacadecimal.Decimal + Collection CorrectionSource + Issue CorrectionSource + Backfills []BackfillCorrection } func (i CorrectionInput) Validate() error { @@ -99,22 +100,22 @@ func (i CorrectionInput) Validate() error { } type CorrectionPlan struct { - Inputs []ledger.TransactionInput - BreakagePending []breakage.PendingRecord + Inputs []ledger.TransactionInput // LegacyCorrections remain unresolved so the caller can merge selections // from the same original transaction before invoking template correction. LegacyCorrections []transactions.CorrectionInput } type LegacyCorrectionInput struct { - CustomerID customer.CustomerID - ChargeID string - At time.Time - Amount alpacadecimal.Decimal - OriginalGroup ledger.TransactionGroup - Collection ledger.Transaction - Issue ledger.Transaction - BackingGroupID *string + BreakagePosting breakage.PostingInput + CustomerID customer.CustomerID + ChargeID string + At time.Time + Amount alpacadecimal.Decimal + OriginalGroup ledger.TransactionGroup + Collection ledger.Transaction + Issue ledger.Transaction + BackingGroupID *string } func (i LegacyCorrectionInput) Validate() error { diff --git a/openmeter/ledger/advance/service/correction.go b/openmeter/ledger/advance/service/correction.go index 56439f1442..4dc1bc9b1a 100644 --- a/openmeter/ledger/advance/service/correction.go +++ b/openmeter/ledger/advance/service/correction.go @@ -27,7 +27,6 @@ func (s *service) PlanCorrection(ctx context.Context, input advance.CorrectionIn } out.Inputs = append(out.Inputs, planned.Inputs...) - out.BreakagePending = append(out.BreakagePending, planned.BreakagePending...) } for _, source := range []advance.CorrectionSource{input.Collection, input.Issue} { @@ -92,7 +91,8 @@ func (s *service) correctBackfill(ctx context.Context, input advance.CorrectionI continue } - reopened, pending, err := s.breakage.ReopenRelease(ctx, breakage.ReopenReleaseInput{ + reopened, err := s.breakage.ReopenRelease(ctx, breakage.ReopenReleaseInput{ + PostingInput: input.BreakagePosting, Release: release, Amount: take, SourceKind: breakage.SourceKindUsageCorrection, @@ -105,7 +105,6 @@ func (s *service) correctBackfill(ctx context.Context, input advance.CorrectionI } out.Inputs = append(out.Inputs, reopened) - out.BreakagePending = append(out.BreakagePending, pending) remaining = remaining.Sub(take) } diff --git a/openmeter/ledger/advance/service/legacy_correction.go b/openmeter/ledger/advance/service/legacy_correction.go index 4b69e9b065..74999c8457 100644 --- a/openmeter/ledger/advance/service/legacy_correction.go +++ b/openmeter/ledger/advance/service/legacy_correction.go @@ -71,7 +71,7 @@ func (s *service) PlanLegacyCorrection(ctx context.Context, input advance.Legacy OriginalGroup: group, }) - out.Inputs, out.BreakagePending, err = s.resolveAdvanceBackfillBreakageReopenInputs(ctx, input, group, input.Amount) + out.Inputs, err = s.resolveAdvanceBackfillBreakageReopenInputs(ctx, input, group, input.Amount) if err != nil { return out, err } @@ -140,18 +140,17 @@ func (s *service) reissueBackfilledCredit(ctx context.Context, input advance.Leg return out, nil } -func (s *service) resolveAdvanceBackfillBreakageReopenInputs(ctx context.Context, input advance.LegacyCorrectionInput, backingGroup ledger.TransactionGroup, amount alpacadecimal.Decimal) ([]ledger.TransactionInput, []breakage.PendingRecord, error) { +func (s *service) resolveAdvanceBackfillBreakageReopenInputs(ctx context.Context, input advance.LegacyCorrectionInput, backingGroup ledger.TransactionGroup, amount alpacadecimal.Decimal) ([]ledger.TransactionInput, error) { releases, err := s.breakage.ListReleases(ctx, breakage.ListReleasesInput{ CustomerID: input.CustomerID, SourceTransactionGroupID: []string{backingGroup.ID().ID}, ReleaseSourceKind: []breakage.SourceKind{breakage.SourceKindAdvanceBackfill}, }) if err != nil { - return nil, nil, fmt.Errorf("list advance-backfill breakage releases: %w", err) + return nil, fmt.Errorf("list advance-backfill breakage releases: %w", err) } inputs := make([]ledger.TransactionInput, 0, len(releases)) - pending := make([]breakage.PendingRecord, 0, len(releases)) releaseFactsByTransactionID := breakageReleaseFactsByTransactionID(backingGroup) remaining := amount for _, release := range releases { @@ -168,8 +167,8 @@ func (s *service) resolveAdvanceBackfillBreakageReopenInputs(ctx context.Context if releaseFacts.SpendChargeID != nil && *releaseFacts.SpendChargeID != input.ChargeID { continue } - - reopenInput, reopenRecord, err := s.breakage.ReopenRelease(ctx, breakage.ReopenReleaseInput{ + reopenInput, err := s.breakage.ReopenRelease(ctx, breakage.ReopenReleaseInput{ + PostingInput: input.BreakagePosting, Release: release, Amount: reopenAmount, SourceKind: breakage.SourceKindUsageCorrection, @@ -177,15 +176,14 @@ func (s *service) resolveAdvanceBackfillBreakageReopenInputs(ctx context.Context SpendChargeID: releaseFacts.SpendChargeID, }) if err != nil { - return nil, nil, fmt.Errorf("resolve advance-backfill breakage reopen: %w", err) + return nil, fmt.Errorf("resolve advance-backfill breakage reopen: %w", err) } inputs = append(inputs, reopenInput) - pending = append(pending, reopenRecord) remaining = remaining.Sub(reopenAmount) } - return inputs, pending, nil + return inputs, nil } func breakageReleaseFactsByTransactionID(group ledger.TransactionGroup) map[string]ledger.EntryIdentityParts { diff --git a/openmeter/ledger/breakage/README.md b/openmeter/ledger/breakage/README.md index b867a91cfd..21e1d3a8f7 100644 --- a/openmeter/ledger/breakage/README.md +++ b/openmeter/ledger/breakage/README.md @@ -4,6 +4,12 @@ This package keeps future credit-expiration ledger entries aligned with actual c The ledger is the accounting source of truth. Breakage records are an allocation/index layer: they let later collection and correction flows find open planned breakage, reopen released breakage, and project customer-visible expired credit. +Breakage operations write complete bookkeeping using preassigned ledger IDs and +return postings for the caller's group. Both must run in one database transaction; +deferred ledger FKs reject unposted references at commit. Later operations in that +transaction see the updated bookkeeping. Source links are typed record fields; +annotations provide context and backlinks. + ## Notation - `FBO(r)` is the customer credit account route. @@ -54,13 +60,13 @@ would leave the wrong credit available. Use route dimensions for fields that define collection eligibility, ordering, or routing validation. -Use annotations or breakage records for metadata and links. +Use breakage records for accounting links and annotations for context. That means: - `credit_priority` is a route dimension because it affects normal FBO collection. - `expires_at` is represented by planned breakage `booked_at`. -- breakage kind and source links are annotations/record data. +- breakage records own source links; annotations identify breakage activity. - breakage account routes should carry only dimensions relevant to breakage accounting/revenue recognition. Breakage-generated FBO entries must not be treated as normal credit issuance or usage. They are marked as breakage activity: diff --git a/openmeter/ledger/breakage/noop.go b/openmeter/ledger/breakage/noop.go index d5bae7825b..3275db8295 100644 --- a/openmeter/ledger/breakage/noop.go +++ b/openmeter/ledger/breakage/noop.go @@ -13,16 +13,16 @@ type NoopService struct{} var _ Service = NoopService{} -func (NoopService) PlanIssuance(context.Context, PlanIssuanceInput) ([]ledger.TransactionInput, []PendingRecord, error) { - return nil, nil, nil +func (NoopService) PlanIssuance(context.Context, PlanIssuanceInput) ([]ledger.TransactionInput, error) { + return nil, nil } -func (NoopService) ReleasePlan(context.Context, ReleasePlanInput) (ledger.TransactionInput, PendingRecord, error) { - return nil, PendingRecord{}, nil +func (NoopService) ReleasePlan(context.Context, ReleasePlanInput) (ledger.TransactionInput, error) { + return nil, nil } -func (NoopService) ReopenRelease(context.Context, ReopenReleaseInput) (ledger.TransactionInput, PendingRecord, error) { - return nil, PendingRecord{}, nil +func (NoopService) ReopenRelease(context.Context, ReopenReleaseInput) (ledger.TransactionInput, error) { + return nil, nil } func (NoopService) ListPlans(context.Context, ListPlansInput) ([]Plan, error) { @@ -41,10 +41,6 @@ func (NoopService) ListExpiredBreakageImpacts(context.Context, ListExpiredBreaka return ListExpiredBreakageImpactsResult{}, nil } -func (NoopService) PersistCommittedRecords(context.Context, []PendingRecord, ledger.TransactionGroup) error { - return nil -} - // NewNoopService returns a breakage service that never plans or releases // breakage. func NewNoopService() Service { diff --git a/openmeter/ledger/breakage/service.go b/openmeter/ledger/breakage/service.go index c000eec708..3564af60cb 100644 --- a/openmeter/ledger/breakage/service.go +++ b/openmeter/ledger/breakage/service.go @@ -14,24 +14,27 @@ import ( "github.com/openmeterio/openmeter/openmeter/ledger" "github.com/openmeterio/openmeter/openmeter/ledger/transactions" "github.com/openmeterio/openmeter/pkg/currencyx" + "github.com/openmeterio/openmeter/pkg/framework/transaction" "github.com/openmeterio/openmeter/pkg/models" ) +// Service writes bookkeeping in the caller's transaction and returns identified +// postings for its group. A successful operation cannot be discarded independently. type Service interface { // PlanIssuance creates the future expiration entries for newly issued // expiring credit. ImmediateReleases handles credit that covers already // consumed advance: the issued credit has an expiry, but the covered slice is // already used, so its planned breakage is released in the same ledger group. - PlanIssuance(ctx context.Context, input PlanIssuanceInput) ([]ledger.TransactionInput, []PendingRecord, error) + PlanIssuance(ctx context.Context, input PlanIssuanceInput) ([]ledger.TransactionInput, error) // ReleasePlan creates a future-dated inverse entry that reduces a planned // breakage amount because the underlying expiring credit has been consumed or // otherwise removed before expiry. - ReleasePlan(ctx context.Context, input ReleasePlanInput) (ledger.TransactionInput, PendingRecord, error) + ReleasePlan(ctx context.Context, input ReleasePlanInput) (ledger.TransactionInput, error) // ReopenRelease creates a future-dated entry that increases breakage again // because a correction made previously consumed expiring credit unused. - ReopenRelease(ctx context.Context, input ReopenReleaseInput) (ledger.TransactionInput, PendingRecord, error) + ReopenRelease(ctx context.Context, input ReopenReleaseInput) (ledger.TransactionInput, error) // ListPlans returns unreleased planned breakage in the same order the FBO // collector must consume expiring credit. @@ -49,10 +52,6 @@ type Service interface { // ListExpiredBreakageImpacts returns customer-visible breakage impacts by // netting raw breakage rows that have reached expiry. ListExpiredBreakageImpacts(ctx context.Context, input ListExpiredBreakageImpactsInput) (ListExpiredBreakageImpactsResult, error) - - // PersistCommittedRecords turns pending record metadata into durable - // rows after the corresponding breakage ledger transactions have committed. - PersistCommittedRecords(ctx context.Context, pending []PendingRecord, group ledger.TransactionGroup) error } type Config struct { @@ -99,6 +98,8 @@ type service struct { // PlanIssuanceInput describes newly issued expiring credit and, optionally, the // slice that immediately covers already-consumed advance. type PlanIssuanceInput struct { + PostingInput + CustomerID customer.CustomerID Amount alpacadecimal.Decimal @@ -123,6 +124,10 @@ type PlanIssuanceImmediateRelease struct { func (i PlanIssuanceInput) Validate() error { var errs []error + if err := i.PostingInput.Validate(); err != nil { + errs = append(errs, err) + } + if err := i.CustomerID.Validate(); err != nil { errs = append(errs, fmt.Errorf("customer id: %w", err)) } @@ -178,18 +183,42 @@ func (i PlanIssuanceInput) Validate() error { // ReleasePlanInput describes how much of one open plan should be released and // which business flow caused the release. type ReleasePlanInput struct { - Plan Plan - Amount alpacadecimal.Decimal - SourceKind SourceKind - SourceEntryIdentityKey string - SourceChargeID *string - SpendChargeID *string - CollectionOriginID *string + PostingInput + + Plan Plan + Amount alpacadecimal.Decimal + SourceKind SourceKind + SourceTransactionID *string + SourceEntryID *string + SourceChargeID *string + SpendChargeID *string + CollectionOriginID *string } func (i ReleasePlanInput) Validate() error { var errs []error + if err := i.PostingInput.Validate(); err != nil { + errs = append(errs, err) + } + + if i.SourceEntryID != nil && i.SourceTransactionID == nil { + errs = append(errs, errors.New("source entry requires a source transaction")) + } + for field, id := range map[string]*string{ + "source transaction id": i.SourceTransactionID, + "source entry id": i.SourceEntryID, + } { + if id == nil { + continue + } + if *id == "" { + errs = append(errs, fmt.Errorf("%s cannot be empty", field)) + } else if err := ledger.ValidateAssignedID(*id); err != nil { + errs = append(errs, fmt.Errorf("%s: %w", field, err)) + } + } + if err := i.Plan.Record.ValidateForReference(); err != nil { errs = append(errs, fmt.Errorf("plan: %w", err)) } @@ -226,6 +255,8 @@ func (i ReleasePlanInput) Validate() error { // ReopenReleaseInput describes how much of one released plan should be reopened // and which correction flow caused it. type ReopenReleaseInput struct { + PostingInput + Release Release Amount alpacadecimal.Decimal SourceKind SourceKind @@ -237,6 +268,10 @@ type ReopenReleaseInput struct { func (i ReopenReleaseInput) Validate() error { var errs []error + if err := i.PostingInput.Validate(); err != nil { + errs = append(errs, err) + } + if err := i.Release.Record.ValidateForReference(); err != nil { errs = append(errs, fmt.Errorf("release: %w", err)) } @@ -307,20 +342,20 @@ func (c Record) ValidateForReference() error { // PlanIssuance returns ledger inputs instead of committing them. The caller owns // the surrounding ledger transaction group so normal credit movement and // breakage movement stay atomic. -func (s *service) PlanIssuance(ctx context.Context, input PlanIssuanceInput) ([]ledger.TransactionInput, []PendingRecord, error) { +func (s *service) PlanIssuance(ctx context.Context, input PlanIssuanceInput) ([]ledger.TransactionInput, error) { if err := input.Validate(); err != nil { - return nil, nil, err + return nil, err } fboAddress, breakageAddress, err := s.resolvePlanAddresses(ctx, input) if err != nil { - return nil, nil, err + return nil, err } priority := resolveCreditPriority(input.CreditPriority) planID := newRecordID(input.CustomerID.Namespace) - planRecord := PendingRecord{Record: Record{ + planRecord := Record{ ID: planID, Kind: ledger.BreakageKindPlan, Amount: input.Amount, @@ -332,7 +367,7 @@ func (s *service) PlanIssuance(ctx context.Context, input PlanIssuanceInput) ([] SourceChargeID: input.SourceChargeID, FBOSubAccountID: fboAddress.SubAccountID(), BreakageSubAccountID: breakageAddress.SubAccountID(), - }} + } planTx, err := s.resolveBreakageTemplate(ctx, input.CustomerID, planID.ID, nil, transactions.PlanCustomerFBOBreakageTemplate{ At: input.ExpiresAt, @@ -351,20 +386,25 @@ func (s *service) PlanIssuance(ctx context.Context, input PlanIssuanceInput) ([] }, }) if err != nil { - return nil, nil, fmt.Errorf("resolve planned breakage: %w", err) + return nil, fmt.Errorf("resolve planned breakage: %w", err) + } + + planTx, err = s.recordPosting(ctx, input.PostingInput, planRecord, planTx) + if err != nil { + return nil, err } inputs := []ledger.TransactionInput{planTx} - pending := []PendingRecord{planRecord} for _, immediateRelease := range input.ImmediateReleases { if !immediateRelease.Amount.IsPositive() { continue } - releaseTx, releaseRecord, err := s.ReleasePlan(ctx, ReleasePlanInput{ + releaseTx, err := s.ReleasePlan(ctx, ReleasePlanInput{ + PostingInput: input.PostingInput, Plan: Plan{ - Record: planRecord.Record, + Record: planRecord, OpenAmount: input.Amount, FBOAddress: fboAddress, BreakageAddress: breakageAddress, @@ -376,40 +416,38 @@ func (s *service) PlanIssuance(ctx context.Context, input PlanIssuanceInput) ([] CollectionOriginID: immediateRelease.CollectionOriginID, }) if err != nil { - return nil, nil, fmt.Errorf("resolve immediate breakage release: %w", err) + return nil, fmt.Errorf("resolve immediate breakage release: %w", err) } inputs = append(inputs, releaseTx) - pending = append(pending, releaseRecord) } - return inputs, pending, nil + return inputs, nil } -func (s *service) ReleasePlan(ctx context.Context, input ReleasePlanInput) (ledger.TransactionInput, PendingRecord, error) { +func (s *service) ReleasePlan(ctx context.Context, input ReleasePlanInput) (ledger.TransactionInput, error) { if err := input.Validate(); err != nil { - return nil, PendingRecord{}, err + return nil, err } releaseID := newRecordID(input.Plan.ID.Namespace) planID := input.Plan.ID.ID - record := PendingRecord{ - Record: Record{ - ID: releaseID, - Kind: ledger.BreakageKindRelease, - Amount: input.Amount, - CustomerID: input.Plan.CustomerID, - Currency: input.Plan.Currency, - CreditPriority: input.Plan.CreditPriority, - ExpiresAt: input.Plan.ExpiresAt, - SourceKind: input.SourceKind, - SourceChargeID: input.SourceChargeID, - FBOSubAccountID: input.Plan.FBOSubAccountID, - BreakageSubAccountID: input.Plan.BreakageSubAccountID, - PlanID: &planID, - }, - SourceEntryIdentityKey: input.SourceEntryIdentityKey, + record := Record{ + ID: releaseID, + Kind: ledger.BreakageKindRelease, + Amount: input.Amount, + CustomerID: input.Plan.CustomerID, + Currency: input.Plan.Currency, + CreditPriority: input.Plan.CreditPriority, + ExpiresAt: input.Plan.ExpiresAt, + SourceKind: input.SourceKind, + SourceChargeID: input.SourceChargeID, + FBOSubAccountID: input.Plan.FBOSubAccountID, + BreakageSubAccountID: input.Plan.BreakageSubAccountID, + PlanID: &planID, + SourceTransactionID: input.SourceTransactionID, + SourceEntryID: input.SourceEntryID, } tx, err := s.resolveBreakageTemplate(ctx, input.Plan.CustomerID, releaseID.ID, &planID, transactions.ReleaseCustomerFBOBreakageTemplate{ @@ -427,22 +465,22 @@ func (s *service) ReleasePlan(ctx context.Context, input ReleasePlanInput) (ledg BreakageIdentity: releaseBreakageIdentity(input.SourceChargeID, input.SpendChargeID, input.CollectionOriginID), }) if err != nil { - return nil, PendingRecord{}, fmt.Errorf("resolve breakage release: %w", err) + return nil, fmt.Errorf("resolve breakage release: %w", err) } - return tx, record, nil + return s.recordPosting(ctx, input.PostingInput, record, tx) } -func (s *service) ReopenRelease(ctx context.Context, input ReopenReleaseInput) (ledger.TransactionInput, PendingRecord, error) { +func (s *service) ReopenRelease(ctx context.Context, input ReopenReleaseInput) (ledger.TransactionInput, error) { if err := input.Validate(); err != nil { - return nil, PendingRecord{}, err + return nil, err } reopenID := newRecordID(input.Release.ID.Namespace) planID := *input.Release.PlanID releaseID := input.Release.ID.ID - record := PendingRecord{Record: Record{ + record := Record{ ID: reopenID, Kind: ledger.BreakageKindReopen, Amount: input.Amount, @@ -456,7 +494,7 @@ func (s *service) ReopenRelease(ctx context.Context, input ReopenReleaseInput) ( BreakageSubAccountID: input.Release.BreakageSubAccountID, PlanID: &planID, ReleaseID: &releaseID, - }} + } tx, err := s.resolveBreakageTemplate(ctx, input.Release.CustomerID, reopenID.ID, &planID, transactions.ReopenCustomerFBOBreakageTemplate{ At: input.Release.ExpiresAt, @@ -473,10 +511,10 @@ func (s *service) ReopenRelease(ctx context.Context, input ReopenReleaseInput) ( BreakageIdentity: releaseBreakageIdentity(input.SourceChargeID, input.SpendChargeID, input.CollectionOriginID), }) if err != nil { - return nil, PendingRecord{}, fmt.Errorf("resolve breakage reopen: %w", err) + return nil, fmt.Errorf("resolve breakage reopen: %w", err) } - return tx, record, nil + return s.recordPosting(ctx, input.PostingInput, record, tx) } func (s *service) ListPlans(ctx context.Context, input ListPlansInput) ([]Plan, error) { @@ -610,84 +648,29 @@ func (s *service) ListExpiredRecords(ctx context.Context, input ListExpiredRecor return s.adapter.ListExpiredRecords(ctx, input) } -func (s *service) PersistCommittedRecords(ctx context.Context, pending []PendingRecord, group ledger.TransactionGroup) error { - if len(pending) == 0 { - return nil +// recordPosting writes complete bookkeeping before the journal group is posted. +// Deferred ledger FKs reject discarded postings at the enclosing transaction's commit. +func (s *service) recordPosting(ctx context.Context, input PostingInput, record Record, posting ledger.TransactionInput) (ledger.TransactionInput, error) { + if _, err := transaction.GetDriverFromContext(ctx); err != nil { + return nil, fmt.Errorf("breakage requires the caller's transaction: %w", err) } - if group == nil { - return errors.New("transaction group is required") - } - - pendingByID := make(map[string]PendingRecord, len(pending)) - for _, item := range pending { - pendingByID[item.ID.ID] = item - } - - sourceEntriesByIdentity := committedSourceEntriesByIdentity(group) - - records := make([]Record, 0, len(pending)) - groupID := group.ID().ID - for _, tx := range group.Transactions() { - recordID, ok := breakageRecordID(tx.Annotations()) - if !ok { - continue - } - - pendingRecord, ok := pendingByID[recordID] - if !ok { - return fmt.Errorf("committed breakage transaction %s has unknown record id %s", tx.ID().ID, recordID) - } - - record := pendingRecord.Record - record.BreakageTransactionGroupID = groupID - record.BreakageTransactionID = tx.ID().ID - record.Annotations = tx.Annotations() - if record.SourceTransactionGroupID == nil { - record.SourceTransactionGroupID = &groupID - } - - if pendingRecord.SourceEntryIdentityKey != "" { - sourceEntry, ok := sourceEntriesByIdentity[pendingRecord.SourceEntryIdentityKey] - if !ok { - return fmt.Errorf("source entry with identity key %s not found for breakage record %s", pendingRecord.SourceEntryIdentityKey, recordID) - } - - sourceTransactionID := sourceEntry.TransactionID().ID - sourceEntryID := sourceEntry.ID().ID - record.SourceTransactionID = &sourceTransactionID - record.SourceEntryID = &sourceEntryID - } - - records = append(records, record) - delete(pendingByID, recordID) - } - - if len(pendingByID) > 0 { - return fmt.Errorf("missing committed breakage transactions for %d pending records", len(pendingByID)) + posting, err := ledger.PreassignIDs(posting) + if err != nil { + return nil, fmt.Errorf("assign breakage posting IDs: %w", err) } + posting = transactions.WithAnnotations(posting, input.Annotations) - return s.adapter.CreateRecords(ctx, CreateRecordsInput{Records: records}) -} - -func committedSourceEntriesByIdentity(group ledger.TransactionGroup) map[string]ledger.Entry { - out := make(map[string]ledger.Entry) - - for _, tx := range group.Transactions() { - if _, ok := breakageRecordID(tx.Annotations()); ok { - continue - } - - for _, entry := range tx.Entries() { - if entry.IdentityKey() == "" { - continue - } + record.BreakageTransactionGroupID = input.TransactionGroupID + record.BreakageTransactionID = posting.AssignedID() + record.SourceTransactionGroupID = &input.TransactionGroupID + record.Annotations = posting.Annotations() - out[entry.IdentityKey()] = entry - } + if err := s.adapter.CreateRecords(ctx, CreateRecordsInput{Records: []Record{record}}); err != nil { + return nil, fmt.Errorf("persist breakage record: %w", err) } - return out + return posting, nil } func (s *service) resolvePlanAddresses(ctx context.Context, input PlanIssuanceInput) (ledger.PostingAddress, ledger.PostingAddress, error) { @@ -815,20 +798,6 @@ func breakageKindForTemplate(template transactions.TransactionTemplate) (ledger. } } -func breakageRecordID(annotations models.Annotations) (string, bool) { - raw, ok := annotations[ledger.AnnotationBreakageRecordID] - if !ok { - return "", false - } - - value, ok := raw.(string) - if !ok || value == "" { - return "", false - } - - return value, true -} - func resolveCreditPriority(priority *int) int { if priority == nil { return ledger.DefaultCustomerFBOPriority diff --git a/openmeter/ledger/breakage/service_test.go b/openmeter/ledger/breakage/service_test.go new file mode 100644 index 0000000000..7449ffb9b1 --- /dev/null +++ b/openmeter/ledger/breakage/service_test.go @@ -0,0 +1,288 @@ +package breakage_test + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/alpacahq/alpacadecimal" + "github.com/oklog/ulid/v2" + "github.com/stretchr/testify/require" + + enttx "github.com/openmeterio/openmeter/openmeter/ent/tx" + "github.com/openmeterio/openmeter/openmeter/ledger" + "github.com/openmeterio/openmeter/openmeter/ledger/breakage" + breakageadapter "github.com/openmeterio/openmeter/openmeter/ledger/breakage/adapter" + ledgertestutils "github.com/openmeterio/openmeter/openmeter/ledger/testutils" + "github.com/openmeterio/openmeter/openmeter/ledger/transactions" + "github.com/openmeterio/openmeter/pkg/framework/transaction" + "github.com/openmeterio/openmeter/pkg/models" +) + +func TestBreakageBookkeepingPrecedesPosting(t *testing.T) { + env, service := newBreakageEnv(t) + groupID := ulid.Make().String() + + // given: an expiring issuance whose ledger group has not been posted. + err := transaction.RunWithNoValue(t.Context(), enttx.NewCreator(env.DB), func(ctx context.Context) error { + inputs, err := service.PlanIssuance(ctx, expiringIssuance(env, groupID)) + require.NoError(t, err) + require.Len(t, inputs, 1) + + // when: another operation reads bookkeeping in the same transaction. + plans, err := service.ListPlans(ctx, breakage.ListPlansInput{ + CustomerID: env.CustomerID, + Currency: env.Currency, + AsOf: env.Now(), + }) + require.NoError(t, err) + require.Len(t, plans, 1) + + // then: the complete reference already exists and names the returned posting. + require.Equal(t, groupID, plans[0].BreakageTransactionGroupID) + require.Equal(t, groupID, *plans[0].SourceTransactionGroupID) + require.Equal(t, inputs[0].AssignedID(), plans[0].BreakageTransactionID) + require.Equal(t, 10.0, plans[0].OpenAmount.InexactFloat64()) + count, err := env.DB.LedgerTransaction.Query().Count(t.Context()) + require.NoError(t, err) + require.Zero(t, count) + + issue, err := resolveCreditIssuance(ctx, env) + if err != nil { + return err + } + inputs = append(issue, inputs...) + _, err = env.Deps.HistoricalLedger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs(env.Namespace, nil, inputs...), groupID)) + return err + }) + require.NoError(t, err) + + group, err := env.Deps.HistoricalLedger.GetTransactionGroup(t.Context(), models.NamespacedID{Namespace: env.Namespace, ID: groupID}) + require.NoError(t, err) + require.Equal(t, groupID, group.ID().ID) + require.Len(t, group.Transactions(), 2) +} + +func TestBreakageReferencesDistinctPostingsWithIdenticalAccountingIdentity(t *testing.T) { + env, service := newBreakageEnv(t) + issueGroupID := ulid.Make().String() + + // given: one expiry and two consumption postings with identical entry identities. + err := transaction.RunWithNoValue(t.Context(), enttx.NewCreator(env.DB), func(ctx context.Context) error { + inputs, err := service.PlanIssuance(ctx, expiringIssuance(env, issueGroupID)) + if err != nil { + return err + } + issue, err := resolveCreditIssuance(ctx, env) + if err != nil { + return err + } + inputs = append(issue, inputs...) + _, err = env.Deps.HistoricalLedger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs(env.Namespace, nil, inputs...), issueGroupID)) + return err + }) + require.NoError(t, err) + + groupID := ulid.Make().String() + var sourceEntries []ledger.EntryInput + var sourceTransactions []string + err = transaction.RunWithNoValue(t.Context(), enttx.NewCreator(env.DB), func(ctx context.Context) error { + plans, err := service.ListPlans(ctx, breakage.ListPlansInput{CustomerID: env.CustomerID, Currency: env.Currency, AsOf: env.Now()}) + require.NoError(t, err) + require.Len(t, plans, 1) + + template := transactions.TransferCustomerFBOToAccruedTemplate{ + At: env.Now(), + Currency: env.CurrencyReference(), + Sources: []transactions.PostingAmount{{ + Address: plans[0].FBOAddress, + Amount: alpacadecimal.NewFromInt(2), + }}, + } + inputs, err := transactions.ResolveTransactions(ctx, transactions.ResolverDependencies{ + AccountService: env.Deps.ResolversService, + AccountCatalog: env.Deps.AccountService, + BalanceQuerier: env.Deps.HistoricalLedger, + }, transactions.ResolutionScope{Namespace: env.Namespace, CustomerID: env.CustomerID}, template, template) + require.NoError(t, err) + + for idx := range inputs { + inputs[idx], err = ledger.PreassignIDs(inputs[idx]) + require.NoError(t, err) + for _, entry := range inputs[idx].EntryInputs() { + if entry.PostingAddress().AccountType() == ledger.AccountTypeCustomerFBO { + sourceEntries = append(sourceEntries, entry) + sourceTransactions = append(sourceTransactions, inputs[idx].AssignedID()) + } + } + } + require.Len(t, sourceEntries, 2) + require.Equal(t, sourceEntries[0].IdentityKey(), sourceEntries[1].IdentityKey()) + + // when: each debit releases its expiry before any journal posting is committed. + for idx, entry := range sourceEntries { + entryID := entry.AssignedID() + release, err := service.ReleasePlan(ctx, breakage.ReleasePlanInput{ + PostingInput: breakage.PostingInput{TransactionGroupID: groupID}, + Plan: plans[0], + Amount: alpacadecimal.NewFromInt(2), + SourceKind: breakage.SourceKindUsage, + SourceTransactionID: &sourceTransactions[idx], + SourceEntryID: &entryID, + }) + require.NoError(t, err) + inputs = append(inputs, release) + } + + // then: subsequent expiry planning sees both staged releases. + plans, err = service.ListPlans(ctx, breakage.ListPlansInput{CustomerID: env.CustomerID, Currency: env.Currency, AsOf: env.Now()}) + require.NoError(t, err) + require.Len(t, plans, 1) + require.Equal(t, 6.0, plans[0].OpenAmount.InexactFloat64()) + + _, err = env.Deps.HistoricalLedger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs(env.Namespace, nil, inputs...), groupID)) + return err + }) + require.NoError(t, err) + + releases, err := service.ListReleases(t.Context(), breakage.ListReleasesInput{ + CustomerID: env.CustomerID, + SourceTransactionGroupID: []string{groupID}, + }) + require.NoError(t, err) + require.Len(t, releases, 2) + for idx, entry := range sourceEntries { + var matched *breakage.Release + for releaseIdx := range releases { + if *releases[releaseIdx].SourceEntryID == entry.AssignedID() { + matched = &releases[releaseIdx] + break + } + } + require.NotNil(t, matched) + require.Equal(t, sourceTransactions[idx], *matched.SourceTransactionID) + } +} + +func TestBreakageDiscardedPostingRejectsOuterCommit(t *testing.T) { + env, service := newBreakageEnv(t) + + // given: bookkeeping names a group that will never be posted. + err := transaction.RunWithNoValue(t.Context(), enttx.NewCreator(env.DB), func(ctx context.Context) error { + _, err := service.PlanIssuance(ctx, expiringIssuance(env, ulid.Make().String())) + require.NoError(t, err) + + // when: the caller attempts to commit only bookkeeping. + return nil + }) + + // then: deferred integrity checks reject the commit and discard the record. + require.Error(t, err) + requireNoBreakageRows(t, env) +} + +func TestBreakageBookkeepingRollsBackWithCaller(t *testing.T) { + env, service := newBreakageEnv(t) + failure := errors.New("caller failed") + + // given: a plan has been staged in the caller's transaction. + err := transaction.RunWithNoValue(t.Context(), enttx.NewCreator(env.DB), func(ctx context.Context) error { + _, err := service.PlanIssuance(ctx, expiringIssuance(env, ulid.Make().String())) + require.NoError(t, err) + + // when: the surrounding business operation fails before posting. + return failure + }) + + // then: no bookkeeping survives independently of the operation. + require.ErrorIs(t, err, failure) + requireNoBreakageRows(t, env) +} + +func TestBreakagePostingFailureRollsBackBookkeeping(t *testing.T) { + env, service := newBreakageEnv(t) + groupID := ulid.Make().String() + issue, err := resolveCreditIssuance(t.Context(), env) + require.NoError(t, err) + _, err = env.Deps.HistoricalLedger.CommitGroup(t.Context(), ledger.WithGroupID(transactions.GroupInputs(env.Namespace, nil, issue...), groupID)) + require.NoError(t, err) + + // given: bookkeeping is written before posting a group with a colliding ID. + err = transaction.RunWithNoValue(t.Context(), enttx.NewCreator(env.DB), func(ctx context.Context) error { + inputs, err := service.PlanIssuance(ctx, expiringIssuance(env, groupID)) + require.NoError(t, err) + + // when: the ledger rejects the posting. + _, err = env.Deps.HistoricalLedger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs(env.Namespace, nil, inputs...), groupID)) + return err + }) + + // then: bookkeeping is rolled back and the original credit remains intact. + require.Error(t, err) + count, err := env.DB.LedgerBreakageRecord.Query().Count(t.Context()) + require.NoError(t, err) + require.Zero(t, count) + require.Equal(t, 10.0, env.SumBalance(t, env.FBOSubAccount(t, ledger.DefaultCustomerFBOPriority)).InexactFloat64()) +} + +func TestBreakageRequiresCallerTransaction(t *testing.T) { + env, service := newBreakageEnv(t) + + inputs, err := service.PlanIssuance(t.Context(), expiringIssuance(env, ulid.Make().String())) + require.ErrorContains(t, err, "requires the caller's transaction") + require.Nil(t, inputs) + requireNoBreakageRows(t, env) +} + +func newBreakageEnv(t *testing.T) (*ledgertestutils.IntegrationEnv, breakage.Service) { + t.Helper() + + env := ledgertestutils.NewIntegrationEnv(t, "breakage-posting") + adapter, err := breakageadapter.New(breakageadapter.Config{Client: env.DB}) + require.NoError(t, err) + service, err := breakage.NewService(breakage.Config{ + Adapter: adapter, + Dependencies: transactions.ResolverDependencies{ + AccountService: env.Deps.ResolversService, + AccountCatalog: env.Deps.AccountService, + BalanceQuerier: env.Deps.HistoricalLedger, + }, + }) + require.NoError(t, err) + + return env, service +} + +func expiringIssuance(env *ledgertestutils.IntegrationEnv, groupID string) breakage.PlanIssuanceInput { + return breakage.PlanIssuanceInput{ + PostingInput: breakage.PostingInput{TransactionGroupID: groupID}, + CustomerID: env.CustomerID, + Amount: alpacadecimal.NewFromInt(10), + Currency: env.CurrencyReference(), + ExpiresAt: env.Now().Add(24 * time.Hour), + } +} + +func resolveCreditIssuance(ctx context.Context, env *ledgertestutils.IntegrationEnv) ([]ledger.TransactionInput, error) { + return transactions.ResolveTransactions(ctx, transactions.ResolverDependencies{ + AccountService: env.Deps.ResolversService, + AccountCatalog: env.Deps.AccountService, + BalanceQuerier: env.Deps.HistoricalLedger, + }, transactions.ResolutionScope{Namespace: env.Namespace, CustomerID: env.CustomerID}, transactions.IssueCustomerReceivableTemplate{ + At: env.Now(), + Amount: alpacadecimal.NewFromInt(10), + Currency: env.CurrencyReference(), + }) +} + +func requireNoBreakageRows(t *testing.T, env *ledgertestutils.IntegrationEnv) { + t.Helper() + + count, err := env.DB.LedgerBreakageRecord.Query().Count(t.Context()) + require.NoError(t, err) + require.Zero(t, count) + count, err = env.DB.LedgerTransactionGroup.Query().Count(t.Context()) + require.NoError(t, err) + require.Zero(t, count) +} diff --git a/openmeter/ledger/breakage/types.go b/openmeter/ledger/breakage/types.go index eeecb11cbc..5133eedcbd 100644 --- a/openmeter/ledger/breakage/types.go +++ b/openmeter/ledger/breakage/types.go @@ -15,6 +15,27 @@ import ( "github.com/openmeterio/openmeter/pkg/models" ) +// PostingInput identifies the caller's group before its entries are posted. +// Bookkeeping and the returned postings must complete in the same transaction. +type PostingInput struct { + TransactionGroupID string + Annotations models.Annotations +} + +var _ models.Validator = PostingInput{} + +func (i PostingInput) Validate() error { + var errs []error + + if i.TransactionGroupID == "" { + errs = append(errs, errors.New("transaction group id is required")) + } else if err := ledger.ValidateAssignedID(i.TransactionGroupID); err != nil { + errs = append(errs, fmt.Errorf("transaction group id: %w", err)) + } + + return models.NewNillableGenericValidationError(errors.Join(errs...)) +} + type SourceKind = ledger.BreakageSourceKind const ( @@ -89,16 +110,6 @@ type Release struct { BreakageAddress ledger.PostingAddress } -// PendingRecord is a record row that has been planned before the ledger commit. -// PersistCommittedRecords fills in committed ledger ids after CommitGroup -// succeeds. SourceEntryIdentityKey is transient; it lets usage releases attach -// to the committed FBO source entry without knowing the entry id before commit. -type PendingRecord struct { - Record - - SourceEntryIdentityKey string -} - // ListPlansInput selects expiring credit that can still produce breakage as of // the collector's AsOf time. Plans expiring at or before AsOf are not // candidates because those credits are already expired from the collector's @@ -172,8 +183,7 @@ func (i BreakageImpact) Cursor() ledger.TransactionCursor { } } -// CreateRecordsInput persists record rows for already committed -// breakage ledger transactions. +// CreateRecordsInput persists bookkeeping for identified breakage postings. type CreateRecordsInput struct { Records []Record } diff --git a/openmeter/ledger/chargeadapter/creditpurchase.go b/openmeter/ledger/chargeadapter/creditpurchase.go index e0d37e127c..40866da343 100644 --- a/openmeter/ledger/chargeadapter/creditpurchase.go +++ b/openmeter/ledger/chargeadapter/creditpurchase.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/alpacahq/alpacadecimal" + "github.com/oklog/ulid/v2" "github.com/samber/lo" chargecreditpurchase "github.com/openmeterio/openmeter/openmeter/billing/charges/creditpurchase" @@ -326,35 +327,31 @@ func (h *creditPurchaseHandler) issueCreditPurchase(ctx context.Context, input c return chargecreditpurchase.CreditGrantResult{}, fmt.Errorf("resolve transactions: %w", err) } - var pendingBreakage []breakage.PendingRecord + groupID := ulid.Make().String() if breakageInput := issuance.mapBreakageInput(); breakageInput != nil { - breakageInputs, pending, err := h.breakage.PlanIssuance(ctx, *breakageInput) + breakageInput.PostingInput = breakage.PostingInput{TransactionGroupID: groupID, Annotations: annotations} + breakageInputs, err := h.breakage.PlanIssuance(ctx, *breakageInput) if err != nil { return chargecreditpurchase.CreditGrantResult{}, fmt.Errorf("resolve breakage plan: %w", err) } inputs = append(inputs, breakageInputs...) - pendingBreakage = pending } if len(inputs) == 0 { return chargecreditpurchase.CreditGrantResult{}, nil } - transactionGroup, err := h.commitTransactions(ctx, transactions.GroupInputs( + transactionGroup, err := h.commitTransactions(ctx, ledger.WithGroupID(transactions.GroupInputs( charge.Namespace, annotations, inputs..., - )) + ), groupID)) if err != nil { return chargecreditpurchase.CreditGrantResult{}, err } - if err := h.breakage.PersistCommittedRecords(ctx, pendingBreakage, transactionGroup); err != nil { - return chargecreditpurchase.CreditGrantResult{}, fmt.Errorf("persist breakage records: %w", err) - } - return chargecreditpurchase.CreditGrantResult{ GroupReference: ledgertransaction.GroupReference{TransactionGroupID: transactionGroup.ID().ID}, BackfillAllocations: backfillPlan.LegacyAllocations, @@ -372,7 +369,7 @@ func (h *creditPurchaseHandler) commitTransactions(ctx context.Context, group le } } - committed, err := h.ledger.CommitGroup(ctx, transactions.GroupInputs(group.Namespace(), group.Annotations(), inputs...)) + committed, err := h.ledger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs(group.Namespace(), group.Annotations(), inputs...), group.AssignedID())) if err != nil { return nil, fmt.Errorf("commit ledger transaction group: %w", err) } diff --git a/openmeter/ledger/collector/collect.go b/openmeter/ledger/collector/collect.go index a08f18b073..a8968f6c2d 100644 --- a/openmeter/ledger/collector/collect.go +++ b/openmeter/ledger/collector/collect.go @@ -2,9 +2,9 @@ package collector import ( "context" + "errors" "fmt" "maps" - "strconv" "github.com/alpacahq/alpacadecimal" "github.com/oklog/ulid/v2" @@ -34,24 +34,30 @@ type accrualCollector struct { type collectedInputs []ledger.TransactionInput -type resolvedCollectedInputs struct { - inputs []ledger.TransactionInput - breakagePending []breakage.PendingRecord -} - func (c *accrualCollector) collectToAccrued(ctx context.Context, input CollectToAccruedInput) (creditrealization.CreateAllocationInputs, error) { run := func(ctx context.Context) (creditrealization.CreateAllocationInputs, error) { if input.Amount.IsZero() { return nil, nil } - resolved, err := c.resolveCollectedInputs(ctx, input, input.Amount) + groupAnnotations := input.Annotations + if groupAnnotations == nil { + groupAnnotations = ledger.ChargeAnnotations(models.NamespacedID{ + Namespace: input.Namespace, + ID: input.ChargeID, + }) + } + + posting := breakage.PostingInput{ + TransactionGroupID: ulid.Make().String(), + Annotations: groupAnnotations, + } + + inputs, err := c.resolveCollectedInputs(ctx, posting, input) if err != nil { return nil, err } - inputs := resolved.inputs - // Credit-only: if the wallet didn't cover the full accrual, issue advance and // move that slice through the advance-to-accrued path. if shortfall := input.Amount.Sub(collectedInputs(inputs).collectedFBOAmount()); c.shouldAdvanceShortfall(input, shortfall) { @@ -76,36 +82,21 @@ func (c *accrualCollector) collectToAccrued(ctx context.Context, input CollectTo return nil, nil } - groupAnnotations := input.Annotations - if groupAnnotations == nil { - groupAnnotations = ledger.ChargeAnnotations(models.NamespacedID{ - Namespace: input.Namespace, - ID: input.ChargeID, - }) - } - for i, txInput := range inputs { if txInput != nil { inputs[i] = transactions.WithAnnotations(txInput, groupAnnotations) } } - transactionGroup, err := c.ledger.CommitGroup(ctx, transactions.GroupInputs( + transactionGroup, err := c.ledger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs( input.Namespace, groupAnnotations, inputs..., - )) + ), posting.TransactionGroupID)) if err != nil { return nil, fmt.Errorf("commit ledger transaction group: %w", err) } - // Breakage rows describe committed breakage ledger transactions, so - // they must be persisted in the same transaction context as the - // ledger group. - if err := c.breakage.PersistCommittedRecords(ctx, resolved.breakagePending, transactionGroup); err != nil { - return nil, fmt.Errorf("persist breakage records: %w", err) - } - return collectedInputs(inputs).toCreditRealizations(input.ServicePeriod, transactionGroup.ID().ID), nil } @@ -118,15 +109,6 @@ func (c *accrualCollector) collectToReceivable(ctx context.Context, input Collec return nil, nil } - resolved, err := c.resolveCoveredReceivableInputs(ctx, input) - if err != nil { - return nil, err - } - - if len(resolved.inputs) == 0 { - return nil, nil - } - groupAnnotations := input.Annotations if groupAnnotations == nil { groupAnnotations = ledger.ChargeAnnotations(models.NamespacedID{ @@ -135,34 +117,43 @@ func (c *accrualCollector) collectToReceivable(ctx context.Context, input Collec }) } - for i, txInput := range resolved.inputs { + posting := breakage.PostingInput{ + TransactionGroupID: ulid.Make().String(), + Annotations: groupAnnotations, + } + + inputs, err := c.resolveCoveredReceivableInputs(ctx, posting, input) + if err != nil { + return nil, err + } + if len(inputs) == 0 { + return nil, nil + } + + for i, txInput := range inputs { if txInput != nil { - resolved.inputs[i] = transactions.WithAnnotations(txInput, groupAnnotations) + inputs[i] = transactions.WithAnnotations(txInput, groupAnnotations) } } - transactionGroup, err := c.ledger.CommitGroup(ctx, transactions.GroupInputs( + transactionGroup, err := c.ledger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs( input.Namespace, groupAnnotations, - resolved.inputs..., - )) + inputs..., + ), posting.TransactionGroupID)) if err != nil { return nil, fmt.Errorf("commit ledger transaction group: %w", err) } - if err := c.breakage.PersistCommittedRecords(ctx, resolved.breakagePending, transactionGroup); err != nil { - return nil, fmt.Errorf("persist breakage records: %w", err) - } - - return collectedInputs(resolved.inputs).toCreditRealizations(input.ServicePeriod, transactionGroup.ID().ID), nil + return collectedInputs(inputs).toCreditRealizations(input.ServicePeriod, transactionGroup.ID().ID), nil } return transaction.Run(ctx, c.transactionManager, run) } -func (c *accrualCollector) resolveCoveredReceivableInputs(ctx context.Context, input CollectToReceivableInput) (resolvedCollectedInputs, error) { +func (c *accrualCollector) resolveCoveredReceivableInputs(ctx context.Context, posting breakage.PostingInput, input CollectToReceivableInput) ([]ledger.TransactionInput, error) { if err := ledger.ValidateTransactionAmount(input.Amount); err != nil { - return resolvedCollectedInputs{}, fmt.Errorf("amount: %w", err) + return nil, fmt.Errorf("amount: %w", err) } selections, err := c.collectCustomerFBOSelections( @@ -174,11 +165,11 @@ func (c *accrualCollector) resolveCoveredReceivableInputs(ctx context.Context, i input.SourceBalanceAsOf, ) if err != nil { - return resolvedCollectedInputs{}, fmt.Errorf("collect customer FBO: %w", err) + return nil, fmt.Errorf("collect customer FBO: %w", err) } if len(selections) == 0 { - return resolvedCollectedInputs{}, nil + return nil, nil } for i := range selections { @@ -200,40 +191,42 @@ func (c *accrualCollector) resolveCoveredReceivableInputs(ctx context.Context, i }, ) if err != nil { - return resolvedCollectedInputs{}, fmt.Errorf("resolve transactions: %w", err) + return nil, fmt.Errorf("resolve transactions: %w", err) } - breakageInputs, pending, err := c.resolveCollectionBreakageInputs(ctx, input.ChargeID, selections) + inputs, err = c.resolveCollectionBreakageInputs(ctx, collectionBreakageInput{ + Posting: posting, + ChargeID: input.ChargeID, + Selections: selections, + Postings: inputs, + }) if err != nil { - return resolvedCollectedInputs{}, err + return nil, err } - return resolvedCollectedInputs{ - inputs: append(inputs, breakageInputs...), - breakagePending: pending, - }, nil + return inputs, nil } -func (c *accrualCollector) resolveCollectedInputs(ctx context.Context, input CollectToAccruedInput, amount alpacadecimal.Decimal) (resolvedCollectedInputs, error) { - if err := ledger.ValidateTransactionAmount(amount); err != nil { - return resolvedCollectedInputs{}, fmt.Errorf("amount: %w", err) +func (c *accrualCollector) resolveCollectedInputs(ctx context.Context, posting breakage.PostingInput, input CollectToAccruedInput) ([]ledger.TransactionInput, error) { + if err := ledger.ValidateTransactionAmount(input.Amount); err != nil { + return nil, fmt.Errorf("amount: %w", err) } if err := input.Currency.Validate(); err != nil { - return resolvedCollectedInputs{}, fmt.Errorf("currency: %w", err) + return nil, fmt.Errorf("currency: %w", err) } if input.Currency.IsCustom() && !input.Currency.IsResolved() { - return resolvedCollectedInputs{}, fmt.Errorf("currency: custom currency must be resolved") + return nil, fmt.Errorf("currency: custom currency must be resolved") } - selections, err := c.collectCustomerFBOSelections(ctx, c.customerID(input), input.Currency, ledger.Route{Filters: input.Filters}, amount, input.SourceBalanceAsOf) + selections, err := c.collectCustomerFBOSelections(ctx, c.customerID(input), input.Currency, ledger.Route{Filters: input.Filters}, input.Amount, input.SourceBalanceAsOf) if err != nil { - return resolvedCollectedInputs{}, fmt.Errorf("collect customer FBO: %w", err) + return nil, fmt.Errorf("collect customer FBO: %w", err) } if len(selections) == 0 { - return resolvedCollectedInputs{}, nil + return nil, nil } for i := range selections { @@ -254,26 +247,82 @@ func (c *accrualCollector) resolveCollectedInputs(ctx context.Context, input Col }, ) if err != nil { - return resolvedCollectedInputs{}, fmt.Errorf("resolve transactions: %w", err) + return nil, fmt.Errorf("resolve transactions: %w", err) } - breakageInputs, pending, err := c.resolveCollectionBreakageInputs(ctx, input.ChargeID, selections) + inputs, err = c.resolveCollectionBreakageInputs(ctx, collectionBreakageInput{ + Posting: posting, + ChargeID: input.ChargeID, + Selections: selections, + Postings: inputs, + }) if err != nil { - return resolvedCollectedInputs{}, err + return nil, err } - return resolvedCollectedInputs{ - inputs: append(inputs, breakageInputs...), - breakagePending: pending, - }, nil + return inputs, nil } -func (c *accrualCollector) resolveCollectionBreakageInputs(ctx context.Context, chargeID string, selections []fboCollectionSelection) ([]ledger.TransactionInput, []breakage.PendingRecord, error) { - var inputs []ledger.TransactionInput - var pending []breakage.PendingRecord +type sourcePostingIDs struct { + transactionID string + entryID string +} + +type collectionBreakageInput struct { + Posting breakage.PostingInput + ChargeID string + Selections []fboCollectionSelection + Postings []ledger.TransactionInput +} + +var _ models.Validator = collectionBreakageInput{} + +func (i collectionBreakageInput) Validate() error { + var errs []error + + if err := i.Posting.Validate(); err != nil { + errs = append(errs, err) + } + + if i.ChargeID == "" { + errs = append(errs, errors.New("charge id is required")) + } + + return models.NewNillableGenericValidationError(errors.Join(errs...)) +} + +func (c *accrualCollector) resolveCollectionBreakageInputs(ctx context.Context, input collectionBreakageInput) ([]ledger.TransactionInput, error) { + if err := input.Validate(); err != nil { + return nil, err + } + + inputs := input.Postings + + // Capture concrete source IDs while the resolved postings are still in hand. + // A collection origin identifies exactly one selected FBO debit in this group. + sourcesByOrigin := make(map[string]sourcePostingIDs, len(input.Selections)) + for idx, input := range inputs { + identified, err := ledger.PreassignIDs(input) + if err != nil { + return nil, err + } + inputs[idx] = identified + + for _, entry := range identified.EntryInputs() { + if entry.PostingAddress().AccountType() != ledger.AccountTypeCustomerFBO || !entry.Amount().IsNegative() { + continue + } + originID := lo.FromPtr(entry.Provenance().CollectionOriginID) + if _, exists := sourcesByOrigin[originID]; exists { + return nil, fmt.Errorf("collection origin %s has multiple FBO source entries", originID) + } + sourcesByOrigin[originID] = sourcePostingIDs{transactionID: identified.AssignedID(), entryID: entry.AssignedID()} + } + } + releaseRemainingByPlanID := make(map[string]alpacadecimal.Decimal) - for idx, selection := range selections { + for _, selection := range input.Selections { if selection.source.breakagePlan == nil { continue } @@ -287,41 +336,35 @@ func (c *accrualCollector) resolveCollectionBreakageInputs(ctx context.Context, // Legacy source-less plans can reserve multiple selected source slices, // so guard the aggregate release amount before writing release records. if selection.amount.GreaterThan(remaining) { - return nil, nil, fmt.Errorf("breakage release amount %s exceeds remaining plan amount %s for plan %s", selection.amount, remaining, plan.ID.ID) + return nil, fmt.Errorf("breakage release amount %s exceeds remaining plan amount %s for plan %s", selection.amount, remaining, plan.ID.ID) } releaseRemainingByPlanID[plan.ID.ID] = remaining.Sub(selection.amount) - releaseInput, releaseRecord, err := c.breakage.ReleasePlan(ctx, breakage.ReleasePlanInput{ - Plan: plan, - Amount: selection.amount, - SourceKind: breakage.SourceKindUsage, - SourceChargeID: selection.source.sourceChargeID, - SpendChargeID: &chargeID, - CollectionOriginID: selection.collectionOriginID, - SourceEntryIdentityKey: func() string { - collectionSource := strconv.Itoa(idx) - identityKey, _ := ledger.EntryIdentityParts{ - CollectionSource: &collectionSource, - Provenance: ledger.Provenance{ - CollectionOriginID: selection.collectionOriginID, - SourceChargeID: selection.source.sourceChargeID, - SpendChargeID: &chargeID, - }, - }.Text() - - return string(identityKey) - }(), + source, ok := sourcesByOrigin[lo.FromPtr(selection.collectionOriginID)] + if !ok { + return nil, fmt.Errorf("selected credit has no resolved FBO source posting") + } + + releaseInput, err := c.breakage.ReleasePlan(ctx, breakage.ReleasePlanInput{ + PostingInput: input.Posting, + Plan: plan, + Amount: selection.amount, + SourceKind: breakage.SourceKindUsage, + SourceChargeID: selection.source.sourceChargeID, + SpendChargeID: &input.ChargeID, + CollectionOriginID: selection.collectionOriginID, + SourceTransactionID: &source.transactionID, + SourceEntryID: &source.entryID, }) if err != nil { - return nil, nil, fmt.Errorf("resolve breakage release: %w", err) + return nil, fmt.Errorf("resolve breakage release: %w", err) } inputs = append(inputs, releaseInput) - pending = append(pending, releaseRecord) } - return inputs, pending, nil + return inputs, nil } func (c *accrualCollector) shouldAdvanceShortfall(input CollectToAccruedInput, shortfall alpacadecimal.Decimal) bool { diff --git a/openmeter/ledger/collector/collection_fbo_test.go b/openmeter/ledger/collector/collection_fbo_test.go index 6d979352c4..3fb5acca7a 100644 --- a/openmeter/ledger/collector/collection_fbo_test.go +++ b/openmeter/ledger/collector/collection_fbo_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/alpacahq/alpacadecimal" + "github.com/oklog/ulid/v2" "github.com/samber/lo" "github.com/stretchr/testify/require" @@ -915,24 +916,41 @@ func bookExpiringCreditWithFeatures( ) require.NoError(t, err) - breakageInputs, pending, err := breakageService.PlanIssuance(t.Context(), ledgerbreakage.PlanIssuanceInput{ - CustomerID: env.CustomerID, - Amount: creditAmount, - Currency: env.CurrencyReference(), - CreditPriority: &priority, - Filters: ledger.CreditFilters{Version: ledger.CreditFiltersVersion1, Features: features}, - ExpiresAt: expiresAt, - SourceChargeID: sourceChargeID, + groupID := ulid.Make().String() + err = transaction.RunWithNoValue(t.Context(), enttx.NewCreator(env.DB), func(ctx context.Context) error { + breakageInputs, err := breakageService.PlanIssuance(ctx, ledgerbreakage.PlanIssuanceInput{ + PostingInput: ledgerbreakage.PostingInput{TransactionGroupID: groupID}, + CustomerID: env.CustomerID, + Amount: creditAmount, + Currency: env.CurrencyReference(), + CreditPriority: &priority, + Filters: ledger.CreditFilters{Version: ledger.CreditFiltersVersion1, Features: features}, + ExpiresAt: expiresAt, + SourceChargeID: sourceChargeID, + }) + if err != nil { + return err + } + + inputs = append(inputs, breakageInputs...) + _, err = env.Deps.HistoricalLedger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs(env.Namespace, nil, inputs...), groupID)) + return err }) require.NoError(t, err) - require.Len(t, pending, 1) - inputs = append(inputs, breakageInputs...) - group, err := env.Deps.HistoricalLedger.CommitGroup(t.Context(), transactions.GroupInputs(env.Namespace, nil, inputs...)) + plans, err := breakageService.ListPlans(t.Context(), ledgerbreakage.ListPlansInput{ + CustomerID: env.CustomerID, + Currency: env.Currency, + AsOf: env.Now(), + }) require.NoError(t, err) - require.NoError(t, breakageService.PersistCommittedRecords(t.Context(), pending, group)) - - return pending[0].ID.ID + for _, plan := range plans { + if plan.BreakageTransactionGroupID == groupID { + return plan.ID.ID + } + } + t.Fatal("issued credit has no breakage plan") + return "" } func bookFutureFBOCollection(t *testing.T, env *ledgertestutils.IntegrationEnv, priority int, amount int64, at time.Time) { diff --git a/openmeter/ledger/collector/correction/breakage.go b/openmeter/ledger/collector/correction/breakage.go index 8af86fd0fd..1620c7c557 100644 --- a/openmeter/ledger/collector/correction/breakage.go +++ b/openmeter/ledger/collector/correction/breakage.go @@ -18,15 +18,15 @@ type correctedFBOEntry struct { amount alpacadecimal.Decimal } -func (c *Corrector) resolveBreakageReopenInputs(ctx context.Context, input Input, correction transactions.CorrectionInput) ([]ledger.TransactionInput, []breakage.PendingRecord, error) { +func (c *Corrector) resolveBreakageReopenInputs(ctx context.Context, input Input, correction transactions.CorrectionInput) ([]ledger.TransactionInput, error) { templateCode, err := ledger.TransactionTemplateCodeFromAnnotations(correction.OriginalTransaction.Annotations()) if err != nil { - return nil, nil, fmt.Errorf("transaction %s template code: %w", correction.OriginalTransaction.ID().ID, err) + return nil, fmt.Errorf("transaction %s template code: %w", correction.OriginalTransaction.ID().ID, err) } if templateCode != transactions.TemplateCode(transactions.TransferCustomerFBOToAccruedTemplate{}) && templateCode != transactions.TemplateCode(transactions.CoverCustomerReceivableTemplate{}) { - return nil, nil, nil + return nil, nil } correctedEntries := correctedFBOEntriesForAmount(correction.OriginalTransaction, correction.Amount) @@ -40,7 +40,7 @@ func (c *Corrector) resolveBreakageReopenInputs(ctx context.Context, input Input } if len(correctedEntries) == 0 { - return nil, nil, nil + return nil, nil } sourceEntryIDs := make([]string, 0, len(correctedEntries)) @@ -53,7 +53,7 @@ func (c *Corrector) resolveBreakageReopenInputs(ctx context.Context, input Input SourceEntryID: sourceEntryIDs, }) if err != nil { - return nil, nil, fmt.Errorf("list breakage releases: %w", err) + return nil, fmt.Errorf("list breakage releases: %w", err) } releasesBySourceEntryID := make(map[string][]breakage.Release, len(releases)) @@ -66,7 +66,6 @@ func (c *Corrector) resolveBreakageReopenInputs(ctx context.Context, input Input } inputs := make([]ledger.TransactionInput, 0, len(releases)) - pending := make([]breakage.PendingRecord, 0, len(releases)) for _, correctedEntry := range correctedEntries { remaining := correctedEntry.amount for _, release := range releasesBySourceEntryID[correctedEntry.entry.ID().ID] { @@ -79,7 +78,8 @@ func (c *Corrector) resolveBreakageReopenInputs(ctx context.Context, input Input continue } - reopenInput, reopenRecord, err := c.breakage.ReopenRelease(ctx, breakage.ReopenReleaseInput{ + reopenInput, err := c.breakage.ReopenRelease(ctx, breakage.ReopenReleaseInput{ + PostingInput: input.breakagePosting, Release: release, Amount: amount, SourceKind: breakage.SourceKindUsageCorrection, @@ -88,16 +88,15 @@ func (c *Corrector) resolveBreakageReopenInputs(ctx context.Context, input Input CollectionOriginID: correctedEntry.entry.Provenance().CollectionOriginID, }) if err != nil { - return nil, nil, fmt.Errorf("resolve breakage reopen: %w", err) + return nil, fmt.Errorf("resolve breakage reopen: %w", err) } inputs = append(inputs, reopenInput) - pending = append(pending, reopenRecord) remaining = remaining.Sub(amount) } } - return inputs, pending, nil + return inputs, nil } func correctedFBOEntriesForAmount(transaction ledger.Transaction, amount alpacadecimal.Decimal) []correctedFBOEntry { diff --git a/openmeter/ledger/collector/correction/correct.go b/openmeter/ledger/collector/correction/correct.go index ec0346c3b6..4ee4944a31 100644 --- a/openmeter/ledger/collector/correction/correct.go +++ b/openmeter/ledger/collector/correction/correct.go @@ -4,10 +4,13 @@ import ( "context" "fmt" + "github.com/oklog/ulid/v2" + "github.com/openmeterio/openmeter/openmeter/billing/charges/models/creditrealization" "github.com/openmeterio/openmeter/openmeter/billing/charges/models/ledgertransaction" "github.com/openmeterio/openmeter/openmeter/customer" "github.com/openmeterio/openmeter/openmeter/ledger" + "github.com/openmeterio/openmeter/openmeter/ledger/breakage" "github.com/openmeterio/openmeter/openmeter/ledger/transactions" "github.com/openmeterio/openmeter/pkg/framework/transaction" "github.com/openmeterio/openmeter/pkg/models" @@ -35,6 +38,19 @@ func (c *Corrector) Correct(ctx context.Context, input Input) (creditrealization return nil, err } + groupAnnotations := input.Annotations + if groupAnnotations == nil { + groupAnnotations = ledger.ChargeAnnotations(models.NamespacedID{ + Namespace: input.Namespace, + ID: input.ChargeID, + }) + } + + input.breakagePosting = breakage.PostingInput{ + TransactionGroupID: ulid.Make().String(), + Annotations: groupAnnotations, + } + plan, err := c.prepareCorrections(ctx, input) if err != nil { return nil, err @@ -44,14 +60,6 @@ func (c *Corrector) Correct(ctx context.Context, input Input) (creditrealization return nil, nil } - groupAnnotations := input.Annotations - if groupAnnotations == nil { - groupAnnotations = ledger.ChargeAnnotations(models.NamespacedID{ - Namespace: input.Namespace, - ID: input.ChargeID, - }) - } - // Write the whole correction batch as one group and point every new correction // realization at that group. for i, txInput := range plan.inputs { @@ -60,19 +68,15 @@ func (c *Corrector) Correct(ctx context.Context, input Input) (creditrealization } } - transactionGroup, err := c.ledger.CommitGroup(ctx, transactions.GroupInputs( + transactionGroup, err := c.ledger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs( input.Namespace, groupAnnotations, plan.inputs..., - )) + ), input.breakagePosting.TransactionGroupID)) if err != nil { return nil, fmt.Errorf("commit correction transaction group: %w", err) } - if err := c.breakage.PersistCommittedRecords(ctx, plan.breakagePending, transactionGroup); err != nil { - return nil, fmt.Errorf("persist breakage records: %w", err) - } - for i := range plan.realizations { plan.realizations[i].LedgerTransaction = ledgertransaction.GroupReference{ TransactionGroupID: transactionGroup.ID().ID, diff --git a/openmeter/ledger/collector/correction/input.go b/openmeter/ledger/collector/correction/input.go index cf3df08903..1131853129 100644 --- a/openmeter/ledger/collector/correction/input.go +++ b/openmeter/ledger/collector/correction/input.go @@ -8,10 +8,12 @@ import ( "github.com/openmeterio/openmeter/openmeter/billing/charges/legacylineage" "github.com/openmeterio/openmeter/openmeter/billing/charges/models/creditrealization" "github.com/openmeterio/openmeter/openmeter/customer" + "github.com/openmeterio/openmeter/openmeter/ledger/breakage" "github.com/openmeterio/openmeter/pkg/models" ) type Input struct { + breakagePosting breakage.PostingInput Namespace string ChargeID string CustomerID string diff --git a/openmeter/ledger/collector/correction/legacy_correction.go b/openmeter/ledger/collector/correction/legacy_correction.go index e0e408172c..ffa01312fe 100644 --- a/openmeter/ledger/collector/correction/legacy_correction.go +++ b/openmeter/ledger/collector/correction/legacy_correction.go @@ -94,7 +94,7 @@ func (b *legacyCorrectionBatch) resolve(ctx context.Context) (correctionPlan, er } for _, correction := range corrections { - breakageInputs, pending, err := b.corrector.resolveBreakageReopenInputs(ctx, b.input, correction) + breakageInputs, err := b.corrector.resolveBreakageReopenInputs(ctx, b.input, correction) if err != nil { return correctionPlan{}, err } @@ -106,7 +106,6 @@ func (b *legacyCorrectionBatch) resolve(ctx context.Context) (correctionPlan, er plan.inputs = append(plan.inputs, breakageInputs...) plan.inputs = append(plan.inputs, postings...) - plan.breakagePending = append(plan.breakagePending, pending...) } return plan, nil @@ -488,6 +487,7 @@ func (c *Corrector) planUntrackedCorrection(ctx context.Context, input Input, so func (c *Corrector) planLegacyAdvanceCorrection(ctx context.Context, input Input, source collectedSource, backingGroupID *string, amount alpacadecimal.Decimal) (legacyCorrectionPlan, error) { plan, err := c.advance.PlanLegacyCorrection(ctx, advance.LegacyCorrectionInput{ + BreakagePosting: input.breakagePosting, CustomerID: customer.CustomerID{ Namespace: input.Namespace, ID: input.CustomerID, @@ -506,8 +506,7 @@ func (c *Corrector) planLegacyAdvanceCorrection(ctx context.Context, input Input return legacyCorrectionPlan{ correctionPlan: correctionPlan{ - inputs: plan.Inputs, - breakagePending: plan.BreakagePending, + inputs: plan.Inputs, }, legacyCorrections: plan.LegacyCorrections, }, nil diff --git a/openmeter/ledger/collector/correction/origin_correction.go b/openmeter/ledger/collector/correction/origin_correction.go index 27839fcf5a..23ff6a86eb 100644 --- a/openmeter/ledger/collector/correction/origin_correction.go +++ b/openmeter/ledger/collector/correction/origin_correction.go @@ -136,6 +136,7 @@ func (c *Corrector) correctOrigin(ctx context.Context, input Input, source colle } plan, err := c.advance.PlanCorrection(ctx, advance.CorrectionInput{ + BreakagePosting: input.breakagePosting, CustomerID: customer.CustomerID{ Namespace: input.Namespace, ID: input.CustomerID, @@ -151,7 +152,6 @@ func (c *Corrector) correctOrigin(ctx context.Context, input Input, source colle } out.inputs = append(out.inputs, plan.Inputs...) - out.breakagePending = append(out.breakagePending, plan.BreakagePending...) return out, nil } @@ -171,13 +171,12 @@ func (c *Corrector) correctOrigin(ctx context.Context, input Input, source colle Amount: amount, At: input.AllocateAt, } - inputs, pending, err := c.resolveBreakageReopenInputs(ctx, input, plan) + inputs, err := c.resolveBreakageReopenInputs(ctx, input, plan) if err != nil { return out, err } out.inputs = append(out.inputs, inputs...) - out.breakagePending = append(out.breakagePending, pending...) return out, nil } diff --git a/openmeter/ledger/collector/correction/prepare.go b/openmeter/ledger/collector/correction/prepare.go index d12860a853..cd9848c04f 100644 --- a/openmeter/ledger/collector/correction/prepare.go +++ b/openmeter/ledger/collector/correction/prepare.go @@ -7,19 +7,16 @@ import ( "github.com/openmeterio/openmeter/openmeter/billing/charges/models/creditrealization" "github.com/openmeterio/openmeter/openmeter/ledger" - "github.com/openmeterio/openmeter/openmeter/ledger/breakage" "github.com/openmeterio/openmeter/pkg/models" ) type correctionPlan struct { - inputs []ledger.TransactionInput - breakagePending []breakage.PendingRecord - realizations creditrealization.CreateCorrectionInputs + inputs []ledger.TransactionInput + realizations creditrealization.CreateCorrectionInputs } func (p *correctionPlan) append(other correctionPlan) { p.inputs = append(p.inputs, other.inputs...) - p.breakagePending = append(p.breakagePending, other.breakagePending...) p.realizations = append(p.realizations, other.realizations...) } diff --git a/openmeter/ledger/creditvoid/service.go b/openmeter/ledger/creditvoid/service.go index 499e627cb9..d2d8f3e263 100644 --- a/openmeter/ledger/creditvoid/service.go +++ b/openmeter/ledger/creditvoid/service.go @@ -345,10 +345,10 @@ func (s *service) openExpiryPlansBySubAccount(ctx context.Context, input VoidCre func (s *service) executeVoid(ctx context.Context, input VoidCreditPurchaseInput, plan voidPlan) (VoidCreditPurchaseResult, error) { var ( inputs []ledger.TransactionInput - pendingBreakage []breakage.PendingRecord pendingVoidRecords []pendingVoidRecord ) amount := alpacadecimal.Zero + groupID := ulid.Make().String() for _, slice := range plan.slices { voidTx, voidRecord, err := s.resolveVoidSlice(ctx, input, plan.voidedAt, slice) @@ -364,7 +364,8 @@ func (s *service) executeVoid(ctx context.Context, input VoidCreditPurchaseInput continue } - releaseTx, releaseRecord, err := s.breakage.ReleasePlan(ctx, breakage.ReleasePlanInput{ + releaseTx, err := s.breakage.ReleasePlan(ctx, breakage.ReleasePlanInput{ + PostingInput: breakage.PostingInput{TransactionGroupID: groupID, Annotations: input.Annotations}, Plan: *slice.expiryPlan, Amount: slice.amount, SourceKind: breakage.SourceKindCreditPurchase, @@ -375,7 +376,6 @@ func (s *service) executeVoid(ctx context.Context, input VoidCreditPurchaseInput } inputs = append(inputs, releaseTx) - pendingBreakage = append(pendingBreakage, releaseRecord) } for i, txInput := range inputs { @@ -384,19 +384,15 @@ func (s *service) executeVoid(ctx context.Context, input VoidCreditPurchaseInput } } - transactionGroup, err := s.ledger.CommitGroup(ctx, transactions.GroupInputs( + transactionGroup, err := s.ledger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs( input.CustomerID.Namespace, input.Annotations, inputs..., - )) + ), groupID)) if err != nil { return VoidCreditPurchaseResult{}, fmt.Errorf("commit ledger transaction group: %w", err) } - if err := s.breakage.PersistCommittedRecords(ctx, pendingBreakage, transactionGroup); err != nil { - return VoidCreditPurchaseResult{}, fmt.Errorf("persist breakage records: %w", err) - } - if err := s.persistCommittedVoidRecords(ctx, pendingVoidRecords, transactionGroup); err != nil { return VoidCreditPurchaseResult{}, fmt.Errorf("persist void records: %w", err) } diff --git a/openmeter/ledger/customerbalance/expired_loader_test.go b/openmeter/ledger/customerbalance/expired_loader_test.go index 7a8cc713f4..e0d83b6cbe 100644 --- a/openmeter/ledger/customerbalance/expired_loader_test.go +++ b/openmeter/ledger/customerbalance/expired_loader_test.go @@ -2,11 +2,13 @@ package customerbalance import ( "cmp" + "context" "slices" "testing" "time" "github.com/alpacahq/alpacadecimal" + "github.com/oklog/ulid/v2" "github.com/samber/lo" "github.com/samber/mo" "github.com/stretchr/testify/require" @@ -16,11 +18,13 @@ import ( chargemeta "github.com/openmeterio/openmeter/openmeter/billing/charges/meta" "github.com/openmeterio/openmeter/openmeter/currencies" currenciestestutils "github.com/openmeterio/openmeter/openmeter/currencies/testutils" + enttx "github.com/openmeterio/openmeter/openmeter/ent/tx" "github.com/openmeterio/openmeter/openmeter/ledger" ledgerbreakage "github.com/openmeterio/openmeter/openmeter/ledger/breakage" "github.com/openmeterio/openmeter/openmeter/ledger/transactions" "github.com/openmeterio/openmeter/openmeter/productcatalog" "github.com/openmeterio/openmeter/pkg/clock" + "github.com/openmeterio/openmeter/pkg/framework/transaction" "github.com/openmeterio/openmeter/pkg/timeutil" ) @@ -138,14 +142,12 @@ func TestListCreditTransactionsExpiredBreakagePreservesCustomCurrencyIdentity(t env.bookFBOBalanceInCurrencyReferenceWithFeatures(t, amount, grant.currency, nil) env.fundOpenReceivableInCurrencyReferenceWithFeatures(t, amount, grant.currency, nil) - inputs, pending, err := env.BreakageService.PlanIssuance(t.Context(), ledgerbreakage.PlanIssuanceInput{ + env.bookBreakageIssuance(t, ledgerbreakage.PlanIssuanceInput{ CustomerID: env.CustomerID, Amount: amount, Currency: grant.currency, ExpiresAt: expiresAt, }) - require.NoError(t, err) - env.commitBreakageRecords(t, inputs, pending) } expiredType := CreditTransactionTypeExpired @@ -198,15 +200,13 @@ func TestListCreditTransactionsExpiredBreakageFeatureFilter(t *testing.T) { env.bookFBOBalanceWithFeatures(t, amount, spec.features) env.fundOpenReceivableInCurrencyWithFeatures(t, amount, env.Currency, spec.features) - inputs, pending, err := env.BreakageService.PlanIssuance(t.Context(), ledgerbreakage.PlanIssuanceInput{ + env.bookBreakageIssuance(t, ledgerbreakage.PlanIssuanceInput{ CustomerID: env.CustomerID, Amount: amount, Currency: env.CurrencyReference(), Filters: ledger.CreditFilters{Version: ledger.CreditFiltersVersion1, Features: spec.features}, ExpiresAt: expiresAt, }) - require.NoError(t, err) - env.commitBreakageRecords(t, inputs, pending) } expiredType := CreditTransactionTypeExpired @@ -624,15 +624,12 @@ func (e *testEnv) bookExpiredListingState(t *testing.T, issuedAt time.Time, spec e.fundOpenReceivable(t, total) for _, spec := range specs { - inputs, pending, err := e.BreakageService.PlanIssuance(t.Context(), ledgerbreakage.PlanIssuanceInput{ + e.bookBreakageIssuance(t, ledgerbreakage.PlanIssuanceInput{ CustomerID: e.CustomerID, Amount: alpacadecimal.NewFromInt(spec.amount), Currency: e.CurrencyReference(), ExpiresAt: issuedAt.Add(spec.expiresAfter), }) - require.NoError(t, err) - - e.commitBreakageRecords(t, inputs, pending) } plans, err := e.BreakageService.ListPlans(t.Context(), ledgerbreakage.ListPlansInput{ @@ -656,14 +653,21 @@ func (e *testEnv) bookExpiredListingState(t *testing.T, issuedAt time.Time, spec t.Cleanup(clock.UnFreeze) e.bookFBOUsage(t, usageAt, plan.FBOAddress, releaseAmount) - releaseInput, releaseRecord, err := e.BreakageService.ReleasePlan(t.Context(), ledgerbreakage.ReleasePlanInput{ - Plan: plan, - Amount: releaseAmount, - SourceKind: ledgerbreakage.SourceKindUsage, + releaseGroupID := e.commitBreakagePostings(t, func(ctx context.Context, posting ledgerbreakage.PostingInput) ([]ledger.TransactionInput, error) { + input, err := e.BreakageService.ReleasePlan(ctx, ledgerbreakage.ReleasePlanInput{ + PostingInput: posting, + Plan: plan, + Amount: releaseAmount, + SourceKind: ledgerbreakage.SourceKindUsage, + }) + return []ledger.TransactionInput{input}, err + }) + releases, err := e.BreakageService.ListReleases(t.Context(), ledgerbreakage.ListReleasesInput{ + CustomerID: e.CustomerID, + SourceTransactionGroupID: []string{releaseGroupID}, }) require.NoError(t, err) - - e.commitBreakageRecords(t, []ledger.TransactionInput{releaseInput}, []ledgerbreakage.PendingRecord{releaseRecord}) + require.Len(t, releases, 1) if spec.reopen == 0 { continue @@ -677,19 +681,15 @@ func (e *testEnv) bookExpiredListingState(t *testing.T, issuedAt time.Time, spec e.bookFBORestore(t, reopenAt, reopenAmount) e.fundOpenReceivable(t, reopenAmount) - reopenInput, reopenRecord, err := e.BreakageService.ReopenRelease(t.Context(), ledgerbreakage.ReopenReleaseInput{ - Release: ledgerbreakage.Release{ - Record: releaseRecord.Record, - OpenAmount: releaseAmount, - FBOAddress: plan.FBOAddress, - BreakageAddress: plan.BreakageAddress, - }, - Amount: reopenAmount, - SourceKind: ledgerbreakage.SourceKindUsageCorrection, + e.commitBreakagePostings(t, func(ctx context.Context, posting ledgerbreakage.PostingInput) ([]ledger.TransactionInput, error) { + input, err := e.BreakageService.ReopenRelease(ctx, ledgerbreakage.ReopenReleaseInput{ + PostingInput: posting, + Release: releases[0], + Amount: reopenAmount, + SourceKind: ledgerbreakage.SourceKindUsageCorrection, + }) + return []ledger.TransactionInput{input}, err }) - require.NoError(t, err) - - e.commitBreakageRecords(t, []ledger.TransactionInput{reopenInput}, []ledgerbreakage.PendingRecord{reopenRecord}) } } @@ -753,12 +753,31 @@ func (e *testEnv) createPromotionalCreditFunding(t *testing.T, fundedAt time.Tim return result.Charge } -func (e *testEnv) commitBreakageRecords(t *testing.T, inputs []ledger.TransactionInput, pending []ledgerbreakage.PendingRecord) { +func (e *testEnv) bookBreakageIssuance(t *testing.T, input ledgerbreakage.PlanIssuanceInput) { + t.Helper() + + e.commitBreakagePostings(t, func(ctx context.Context, posting ledgerbreakage.PostingInput) ([]ledger.TransactionInput, error) { + input.PostingInput = posting + return e.BreakageService.PlanIssuance(ctx, input) + }) +} + +func (e *testEnv) commitBreakagePostings(t *testing.T, resolve func(context.Context, ledgerbreakage.PostingInput) ([]ledger.TransactionInput, error)) string { t.Helper() - group, err := e.Deps.HistoricalLedger.CommitGroup(t.Context(), transactions.GroupInputs(e.Namespace, nil, inputs...)) + groupID := ulid.Make().String() + err := transaction.RunWithNoValue(t.Context(), enttx.NewCreator(e.DB), func(ctx context.Context) error { + inputs, err := resolve(ctx, ledgerbreakage.PostingInput{TransactionGroupID: groupID}) + if err != nil { + return err + } + + _, err = e.Deps.HistoricalLedger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs(e.Namespace, nil, inputs...), groupID)) + return err + }) require.NoError(t, err) - require.NoError(t, e.BreakageService.PersistCommittedRecords(t.Context(), pending, group)) + + return groupID } func (e *testEnv) bookFBOUsage(t *testing.T, at time.Time, address ledger.PostingAddress, amount alpacadecimal.Decimal) { diff --git a/tools/migrate/README.md b/tools/migrate/README.md index 5ddf0b1e4f..0f375a2201 100644 --- a/tools/migrate/README.md +++ b/tools/migrate/README.md @@ -1,3 +1,8 @@ +## Schema source + +Atlas loads Ent's schema through `entschema`, including deferred FK definitions +from `deferred_constraints.sql` that Ent cannot express. + ## View SQL Helper Generate SQL definitions for `ent.View` schemas: diff --git a/tools/migrate/deferred_constraints.sql b/tools/migrate/deferred_constraints.sql new file mode 100644 index 0000000000..bfc2122fe3 --- /dev/null +++ b/tools/migrate/deferred_constraints.sql @@ -0,0 +1,8 @@ +-- Breakage bookkeeping is inserted before the caller posts its ledger group. +-- References must exist when the enclosing transaction commits. +ALTER TABLE "ledger_breakage_records" + ALTER CONSTRAINT "ledger_breakage_records_ledger_entries_source_breakage_records" DEFERRABLE INITIALLY DEFERRED, + ALTER CONSTRAINT "ledger_breakage_records_ledger_transaction_groups_breakage_reco" DEFERRABLE INITIALLY DEFERRED, + ALTER CONSTRAINT "ledger_breakage_records_ledger_transaction_groups_source_breaka" DEFERRABLE INITIALLY DEFERRED, + ALTER CONSTRAINT "ledger_breakage_records_ledger_transactions_breakage_records" DEFERRABLE INITIALLY DEFERRED, + ALTER CONSTRAINT "ledger_breakage_records_ledger_transactions_source_breakage_rec" DEFERRABLE INITIALLY DEFERRED; diff --git a/tools/migrate/entschema/main.go b/tools/migrate/entschema/main.go new file mode 100644 index 0000000000..3e6c109314 --- /dev/null +++ b/tools/migrate/entschema/main.go @@ -0,0 +1,52 @@ +package main + +import ( + "fmt" + "os" + + "entgo.io/ent/dialect" + "entgo.io/ent/dialect/sql/schema" + "entgo.io/ent/entc" + "entgo.io/ent/entc/gen" + "github.com/spf13/cobra" +) + +func main() { + cmd := &cobra.Command{ + Use: "entschema", + RunE: func(cmd *cobra.Command, _ []string) error { + graph, err := entc.LoadGraph("./openmeter/ent/schema", &gen.Config{}) + if err != nil { + return err + } + tables, err := graph.Tables() + if err != nil { + return err + } + views, err := graph.Views() + if err != nil { + return err + } + ddl, err := schema.DDL(cmd.Context(), schema.DDLArgs{ + Dialect: dialect.Postgres, + Version: "15", + Tables: append(tables, views...), + }) + if err != nil { + return err + } + + // Ent cannot express deferred FKs. Include their timing in Atlas's + // desired schema so future diffs preserve it. + constraints, err := os.ReadFile("tools/migrate/deferred_constraints.sql") + if err != nil { + return err + } + _, err = fmt.Fprintf(cmd.OutOrStdout(), "%s\n%s", ddl, constraints) + return err + }, + } + if err := cmd.Execute(); err != nil { + os.Exit(1) + } +} diff --git a/tools/migrate/ledger_breakage_references_test.go b/tools/migrate/ledger_breakage_references_test.go new file mode 100644 index 0000000000..98fa3a7af9 --- /dev/null +++ b/tools/migrate/ledger_breakage_references_test.go @@ -0,0 +1,70 @@ +package migrate_test + +import ( + "database/sql" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestBreakageLedgerReferenceConstraintTiming(t *testing.T) { + const ( + versionBefore = 20260925133042 + versionAfter = 20261005135427 + ) + + runner{ + stops: stops{ + { + version: versionBefore, + direction: directionUp, + action: func(t *testing.T, db *sql.DB) { + requireBreakageReferenceTiming(t, db, false) + }, + }, + { + version: versionAfter, + direction: directionUp, + action: func(t *testing.T, db *sql.DB) { + // Ledger references may be unresolved until the caller posts its group. + requireBreakageReferenceTiming(t, db, true) + }, + }, + { + version: versionBefore, + direction: directionDown, + action: func(t *testing.T, db *sql.DB) { + requireBreakageReferenceTiming(t, db, false) + }, + }, + }, + }.Test(t) +} + +func requireBreakageReferenceTiming(t *testing.T, db *sql.DB, deferred bool) { + t.Helper() + + var count int + var deferrable, initiallyDeferred bool + err := db.QueryRowContext(t.Context(), ` + SELECT COUNT(*), BOOL_AND(condeferrable), BOOL_AND(condeferred) + FROM pg_constraint + WHERE conrelid = 'ledger_breakage_records'::regclass + AND confrelid IN ('ledger_entries'::regclass, 'ledger_transactions'::regclass, 'ledger_transaction_groups'::regclass) + `).Scan(&count, &deferrable, &initiallyDeferred) + require.NoError(t, err) + require.Equal(t, 5, count) + require.Equal(t, deferred, deferrable) + require.Equal(t, deferred, initiallyDeferred) + + // Plan/release and subaccount references remain immediate. + err = db.QueryRowContext(t.Context(), ` + SELECT COUNT(*) FROM pg_constraint + WHERE conrelid = 'ledger_breakage_records'::regclass + AND contype = 'f' + AND confrelid NOT IN ('ledger_entries'::regclass, 'ledger_transactions'::regclass, 'ledger_transaction_groups'::regclass) + AND NOT condeferrable AND NOT condeferred + `).Scan(&count) + require.NoError(t, err) + require.Equal(t, 4, count) +} diff --git a/tools/migrate/migrations/20261005135427_defer_breakage_ledger_references.down.sql b/tools/migrate/migrations/20261005135427_defer_breakage_ledger_references.down.sql new file mode 100644 index 0000000000..6989170303 --- /dev/null +++ b/tools/migrate/migrations/20261005135427_defer_breakage_ledger_references.down.sql @@ -0,0 +1,2 @@ +-- reverse: modify "ledger_breakage_records" table +ALTER TABLE "ledger_breakage_records" ALTER CONSTRAINT "ledger_breakage_records_ledger_transactions_source_breakage_rec" NOT DEFERRABLE INITIALLY IMMEDIATE, ALTER CONSTRAINT "ledger_breakage_records_ledger_transactions_breakage_records" NOT DEFERRABLE INITIALLY IMMEDIATE, ALTER CONSTRAINT "ledger_breakage_records_ledger_transaction_groups_source_breaka" NOT DEFERRABLE INITIALLY IMMEDIATE, ALTER CONSTRAINT "ledger_breakage_records_ledger_transaction_groups_breakage_reco" NOT DEFERRABLE INITIALLY IMMEDIATE, ALTER CONSTRAINT "ledger_breakage_records_ledger_entries_source_breakage_records" NOT DEFERRABLE INITIALLY IMMEDIATE; diff --git a/tools/migrate/migrations/20261005135427_defer_breakage_ledger_references.up.sql b/tools/migrate/migrations/20261005135427_defer_breakage_ledger_references.up.sql new file mode 100644 index 0000000000..880ba447b9 --- /dev/null +++ b/tools/migrate/migrations/20261005135427_defer_breakage_ledger_references.up.sql @@ -0,0 +1,2 @@ +-- modify "ledger_breakage_records" table +ALTER TABLE "ledger_breakage_records" ALTER CONSTRAINT "ledger_breakage_records_ledger_entries_source_breakage_records" DEFERRABLE INITIALLY DEFERRED, ALTER CONSTRAINT "ledger_breakage_records_ledger_transaction_groups_breakage_reco" DEFERRABLE INITIALLY DEFERRED, ALTER CONSTRAINT "ledger_breakage_records_ledger_transaction_groups_source_breaka" DEFERRABLE INITIALLY DEFERRED, ALTER CONSTRAINT "ledger_breakage_records_ledger_transactions_breakage_records" DEFERRABLE INITIALLY DEFERRED, ALTER CONSTRAINT "ledger_breakage_records_ledger_transactions_source_breakage_rec" DEFERRABLE INITIALLY DEFERRED; diff --git a/tools/migrate/migrations/atlas.sum b/tools/migrate/migrations/atlas.sum index 6a19854c84..2fec73af31 100644 --- a/tools/migrate/migrations/atlas.sum +++ b/tools/migrate/migrations/atlas.sum @@ -1,4 +1,4 @@ -h1:7gd5eHSsGN4XynaF7wF19pKFMst7/uZk6hGafMtbB2o= +h1:a/jjdfEyCyoP3d5U4NWpx9dEv+lD/MKl0drz9PRR3Cw= 20240826120919_init.up.sql h1:tc1V91/smlmaeJGQ8h+MzTEeFjjnrrFDbDAjOYJK91o= 20240903155435_entitlement-expired-index.up.sql h1:Hp8u5uckmLXc1cRvWU0AtVnnK8ShlpzZNp8pbiJLhac= 20240917172257_billing-entities.up.sql h1:Q1dAMo0Vjiit76OybClNfYPGC5nmvov2/M2W1ioi4Kw= @@ -299,3 +299,4 @@ h1:7gd5eHSsGN4XynaF7wF19pKFMst7/uZk6hGafMtbB2o= 20260921133354_credit_filter_cutover.up.sql h1:8EburzTZYQfHm3+vJcSj8pvWuVqMJdEPDpDe9YCgYBI= 20260922140147_credit_plan_filters.up.sql h1:442yNkCRktayTLyVSwqhPlQr8d/vdydz+EVVQJIP9fU= 20260925133042_system_event_outbox.up.sql h1:ZVp1yS5B1i1gZrQrO0/W5s4x41xEaVXcFIAHz2/kD3E= +20261005135427_defer_breakage_ledger_references.up.sql h1:lbLP27I0AwFURLhznoQchRpySiewqKh8h4iCKq9WvNs= From eecf415f3b2536f35d70b06e20047dca3a5d054a Mon Sep 17 00:00:00 2001 From: Alex Goth <64845621+GAlexIHU@users.noreply.github.com> Date: Tue, 6 Oct 2026 14:10:46 +0200 Subject: [PATCH 4/5] refactor(migrate): separate schema exporters from commands --- atlas.hcl | 2 +- openmeter/ent/entc.go | 5 -- openmeter/ent/generate.go | 1 + tools/migrate/README.md | 9 ++- tools/migrate/cmd/entschema/main.go | 37 ++++++++++ tools/migrate/cmd/viewgen/main.go | 15 +++- tools/migrate/entschema/entschema.go | 75 ++++++++++++++++++++ tools/migrate/entschema/entschema_test.go | 85 +++++++++++++++++++++++ tools/migrate/entschema/main.go | 52 -------------- tools/migrate/viewgen/viewgen.go | 23 ------ 10 files changed, 218 insertions(+), 86 deletions(-) create mode 100644 tools/migrate/cmd/entschema/main.go create mode 100644 tools/migrate/entschema/entschema.go create mode 100644 tools/migrate/entschema/entschema_test.go delete mode 100644 tools/migrate/entschema/main.go diff --git a/atlas.hcl b/atlas.hcl index b3c4c63f86..9185e61e83 100644 --- a/atlas.hcl +++ b/atlas.hcl @@ -1,5 +1,5 @@ data "external_schema" "ent" { - program = ["go", "run", "-mod=readonly", "./tools/migrate/entschema"] + program = ["go", "run", "-mod=readonly", "./tools/migrate/cmd/entschema"] } env "local" { diff --git a/openmeter/ent/entc.go b/openmeter/ent/entc.go index e395d355e2..dc879de8cd 100644 --- a/openmeter/ent/entc.go +++ b/openmeter/ent/entc.go @@ -14,7 +14,6 @@ import ( "github.com/openmeterio/openmeter/pkg/framework/entutils/entmixinaccessor" "github.com/openmeterio/openmeter/pkg/framework/entutils/entpaginate" "github.com/openmeterio/openmeter/pkg/framework/entutils/entsetorclear" - "github.com/openmeterio/openmeter/tools/migrate/viewgen" ) func main() { @@ -41,8 +40,4 @@ func main() { if err != nil { log.Fatal("running ent codegen:", err) } - - if err := viewgen.GenerateFile("./schema", "../../tools/migrate/views.sql"); err != nil { - log.Fatal("generating views SQL:", err) - } } diff --git a/openmeter/ent/generate.go b/openmeter/ent/generate.go index 904d39c382..4a2ffbe18d 100644 --- a/openmeter/ent/generate.go +++ b/openmeter/ent/generate.go @@ -1,3 +1,4 @@ package entdb //go:generate go run -mod=readonly entc.go +//go:generate go run -mod=readonly ../../tools/migrate/cmd/viewgen -schema ./schema -out ../../tools/migrate/views.sql diff --git a/tools/migrate/README.md b/tools/migrate/README.md index 0f375a2201..3b3721bdcc 100644 --- a/tools/migrate/README.md +++ b/tools/migrate/README.md @@ -1,7 +1,8 @@ ## Schema source -Atlas loads Ent's schema through `entschema`, including deferred FK definitions -from `deferred_constraints.sql` that Ent cannot express. +Atlas runs `cmd/entschema`, which uses the reusable `entschema.GenerateSQL` +exporter with a schema path, PostgreSQL version, and supplemental DDL. +`deferred_constraints.sql` supplies FK definitions that Ent cannot express. ## View SQL Helper @@ -11,4 +12,6 @@ Generate SQL definitions for `ent.View` schemas: make generate-view-sql ``` -This writes `tools/migrate/views.sql` by loading `openmeter/ent/schema` via Ent's schema loader and emitting Postgres `CREATE VIEW` statements from `EntSQL` view annotations. +`viewgen.GenerateSQL` returns PostgreSQL `CREATE VIEW` statements from EntSQL +annotations. The command adds the regeneration header and writes +`tools/migrate/views.sql`. diff --git a/tools/migrate/cmd/entschema/main.go b/tools/migrate/cmd/entschema/main.go new file mode 100644 index 0000000000..8b47051374 --- /dev/null +++ b/tools/migrate/cmd/entschema/main.go @@ -0,0 +1,37 @@ +package main + +import ( + "fmt" + "os" + + "github.com/spf13/cobra" + + "github.com/openmeterio/openmeter/tools/migrate/entschema" +) + +func main() { + cmd := &cobra.Command{ + Use: "entschema", + RunE: func(cmd *cobra.Command, _ []string) error { + constraints, err := os.ReadFile("tools/migrate/deferred_constraints.sql") + if err != nil { + return err + } + + ddl, err := entschema.GenerateSQL(cmd.Context(), entschema.GenerateSQLInput{ + SchemaPath: "./openmeter/ent/schema", + PostgresVersion: "15", + SupplementalDDL: string(constraints), + }) + if err != nil { + return err + } + + _, err = fmt.Fprint(cmd.OutOrStdout(), ddl) + return err + }, + } + if err := cmd.Execute(); err != nil { + os.Exit(1) + } +} diff --git a/tools/migrate/cmd/viewgen/main.go b/tools/migrate/cmd/viewgen/main.go index c5b90fd5a6..dc500592df 100644 --- a/tools/migrate/cmd/viewgen/main.go +++ b/tools/migrate/cmd/viewgen/main.go @@ -4,6 +4,7 @@ import ( "flag" "fmt" "os" + "path/filepath" "github.com/openmeterio/openmeter/tools/migrate/viewgen" ) @@ -11,13 +12,23 @@ import ( func main() { var ( schemaPath = flag.String("schema", "./openmeter/ent/schema", "path to the ent schema package") - outPath = flag.String("out", viewgen.DefaultOutputPath, "output SQL file path") + outPath = flag.String("out", "./tools/migrate/views.sql", "output SQL file path") ) flag.Parse() - if err := viewgen.GenerateFile(*schemaPath, *outPath); err != nil { + sql, err := viewgen.GenerateSQL(*schemaPath) + if err != nil { exitf("%v", err) } + + if err := os.MkdirAll(filepath.Dir(*outPath), 0o755); err != nil { + exitf("create output dir: %v", err) + } + + output := append([]byte("-- Code generated by viewgen, DO NOT EDIT.\n-- Regenerate with: make generate-view-sql\n"), sql...) + if err := os.WriteFile(*outPath, output, 0o644); err != nil { + exitf("write output: %v", err) + } } func exitf(format string, args ...any) { diff --git a/tools/migrate/entschema/entschema.go b/tools/migrate/entschema/entschema.go new file mode 100644 index 0000000000..727af76bbc --- /dev/null +++ b/tools/migrate/entschema/entschema.go @@ -0,0 +1,75 @@ +package entschema + +import ( + "context" + "errors" + "fmt" + + "entgo.io/ent/dialect" + "entgo.io/ent/dialect/sql/schema" + "entgo.io/ent/entc" + "entgo.io/ent/entc/gen" + + "github.com/openmeterio/openmeter/pkg/models" +) + +type GenerateSQLInput struct { + SchemaPath string + PostgresVersion string + // SupplementalDDL preserves database features that Ent cannot express. + SupplementalDDL string +} + +var _ models.Validator = GenerateSQLInput{} + +func (i GenerateSQLInput) Validate() error { + var errs []error + + if i.SchemaPath == "" { + errs = append(errs, errors.New("schema path is required")) + } + + if i.PostgresVersion == "" { + errs = append(errs, errors.New("postgres version is required")) + } + + return models.NewNillableGenericValidationError(errors.Join(errs...)) +} + +// GenerateSQL exports one Ent schema's tables and views as PostgreSQL DDL, +// followed by any supplemental DDL for that schema source. +func GenerateSQL(ctx context.Context, input GenerateSQLInput) (string, error) { + if err := input.Validate(); err != nil { + return "", err + } + + graph, err := entc.LoadGraph(input.SchemaPath, &gen.Config{}) + if err != nil { + return "", fmt.Errorf("load ent schema: %w", err) + } + + tables, err := graph.Tables() + if err != nil { + return "", fmt.Errorf("schema tables: %w", err) + } + + views, err := graph.Views() + if err != nil { + return "", fmt.Errorf("schema views: %w", err) + } + + ddl, err := schema.DDL(ctx, schema.DDLArgs{ + Dialect: dialect.Postgres, + Version: input.PostgresVersion, + Tables: append(tables, views...), + }) + if err != nil { + return "", fmt.Errorf("schema DDL: %w", err) + } + + if input.SupplementalDDL != "" { + ddl += "\n" + input.SupplementalDDL + } + + return ddl, nil +} diff --git a/tools/migrate/entschema/entschema_test.go b/tools/migrate/entschema/entschema_test.go new file mode 100644 index 0000000000..cabdceb3b4 --- /dev/null +++ b/tools/migrate/entschema/entschema_test.go @@ -0,0 +1,85 @@ +package entschema_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/openmeterio/openmeter/tools/migrate/entschema" +) + +func TestGenerateSQL_IndependentSchemaSources(t *testing.T) { + // given: two independent Ent schemas with source-specific supplemental DDL. + const supplementalDDL = `COMMENT ON TABLE "example1s" IS 'schema-specific DDL';` + + for _, tc := range []struct { + name string + input entschema.GenerateSQLInput + expectedSQL []string + excludedSQL []string + }{ + { + name: "first schema with supplemental DDL", + input: entschema.GenerateSQLInput{ + SchemaPath: "../../../pkg/framework/entutils/testutils/ent1/schema", + PostgresVersion: "15", + SupplementalDDL: supplementalDDL, + }, + expectedSQL: []string{`CREATE TABLE "example1s"`, `"example_value_1"`, supplementalDDL}, + excludedSQL: []string{`"example2s"`, `"example_value_2"`}, + }, + { + name: "second schema without supplemental DDL", + input: entschema.GenerateSQLInput{ + SchemaPath: "../../../pkg/framework/entutils/testutils/ent2/schema", + PostgresVersion: "15", + }, + expectedSQL: []string{`CREATE TABLE "example2s"`, `"example_value_2"`}, + excludedSQL: []string{`"example1s"`, `"example_value_1"`, supplementalDDL}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + // when: the reusable exporter is configured for this source. + ddl, err := entschema.GenerateSQL(t.Context(), tc.input) + require.NoError(t, err) + + // then: output contains only this source's schema and supplemental DDL. + for _, expected := range tc.expectedSQL { + require.Contains(t, ddl, expected) + } + + for _, excluded := range tc.excludedSQL { + require.NotContains(t, ddl, excluded) + } + }) + } +} + +func TestGenerateSQL_InvalidInput(t *testing.T) { + for _, tc := range []struct { + name string + input entschema.GenerateSQLInput + expectedError string + }{ + { + name: "missing schema path", + input: entschema.GenerateSQLInput{PostgresVersion: "15"}, + expectedError: "schema path is required", + }, + { + name: "missing postgres version", + input: entschema.GenerateSQLInput{SchemaPath: "../../../pkg/framework/entutils/testutils/ent1/schema"}, + expectedError: "postgres version is required", + }, + { + name: "missing schema package", + input: entschema.GenerateSQLInput{SchemaPath: "testdata/missing", PostgresVersion: "15"}, + expectedError: "load ent schema", + }, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := entschema.GenerateSQL(t.Context(), tc.input) + require.ErrorContains(t, err, tc.expectedError) + }) + } +} diff --git a/tools/migrate/entschema/main.go b/tools/migrate/entschema/main.go deleted file mode 100644 index 3e6c109314..0000000000 --- a/tools/migrate/entschema/main.go +++ /dev/null @@ -1,52 +0,0 @@ -package main - -import ( - "fmt" - "os" - - "entgo.io/ent/dialect" - "entgo.io/ent/dialect/sql/schema" - "entgo.io/ent/entc" - "entgo.io/ent/entc/gen" - "github.com/spf13/cobra" -) - -func main() { - cmd := &cobra.Command{ - Use: "entschema", - RunE: func(cmd *cobra.Command, _ []string) error { - graph, err := entc.LoadGraph("./openmeter/ent/schema", &gen.Config{}) - if err != nil { - return err - } - tables, err := graph.Tables() - if err != nil { - return err - } - views, err := graph.Views() - if err != nil { - return err - } - ddl, err := schema.DDL(cmd.Context(), schema.DDLArgs{ - Dialect: dialect.Postgres, - Version: "15", - Tables: append(tables, views...), - }) - if err != nil { - return err - } - - // Ent cannot express deferred FKs. Include their timing in Atlas's - // desired schema so future diffs preserve it. - constraints, err := os.ReadFile("tools/migrate/deferred_constraints.sql") - if err != nil { - return err - } - _, err = fmt.Fprintf(cmd.OutOrStdout(), "%s\n%s", ddl, constraints) - return err - }, - } - if err := cmd.Execute(); err != nil { - os.Exit(1) - } -} diff --git a/tools/migrate/viewgen/viewgen.go b/tools/migrate/viewgen/viewgen.go index d7c923ff6c..e26200dc64 100644 --- a/tools/migrate/viewgen/viewgen.go +++ b/tools/migrate/viewgen/viewgen.go @@ -4,8 +4,6 @@ import ( "bytes" "encoding/json" "fmt" - "os" - "path/filepath" "sort" "strings" @@ -17,8 +15,6 @@ import ( "github.com/openmeterio/openmeter/pkg/strcase" ) -const DefaultOutputPath = "./tools/migrate/views.sql" - type ViewDef struct { Name string Query string @@ -38,23 +34,6 @@ func GenerateSQL(schemaPath string) ([]byte, error) { return RenderSQL(views), nil } -func GenerateFile(schemaPath, outPath string) error { - sql, err := GenerateSQL(schemaPath) - if err != nil { - return err - } - - if err := os.MkdirAll(filepath.Dir(outPath), 0o755); err != nil { - return fmt.Errorf("create output dir: %w", err) - } - - if err := os.WriteFile(outPath, sql, 0o644); err != nil { - return fmt.Errorf("write output: %w", err) - } - - return nil -} - func CollectViews(schemas []*load.Schema, dialectName string) ([]ViewDef, error) { views := make([]ViewDef, 0) @@ -105,8 +84,6 @@ func CollectViews(schemas []*load.Schema, dialectName string) ([]ViewDef, error) func RenderSQL(views []ViewDef) []byte { var buf bytes.Buffer - buf.WriteString("-- Code generated by viewgen, DO NOT EDIT.\n") - buf.WriteString("-- Regenerate with: make generate-view-sql\n") for i, view := range views { if i > 0 { buf.WriteString("\n\n") From 4dc6f87f46cf9a6f802efd80cc2414896606a40e Mon Sep 17 00:00:00 2001 From: Alex Goth <64845621+GAlexIHU@users.noreply.github.com> Date: Tue, 6 Oct 2026 16:55:17 +0200 Subject: [PATCH 5/5] style(ledger): adopt boundary spacing lint rules --- openmeter/ledger/advance/service/legacy_correction.go | 1 + openmeter/ledger/breakage/service.go | 3 +++ openmeter/ledger/breakage/service_test.go | 9 +++++++++ .../ledger/chargeadapter/customcurrencyoverage.go | 2 ++ .../chargeadapter/flatfee_customcurrency_test.go | 4 ++++ openmeter/ledger/collector/collect.go | 4 ++++ openmeter/ledger/collector/collection_fbo_test.go | 3 +++ .../ledger/customerbalance/expired_loader_test.go | 3 +++ openmeter/ledger/historical/adapter/ledger.go | 7 +++++++ openmeter/ledger/historical/ledger.go | 1 + openmeter/ledger/historical/repo.go | 2 ++ openmeter/ledger/ids.go | 5 +++++ openmeter/ledger/transactions/input_test.go | 3 +++ openmeter/ledger/validations.go | 11 +++++++++++ tools/migrate/cmd/entschema/main.go | 1 + 15 files changed, 59 insertions(+) diff --git a/openmeter/ledger/advance/service/legacy_correction.go b/openmeter/ledger/advance/service/legacy_correction.go index 74999c8457..cfc43e137c 100644 --- a/openmeter/ledger/advance/service/legacy_correction.go +++ b/openmeter/ledger/advance/service/legacy_correction.go @@ -167,6 +167,7 @@ func (s *service) resolveAdvanceBackfillBreakageReopenInputs(ctx context.Context if releaseFacts.SpendChargeID != nil && *releaseFacts.SpendChargeID != input.ChargeID { continue } + reopenInput, err := s.breakage.ReopenRelease(ctx, breakage.ReopenReleaseInput{ PostingInput: input.BreakagePosting, Release: release, diff --git a/openmeter/ledger/breakage/service.go b/openmeter/ledger/breakage/service.go index 3564af60cb..64527be4e9 100644 --- a/openmeter/ledger/breakage/service.go +++ b/openmeter/ledger/breakage/service.go @@ -205,6 +205,7 @@ func (i ReleasePlanInput) Validate() error { if i.SourceEntryID != nil && i.SourceTransactionID == nil { errs = append(errs, errors.New("source entry requires a source transaction")) } + for field, id := range map[string]*string{ "source transaction id": i.SourceTransactionID, "source entry id": i.SourceEntryID, @@ -212,6 +213,7 @@ func (i ReleasePlanInput) Validate() error { if id == nil { continue } + if *id == "" { errs = append(errs, fmt.Errorf("%s cannot be empty", field)) } else if err := ledger.ValidateAssignedID(*id); err != nil { @@ -659,6 +661,7 @@ func (s *service) recordPosting(ctx context.Context, input PostingInput, record if err != nil { return nil, fmt.Errorf("assign breakage posting IDs: %w", err) } + posting = transactions.WithAnnotations(posting, input.Annotations) record.BreakageTransactionGroupID = input.TransactionGroupID diff --git a/openmeter/ledger/breakage/service_test.go b/openmeter/ledger/breakage/service_test.go index 7449ffb9b1..8bafd04348 100644 --- a/openmeter/ledger/breakage/service_test.go +++ b/openmeter/ledger/breakage/service_test.go @@ -52,8 +52,10 @@ func TestBreakageBookkeepingPrecedesPosting(t *testing.T) { if err != nil { return err } + inputs = append(issue, inputs...) _, err = env.Deps.HistoricalLedger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs(env.Namespace, nil, inputs...), groupID)) + return err }) require.NoError(t, err) @@ -74,12 +76,15 @@ func TestBreakageReferencesDistinctPostingsWithIdenticalAccountingIdentity(t *te if err != nil { return err } + issue, err := resolveCreditIssuance(ctx, env) if err != nil { return err } + inputs = append(issue, inputs...) _, err = env.Deps.HistoricalLedger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs(env.Namespace, nil, inputs...), issueGroupID)) + return err }) require.NoError(t, err) @@ -117,6 +122,7 @@ func TestBreakageReferencesDistinctPostingsWithIdenticalAccountingIdentity(t *te } } } + require.Len(t, sourceEntries, 2) require.Equal(t, sourceEntries[0].IdentityKey(), sourceEntries[1].IdentityKey()) @@ -142,6 +148,7 @@ func TestBreakageReferencesDistinctPostingsWithIdenticalAccountingIdentity(t *te require.Equal(t, 6.0, plans[0].OpenAmount.InexactFloat64()) _, err = env.Deps.HistoricalLedger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs(env.Namespace, nil, inputs...), groupID)) + return err }) require.NoError(t, err) @@ -160,6 +167,7 @@ func TestBreakageReferencesDistinctPostingsWithIdenticalAccountingIdentity(t *te break } } + require.NotNil(t, matched) require.Equal(t, sourceTransactions[idx], *matched.SourceTransactionID) } @@ -215,6 +223,7 @@ func TestBreakagePostingFailureRollsBackBookkeeping(t *testing.T) { // when: the ledger rejects the posting. _, err = env.Deps.HistoricalLedger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs(env.Namespace, nil, inputs...), groupID)) + return err }) diff --git a/openmeter/ledger/chargeadapter/customcurrencyoverage.go b/openmeter/ledger/chargeadapter/customcurrencyoverage.go index 091c48f99c..1bae7ba32f 100644 --- a/openmeter/ledger/chargeadapter/customcurrencyoverage.go +++ b/openmeter/ledger/chargeadapter/customcurrencyoverage.go @@ -265,9 +265,11 @@ func (h *customCurrencyOverageHandler) correct(ctx context.Context, input correc if !slices.Contains(expectedTemplateCodes, templateCode) { return fmt.Errorf("unexpected transaction template %s in custom-currency overage group", templateCode) } + if _, seen := seenTemplates[templateCode]; seen { return fmt.Errorf("duplicate transaction template %s in custom-currency overage group", templateCode) } + seenTemplates[templateCode] = struct{}{} } diff --git a/openmeter/ledger/chargeadapter/flatfee_customcurrency_test.go b/openmeter/ledger/chargeadapter/flatfee_customcurrency_test.go index 9902418572..7d0f6ff8ec 100644 --- a/openmeter/ledger/chargeadapter/flatfee_customcurrency_test.go +++ b/openmeter/ledger/chargeadapter/flatfee_customcurrency_test.go @@ -274,6 +274,7 @@ func TestOnFlatFeeCustomCurrencyOverageAccruedCorrection_TransactionOrder(t *tes for idx, originalIdx := range tc.order { orderedInputs[idx] = inputs[originalIdx] } + group, err := env.Deps.HistoricalLedger.CommitGroup(t.Context(), transactions.GroupInputs(env.Namespace, nil, orderedInputs...)) require.NoError(t, err) group, err = env.Deps.HistoricalLedger.GetTransactionGroup(t.Context(), group.ID()) @@ -282,6 +283,7 @@ func TestOnFlatFeeCustomCurrencyOverageAccruedCorrection_TransactionOrder(t *tes for idx, transaction := range group.Transactions() { require.Equal(t, orderedInputs[idx].Annotations()[ledger.AnnotationTransactionTemplateCode], transaction.Annotations()[ledger.AnnotationTransactionTemplateCode]) } + run.AccruedUsage = &invoicedusage.AccruedUsage{ ServicePeriod: run.ServicePeriod, Totals: totals.Totals{Amount: alpacadecimal.NewFromInt(10), Total: alpacadecimal.NewFromInt(10)}, @@ -307,8 +309,10 @@ func TestOnFlatFeeCustomCurrencyOverageAccruedCorrection_TransactionOrder(t *tes }) require.NoError(t, err) require.Empty(t, corrections.Items) + return } + require.NoError(t, err) // then: every native and fiat leg is reversed regardless of enumeration order diff --git a/openmeter/ledger/collector/collect.go b/openmeter/ledger/collector/collect.go index a8968f6c2d..5f208ca7dc 100644 --- a/openmeter/ledger/collector/collect.go +++ b/openmeter/ledger/collector/collect.go @@ -126,6 +126,7 @@ func (c *accrualCollector) collectToReceivable(ctx context.Context, input Collec if err != nil { return nil, err } + if len(inputs) == 0 { return nil, nil } @@ -306,16 +307,19 @@ func (c *accrualCollector) resolveCollectionBreakageInputs(ctx context.Context, if err != nil { return nil, err } + inputs[idx] = identified for _, entry := range identified.EntryInputs() { if entry.PostingAddress().AccountType() != ledger.AccountTypeCustomerFBO || !entry.Amount().IsNegative() { continue } + originID := lo.FromPtr(entry.Provenance().CollectionOriginID) if _, exists := sourcesByOrigin[originID]; exists { return nil, fmt.Errorf("collection origin %s has multiple FBO source entries", originID) } + sourcesByOrigin[originID] = sourcePostingIDs{transactionID: identified.AssignedID(), entryID: entry.AssignedID()} } } diff --git a/openmeter/ledger/collector/collection_fbo_test.go b/openmeter/ledger/collector/collection_fbo_test.go index 3fb5acca7a..d58dcc1b7c 100644 --- a/openmeter/ledger/collector/collection_fbo_test.go +++ b/openmeter/ledger/collector/collection_fbo_test.go @@ -934,6 +934,7 @@ func bookExpiringCreditWithFeatures( inputs = append(inputs, breakageInputs...) _, err = env.Deps.HistoricalLedger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs(env.Namespace, nil, inputs...), groupID)) + return err }) require.NoError(t, err) @@ -949,7 +950,9 @@ func bookExpiringCreditWithFeatures( return plan.ID.ID } } + t.Fatal("issued credit has no breakage plan") + return "" } diff --git a/openmeter/ledger/customerbalance/expired_loader_test.go b/openmeter/ledger/customerbalance/expired_loader_test.go index e0d83b6cbe..3eaf4d32a8 100644 --- a/openmeter/ledger/customerbalance/expired_loader_test.go +++ b/openmeter/ledger/customerbalance/expired_loader_test.go @@ -660,6 +660,7 @@ func (e *testEnv) bookExpiredListingState(t *testing.T, issuedAt time.Time, spec Amount: releaseAmount, SourceKind: ledgerbreakage.SourceKindUsage, }) + return []ledger.TransactionInput{input}, err }) releases, err := e.BreakageService.ListReleases(t.Context(), ledgerbreakage.ListReleasesInput{ @@ -688,6 +689,7 @@ func (e *testEnv) bookExpiredListingState(t *testing.T, issuedAt time.Time, spec Amount: reopenAmount, SourceKind: ledgerbreakage.SourceKindUsageCorrection, }) + return []ledger.TransactionInput{input}, err }) } @@ -773,6 +775,7 @@ func (e *testEnv) commitBreakagePostings(t *testing.T, resolve func(context.Cont } _, err = e.Deps.HistoricalLedger.CommitGroup(ctx, ledger.WithGroupID(transactions.GroupInputs(e.Namespace, nil, inputs...), groupID)) + return err }) require.NoError(t, err) diff --git a/openmeter/ledger/historical/adapter/ledger.go b/openmeter/ledger/historical/adapter/ledger.go index b6b7d10348..0ec73eab28 100644 --- a/openmeter/ledger/historical/adapter/ledger.go +++ b/openmeter/ledger/historical/adapter/ledger.go @@ -109,6 +109,7 @@ func (r *repo) BookTransaction(ctx context.Context, groupID models.NamespacedID, if input == nil { return nil, ledger.ErrTransactionInputRequired } + if err := ledger.ValidateAssignedID(input.AssignedID()); err != nil { return nil, fmt.Errorf("transaction ID: %w", err) } @@ -122,6 +123,7 @@ func (r *repo) BookTransaction(ctx context.Context, groupID models.NamespacedID, if id := input.AssignedID(); id != "" { create.SetID(id) } + entity, err := create.Save(ctx) if err != nil { return nil, fmt.Errorf("failed to create ledger transaction: %w", err) @@ -138,9 +140,11 @@ func (r *repo) BookTransaction(ctx context.Context, groupID models.NamespacedID, if entryInput == nil { return nil, fmt.Errorf("entries[%d]: entry is required", idx) } + if err := ledger.ValidateAssignedID(entryInput.AssignedID()); err != nil { return nil, fmt.Errorf("entries[%d] ID: %w", idx, err) } + subAccountID := entryInput.PostingAddress().SubAccountID() route := entryInput.PostingAddress().Route() accountTypesBySubAccountID[subAccountID] = entryInput.PostingAddress().AccountType() @@ -163,6 +167,7 @@ func (r *repo) BookTransaction(ctx context.Context, groupID models.NamespacedID, if id := entryInput.AssignedID(); id != "" { create.SetID(id) } + createInputs = append(createInputs, create) } @@ -220,6 +225,7 @@ func (r *repo) CreateTransactionGroup(ctx context.Context, transactionGroup ledg if err := transactionGroup.Validate(); err != nil { return ledgerhistorical.TransactionGroupData{}, err } + return entutils.TransactingRepo(ctx, r, func(ctx context.Context, tx *repo) (ledgerhistorical.TransactionGroupData, error) { create := tx.db.LedgerTransactionGroup.Create(). SetNamespace(transactionGroup.Namespace). @@ -227,6 +233,7 @@ func (r *repo) CreateTransactionGroup(ctx context.Context, transactionGroup ledg if transactionGroup.ID != "" { create.SetID(transactionGroup.ID) } + entity, err := create.Save(ctx) if err != nil { return ledgerhistorical.TransactionGroupData{}, fmt.Errorf("failed to create transaction group: %w", err) diff --git a/openmeter/ledger/historical/ledger.go b/openmeter/ledger/historical/ledger.go index 7ca7154094..9ce1c12fd6 100644 --- a/openmeter/ledger/historical/ledger.go +++ b/openmeter/ledger/historical/ledger.go @@ -63,6 +63,7 @@ func (l *Ledger) CommitGroup(ctx context.Context, group ledger.TransactionGroupI if err := ledger.ValidateTransactionGroupInputWith(ctx, group, l.routingValidator); err != nil { return nil, fmt.Errorf("failed to validate transaction group: %w", err) } + txInputs := group.Transactions() return transaction.Run(ctx, l.repo, func(ctx context.Context) (*TransactionGroup, error) { diff --git a/openmeter/ledger/historical/repo.go b/openmeter/ledger/historical/repo.go index 7cd554cbd4..12ec287cf7 100644 --- a/openmeter/ledger/historical/repo.go +++ b/openmeter/ledger/historical/repo.go @@ -78,9 +78,11 @@ func (i CreateTransactionGroupInput) Validate() error { if i.Namespace == "" { errs = append(errs, errors.New("namespace is required")) } + if err := ledger.ValidateAssignedID(i.ID); err != nil { errs = append(errs, fmt.Errorf("ID: %w", err)) } + return models.NewNillableGenericValidationError(errors.Join(errs...)) } diff --git a/openmeter/ledger/ids.go b/openmeter/ledger/ids.go index 2698a23d0d..7eccf04f6d 100644 --- a/openmeter/ledger/ids.go +++ b/openmeter/ledger/ids.go @@ -34,6 +34,7 @@ func PreassignIDs(input TransactionInput) (TransactionInput, error) { if input == nil { return nil, ErrTransactionInputRequired } + if err := ValidateAssignedID(input.AssignedID()); err != nil { return nil, fmt.Errorf("transaction ID: %w", err) } @@ -44,9 +45,11 @@ func PreassignIDs(input TransactionInput) (TransactionInput, error) { if entry == nil { return nil, fmt.Errorf("entries[%d]: entry is required", idx) } + if err := ValidateAssignedID(entry.AssignedID()); err != nil { return nil, fmt.Errorf("entries[%d] ID: %w", idx, err) } + identifiedEntries[idx] = entry if entry.AssignedID() == "" { identifiedEntries[idx] = WithEntryID(entry, ulid.Make().String()) @@ -57,6 +60,7 @@ func PreassignIDs(input TransactionInput) (TransactionInput, error) { if id == "" { id = ulid.Make().String() } + return &identifiedTransactionInput{TransactionInput: input, id: id, entries: identifiedEntries}, nil } @@ -96,6 +100,7 @@ func (i *identifiedTransactionInput) AsGroupInput(namespace string, annotations // Reuse the underlying group metadata, but retain this wrapper as its // transaction so grouping cannot discard assigned IDs or entry overrides. group := i.TransactionInput.AsGroupInput(namespace, annotations) + return &identifiedGroupInput{ TransactionGroupInput: group, id: group.AssignedID(), diff --git a/openmeter/ledger/transactions/input_test.go b/openmeter/ledger/transactions/input_test.go index d49b4b8712..35a3548b28 100644 --- a/openmeter/ledger/transactions/input_test.go +++ b/openmeter/ledger/transactions/input_test.go @@ -102,6 +102,7 @@ func (e *transactionsTestEnv) requireCommittedIDs(t *testing.T, input ledger.Tra if groupID != "" { require.Equal(t, groupID, group.ID().ID) } + require.NotEmpty(t, group.ID().ID) require.NoError(t, ledger.ValidateAssignedID(group.ID().ID)) require.Equal(t, groupInput.Annotations(), group.Annotations()) @@ -111,6 +112,7 @@ func (e *transactionsTestEnv) requireCommittedIDs(t *testing.T, input ledger.Tra if input.AssignedID() != "" { require.Equal(t, input.AssignedID(), committed.ID().ID) } + require.NotEmpty(t, committed.ID().ID) require.NoError(t, ledger.ValidateAssignedID(committed.ID().ID)) require.Equal(t, input.Annotations(), committed.Annotations()) @@ -129,6 +131,7 @@ func (e *transactionsTestEnv) requireCommittedIDs(t *testing.T, input ledger.Tra if entryInput.AssignedID() != "" { require.Equal(t, entryInput.AssignedID(), matched.ID().ID) } + require.NotEmpty(t, matched.ID().ID) require.NoError(t, ledger.ValidateAssignedID(matched.ID().ID)) require.Equal(t, entryInput.Amount().InexactFloat64(), matched.Amount().InexactFloat64()) diff --git a/openmeter/ledger/validations.go b/openmeter/ledger/validations.go index d22f055c41..1fe516b3c3 100644 --- a/openmeter/ledger/validations.go +++ b/openmeter/ledger/validations.go @@ -194,9 +194,11 @@ func ValidateAssignedID(id string) error { if id == "" { return nil } + if _, err := ulid.ParseStrict(id); err != nil { return fmt.Errorf("invalid ledger ID: %w", err) } + return nil } @@ -212,6 +214,7 @@ func ValidateTransactionGroupInputWith(ctx context.Context, group TransactionGro if group.Namespace() == "" { errs = append(errs, errors.New("namespace is required")) } + if err := ValidateAssignedID(group.AssignedID()); err != nil { errs = append(errs, fmt.Errorf("group ID: %w", err)) } @@ -220,33 +223,41 @@ func ValidateTransactionGroupInputWith(ctx context.Context, group TransactionGro if len(inputs) == 0 { errs = append(errs, ErrTransactionGroupEmpty) } + transactionIDs := make(map[string]struct{}, len(inputs)) entryIDs := make(map[string]struct{}) for idx, input := range inputs { if err := ValidateTransactionInputWith(ctx, input, routingValidator); err != nil { errs = append(errs, fmt.Errorf("transactions[%d]: %w", idx, err)) } + if input == nil { continue } + if id := input.AssignedID(); id != "" { if _, exists := transactionIDs[id]; exists { errs = append(errs, fmt.Errorf("transactions[%d]: duplicate transaction ID %s", idx, id)) } + transactionIDs[id] = struct{}{} } + for entryIdx, entry := range input.EntryInputs() { if entry == nil { continue } + if id := entry.AssignedID(); id != "" { if _, exists := entryIDs[id]; exists { errs = append(errs, fmt.Errorf("transactions[%d].entries[%d]: duplicate entry ID %s", idx, entryIdx, id)) } + entryIDs[id] = struct{}{} } } } + return models.NewNillableGenericValidationError(errors.Join(errs...)) } diff --git a/tools/migrate/cmd/entschema/main.go b/tools/migrate/cmd/entschema/main.go index 8b47051374..086bfc080a 100644 --- a/tools/migrate/cmd/entschema/main.go +++ b/tools/migrate/cmd/entschema/main.go @@ -28,6 +28,7 @@ func main() { } _, err = fmt.Fprint(cmd.OutOrStdout(), ddl) + return err }, }