diff --git a/app/post_metadata.go b/app/post_metadata.go index 9dbe12f179..bd34624d70 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/dyatlov/go-opengraph/opengraph" + "github.com/mattermost/mattermost-server/mlog" "github.com/mattermost/mattermost-server/model" "github.com/mattermost/mattermost-server/utils" "github.com/mattermost/mattermost-server/utils/markdown" @@ -51,106 +52,134 @@ func (a *App) PreparePostListForClient(originalList *model.PostList) (*model.Pos func (a *App) PreparePostForClient(originalPost *model.Post) (*model.Post, *model.AppError) { post := originalPost.Clone() - needReactionCounts := post.ReactionCounts == nil - needEmojis := post.Emojis == nil - needOpenGraphData := post.OpenGraphData == nil - needImageDimensions := post.ImageDimensions == nil - - // Get reactions to post - var reactions []*model.Reaction - if needReactionCounts || needEmojis { - var err *model.AppError - reactions, err = a.GetReactionsForPost(post.Id) - if err != nil { - return post, err - } - } - - if needReactionCounts { - post.ReactionCounts = model.CountReactions(reactions) - } - - // Get emojis for post - if needEmojis { - emojis, err := a.getCustomEmojisForPost(post.Message, reactions) - if err != nil { - return post, err - } - - post.Emojis = emojis - } - - // Get files for post - if post.FileInfos == nil { - fileInfos, err := a.GetFileInfosForPost(post.Id, false) - if err != nil { - return post, err - } - - post.FileInfos = fileInfos - } - - // Proxy image links in post + // Proxy image links before constructing metadata so that requests go through the proxy post = a.PostWithProxyAddedToImageURLs(post) - // Get OpenGraph and image metadata - if needOpenGraphData || needImageDimensions { - err := a.preparePostWithOpenGraphAndImageMetadata(post, needOpenGraphData, needImageDimensions) - if err != nil { - return post, err + if post.Metadata == nil { + post.Metadata = &model.PostMetadata{} + + // Emojis and reaction counts + if emojis, reactionCounts, err := a.getEmojisAndReactionCountsForPost(post); err != nil { + mlog.Warn("Failed to get emojis and reactions for a post", mlog.String("post_id", post.Id), mlog.Any("err", err)) + } else { + post.Metadata.Emojis = emojis + post.Metadata.ReactionCounts = reactionCounts } + + // Files + if fileInfos, err := a.GetFileInfosForPost(post.Id, false); err != nil { + mlog.Warn("Failed to get files for a post", mlog.String("post_id", post.Id), mlog.Any("err", err)) + } else { + post.Metadata.Files = fileInfos + } + + // Embeds and image dimensions + firstLink, images := getFirstLinkAndImages(post.Message) + + if embed, err := a.getEmbedForPost(post, firstLink); err != nil { + mlog.Warn("Failed to get embedded content for a post", mlog.String("post_id", post.Id), mlog.Any("err", err)) + } else if embed == nil { + post.Metadata.Embeds = []*model.PostEmbed{} + } else { + post.Metadata.Embeds = []*model.PostEmbed{embed} + } + + post.Metadata.Images = a.getImagesForPost(post, images) } return post, nil } -func (a *App) preparePostWithOpenGraphAndImageMetadata(post *model.Post, needOpenGraphData, needImageDimensions bool) *model.AppError { - var appError *model.AppError - - if needOpenGraphData { - post.OpenGraphData = []*opengraph.OpenGraph{} +func (a *App) getEmojisAndReactionCountsForPost(post *model.Post) ([]*model.Emoji, model.ReactionCounts, *model.AppError) { + reactions, err := a.GetReactionsForPost(post.Id) + if err != nil { + return nil, nil, err } - if needImageDimensions { - post.ImageDimensions = []*model.PostImageDimensions{} + emojis, err := a.getCustomEmojisForPost(post.Message, reactions) + if err != nil { + return nil, nil, err } - firstLink, images := getFirstLinkAndImages(post.Message) + return emojis, model.CountReactions(reactions), nil +} + +func (a *App) getEmbedForPost(post *model.Post, firstLink string) (*model.PostEmbed, error) { + if _, ok := post.Props["attachments"]; ok { + return &model.PostEmbed{ + Type: model.POST_EMBED_MESSAGE_ATTACHMENT, + }, nil + } - // Look at the first link to see if it's a web page or an image if firstLink != "" { - og, dimensions, err := a.getLinkMetadata(firstLink, true) + og, image, err := a.getLinkMetadata(firstLink, true) if err != nil { - // Keep going so that one bad link doesn't prevent other image dimensions from being sent to the client - appError = model.NewAppError("PreparePostForClient", "app.post.metadata.link.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, err } - if needOpenGraphData { - post.OpenGraphData = append(post.OpenGraphData, og) + if og != nil { + return &model.PostEmbed{ + Type: model.POST_EMBED_OPENGRAPH, + URL: firstLink, + Data: og, + }, nil } - if needImageDimensions { - post.ImageDimensions = append(post.ImageDimensions, dimensions) + if image != nil { + // Note that we're not passing the image info here since they'll be part of the PostMetadata.Images field + return &model.PostEmbed{ + Type: model.POST_EMBED_IMAGE, + URL: firstLink, + }, nil } } - if needImageDimensions { - // And dimensions for other images - for _, image := range images { - _, dimensions, err := a.getLinkMetadata(image, true) - if err != nil { - // Keep going so that one bad link doesn't prevent other image dimensions from being sent to the client - appError = model.NewAppError("PreparePostForClient", "app.post.metadata.link.app_error", nil, err.Error(), http.StatusInternalServerError) - continue - } + return nil, nil +} - if dimensions != nil { - post.ImageDimensions = append(post.ImageDimensions, dimensions) +func (a *App) getImagesForPost(post *model.Post, imageURLs []string) map[string]*model.PostImage { + images := map[string]*model.PostImage{} + + for _, embed := range post.Metadata.Embeds { + switch embed.Type { + case model.POST_EMBED_IMAGE: + // These dimensions will generally be cached by a previous call to getEmbedForPost + imageURLs = append(imageURLs, embed.URL) + + case model.POST_EMBED_MESSAGE_ATTACHMENT: + imageURLs = append(imageURLs, getImagesInMessageAttachments(post)...) + + case model.POST_EMBED_OPENGRAPH: + for _, image := range embed.Data.(*opengraph.OpenGraph).Images { + if image.Width != 0 || image.Height != 0 { + // The site has already told us the image dimensions + images[image.URL] = &model.PostImage{ + Width: int(image.Width), + Height: int(image.Height), + } + } else { + // The site did not specify its image dimensions + imageURLs = append(imageURLs, image.URL) + } } } } - return appError + // Removing duplicates isn't strictly since images is a map, but it feels safer to do it beforehand + if len(imageURLs) > 1 { + imageURLs = model.RemoveDuplicateStrings(imageURLs) + } + + for _, imageURL := range imageURLs { + if _, image, err := a.getLinkMetadata(imageURL, true); err != nil { + mlog.Warn("Failed to get dimensions of an image in a post", + mlog.String("post_id", post.Id), mlog.String("image_url", imageURL), mlog.Any("err", err)) + } else { + images[imageURL] = image + } + } + + return images } func (a *App) getCustomEmojisForPost(message string, reactions []*model.Reaction) ([]*model.Emoji, *model.AppError) { @@ -200,20 +229,37 @@ func getFirstLinkAndImages(str string) (string, []string) { return true }) - if len(images) > 1 { - images = model.RemoveDuplicateStrings(images) - } - return firstLink, images } -func (a *App) getLinkMetadata(requestURL string, useCache bool) (*opengraph.OpenGraph, *model.PostImageDimensions, error) { +func getImagesInMessageAttachments(post *model.Post) []string { + var images []string + + for _, attachment := range post.Attachments() { + _, imagesInText := getFirstLinkAndImages(attachment.Text) + images = append(images, imagesInText...) + + _, imagesInPretext := getFirstLinkAndImages(attachment.Pretext) + images = append(images, imagesInPretext...) + + for _, field := range attachment.Fields { + if value, ok := field.Value.(string); ok { + _, imagesInFieldValue := getFirstLinkAndImages(value) + images = append(images, imagesInFieldValue...) + } + } + } + + return images +} + +func (a *App) getLinkMetadata(requestURL string, useCache bool) (*opengraph.OpenGraph, *model.PostImage, error) { // Check cache if useCache { - og, dimensions, ok := getLinkMetadataFromCache(requestURL) + og, image, ok := getLinkMetadataFromCache(requestURL) if ok { - return og, dimensions, nil + return og, image, nil } } @@ -232,17 +278,17 @@ func (a *App) getLinkMetadata(requestURL string, useCache bool) (*opengraph.Open defer consumeAndClose(res) // Parse the data - og, dimensions, err := a.parseLinkMetadata(requestURL, res.Body, res.Header.Get("Content-Type")) + og, image, err := a.parseLinkMetadata(requestURL, res.Body, res.Header.Get("Content-Type")) // Write back to cache if useCache { - cacheLinkMetadata(requestURL, og, dimensions) + cacheLinkMetadata(requestURL, og, image) } - return og, dimensions, err + return og, image, err } -func getLinkMetadataFromCache(requestURL string) (*opengraph.OpenGraph, *model.PostImageDimensions, bool) { +func getLinkMetadataFromCache(requestURL string) (*opengraph.OpenGraph, *model.PostImage, bool) { cached, ok := linkCache.Get(requestURL) if !ok { return nil, nil, false @@ -251,28 +297,28 @@ func getLinkMetadataFromCache(requestURL string) (*opengraph.OpenGraph, *model.P switch v := cached.(type) { case *opengraph.OpenGraph: return v, nil, true - case *model.PostImageDimensions: + case *model.PostImage: return nil, v, true default: return nil, nil, true } } -func cacheLinkMetadata(requestURL string, og *opengraph.OpenGraph, dimensions *model.PostImageDimensions) { +func cacheLinkMetadata(requestURL string, og *opengraph.OpenGraph, image *model.PostImage) { var val interface{} if og != nil { val = og - } else if dimensions != nil { - val = dimensions + } else if image != nil { + val = image } linkCache.AddWithExpiresInSecs(requestURL, val, LINK_CACHE_DURATION) } -func (a *App) parseLinkMetadata(requestURL string, body io.Reader, contentType string) (*opengraph.OpenGraph, *model.PostImageDimensions, error) { +func (a *App) parseLinkMetadata(requestURL string, body io.Reader, contentType string) (*opengraph.OpenGraph, *model.PostImage, error) { if strings.HasPrefix(contentType, "image") { - dimensions, err := parseImageDimensions(requestURL, body) - return nil, dimensions, err + image, err := parseImages(body) + return nil, image, err } else if strings.HasPrefix(contentType, "text/html") { og := a.ParseOpenGraphMetadata(requestURL, body, contentType) @@ -289,17 +335,16 @@ func (a *App) parseLinkMetadata(requestURL string, body io.Reader, contentType s } } -func parseImageDimensions(requestURL string, body io.Reader) (*model.PostImageDimensions, error) { +func parseImages(body io.Reader) (*model.PostImage, error) { config, _, err := image.DecodeConfig(body) if err != nil { return nil, err } - dimensions := &model.PostImageDimensions{ - URL: requestURL, + image := &model.PostImage{ Width: config.Width, Height: config.Height, } - return dimensions, nil + return image, nil } diff --git a/app/post_metadata_test.go b/app/post_metadata_test.go index 232ca366e5..e21f8bdbda 100644 --- a/app/post_metadata_test.go +++ b/app/post_metadata_test.go @@ -41,20 +41,22 @@ func TestPreparePostForClient(t *testing.T) { clientPost, err := th.App.PreparePostForClient(post) require.Nil(t, err) - assert.NotEqual(t, clientPost, post, "should've returned a new post") - assert.Equal(t, message, post.Message, "shouldn't have mutated post.Message") - assert.NotEqual(t, nil, post.ReactionCounts, "shouldn't have mutated post.ReactionCounts") - assert.NotEqual(t, nil, post.FileInfos, "shouldn't have mutated post.FileInfos") - assert.NotEqual(t, nil, post.Emojis, "shouldn't have mutated post.Emojis") - assert.NotEqual(t, nil, post.ImageDimensions, "shouldn't have mutated post.ImageDimensions") - assert.NotEqual(t, nil, post.OpenGraphData, "shouldn't have mutated post.OpenGraphData") + t.Run("doesn't mutate provided post", func(t *testing.T) { + assert.NotEqual(t, clientPost, post, "should've returned a new post") - assert.Equal(t, clientPost.Message, post.Message, "shouldn't have changed Message") - assert.Len(t, post.ReactionCounts, 0, "should've populated ReactionCounts") - assert.Len(t, post.FileInfos, 0, "should've populated FileInfos") - assert.Len(t, post.Emojis, 0, "should've populated Emojis") - assert.Len(t, post.ImageDimensions, 0, "should've populated ImageDimensions") - assert.Len(t, post.OpenGraphData, 0, "should've populated OpenGraphData") + assert.Equal(t, message, post.Message, "shouldn't have mutated post.Message") + assert.Equal(t, (*model.PostMetadata)(nil), post.Metadata, "shouldn't have mutated post.Metadata") + }) + + t.Run("populates all fields", func(t *testing.T) { + assert.Equal(t, message, clientPost.Message, "shouldn't have changed Message") + assert.NotEqual(t, nil, clientPost.Metadata, "should've populated Metadata") + assert.Len(t, clientPost.Metadata.Embeds, 0, "should've populated Embeds") + assert.Len(t, clientPost.Metadata.ReactionCounts, 0, "should've populated ReactionCounts") + assert.Len(t, clientPost.Metadata.Files, 0, "should've populated Files") + assert.Len(t, clientPost.Metadata.Emojis, 0, "should've populated Emojis") + assert.Len(t, clientPost.Metadata.Images, 0, "should've populated Images") + }) }) t.Run("metadata already set", func(t *testing.T) { @@ -83,10 +85,10 @@ func TestPreparePostForClient(t *testing.T) { assert.Equal(t, model.ReactionCounts{ "smile": 1, - }, clientPost.ReactionCounts, "should've populated post.ReactionCounts") + }, clientPost.Metadata.ReactionCounts, "should've populated ReactionCounts") }) - t.Run("file infos", func(t *testing.T) { + t.Run("files", func(t *testing.T) { th := setup() defer th.TearDown() @@ -105,7 +107,7 @@ func TestPreparePostForClient(t *testing.T) { clientPost, err := th.App.PreparePostForClient(post) require.Nil(t, err) - assert.Equal(t, []*model.FileInfo{fileInfo}, clientPost.FileInfos, "should've populated post.FileInfos") + assert.Equal(t, []*model.FileInfo{fileInfo}, clientPost.Metadata.Files, "should've populated Files") }) t.Run("emojis without custom emojis enabled", func(t *testing.T) { @@ -132,10 +134,16 @@ func TestPreparePostForClient(t *testing.T) { clientPost, err := th.App.PreparePostForClient(post) require.Nil(t, err) - assert.Len(t, clientPost.ReactionCounts, 2, "should've populated post.ReactionCounts") - assert.Equal(t, 1, clientPost.ReactionCounts["smile"], "should've populated post.ReactionCounts for smile") - assert.Equal(t, 2, clientPost.ReactionCounts["angry"], "should've populated post.ReactionCounts for angry") - assert.ElementsMatch(t, []*model.Emoji{}, clientPost.Emojis, "should've populated empty post.Emojis") + t.Run("populates emojis", func(t *testing.T) { + assert.ElementsMatch(t, []*model.Emoji{}, clientPost.Metadata.Emojis, "should've populated empty Emojis") + }) + + t.Run("populates reaction counts", func(t *testing.T) { + reactionCounts := clientPost.Metadata.ReactionCounts + assert.Len(t, reactionCounts, 2, "should've populated ReactionCounts") + assert.Equal(t, 1, reactionCounts["smile"], "should've included 'smile' in ReactionCounts") + assert.Equal(t, 2, reactionCounts["angry"], "should've included 'angry' in ReactionCounts") + }) }) t.Run("emojis with custom emojis enabled", func(t *testing.T) { @@ -165,11 +173,17 @@ func TestPreparePostForClient(t *testing.T) { clientPost, err := th.App.PreparePostForClient(post) require.Nil(t, err) - assert.Len(t, clientPost.ReactionCounts, 3, "should've populated post.ReactionCounts") - assert.Equal(t, 1, clientPost.ReactionCounts[emoji1.Name], "should've populated post.ReactionCounts for emoji1") - assert.Equal(t, 2, clientPost.ReactionCounts[emoji2.Name], "should've populated post.ReactionCounts for emoji2") - assert.Equal(t, 1, clientPost.ReactionCounts["angry"], "should've populated post.ReactionCounts for angry") - assert.ElementsMatch(t, []*model.Emoji{emoji1, emoji2, emoji3}, clientPost.Emojis, "should've populated post.Emojis") + t.Run("pupulates emojis", func(t *testing.T) { + assert.ElementsMatch(t, []*model.Emoji{emoji1, emoji2, emoji3}, clientPost.Metadata.Emojis, "should've populated post.Emojis") + }) + + t.Run("populates reaction counts", func(t *testing.T) { + reactionCounts := clientPost.Metadata.ReactionCounts + assert.Len(t, reactionCounts, 3, "should've populated ReactionCounts") + assert.Equal(t, 1, reactionCounts[emoji1.Name], "should've included emoji1 in ReactionCounts") + assert.Equal(t, 2, reactionCounts[emoji2.Name], "should've included emoji2 in ReactionCounts") + assert.Equal(t, 1, reactionCounts["angry"], "should've included angry in ReactionCounts") + }) }) t.Run("markdown image dimensions", func(t *testing.T) { @@ -186,20 +200,35 @@ func TestPreparePostForClient(t *testing.T) { clientPost, err := th.App.PreparePostForClient(post) require.Nil(t, err) - assert.Len(t, clientPost.ImageDimensions, 2) - assert.Equal(t, &model.PostImageDimensions{ - URL: "https://github.com/hmhealey/test-files/raw/master/logoVertical.png", - Width: 1068, - Height: 552, - }, clientPost.ImageDimensions[0]) - assert.Equal(t, &model.PostImageDimensions{ - URL: "https://github.com/hmhealey/test-files/raw/master/icon.png", - Width: 501, - Height: 501, - }, clientPost.ImageDimensions[1]) + t.Run("populates image dimensions", func(t *testing.T) { + imageDimensions := clientPost.Metadata.Images + assert.Len(t, imageDimensions, 2) + assert.Equal(t, &model.PostImage{ + Width: 1068, + Height: 552, + }, imageDimensions["https://github.com/hmhealey/test-files/raw/master/logoVertical.png"]) + assert.Equal(t, &model.PostImage{ + Width: 501, + Height: 501, + }, imageDimensions["https://github.com/hmhealey/test-files/raw/master/icon.png"]) + }) }) - t.Run("linked image dimensions", func(t *testing.T) { + t.Run("proxy linked images", func(t *testing.T) { + th := setup() + defer th.TearDown() + + testProxyLinkedImage(t, th, false) + }) + + t.Run("proxy opengraph images", func(t *testing.T) { + th := setup() + defer th.TearDown() + + testProxyOpenGraphImage(t, th, false) + }) + + t.Run("image embed", func(t *testing.T) { th := setup() defer th.TearDown() @@ -214,23 +243,28 @@ func TestPreparePostForClient(t *testing.T) { clientPost, err := th.App.PreparePostForClient(post) require.Nil(t, err) - // Reminder that only the first link gets dimensions - assert.Len(t, clientPost.ImageDimensions, 1) - assert.Equal(t, &model.PostImageDimensions{ - URL: "https://github.com/hmhealey/test-files/raw/master/logoVertical.png", - Width: 1068, - Height: 552, - }, clientPost.ImageDimensions[0]) + // Reminder that only the first link gets an embed and dimensions + + t.Run("populates embeds", func(t *testing.T) { + assert.ElementsMatch(t, []*model.PostEmbed{ + { + Type: model.POST_EMBED_IMAGE, + URL: "https://github.com/hmhealey/test-files/raw/master/logoVertical.png", + }, + }, clientPost.Metadata.Embeds) + }) + + t.Run("populates image dimensions", func(t *testing.T) { + imageDimensions := clientPost.Metadata.Images + assert.Len(t, imageDimensions, 1) + assert.Equal(t, &model.PostImage{ + Width: 1068, + Height: 552, + }, imageDimensions["https://github.com/hmhealey/test-files/raw/master/logoVertical.png"]) + }) }) - t.Run("proxy linked images", func(t *testing.T) { - th := setup() - defer th.TearDown() - - testProxyLinkedImage(t, th, false) - }) - - t.Run("opengraph", func(t *testing.T) { + t.Run("opengraph embed", func(t *testing.T) { th := setup() defer th.TearDown() @@ -244,30 +278,73 @@ func TestPreparePostForClient(t *testing.T) { clientPost, err := th.App.PreparePostForClient(post) require.Nil(t, err) - assert.Len(t, clientPost.OpenGraphData, 1) - assert.Equal(t, &opengraph.OpenGraph{ - Description: "Contribute to hmhealey/test-files development by creating an account on GitHub.", - SiteName: "GitHub", - Title: "hmhealey/test-files", - Type: "object", - URL: "https://github.com/hmhealey/test-files", - Images: []*opengraph.Image{ + t.Run("populates embeds", func(t *testing.T) { + assert.ElementsMatch(t, []*model.PostEmbed{ { - URL: "https://avatars1.githubusercontent.com/u/3277310?s=400&v=4", + Type: model.POST_EMBED_OPENGRAPH, + URL: "https://github.com/hmhealey/test-files", + Data: &opengraph.OpenGraph{ + Description: "Contribute to hmhealey/test-files development by creating an account on GitHub.", + SiteName: "GitHub", + Title: "hmhealey/test-files", + Type: "object", + URL: "https://github.com/hmhealey/test-files", + Images: []*opengraph.Image{ + { + URL: "https://avatars1.githubusercontent.com/u/3277310?s=400&v=4", + }, + }, + }, }, - }, - }, clientPost.OpenGraphData[0]) + }, clientPost.Metadata.Embeds) + }) + + t.Run("populates image dimensions", func(t *testing.T) { + imageDimensions := clientPost.Metadata.Images + assert.Len(t, imageDimensions, 1) + assert.Equal(t, &model.PostImage{ + Width: 420, + Height: 420, + }, imageDimensions["https://avatars1.githubusercontent.com/u/3277310?s=400&v=4"]) + }) }) - t.Run("opengraph image dimensions", func(t *testing.T) { - // TODO - }) - - t.Run("proxy opengraph images", func(t *testing.T) { + t.Run("message attachment embed", func(t *testing.T) { th := setup() defer th.TearDown() - testProxyOpenGraphImage(t, th, false) + post, err := th.App.CreatePost(&model.Post{ + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Props: map[string]interface{}{ + "attachments": []interface{}{ + map[string]interface{}{ + "text": "![icon](https://github.com/hmhealey/test-files/raw/master/icon.png)", + }, + }, + }, + }, th.BasicChannel, false) + require.Nil(t, err) + + clientPost, err := th.App.PreparePostForClient(post) + require.Nil(t, err) + + t.Run("populates embeds", func(t *testing.T) { + assert.ElementsMatch(t, []*model.PostEmbed{ + { + Type: model.POST_EMBED_MESSAGE_ATTACHMENT, + }, + }, clientPost.Metadata.Embeds) + }) + + t.Run("populates image dimensions", func(t *testing.T) { + imageDimensions := clientPost.Metadata.Images + assert.Len(t, imageDimensions, 1) + assert.Equal(t, &model.PostImage{ + Width: 501, + Height: 501, + }, imageDimensions["https://github.com/hmhealey/test-files/raw/master/icon.png"]) + }) }) } @@ -346,15 +423,20 @@ func testProxyOpenGraphImage(t *testing.T, th *TestHelper, shouldProxy bool) { image.URL = "https://avatars1.githubusercontent.com/u/3277310?s=400&v=4" } - assert.Len(t, clientPost.OpenGraphData, 1) - assert.Equal(t, &opengraph.OpenGraph{ - Description: "Contribute to hmhealey/test-files development by creating an account on GitHub.", - SiteName: "GitHub", - Title: "hmhealey/test-files", - Type: "object", - URL: "https://github.com/hmhealey/test-files", - Images: []*opengraph.Image{image}, - }, clientPost.OpenGraphData[0]) + assert.ElementsMatch(t, []*model.PostEmbed{ + { + Type: model.POST_EMBED_OPENGRAPH, + URL: "https://github.com/hmhealey/test-files", + Data: &opengraph.OpenGraph{ + Description: "Contribute to hmhealey/test-files development by creating an account on GitHub.", + SiteName: "GitHub", + Title: "hmhealey/test-files", + Type: "object", + URL: "https://github.com/hmhealey/test-files", + Images: []*opengraph.Image{image}, + }, + }, + }, clientPost.Metadata.Embeds) } func TestGetCustomEmojisForPost_Message(t *testing.T) { @@ -503,7 +585,7 @@ func TestGetFirstLinkAndImages(t *testing.T) { "multiple images with duplicate": { Input: "this is a ![our logo](http://example.com/logo) and ![their logo](http://example.com/logo2) and ![my logo which is their logo](http://example.com/logo2)", ExpectedFirstLink: "", - ExpectedImages: []string{"http://example.com/logo", "http://example.com/logo2"}, + ExpectedImages: []string{"http://example.com/logo", "http://example.com/logo2", "http://example.com/logo2"}, }, "reference image": { Input: `this is a ![our logo][logo] @@ -534,6 +616,166 @@ func TestGetFirstLinkAndImages(t *testing.T) { } } +func TestGetImagesInMessageAttachments(t *testing.T) { + for _, test := range []struct { + Name string + Post *model.Post + Expected []string + }{ + { + Name: "no attachments", + Post: &model.Post{}, + Expected: []string{}, + }, + { + Name: "empty attachments", + Post: &model.Post{ + Props: map[string]interface{}{ + "attachments": []*model.SlackAttachment{}, + }, + }, + Expected: []string{}, + }, + { + Name: "attachment with no fields that can contain images", + Post: &model.Post{ + Props: map[string]interface{}{ + "attachments": []*model.SlackAttachment{ + { + Title: "This is the title", + }, + }, + }, + }, + Expected: []string{}, + }, + { + Name: "images in text", + Post: &model.Post{ + Props: map[string]interface{}{ + "attachments": []*model.SlackAttachment{ + { + Text: "![logo](https://example.com/logo) and ![icon](https://example.com/icon)", + }, + }, + }, + }, + Expected: []string{"https://example.com/logo", "https://example.com/icon"}, + }, + { + Name: "images in pretext", + Post: &model.Post{ + Props: map[string]interface{}{ + "attachments": []*model.SlackAttachment{ + { + Pretext: "![logo](https://example.com/logo1) and ![icon](https://example.com/icon1)", + }, + }, + }, + }, + Expected: []string{"https://example.com/logo1", "https://example.com/icon1"}, + }, + { + Name: "images in fields", + Post: &model.Post{ + Props: map[string]interface{}{ + "attachments": []*model.SlackAttachment{ + { + Fields: []*model.SlackAttachmentField{ + { + Value: "![logo](https://example.com/logo2) and ![icon](https://example.com/icon2)", + }, + }, + }, + }, + }, + }, + Expected: []string{"https://example.com/logo2", "https://example.com/icon2"}, + }, + { + Name: "images in multiple fields", + Post: &model.Post{ + Props: map[string]interface{}{ + "attachments": []*model.SlackAttachment{ + { + Fields: []*model.SlackAttachmentField{ + { + Value: "![logo](https://example.com/logo)", + }, + { + Value: "![icon](https://example.com/icon)", + }, + }, + }, + }, + }, + }, + Expected: []string{"https://example.com/logo", "https://example.com/icon"}, + }, + { + Name: "non-string field", + Post: &model.Post{ + Props: map[string]interface{}{ + "attachments": []*model.SlackAttachment{ + { + Fields: []*model.SlackAttachmentField{ + { + Value: 77, + }, + }, + }, + }, + }, + }, + Expected: []string{}, + }, + { + Name: "images in multiple locations", + Post: &model.Post{ + Props: map[string]interface{}{ + "attachments": []*model.SlackAttachment{ + { + Text: "![text](https://example.com/text)", + Pretext: "![pretext](https://example.com/pretext)", + Fields: []*model.SlackAttachmentField{ + { + Value: "![field1](https://example.com/field1)", + }, + { + Value: "![field2](https://example.com/field2)", + }, + }, + }, + }, + }, + }, + Expected: []string{"https://example.com/text", "https://example.com/pretext", "https://example.com/field1", "https://example.com/field2"}, + }, + { + Name: "multiple attachments", + Post: &model.Post{ + Props: map[string]interface{}{ + "attachments": []*model.SlackAttachment{ + { + Text: "![logo](https://example.com/logo)", + }, + { + Text: "![icon](https://example.com/icon)", + }, + }, + }, + }, + Expected: []string{"https://example.com/logo", "https://example.com/icon"}, + }, + } { + t.Run(test.Name, func(t *testing.T) { + images := getImagesInMessageAttachments(test.Post) + + assert.ElementsMatch(t, images, test.Expected) + }) + } +} + func TestParseLinkMetadata(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() @@ -565,8 +807,7 @@ func TestParseLinkMetadata(t *testing.T) { assert.Nil(t, err) assert.Nil(t, og) - assert.Equal(t, &model.PostImageDimensions{ - URL: imageURL, + assert.Equal(t, &model.PostImage{ Width: 408, Height: 336, }, dimensions) @@ -608,29 +849,25 @@ func TestParseLinkMetadata(t *testing.T) { }) } -func TestParseImageDimensions(t *testing.T) { +func TestParseImages(t *testing.T) { for name, testCase := range map[string]struct { FileName string - URL string ExpectedWidth int ExpectedHeight int ExpectError bool }{ "png": { FileName: "test.png", - URL: "https://example.com/test.png", ExpectedWidth: 408, ExpectedHeight: 336, }, "animated gif": { FileName: "testgif.gif", - URL: "http://example.com/test.gif?foo=bar", ExpectedWidth: 118, ExpectedHeight: 118, }, "not an image": { FileName: "README.md", - URL: "https://example.com/test.png", ExpectError: true, }, } { @@ -638,14 +875,13 @@ func TestParseImageDimensions(t *testing.T) { file, err := testutils.ReadTestFile(testCase.FileName) require.Nil(t, err) - dimensions, err := parseImageDimensions(testCase.URL, bytes.NewReader(file)) + dimensions, err := parseImages(bytes.NewReader(file)) if testCase.ExpectError { require.NotNil(t, err) } else { require.Nil(t, err) require.NotNil(t, dimensions) - require.Equal(t, testCase.URL, dimensions.URL) require.Equal(t, testCase.ExpectedWidth, dimensions.Width) require.Equal(t, testCase.ExpectedHeight, dimensions.Height) } diff --git a/model/post.go b/model/post.go index a64391d4da..0a4e0c076f 100644 --- a/model/post.go +++ b/model/post.go @@ -11,7 +11,6 @@ import ( "strings" "unicode/utf8" - "github.com/dyatlov/go-opengraph/opengraph" "github.com/mattermost/mattermost-server/utils/markdown" ) @@ -83,18 +82,8 @@ type Post struct { PendingPostId string `json:"pending_post_id" db:"-"` HasReactions bool `json:"has_reactions,omitempty"` // Deprecated, do not use this field any more - // Transient fields populated before sending posts to the client - ReactionCounts ReactionCounts `json:"reaction_counts" db:"-"` - FileInfos []*FileInfo `json:"file_infos" db:"-"` - ImageDimensions []*PostImageDimensions `json:"image_dimensions" db:"-"` - OpenGraphData []*opengraph.OpenGraph `json:"opengraph_data" db:"-"` - Emojis []*Emoji `json:"emojis" db:"-"` -} - -type PostImageDimensions struct { - URL string `json:"url"` - Width int `json:"width"` - Height int `json:"height"` + // Transient data populated before sending a post to the client + Metadata *PostMetadata `json:"metadata" db:"-"` } type PostEphemeral struct { diff --git a/model/post_embed.go b/model/post_embed.go new file mode 100644 index 0000000000..6e8e33cadb --- /dev/null +++ b/model/post_embed.go @@ -0,0 +1,22 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package model + +const ( + POST_EMBED_IMAGE PostEmbedType = "image" + POST_EMBED_MESSAGE_ATTACHMENT PostEmbedType = "message_attachment" + POST_EMBED_OPENGRAPH PostEmbedType = "opengraph" +) + +type PostEmbedType string + +type PostEmbed struct { + Type PostEmbedType `json:"type"` + + // The URL of the embedded content. Used for image and OpenGraph embeds. + URL string `json:"url,omitempty"` + + // Any additional data for the embedded content. Only used for OpenGraph embeds. + Data interface{} `json:"data,omitempty"` +} diff --git a/model/post_metadata.go b/model/post_metadata.go new file mode 100644 index 0000000000..1ec0695fae --- /dev/null +++ b/model/post_metadata.go @@ -0,0 +1,28 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package model + +type PostMetadata struct { + // An array of the information required to render additional details about the contents of this post. + Embeds []*PostEmbed `json:"embeds,omitempty"` + + // An arrayof the custom emojis used in the post or in reactions to the post. + Emojis []*Emoji `json:"emojis,omitempty"` + + // An array of information about the file attachments on the post. + Files []*FileInfo `json:"files,omitempty"` + + // A map of image URL to information about all external images in the post. This includes image embeds, + // inline Markdown images, OpenGraph images, and message attachment images, but it does not contain the dimensions + // of file attachments which are contained in PostMetadata.FileInfos. + Images map[string]*PostImage `json:"images,omitempty"` + + // A map of emoji names to a count of users that reacted with the given emoji. + ReactionCounts ReactionCounts `json:"reaction_counts,omitempty"` +} + +type PostImage struct { + Width int `json:"width"` + Height int `json:"height"` +}