fix(config/db): share one account predicate and key the RI utilization cache by account

go.mod

@@ -59,7 +59,7 @@ require (
 	go.opentelemetry.io/otel/metric v1.45.0 // indirect
 	go.opentelemetry.io/otel/trace v1.45.0 // indirect
 	golang.org/x/crypto v0.58.0
-	golang.org/x/net v0.61.0 // indirect
+	golang.org/x/net v0.62.0 // indirect
 	golang.org/x/oauth2 v0.37.0
 	golang.org/x/sync v0.24.0
 	golang.org/x/sys v0.49.0 // indirect

go.sum

@@ -367,8 +367,8 @@ golang.org/x/crypto v0.58.0 h1:COkYLr4k7nDI4r6QtMML2AfzeIswWHAJthLS6K/M/54=
 golang.org/x/crypto v0.58.0/go.mod h1:Xh+kl5A+M0gMiWDytH2Hhr5Z7WbNCpv/eB/EFgfSk+w=
 golang.org/x/mod v0.42.0 h1:ICeL3t2L30ljSPHHxDnR4fqVcBt/zR2zJAb9juavmQg=
 golang.org/x/mod v0.42.0/go.mod h1:u5fzIrRInkL8MaO1cbRkUHT64v303YIT5KEqpNr7jRs=
-golang.org/x/net v0.61.0 h1:RR+6j/BTBCrggOLugxa+66R8FvoN6MHlzDlZIbN9YFw=
-golang.org/x/net v0.61.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI=
+golang.org/x/net v0.62.0 h1:18ELalfM5+XNRwzM2nYqXsZQ9ByS1uhj2O/U7+5jqRY=
+golang.org/x/net v0.62.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI=
 golang.org/x/oauth2 v0.37.0 h1:JUlcxA8oAtauLfiH8FX2/FkAWHAdi0QtGCGc+hofE98=
 golang.org/x/oauth2 v0.37.0/go.mod h1:IxwZNxUULJmpBFf9K/9NTMSIfZZuvuTy1gGxhigP/58=
 golang.org/x/sync v0.24.0 h1:iqiDrGoDgyuvAKrhLQthUEzU/R378N78gXbBjxed7oA=

internal/accountpredicate/accountpredicate.go

@@ -0,0 +1,100 @@
+// Package accountpredicate builds the dual-column account WHERE fragment
+// shared by every read path that scopes savings_snapshots or purchase_history
+// rows to a set of cloud accounts (issue #120). Keeping the one builder here
+// means a predicate fix is a single edit instead of three that can drift.
+//
+// Both tables carry two account identifiers, either of which may be the only
+// one populated on a row: cloud_account_id (the cloud_accounts UUID FK, NULL
+// on the AWS ambient and legacy rows) and account_id (the cloud-provider
+// external number). Matching only cloud_account_id silently dropped every NULL
+// row (issues #701/#498) and matching only account_id dropped UUID-only rows
+// (#866), so the predicate ORs both:
+//
+//	(cloud_account_id = ANY($u)
+//	   OR <provider gate for p1> AND account_id = ANY($e1)
+//	   OR ...)
+//
+// The external-id half is grouped per provider so each external number only
+// matches rows of its own provider, preserving the (provider, external_id)
+// pairing: a filter for aws/123 never pulls azure/123 rows. The "" provider key
+// (legacy raw external number, unknown provider) matches account_id with no
+// provider gate. cloud_account_id is compared directly (no ::text cast) so the
+// plain cloud_account_id UUID index (migration 000011) can back it. Providers
+// are sorted so the SQL and its bind order are deterministic.
+package accountpredicate
+
+import (
+	"fmt"
+	"sort"
+	"strings"
+)
+
+// ProviderMatch selects how the per-provider gate compares the stored
+// provider column with the filter's provider key.
+type ProviderMatch int
+
+const (
+	// ProviderExact gates on `provider = $p`. Used by the analytics reads of
+	// savings_snapshots and purchase_history.
+	ProviderExact ProviderMatch = iota
+
+	// ProviderCaseInsensitiveOrUnknown gates on
+	// `(provider = '' OR LOWER(provider) = LOWER($p))`, so a row stored as
+	// "AWS" still matches an "aws" filter and a row with an empty (unknown)
+	// stored provider matches on account_id alone, like the "" filter key
+	// does (issue #24). A row stored under a different, non-empty provider is
+	// still excluded. Used by the internal/config purchase_history reads.
+	ProviderCaseInsensitiveOrUnknown
+)
+
+// Build appends the account binds to args and returns the parenthesized OR
+// clause that references them by position, plus the extended args. When
+// neither input yields a disjunct (both empty, or every provider key maps to
+// no IDs) it returns an empty clause and args unchanged; each caller decides
+// what "no account filter" means for its query.
+func Build(args []any, accountUUIDs []string, externalIDsByProvider map[string][]string, match ProviderMatch) (clause string, outArgs []any) {
+	var ors []string
+	if len(accountUUIDs) > 0 {
+		args = append(args, accountUUIDs)
+		ors = append(ors, fmt.Sprintf("cloud_account_id = ANY($%d)", len(args)))
+	}
+	for _, provider := range sortedProviderKeys(externalIDsByProvider) {
+		exts := externalIDsByProvider[provider]
+		if len(exts) == 0 {
+			continue
+		}
+		if provider == "" {
+			args = append(args, exts)
+			ors = append(ors, fmt.Sprintf("account_id = ANY($%d)", len(args)))
+			continue
+		}
+		args = append(args, provider)
+		providerArg := len(args)
+		args = append(args, exts)
+		ors = append(ors, fmt.Sprintf(match.gateFormat(), providerArg, len(args)))
+	}
+	if len(ors) == 0 {
+		return "", args
+	}
+	return "(" + strings.Join(ors, " OR ") + ")", args
+}
+
+// gateFormat returns the per-provider disjunct with two %d verbs: the
+// provider bind position, then the external-ID array bind position.
+func (m ProviderMatch) gateFormat() string {
+	if m == ProviderCaseInsensitiveOrUnknown {
+		return "((provider = '' OR LOWER(provider) = LOWER($%d)) AND account_id = ANY($%d))"
+	}
+	return "(provider = $%d AND account_id = ANY($%d))"
+}
+
+// sortedProviderKeys returns the map keys in ascending order so the generated
+// SQL (and its bind-arg ordering) is deterministic across calls and testable.
+func sortedProviderKeys(m map[string][]string) []string {
+	keys := make([]string, 0, len(m))
+	for k := range m {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+	return keys
+}

internal/accountpredicate/accountpredicate_test.go

@@ -0,0 +1,82 @@
+package accountpredicate
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+// TestBuild_PinnedSQL pins the exact clause and bind args for both provider
+// gates. The three callers (internal/analytics, internal/api, internal/config)
+// produced these strings before the builder moved here (issue #120 item 5);
+// each caller also keeps its own pinning test.
+func TestBuild_PinnedSQL(t *testing.T) {
+	mixed := map[string][]string{"azure": {"sub-1"}, "": {"raw"}, "aws": {"123", "456"}, "gcp": nil}
+	tests := []struct {
+		name       string
+		base       []any
+		uuids      []string
+		ext        map[string][]string
+		match      ProviderMatch
+		wantClause string
+		wantArgs   []any
+	}{
+		{name: "no filter keeps args", base: []any{"s"}, match: ProviderExact, wantClause: "", wantArgs: []any{"s"}},
+		{name: "providers with no ids keep args", base: []any{"s"}, ext: map[string][]string{"aws": {}, "azure": nil}, match: ProviderExact, wantClause: "", wantArgs: []any{"s"}},
+		{name: "no filter, nil base", match: ProviderCaseInsensitiveOrUnknown, wantClause: "", wantArgs: nil},
+		{
+			name:       "uuids only",
+			base:       []any{"s", "e"},
+			uuids:      []string{"u1", "u2"},
+			match:      ProviderExact,
+			wantClause: "(cloud_account_id = ANY($3))",
+			wantArgs:   []any{"s", "e", []string{"u1", "u2"}},
+		},
+		{
+			name:  "exact gate, mixed providers sorted",
+			base:  []any{"s", "e"},
+			uuids: []string{"u1"},
+			ext:   mixed,
+			match: ProviderExact,
+			wantClause: "(cloud_account_id = ANY($3) OR account_id = ANY($4) OR " +
+				"(provider = $5 AND account_id = ANY($6)) OR (provider = $7 AND account_id = ANY($8)))",
+			wantArgs: []any{"s", "e", []string{"u1"}, []string{"raw"}, "aws", []string{"123", "456"}, "azure", []string{"sub-1"}},
+		},
+		{
+			name:  "case-insensitive gate, mixed providers sorted",
+			uuids: []string{"u1"},
+			ext:   mixed,
+			match: ProviderCaseInsensitiveOrUnknown,
+			wantClause: "(cloud_account_id = ANY($1) OR account_id = ANY($2) OR " +
+				"((provider = '' OR LOWER(provider) = LOWER($3)) AND account_id = ANY($4)) OR " +
+				"((provider = '' OR LOWER(provider) = LOWER($5)) AND account_id = ANY($6)))",
+			wantArgs: []any{[]string{"u1"}, []string{"raw"}, "aws", []string{"123", "456"}, "azure", []string{"sub-1"}},
+		},
+		{
+			name:       "provider key is bound as given",
+			ext:        map[string][]string{"AWS": {"123"}},
+			match:      ProviderCaseInsensitiveOrUnknown,
+			wantClause: "(((provider = '' OR LOWER(provider) = LOWER($1)) AND account_id = ANY($2)))",
+			wantArgs:   []any{"AWS", []string{"123"}},
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			clause, args := Build(tc.base, tc.uuids, tc.ext, tc.match)
+			assert.Equal(t, tc.wantClause, clause)
+			assert.Equal(t, tc.wantArgs, args)
+		})
+	}
+}
+
+// TestBuild_Deterministic guards the sorted provider order: Go map iteration
+// is randomized, so an unsorted builder would fail this within a few runs.
+func TestBuild_Deterministic(t *testing.T) {
+	ext := map[string][]string{"gcp": {"p"}, "azure": {"s"}, "aws": {"a"}, "oci": {"o"}}
+	first, firstArgs := Build(nil, nil, ext, ProviderExact)
+	for range 50 {
+		clause, args := Build(nil, nil, ext, ProviderExact)
+		assert.Equal(t, first, clause)
+		assert.Equal(t, firstArgs, args)
+	}
+}

internal/analytics/postgres_analytics.go

@@ -5,10 +5,9 @@ import (
 	"database/sql"
 	"encoding/json"
 	"fmt"
-	"sort"
-	"strings"
 	"time"
 
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/accountpredicate"
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/database"
 	"github.com/google/uuid"
 	"github.com/jackc/pgx/v5"
@@ -42,51 +41,16 @@ var _ AnalyticsStore = (*PostgresAnalyticsStore)(nil)
 // leading binds; the account array binds (if any) are appended after them and
 // the returned clause references them by the right positions.
 //
-// savings_snapshots carries two account identifiers, either of which may be the
-// only one populated on a row: account_id (the cloud-provider external number)
-// and cloud_account_id (the cloud_accounts UUID FK, NULL on the AWS ambient and
-// legacy rows). Matching only one column silently drops rows that carry only the
-// other, so we OR both, with the external-id half grouped per provider so a
-// reused external number across providers cannot leak the wrong rows. This
-// mirrors api.accountFilterClause on the live purchase_history path
-// (issue #701/#498/#866). Both empty -> "TRUE" (caller must enforce scope).
+// The predicate itself lives in internal/accountpredicate, shared with the
+// purchase_history read paths in internal/api and internal/config (issue
+// #120), so a fix to it is one edit. Both empty -> "TRUE" (caller must enforce
+// scope).
 func accountFilterClause(accountUUIDs []string, accountExternalIDsByProvider map[string][]string, baseArgs []any) (clause string, args []any) {
-	args = baseArgs
-	var ors []string
-	if len(accountUUIDs) > 0 {
-		args = append(args, accountUUIDs)
-		ors = append(ors, fmt.Sprintf("cloud_account_id = ANY($%d)", len(args)))
-	}
-	for _, provider := range sortedProviderKeys(accountExternalIDsByProvider) {
-		exts := accountExternalIDsByProvider[provider]
-		if len(exts) == 0 {
-			continue
-		}
-		if provider == "" {
-			args = append(args, exts)
-			ors = append(ors, fmt.Sprintf("account_id = ANY($%d)", len(args)))
-			continue
-		}
-		args = append(args, provider)
-		providerArg := len(args)
-		args = append(args, exts)
-		ors = append(ors, fmt.Sprintf("(provider = $%d AND account_id = ANY($%d))", providerArg, len(args)))
-	}
-	if len(ors) == 0 {
+	clause, args = accountpredicate.Build(baseArgs, accountUUIDs, accountExternalIDsByProvider, accountpredicate.ProviderExact)
+	if clause == "" {
 		return "TRUE", args
 	}
-	return "(" + strings.Join(ors, " OR ") + ")", args
-}
-
-// sortedProviderKeys returns the map keys in ascending order so generated SQL
-// (and its bind-arg ordering) is deterministic and testable.
-func sortedProviderKeys(m map[string][]string) []string {
-	keys := make([]string, 0, len(m))
-	for k := range m {
-		keys = append(keys, k)
-	}
-	sort.Strings(keys)
-	return keys
+	return clause, args
 }
 
 // ==========================================

internal/analytics/postgres_analytics_mock_test.go

@@ -66,6 +66,26 @@ func TestAccountFilterClause(t *testing.T) {
 		assert.Equal(t, "(account_id = ANY($3))", clause)
 		assert.Len(t, args, 3)
 	})
+
+	// Pins the exact bind args too (issue #120 item 5), so moving the builder
+	// into the shared package cannot reorder or drop a bind.
+	t.Run("bind args are pinned in clause order", func(t *testing.T) {
+		clause, args := accountFilterClause(
+			[]string{"u1"},
+			map[string][]string{"azure": {"sub-1"}, "": {"raw"}, "aws": {"123"}, "gcp": nil},
+			base(),
+		)
+		assert.Equal(t,
+			"(cloud_account_id = ANY($3) OR account_id = ANY($4) OR (provider = $5 AND account_id = ANY($6)) OR (provider = $7 AND account_id = ANY($8)))",
+			clause)
+		assert.Equal(t, []any{"start", "end", []string{"u1"}, []string{"raw"}, "aws", []string{"123"}, "azure", []string{"sub-1"}}, args)
+	})
+
+	t.Run("providers with no ids yield TRUE", func(t *testing.T) {
+		clause, args := accountFilterClause(nil, map[string][]string{"aws": {}}, base())
+		assert.Equal(t, "TRUE", clause)
+		assert.Equal(t, []any{"start", "end"}, args)
+	})
 }
 
 // TestPartitionDateCalculation verifies partition date calculation logic.

internal/api/analytics_postgres.go

@@ -5,10 +5,9 @@ import (
 	"context"
 	"database/sql"
 	"fmt"
-	"sort"
-	"strings"
 	"time"
 
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/accountpredicate"
 	"github.com/LeanerCloud/cloud-commitments-platform/internal/database"
 	"github.com/jackc/pgx/v5"
 	"github.com/jackc/pgx/v5/pgconn"
@@ -83,62 +82,18 @@ func dimensionToColumn(dimension string) (string, error) {
 // are appended after them and the returned clause references them by the right
 // positions.
 //
-// purchase_history carries two account identifiers, either of which may be the
-// only one populated on a row: account_id (the cloud-provider external number,
-// always populated) and cloud_account_id (the cloud_accounts UUID FK, NULL on
-// every direct-execute/ambient/pre-000011 row). The top-bar chip emits the
-// UUID, so matching only cloud_account_id silently dropped every NULL row
-// (issue #701/#498) and matching only account_id dropped UUID-only rows (#866).
-//
-// The external-id half is grouped per provider so each external number only
-// matches rows of its own provider:
-//
-//	(cloud_account_id = ANY($u)
-//	   OR (provider = $p AND account_id = ANY($extsForP)) OR ...)
-//
-// preserving the (provider, external_id) pairing so a filter for aws/123 never
-// pulls azure/123 rows. The "" provider key (legacy raw external number, unknown
-// provider) matches account_id with no provider gate. cloud_account_id is
-// compared directly (no ::text cast) so the plain cloud_account_id UUID index
-// (migration 000011) can back it. Providers are sorted for deterministic SQL.
-// Both empty -> "TRUE" (no account filter).
+// purchase_history carries two account identifiers (account_id, always
+// populated, and cloud_account_id, NULL on every direct-execute/ambient/
+// pre-000011 row), so the predicate ORs both with a per-provider gate on the
+// external-id half. It lives in internal/accountpredicate, shared with the
+// savings_snapshots reads in internal/analytics and the purchase_history reads
+// in internal/config (issue #120). Both empty -> "TRUE" (no account filter).
 func accountFilterClause(accountUUIDs []string, accountExternalIDsByProvider map[string][]string, baseArgs []any) (clause string, args []any) {
-	args = baseArgs
-	var ors []string
-	if len(accountUUIDs) > 0 {
-		args = append(args, accountUUIDs)
-		ors = append(ors, fmt.Sprintf("cloud_account_id = ANY($%d)", len(args)))
-	}
-	for _, provider := range sortedProviderKeys(accountExternalIDsByProvider) {
-		exts := accountExternalIDsByProvider[provider]
-		if len(exts) == 0 {
-			continue
-		}
-		if provider == "" {
-			args = append(args, exts)
-			ors = append(ors, fmt.Sprintf("account_id = ANY($%d)", len(args)))
-			continue
-		}
-		args = append(args, provider)
-		providerArg := len(args)
-		args = append(args, exts)
-		ors = append(ors, fmt.Sprintf("(provider = $%d AND account_id = ANY($%d))", providerArg, len(args)))
-	}
-	if len(ors) == 0 {
+	clause, args = accountpredicate.Build(baseArgs, accountUUIDs, accountExternalIDsByProvider, accountpredicate.ProviderExact)
+	if clause == "" {
 		return "TRUE", args
 	}
-	return "(" + strings.Join(ors, " OR ") + ")", args
-}
-
-// sortedProviderKeys returns the map keys in ascending order so the generated
-// SQL (and its bind-arg ordering) is deterministic across calls and testable.
-func sortedProviderKeys(m map[string][]string) []string {
-	keys := make([]string, 0, len(m))
-	for k := range m {
-		keys = append(keys, k)
-	}
-	sort.Strings(keys)
-	return keys
+	return clause, args
 }
 
 // QueryHistory aggregates purchase_history rows bucketed by interval.