From c1a27d8360b32096b112a46db48e9637aca5d493 Mon Sep 17 00:00:00 2001
From: Jesse Hallam <jesse.hallam@gmail.com>
Date: Thu, 28 Oct 2021 11:08:09 -0300
Subject: [PATCH] MM-39524: fix WebConn caching stale channel members (#18840)

When configured in a master/slave database environment, a read replica can sometimes return stale data to a `WebConn`, resulting in the user missing out on websocket events targetting that channel until the `WebConn` cache expires.

This is most easily reproducible by using Playbooks on community and starting a new run. The owner, or any automatically invited participants, typically find the websocket events dropped in that channel for up to 30 minutes, even after multiple page refreshes.

I've reproduced this locally, and while I've extended the unit tests, I note that they don't actually exercise this case given the need for a dedicated slave database during unit tests.

Fixes: https://mattermost.atlassian.net/browse/MM-39524
---
 app/web_conn.go      |  2 +-
 app/web_conn_test.go | 55 ++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/app/web_conn.go b/app/web_conn.go
index 6d58ce9a85..2c30f50387 100644
--- a/app/web_conn.go
+++ b/app/web_conn.go
@@ -728,7 +728,7 @@ func (wc *WebConn) shouldSendEvent(msg *model.WebSocketEvent) bool {
 		}
 
 		if wc.allChannelMembers == nil {
-			result, err := wc.App.Srv().Store.Channel().GetAllChannelMembersForUser(wc.UserId, true, false)
+			result, err := wc.App.Srv().Store.Channel().GetAllChannelMembersForUser(wc.UserId, false, false)
 			if err != nil {
 				mlog.Error("webhub.shouldSendEvent.", mlog.Err(err))
 				return false
diff --git a/app/web_conn_test.go b/app/web_conn_test.go
index 89bee5b1b5..fa4d4b5a34 100644
--- a/app/web_conn_test.go
+++ b/app/web_conn_test.go
@@ -72,6 +72,13 @@ func TestWebConnShouldSendEvent(t *testing.T) {
 	adminUserWc.SetSessionToken(session3.Token)
 	adminUserWc.SetSessionExpiresAt(session3.ExpiresAt)
 
+	// By default, only BasicUser and BasicUser2 get added to the BasicTeam.
+	th.LinkUserToTeam(th.SystemAdminUser, th.BasicTeam)
+
+	// Create another channel with just BasicUser (implicitly) and SystemAdminUser to test channel broadcast
+	channel2 := th.CreateChannel(th.BasicTeam)
+	th.AddUserToChannel(th.SystemAdminUser, channel2)
+
 	cases := []struct {
 		Description   string
 		Broadcast     *model.WebsocketBroadcast
@@ -90,12 +97,52 @@ func TestWebConnShouldSendEvent(t *testing.T) {
 
 	event := model.NewWebSocketEvent("some_event", "", "", "", nil)
 	for _, c := range cases {
-		event = event.SetBroadcast(c.Broadcast)
-		assert.Equal(t, c.User1Expected, basicUserWc.shouldSendEvent(event), c.Description)
-		assert.Equal(t, c.User2Expected, basicUser2Wc.shouldSendEvent(event), c.Description)
-		assert.Equal(t, c.AdminExpected, adminUserWc.shouldSendEvent(event), c.Description)
+		t.Run(c.Description, func(t *testing.T) {
+			event = event.SetBroadcast(c.Broadcast)
+			if c.User1Expected {
+				assert.True(t, basicUserWc.shouldSendEvent(event), "expected user 1")
+			} else {
+				assert.False(t, basicUserWc.shouldSendEvent(event), "did not expect user 1")
+			}
+			if c.User2Expected {
+				assert.True(t, basicUser2Wc.shouldSendEvent(event), "expected user 2")
+			} else {
+				assert.False(t, basicUser2Wc.shouldSendEvent(event), "did not expect user 2")
+			}
+			if c.AdminExpected {
+				assert.True(t, adminUserWc.shouldSendEvent(event), "expected admin")
+			} else {
+				assert.False(t, adminUserWc.shouldSendEvent(event), "did not expect admin")
+			}
+		})
 	}
 
+	t.Run("should send to basic user in basic channel", func(t *testing.T) {
+		event = event.SetBroadcast(&model.WebsocketBroadcast{ChannelId: th.BasicChannel.Id})
+
+		assert.True(t, basicUserWc.shouldSendEvent(event), "expected user 1")
+		assert.False(t, basicUser2Wc.shouldSendEvent(event), "did not expect user 2")
+		assert.False(t, adminUserWc.shouldSendEvent(event), "did not expect admin")
+	})
+
+	t.Run("should send to basic user and admin in channel2", func(t *testing.T) {
+		event = event.SetBroadcast(&model.WebsocketBroadcast{ChannelId: channel2.Id})
+
+		assert.True(t, basicUserWc.shouldSendEvent(event), "expected user 1")
+		assert.False(t, basicUser2Wc.shouldSendEvent(event), "did not expect user 2")
+		assert.True(t, adminUserWc.shouldSendEvent(event), "expected admin")
+	})
+
+	t.Run("channel member cache invalidated after user added to channel", func(t *testing.T) {
+		th.AddUserToChannel(th.BasicUser2, channel2)
+		basicUser2Wc.InvalidateCache()
+
+		event = event.SetBroadcast(&model.WebsocketBroadcast{ChannelId: channel2.Id})
+		assert.True(t, basicUserWc.shouldSendEvent(event), "expected user 1")
+		assert.True(t, basicUser2Wc.shouldSendEvent(event), "expected user 2")
+		assert.True(t, adminUserWc.shouldSendEvent(event), "expected admin")
+	})
+
 	event2 := model.NewWebSocketEvent(model.WebsocketEventUpdateTeam, th.BasicTeam.Id, "", "", nil)
 	assert.True(t, basicUserWc.shouldSendEvent(event2))
 	assert.True(t, basicUser2Wc.shouldSendEvent(event2))