shishantbiswas/coder
1
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-06-02 20:48:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

perf: use a single query for notification target lookups (#20574)

Browse Source 
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.
This commit is contained in:
Ethan
2025-11-11 21:23:23 -05:00
committed by GitHub
parent 903c045b9c
commit e49c917bb0
3 changed files with 45 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/templates.go
+3 -11
View File
@@ -1130,19 +1130,11 @@ func (api *API) convertTemplate(
// findTemplateAdmins fetches all users with template admin permission including owners.
func findTemplateAdmins(ctx context.Context, store database.Store) ([]database.GetUsersRow, error) {
// Notice: we can't scrape the user information in parallel as pq
// fails with: unexpected describe rows response: 'D'
owners, err := store.GetUsers(ctx, database.GetUsersParams{
RbacRole: []string{codersdk.RoleOwner},
templateAdmins, err := store.GetUsers(ctx, database.GetUsersParams{
RbacRole: []string{codersdk.RoleTemplateAdmin, codersdk.RoleOwner},
})
if err != nil {
return nil, xerrors.Errorf("get owners: %w", err)
}
templateAdmins, err := store.GetUsers(ctx, database.GetUsersParams{
RbacRole: []string{codersdk.RoleTemplateAdmin},
})
if err != nil {
return nil, xerrors.Errorf("get template admins: %w", err)
}
return append(owners, templateAdmins...), nil
return templateAdmins, nil
}
coderd/users.go
+3 -11
View File
@@ -1537,21 +1537,13 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create
// findUserAdmins fetches all users with user admin permission including owners.
func findUserAdmins(ctx context.Context, store database.Store) ([]database.GetUsersRow, error) {
// Notice: we can't scrape the user information in parallel as pq
// fails with: unexpected describe rows response: 'D'
owners, err := store.GetUsers(ctx, database.GetUsersParams{
RbacRole: []string{codersdk.RoleOwner},
userAdmins, err := store.GetUsers(ctx, database.GetUsersParams{
RbacRole: []string{codersdk.RoleOwner, codersdk.RoleUserAdmin},
})
if err != nil {
return nil, xerrors.Errorf("get owners: %w", err)
}
userAdmins, err := store.GetUsers(ctx, database.GetUsersParams{
RbacRole: []string{codersdk.RoleUserAdmin},
})
if err != nil {
return nil, xerrors.Errorf("get user admins: %w", err)
}
return append(owners, userAdmins...), nil
return userAdmins, nil
}
func convertUsers(users []database.User, organizationIDsByUserID map[uuid.UUID][]uuid.UUID) []codersdk.User {
coderd/users_test.go
+39 -23
View File
@@ -599,21 +599,28 @@ func TestNotifyDeletedUser(t *testing.T) {
// then
sent := notifyEnq.Sent()
require.Len(t, sent, 5)
// sent[0]: "User admin" account created, "owner" notified
// sent[1]: "Member" account created, "owner" notified
// sent[2]: "Member" account created, "user admin" notified
// Other notifications:
// "User admin" account created, "owner" notified
// "Member" account created, "owner" notified
// "Member" account created, "user admin" notified
// "Member" account deleted, "owner" notified
require.Equal(t, notifications.TemplateUserAccountDeleted, sent[3].TemplateID)
require.Equal(t, firstUser.UserID, sent[3].UserID)
require.Contains(t, sent[3].Targets, member.ID)
require.Equal(t, member.Username, sent[3].Labels["deleted_account_name"])
ownerNotifications := notifyEnq.Sent(func(n *notificationstest.FakeNotification) bool {
return n.TemplateID == notifications.TemplateUserAccountDeleted &&
n.UserID == firstUser.UserID &&
slices.Contains(n.Targets, member.ID) &&
n.Labels["deleted_account_name"] == member.Username
})
require.Len(t, ownerNotifications, 1)
// "Member" account deleted, "user admin" notified
require.Equal(t, notifications.TemplateUserAccountDeleted, sent[4].TemplateID)
require.Equal(t, userAdmin.ID, sent[4].UserID)
require.Contains(t, sent[4].Targets, member.ID)
require.Equal(t, member.Username, sent[4].Labels["deleted_account_name"])
adminNotifications := notifyEnq.Sent(func(n *notificationstest.FakeNotification) bool {
return n.TemplateID == notifications.TemplateUserAccountDeleted &&
n.UserID == userAdmin.ID &&
slices.Contains(n.Targets, member.ID) &&
n.Labels["deleted_account_name"] == member.Username
})
require.Len(t, adminNotifications, 1)
})
}
@@ -960,22 +967,31 @@ func TestNotifyCreatedUser(t *testing.T) {
require.Len(t, sent, 3)
// "User admin" account created, "owner" notified
require.Equal(t, notifications.TemplateUserAccountCreated, sent[0].TemplateID)
require.Equal(t, firstUser.UserID, sent[0].UserID)
require.Contains(t, sent[0].Targets, userAdmin.ID)
require.Equal(t, userAdmin.Username, sent[0].Labels["created_account_name"])
ownerNotifiedAboutUserAdmin := notifyEnq.Sent(func(n *notificationstest.FakeNotification) bool {
return n.TemplateID == notifications.TemplateUserAccountCreated &&
n.UserID == firstUser.UserID &&
slices.Contains(n.Targets, userAdmin.ID) &&
n.Labels["created_account_name"] == userAdmin.Username
})
require.Len(t, ownerNotifiedAboutUserAdmin, 1)
// "Member" account created, "owner" notified
require.Equal(t, notifications.TemplateUserAccountCreated, sent[1].TemplateID)
require.Equal(t, firstUser.UserID, sent[1].UserID)
require.Contains(t, sent[1].Targets, member.ID)
require.Equal(t, member.Username, sent[1].Labels["created_account_name"])
ownerNotifiedAboutMember := notifyEnq.Sent(func(n *notificationstest.FakeNotification) bool {
return n.TemplateID == notifications.TemplateUserAccountCreated &&
n.UserID == firstUser.UserID &&
slices.Contains(n.Targets, member.ID) &&
n.Labels["created_account_name"] == member.Username
})
require.Len(t, ownerNotifiedAboutMember, 1)
// "Member" account created, "user admin" notified
require.Equal(t, notifications.TemplateUserAccountCreated, sent[1].TemplateID)
require.Equal(t, userAdmin.ID, sent[2].UserID)
require.Contains(t, sent[2].Targets, member.ID)
require.Equal(t, member.Username, sent[2].Labels["created_account_name"])
userAdminNotifiedAboutMember := notifyEnq.Sent(func(n *notificationstest.FakeNotification) bool {
return n.TemplateID == notifications.TemplateUserAccountCreated &&
n.UserID == userAdmin.ID &&
slices.Contains(n.Targets, member.ID) &&
n.Labels["created_account_name"] == member.Username
})
require.Len(t, userAdminNotifiedAboutMember, 1)
})
}