Skip to content

Commit be5d6ba

Browse files
authored
feat: update gRPC service layer for instrument-based quantities (#1217)
* feat: update gRPC service layer for instrument-based quantities - Map dimension field in toProtoFacility response (grpc_mappers.go) - Remove dead mapCurrency function and unused currency constants - Resolve dimension from Reference Data service in InitiateCurrentAccount - Use NewCurrentAccountWithDimension to persist resolved dimension - Fall back to CURRENCY dimension when Reference Data unavailable - Add InstrumentGetter interface and WithInstrumentGetter service option - Wire Reference Data client as instrument getter in main.go - Remove dead overdraft fields from saveHandler (grpc_control_endpoints.go) - Remove TestCurrencyMapping test (mapCurrency no longer exists) * fix: map registry MONETARY dimension to domain CURRENCY constant The reference-data registry uses "MONETARY" for currency instruments while the domain quantity package uses "CURRENCY". Without this mapping, NewCurrentAccountWithDimension always fails when instrumentGetter is configured. Add mapRegistryDimension helper and test. * fix: address CodeRabbit review comments on instrument error handling and metrics - Distinguish registry.ErrNotFound from transient errors in GetInstrument: unknown instrument_code returns codes.InvalidArgument, transient failures return codes.Unavailable with external service error metric recorded - Rename balanceGauge label from "currency" to "instrument_code" to reflect the broader instrument model (kWh, GPU_HOUR, GBP, etc.) - Update all three RecordBalance callers to pass account.InstrumentCode() instead of currency string for consistent label values --------- Co-authored-by: Ben Coombs <bjcoombs@users.noreply.github.com>
1 parent 6651878 commit be5d6ba

9 files changed

Lines changed: 86 additions & 74 deletions

File tree

services/current-account/client/starlark.go

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,6 @@ func terminateLienHandler(client *Client) saga.Handler {
262262
//
263263
// Parameters:
264264
// - account_id (string): The account identifier to update
265-
// - overdraft_limit (decimal): New overdraft limit (optional)
266-
// - overdraft_enabled (bool): Enable/disable overdraft (optional)
267265
//
268266
// Returns a map containing:
269267
// - account_id: The account identifier
@@ -275,30 +273,10 @@ func saveHandler(client *Client) saga.Handler {
275273
return nil, err
276274
}
277275

278-
// Build request - all fields are optional except account_id
279276
req := &currentaccountv1.UpdateCurrentAccountRequest{
280277
AccountId: accountID,
281278
}
282279

283-
// Optional overdraft_limit
284-
if overdraftLimitVal, ok := params["overdraft_limit"]; ok {
285-
if overdraftLimit, err := saga.RequireDecimalParam(map[string]any{"overdraft_limit": overdraftLimitVal}, "overdraft_limit"); err == nil && !overdraftLimit.IsZero() {
286-
// Get currency from params - it should be provided when overdraft_limit is set
287-
currency := ""
288-
if currencyVal, ok := params["currency"].(string); ok {
289-
currency = currencyVal
290-
}
291-
req.OverdraftLimit = &commonv1.MoneyAmount{
292-
Amount: convertDecimalToMoney(overdraftLimit, currency),
293-
}
294-
}
295-
}
296-
297-
// Optional overdraft_enabled
298-
if overdraftEnabled, ok := params["overdraft_enabled"].(bool); ok {
299-
req.OverdraftEnabled = &overdraftEnabled
300-
}
301-
302280
clientCtx := prepareClientContext(ctx)
303281

304282
resp, err := client.UpdateCurrentAccount(clientCtx, req)

services/current-account/cmd/main.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -570,6 +570,12 @@ func createServiceWithClients(
570570
return nil, nil, fmt.Errorf("failed to create service with existing clients: %w", err)
571571
}
572572

573+
// Apply optional service features that use the functional options pattern.
574+
if refDataClient != nil {
575+
svc.ApplyOptions(service.WithInstrumentGetter(refDataClient))
576+
logger.Info("instrument dimension resolution enabled via Reference Data service")
577+
}
578+
573579
// Create Starlark handler registry with service client handlers.
574580
// This enables saga scripts to call real services (not mocks).
575581
// See PRD: docs/prd/starlark-service-bindings.md

services/current-account/observability/metrics.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ var (
4141
Name: "current_account_balance_cents",
4242
Help: "Current account balance in cents",
4343
},
44-
[]string{"currency"},
44+
[]string{"instrument_code"},
4545
)
4646

4747
// Saga metrics
@@ -214,8 +214,8 @@ func RecordWithdrawal(currency string) {
214214
}
215215

216216
// RecordBalance records the current account balance
217-
func RecordBalance(balanceCents int64, currency string) {
218-
balanceGauge.WithLabelValues(currency).Set(float64(balanceCents))
217+
func RecordBalance(balanceCents int64, instrumentCode string) {
218+
balanceGauge.WithLabelValues(instrumentCode).Set(float64(balanceCents))
219219
}
220220

221221
// RecordSagaFailure records a saga failure

services/current-account/service/grpc_account_endpoints.go

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
caobservability "github.com/meridianhub/meridian/services/current-account/observability"
1616
"github.com/meridianhub/meridian/services/reference-data/accounttype"
1717
celutil "github.com/meridianhub/meridian/services/reference-data/cel"
18+
"github.com/meridianhub/meridian/services/reference-data/registry"
1819
vf "github.com/meridianhub/meridian/shared/pkg/valuationfeature"
1920
"github.com/meridianhub/meridian/shared/platform/tenant"
2021
"google.golang.org/grpc/codes"
@@ -37,13 +38,43 @@ func (s *Service) InitiateCurrentAccount(ctx context.Context, req *pb.InitiateCu
3738
// Generate account ID
3839
accountID := fmt.Sprintf("ACC-%s", uuid.New().String()[:8])
3940

40-
// Use instrument_code directly as the account's native instrument
41-
currency := req.InstrumentCode
42-
if currency == "" {
41+
// Validate instrument_code is provided
42+
instrumentCode := req.InstrumentCode
43+
if instrumentCode == "" {
4344
operationStatus = operationStatusInvalidCurrency
4445
return nil, status.Errorf(codes.InvalidArgument, "instrument_code is required")
4546
}
4647

48+
// Resolve dimension from Reference Data service when available.
49+
// Dimension classifies the instrument type (e.g. "CURRENCY", "ENERGY", "COMPUTE").
50+
// Falls back to CURRENCY for backward compatibility when the getter is not configured.
51+
dimension := "CURRENCY"
52+
if s.instrumentGetter != nil {
53+
cachedInstrument, err := s.instrumentGetter.GetInstrument(ctx, instrumentCode, 0)
54+
if err != nil {
55+
if errors.Is(err, registry.ErrNotFound) {
56+
// Instrument does not exist in Reference Data - caller supplied an invalid instrument_code
57+
operationStatus = "instrument_not_found"
58+
s.logger.Warn("instrument not found in Reference Data, cannot create account",
59+
"instrument_code", instrumentCode,
60+
"account_id", accountID)
61+
return nil, status.Errorf(codes.InvalidArgument, "unknown instrument_code: %s", instrumentCode)
62+
}
63+
// Transient failure (network error, service unavailable, timeout)
64+
operationStatus = "instrument_lookup_failed"
65+
s.logger.Error("instrument lookup failed due to transient error, cannot create account",
66+
"instrument_code", instrumentCode,
67+
"account_id", accountID,
68+
"error", err)
69+
caobservability.RecordExternalServiceError("reference_data", "get_instrument")
70+
return nil, status.Errorf(codes.Unavailable, "instrument lookup failed, please retry")
71+
}
72+
// Map from reference-data registry dimension ("MONETARY") to domain quantity
73+
// dimension ("CURRENCY"). The registry uses "MONETARY" while the domain quantity
74+
// package uses "CURRENCY" - other dimensions are identical across both packages.
75+
dimension = mapRegistryDimension(string(cachedInstrument.Definition.Dimension))
76+
}
77+
4778
// Validate party exists and is active (if party client is configured)
4879
if s.partyClient != nil {
4980
partyValidationStart := time.Now()
@@ -185,12 +216,13 @@ func (s *Service) InitiateCurrentAccount(ctx context.Context, req *pb.InitiateCu
185216
"account_id", accountID)
186217
}
187218

188-
// Create domain model
189-
account, err := domain.NewCurrentAccount(
219+
// Create domain model with resolved instrument and dimension
220+
account, err := domain.NewCurrentAccountWithDimension(
190221
accountID,
191222
req.ExternalIdentifier,
192223
req.PartyId,
193-
currency,
224+
instrumentCode,
225+
dimension,
194226
opts...,
195227
)
196228
if err != nil {
@@ -216,7 +248,7 @@ func (s *Service) InitiateCurrentAccount(ctx context.Context, req *pb.InitiateCu
216248
}
217249

218250
// Record initial balance
219-
caobservability.RecordBalance(safeMinorUnits(account.Balance()), currency)
251+
caobservability.RecordBalance(safeMinorUnits(account.Balance()), instrumentCode)
220252

221253
// Convert to proto response
222254
return &pb.InitiateCurrentAccountResponse{

services/current-account/service/grpc_deposit_endpoints.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ func (s *Service) ExecuteDeposit(ctx context.Context, req *pb.ExecuteDepositRequ
202202
resp.NewBalance = toMoneyAmount(account.Balance())
203203
resp.AvailableBalance = toMoneyAmount(account.AvailableBalance())
204204
// Record balance gauge only when we have accurate post-transaction balance
205-
caobservability.RecordBalance(safeMinorUnits(account.Balance()), string(account.Balance().Currency()))
205+
caobservability.RecordBalance(safeMinorUnits(account.Balance()), account.InstrumentCode())
206206
}
207207

208208
// Store successful result in Redis for future idempotency checks

services/current-account/service/grpc_mappers.go

Lines changed: 13 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,13 @@ import (
1010
"google.golang.org/protobuf/types/known/timestamppb"
1111
)
1212

13-
// Currency code constants
14-
const (
15-
currencyGBP = "GBP"
16-
currencyUSD = "USD"
17-
currencyEUR = "EUR"
18-
)
19-
2013
func toProtoFacility(account domain.CurrentAccount) *pb.CurrentAccountFacility {
2114
return &pb.CurrentAccountFacility{
2215
AccountId: account.AccountID(),
2316
ExternalIdentifier: account.ExternalIdentifier(),
2417
AccountStatus: mapStatusToProto(account.Status()),
2518
InstrumentCode: account.InstrumentCode(),
19+
Dimension: account.Dimension(),
2620
CreatedAt: timestamppb.New(account.CreatedAt()),
2721
UpdatedAt: timestamppb.New(account.UpdatedAt()),
2822
// #nosec G115 - Version is bounded by database constraints
@@ -107,6 +101,18 @@ func mapWithdrawalStatusToProto(status domain.WithdrawalStatus) pb.WithdrawalSta
107101
}
108102
}
109103

104+
// mapRegistryDimension converts a reference-data registry dimension string to the
105+
// domain quantity dimension string used by the current-account service.
106+
//
107+
// The reference-data registry uses "MONETARY" for currency instruments, while the
108+
// domain quantity package uses "CURRENCY". All other dimension values are identical.
109+
func mapRegistryDimension(registryDimension string) string {
110+
if registryDimension == "MONETARY" {
111+
return "CURRENCY"
112+
}
113+
return registryDimension
114+
}
115+
110116
func mapStatusToProto(status domain.AccountStatus) pb.AccountStatus {
111117
switch status {
112118
case domain.AccountStatusActive:
@@ -119,24 +125,3 @@ func mapStatusToProto(status domain.AccountStatus) pb.AccountStatus {
119125
return pb.AccountStatus_ACCOUNT_STATUS_UNSPECIFIED
120126
}
121127
}
122-
123-
func mapCurrency(currency commonpb.Currency) string {
124-
switch currency {
125-
case commonpb.Currency_CURRENCY_GBP:
126-
return currencyGBP
127-
case commonpb.Currency_CURRENCY_USD:
128-
return currencyUSD
129-
case commonpb.Currency_CURRENCY_EUR:
130-
return currencyEUR
131-
case commonpb.Currency_CURRENCY_UNSPECIFIED,
132-
commonpb.Currency_CURRENCY_JPY,
133-
commonpb.Currency_CURRENCY_CHF,
134-
commonpb.Currency_CURRENCY_CAD,
135-
commonpb.Currency_CURRENCY_AUD:
136-
// Return empty string for unsupported currencies
137-
// Caller should validate and return error
138-
return ""
139-
default:
140-
return ""
141-
}
142-
}

services/current-account/service/grpc_service.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,15 @@ type AccountTypeCache interface {
143143
// Option is a functional option for configuring optional Service dependencies.
144144
type Option func(*Service)
145145

146+
// WithInstrumentGetter sets the instrument getter for dimension resolution from Reference Data.
147+
// When set, the service resolves the account dimension from the instrument_code during account creation.
148+
// When nil, dimension defaults to CURRENCY (backward-compatible behavior).
149+
func WithInstrumentGetter(getter InstrumentGetter) Option {
150+
return func(s *Service) {
151+
s.instrumentGetter = getter
152+
}
153+
}
154+
146155
// WithAccountTypeCache sets the account type cache for product type resolution.
147156
func WithAccountTypeCache(cache AccountTypeCache) Option {
148157
return func(s *Service) {
@@ -198,6 +207,7 @@ type Service struct {
198207
posKeepingClient PositionKeepingClient
199208
finAcctClient FinancialAccountingClient
200209
partyClient PartyClient
210+
instrumentGetter InstrumentGetter // Optional: resolves dimension from instrument_code via Reference Data
201211
accountTypeCache AccountTypeCache // Optional: resolves product type definitions
202212
valuationEngine ValuationEngine // Optional: executes valuation method logic
203213
accountConfig *config.AccountConfig

services/current-account/service/grpc_service_test.go

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -583,24 +583,25 @@ func TestRetrieveCurrentAccountNotFound(t *testing.T) {
583583
}
584584
}
585585

586-
func TestCurrencyMapping(t *testing.T) {
586+
func TestMapRegistryDimension(t *testing.T) {
587587
tests := []struct {
588-
name string
589-
currency commonpb.Currency
588+
input string
590589
expected string
591590
}{
592-
{"GBP", commonpb.Currency_CURRENCY_GBP, "GBP"},
593-
{"USD", commonpb.Currency_CURRENCY_USD, "USD"},
594-
{"EUR", commonpb.Currency_CURRENCY_EUR, "EUR"},
595-
{"Unspecified returns empty", commonpb.Currency_CURRENCY_UNSPECIFIED, ""},
596-
{"Unsupported JPY returns empty", commonpb.Currency_CURRENCY_JPY, ""},
591+
{"MONETARY", "CURRENCY"},
592+
{"CURRENCY", "CURRENCY"},
593+
{"ENERGY", "ENERGY"},
594+
{"COMPUTE", "COMPUTE"},
595+
{"MASS", "MASS"},
596+
{"VOLUME", "VOLUME"},
597+
{"TIME", "TIME"},
598+
{"", ""},
597599
}
598-
599600
for _, tt := range tests {
600-
t.Run(tt.name, func(t *testing.T) {
601-
result := mapCurrency(tt.currency)
601+
t.Run(tt.input, func(t *testing.T) {
602+
result := mapRegistryDimension(tt.input)
602603
if result != tt.expected {
603-
t.Errorf("Expected %s, got %s", tt.expected, result)
604+
t.Errorf("mapRegistryDimension(%q) = %q, want %q", tt.input, result, tt.expected)
604605
}
605606
})
606607
}

services/current-account/service/grpc_withdrawal_execute.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ func (s *Service) ExecuteWithdrawal(ctx context.Context, req *pb.ExecuteWithdraw
301301
resp.NewBalance = toMoneyAmount(account.Balance())
302302
resp.AvailableBalance = toMoneyAmount(account.AvailableBalance())
303303
// Record balance gauge only when we have accurate post-transaction balance
304-
caobservability.RecordBalance(safeMinorUnits(account.Balance()), string(account.Balance().Currency()))
304+
caobservability.RecordBalance(safeMinorUnits(account.Balance()), account.InstrumentCode())
305305
}
306306

307307
// Mark pending withdrawal as completed (if executing a pending withdrawal)

0 commit comments

Comments
 (0)