Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commite49c917

Browse files
perf: use a single query for notification target lookups (#20574)
Somewhat minor inefficiency in notifications I discovered during a scaletest where I was creating many users. Our `GetUsers` query filter for rbac roles uses the `&&` operator on arrays, which is the intersection of the two arrays. Despite that, we were making seperate DB queries for each role, and then collating the results. I didn't see any other instances of this.The test changes are required as the order of outgoing notifications is now non-deterministic.
1 parent903c045 commite49c917

File tree

3 files changed

+45
-45
lines changed

3 files changed

+45
-45
lines changed

‎coderd/templates.go‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,19 +1130,11 @@ func (api *API) convertTemplate(
11301130

11311131
// findTemplateAdmins fetches all users with template admin permission including owners.
11321132
funcfindTemplateAdmins(ctx context.Context,store database.Store) ([]database.GetUsersRow,error) {
1133-
// Notice: we can't scrape the user information in parallel as pq
1134-
// fails with: unexpected describe rows response: 'D'
1135-
owners,err:=store.GetUsers(ctx, database.GetUsersParams{
1136-
RbacRole: []string{codersdk.RoleOwner},
1137-
})
1138-
iferr!=nil {
1139-
returnnil,xerrors.Errorf("get owners: %w",err)
1140-
}
11411133
templateAdmins,err:=store.GetUsers(ctx, database.GetUsersParams{
1142-
RbacRole: []string{codersdk.RoleTemplateAdmin},
1134+
RbacRole: []string{codersdk.RoleTemplateAdmin,codersdk.RoleOwner},
11431135
})
11441136
iferr!=nil {
1145-
returnnil,xerrors.Errorf("gettemplate admins: %w",err)
1137+
returnnil,xerrors.Errorf("getowners: %w",err)
11461138
}
1147-
returnappend(owners,templateAdmins...),nil
1139+
returntemplateAdmins,nil
11481140
}

‎coderd/users.go‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1537,21 +1537,13 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create
15371537

15381538
// findUserAdmins fetches all users with user admin permission including owners.
15391539
funcfindUserAdmins(ctx context.Context,store database.Store) ([]database.GetUsersRow,error) {
1540-
// Notice: we can't scrape the user information in parallel as pq
1541-
// fails with: unexpected describe rows response: 'D'
1542-
owners,err:=store.GetUsers(ctx, database.GetUsersParams{
1543-
RbacRole: []string{codersdk.RoleOwner},
1544-
})
1545-
iferr!=nil {
1546-
returnnil,xerrors.Errorf("get owners: %w",err)
1547-
}
15481540
userAdmins,err:=store.GetUsers(ctx, database.GetUsersParams{
1549-
RbacRole: []string{codersdk.RoleUserAdmin},
1541+
RbacRole: []string{codersdk.RoleOwner,codersdk.RoleUserAdmin},
15501542
})
15511543
iferr!=nil {
1552-
returnnil,xerrors.Errorf("getuser admins: %w",err)
1544+
returnnil,xerrors.Errorf("getowners: %w",err)
15531545
}
1554-
returnappend(owners,userAdmins...),nil
1546+
returnuserAdmins,nil
15551547
}
15561548

15571549
funcconvertUsers(users []database.User,organizationIDsByUserIDmap[uuid.UUID][]uuid.UUID) []codersdk.User {

‎coderd/users_test.go‎

Lines changed: 39 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -599,21 +599,28 @@ func TestNotifyDeletedUser(t *testing.T) {
599599
// then
600600
sent:=notifyEnq.Sent()
601601
require.Len(t,sent,5)
602-
// sent[0]: "User admin" account created, "owner" notified
603-
// sent[1]: "Member" account created, "owner" notified
604-
// sent[2]: "Member" account created, "user admin" notified
602+
// Other notifications:
603+
// "User admin" account created, "owner" notified
604+
// "Member" account created, "owner" notified
605+
// "Member" account created, "user admin" notified
605606

606607
// "Member" account deleted, "owner" notified
607-
require.Equal(t,notifications.TemplateUserAccountDeleted,sent[3].TemplateID)
608-
require.Equal(t,firstUser.UserID,sent[3].UserID)
609-
require.Contains(t,sent[3].Targets,member.ID)
610-
require.Equal(t,member.Username,sent[3].Labels["deleted_account_name"])
608+
ownerNotifications:=notifyEnq.Sent(func(n*notificationstest.FakeNotification)bool {
609+
returnn.TemplateID==notifications.TemplateUserAccountDeleted&&
610+
n.UserID==firstUser.UserID&&
611+
slices.Contains(n.Targets,member.ID)&&
612+
n.Labels["deleted_account_name"]==member.Username
613+
})
614+
require.Len(t,ownerNotifications,1)
611615

612616
// "Member" account deleted, "user admin" notified
613-
require.Equal(t,notifications.TemplateUserAccountDeleted,sent[4].TemplateID)
614-
require.Equal(t,userAdmin.ID,sent[4].UserID)
615-
require.Contains(t,sent[4].Targets,member.ID)
616-
require.Equal(t,member.Username,sent[4].Labels["deleted_account_name"])
617+
adminNotifications:=notifyEnq.Sent(func(n*notificationstest.FakeNotification)bool {
618+
returnn.TemplateID==notifications.TemplateUserAccountDeleted&&
619+
n.UserID==userAdmin.ID&&
620+
slices.Contains(n.Targets,member.ID)&&
621+
n.Labels["deleted_account_name"]==member.Username
622+
})
623+
require.Len(t,adminNotifications,1)
617624
})
618625
}
619626

@@ -960,22 +967,31 @@ func TestNotifyCreatedUser(t *testing.T) {
960967
require.Len(t,sent,3)
961968

962969
// "User admin" account created, "owner" notified
963-
require.Equal(t,notifications.TemplateUserAccountCreated,sent[0].TemplateID)
964-
require.Equal(t,firstUser.UserID,sent[0].UserID)
965-
require.Contains(t,sent[0].Targets,userAdmin.ID)
966-
require.Equal(t,userAdmin.Username,sent[0].Labels["created_account_name"])
970+
ownerNotifiedAboutUserAdmin:=notifyEnq.Sent(func(n*notificationstest.FakeNotification)bool {
971+
returnn.TemplateID==notifications.TemplateUserAccountCreated&&
972+
n.UserID==firstUser.UserID&&
973+
slices.Contains(n.Targets,userAdmin.ID)&&
974+
n.Labels["created_account_name"]==userAdmin.Username
975+
})
976+
require.Len(t,ownerNotifiedAboutUserAdmin,1)
967977

968978
// "Member" account created, "owner" notified
969-
require.Equal(t,notifications.TemplateUserAccountCreated,sent[1].TemplateID)
970-
require.Equal(t,firstUser.UserID,sent[1].UserID)
971-
require.Contains(t,sent[1].Targets,member.ID)
972-
require.Equal(t,member.Username,sent[1].Labels["created_account_name"])
979+
ownerNotifiedAboutMember:=notifyEnq.Sent(func(n*notificationstest.FakeNotification)bool {
980+
returnn.TemplateID==notifications.TemplateUserAccountCreated&&
981+
n.UserID==firstUser.UserID&&
982+
slices.Contains(n.Targets,member.ID)&&
983+
n.Labels["created_account_name"]==member.Username
984+
})
985+
require.Len(t,ownerNotifiedAboutMember,1)
973986

974987
// "Member" account created, "user admin" notified
975-
require.Equal(t,notifications.TemplateUserAccountCreated,sent[1].TemplateID)
976-
require.Equal(t,userAdmin.ID,sent[2].UserID)
977-
require.Contains(t,sent[2].Targets,member.ID)
978-
require.Equal(t,member.Username,sent[2].Labels["created_account_name"])
988+
userAdminNotifiedAboutMember:=notifyEnq.Sent(func(n*notificationstest.FakeNotification)bool {
989+
returnn.TemplateID==notifications.TemplateUserAccountCreated&&
990+
n.UserID==userAdmin.ID&&
991+
slices.Contains(n.Targets,member.ID)&&
992+
n.Labels["created_account_name"]==member.Username
993+
})
994+
require.Len(t,userAdminNotifiedAboutMember,1)
979995
})
980996
}
981997

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp