Skip to content

Commit baf7766

Browse files
grvijayanclaude
andcommitted
fix(bootstrap): only ignore a missing row when ensuring the team
Any other GetTeamByID error now returns instead of falling through to CreateTeam, which surfaced it as a misleading unique or foreign key error. Give the import tests random IDs so reruns and parallel tests cannot collide, and fix the undefined testV2Pool that broke the integration build. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent c27a2d1 commit baf7766

3 files changed

Lines changed: 110 additions & 32 deletions

File tree

internal/bootstrap/users/ensure.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,17 +45,21 @@ func ensureProject(ctx context.Context, stmts service.AllStatements, projectID s
4545
func ensureTeam(ctx context.Context, stmts service.AllStatements, projectID, teamID string) error {
4646
// Team names are unique per project too, so a UniqueError from the insert
4747
// below does not prove the team exists.
48-
if _, err := stmts.GetTeamByID(ctx, projectID, teamID); err == nil {
48+
_, err := stmts.GetTeamByID(ctx, projectID, teamID)
49+
if err == nil {
4950
return nil
5051
}
52+
// Only a missing row means we still have to create it.
53+
if _, ok := errors.AsType[*database.NoRowFoundError](err); !ok {
54+
return fmt.Errorf("get team %q: %w", teamID, err)
55+
}
5156
// The bootstrap header carries no team name, so derive a placeholder
5257
// name from the team ID to satisfy the NOT NULL name column.
53-
err := stmts.CreateTeam(ctx, &domain.Team{
58+
if err := stmts.CreateTeam(ctx, &domain.Team{
5459
ProjectID: projectID,
5560
ID: teamID,
5661
Name: "team-" + teamID,
57-
})
58-
if err != nil {
62+
}); err != nil {
5963
return fmt.Errorf("ensure team %q: %w", teamID, err)
6064
}
6165
return nil
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package users
2+
3+
import (
4+
"errors"
5+
"testing"
6+
7+
"github.com/stretchr/testify/require"
8+
"github.com/zitadel/nextgen/internal/domain"
9+
servicemocks "github.com/zitadel/nextgen/internal/service/mocks"
10+
"github.com/zitadel/nextgen/internal/storage/v2/database"
11+
"go.uber.org/mock/gomock"
12+
)
13+
14+
func TestEnsureTeam(t *testing.T) {
15+
readErr := errors.New("connection refused")
16+
17+
tests := []struct {
18+
name string
19+
// expect declares the reads and writes the case allows; an unexpected
20+
// CreateTeam fails the test on its own.
21+
expect func(*servicemocks.MockAllStatements)
22+
wantErr error
23+
}{
24+
{
25+
name: "existing team is not recreated",
26+
expect: func(stmts *servicemocks.MockAllStatements) {
27+
stmts.EXPECT().GetTeamByID(gomock.Any(), "proj_1", "team_1").
28+
Return(&domain.Team{ProjectID: "proj_1", ID: "team_1"}, nil)
29+
},
30+
},
31+
{
32+
name: "missing team is created",
33+
expect: func(stmts *servicemocks.MockAllStatements) {
34+
stmts.EXPECT().GetTeamByID(gomock.Any(), "proj_1", "team_1").
35+
Return(nil, database.NewNoRowFoundError(errors.New("no rows")))
36+
stmts.EXPECT().CreateTeam(gomock.Any(), &domain.Team{
37+
ProjectID: "proj_1",
38+
ID: "team_1",
39+
Name: "team-team_1",
40+
}).Return(nil)
41+
},
42+
},
43+
{
44+
name: "read failure is returned instead of creating",
45+
expect: func(stmts *servicemocks.MockAllStatements) {
46+
stmts.EXPECT().GetTeamByID(gomock.Any(), "proj_1", "team_1").
47+
Return(nil, readErr)
48+
},
49+
wantErr: readErr,
50+
},
51+
}
52+
53+
for _, tt := range tests {
54+
t.Run(tt.name, func(t *testing.T) {
55+
stmts := servicemocks.NewMockAllStatements(gomock.NewController(t))
56+
tt.expect(stmts)
57+
58+
err := ensureTeam(t.Context(), stmts, "proj_1", "team_1")
59+
if tt.wantErr != nil {
60+
require.ErrorIs(t, err, tt.wantErr)
61+
return
62+
}
63+
require.NoError(t, err)
64+
})
65+
}
66+
}

internal/bootstrap/users/import_test.go

Lines changed: 36 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package users_test
44

55
import (
66
"context"
7+
"crypto/rand"
78
"encoding/json"
89
"os"
910
"path/filepath"
@@ -42,25 +43,29 @@ func TestImport_loadAndSkip(t *testing.T) {
4243
ctx := t.Context()
4344
hasher := testHasher(t)
4445
v2Pool := testPool(t)
46+
stmts := v2Pool.Statements()
47+
48+
suffix := rand.Text()
49+
projectID := "proj_" + suffix
50+
userID := "usr_" + suffix
51+
t.Cleanup(func() { _ = stmts.DeleteProjectByID(context.Background(), projectID) })
4552

46-
dir := t.TempDir()
47-
path := filepath.Join(dir, "user.json")
48-
writeUserFile(t, path, "usr_import_1")
53+
path := writeUserFile(t, projectID, "team_"+suffix, userID)
4954

5055
require.NoError(t, users.Import(ctx, v2Pool, hasher, "postgres", []string{path}))
5156

52-
got, err := v2Pool.Statements().GetUser(ctx, database.And(
53-
database.Equal(database.Col(domain.UserFieldProjectID), "proj_demo"),
54-
database.Equal(database.Col(domain.UserFieldID), "usr_import_1"),
57+
got, err := stmts.GetUser(ctx, database.And(
58+
database.Equal(database.Col(domain.UserFieldProjectID), projectID),
59+
database.Equal(database.Col(domain.UserFieldID), userID),
5560
), service.UserQueryOptions{
5661
AttributeKeys: []string{"username"},
5762
})
5863
require.NoError(t, err)
59-
require.Equal(t, "usr_import_1", got.ID)
64+
require.Equal(t, userID, got.ID)
6065

61-
pw, err := v2Pool.Statements().GetUserPassword(ctx, database.And(
62-
database.Equal(database.Col(domain.UserPasswordFieldProjectID), "proj_demo"),
63-
database.Equal(database.Col(domain.UserPasswordFieldUserID), "usr_import_1"),
66+
pw, err := stmts.GetUserPassword(ctx, database.And(
67+
database.Equal(database.Col(domain.UserPasswordFieldProjectID), projectID),
68+
database.Equal(database.Col(domain.UserPasswordFieldUserID), userID),
6469
))
6570
require.NoError(t, err)
6671
require.NotEmpty(t, pw.EncodedHash)
@@ -76,47 +81,50 @@ func TestImport_spannerRejected(t *testing.T) {
7681

7782
func TestImport_teamPlaceholderNameTaken(t *testing.T) {
7883
ctx := t.Context()
79-
v2Pool := testV2Pool(t)
84+
v2Pool := testPool(t)
8085
stmts := v2Pool.Statements()
8186

87+
suffix := rand.Text()
88+
projectID := "proj_" + suffix
89+
teamID := "team_" + suffix
90+
8291
require.NoError(t, stmts.CreateProject(ctx, &domain.Project{
83-
ID: "proj_123",
84-
Name: "project-proj_123",
92+
ID: projectID,
93+
Name: "project-" + projectID,
8594
PreviewOrigins: []string{},
8695
}))
87-
t.Cleanup(func() { _ = stmts.DeleteProjectByID(context.Background(), "proj_123") })
96+
t.Cleanup(func() { _ = stmts.DeleteProjectByID(context.Background(), projectID) })
8897

8998
// The placeholder name is derived from the team ID, so parking it on another
9099
// team makes the insert fail on the name index instead of the primary key.
91100
require.NoError(t, stmts.CreateTeam(ctx, &domain.Team{
92-
ProjectID: "proj_123",
93-
ID: "team_456",
94-
Name: "team-team_123",
101+
ProjectID: projectID,
102+
ID: teamID + "_other",
103+
Name: "team-" + teamID,
95104
}))
96105

97-
doc := validDocument()
98-
doc.Header.ProjectID = "proj_123"
99-
doc.Header.TeamID = "team_123"
100-
doc.Header.ID = "usr_name_123"
101-
raw, err := json.Marshal(doc)
102-
require.NoError(t, err)
103-
path := filepath.Join(t.TempDir(), "user.json")
104-
require.NoError(t, os.WriteFile(path, raw, 0o600))
106+
path := writeUserFile(t, projectID, teamID, "usr_"+suffix)
105107

106108
// The import fails either way; what matters is that it names the team it
107109
// could not create, rather than a later team_memberships insert tripping
108110
// over the missing team's foreign key.
109-
err = users.Import(ctx, v2Pool, testHasher(t), "postgres", []string{path})
111+
err := users.Import(ctx, v2Pool, testHasher(t), "postgres", []string{path})
110112
require.Error(t, err)
111-
require.Contains(t, err.Error(), `ensure team "team_123"`)
113+
require.Contains(t, err.Error(), `ensure team "`+teamID+`"`)
112114
require.Contains(t, err.Error(), "uq_teams_project_name")
113115
}
114116

115-
func writeUserFile(t *testing.T, path, userID string) {
117+
// writeUserFile writes a valid user document carrying the given IDs and returns
118+
// its path.
119+
func writeUserFile(t *testing.T, projectID, teamID, userID string) string {
116120
t.Helper()
117121
doc := validDocument()
122+
doc.Header.ProjectID = projectID
123+
doc.Header.TeamID = teamID
118124
doc.Header.ID = userID
119125
raw, err := json.Marshal(doc)
120126
require.NoError(t, err)
127+
path := filepath.Join(t.TempDir(), "user.json")
121128
require.NoError(t, os.WriteFile(path, raw, 0o600))
129+
return path
122130
}

0 commit comments

Comments
 (0)