From fab63f8ba2afc62d8ede10cc1ef318a68fd4518a Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Thu, 8 Nov 2018 10:47:35 -0500 Subject: [PATCH] MM-12829 Check message attachments for emojis for post metadata (#9797) --- app/post_metadata.go | 62 +++++++--- app/post_metadata_test.go | 254 +++++++++++++++++++++++++++----------- 2 files changed, 230 insertions(+), 86 deletions(-) diff --git a/app/post_metadata.go b/app/post_metadata.go index 57e2246348..a1cc43b0c3 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -96,7 +96,7 @@ func (a *App) getEmojisAndReactionsForPost(post *model.Post) ([]*model.Emoji, [] return nil, nil, err } - emojis, err := a.getCustomEmojisForPost(post.Message, reactions) + emojis, err := a.getCustomEmojisForPost(post, reactions) if err != nil { return nil, nil, err } @@ -182,27 +182,55 @@ func (a *App) getImagesForPost(post *model.Post, imageURLs []string) map[string] return images } -func (a *App) getCustomEmojisForPost(message string, reactions []*model.Reaction) ([]*model.Emoji, *model.AppError) { +func getEmojiNamesForString(s string) []string { + names := model.EMOJI_PATTERN.FindAllString(s, -1) + + for i, name := range names { + names[i] = strings.Trim(name, ":") + } + + return names +} + +func getEmojiNamesForPost(post *model.Post, reactions []*model.Reaction) []string { + // Post message + names := getEmojiNamesForString(post.Message) + + // Reactions + for _, reaction := range reactions { + names = append(names, reaction.EmojiName) + } + + // Post attachments + for _, attachment := range post.Attachments() { + if attachment.Text != "" { + names = append(names, getEmojiNamesForString(attachment.Text)...) + } + + if attachment.Pretext != "" { + names = append(names, getEmojiNamesForString(attachment.Pretext)...) + } + + for _, field := range attachment.Fields { + if value, ok := field.Value.(string); ok { + names = append(names, getEmojiNamesForString(value)...) + } + } + } + + // Remove duplicates + names = model.RemoveDuplicateStrings(names) + + return names +} + +func (a *App) getCustomEmojisForPost(post *model.Post, reactions []*model.Reaction) ([]*model.Emoji, *model.AppError) { if !*a.Config().ServiceSettings.EnableCustomEmoji { // Only custom emoji are returned return []*model.Emoji{}, nil } - names := model.EMOJI_PATTERN.FindAllString(message, -1) - - for _, reaction := range reactions { - names = append(names, reaction.EmojiName) - } - - if len(names) == 0 { - return []*model.Emoji{}, nil - } - - names = model.RemoveDuplicateStrings(names) - - for i, name := range names { - names[i] = strings.Trim(name, ":") - } + names := getEmojiNamesForPost(post, reactions) return a.GetMultipleEmojiByName(names) } diff --git a/app/post_metadata_test.go b/app/post_metadata_test.go index 4ebb8773fd..0aa19e5709 100644 --- a/app/post_metadata_test.go +++ b/app/post_metadata_test.go @@ -127,6 +127,13 @@ func TestPreparePostForClient(t *testing.T) { UserId: th.BasicUser.Id, ChannelId: th.BasicChannel.Id, Message: ":" + emoji.Name + ": :taco:", + Props: map[string]interface{}{ + "attachments": []*model.SlackAttachment{ + { + Text: ":" + emoji.Name + ":", + }, + }, + }, }, th.BasicChannel, false) require.Nil(t, err) @@ -158,11 +165,19 @@ func TestPreparePostForClient(t *testing.T) { emoji1 := th.CreateEmoji() emoji2 := th.CreateEmoji() emoji3 := th.CreateEmoji() + emoji4 := th.CreateEmoji() post, err := th.App.CreatePost(&model.Post{ UserId: th.BasicUser.Id, ChannelId: th.BasicChannel.Id, Message: ":" + emoji3.Name + ": :taco:", + Props: map[string]interface{}{ + "attachments": []*model.SlackAttachment{ + { + Text: ":" + emoji4.Name + ":", + }, + }, + }, }, th.BasicChannel, false) require.Nil(t, err) @@ -175,7 +190,7 @@ func TestPreparePostForClient(t *testing.T) { require.Nil(t, err) t.Run("pupulates emojis", func(t *testing.T) { - assert.ElementsMatch(t, []*model.Emoji{emoji1, emoji2, emoji3}, clientPost.Metadata.Emojis, "should've populated post.Emojis") + assert.ElementsMatch(t, []*model.Emoji{emoji1, emoji2, emoji3, emoji4}, clientPost.Metadata.Emojis, "should've populated post.Emojis") }) t.Run("populates reaction counts", func(t *testing.T) { @@ -437,91 +452,147 @@ func testProxyOpenGraphImage(t *testing.T, th *TestHelper, shouldProxy bool) { }, clientPost.Metadata.Embeds) } -func TestGetCustomEmojisForPost_Message(t *testing.T) { - th := Setup().InitBasic() - defer th.TearDown() - - th.App.UpdateConfig(func(cfg *model.Config) { - *cfg.ServiceSettings.EnableCustomEmoji = true - }) - - emoji1 := th.CreateEmoji() - emoji2 := th.CreateEmoji() - emoji3 := th.CreateEmoji() - +func TestGetEmojiNamesForString(t *testing.T) { testCases := []struct { - Description string - Input string - Expected []*model.Emoji - SkipExpectations bool + Description string + Input string + Expected []string }{ { - Description: "no emojis", - Input: "this is a string", - Expected: []*model.Emoji{}, - SkipExpectations: true, + Description: "no emojis", + Input: "this is a string", + Expected: []string{}, }, { Description: "one emoji", - Input: "this is an :" + emoji1.Name + ": string", - Expected: []*model.Emoji{ - emoji1, - }, + Input: "this is an :emoji1: string", + Expected: []string{"emoji1"}, }, { Description: "two emojis", - Input: "this is a :" + emoji3.Name + ": :" + emoji2.Name + ": string", - Expected: []*model.Emoji{ - emoji3, - emoji2, - }, + Input: "this is a :emoji3: :emoji2: string", + Expected: []string{"emoji3", "emoji2"}, }, { Description: "punctuation around emojis", - Input: ":" + emoji3.Name + ":/:" + emoji1.Name + ": (:" + emoji2.Name + ":)", - Expected: []*model.Emoji{ - emoji3, - emoji1, - emoji2, - }, + Input: ":emoji3:/:emoji1: (:emoji2:)", + Expected: []string{"emoji3", "emoji1", "emoji2"}, }, { Description: "adjacent emojis", - Input: ":" + emoji3.Name + "::" + emoji1.Name + ":", - Expected: []*model.Emoji{ - emoji3, - emoji1, - }, + Input: ":emoji3::emoji1:", + Expected: []string{"emoji3", "emoji1"}, }, { Description: "duplicate emojis", - Input: "" + emoji1.Name + ": :" + emoji1.Name + ": :" + emoji1.Name + ": :" + emoji2.Name + ": :" + emoji2.Name + ": :" + emoji1.Name + ":", - Expected: []*model.Emoji{ - emoji1, - emoji2, - }, + Input: ":emoji1: :emoji1: :emoji1::emoji2::emoji2: :emoji1:", + Expected: []string{"emoji1", "emoji1", "emoji1", "emoji2", "emoji2", "emoji1"}, }, { Description: "fake emojis", Input: "these don't exist :tomato: :potato: :rotato:", - Expected: []*model.Emoji{}, - }, - { - Description: "fake and real emojis", - Input: ":tomato::" + emoji1.Name + ": :potato: :" + emoji2.Name + ":", - Expected: []*model.Emoji{ - emoji1, - emoji2, - }, + Expected: []string{"tomato", "potato", "rotato"}, }, } for _, testCase := range testCases { testCase := testCase t.Run(testCase.Description, func(t *testing.T) { - emojis, err := th.App.getCustomEmojisForPost(testCase.Input, nil) - assert.Nil(t, err, "failed to get emojis in message") - assert.ElementsMatch(t, emojis, testCase.Expected, "received incorrect emojis") + emojis := getEmojiNamesForString(testCase.Input) + assert.ElementsMatch(t, emojis, testCase.Expected, "received incorrect emoji names") + }) + } +} + +func TestGetEmojiNamesForPost(t *testing.T) { + testCases := []struct { + Description string + Post *model.Post + Reactions []*model.Reaction + Expected []string + }{ + { + Description: "no emojis", + Post: &model.Post{ + Message: "this is a post", + }, + Expected: []string{}, + }, + { + Description: "in post message", + Post: &model.Post{ + Message: "this is :emoji:", + }, + Expected: []string{"emoji"}, + }, + { + Description: "in reactions", + Post: &model.Post{}, + Reactions: []*model.Reaction{ + { + EmojiName: "emoji1", + }, + { + EmojiName: "emoji2", + }, + }, + Expected: []string{"emoji1", "emoji2"}, + }, + { + Description: "in message attachments", + Post: &model.Post{ + Message: "this is a post", + Props: map[string]interface{}{ + "attachments": []*model.SlackAttachment{ + { + Text: ":emoji1:", + Pretext: ":emoji2:", + }, + { + Fields: []*model.SlackAttachmentField{ + { + Value: ":emoji3:", + }, + { + Value: ":emoji4:", + }, + }, + }, + }, + }, + }, + Expected: []string{"emoji1", "emoji2", "emoji3", "emoji4"}, + }, + { + Description: "with duplicates", + Post: &model.Post{ + Message: "this is :emoji1", + Props: map[string]interface{}{ + "attachments": []*model.SlackAttachment{ + { + Text: ":emoji2:", + Pretext: ":emoji2:", + Fields: []*model.SlackAttachmentField{ + { + Value: ":emoji3:", + }, + { + Value: ":emoji1:", + }, + }, + }, + }, + }, + }, + Expected: []string{"emoji1", "emoji2", "emoji3"}, + }, + } + + for _, testCase := range testCases { + testCase := testCase + t.Run(testCase.Description, func(t *testing.T) { + emojis := getEmojiNamesForPost(testCase.Post, testCase.Reactions) + assert.ElementsMatch(t, emojis, testCase.Expected, "received incorrect emoji names") }) } } @@ -534,19 +605,64 @@ func TestGetCustomEmojisForPost(t *testing.T) { *cfg.ServiceSettings.EnableCustomEmoji = true }) - emoji1 := th.CreateEmoji() - emoji2 := th.CreateEmoji() - - reactions := []*model.Reaction{ - { - UserId: th.BasicUser.Id, - EmojiName: emoji1.Name, - }, + emojis := []*model.Emoji{ + th.CreateEmoji(), + th.CreateEmoji(), + th.CreateEmoji(), + th.CreateEmoji(), + th.CreateEmoji(), + th.CreateEmoji(), } - emojis, err := th.App.getCustomEmojisForPost(":"+emoji2.Name+":", reactions) - assert.Nil(t, err, "failed to get emojis for post") - assert.ElementsMatch(t, emojis, []*model.Emoji{emoji1, emoji2}, "received incorrect emojis") + t.Run("from different parts of the post", func(t *testing.T) { + reactions := []*model.Reaction{ + { + UserId: th.BasicUser.Id, + EmojiName: emojis[0].Name, + }, + } + + post := &model.Post{ + Message: ":" + emojis[1].Name + ":", + Props: map[string]interface{}{ + "attachments": []*model.SlackAttachment{ + { + Pretext: ":" + emojis[2].Name + ":", + Text: ":" + emojis[3].Name + ":", + Fields: []*model.SlackAttachmentField{ + { + Value: ":" + emojis[4].Name + ":", + }, + { + Value: ":" + emojis[5].Name + ":", + }, + }, + }, + }, + }, + } + + emojisForPost, err := th.App.getCustomEmojisForPost(post, reactions) + assert.Nil(t, err, "failed to get emojis for post") + assert.ElementsMatch(t, emojisForPost, emojis, "received incorrect emojis") + }) + + t.Run("with emojis that don't exist", func(t *testing.T) { + post := &model.Post{ + Message: ":secret: :" + emojis[0].Name + ":", + Props: map[string]interface{}{ + "attachments": []*model.SlackAttachment{ + { + Text: ":imaginary:", + }, + }, + }, + } + + emojisForPost, err := th.App.getCustomEmojisForPost(post, nil) + assert.Nil(t, err, "failed to get emojis for post") + assert.ElementsMatch(t, emojisForPost, []*model.Emoji{emojis[0]}, "received incorrect emojis") + }) } func TestGetFirstLinkAndImages(t *testing.T) {