From d95e3bc481b9714f5ed76a00ff36a938e857086d Mon Sep 17 00:00:00 2001 From: aman Date: Wed, 30 Sep 2026 12:23:51 +0530 Subject: [PATCH 1/2] feat(store): role names are unique over live rows only Replace the plain unique constraint on roles (org_id, name) with a unique index over rows where deleted_at is null, and have the role upsert repeat that filter in its conflict target. The upsert's update branch now also sets updated_at. --- .../20260929100000_roles_org_id_name_live_unique.down.sql | 2 ++ .../20260929100000_roles_org_id_name_live_unique.up.sql | 2 ++ internal/store/postgres/postgres.go | 8 +++----- internal/store/postgres/role_repository.go | 3 ++- 4 files changed, 9 insertions(+), 6 deletions(-) create mode 100644 internal/store/postgres/migrations/20260929100000_roles_org_id_name_live_unique.down.sql create mode 100644 internal/store/postgres/migrations/20260929100000_roles_org_id_name_live_unique.up.sql diff --git a/internal/store/postgres/migrations/20260929100000_roles_org_id_name_live_unique.down.sql b/internal/store/postgres/migrations/20260929100000_roles_org_id_name_live_unique.down.sql new file mode 100644 index 000000000..1bf8f7be9 --- /dev/null +++ b/internal/store/postgres/migrations/20260929100000_roles_org_id_name_live_unique.down.sql @@ -0,0 +1,2 @@ +ALTER TABLE roles ADD CONSTRAINT roles_org_id_name_key UNIQUE (org_id, name); +DROP INDEX IF EXISTS uq_roles_org_id_name_live; diff --git a/internal/store/postgres/migrations/20260929100000_roles_org_id_name_live_unique.up.sql b/internal/store/postgres/migrations/20260929100000_roles_org_id_name_live_unique.up.sql new file mode 100644 index 000000000..39f82231b --- /dev/null +++ b/internal/store/postgres/migrations/20260929100000_roles_org_id_name_live_unique.up.sql @@ -0,0 +1,2 @@ +CREATE UNIQUE INDEX IF NOT EXISTS uq_roles_org_id_name_live ON roles (org_id, name) WHERE deleted_at IS NULL; +ALTER TABLE roles DROP CONSTRAINT IF EXISTS roles_org_id_name_key; diff --git a/internal/store/postgres/postgres.go b/internal/store/postgres/postgres.go index 39fecc537..e361a37b7 100644 --- a/internal/store/postgres/postgres.go +++ b/internal/store/postgres/postgres.go @@ -40,11 +40,9 @@ func fromLive(table string) *goqu.SelectDataset { return dialect.From(table).Where(live(table)) } -// liveConflictTarget builds the ON CONFLICT target for a unique index that -// covers only live rows. For example "urn" renders as -// ON CONFLICT (urn) WHERE (deleted_at IS NULL). Postgres uses such an index only -// when the clause names its condition. goqu wraps the target in parentheses as -// is, which is why the string ends open. +// liveConflictTarget is the ON CONFLICT target for a unique index over live +// rows. Postgres matches such an index only when the target repeats its WHERE +// clause. goqu wraps the target in parentheses as is, so the string ends open. func liveConflictTarget(columns string) string { return columns + ") WHERE (deleted_at IS NULL" } diff --git a/internal/store/postgres/role_repository.go b/internal/store/postgres/role_repository.go index bb3f388e1..eab0a9350 100644 --- a/internal/store/postgres/role_repository.go +++ b/internal/store/postgres/role_repository.go @@ -132,12 +132,13 @@ func (r RoleRepository) Upsert(ctx context.Context, rl role.Role) (role.Role, er "state": rl.State, "metadata": marshaledMetadata, "scopes": pq.Array(rl.Scopes), - }).OnConflict(goqu.DoUpdate("org_id, name", goqu.Record{ + }).OnConflict(goqu.DoUpdate(liveConflictTarget("org_id, name"), goqu.Record{ "title": rl.Title, "permissions": marshaledPermissions, "state": rl.State, "metadata": marshaledMetadata, "scopes": pq.Array(rl.Scopes), + "updated_at": goqu.L("now()"), })).Returning(&Role{}).ToSQL() if err != nil { return role.Role{}, fmt.Errorf("%w: %s", errQuery, err) From 7f48580784506101a929507ab48c0b66a7edb6f8 Mon Sep 17 00:00:00 2001 From: aman Date: Wed, 30 Sep 2026 12:23:51 +0530 Subject: [PATCH 2/2] test(store): cover the role upsert against soft-deleted rows A deleted name gets a new row, a reused id still conflicts, and a live duplicate updates in place and moves updated_at. --- .../store/postgres/role_repository_test.go | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/internal/store/postgres/role_repository_test.go b/internal/store/postgres/role_repository_test.go index 5597dc30f..a30188bcf 100644 --- a/internal/store/postgres/role_repository_test.go +++ b/internal/store/postgres/role_repository_test.go @@ -190,6 +190,68 @@ func (s *RoleRepositoryTestSuite) TestCreate() { } }) } + + s.Run("should create a new row when the name belongs to a soft-deleted role", func() { + deleted := s.roles[3] + if _, err := s.client.ExecContext(s.ctx, "UPDATE roles SET deleted_at = now() WHERE id = $1", deleted.ID); err != nil { + s.T().Fatal(err) + } + + got, err := s.repository.Upsert(s.ctx, role.Role{ + Name: deleted.Name, + Title: "Test Title", + Permissions: []string{"user"}, + OrgID: s.orgID, + Metadata: metadata.Metadata{}, + }) + s.Assert().NoError(err) + if got.ID == deleted.ID { + s.T().Fatalf("got the deleted row %s back, expected a new row", deleted.ID) + } + + var rows int + if err := s.client.QueryRowxContext(s.ctx, "SELECT count(*) FROM roles WHERE org_id = $1 AND name = $2", s.orgID, deleted.Name).Scan(&rows); err != nil { + s.T().Fatal(err) + } + if rows != 2 { + s.T().Fatalf("got %d rows named %s, expected the deleted row and the new one", rows, deleted.Name) + } + }) + + s.Run("should return conflict when the id belongs to a soft-deleted role", func() { + deleted := s.roles[4] + if _, err := s.client.ExecContext(s.ctx, "UPDATE roles SET deleted_at = now() WHERE id = $1", deleted.ID); err != nil { + s.T().Fatal(err) + } + + _, err := s.repository.Upsert(s.ctx, role.Role{ + ID: deleted.ID, + Name: "role with a reused id", + OrgID: s.orgID, + Metadata: metadata.Metadata{}, + }) + s.Assert().ErrorIs(err, role.ErrConflict) + }) + + s.Run("should update the live role and move updated_at when the name is taken", func() { + before, err := s.repository.Get(s.ctx, s.roles[2].ID) + if err != nil { + s.T().Fatal(err) + } + + got, err := s.repository.Upsert(s.ctx, role.Role{ + Name: before.Name, + Title: "changed", + Permissions: before.Permissions, + Scopes: before.Scopes, + OrgID: s.orgID, + Metadata: metadata.Metadata{}, + }) + s.Assert().NoError(err) + s.Assert().Equal(before.ID, got.ID) + s.Assert().Equal("changed", got.Title) + s.Assert().True(got.UpdatedAt.After(before.UpdatedAt)) + }) } func (s *RoleRepositoryTestSuite) TestList() {