Skip to content

Commit 2468628

Browse files
refactor(customer): return map[string]*Customer from GetCustomersByUsageAttribution
Align the bulk resolver's semantics with the single-key lookup and the repo's featureresolver.BatchResolve convention: every input key is present in the returned map, with a nil value marking not-found (was: value map, absent = not-found). Callers can distinguish requested-but-missing from not-requested, and the map holds pointers instead of copied Customer structs.
1 parent 7fff147 commit 2468628

7 files changed

Lines changed: 50 additions & 34 deletions

File tree

openmeter/customer/service.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,6 @@ type CustomerService interface {
2626
DeleteCustomer(ctx context.Context, customer DeleteCustomerInput) error
2727
GetCustomer(ctx context.Context, customer GetCustomerInput) (*Customer, error)
2828
GetCustomerByUsageAttribution(ctx context.Context, input GetCustomerByUsageAttributionInput) (*Customer, error)
29-
GetCustomersByUsageAttribution(ctx context.Context, input GetCustomersByUsageAttributionInput) (map[string]Customer, error)
29+
GetCustomersByUsageAttribution(ctx context.Context, input GetCustomersByUsageAttributionInput) (map[string]*Customer, error)
3030
UpdateCustomer(ctx context.Context, params UpdateCustomerInput) (*Customer, error)
3131
}

openmeter/customer/service/customer.go

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -150,19 +150,20 @@ func (s *Service) GetCustomerByUsageAttribution(ctx context.Context, input custo
150150
return nil, err
151151
}
152152

153-
c, ok := resolved[input.Key]
154-
if !ok {
153+
c := resolved[input.Key]
154+
if c == nil {
155155
return nil, models.NewGenericNotFoundError(
156156
fmt.Errorf("customer with subject key %s not found in %s namespace", input.Key, input.Namespace),
157157
)
158158
}
159159

160-
return &c, nil
160+
return c, nil
161161
}
162162

163163
// GetCustomersByUsageAttribution resolves multiple customers by usage attribution keys in a single
164164
// query, mapping each input key to the customer it matches with key-over-subject precedence applied.
165-
func (s *Service) GetCustomersByUsageAttribution(ctx context.Context, input customer.GetCustomersByUsageAttributionInput) (map[string]customer.Customer, error) {
165+
// Every input key is present in the returned map; keys that match no customer map to a nil value.
166+
func (s *Service) GetCustomersByUsageAttribution(ctx context.Context, input customer.GetCustomersByUsageAttributionInput) (map[string]*customer.Customer, error) {
166167
if err := input.Validate(); err != nil {
167168
return nil, models.NewGenericValidationError(
168169
fmt.Errorf("error getting customers by usage attribution: %w", err),
@@ -175,7 +176,7 @@ func (s *Service) GetCustomersByUsageAttribution(ctx context.Context, input cust
175176
// resolveCustomersByUsageAttribution runs the shared usage-attribution lookup: fetch the raw
176177
// candidate customers from the adapter, then apply key-over-subject precedence. Both the single-key
177178
// and bulk service methods route through this so their core resolution stays identical.
178-
func (s *Service) resolveCustomersByUsageAttribution(ctx context.Context, input customer.GetCustomersByUsageAttributionInput) (map[string]customer.Customer, error) {
179+
func (s *Service) resolveCustomersByUsageAttribution(ctx context.Context, input customer.GetCustomersByUsageAttributionInput) (map[string]*customer.Customer, error) {
179180
customers, err := s.adapter.GetCustomersByUsageAttribution(ctx, input)
180181
if err != nil {
181182
return nil, err
@@ -188,13 +189,15 @@ func (s *Service) resolveCustomersByUsageAttribution(ctx context.Context, input
188189
// either by the customer's own key or by one of its subject keys; when a key matches both a
189190
// distinct key-owner and a distinct subject-owner, the key-owner takes precedence. Key-over-subject
190191
// collisions are structurally rare and resolved deterministically here, so they are not surfaced as
191-
// errors or logs; investigate the underlying data via the database if ever needed. Keys with no
192-
// match are absent from the returned map.
193-
func resolveCustomersByKeyWithPrecedence(customers []customer.Customer, keys []string) map[string]customer.Customer {
194-
byKey := make(map[string]customer.Customer, len(customers))
195-
bySubject := make(map[string]customer.Customer, len(customers))
192+
// errors or logs; investigate the underlying data via the database if ever needed. Every input key
193+
// is present in the returned map; keys that match no customer have a nil value.
194+
func resolveCustomersByKeyWithPrecedence(customers []customer.Customer, keys []string) map[string]*customer.Customer {
195+
byKey := make(map[string]*customer.Customer, len(customers))
196+
bySubject := make(map[string]*customer.Customer, len(customers))
197+
198+
for i := range customers {
199+
c := &customers[i]
196200

197-
for _, c := range customers {
198201
if c.Key != nil {
199202
byKey[*c.Key] = c
200203
}
@@ -208,7 +211,7 @@ func resolveCustomersByKeyWithPrecedence(customers []customer.Customer, keys []s
208211
}
209212
}
210213

211-
resolved := make(map[string]customer.Customer, len(keys))
214+
resolved := make(map[string]*customer.Customer, len(keys))
212215

213216
for _, k := range keys {
214217
if keyOwner, ok := byKey[k]; ok {
@@ -218,7 +221,10 @@ func resolveCustomersByKeyWithPrecedence(customers []customer.Customer, keys []s
218221

219222
if subjectOwner, ok := bySubject[k]; ok {
220223
resolved[k] = subjectOwner
224+
continue
221225
}
226+
227+
resolved[k] = nil // present, but no matching customer
222228
}
223229

224230
return resolved

openmeter/customer/service/customer_test.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,19 @@ func Test_resolveCustomersByKeyWithPrecedence(t *testing.T) {
2222
t.Run("MatchByKey", func(t *testing.T) {
2323
resolved := resolveCustomersByKeyWithPrecedence([]customer.Customer{customerA}, []string{"key-a"})
2424

25-
assert.Equal(t, map[string]customer.Customer{"key-a": customerA}, resolved)
25+
assert.Equal(t, map[string]*customer.Customer{"key-a": &customerA}, resolved)
2626
})
2727

2828
t.Run("MatchBySubject", func(t *testing.T) {
2929
resolved := resolveCustomersByKeyWithPrecedence([]customer.Customer{customerA}, []string{"subject-a"})
3030

31-
assert.Equal(t, map[string]customer.Customer{"subject-a": customerA}, resolved)
31+
assert.Equal(t, map[string]*customer.Customer{"subject-a": &customerA}, resolved)
3232
})
3333

34-
t.Run("UnmatchedKeyIsAbsent", func(t *testing.T) {
34+
t.Run("UnmatchedKeyIsNil", func(t *testing.T) {
3535
resolved := resolveCustomersByKeyWithPrecedence([]customer.Customer{customerA}, []string{"no-such-key"})
3636

37-
assert.Empty(t, resolved)
37+
assert.Equal(t, map[string]*customer.Customer{"no-such-key": nil}, resolved)
3838
})
3939

4040
t.Run("KeyOwnerTakesPrecedenceOverDistinctSubjectOwner", func(t *testing.T) {
@@ -55,7 +55,7 @@ func Test_resolveCustomersByKeyWithPrecedence(t *testing.T) {
5555
[]string{"shared"},
5656
)
5757

58-
assert.Equal(t, map[string]customer.Customer{"shared": sharedKeyCustomer}, resolved)
58+
assert.Equal(t, map[string]*customer.Customer{"shared": &sharedKeyCustomer}, resolved)
5959
})
6060

6161
t.Run("SameCustomerMatchedByOwnKeyAndSubjectKey", func(t *testing.T) {
@@ -69,7 +69,7 @@ func Test_resolveCustomersByKeyWithPrecedence(t *testing.T) {
6969

7070
resolved := resolveCustomersByKeyWithPrecedence([]customer.Customer{selfMatched}, []string{"dual"})
7171

72-
assert.Equal(t, map[string]customer.Customer{"dual": selfMatched}, resolved)
72+
assert.Equal(t, map[string]*customer.Customer{"dual": &selfMatched}, resolved)
7373
})
7474

7575
t.Run("CrossCollisionResolvesEachKeyToItsOwnKeyOwner", func(t *testing.T) {
@@ -96,9 +96,9 @@ func Test_resolveCustomersByKeyWithPrecedence(t *testing.T) {
9696
[]string{"key-1", "key-2"},
9797
)
9898

99-
assert.Equal(t, map[string]customer.Customer{
100-
"key-1": crossA,
101-
"key-2": crossB,
99+
assert.Equal(t, map[string]*customer.Customer{
100+
"key-1": &crossA,
101+
"key-2": &crossB,
102102
}, resolved)
103103
})
104104
}

openmeter/customer/service/customer_usage_attribution_benchmark_test.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,8 @@ func BenchmarkCustomersUsageAttributionBulkLookup(b *testing.B) {
125125

126126
keys := uabench.BulkKeys(customerCount)
127127

128+
// Count non-nil entries: every input key is present in the result map, with a nil value for
129+
// keys that resolved to no customer.
128130
get := func(ctx context.Context) (int, error) {
129131
resolved, err := svc.GetCustomersByUsageAttribution(ctx, customer.GetCustomersByUsageAttributionInput{
130132
Namespace: namespace,
@@ -133,7 +135,13 @@ func BenchmarkCustomersUsageAttributionBulkLookup(b *testing.B) {
133135
if err != nil {
134136
return 0, err
135137
}
136-
return len(resolved), nil
138+
matched := 0
139+
for _, c := range resolved {
140+
if c != nil {
141+
matched++
142+
}
143+
}
144+
return matched, nil
137145
}
138146

139147
count, err := get(b.Context())

openmeter/customer/service/service_test.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ func Test_CustomerService(t *testing.T) {
220220

221221
// Test_GetCustomersByUsageAttribution protects the bulk method's contract with the same rigor as
222222
// Test_GetCustomerByUsageAttribution does the single-key one: each input key maps to its customer
223-
// with key-over-subject precedence, and unmatched keys are simply absent (no error). The predicate
223+
// with key-over-subject precedence, and unmatched keys are present with a nil value (no error). The predicate
224224
// itself is covered by the adapter's TestGetCustomersByUsageAttribution, and the fully symmetric
225225
// {K1,K2} cross-collision by Test_resolveCustomersByKeyWithPrecedence — that state cannot be built
226226
// through the service here, because CreateCustomer rejects a customer whose key overlaps another
@@ -283,23 +283,25 @@ func Test_GetCustomersByUsageAttribution(t *testing.T) {
283283
assert.Equal(t, p.ID, got["shared"].ID, "a direct customer-key match must win over a subject-key match")
284284
})
285285

286-
t.Run("UnmatchedKeysAbsentNoError", func(t *testing.T) {
286+
t.Run("UnmatchedKeysNil", func(t *testing.T) {
287287
// given:
288288
// - a mix of a known key and an unknown one
289289
// then:
290-
// - the unknown key is simply absent from the map (unlike the single-key method, the bulk
291-
// method does NOT return a not-found error)
290+
// - every input key is present in the map; the unknown key has a nil value (unlike the
291+
// single-key method, the bulk method does NOT return a not-found error)
292292
known := create(t, "known-key")
293293

294294
got, err := env.CustomerService.GetCustomersByUsageAttribution(ctx, customer.GetCustomersByUsageAttributionInput{
295295
Namespace: namespace,
296296
Keys: []string{"known-key", "totally-unknown"},
297297
})
298298
require.NoError(t, err)
299-
require.Len(t, got, 1)
299+
require.Len(t, got, 2)
300+
require.NotNil(t, got["known-key"])
300301
assert.Equal(t, known.ID, got["known-key"].ID)
301-
_, ok := got["totally-unknown"]
302-
assert.False(t, ok, "unmatched key must be absent from the result map")
302+
unknown, ok := got["totally-unknown"]
303+
assert.True(t, ok, "every input key must be present in the result map")
304+
assert.Nil(t, unknown, "unmatched key must have a nil value")
303305
})
304306

305307
t.Run("EmptyKeysFailsValidation", func(t *testing.T) {

openmeter/governance/service/service.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -303,9 +303,9 @@ func (s *service) resolveCustomers(ctx context.Context, input governance.QueryAc
303303

304304
// Iterate the input keys in order so matched keys and not-found errors keep input ordering.
305305
for _, key := range input.CustomerKeys {
306-
cus, ok := keyToCustomer[key]
306+
cus := keyToCustomer[key]
307307

308-
if !ok {
308+
if cus == nil {
309309
queryErrors = append(queryErrors, governance.QueryError{
310310
CustomerKey: key,
311311
Code: governance.QueryErrorCustomerNotFound,
@@ -318,7 +318,7 @@ func (s *service) resolveCustomers(ctx context.Context, input governance.QueryAc
318318
rc.matched = append(rc.matched, key)
319319
} else {
320320
customerMap[cus.ID] = &resolvedCustomer{
321-
customer: cus,
321+
customer: *cus,
322322
matched: []string{key},
323323
}
324324
}

openmeter/server/server_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1458,7 +1458,7 @@ func (n NoopCustomerService) GetCustomerByUsageAttribution(ctx context.Context,
14581458
return &customer.Customer{}, nil
14591459
}
14601460

1461-
func (n NoopCustomerService) GetCustomersByUsageAttribution(ctx context.Context, input customer.GetCustomersByUsageAttributionInput) (map[string]customer.Customer, error) {
1461+
func (n NoopCustomerService) GetCustomersByUsageAttribution(ctx context.Context, input customer.GetCustomersByUsageAttributionInput) (map[string]*customer.Customer, error) {
14621462
return nil, nil
14631463
}
14641464

0 commit comments

Comments
 (0)