From 252c2ba925ac21fbc5a36a3810216f646973228f Mon Sep 17 00:00:00 2001 From: Michelangelo Mori <328978+blkt@users.noreply.github.com> Date: Tue, 21 Apr 2026 19:31:55 +0200 Subject: [PATCH] Persist `claims` on registry upsert conflict The `UpsertRegistry` SQL query only updated `updated_at` on conflict, silently dropping any new `claims` value. This meant that editing a registry via `PUT /v1/registries/{name}` would appear to succeed (the timestamp advanced) but the claims column kept its original value. Add `claims = EXCLUDED.claims` to the `ON CONFLICT DO UPDATE SET` clause and regenerate the sqlc code. A new regression test (`TestUpsertRegistryUpdatesClaims`) pins both the value-update and clear-to-nil paths. --- database/queries/registry.sql | 2 +- internal/db/sqlc/registry.sql.go | 2 +- internal/db/sqlc/registry_test.go | 120 ++++++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 2 deletions(-) diff --git a/database/queries/registry.sql b/database/queries/registry.sql index c4ab4dbf..49650daa 100644 --- a/database/queries/registry.sql +++ b/database/queries/registry.sql @@ -16,7 +16,7 @@ FROM registry WHERE name = sqlc.arg(name); -- Business logic in Go guards against cross-type overwrites. INSERT INTO registry (name, claims, creation_type, created_at, updated_at) VALUES (sqlc.arg(name), sqlc.narg(claims), sqlc.arg(creation_type), sqlc.arg(created_at), sqlc.arg(updated_at)) -ON CONFLICT (name) DO UPDATE SET updated_at = EXCLUDED.updated_at +ON CONFLICT (name) DO UPDATE SET claims = EXCLUDED.claims, updated_at = EXCLUDED.updated_at RETURNING *; -- name: DeleteRegistry :execrows diff --git a/internal/db/sqlc/registry.sql.go b/internal/db/sqlc/registry.sql.go index 6c306418..5b326759 100644 --- a/internal/db/sqlc/registry.sql.go +++ b/internal/db/sqlc/registry.sql.go @@ -188,7 +188,7 @@ func (q *Queries) UnlinkRegistrySource(ctx context.Context, arg UnlinkRegistrySo const upsertRegistry = `-- name: UpsertRegistry :one INSERT INTO registry (name, claims, creation_type, created_at, updated_at) VALUES ($1, $2, $3, $4, $5) -ON CONFLICT (name) DO UPDATE SET updated_at = EXCLUDED.updated_at +ON CONFLICT (name) DO UPDATE SET claims = EXCLUDED.claims, updated_at = EXCLUDED.updated_at RETURNING id, name, claims, creation_type, created_at, updated_at ` diff --git a/internal/db/sqlc/registry_test.go b/internal/db/sqlc/registry_test.go index 31fbaf25..81f5b187 100644 --- a/internal/db/sqlc/registry_test.go +++ b/internal/db/sqlc/registry_test.go @@ -910,3 +910,123 @@ func TestGetRegistryByName(t *testing.T) { }) } } + +func TestUpsertRegistryUpdatesClaims(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + setupFunc func(t *testing.T, queries *Queries) + scenarioFunc func(t *testing.T, queries *Queries) + }{ + { + name: "claims are updated on upsert conflict", + //nolint:thelper // We want to see these lines in the test output + setupFunc: func(t *testing.T, queries *Queries) { + createdAt := time.Now().UTC() + oldClaims, err := json.Marshal(map[string]string{"org": "acme"}) + require.NoError(t, err) + + _, err = queries.UpsertRegistry(context.Background(), UpsertRegistryParams{ + Name: "claims-update-test", + Claims: oldClaims, + CreationType: CreationTypeAPI, + CreatedAt: &createdAt, + UpdatedAt: &createdAt, + }) + require.NoError(t, err) + }, + //nolint:thelper // We want to see these lines in the test output + scenarioFunc: func(t *testing.T, queries *Queries) { + updatedAt := time.Now().UTC() + newClaims, err := json.Marshal(map[string]string{"org": "contoso", "team": "eng"}) + require.NoError(t, err) + + // Upsert the same registry with updated claims. + returned, err := queries.UpsertRegistry(context.Background(), UpsertRegistryParams{ + Name: "claims-update-test", + Claims: newClaims, + CreationType: CreationTypeAPI, + CreatedAt: &updatedAt, + UpdatedAt: &updatedAt, + }) + require.NoError(t, err) + + // The RETURNING row must carry the new claims. + var returnedClaims map[string]string + err = json.Unmarshal(returned.Claims, &returnedClaims) + require.NoError(t, err) + require.Equal(t, "contoso", returnedClaims["org"]) + require.Equal(t, "eng", returnedClaims["team"]) + _, hasAcme := returnedClaims["org"] + require.True(t, hasAcme) // key exists but value is "contoso", not "acme" + require.NotEqual(t, "acme", returnedClaims["org"]) + + // A fresh GET must also reflect the updated claims. + fetched, err := queries.GetRegistryByName(context.Background(), "claims-update-test") + require.NoError(t, err) + + var fetchedClaims map[string]string + err = json.Unmarshal(fetched.Claims, &fetchedClaims) + require.NoError(t, err) + require.Equal(t, "contoso", fetchedClaims["org"]) + require.Equal(t, "eng", fetchedClaims["team"]) + require.NotEqual(t, "acme", fetchedClaims["org"]) + }, + }, + { + name: "claims cleared to nil on upsert conflict", + //nolint:thelper // We want to see these lines in the test output + setupFunc: func(t *testing.T, queries *Queries) { + createdAt := time.Now().UTC() + oldClaims, err := json.Marshal(map[string]string{"org": "acme"}) + require.NoError(t, err) + + _, err = queries.UpsertRegistry(context.Background(), UpsertRegistryParams{ + Name: "claims-nil-test", + Claims: oldClaims, + CreationType: CreationTypeAPI, + CreatedAt: &createdAt, + UpdatedAt: &createdAt, + }) + require.NoError(t, err) + }, + //nolint:thelper // We want to see these lines in the test output + scenarioFunc: func(t *testing.T, queries *Queries) { + updatedAt := time.Now().UTC() + + // Upsert the same registry with nil claims. + returned, err := queries.UpsertRegistry(context.Background(), UpsertRegistryParams{ + Name: "claims-nil-test", + Claims: nil, + CreationType: CreationTypeAPI, + CreatedAt: &updatedAt, + UpdatedAt: &updatedAt, + }) + require.NoError(t, err) + + // The RETURNING row must have nil claims. + require.Nil(t, returned.Claims) + + // A fresh GET must also show nil claims. + fetched, err := queries.GetRegistryByName(context.Background(), "claims-nil-test") + require.NoError(t, err) + require.Nil(t, fetched.Claims) + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + db, cleanupFunc := database.SetupTestDB(t) + t.Cleanup(cleanupFunc) + queries := New(db) + require.NotNil(t, queries) + + tc.setupFunc(t, queries) + tc.scenarioFunc(t, queries) + }) + } +}