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
Этот коммит содержится в:
Jesse Hallam
2021-10-28 11:08:09 -03:00
коммит произвёл GitHub
родитель f621989c58
Коммит c1a27d8360
2 изменённых файлов: 52 добавлений и 5 удалений

Просмотреть файл

@@ -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

Просмотреть файл

@@ -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))