From cae59d327c8c79a77236c131da528daf74eb4a0e Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 12 Nov 2020 15:26:35 +0530 Subject: [PATCH] MM-29486: Fix racy test TestPatchChannelModeration (#16256) * MM-29486: Fix racy test TestPatchChannelModeration - We avoid appending to the slice in `(r *Role) IsValidWithoutId` by just iterating the 2 slices separately. - We pass deep copies of channels to prevent racy modification. https://mattermost.atlassian.net/browse/MM-29486 ```release-note NONE ``` * fix lint * simplify perm check * improve more Co-authored-by: Mattermod --- app/channel_test.go | 8 ++++---- model/role.go | 13 +++++++------ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/app/channel_test.go b/app/channel_test.go index 0ebf6a8e0f..2850323a42 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -1828,15 +1828,15 @@ func TestPatchChannelModerationsForChannel(t *testing.T) { wg.Add(20) for i := 0; i < 10; i++ { go func() { - th.App.PatchChannelModerationsForChannel(channel, addCreatePosts) - th.App.PatchChannelModerationsForChannel(channel, removeCreatePosts) + th.App.PatchChannelModerationsForChannel(channel.DeepCopy(), addCreatePosts) + th.App.PatchChannelModerationsForChannel(channel.DeepCopy(), removeCreatePosts) wg.Done() }() } for i := 0; i < 10; i++ { go func() { - th.App.PatchChannelModerationsForChannel(channel, addCreatePosts) - th.App.PatchChannelModerationsForChannel(channel, removeCreatePosts) + th.App.PatchChannelModerationsForChannel(channel.DeepCopy(), addCreatePosts) + th.App.PatchChannelModerationsForChannel(channel.DeepCopy(), removeCreatePosts) wg.Done() }() } diff --git a/model/role.go b/model/role.go index e880a1d844..fe3bdb0a3c 100644 --- a/model/role.go +++ b/model/role.go @@ -482,15 +482,16 @@ func (r *Role) IsValidWithoutId() bool { return false } - for _, permission := range r.Permissions { - permissionValidated := false - for _, p := range append(AllPermissions, DeprecatedPermissions...) { + check := func(perms []*Permission, permission string) bool { + for _, p := range perms { if permission == p.Id { - permissionValidated = true - break + return true } } - + return false + } + for _, permission := range r.Permissions { + permissionValidated := check(AllPermissions, permission) || check(DeprecatedPermissions, permission) if !permissionValidated { return false }