Skip to content

Commit 34a971a

Browse files
EmyrkEmyrk
committed
chore: handle db conflicts gracefully
1 parent ef928e4 commit 34a971a

File tree

5 files changed

+19
-4
lines changed

5 files changed

+19
-4
lines changed

‎coderd/database/querier.go

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/groupmembers.sql

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ SELECT
4646
groups.id
4747
FROM
4848
groups
49+
-- If there is a conflict, the user is already a member
50+
ON CONFLICT DO NOTHING
4951
RETURNING group_id;
5052

5153
-- name: RemoveUserFromAllGroups :exec

‎coderd/idpsync/group.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,8 @@ func (s AGPLIDPSync) ApplyGroupDifference(ctx context.Context, tx database.Store
226226
}
227227

228228
if len(add) > 0 {
229+
add = slice.Unique(add)
230+
// Defensive programming to only insert uniques.
229231
assignedGroupIDs, err := tx.InsertUserGroupsByID(ctx, database.InsertUserGroupsByIDParams{
230232
UserID: user.ID,
231233
GroupIds: add,

‎coderd/idpsync/group_test.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@ package idpsync_test
22

33
import (
44
"context"
5+
"database/sql"
56
"regexp"
67
"testing"
78

89
"github.com/golang-jwt/jwt/v4"
910
"github.com/google/uuid"
1011
"github.com/stretchr/testify/require"
1112
"golang.org/x/exp/slices"
13+
"golang.org/x/xerrors"
1214

1315
"cdr.dev/slog/sloggers/slogtest"
1416
"github.com/coder/coder/v2/coderd/coderdtest"
@@ -64,6 +66,7 @@ func TestParseGroupClaims(t *testing.T) {
6466
func TestGroupSyncTable(t *testing.T) {
6567
t.Parallel()
6668

69+
// Last checked, takes 30s with postgres on a fast machine.
6770
if dbtestutil.WillUsePostgres() {
6871
t.Skip("Skipping test because it populates a lot of db entries, which is slow on postgres.")
6972
}
@@ -553,10 +556,15 @@ func TestApplyGroupDifference(t *testing.T) {
553556
func SetupOrganization(t *testing.T, s *idpsync.AGPLIDPSync, db database.Store, user database.User, orgID uuid.UUID, def orgSetupDefinition) {
554557
t.Helper()
555558

556-
org := dbgen.Organization(t, db, database.Organization{
557-
ID: orgID,
558-
})
559-
_, err := db.InsertAllUsersGroup(context.Background(), org.ID)
559+
// Account that the org might be the default organization
560+
org, err := db.GetOrganizationByID(context.Background(), orgID)
561+
if xerrors.Is(err, sql.ErrNoRows) {
562+
org = dbgen.Organization(t, db, database.Organization{
563+
ID: orgID,
564+
})
565+
}
566+
567+
_, err = db.InsertAllUsersGroup(context.Background(), org.ID)
560568
if !database.IsUniqueViolation(err) {
561569
require.NoError(t, err, "Everyone group for an org")
562570
}

0 commit comments

Comments
 (0)