Skip to content

Commit 9633a0c

Browse files
committed
fix: repair final 6 integration failures + advisory-lock bug
Five test-side fixes exposed by the now-complete schema / first CI run of the suite: - EmailVerification: verification tokens are hashed at rest (pkg/tokenhash), so assert the hash, not the raw token (same as SessionLifecycle). - OutboxStore: jsonb round-trips through Postgres's canonical form (space after colon); compare with JSONEq instead of byte equality. - RLSAudit_SentinelPolicyShape: pg_policies renders parsed expressions with explicit ::text casts; match that normalized shape in the sentinel fragment. - TxManager_Reentrant: the inner write used a raw pool.Exec, which grabs its own connection and commits independently — route it through DBFromContext so it joins the (re-entrant) transaction and rolls back with the outer. - RLS_TenantIsolation: newTenantUser built emails from a truncated UUIDv7 prefix; UUIDv7's leading bits are the ms timestamp, so users minted in the same tick collided on the unique email. Use the full UUID. One production fix: - PartitionManager advisory lock ran pg_try_advisory_lock via pool.QueryRow, so the session lock landed on an arbitrary pooled connection that was handed straight back, and pg_advisory_unlock ran on a possibly-different connection (silent no-op → leaked lock, and re-entrant TRUE for a second caller sharing the pool). Hold a dedicated pgxpool.Conn for the lock's lifetime, guarded by a mutex, and unlock+release on that same connection.
1 parent 6958ddf commit 9633a0c

6 files changed

Lines changed: 67 additions & 15 deletions

File tree

internal/adapter/postgres/identity_repo_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,8 @@ func TestIdentityRepo_EmailVerification(t *testing.T) {
217217
assert.Equal(t, verification.ID, got.ID)
218218
assert.Equal(t, verification.UserID, got.UserID)
219219
assert.Equal(t, verification.Email, got.Email)
220-
assert.Equal(t, verification.Token, got.Token)
220+
// Verification tokens are hashed at rest (pkg/tokenhash), like refresh tokens.
221+
assert.Equal(t, tokenhash.Hash(verification.Token), got.Token)
221222

222223
// Not found
223224
_, err = repo.GetEmailVerification(ctx, "nonexistent-token")

internal/adapter/postgres/outbox_repo_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@ func TestOutboxStore(t *testing.T) {
7272
require.NoError(t, err)
7373
require.Len(t, events, 1)
7474
assert.Equal(t, "subscription.activated", events[0].EventType)
75-
assert.Equal(t, payload, events[0].Payload)
75+
// jsonb round-trips through Postgres's canonical form (a space after the
76+
// colon), so compare semantically rather than byte-for-byte.
77+
assert.JSONEq(t, string(payload), string(events[0].Payload))
7678
assert.NotEmpty(t, events[0].ID)
7779
assert.False(t, events[0].CreatedAt.IsZero())
7880
}

internal/adapter/postgres/partition_manager.go

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,15 @@ type PartitionManager struct {
8484
logger *slog.Logger
8585
lookahead int // quarters ahead to ensure
8686
retention time.Duration // 0 = no cleanup
87+
88+
// lockMu guards lockConn. A session-level advisory lock lives on the
89+
// connection that acquired it, so the lock must be held on a dedicated
90+
// connection for its whole lifetime — pool.QueryRow would run the lock on
91+
// an arbitrary pooled connection and hand it straight back, leaving the
92+
// lock stranded on a random connection and pg_advisory_unlock running on a
93+
// different one (a silent no-op → leaked lock).
94+
lockMu sync.Mutex
95+
lockConn *pgxpool.Conn
8796
}
8897

8998
// NewPartitionManager creates a PartitionManager that pre-creates future
@@ -130,26 +139,57 @@ func (pm *PartitionManager) Run(ctx context.Context) {
130139
}
131140
}
132141

133-
// tryAdvisoryLock attempts to acquire a session-level advisory lock.
134-
// Returns true if the lock was acquired, false if another pod holds it
135-
// or the query fails.
142+
// tryAdvisoryLock attempts to acquire a session-level advisory lock on a
143+
// dedicated connection held for the lock's lifetime. Returns true if the lock
144+
// was acquired, false if another session holds it or the query fails.
136145
func (pm *PartitionManager) tryAdvisoryLock(ctx context.Context) bool {
137-
var acquired bool
138-
err := pm.pool.QueryRow(ctx, "SELECT pg_try_advisory_lock($1)", partitionManagerLockID).Scan(&acquired)
146+
pm.lockMu.Lock()
147+
defer pm.lockMu.Unlock()
148+
149+
if pm.lockConn != nil {
150+
// This manager already holds the lock; do not stack a re-entrant
151+
// acquisition (session advisory locks nest, and a single unlock would
152+
// then leave the lock held).
153+
return false
154+
}
155+
156+
conn, err := pm.pool.Acquire(ctx)
139157
if err != nil {
158+
pm.logger.Warn("partition manager: advisory lock connection acquire failed", slog.Any("error", err))
159+
return false
160+
}
161+
162+
var acquired bool
163+
if err := conn.QueryRow(ctx, "SELECT pg_try_advisory_lock($1)", partitionManagerLockID).Scan(&acquired); err != nil {
140164
pm.logger.Warn("partition manager: advisory lock query failed", slog.Any("error", err))
165+
conn.Release()
166+
return false
167+
}
168+
if !acquired {
169+
conn.Release()
141170
return false
142171
}
143-
return acquired
172+
173+
pm.lockConn = conn
174+
return true
144175
}
145176

146177
// releaseAdvisoryLock releases the session-level advisory lock previously
147-
// acquired by tryAdvisoryLock. Errors are logged but not propagated.
178+
// acquired by tryAdvisoryLock, unlocking on the SAME connection that holds it
179+
// and returning that connection to the pool. Errors are logged but not
180+
// propagated. A no-op if this manager does not currently hold the lock.
148181
func (pm *PartitionManager) releaseAdvisoryLock(ctx context.Context) {
149-
_, err := pm.pool.Exec(ctx, "SELECT pg_advisory_unlock($1)", partitionManagerLockID)
150-
if err != nil {
182+
pm.lockMu.Lock()
183+
defer pm.lockMu.Unlock()
184+
185+
if pm.lockConn == nil {
186+
return
187+
}
188+
if _, err := pm.lockConn.Exec(ctx, "SELECT pg_advisory_unlock($1)", partitionManagerLockID); err != nil {
151189
pm.logger.Warn("partition manager: advisory unlock failed", slog.Any("error", err))
152190
}
191+
pm.lockConn.Release()
192+
pm.lockConn = nil
153193
}
154194

155195
// ensure delegates to OutboxRepository.EnsurePartitions to pre-create future

internal/adapter/postgres/rls_audit_test.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,11 @@ func TestRLSAudit_SentinelPolicyShape(t *testing.T) {
146146
pool := setupAuditDB(t)
147147
ctx := context.Background()
148148

149-
// e.g. current_setting('app.tenant_id', true) = '*' — sentinel from the named const.
150-
sentinelFragment := "current_setting('app.tenant_id', true) = '" + sentinelGUC + "'"
149+
// pg_policies renders the parsed expression in Postgres's canonical form,
150+
// which adds explicit ::text casts to the literal and the GUC argument. Match
151+
// that normalized shape, e.g.:
152+
// (current_setting('app.tenant_id'::text, true) = '*'::text)
153+
sentinelFragment := "current_setting('app.tenant_id'::text, true) = '" + sentinelGUC + "'::text"
151154

152155
for _, tbl := range phaseCTenantTables {
153156
policyName := tbl.policy

internal/adapter/postgres/rls_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,10 @@ func newTenantUser(t *testing.T, tenantID string) *identity.PlatformUser {
6767

6868
return &identity.PlatformUser{
6969
ID: uuid.Must(uuid.NewV7()).String(),
70-
Email: fmt.Sprintf("user-%s@test.com", uuid.Must(uuid.NewV7()).String()[:8]),
70+
// Full UUID (not a truncated prefix): UUIDv7's leading bits are the
71+
// millisecond timestamp, so several users minted in the same tick would
72+
// share a truncated prefix and collide on the unique email constraint.
73+
Email: fmt.Sprintf("user-%s@test.com", uuid.Must(uuid.NewV7()).String()),
7174
PasswordHash: "$2a$10$abcdefghijklmnopqrstuuABCDEFGHIJKLMNOPQRSTUVWXYZ012",
7275
EmailVerified: false,
7376
Role: identity.RoleCustomer,

internal/adapter/postgres/txmanager_reentrant_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,10 @@ func TestTxManager_Reentrant(t *testing.T) {
4646
err := tm.RunInTx(ctx, func(outerCtx context.Context) error {
4747
// Inner RunInTx: must join the outer tx (re-entrant path).
4848
innerErr := tm.RunInTx(outerCtx, func(innerCtx context.Context) error {
49-
_, execErr := pool.Exec(innerCtx,
49+
// Write through the context-bound handle so the INSERT participates in
50+
// the (re-entrant) transaction. A raw pool.Exec would grab its own
51+
// connection and commit independently, defeating the test's premise.
52+
_, execErr := postgres.DBFromContext(innerCtx, pool).Exec(innerCtx,
5053
`INSERT INTO identity.platform_users (id, email, password_hash, role, email_verified)
5154
VALUES ($1, $2, 'hash', 'customer', false)`,
5255
userID, "reentrant@test.com",

0 commit comments

Comments
 (0)