From 5e076ea36fdfc77de8247dabebffdb4da668d8de Mon Sep 17 00:00:00 2001 From: Martin Kraft Date: Fri, 26 Mar 2021 14:09:32 -0400 Subject: [PATCH] =?UTF-8?q?MM-33402:=20Invalidated=20the=20LRU=20cache=20f?= =?UTF-8?q?or=20channel=20members=20after=20moderat=E2=80=A6=20(#17171)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * MM-33402: Invalidated the LRU cache for channel members after moderation changes. * MM-33402: Handles cache invalidation error. * Revert "MM-33402: Handles cache invalidation error." This reverts commit 2d30d24658321bbaa32ebfc181dbdade685fd23d. * MM-33402: Un-exports method. * MM-33402: Removes log warning from previous code use. * MM-33402: Handles possible (store.ChannelStore).GetMembers error. Co-authored-by: Mattermod --- app/channel.go | 36 ++++++++++++++++++++++++++++-------- app/channel_test.go | 31 +++++++++++++++++++++++++++++++ i18n/en.json | 4 ++++ 3 files changed, 63 insertions(+), 8 deletions(-) diff --git a/app/channel.go b/app/channel.go index 93b55e2641..dd2397c843 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1029,6 +1029,14 @@ func (a *App) PatchChannelModerationsForChannel(channel *model.Channel, channelM } } + cErr := a.forEachChannelMember(channel.Id, func(channelMember model.ChannelMember) error { + a.Srv().Store.Channel().InvalidateAllChannelMembersForUser(channelMember.UserId) + return nil + }) + if cErr != nil { + return nil, model.NewAppError("PatchChannelModerationsForChannel", "api.channel.patch_channel_moderations.cache_invalidation.error", nil, cErr.Error(), http.StatusInternalServerError) + } + return buildChannelModerations(channel.Type, memberRole, guestRole, higherScopedMemberRole, higherScopedGuestRole), nil } @@ -2981,23 +2989,20 @@ func (a *App) FillInChannelsProps(channelList *model.ChannelList) *model.AppErro return nil } -func (a *App) ClearChannelMembersCache(channelID string) { +func (a *App) forEachChannelMember(channelID string, f func(model.ChannelMember) error) error { perPage := 100 page := 0 for { channelMembers, err := a.Srv().Store.Channel().GetMembers(channelID, page*perPage, perPage) if err != nil { - a.Log().Warn("error clearing cache for channel members", mlog.String("channel_id", channelID)) - break + return err } for _, channelMember := range *channelMembers { - a.ClearSessionCacheForUser(channelMember.UserId) - - message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_CHANNEL_MEMBER_UPDATED, "", "", channelMember.UserId, nil) - message.Add("channelMember", channelMember.ToJson()) - a.Publish(message) + if err = f(channelMember); err != nil { + return err + } } length := len(*(channelMembers)) @@ -3007,6 +3012,21 @@ func (a *App) ClearChannelMembersCache(channelID string) { page++ } + + return nil +} + +func (a *App) ClearChannelMembersCache(channelID string) { + clearSessionCache := func(channelMember model.ChannelMember) error { + a.ClearSessionCacheForUser(channelMember.UserId) + message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_CHANNEL_MEMBER_UPDATED, "", "", channelMember.UserId, nil) + message.Add("channelMember", channelMember.ToJson()) + a.Publish(message) + return nil + } + if err := a.forEachChannelMember(channelID, clearSessionCache); err != nil { + a.Log().Warn("error clearing cache for channel members", mlog.String("channel_id", channelID)) + } } func (a *App) GetMemberCountsByGroup(channelID string, includeTimezones bool) ([]*model.ChannelMemberCountByGroup, *model.AppError) { diff --git a/app/channel_test.go b/app/channel_test.go index f1a5828835..2f60630bec 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -1517,6 +1517,9 @@ func TestPatchChannelModerationsForChannel(t *testing.T) { th.App.SetPhase2PermissionsMigrationStatus(true) channel := th.BasicChannel + user := th.BasicUser + th.AddUserToChannel(user, channel) + createPosts := model.ChannelModeratedPermissions[0] createReactions := model.ChannelModeratedPermissions[1] manageMembers := model.ChannelModeratedPermissions[2] @@ -1901,6 +1904,34 @@ func TestPatchChannelModerationsForChannel(t *testing.T) { assert.Contains(t, higherScopedGuestRole.Permissions, createPosts) }) + t.Run("Updates the authorization to create post", func(t *testing.T) { + addCreatePosts := []*model.ChannelModerationPatch{ + { + Name: &createPosts, + Roles: &model.ChannelModeratedRolesPatch{ + Members: model.NewBool(true), + }, + }, + } + removeCreatePosts := []*model.ChannelModerationPatch{ + { + Name: &createPosts, + Roles: &model.ChannelModeratedRolesPatch{ + Members: model.NewBool(false), + }, + }, + } + + mockSession := model.Session{UserId: user.Id} + + _, err := th.App.PatchChannelModerationsForChannel(channel.DeepCopy(), addCreatePosts) + require.Nil(t, err) + require.True(t, th.App.SessionHasPermissionToChannel(mockSession, channel.Id, model.PERMISSION_CREATE_POST)) + + _, err = th.App.PatchChannelModerationsForChannel(channel.DeepCopy(), removeCreatePosts) + require.Nil(t, err) + require.False(t, th.App.SessionHasPermissionToChannel(mockSession, channel.Id, model.PERMISSION_CREATE_POST)) + }) } // TestMarkChannelsAsViewedPanic verifies that returning an error from a.GetUser diff --git a/i18n/en.json b/i18n/en.json index 66232c579f..a681f198f8 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -343,6 +343,10 @@ "id": "api.channel.move_channel.type.invalid", "translation": "Unable to move direct or group message channels" }, + { + "id": "api.channel.patch_channel_moderations.cache_invalidation.error", + "translation": "Error invalidating cache" + }, { "id": "api.channel.patch_channel_moderations.license.error", "translation": "Your license does not support channel moderation"