[MM-24987] Dont call GetSchemeRolesForChannel and instead load the scheme from channel (#14514)

* MM-24987 Dont call a.GetSchemeRolesForChannel and instead load the scheme using the channel directly

* Trigger CI

* MM-24987 Add a test for concurrent patch to channel moderation

* MM-24987 Cleanup
This commit is contained in:
Farhan Munshi
2020-05-08 17:50:45 -04:00
committed by GitHub
parent 8fe003876d
commit fff93cb73f
2 changed files with 61 additions and 8 deletions

View File

@@ -494,8 +494,10 @@ func (a *App) CreateChannelScheme(channel *model.Channel) (*model.Scheme, *model
// DeleteChannelScheme deletes a channels scheme and sets its SchemeId to nil.
func (a *App) DeleteChannelScheme(channel *model.Channel) (*model.Channel, *model.AppError) {
if _, err := a.DeleteScheme(*channel.SchemeId); err != nil {
return nil, err
if channel.SchemeId != nil && len(*channel.SchemeId) != 0 {
if _, err := a.DeleteScheme(*channel.SchemeId); err != nil {
return nil, err
}
}
channel.SchemeId = nil
return a.UpdateChannelScheme(channel)
@@ -760,22 +762,26 @@ func (a *App) PatchChannelModerationsForChannel(channel *model.Channel, channelM
}
}
var scheme *model.Scheme
// Channel has no scheme so create one
if channel.SchemeId == nil || len(*channel.SchemeId) == 0 {
if _, err = a.CreateChannelScheme(channel); err != nil {
scheme, err = a.CreateChannelScheme(channel)
if err != nil {
return nil, err
}
message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_CHANNEL_SCHEME_UPDATED, "", channel.Id, "", nil)
a.Publish(message)
mlog.Info("Permission scheme created.", mlog.String("channel_id", channel.Id), mlog.String("channel_name", channel.Name))
} else {
scheme, err = a.GetScheme(*channel.SchemeId)
if err != nil {
return nil, err
}
}
guestRoleName, memberRoleName, _, err := a.GetSchemeRolesForChannel(channel.Id)
if err != nil {
return nil, err
}
guestRoleName := scheme.DefaultChannelGuestRole
memberRoleName := scheme.DefaultChannelUserRole
memberRole, err := a.GetRoleByName(memberRoleName)
if err != nil {
return nil, err

View File

@@ -8,6 +8,7 @@ import (
"net/http"
"sort"
"strings"
"sync"
"testing"
"github.com/stretchr/testify/assert"
@@ -1657,6 +1658,52 @@ func TestPatchChannelModerationsForChannel(t *testing.T) {
}
})
}
t.Run("Handles concurrent patch requests gracefully", func(t *testing.T) {
addCreatePosts := []*model.ChannelModerationPatch{
{
Name: &createPosts,
Roles: &model.ChannelModeratedRolesPatch{
Members: model.NewBool(false),
Guests: model.NewBool(false),
},
},
}
removeCreatePosts := []*model.ChannelModerationPatch{
{
Name: &createPosts,
Roles: &model.ChannelModeratedRolesPatch{
Members: model.NewBool(false),
Guests: model.NewBool(false),
},
},
}
wg := sync.WaitGroup{}
wg.Add(20)
for i := 0; i < 10; i++ {
go func() {
th.App.PatchChannelModerationsForChannel(channel, addCreatePosts)
th.App.PatchChannelModerationsForChannel(channel, removeCreatePosts)
wg.Done()
}()
}
for i := 0; i < 10; i++ {
go func() {
th.App.PatchChannelModerationsForChannel(channel, addCreatePosts)
th.App.PatchChannelModerationsForChannel(channel, removeCreatePosts)
wg.Done()
}()
}
wg.Wait()
higherScopedGuestRoleName, higherScopedMemberRoleName, _, _ := th.App.GetTeamSchemeChannelRoles(channel.TeamId)
higherScopedMemberRole, _ := th.App.GetRoleByName(higherScopedMemberRoleName)
higherScopedGuestRole, _ := th.App.GetRoleByName(higherScopedGuestRoleName)
assert.Contains(t, higherScopedMemberRole.Permissions, createPosts)
assert.Contains(t, higherScopedGuestRole.Permissions, createPosts)
})
}
// TestMarkChannelsAsViewedPanic verifies that returning an error from a.GetUser