From 32037706b545b009b555eacefc2e15fa65aa484d Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Mon, 17 Feb 2025 19:39:39 +0530 Subject: [PATCH] MM-62960: Support both webhub iteration scopes properly (#30224) When Channel iteration mode is enabled, we need to ensure that channel scoped events do not fall through to the connIndex.All() condition. This is possible because there are multiple hubs in a given system. So a single event will flow through all of them and in some hubs, a channel scoped event might not have any connections. In that case, we need to stop processing further. https://mattermost.atlassian.net/browse/MM-62960 ```release-note NONE ``` --- server/channels/api4/post_test.go | 21 ++++++++++++++++++-- server/channels/app/platform/web_hub.go | 8 ++++++++ server/channels/app/platform/web_hub_test.go | 6 +++++- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/server/channels/api4/post_test.go b/server/channels/api4/post_test.go index 715d432592..2ba7c0a5ae 100644 --- a/server/channels/api4/post_test.go +++ b/server/channels/api4/post_test.go @@ -3354,8 +3354,25 @@ func TestPermanentDeletePost(t *testing.T) { } func TestWebHubMembership(t *testing.T) { - th := Setup(t).InitBasic() - defer th.TearDown() + t.Run("WithChannelIteration", func(t *testing.T) { + th := SetupConfig(t, func(cfg *model.Config) { + *cfg.ServiceSettings.EnableWebHubChannelIteration = true + }).InitBasic() + defer th.TearDown() + + _testWebHubMembership(th, t) + }) + + t.Run("WithoutChannelIteration", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + _testWebHubMembership(th, t) + }) +} + +func _testWebHubMembership(th *TestHelper, t *testing.T) { + t.Helper() u1 := th.CreateUser() th.LinkUserToTeam(u1, th.BasicTeam) diff --git a/server/channels/app/platform/web_hub.go b/server/channels/app/platform/web_hub.go index 6afafd87c2..948272f42f 100644 --- a/server/channels/app/platform/web_hub.go +++ b/server/channels/app/platform/web_hub.go @@ -682,6 +682,14 @@ func (h *Hub) Start() { continue } + // There are multiple hubs in a system. So while supporting both channel based iteration and the old + // method, there would be events scoped to a channel being sent to multiple hubs. And only one hub would + // have the targetConns. Therefore, we need to stop here if channel based iteration is enabled, and it's a + // channel-scoped event. + if channelID := msg.GetBroadcast().ChannelId; channelID != "" && *h.platform.Config().ServiceSettings.EnableWebHubChannelIteration { + continue + } + for webConn := range connIndex.All() { broadcast(webConn) } diff --git a/server/channels/app/platform/web_hub_test.go b/server/channels/app/platform/web_hub_test.go index 74fea05dbf..bd931646db 100644 --- a/server/channels/app/platform/web_hub_test.go +++ b/server/channels/app/platform/web_hub_test.go @@ -386,7 +386,11 @@ func TestHubConnIndex(t *testing.T) { t.Run("ForChannel", func(t *testing.T) { require.Len(t, connIndex.byChannelID, 1) - require.Equal(t, []*WebConn{wc1, wc2, wc3}, connIndex.ForChannel(th.BasicChannel.Id)) + ids := make([]string, 0) + for _, c := range connIndex.ForChannel(th.BasicChannel.Id) { + ids = append(ids, c.GetConnectionID()) + } + require.ElementsMatch(t, []string{wc1ID, wc2ID, wc3ID}, ids) require.Len(t, connIndex.ForChannel("notexist"), 0) })