General GraphQL resolver optimizations (#20611)

api4/resolver_channel.go

@@ -71,16 +71,16 @@ func parseChannelCursor(cursor string) (channelID string, ok bool) {
 		return "", false
 	}
 
-	parts := strings.Split(string(decoded), "-")
+	prefix, id, found := strings.Cut(string(decoded), "-")
-	if len(parts) != 2 {
+	if !found {
 		return "", false
 	}
 
-	if cursorPrefix(parts[0]) != channelCursorPrefix {
+	if cursorPrefix(prefix) != channelCursorPrefix {
 		return "", false
 	}
 
-	return parts[1], true
+	return id, true
 }
 
 func postProcessChannels(c *web.Context, channels []*model.Channel) ([]*channel, error) {
@@ -111,8 +111,9 @@ func postProcessChannels(c *web.Context, channels []*model.Channel) ([]*channel,
 	}
 
 	// Convert to the wrapper format.
-	res := make([]*channel, 0, len(channels))
+	nameCache := make(map[string]string)
-	for _, ch := range channels {
+	res := make([]*channel, len(channels))
+	for i, ch := range channels {
 		prettyName := ch.DisplayName
 
 		if ch.IsGroupOrDirect() {
@@ -121,37 +122,39 @@ func postProcessChannels(c *web.Context, channels []*model.Channel) ([]*channel,
 			if users == nil {
 				return nil, fmt.Errorf("user info not found for channel id: %s", ch.Id)
 			}
-			prettyName = getPrettyDNForUsers(nameFormat, users, c.AppContext.Session().UserId)
+			prettyName = getPrettyDNForUsers(nameFormat, users, c.AppContext.Session().UserId, nameCache)
 		}
 
-		res = append(res, &channel{Channel: *ch, PrettyDisplayName: prettyName})
+		res[i] = &channel{Channel: *ch, PrettyDisplayName: prettyName}
 	}
 
 	return res, nil
 }
 
-func getPrettyDNForUsers(displaySetting string, users []*model.User, omitUserId string) string {
+func getPrettyDNForUsers(displaySetting string, users []*model.User, omitUserId string, cache map[string]string) string {
 	displayNames := make([]string, 0, len(users))
-	// TODO: optimize this logic.
-	// Name computation happens repeatedly for the same user from
-	// multiple channels.
 	for _, u := range users {
 		if u.Id == omitUserId {
 			continue
 		}
-		displayNames = append(displayNames, getPrettyDNForUser(displaySetting, u))
+		displayNames = append(displayNames, getPrettyDNForUser(displaySetting, u, cache))
 	}
 
 	sort.Strings(displayNames)
 	result := strings.Join(displayNames, ", ")
 	if result == "" {
 		// Self DM
-		result = getPrettyDNForUser(displaySetting, users[0])
+		result = getPrettyDNForUser(displaySetting, users[0], cache)
 	}
 	return result
 }
 
-func getPrettyDNForUser(displaySetting string, user *model.User) string {
+func getPrettyDNForUser(displaySetting string, user *model.User, cache map[string]string) string {
+	// use the cache first
+	if name, ok := cache[user.Id]; ok {
+		return name
+	}
+
 	var displayName string
 	switch displaySetting {
 	case "nickname_full_name":
@@ -171,5 +174,8 @@ func getPrettyDNForUser(displaySetting string, user *model.User) string {
 		displayName = user.Username
 	}
 
+	// update the cache
+	cache[user.Id] = displayName
+
 	return displayName
 }

api4/resolver_channel_test.go

@@ -440,7 +440,7 @@ func TestGetPrettyDNForUsers(t *testing.T) {
 				LastName:  "last2",
 			},
 		}
-		assert.Equal(t, "nick2", getPrettyDNForUsers("nickname_full_name", users, "user1"))
+		assert.Equal(t, "nick2", getPrettyDNForUsers("nickname_full_name", users, "user1", map[string]string{}))
 
 		users = []*model.User{
 			{
@@ -456,7 +456,7 @@ func TestGetPrettyDNForUsers(t *testing.T) {
 				LastName:  "last2",
 			},
 		}
-		assert.Equal(t, "first2 last2", getPrettyDNForUsers("nickname_full_name", users, "user1"))
+		assert.Equal(t, "first2 last2", getPrettyDNForUsers("nickname_full_name", users, "user1", map[string]string{}))
 	})
 
 	t.Run("full_name", func(t *testing.T) {
@@ -476,7 +476,7 @@ func TestGetPrettyDNForUsers(t *testing.T) {
 				LastName:  "last2",
 			},
 		}
-		assert.Equal(t, "first2 last2", getPrettyDNForUsers("full_name", users, "user1"))
+		assert.Equal(t, "first2 last2", getPrettyDNForUsers("full_name", users, "user1", map[string]string{}))
 
 		users = []*model.User{
 			{
@@ -488,7 +488,7 @@ func TestGetPrettyDNForUsers(t *testing.T) {
 				Username: "user2",
 			},
 		}
-		assert.Equal(t, "user2", getPrettyDNForUsers("full_name", users, "user1"))
+		assert.Equal(t, "user2", getPrettyDNForUsers("full_name", users, "user1", map[string]string{}))
 	})
 
 	t.Run("username", func(t *testing.T) {
@@ -508,8 +508,33 @@ func TestGetPrettyDNForUsers(t *testing.T) {
 				LastName:  "last2",
 			},
 		}
-		assert.Equal(t, "user2", getPrettyDNForUsers("username", users, "user1"))
+		assert.Equal(t, "user2", getPrettyDNForUsers("username", users, "user1", map[string]string{}))
 	})
+
+	t.Run("cache", func(t *testing.T) {
+		users := []*model.User{
+			{
+				Id:        "user1",
+				Nickname:  "nick1",
+				Username:  "user1",
+				FirstName: "first1",
+				LastName:  "last1",
+			},
+			{
+				Id:        "user2",
+				Nickname:  "nick2",
+				Username:  "user2",
+				FirstName: "first2",
+				LastName:  "last2",
+			},
+		}
+
+		cache := map[string]string{}
+		assert.Equal(t, "first2 last2", getPrettyDNForUsers("full_name", users, "user1", cache))
+		cache["user2"] = "teststring!!"
+		assert.Equal(t, "teststring!!", getPrettyDNForUsers("full_name", users, "user1", cache))
+	})
+
 }
 
 func TestChannelCursor(t *testing.T) {

api4/resolver_team.go

@@ -61,23 +61,23 @@ func getGraphQLTeams(c *web.Context, teamIDs []string) ([]*model.Team, error) {
 	}
 
 	if len(teams) != len(teamIDs) {
-		return nil, fmt.Errorf("All teams were not found. Requested %d; Found %d", len(teamIDs), len(teams))
+		return nil, fmt.Errorf("all teams were not found. Requested %d; Found %d", len(teamIDs), len(teams))
 	}
 
 	// We pre-calculate this so that it's not computed in separate goroutines outside
 	// the dataloader.
-	for i := range teams {
+	for i, team := range teams {
-		if (!teams[i].AllowOpenInvite || teams[i].Type != model.TeamOpen) &&
+		if (!team.AllowOpenInvite || team.Type != model.TeamOpen) &&
-			!c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), teams[i].Id, model.PermissionViewTeam) {
+			!c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), team.Id, model.PermissionViewTeam) {
 			c.SetPermissionError(model.PermissionViewTeam)
 			return nil, c.Err
 		}
 
-		teams[i] = c.App.SanitizeTeam(*c.AppContext.Session(), teams[i])
+		teams[i] = c.App.SanitizeTeam(*c.AppContext.Session(), team)
 	}
 
 	// The teams need to be in the exact same order as the input slice.
-	tmp := make(map[string]*model.Team)
+	tmp := make(map[string]*model.Team, len(teams))
 	for _, ch := range teams {
 		tmp[ch.Id] = ch
 	}