chore: handle db conflicts gracefully

coder · Emyrk · Sep 11, 2024 · Sep 3, 2024 · Sep 3, 2024 · Sep 3, 2024
commit 6491f6ac295dfc3a765d95ef883d14f61647d4fc
diff --git a/coderd/database/querier.go b/coderd/database/querier.go
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/groupmembers.sql b/coderd/database/queries/groupmembers.sql
@@ -46,6 +46,8 @@ SELECT
 	groups.id
 FROM
 	groups
+-- If there is a conflict, the user is already a member
+ON CONFLICT DO NOTHING
 RETURNING group_id;
 
 -- name: RemoveUserFromAllGroups :exec

diff --git a/coderd/idpsync/group.go b/coderd/idpsync/group.go
@@ -226,6 +226,8 @@ func (s AGPLIDPSync) ApplyGroupDifference(ctx context.Context, tx database.Store
 	}
 
 	if len(add) > 0 {
+		add = slice.Unique(add)
+		// Defensive programming to only insert uniques.
 		assignedGroupIDs, err := tx.InsertUserGroupsByID(ctx, database.InsertUserGroupsByIDParams{
 			UserID:   user.ID,
 			GroupIds: add,

diff --git a/coderd/idpsync/group_test.go b/coderd/idpsync/group_test.go
@@ -2,13 +2,15 @@ package idpsync_test
 
 import (
 	"context"
+	"database/sql"
 	"regexp"
 	"testing"
 
 	"github.com/golang-jwt/jwt/v4"
 	"github.com/google/uuid"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/exp/slices"
+	"golang.org/x/xerrors"
 
 	"cdr.dev/slog/sloggers/slogtest"
 	"github.com/coder/coder/v2/coderd/coderdtest"
@@ -64,6 +66,7 @@ func TestParseGroupClaims(t *testing.T) {
 func TestGroupSyncTable(t *testing.T) {
 	t.Parallel()
 
+	// Last checked, takes 30s with postgres on a fast machine.
 	if dbtestutil.WillUsePostgres() {
 		t.Skip("Skipping test because it populates a lot of db entries, which is slow on postgres.")
 	}
@@ -553,10 +556,15 @@ func TestApplyGroupDifference(t *testing.T) {
 func SetupOrganization(t *testing.T, s *idpsync.AGPLIDPSync, db database.Store, user database.User, orgID uuid.UUID, def orgSetupDefinition) {
 	t.Helper()
 
-	org := dbgen.Organization(t, db, database.Organization{
-		ID: orgID,
-	})
-	_, err := db.InsertAllUsersGroup(context.Background(), org.ID)
+	// Account that the org might be the default organization
+	org, err := db.GetOrganizationByID(context.Background(), orgID)
+	if xerrors.Is(err, sql.ErrNoRows) {
+		org = dbgen.Organization(t, db, database.Organization{
+			ID: orgID,
+		})
+	}
+
+	_, err = db.InsertAllUsersGroup(context.Background(), org.ID)
 	if !database.IsUniqueViolation(err) {
 		require.NoError(t, err, "Everyone group for an org")
 	}