From 1bfd3a6a6ea04ead1d26e4d2cd70e5bd996673b0 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Tue, 20 May 2025 17:16:28 -0400 Subject: [PATCH] Slightly improve performance of sidebar update APIs (#31061) --- server/channels/api4/channel_category.go | 28 +++++++++-------- server/channels/api4/channel_category_test.go | 30 +++++++++---------- 2 files changed, 30 insertions(+), 28 deletions(-) diff --git a/server/channels/api4/channel_category.go b/server/channels/api4/channel_category.go index 500a516b4c8..517794dbc66 100644 --- a/server/channels/api4/channel_category.go +++ b/server/channels/api4/channel_category.go @@ -236,7 +236,7 @@ func validateSidebarCategory(c *Context, teamId, userId string, category *model. return model.NewAppError("validateSidebarCategory", "api.invalid_channel", nil, "", http.StatusBadRequest).Wrap(appErr) } - category.Channels = validateSidebarCategoryChannels(c, userId, category.Channels, channels) + category.Channels = validateSidebarCategoryChannels(c, userId, category.Channels, channelListToMap(channels)) return nil } @@ -250,28 +250,30 @@ func validateSidebarCategories(c *Context, teamId, userId string, categories []* return model.NewAppError("validateSidebarCategory", "api.invalid_channel", nil, "", http.StatusBadRequest).Wrap(err) } + channelMap := channelListToMap(channels) + for _, category := range categories { - category.Channels = validateSidebarCategoryChannels(c, userId, category.Channels, channels) + category.Channels = validateSidebarCategoryChannels(c, userId, category.Channels, channelMap) } return nil } +func channelListToMap(channelList model.ChannelList) map[string]*model.Channel { + channelMap := make(map[string]*model.Channel, len(channelList)) + for _, channel := range channelList { + channelMap[channel.Id] = channel + } + return channelMap +} + // validateSidebarCategoryChannels returns a normalized slice of channel IDs by removing duplicates from it and -// ensuring that it only contains IDs of channels in the given ChannelList. -func validateSidebarCategoryChannels(c *Context, userId string, channelIds []string, channels model.ChannelList) []string { +// ensuring that it only contains IDs of channels in the given map of Channels by IDs. +func validateSidebarCategoryChannels(c *Context, userId string, channelIds []string, channelMap map[string]*model.Channel) []string { var filtered []string for _, channelId := range channelIds { - found := false - for _, channel := range channels { - if channel.Id == channelId { - found = true - break - } - } - - if found { + if _, ok := channelMap[channelId]; ok { filtered = append(filtered, channelId) } else { c.Logger.Info("Stopping user from adding channel to their sidebar when they are not a member", mlog.String("user_id", userId), mlog.String("channel_id", channelId)) diff --git a/server/channels/api4/channel_category_test.go b/server/channels/api4/channel_category_test.go index da8bcb27234..5e94d6d8928 100644 --- a/server/channels/api4/channel_category_test.go +++ b/server/channels/api4/channel_category_test.go @@ -1007,11 +1007,11 @@ func TestValidateSidebarCategoryChannels(t *testing.T) { } t.Run("should filter valid channels", func(t *testing.T) { - // Create test channels - channels := model.ChannelList{ + // Create test channelMap + channelMap := channelListToMap(model.ChannelList{ th.BasicChannel, th.BasicChannel2, - } + }) // Test with valid channel IDs channelIds := []string{ @@ -1019,42 +1019,42 @@ func TestValidateSidebarCategoryChannels(t *testing.T) { th.BasicChannel2.Id, } - filtered := validateSidebarCategoryChannels(c, th.BasicUser.Id, channelIds, channels) + filtered := validateSidebarCategoryChannels(c, th.BasicUser.Id, channelIds, channelMap) require.Len(t, filtered, 2) require.ElementsMatch(t, channelIds, filtered) }) t.Run("should filter out invalid channels", func(t *testing.T) { - channels := model.ChannelList{ + channelMap := channelListToMap(model.ChannelList{ th.BasicChannel, - } + }) channelIds := []string{ th.BasicChannel.Id, "invalid_channel_id", } - filtered := validateSidebarCategoryChannels(c, th.BasicUser.Id, channelIds, channels) + filtered := validateSidebarCategoryChannels(c, th.BasicUser.Id, channelIds, channelMap) require.Len(t, filtered, 1) require.Contains(t, filtered, th.BasicChannel.Id) require.NotContains(t, filtered, "invalid_channel_id") }) t.Run("should handle empty channel list", func(t *testing.T) { - channels := model.ChannelList{} + channelMap := channelListToMap(model.ChannelList{}) channelIds := []string{th.BasicChannel.Id} - filtered := validateSidebarCategoryChannels(c, th.BasicUser.Id, channelIds, channels) + filtered := validateSidebarCategoryChannels(c, th.BasicUser.Id, channelIds, channelMap) require.Empty(t, filtered) }) t.Run("should handle empty channelIds", func(t *testing.T) { - channels := model.ChannelList{ + channelMap := channelListToMap(model.ChannelList{ th.BasicChannel, th.BasicChannel2, - } + }) - filtered := validateSidebarCategoryChannels(c, th.BasicUser.Id, []string{}, channels) + filtered := validateSidebarCategoryChannels(c, th.BasicUser.Id, []string{}, channelMap) require.Empty(t, filtered) }) @@ -1064,9 +1064,9 @@ func TestValidateSidebarCategoryChannels(t *testing.T) { }) t.Run("should prevent duplicate channel IDs", func(t *testing.T) { - channels := model.ChannelList{ + channelMap := channelListToMap(model.ChannelList{ th.BasicChannel, - } + }) // Include duplicate channel IDs channelIds := []string{ @@ -1074,7 +1074,7 @@ func TestValidateSidebarCategoryChannels(t *testing.T) { th.BasicChannel.Id, } - filtered := validateSidebarCategoryChannels(c, th.BasicUser.Id, channelIds, channels) + filtered := validateSidebarCategoryChannels(c, th.BasicUser.Id, channelIds, channelMap) require.Len(t, filtered, 1) require.Equal(t, []string{th.BasicChannel.Id}, filtered) })