diff --git a/api4/command_test.go b/api4/command_test.go index dcd1b2fdf5..5c58d0ea1f 100644 --- a/api4/command_test.go +++ b/api4/command_test.go @@ -616,7 +616,6 @@ func TestExecuteGetCommand(t *testing.T) { assert.True(t, len(commandResponse.TriggerId) == 26) expectedCommandResponse.TriggerId = commandResponse.TriggerId - expectedCommandResponse.Props["from_webhook"] = "true" require.Equal(t, expectedCommandResponse, commandResponse) } @@ -675,9 +674,7 @@ func TestExecutePostCommand(t *testing.T) { assert.True(t, len(commandResponse.TriggerId) == 26) expectedCommandResponse.TriggerId = commandResponse.TriggerId - expectedCommandResponse.Props["from_webhook"] = "true" require.Equal(t, expectedCommandResponse, commandResponse) - } func TestExecuteCommandAgainstChannelOnAnotherTeam(t *testing.T) { diff --git a/api4/post_test.go b/api4/post_test.go index a90f797102..f724531f08 100644 --- a/api4/post_test.go +++ b/api4/post_test.go @@ -38,7 +38,7 @@ func TestCreatePost(t *testing.T) { require.Equal(t, "#hashtag", rpost.Hashtags, "hashtag didn't match") require.Empty(t, rpost.FileIds) require.Equal(t, 0, int(rpost.EditAt), "newly created post shouldn't have EditAt set") - require.Nil(t, rpost.Props[model.PROPS_ADD_CHANNEL_MEMBER], "newly created post shouldn't have Props['add_channel_member'] set") + require.Nil(t, rpost.GetProp(model.PROPS_ADD_CHANNEL_MEMBER), "newly created post shouldn't have Props['add_channel_member'] set") post.RootId = rpost.Id post.ParentId = rpost.Id @@ -574,7 +574,7 @@ func TestCreatePostSendOutOfChannelMentions(t *testing.T) { wpost := model.PostFromJson(strings.NewReader(event.GetData()["post"].(string))) - acm, ok := wpost.Props[model.PROPS_ADD_CHANNEL_MEMBER].(map[string]interface{}) + acm, ok := wpost.GetProp(model.PROPS_ADD_CHANNEL_MEMBER).(map[string]interface{}) require.True(t, ok, "should have received ephemeral post with 'add_channel_member' in props") require.True(t, acm["post_id"] != nil, "should not be nil") require.True(t, acm["user_ids"] != nil, "should not be nil") @@ -677,20 +677,20 @@ func TestUpdatePost(t *testing.T) { t.Run("new message, invalid props", func(t *testing.T) { msg1 := "#hashtag a" + model.NewId() + " update post again" rpost.Message = msg1 - rpost.Props[model.PROPS_ADD_CHANNEL_MEMBER] = "no good" + rpost.AddProp(model.PROPS_ADD_CHANNEL_MEMBER, "no good") rrupost, resp := Client.UpdatePost(rpost.Id, rpost) CheckNoError(t, resp) assert.Equal(t, msg1, rrupost.Message, "failed to update message") assert.Equal(t, "#hashtag", rrupost.Hashtags, "failed to update hashtags") - assert.Nil(t, rrupost.Props[model.PROPS_ADD_CHANNEL_MEMBER], "failed to sanitize Props['add_channel_member'], should be nil") + assert.Nil(t, rrupost.GetProp(model.PROPS_ADD_CHANNEL_MEMBER), "failed to sanitize Props['add_channel_member'], should be nil") actual, resp := Client.GetPost(rpost.Id, "") CheckNoError(t, resp) assert.Equal(t, msg1, actual.Message, "failed to update message") assert.Equal(t, "#hashtag", actual.Hashtags, "failed to update hashtags") - assert.Nil(t, actual.Props[model.PROPS_ADD_CHANNEL_MEMBER], "failed to sanitize Props['add_channel_member'], should be nil") + assert.Nil(t, actual.GetProp(model.PROPS_ADD_CHANNEL_MEMBER), "failed to sanitize Props['add_channel_member'], should be nil") }) t.Run("join/leave post", func(t *testing.T) { @@ -848,7 +848,7 @@ func TestPatchPost(t *testing.T) { assert.False(t, rpost.IsPinned, "IsPinned did not update properly") assert.Equal(t, "#otherhashtag other message", rpost.Message, "Message did not update properly") - assert.Equal(t, *patch.Props, rpost.Props, "Props did not update properly") + assert.Equal(t, *patch.Props, rpost.GetProps(), "Props did not update properly") assert.Equal(t, "#otherhashtag", rpost.Hashtags, "Message did not update properly") assert.Equal(t, model.StringArray(fileIds[0:2]), rpost.FileIds, "FileIds should not update") assert.False(t, rpost.HasReactions, "HasReactions did not update properly") @@ -865,7 +865,7 @@ func TestPatchPost(t *testing.T) { rpost2, resp := Client.PatchPost(post.Id, patch2) CheckNoError(t, resp) - assert.NotEmpty(t, rpost2.Props["attachments"]) + assert.NotEmpty(t, rpost2.GetProp("attachments")) assert.NotEqual(t, rpost.EditAt, rpost2.EditAt) }) diff --git a/app/channel_test.go b/app/channel_test.go index 0b6c1dfc1a..337bd47f9c 100644 --- a/app/channel_test.go +++ b/app/channel_test.go @@ -489,7 +489,7 @@ func TestAddChannelMemberNoUserRequestor(t *testing.T) { assert.Equal(t, model.POST_JOIN_CHANNEL, post.Type) assert.Equal(t, user.Id, post.UserId) - assert.Equal(t, user.Username, post.Props["username"]) + assert.Equal(t, user.Username, post.GetProp("username")) } } diff --git a/app/command.go b/app/command.go index b6406a6038..8f65843e2e 100644 --- a/app/command.go +++ b/app/command.go @@ -520,7 +520,7 @@ func (a *App) HandleCommandResponsePost(command *model.Command, args *model.Comm post.ParentId = args.ParentId post.UserId = args.UserId post.Type = response.Type - post.Props = response.Props + post.SetProps(response.Props) if len(response.ChannelId) != 0 { _, err := a.GetChannelMember(response.ChannelId, args.UserId) diff --git a/app/command_test.go b/app/command_test.go index 4b7d23ee26..eae7bae5e6 100644 --- a/app/command_test.go +++ b/app/command_test.go @@ -104,16 +104,16 @@ func TestHandleCommandResponsePost(t *testing.T) { assert.Equal(t, args.ParentId, post.ParentId) assert.Equal(t, args.UserId, post.UserId) assert.Equal(t, resp.Type, post.Type) - assert.Equal(t, resp.Props, post.Props) + assert.Equal(t, resp.Props, post.GetProps()) assert.Equal(t, resp.Text, post.Message) - assert.Nil(t, post.Props["override_icon_url"]) - assert.Nil(t, post.Props["override_username"]) - assert.Nil(t, post.Props["from_webhook"]) + assert.Nil(t, post.GetProp("override_icon_url")) + assert.Nil(t, post.GetProp("override_username")) + assert.Nil(t, post.GetProp("from_webhook")) // Command is not built in, so it is a bot command. builtIn = false post, err = th.App.HandleCommandResponsePost(command, args, resp, builtIn) - assert.Equal(t, "true", post.Props["from_webhook"]) + assert.Equal(t, "true", post.GetProp("from_webhook")) builtIn = true @@ -135,23 +135,23 @@ func TestHandleCommandResponsePost(t *testing.T) { post, err = th.App.HandleCommandResponsePost(command, args, resp, builtIn) assert.Nil(t, err) - assert.Nil(t, post.Props["override_username"]) + assert.Nil(t, post.GetProp("override_username")) *th.App.Config().ServiceSettings.EnablePostUsernameOverride = true // Override username config is turned on. Override username through command property. post, err = th.App.HandleCommandResponsePost(command, args, resp, builtIn) assert.Nil(t, err) - assert.Equal(t, command.Username, post.Props["override_username"]) - assert.Equal(t, "true", post.Props["from_webhook"]) + assert.Equal(t, command.Username, post.GetProp("override_username")) + assert.Equal(t, "true", post.GetProp("from_webhook")) command.Username = "" // Override username through response property. post, err = th.App.HandleCommandResponsePost(command, args, resp, builtIn) assert.Nil(t, err) - assert.Equal(t, resp.Username, post.Props["override_username"]) - assert.Equal(t, "true", post.Props["from_webhook"]) + assert.Equal(t, resp.Username, post.GetProp("override_username")) + assert.Equal(t, "true", post.GetProp("from_webhook")) *th.App.Config().ServiceSettings.EnablePostUsernameOverride = false @@ -162,23 +162,23 @@ func TestHandleCommandResponsePost(t *testing.T) { post, err = th.App.HandleCommandResponsePost(command, args, resp, builtIn) assert.Nil(t, err) - assert.Nil(t, post.Props["override_icon_url"]) + assert.Nil(t, post.GetProp("override_icon_url")) *th.App.Config().ServiceSettings.EnablePostIconOverride = true // Override icon url config is turned on. Override icon url through command property. post, err = th.App.HandleCommandResponsePost(command, args, resp, builtIn) assert.Nil(t, err) - assert.Equal(t, command.IconURL, post.Props["override_icon_url"]) - assert.Equal(t, "true", post.Props["from_webhook"]) + assert.Equal(t, command.IconURL, post.GetProp("override_icon_url")) + assert.Equal(t, "true", post.GetProp("from_webhook")) command.IconURL = "" // Override icon url through response property. post, err = th.App.HandleCommandResponsePost(command, args, resp, builtIn) assert.Nil(t, err) - assert.Equal(t, resp.IconURL, post.Props["override_icon_url"]) - assert.Equal(t, "true", post.Props["from_webhook"]) + assert.Equal(t, resp.IconURL, post.GetProp("override_icon_url")) + assert.Equal(t, "true", post.GetProp("from_webhook")) // Test Slack text conversion. resp.Text = "" @@ -186,7 +186,7 @@ func TestHandleCommandResponsePost(t *testing.T) { post, err = th.App.HandleCommandResponsePost(command, args, resp, builtIn) assert.Nil(t, err) assert.Equal(t, "@channel", post.Message) - assert.Equal(t, "true", post.Props["from_webhook"]) + assert.Equal(t, "true", post.GetProp("from_webhook")) // Test Slack attachments text conversion. resp.Attachments = []*model.SlackAttachment{ @@ -201,7 +201,7 @@ func TestHandleCommandResponsePost(t *testing.T) { if assert.Len(t, post.Attachments(), 1) { assert.Equal(t, "@here", post.Attachments()[0].Text) } - assert.Equal(t, "true", post.Props["from_webhook"]) + assert.Equal(t, "true", post.GetProp("from_webhook")) channel = th.CreatePrivateChannel(th.BasicTeam) resp.ChannelId = channel.Id diff --git a/app/file.go b/app/file.go index 27d5ed3c45..335b84bbe7 100644 --- a/app/file.go +++ b/app/file.go @@ -331,7 +331,7 @@ func (a *App) MigrateFilenamesToFileInfos(post *model.Post) []*model.FileInfo { // Copy and save the updated post newPost := &model.Post{} - *newPost = *post + newPost = post.Clone() newPost.Filenames = []string{} newPost.FileIds = fileIds diff --git a/app/integration_action.go b/app/integration_action.go index 859da6c59f..a19cc589d5 100644 --- a/app/integration_action.go +++ b/app/integration_action.go @@ -138,14 +138,14 @@ func (a *App) DoPostActionWithCookie(postId, actionId, userId, selectedOption st // Save the original values that may need to be preserved (including selected // Props, i.e. override_username, override_icon_url) for _, key := range model.PostActionRetainPropKeys { - value, ok := post.Props[key] + value, ok := post.GetProps()[key] if ok { retain[key] = value } else { remove = append(remove, key) } } - originalProps = post.Props + originalProps = post.GetProps() originalIsPinned = post.IsPinned originalHasReactions = post.HasReactions @@ -219,14 +219,14 @@ func (a *App) DoPostActionWithCookie(postId, actionId, userId, selectedOption st response.Update.Id = postId // Restore the post attributes and Props that need to be preserved - if response.Update.Props == nil { - response.Update.Props = originalProps + if response.Update.GetProps() == nil { + response.Update.SetProps(originalProps) } else { for key, value := range retain { response.Update.AddProp(key, value) } for _, key := range remove { - delete(response.Update.Props, key) + response.Update.DelProp(key) } } response.Update.IsPinned = originalIsPinned diff --git a/app/integration_action_test.go b/app/integration_action_test.go index c96237beac..500f84df41 100644 --- a/app/integration_action_test.go +++ b/app/integration_action_test.go @@ -57,7 +57,7 @@ func TestPostActionInvalidURL(t *testing.T) { post, err := th.App.CreatePostAsUser(&interactivePost, "") require.Nil(t, err) - attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + attachments, ok := post.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) @@ -158,7 +158,7 @@ func TestPostAction(t *testing.T) { post, err := th.App.CreatePostAsUser(&interactivePost, "") require.Nil(t, err) - attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + attachments, ok := post.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) require.NotEmpty(t, attachments[0].Actions) @@ -195,7 +195,7 @@ func TestPostAction(t *testing.T) { post2, err := th.App.CreatePostAsUser(&menuPost, "") require.Nil(t, err) - attachments2, ok := post2.Props["attachments"].([]*model.SlackAttachment) + attachments2, ok := post2.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) require.NotEmpty(t, attachments2[0].Actions) @@ -253,7 +253,7 @@ func TestPostAction(t *testing.T) { postplugin, err := th.App.CreatePostAsUser(&interactivePostPlugin, "") require.Nil(t, err) - attachmentsPlugin, ok := postplugin.Props["attachments"].([]*model.SlackAttachment) + attachmentsPlugin, ok := postplugin.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) _, err = th.App.DoPostAction(postplugin.Id, attachmentsPlugin[0].Actions[0].Id, th.BasicUser.Id, "") @@ -294,7 +294,7 @@ func TestPostAction(t *testing.T) { postSiteURL, err := th.App.CreatePostAsUser(&interactivePostSiteURL, "") require.Nil(t, err) - attachmentsSiteURL, ok := postSiteURL.Props["attachments"].([]*model.SlackAttachment) + attachmentsSiteURL, ok := postSiteURL.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) _, err = th.App.DoPostAction(postSiteURL.Id, attachmentsSiteURL[0].Actions[0].Id, th.BasicUser.Id, "") @@ -336,7 +336,7 @@ func TestPostAction(t *testing.T) { postSubpath, err := th.App.CreatePostAsUser(&interactivePostSubpath, "") require.Nil(t, err) - attachmentsSubpath, ok := postSubpath.Props["attachments"].([]*model.SlackAttachment) + attachmentsSubpath, ok := postSubpath.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) _, err = th.App.DoPostAction(postSubpath.Id, attachmentsSubpath[0].Actions[0].Id, th.BasicUser.Id, "") @@ -410,7 +410,7 @@ func TestPostActionProps(t *testing.T) { post, err := th.App.CreatePostAsUser(&interactivePost, "") require.Nil(t, err) - attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + attachments, ok := post.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) clientTriggerId, err := th.App.DoPostAction(post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") @@ -422,11 +422,11 @@ func TestPostActionProps(t *testing.T) { assert.True(t, newPost.IsPinned) assert.False(t, newPost.HasReactions) - assert.Nil(t, newPost.Props["B"]) - assert.Nil(t, newPost.Props["override_username"]) - assert.Equal(t, "AA", newPost.Props["A"]) - assert.Equal(t, "old_override_icon", newPost.Props["override_icon_url"]) - assert.Equal(t, false, newPost.Props["from_webhook"]) + assert.Nil(t, newPost.GetProp("B")) + assert.Nil(t, newPost.GetProp("override_username")) + assert.Equal(t, "AA", newPost.GetProp("A")) + assert.Equal(t, "old_override_icon", newPost.GetProp("override_icon_url")) + assert.Equal(t, false, newPost.GetProp("from_webhook")) } func TestSubmitInteractiveDialog(t *testing.T) { @@ -578,7 +578,7 @@ func TestPostActionRelativeURL(t *testing.T) { post, err := th.App.CreatePostAsUser(&interactivePost, "") require.Nil(t, err) - attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + attachments, ok := post.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) @@ -618,7 +618,7 @@ func TestPostActionRelativeURL(t *testing.T) { post, err := th.App.CreatePostAsUser(&interactivePost, "") require.Nil(t, err) - attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + attachments, ok := post.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) @@ -658,7 +658,7 @@ func TestPostActionRelativeURL(t *testing.T) { post, err := th.App.CreatePostAsUser(&interactivePost, "") require.Nil(t, err) - attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + attachments, ok := post.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) @@ -699,7 +699,7 @@ func TestPostActionRelativeURL(t *testing.T) { post, err := th.App.CreatePostAsUser(&interactivePost, "") require.Nil(t, err) - attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + attachments, ok := post.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) @@ -739,7 +739,7 @@ func TestPostActionRelativeURL(t *testing.T) { post, err := th.App.CreatePostAsUser(&interactivePost, "") require.Nil(t, err) - attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + attachments, ok := post.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) @@ -813,7 +813,7 @@ func TestPostActionRelativePluginURL(t *testing.T) { post, err := th.App.CreatePostAsUser(&interactivePost, "") require.Nil(t, err) - attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + attachments, ok := post.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) @@ -853,7 +853,7 @@ func TestPostActionRelativePluginURL(t *testing.T) { post, err := th.App.CreatePostAsUser(&interactivePost, "") require.Nil(t, err) - attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + attachments, ok := post.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) @@ -893,7 +893,7 @@ func TestPostActionRelativePluginURL(t *testing.T) { post, err := th.App.CreatePostAsUser(&interactivePost, "") require.Nil(t, err) - attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + attachments, ok := post.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) @@ -933,7 +933,7 @@ func TestPostActionRelativePluginURL(t *testing.T) { post, err := th.App.CreatePostAsUser(&interactivePost, "") require.Nil(t, err) - attachments, ok := post.Props["attachments"].([]*model.SlackAttachment) + attachments, ok := post.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) diff --git a/app/notification.go b/app/notification.go index ff2d4ca656..fa989c293b 100644 --- a/app/notification.go +++ b/app/notification.go @@ -69,7 +69,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod mentions.addMention(otherUserId, DMMention) } - if post.Props["from_webhook"] == "true" { + if post.GetProp("from_webhook") == "true" { mentions.addMention(post.UserId, DMMention) } } else { @@ -81,7 +81,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod // Add an implicit mention when a user is added to a channel // even if the user has set 'username mentions' to false in account settings. if post.Type == model.POST_ADD_TO_CHANNEL { - addedUserId, ok := post.Props[model.POST_PROPS_ADDED_USER_ID].(string) + addedUserId, ok := post.GetProp(model.POST_PROPS_ADDED_USER_ID).(string) if ok { mentions.addMention(addedUserId, KeywordMention) } @@ -103,7 +103,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod } // prevent the user from mentioning themselves - if post.Props["from_webhook"] != "true" { + if post.GetProp("from_webhook") != "true" { mentions.removeMention(post.UserId) } @@ -118,7 +118,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod for _, profile := range profileMap { if (profile.NotifyProps[model.PUSH_NOTIFY_PROP] == model.USER_NOTIFY_ALL || channelMemberNotifyPropsMap[profile.Id][model.PUSH_NOTIFY_PROP] == model.CHANNEL_NOTIFY_ALL) && - (post.UserId != profile.Id || post.Props["from_webhook"] == "true") && + (post.UserId != profile.Id || post.GetProp("from_webhook") == "true") && !post.IsSystemMessage() { allActivityPushUserIds = append(allActivityPushUserIds, profile.Id) } @@ -733,7 +733,7 @@ func (n *PostNotification) GetSenderName(userNameFormat string, overridesAllowed } if overridesAllowed && n.Channel.Type != model.CHANNEL_DIRECT { - if value, ok := n.Post.Props["override_username"]; ok && n.Post.Props["from_webhook"] == "true" { + if value, ok := n.Post.GetProps()["override_username"]; ok && n.Post.GetProp("from_webhook") == "true" { return value.(string) } } diff --git a/app/notification_push.go b/app/notification_push.go index e8dc08fc8b..ab986f2932 100644 --- a/app/notification_push.go +++ b/app/notification_push.go @@ -425,7 +425,7 @@ func DoesNotifyPropsAllowPushNotification(user *model.User, channelNotifyProps m } if (userNotify == model.USER_NOTIFY_ALL || channelNotify == model.CHANNEL_NOTIFY_ALL) && - (post.UserId != user.Id || post.Props["from_webhook"] == "true") { + (post.UserId != user.Id || post.GetProp("from_webhook") == "true") { return true } @@ -521,16 +521,16 @@ func (a *App) buildFullPushNotificationMessage(contentsConfig string, post *mode } msg.SenderName = senderName - if ou, ok := post.Props["override_username"].(string); ok && *cfg.ServiceSettings.EnablePostUsernameOverride { + if ou, ok := post.GetProp("override_username").(string); ok && *cfg.ServiceSettings.EnablePostUsernameOverride { msg.OverrideUsername = ou msg.SenderName = ou } - if oi, ok := post.Props["override_icon_url"].(string); ok && *cfg.ServiceSettings.EnablePostIconOverride { + if oi, ok := post.GetProp("override_icon_url").(string); ok && *cfg.ServiceSettings.EnablePostIconOverride { msg.OverrideIconUrl = oi } - if fw, ok := post.Props["from_webhook"].(string); ok { + if fw, ok := post.GetProp("from_webhook").(string); ok { msg.FromWebhook = fw } diff --git a/app/notification_test.go b/app/notification_test.go index 6e3f9fc521..2e07a913bd 100644 --- a/app/notification_test.go +++ b/app/notification_test.go @@ -1613,7 +1613,7 @@ func TestPostNotificationGetSenderName(t *testing.T) { "overridden username": { post: overriddenPost, allowOverrides: true, - expected: overriddenPost.Props["override_username"].(string), + expected: overriddenPost.GetProp("override_username").(string), }, "overridden username, direct channel": { channel: &model.Channel{Type: model.CHANNEL_DIRECT}, diff --git a/app/plugin_deadlock_test.go b/app/plugin_deadlock_test.go index e46cf42f34..2518e27c16 100644 --- a/app/plugin_deadlock_test.go +++ b/app/plugin_deadlock_test.go @@ -46,7 +46,7 @@ func TestPluginDeadlock(t *testing.T) { } func (p *MyPlugin) MessageWillBePosted(c *plugin.Context, post *model.Post) (*model.Post, string) { - if _, from_plugin := post.Props["from_plugin"]; from_plugin { + if _, from_plugin := post.GetProps()["from_plugin"]; from_plugin { return nil, "" } @@ -121,7 +121,7 @@ func TestPluginDeadlock(t *testing.T) { } func (p *MyPlugin) MessageWillBePosted(c *plugin.Context, post *model.Post) (*model.Post, string) { - if _, from_plugin := post.Props["from_plugin"]; from_plugin { + if _, from_plugin := post.GetProps()["from_plugin"]; from_plugin { return nil, "" } diff --git a/app/post.go b/app/post.go index 915a74a4b1..b9e29456ed 100644 --- a/app/post.go +++ b/app/post.go @@ -73,7 +73,7 @@ func (a *App) CreatePostAsUser(post *model.Post, currentSessionId string) (*mode } // Update the LastViewAt only if the post does not have from_webhook prop set (eg. Zapier app) - if _, ok := post.Props["from_webhook"]; !ok { + if _, ok := post.GetProps()["from_webhook"]; !ok { if _, err := a.MarkChannelsAsViewed([]string{post.ChannelId}, post.UserId, currentSessionId); err != nil { mlog.Error( "Encountered error updating last viewed", @@ -241,12 +241,12 @@ func (a *App) CreatePost(post *model.Post, channel *model.Channel, triggerWebhoo } // Temporary fix so old plugins don't clobber new fields in SlackAttachment struct, see MM-13088 - if attachments, ok := post.Props["attachments"].([]*model.SlackAttachment); ok { + if attachments, ok := post.GetProp("attachments").([]*model.SlackAttachment); ok { jsonAttachments, err := json.Marshal(attachments) if err == nil { attachmentsInterface := []interface{}{} err = json.Unmarshal(jsonAttachments, &attachmentsInterface) - post.Props["attachments"] = attachmentsInterface + post.AddProp("attachments", attachmentsInterface) } if err != nil { mlog.Error("Could not convert post attachments to map interface.", mlog.Err(err)) @@ -384,8 +384,8 @@ func (a *App) FillInPostProps(post *model.Post, channel *model.Channel) *model.A if len(channelMentionsProp) > 0 { post.AddProp("channel_mentions", channelMentionsProp) - } else if post.Props != nil { - delete(post.Props, "channel_mentions") + } else if post.GetProps() != nil { + post.DelProp("channel_mentions") } return nil @@ -439,8 +439,8 @@ func (a *App) SendEphemeralPost(userId string, post *model.Post) *model.Post { if post.CreateAt == 0 { post.CreateAt = model.GetMillis() } - if post.Props == nil { - post.Props = model.StringInterface{} + if post.GetProps() == nil { + post.SetProps(make(model.StringInterface)) } post.GenerateActionIds() @@ -457,8 +457,8 @@ func (a *App) UpdateEphemeralPost(userId string, post *model.Post) *model.Post { post.Type = model.POST_EPHEMERAL post.UpdateAt = model.GetMillis() - if post.Props == nil { - post.Props = model.StringInterface{} + if post.GetProps() == nil { + post.SetProps(make(model.StringInterface)) } post.GenerateActionIds() @@ -526,7 +526,7 @@ func (a *App) UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model } newPost := &model.Post{} - *newPost = *oldPost + newPost = oldPost.Clone() if newPost.Message != post.Message { newPost.Message = post.Message @@ -538,7 +538,7 @@ func (a *App) UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model newPost.IsPinned = post.IsPinned newPost.HasReactions = post.HasReactions newPost.FileIds = post.FileIds - newPost.Props = post.Props + newPost.SetProps(post.GetProps()) } // Avoid deep-equal checks if EditAt was already modified through message change @@ -1220,7 +1220,7 @@ func isCommentMention(user *model.User, post *model.Post, otherPosts map[string] func isPostMention(user *model.User, post *model.Post, keywords map[string][]string, otherPosts map[string]*model.Post, mentionedByThread map[string]bool, checkForCommentMentions bool) bool { // Prevent the user from mentioning themselves - if post.UserId == user.Id && post.Props["from_webhook"] != "true" { + if post.UserId == user.Id && post.GetProp("from_webhook") != "true" { return false } @@ -1232,7 +1232,7 @@ func isPostMention(user *model.User, post *model.Post, keywords map[string][]str // Check for mentions caused by being added to the channel if post.Type == model.POST_ADD_TO_CHANNEL { - if addedUserId, ok := post.Props[model.POST_PROPS_ADDED_USER_ID].(string); ok && addedUserId == user.Id { + if addedUserId, ok := post.GetProp(model.POST_PROPS_ADDED_USER_ID).(string); ok && addedUserId == user.Id { return true } } diff --git a/app/post_metadata.go b/app/post_metadata.go index 3a99d29bd4..1cfeea0e97 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -59,7 +59,7 @@ func (a *App) PreparePostListForClient(originalList *model.PostList) *model.Post // OverrideIconURLIfEmoji changes the post icon override URL prop, if it has an emoji icon, // so that it points to the URL (relative) of the emoji - static if emoji is default, /api if custom. func (a *App) OverrideIconURLIfEmoji(post *model.Post) { - prop, ok := post.Props[model.POST_PROPS_OVERRIDE_ICON_EMOJI] + prop, ok := post.GetProps()[model.POST_PROPS_OVERRIDE_ICON_EMOJI] if !ok || prop == nil { return } @@ -149,7 +149,7 @@ func (a *App) getEmojisAndReactionsForPost(post *model.Post) ([]*model.Emoji, [] } func (a *App) getEmbedForPost(post *model.Post, firstLink string, isNewPost bool) (*model.PostEmbed, error) { - if _, ok := post.Props["attachments"]; ok { + if _, ok := post.GetProps()["attachments"]; ok { return &model.PostEmbed{ Type: model.POST_EMBED_MESSAGE_ATTACHMENT, }, nil diff --git a/app/post_metadata_test.go b/app/post_metadata_test.go index 0660e82fd0..f65ab845ff 100644 --- a/app/post_metadata_test.go +++ b/app/post_metadata_test.go @@ -307,10 +307,10 @@ func TestPreparePostForClient(t *testing.T) { t.Run("does not override icon URL", func(t *testing.T) { clientPost := prepare(false, url, emoji) - s, ok := clientPost.Props[model.POST_PROPS_OVERRIDE_ICON_URL] + s, ok := clientPost.GetProps()[model.POST_PROPS_OVERRIDE_ICON_URL] assert.True(t, ok) assert.EqualValues(t, url, s) - s, ok = clientPost.Props[model.POST_PROPS_OVERRIDE_ICON_EMOJI] + s, ok = clientPost.GetProps()[model.POST_PROPS_OVERRIDE_ICON_EMOJI] assert.True(t, ok) assert.EqualValues(t, emoji, s) }) @@ -318,10 +318,10 @@ func TestPreparePostForClient(t *testing.T) { t.Run("overrides icon URL", func(t *testing.T) { clientPost := prepare(true, url, emoji) - s, ok := clientPost.Props[model.POST_PROPS_OVERRIDE_ICON_URL] + s, ok := clientPost.GetProps()[model.POST_PROPS_OVERRIDE_ICON_URL] assert.True(t, ok) assert.EqualValues(t, overridenUrl, s) - s, ok = clientPost.Props[model.POST_PROPS_OVERRIDE_ICON_EMOJI] + s, ok = clientPost.GetProps()[model.POST_PROPS_OVERRIDE_ICON_EMOJI] assert.True(t, ok) assert.EqualValues(t, emoji, s) }) diff --git a/app/post_test.go b/app/post_test.go index df82dc2f28..ac42960422 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -256,13 +256,13 @@ func TestUpdatePostEditAt(t *testing.T) { defer th.TearDown() post := &model.Post{} - *post = *th.BasicPost + post = th.BasicPost.Clone() post.IsPinned = true saved, err := th.App.UpdatePost(post, true) require.Nil(t, err) assert.Equal(t, saved.EditAt, post.EditAt, "shouldn't have updated post.EditAt when pinning post") - *post = *saved + post = saved.Clone() time.Sleep(time.Millisecond * 100) @@ -279,7 +279,7 @@ func TestUpdatePostTimeLimit(t *testing.T) { defer th.TearDown() post := &model.Post{} - *post = *th.BasicPost + post = th.BasicPost.Clone() th.App.SetLicense(model.NewTestLicense()) @@ -433,7 +433,7 @@ func TestPostChannelMentions(t *testing.T) { "mention-test": map[string]interface{}{ "display_name": "Mention Test", }, - }, result.Props["channel_mentions"]) + }, result.GetProp("channel_mentions")) post.Message = fmt.Sprintf("goodbye, ~%v!", channelToMention.Name) result, err = th.App.UpdatePost(post, false) @@ -442,7 +442,7 @@ func TestPostChannelMentions(t *testing.T) { "mention-test": map[string]interface{}{ "display_name": "Mention Test", }, - }, result.Props["channel_mentions"]) + }, result.GetProp("channel_mentions")) } func TestImageProxy(t *testing.T) { @@ -694,7 +694,7 @@ func TestCreatePost(t *testing.T) { } rpost, err := th.App.CreatePost(postWithNoMention, th.BasicChannel, false) require.Nil(t, err) - assert.Equal(t, rpost.Props, model.StringInterface{}) + assert.Equal(t, rpost.GetProps(), model.StringInterface{}) postWithMention := &model.Post{ ChannelId: th.BasicChannel.Id, @@ -703,7 +703,7 @@ func TestCreatePost(t *testing.T) { } rpost, err = th.App.CreatePost(postWithMention, th.BasicChannel, false) require.Nil(t, err) - assert.Equal(t, rpost.Props, model.StringInterface{}) + assert.Equal(t, rpost.GetProps(), model.StringInterface{}) }) t.Run("Sets prop when post has mentions and user does not have USE_CHANNEL_MENTIONS", func(t *testing.T) { @@ -716,7 +716,7 @@ func TestCreatePost(t *testing.T) { } rpost, err := th.App.CreatePost(postWithNoMention, th.BasicChannel, false) require.Nil(t, err) - assert.Equal(t, rpost.Props, model.StringInterface{}) + assert.Equal(t, rpost.GetProps(), model.StringInterface{}) postWithMention := &model.Post{ ChannelId: th.BasicChannel.Id, @@ -725,7 +725,7 @@ func TestCreatePost(t *testing.T) { } rpost, err = th.App.CreatePost(postWithMention, th.BasicChannel, false) require.Nil(t, err) - assert.Equal(t, rpost.Props[model.POST_PROPS_MENTION_HIGHLIGHT_DISABLED], true) + assert.Equal(t, rpost.GetProp(model.POST_PROPS_MENTION_HIGHLIGHT_DISABLED), true) th.AddPermissionToRole(model.PERMISSION_USE_CHANNEL_MENTIONS.Id, model.CHANNEL_USER_ROLE_ID) }) @@ -787,13 +787,13 @@ func TestPatchPost(t *testing.T) { rpost, err = th.App.PatchPost(rpost.Id, patchWithNoMention) require.Nil(t, err) - assert.Equal(t, rpost.Props, model.StringInterface{}) + assert.Equal(t, rpost.GetProps(), model.StringInterface{}) patchWithMention := &model.PostPatch{Message: model.NewString("This patch has a mention now @here")} rpost, err = th.App.PatchPost(rpost.Id, patchWithMention) require.Nil(t, err) - assert.Equal(t, rpost.Props, model.StringInterface{}) + assert.Equal(t, rpost.GetProps(), model.StringInterface{}) }) t.Run("Sets prop when user does not have USE_CHANNEL_MENTIONS", func(t *testing.T) { @@ -802,13 +802,13 @@ func TestPatchPost(t *testing.T) { patchWithNoMention := &model.PostPatch{Message: model.NewString("This patch still does not have a mention")} rpost, err = th.App.PatchPost(rpost.Id, patchWithNoMention) require.Nil(t, err) - assert.Equal(t, rpost.Props, model.StringInterface{}) + assert.Equal(t, rpost.GetProps(), model.StringInterface{}) patchWithMention := &model.PostPatch{Message: model.NewString("This patch has a mention now @here")} rpost, err = th.App.PatchPost(rpost.Id, patchWithMention) require.Nil(t, err) - assert.Equal(t, rpost.Props[model.POST_PROPS_MENTION_HIGHLIGHT_DISABLED], true) + assert.Equal(t, rpost.GetProp(model.POST_PROPS_MENTION_HIGHLIGHT_DISABLED), true) th.AddPermissionToRole(model.PERMISSION_USE_CHANNEL_MENTIONS.Id, model.CHANNEL_USER_ROLE_ID) }) diff --git a/app/webhook.go b/app/webhook.go index 2b53889514..56a1ebe8ec 100644 --- a/app/webhook.go +++ b/app/webhook.go @@ -168,20 +168,21 @@ func SplitWebhookPost(post *model.Post, maxPostSize int) ([]*model.Post, *model. splits := make([]*model.Post, 0) remainingText := post.Message - base := *post + base := post.Clone() base.Message = "" - base.Props = make(map[string]interface{}) - for k, v := range post.Props { + base.SetProps(make(map[string]interface{})) + for k, v := range post.GetProps() { if k != "attachments" { - base.Props[k] = v + base.AddProp(k, v) } } - if utf8.RuneCountInString(model.StringInterfaceToJson(base.Props)) > model.POST_PROPS_MAX_USER_RUNES { + + if utf8.RuneCountInString(model.StringInterfaceToJson(base.GetProps())) > model.POST_PROPS_MAX_USER_RUNES { return nil, model.NewAppError("SplitWebhookPost", "web.incoming_webhook.split_props_length.app_error", map[string]interface{}{"Max": model.POST_PROPS_MAX_USER_RUNES}, "", http.StatusBadRequest) } for utf8.RuneCountInString(remainingText) > maxPostSize { - split := base + split := base.Clone() x := 0 for index := range remainingText { x++ @@ -191,20 +192,20 @@ func SplitWebhookPost(post *model.Post, maxPostSize int) ([]*model.Post, *model. break } } - splits = append(splits, &split) + splits = append(splits, split) } - split := base + split := base.Clone() split.Message = remainingText - splits = append(splits, &split) + splits = append(splits, split) - attachments, _ := post.Props["attachments"].([]*model.SlackAttachment) + attachments, _ := post.GetProp("attachments").([]*model.SlackAttachment) for _, attachment := range attachments { newAttachment := *attachment for { lastSplit := splits[len(splits)-1] newProps := make(map[string]interface{}) - for k, v := range lastSplit.Props { + for k, v := range lastSplit.GetProps() { newProps[k] = v } origAttachments, _ := newProps["attachments"].([]*model.SlackAttachment) @@ -213,13 +214,13 @@ func SplitWebhookPost(post *model.Post, maxPostSize int) ([]*model.Post, *model. runeCount := utf8.RuneCountInString(newPropsString) if runeCount <= model.POST_PROPS_MAX_USER_RUNES { - lastSplit.Props = newProps + lastSplit.SetProps(newProps) break } if len(origAttachments) > 0 { newSplit := base - splits = append(splits, &newSplit) + splits = append(splits, newSplit) continue } @@ -236,7 +237,7 @@ func SplitWebhookPost(post *model.Post, maxPostSize int) ([]*model.Post, *model. break } } - lastSplit.Props = newProps + lastSplit.SetProps(newProps) break } } diff --git a/app/webhook_test.go b/app/webhook_test.go index b428c62cff..16bd7cb62f 100644 --- a/app/webhook_test.go +++ b/app/webhook_test.go @@ -302,9 +302,9 @@ func TestCreateWebhookPost(t *testing.T) { }, model.POST_SLACK_ATTACHMENT, "") require.Nil(t, err) - assert.Contains(t, post.Props, "from_webhook", "missing from_webhook prop") - assert.Contains(t, post.Props, "attachments", "missing attachments prop") - assert.Contains(t, post.Props, "webhook_display_name", "missing webhook_display_name prop") + assert.Contains(t, post.GetProps(), "from_webhook", "missing from_webhook prop") + assert.Contains(t, post.GetProps(), "attachments", "missing attachments prop") + assert.Contains(t, post.GetProps(), "webhook_display_name", "missing webhook_display_name prop") _, err = th.App.CreateWebhookPost(hook.UserId, th.BasicChannel, "foo", "user", "http://iconurl", "", nil, model.POST_SYSTEM_GENERIC, "") require.NotNil(t, err, "Should have failed - bad post type") @@ -449,7 +449,7 @@ func TestSplitWebhookPost(t *testing.T) { for i, split := range splits { if i < len(tc.Expected) { assert.Equal(t, tc.Expected[i].Message, split.Message) - assert.Equal(t, tc.Expected[i].Props["attachments"], split.Props["attachments"]) + assert.Equal(t, tc.Expected[i].GetProp("attachments"), split.GetProp("attachments")) } } }) @@ -610,17 +610,17 @@ func TestTriggerOutGoingWebhookWithUsernameAndIconURL(t *testing.T) { select { case webhookPost := <-createdPost: assert.Equal(t, webhookPost.Message, "sample response text from test server") - assert.Equal(t, webhookPost.Props["from_webhook"], "true") + assert.Equal(t, webhookPost.GetProp("from_webhook"), "true") if testCase.ExpectedIconUrl != "" { - assert.Equal(t, webhookPost.Props["override_icon_url"], testCase.ExpectedIconUrl) + assert.Equal(t, webhookPost.GetProp("override_icon_url"), testCase.ExpectedIconUrl) } else { - assert.Nil(t, webhookPost.Props["override_icon_url"]) + assert.Nil(t, webhookPost.GetProp("override_icon_url")) } if testCase.ExpectedUsername != "" { - assert.Equal(t, webhookPost.Props["override_username"], testCase.ExpectedUsername) + assert.Equal(t, webhookPost.GetProp("override_username"), testCase.ExpectedUsername) } else { - assert.Nil(t, webhookPost.Props["override_username"]) + assert.Nil(t, webhookPost.GetProp("override_username")) } case <-time.After(5 * time.Second): require.Fail(t, "Timeout, webhook response not created as post") diff --git a/model/integration_action.go b/model/integration_action.go index 3cb0011738..b682b85883 100644 --- a/model/integration_action.go +++ b/model/integration_action.go @@ -368,8 +368,8 @@ func (r *SubmitDialogResponse) ToJson() []byte { func (o *Post) StripActionIntegrations() { attachments := o.Attachments() - if o.Props["attachments"] != nil { - o.Props["attachments"] = attachments + if o.GetProp("attachments") != nil { + o.AddProp("attachments", attachments) } for _, attachment := range attachments { for _, action := range attachment.Actions { @@ -390,10 +390,10 @@ func (o *Post) GetAction(id string) *PostAction { } func (o *Post) GenerateActionIds() { - if o.Props["attachments"] != nil { - o.Props["attachments"] = o.Attachments() + if o.GetProp("attachments") != nil { + o.AddProp("attachments", o.Attachments()) } - if attachments, ok := o.Props["attachments"].([]*SlackAttachment); ok { + if attachments, ok := o.GetProp("attachments").([]*SlackAttachment); ok { for _, attachment := range attachments { for _, action := range attachment.Actions { if action.Id == "" { @@ -411,7 +411,7 @@ func AddPostActionCookies(o *Post, secret []byte) *Post { retainProps := map[string]interface{}{} removeProps := []string{} for _, key := range PostActionRetainPropKeys { - value, ok := p.Props[key] + value, ok := p.GetProps()[key] if ok { retainProps[key] = value } else { diff --git a/model/post.go b/model/post.go index e7aef8345e..eaa58357d3 100644 --- a/model/post.go +++ b/model/post.go @@ -5,11 +5,13 @@ package model import ( "encoding/json" + "errors" "io" "net/http" "regexp" "sort" "strings" + "sync" "unicode/utf8" "github.com/mattermost/mattermost-server/v5/utils/markdown" @@ -83,7 +85,8 @@ type Post struct { MessageSource string `json:"message_source,omitempty" db:"-"` Type string `json:"type"` - Props StringInterface `json:"props"` + propsMu sync.RWMutex `db:"-"` // Unexported mutex used to guard Post.Props. + Props StringInterface `json:"props"` // Deprecated: use GetProps() Hashtags string `json:"hashtags"` Filenames StringArray `json:"filenames,omitempty"` // Deprecated, do not use this field any more FileIds StringArray `json:"file_ids,omitempty"` @@ -156,10 +159,46 @@ type PostForIndexing struct { ParentCreateAt *int64 `json:"parent_create_at"` } -// Clone shallowly copies the post. +// ShallowCopy is an utility function to shallow copy a Post to the given +// destination without touching the internal RWMutex. +func (o *Post) ShallowCopy(dst *Post) error { + if dst == nil { + return errors.New("dst cannot be nil") + } + o.propsMu.RLock() + defer o.propsMu.RUnlock() + dst.propsMu.Lock() + defer dst.propsMu.Unlock() + dst.Id = o.Id + dst.CreateAt = o.CreateAt + dst.UpdateAt = o.UpdateAt + dst.EditAt = o.EditAt + dst.DeleteAt = o.DeleteAt + dst.IsPinned = o.IsPinned + dst.UserId = o.UserId + dst.ChannelId = o.ChannelId + dst.RootId = o.RootId + dst.ParentId = o.ParentId + dst.OriginalId = o.OriginalId + dst.Message = o.Message + dst.MessageSource = o.MessageSource + dst.Type = o.Type + dst.Props = o.Props + dst.Hashtags = o.Hashtags + dst.Filenames = o.Filenames + dst.FileIds = o.FileIds + dst.PendingPostId = o.PendingPostId + dst.HasReactions = o.HasReactions + dst.ReplyCount = o.ReplyCount + dst.Metadata = o.Metadata + return nil +} + +// Clone shallowly copies the post and returns the copy. func (o *Post) Clone() *Post { - copy := *o - return © + copy := &Post{} + o.ShallowCopy(copy) + return copy } func (o *Post) ToJson() string { @@ -199,7 +238,6 @@ func (o *Post) Etag() string { } func (o *Post) IsValid(maxPostSize int) *AppError { - if len(o.Id) != 26 { return NewAppError("Post.IsValid", "model.post.is_valid.id.app_error", nil, "", http.StatusBadRequest) } @@ -286,7 +324,7 @@ func (o *Post) IsValid(maxPostSize int) *AppError { return NewAppError("Post.IsValid", "model.post.is_valid.file_ids.app_error", nil, "id="+o.Id, http.StatusBadRequest) } - if utf8.RuneCountInString(StringInterfaceToJson(o.Props)) > POST_PROPS_MAX_RUNES { + if utf8.RuneCountInString(StringInterfaceToJson(o.GetProps())) > POST_PROPS_MAX_RUNES { return NewAppError("Post.IsValid", "model.post.is_valid.props.app_error", nil, "id="+o.Id, http.StatusBadRequest) } @@ -299,8 +337,8 @@ func (o *Post) SanitizeProps() { } for _, member := range membersToSanitize { - if _, ok := o.Props[member]; ok { - delete(o.Props, member) + if _, ok := o.GetProps()[member]; ok { + o.DelProp(member) } } } @@ -321,8 +359,8 @@ func (o *Post) PreSave() { } func (o *Post) PreCommit() { - if o.Props == nil { - o.Props = make(map[string]interface{}) + if o.GetProps() == nil { + o.SetProps(make(map[string]interface{})) } if o.Filenames == nil { @@ -340,16 +378,49 @@ func (o *Post) PreCommit() { } func (o *Post) MakeNonNil() { - if o.Props == nil { - o.Props = make(map[string]interface{}) + if o.GetProps() == nil { + o.SetProps(make(map[string]interface{})) } } +func (o *Post) DelProp(key string) { + o.propsMu.Lock() + defer o.propsMu.Unlock() + propsCopy := make(map[string]interface{}, len(o.Props)-1) + for k, v := range o.Props { + propsCopy[k] = v + } + delete(propsCopy, key) + o.Props = propsCopy +} + func (o *Post) AddProp(key string, value interface{}) { + o.propsMu.Lock() + defer o.propsMu.Unlock() + propsCopy := make(map[string]interface{}, len(o.Props)+1) + for k, v := range o.Props { + propsCopy[k] = v + } + propsCopy[key] = value + o.Props = propsCopy +} - o.MakeNonNil() +func (o *Post) GetProps() StringInterface { + o.propsMu.RLock() + defer o.propsMu.RUnlock() + return o.Props +} - o.Props[key] = value +func (o *Post) SetProps(props StringInterface) { + o.propsMu.Lock() + defer o.propsMu.Unlock() + o.Props = props +} + +func (o *Post) GetProp(key string) interface{} { + o.propsMu.RLock() + defer o.propsMu.RUnlock() + return o.Props[key] } func (o *Post) IsSystemMessage() bool { @@ -379,7 +450,8 @@ func (o *Post) Patch(patch *PostPatch) { } if patch.Props != nil { - o.Props = *patch.Props + newProps := *patch.Props + o.SetProps(newProps) } if patch.FileIds != nil { @@ -464,11 +536,11 @@ func findAtChannelMention(message string) (mention string, found bool) { } func (o *Post) Attachments() []*SlackAttachment { - if attachments, ok := o.Props["attachments"].([]*SlackAttachment); ok { + if attachments, ok := o.GetProp("attachments").([]*SlackAttachment); ok { return attachments } var ret []*SlackAttachment - if attachments, ok := o.Props["attachments"].([]interface{}); ok { + if attachments, ok := o.GetProp("attachments").([]interface{}); ok { for _, attachment := range attachments { if enc, err := json.Marshal(attachment); err == nil { var decoded SlackAttachment diff --git a/model/post_list.go b/model/post_list.go index 9e63882abf..d00b68b58a 100644 --- a/model/post_list.go +++ b/model/post_list.go @@ -46,9 +46,9 @@ func (o *PostList) StripActionIntegrations() { posts := o.Posts o.Posts = make(map[string]*Post) for id, post := range posts { - pcopy := *post + pcopy := post.Clone() pcopy.StripActionIntegrations() - o.Posts[id] = &pcopy + o.Posts[id] = pcopy } } diff --git a/model/post_test.go b/model/post_test.go index 8a9cbffb5d..09ff0d9d5d 100644 --- a/model/post_test.go +++ b/model/post_test.go @@ -6,6 +6,7 @@ package model import ( "io/ioutil" "strings" + "sync" "testing" "github.com/stretchr/testify/assert" @@ -18,7 +19,7 @@ func TestPostToJson(t *testing.T) { ro := PostFromJson(strings.NewReader(j)) assert.NotNil(t, ro) - assert.Equal(t, o, *ro) + assert.Equal(t, &o, ro.Clone()) } func TestPostFromJsonError(t *testing.T) { @@ -124,7 +125,7 @@ func TestPostSanitizeProps(t *testing.T) { post1.SanitizeProps() - require.Nil(t, post1.Props[PROPS_ADD_CHANNEL_MEMBER]) + require.Nil(t, post1.GetProp(PROPS_ADD_CHANNEL_MEMBER)) post2 := &Post{ Message: "test", @@ -135,7 +136,7 @@ func TestPostSanitizeProps(t *testing.T) { post2.SanitizeProps() - require.Nil(t, post2.Props[PROPS_ADD_CHANNEL_MEMBER]) + require.Nil(t, post2.GetProp(PROPS_ADD_CHANNEL_MEMBER)) post3 := &Post{ Message: "test", @@ -147,9 +148,9 @@ func TestPostSanitizeProps(t *testing.T) { post3.SanitizeProps() - require.Nil(t, post3.Props[PROPS_ADD_CHANNEL_MEMBER]) + require.Nil(t, post3.GetProp(PROPS_ADD_CHANNEL_MEMBER)) - require.NotNil(t, post3.Props["attachments"]) + require.NotNil(t, post3.GetProp("attachments")) } func TestPost_AttachmentsEqual(t *testing.T) { @@ -502,6 +503,182 @@ func BenchmarkRewriteImageURLs(b *testing.B) { }) } } + +func TestPostShallowCopy(t *testing.T) { + var dst *Post + p := &Post{ + Id: NewId(), + } + + err := p.ShallowCopy(dst) + require.Error(t, err) + + dst = &Post{} + err = p.ShallowCopy(dst) + require.NoError(t, err) + require.Equal(t, p, dst) + require.Condition(t, func() bool { + return p != dst + }) +} + +func TestPostClone(t *testing.T) { + p := &Post{ + Id: NewId(), + } + + pp := p.Clone() + require.Equal(t, p, pp) + require.Condition(t, func() bool { + return p != pp + }) + require.Condition(t, func() bool { + return &p.propsMu != &pp.propsMu + }) +} + +func BenchmarkClonePost(b *testing.B) { + p := Post{} + for i := 0; i < b.N; i++ { + _ = p.Clone() + } +} + +func BenchmarkPostPropsGet_indirect(b *testing.B) { + p := Post{ + Props: make(StringInterface), + } + for i := 0; i < b.N; i++ { + _ = p.GetProps() + } +} + +func BenchmarkPostPropsGet_direct(b *testing.B) { + p := Post{ + Props: make(StringInterface), + } + for i := 0; i < b.N; i++ { + _ = p.Props + } +} + +func BenchmarkPostPropsAdd_indirect(b *testing.B) { + p := Post{ + Props: make(StringInterface), + } + for i := 0; i < b.N; i++ { + p.AddProp("test", "somevalue") + } +} + +func BenchmarkPostPropsAdd_direct(b *testing.B) { + p := Post{ + Props: make(StringInterface), + } + for i := 0; i < b.N; i++ { + p.Props["test"] = "somevalue" + } +} + +func BenchmarkPostPropsDel_indirect(b *testing.B) { + p := Post{ + Props: make(StringInterface), + } + p.AddProp("test", "somevalue") + for i := 0; i < b.N; i++ { + p.DelProp("test") + } +} + +func BenchmarkPostPropsDel_direct(b *testing.B) { + p := Post{ + Props: make(StringInterface), + } + for i := 0; i < b.N; i++ { + delete(p.Props, "test") + } +} + +func BenchmarkPostPropGet_direct(b *testing.B) { + p := Post{ + Props: make(StringInterface), + } + p.Props["somekey"] = "somevalue" + for i := 0; i < b.N; i++ { + _ = p.Props["somekey"] + } +} + +func BenchmarkPostPropGet_indirect(b *testing.B) { + p := Post{ + Props: make(StringInterface), + } + p.Props["somekey"] = "somevalue" + for i := 0; i < b.N; i++ { + _ = p.GetProp("somekey") + } +} + +// TestPostPropsDataRace tries to trigger data race conditions related to Post.Props. +// It's meant to be run with the -race flag. +func TestPostPropsDataRace(t *testing.T) { + p := Post{Message: "test"} + + wg := sync.WaitGroup{} + wg.Add(7) + + go func() { + for i := 0; i < 100; i++ { + p.AddProp("test", "test") + } + wg.Done() + }() + + go func() { + for i := 0; i < 100; i++ { + _ = p.GetProp("test") + } + wg.Done() + }() + + go func() { + for i := 0; i < 100; i++ { + p.AddProp("test", "test2") + } + wg.Done() + }() + + go func() { + for i := 0; i < 100; i++ { + _ = p.GetProps()["test"] + } + wg.Done() + }() + + go func() { + for i := 0; i < 100; i++ { + p.DelProp("test") + } + wg.Done() + }() + + go func() { + for i := 0; i < 100; i++ { + p.SetProps(make(StringInterface)) + } + wg.Done() + }() + + go func() { + for i := 0; i < 100; i++ { + _ = p.Clone() + } + wg.Done() + }() + + wg.Wait() +} + func Test_findAtChannelMention(t *testing.T) { testCases := []struct { Name string diff --git a/plugin/example_help_test.go b/plugin/example_help_test.go index f03849a942..fd4f6120c6 100644 --- a/plugin/example_help_test.go +++ b/plugin/example_help_test.go @@ -97,7 +97,7 @@ func (p *HelpPlugin) MessageHasBeenPosted(c *plugin.Context, post *model.Post) { } // Ignore posts this plugin made. - if sentByPlugin, _ := post.Props["sent_by_plugin"].(bool); sentByPlugin { + if sentByPlugin, _ := post.GetProp("sent_by_plugin").(bool); sentByPlugin { return } diff --git a/plugin/helpers_bots.go b/plugin/helpers_bots.go index c25d01ee77..e4702aae08 100644 --- a/plugin/helpers_bots.go +++ b/plugin/helpers_bots.go @@ -148,7 +148,7 @@ func (p *HelpersImpl) ShouldProcessMessage(post *model.Post, options ...ShouldPr return false, nil } - if !messageProcessOptions.AllowWebhook && post.Props["from_webhook"] == "true" { + if !messageProcessOptions.AllowWebhook && post.GetProp("from_webhook") == "true" { return false, nil } diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index f1750bb0aa..817eb3d96e 100644 --- a/store/sqlstore/post_store.go +++ b/store/sqlstore/post_store.go @@ -433,9 +433,9 @@ func (s *SqlPostStore) Delete(postId string, time int64, deleteByID string) *mod return appErr(err.Error()) } - post.Props[model.POST_PROPS_DELETE_BY] = deleteByID + post.AddProp(model.POST_PROPS_DELETE_BY, deleteByID) - _, err = s.GetMaster().Exec("UPDATE Posts SET DeleteAt = :DeleteAt, UpdateAt = :UpdateAt, Props = :Props WHERE Id = :Id OR RootId = :RootId", map[string]interface{}{"DeleteAt": time, "UpdateAt": time, "Id": postId, "RootId": postId, "Props": model.StringInterfaceToJson(post.Props)}) + _, err = s.GetMaster().Exec("UPDATE Posts SET DeleteAt = :DeleteAt, UpdateAt = :UpdateAt, Props = :Props WHERE Id = :Id OR RootId = :RootId", map[string]interface{}{"DeleteAt": time, "UpdateAt": time, "Id": postId, "RootId": postId, "Props": model.StringInterfaceToJson(post.GetProps())}) if err != nil { return appErr(err.Error()) } diff --git a/store/storetest/compliance_store.go b/store/storetest/compliance_store.go index 546e7a30a9..8c0b683037 100644 --- a/store/storetest/compliance_store.go +++ b/store/storetest/compliance_store.go @@ -733,7 +733,7 @@ func testEditExportMessage(t *testing.T, ss store.Store) { //user 1 edits the previous post post1e := &model.Post{} - *post1e = *post1 + post1e = post1.Clone() post1e.Message = "edit " + post1.Message post1e, err = ss.Post().Update(post1e, post1) @@ -845,7 +845,7 @@ func testEditAfterExportMessage(t *testing.T, ss store.Store) { postEditTime := post1.UpdateAt + 1 //user 1 edits the previous post post1e := &model.Post{} - *post1e = *post1 + post1e = post1.Clone() post1e.EditAt = postEditTime post1e.Message = "edit " + post1.Message post1e, err = ss.Post().Update(post1e, post1) diff --git a/store/storetest/file_info_store.go b/store/storetest/file_info_store.go index 3b247c5279..005ad26306 100644 --- a/store/storetest/file_info_store.go +++ b/store/storetest/file_info_store.go @@ -259,16 +259,16 @@ func testFileInfoGetForUser(t *testing.T, ss store.Store) { } func testFileInfoGetWithOptions(t *testing.T, ss store.Store) { - makePost := func(chId string, user string) model.Post { + makePost := func(chId string, user string) *model.Post { post := model.Post{} post.ChannelId = chId post.UserId = user _, err := ss.Post().Save(&post) require.Nil(t, err) - return post + return &post } - makeFile := func(post model.Post, user string, createAt int64, idPrefix string) model.FileInfo { + makeFile := func(post *model.Post, user string, createAt int64, idPrefix string) model.FileInfo { id := model.NewId() id = idPrefix + id[1:] // hacky way to get sortable Ids to confirm secondary Id sort works fileInfo := model.FileInfo{ @@ -298,10 +298,10 @@ func testFileInfoGetWithOptions(t *testing.T, ss store.Store) { post2_2 := makePost(channelId3, userId2) epoch := time.Date(2020, 1, 1, 1, 1, 1, 1, time.UTC) - file1_1 := makeFile(post1_1, userId1, epoch.AddDate(0, 0, 1).Unix(), "a") // file 1 by user 1 - file1_2 := makeFile(post1_2, userId1, epoch.AddDate(0, 0, 2).Unix(), "b") // file 2 by user 1 - file1_3 := makeFile(model.Post{}, userId1, epoch.AddDate(0, 0, 3).Unix(), "c") // file that is not attached to a post - file2_1 := makeFile(post2_1, userId2, epoch.AddDate(0, 0, 4).Unix(), "d") // file 2 by user 1 + file1_1 := makeFile(post1_1, userId1, epoch.AddDate(0, 0, 1).Unix(), "a") // file 1 by user 1 + file1_2 := makeFile(post1_2, userId1, epoch.AddDate(0, 0, 2).Unix(), "b") // file 2 by user 1 + file1_3 := makeFile(&model.Post{}, userId1, epoch.AddDate(0, 0, 3).Unix(), "c") // file that is not attached to a post + file2_1 := makeFile(post2_1, userId2, epoch.AddDate(0, 0, 4).Unix(), "d") // file 2 by user 1 file2_2 := makeFile(post2_2, userId2, epoch.AddDate(0, 0, 5).Unix(), "e") // delete a file diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go index 24527c32ae..f3762494e1 100644 --- a/store/storetest/post_store.go +++ b/store/storetest/post_store.go @@ -88,6 +88,8 @@ func testPostStoreSave(t *testing.T, ss store.Store) { _, err := ss.Post().Save(&rootPost) require.Nil(t, err) + time.Sleep(2 * time.Millisecond) + replyPost := model.Post{} replyPost.ChannelId = rootPost.ChannelId replyPost.UserId = model.NewId() @@ -410,8 +412,7 @@ func testPostStoreUpdate(t *testing.T, ss store.Store) { require.Equal(t, ro1.Message, o1.Message, "Failed to save/get") - o1a := &model.Post{} - *o1a = *ro1 + o1a := ro1.Clone() o1a.Message = ro1.Message + "BBBBBBBBBB" _, err = ss.Post().Update(o1a, ro1) require.Nil(t, err) @@ -422,8 +423,7 @@ func testPostStoreUpdate(t *testing.T, ss store.Store) { ro1a := r1.Posts[o1.Id] require.Equal(t, ro1a.Message, o1a.Message, "Failed to update/get") - o2a := &model.Post{} - *o2a = *ro2 + o2a := ro2.Clone() o2a.Message = ro2.Message + "DDDDDDD" _, err = ss.Post().Update(o2a, ro2) require.Nil(t, err) @@ -434,8 +434,7 @@ func testPostStoreUpdate(t *testing.T, ss store.Store) { require.Equal(t, ro2a.Message, o2a.Message, "Failed to update/get") - o3a := &model.Post{} - *o3a = *ro3 + o3a := ro3.Clone() o3a.Message = ro3.Message + "WWWWWWW" _, err = ss.Post().Update(o3a, ro3) require.Nil(t, err) @@ -460,8 +459,7 @@ func testPostStoreUpdate(t *testing.T, ss store.Store) { require.Nil(t, err) ro4 := r4.Posts[o4.Id] - o4a := &model.Post{} - *o4a = *ro4 + o4a := ro4.Clone() o4a.Filenames = []string{} o4a.FileIds = []string{model.NewId()} _, err = ss.Post().Update(o4a, ro4) @@ -497,7 +495,7 @@ func testPostStoreDelete(t *testing.T, ss store.Store) { posts, _ := ss.Post().GetPostsCreatedAt(o1.ChannelId, o1.CreateAt) post := posts[0] - actual := post.Props[model.POST_PROPS_DELETE_BY] + actual := post.GetProp(model.POST_PROPS_DELETE_BY) assert.Equal(t, deleteByID, actual, "Expected (*Post).Props[model.POST_PROPS_DELETE_BY] to be %v but got %v.", deleteByID, actual) @@ -2291,16 +2289,13 @@ func testPostStoreOverwriteMultiple(t *testing.T, ss store.Store) { require.Equal(t, ro5.Filenames, o5.Filenames, "Failed to save/get") t.Run("overwrite changing message", func(t *testing.T) { - o1a := &model.Post{} - *o1a = *ro1 + o1a := ro1.Clone() o1a.Message = ro1.Message + "BBBBBBBBBB" - o2a := &model.Post{} - *o2a = *ro2 + o2a := ro2.Clone() o2a.Message = ro2.Message + "DDDDDDD" - o3a := &model.Post{} - *o3a = *ro3 + o3a := ro3.Clone() o3a.Message = ro3.Message + "WWWWWWW" _, err = ss.Post().OverwriteMultiple([]*model.Post{o1a, o2a, o3a}) @@ -2324,13 +2319,11 @@ func testPostStoreOverwriteMultiple(t *testing.T, ss store.Store) { }) t.Run("overwrite clearing filenames", func(t *testing.T) { - o4a := &model.Post{} - *o4a = *ro4 + o4a := ro4.Clone() o4a.Filenames = []string{} o4a.FileIds = []string{model.NewId()} - o5a := &model.Post{} - *o5a = *ro5 + o5a := ro5.Clone() o5a.Filenames = []string{} o5a.FileIds = []string{} @@ -2406,20 +2399,17 @@ func testPostStoreOverwrite(t *testing.T, ss store.Store) { require.Equal(t, ro4.Message, o4.Message, "Failed to save/get") t.Run("overwrite changing message", func(t *testing.T) { - o1a := &model.Post{} - *o1a = *ro1 + o1a := ro1.Clone() o1a.Message = ro1.Message + "BBBBBBBBBB" _, err = ss.Post().Overwrite(o1a) require.Nil(t, err) - o2a := &model.Post{} - *o2a = *ro2 + o2a := ro2.Clone() o2a.Message = ro2.Message + "DDDDDDD" _, err = ss.Post().Overwrite(o2a) require.Nil(t, err) - o3a := &model.Post{} - *o3a = *ro3 + o3a := ro3.Clone() o3a.Message = ro3.Message + "WWWWWWW" _, err = ss.Post().Overwrite(o3a) require.Nil(t, err) @@ -2442,8 +2432,7 @@ func testPostStoreOverwrite(t *testing.T, ss store.Store) { }) t.Run("overwrite clearing filenames", func(t *testing.T) { - o4a := &model.Post{} - *o4a = *ro4 + o4a := ro4.Clone() o4a.Filenames = []string{} o4a.FileIds = []string{model.NewId()} _, err = ss.Post().Overwrite(o4a) @@ -2863,8 +2852,7 @@ func testPostStoreGetDirectPostParentsForExportAfterDeleted(t *testing.T, ss sto p1, err = ss.Post().Save(p1) require.Nil(t, err) - o1a := &model.Post{} - *o1a = *p1 + o1a := p1.Clone() o1a.DeleteAt = 1 o1a.Message = p1.Message + "BBBBBBBBBB" _, err = ss.Post().Update(o1a, p1)