From 28ef5856ed5f1c3cfb29a869cb6f14662f964ff3 Mon Sep 17 00:00:00 2001 From: Martin Kraft Date: Fri, 17 Sep 2021 17:47:00 -0400 Subject: [PATCH] MM-38081: Fix for disappearing permalink previews (#18400) * MM-38081: Fix for disappearing previews. * MM-38081: Update method signature in tests. * MM-38081: Adds test replicating bug. --- api4/system_test.go | 1 - app/post_metadata.go | 10 ++--- app/post_metadata_test.go | 91 +++++++++++++++++++++++++++++---------- model/post.go | 7 +++ 4 files changed, 81 insertions(+), 28 deletions(-) diff --git a/api4/system_test.go b/api4/system_test.go index 37c861c2e3..1cb14165bb 100644 --- a/api4/system_test.go +++ b/api4/system_test.go @@ -775,7 +775,6 @@ func TestPushNotificationAck(t *testing.T) { handler.ServeHTTP(resp, req) assert.Equal(t, http.StatusForbidden, resp.Code) - fmt.Printf("DEBUG/resp.Body: %+v\n", resp.Body) assert.NotNil(t, resp.Body) }) } diff --git a/app/post_metadata.go b/app/post_metadata.go index a591f2d23a..0151b0b370 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -218,7 +218,7 @@ func (a *App) getEmbedForPost(post *model.Post, firstLink string, isNewPost bool return nil, nil } - og, image, permalink, err := a.getLinkMetadata(firstLink, post.CreateAt, isNewPost) + og, image, permalink, err := a.getLinkMetadata(firstLink, post.CreateAt, isNewPost, post.GetPreviewedPostProp()) if err != nil { return nil, err } @@ -289,7 +289,7 @@ func (a *App) getImagesForPost(post *model.Post, imageURLs []string, isNewPost b } for _, imageURL := range imageURLs { - if _, image, _, err := a.getLinkMetadata(imageURL, post.CreateAt, isNewPost); err != nil { + if _, image, _, err := a.getLinkMetadata(imageURL, post.CreateAt, isNewPost, post.GetPreviewedPostProp()); err != nil { mlog.Debug("Failed to get dimensions of an image in a post", mlog.String("post_id", post.Id), mlog.String("image_url", imageURL), mlog.Err(err)) } else if image != nil { @@ -453,7 +453,7 @@ func looksLikeAPermalink(url, siteURL string) bool { return matched } -func (a *App) getLinkMetadata(requestURL string, timestamp int64, isNewPost bool) (*opengraph.OpenGraph, *model.PostImage, *model.Permalink, error) { +func (a *App) getLinkMetadata(requestURL string, timestamp int64, isNewPost bool, previewedPostPropVal string) (*opengraph.OpenGraph, *model.PostImage, *model.Permalink, error) { requestURL = resolveMetadataURL(requestURL, a.GetSiteURL()) timestamp = model.FloorToNearestHour(timestamp) @@ -464,14 +464,14 @@ func (a *App) getLinkMetadata(requestURL string, timestamp int64, isNewPost bool permalink = nil } - if ok { + if ok && previewedPostPropVal == "" { return og, image, permalink, nil } // Check the database if this isn't a new post. If it is a new post and the data is cached, it should be in memory. if !isNewPost { og, image, ok = a.getLinkMetadataFromDatabase(requestURL, timestamp) - if ok { + if ok && previewedPostPropVal == "" { cacheLinkMetadata(requestURL, timestamp, og, image, nil) return og, image, nil, nil } diff --git a/app/post_metadata_test.go b/app/post_metadata_test.go index 83a1fc0069..7656c566dc 100644 --- a/app/post_metadata_test.go +++ b/app/post_metadata_test.go @@ -582,6 +582,53 @@ func TestPreparePostForClient(t *testing.T) { preview := firstEmbed.Data.(*model.PreviewPost) require.Equal(t, referencedPost.Id, preview.PostID) }) + + t.Run("permalink preview renders after toggling off the feature", func(t *testing.T) { + th := setup(t) + defer th.TearDown() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.SiteURL = "http://mymattermost.com" + }) + + th.Context.Session().UserId = th.BasicUser.Id + + referencedPost, err := th.App.CreatePost(th.Context, &model.Post{ + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: "hello world", + }, th.BasicChannel, false, true) + require.Nil(t, err) + + link := fmt.Sprintf("%s/%s/pl/%s", *th.App.Config().ServiceSettings.SiteURL, th.BasicTeam.Name, referencedPost.Id) + + previewPost, err := th.App.CreatePost(th.Context, &model.Post{ + UserId: th.BasicUser.Id, + ChannelId: th.BasicChannel.Id, + Message: link, + }, th.BasicChannel, false, true) + require.Nil(t, err) + + clientPost := th.App.PreparePostForClient(previewPost, false, false) + firstEmbed := clientPost.Metadata.Embeds[0] + preview := firstEmbed.Data.(*model.PreviewPost) + require.Equal(t, referencedPost.Id, preview.PostID) + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.EnablePermalinkPreviews = false + }) + + th.App.PreparePostForClient(previewPost, false, false) + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.EnablePermalinkPreviews = true + }) + + clientPost2 := th.App.PreparePostForClient(previewPost, false, false) + firstEmbed2 := clientPost2.Metadata.Embeds[0] + preview2 := firstEmbed2.Data.(*model.PreviewPost) + require.Equal(t, referencedPost.Id, preview2.PostID) + }) } func TestPreparePostForClientWithImageProxy(t *testing.T) { @@ -1729,7 +1776,7 @@ func TestGetLinkMetadata(t *testing.T) { _, _, ok = th.App.getLinkMetadataFromDatabase(requestURL, timestamp) require.False(t, ok, "data should not exist in database") - og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false) + og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false, "") require.NotNil(t, og) assert.Nil(t, img) @@ -1744,7 +1791,7 @@ func TestGetLinkMetadata(t *testing.T) { _, _, ok = th.App.getLinkMetadataFromDatabase(requestURL, timestamp) require.False(t, ok, "data should not exist in database") - og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp+60*1000, false) + og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp+60*1000, false, "") require.NotNil(t, og) assert.Nil(t, img) @@ -1761,7 +1808,7 @@ func TestGetLinkMetadata(t *testing.T) { _, _, ok = th.App.getLinkMetadataFromDatabase(differentURL, timestamp) require.False(t, ok, "data should not exist in database") - og, img, _, err := th.App.getLinkMetadata(differentURL, timestamp, false) + og, img, _, err := th.App.getLinkMetadata(differentURL, timestamp, false, "") assert.Nil(t, og) assert.Nil(t, img) @@ -1777,7 +1824,7 @@ func TestGetLinkMetadata(t *testing.T) { _, _, ok = th.App.getLinkMetadataFromDatabase(requestURL, differentTimestamp) require.False(t, ok, "data should not exist in database") - og, img, _, err := th.App.getLinkMetadata(requestURL, differentTimestamp, false) + og, img, _, err := th.App.getLinkMetadata(requestURL, differentTimestamp, false, "") assert.Nil(t, og) assert.Nil(t, img) @@ -1804,7 +1851,7 @@ func TestGetLinkMetadata(t *testing.T) { _, _, ok = th.App.getLinkMetadataFromDatabase(requestURL, timestamp) require.True(t, ok, "data should already exist in database") - og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false) + og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false, "") require.NotNil(t, og) assert.Nil(t, img) @@ -1821,7 +1868,7 @@ func TestGetLinkMetadata(t *testing.T) { _, _, ok = th.App.getLinkMetadataFromDatabase(requestURL, timestamp) require.True(t, ok, "data should already exist in database") - og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp+60*1000, false) + og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp+60*1000, false, "") require.NotNil(t, og) assert.Nil(t, img) @@ -1840,7 +1887,7 @@ func TestGetLinkMetadata(t *testing.T) { _, _, ok = th.App.getLinkMetadataFromDatabase(differentURL, timestamp) require.False(t, ok, "data should not exist in database") - og, img, _, err := th.App.getLinkMetadata(differentURL, timestamp, false) + og, img, _, err := th.App.getLinkMetadata(differentURL, timestamp, false, "") assert.Nil(t, og) assert.Nil(t, img) @@ -1858,7 +1905,7 @@ func TestGetLinkMetadata(t *testing.T) { _, _, ok = th.App.getLinkMetadataFromDatabase(requestURL, differentTimestamp) require.False(t, ok, "data should not exist in database") - og, img, _, err := th.App.getLinkMetadata(requestURL, differentTimestamp, false) + og, img, _, err := th.App.getLinkMetadata(requestURL, differentTimestamp, false, "") assert.Nil(t, og) assert.Nil(t, img) @@ -1879,7 +1926,7 @@ func TestGetLinkMetadata(t *testing.T) { _, _, ok = th.App.getLinkMetadataFromDatabase(requestURL, timestamp) require.False(t, ok, "data should not exist in database") - og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false) + og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false, "") assert.NotNil(t, og) assert.Nil(t, img) @@ -1899,7 +1946,7 @@ func TestGetLinkMetadata(t *testing.T) { _, _, ok = th.App.getLinkMetadataFromDatabase(requestURL, timestamp) require.False(t, ok, "data should not exist in database") - og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false) + og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false, "") assert.NotNil(t, og) assert.Nil(t, img) @@ -1927,7 +1974,7 @@ func TestGetLinkMetadata(t *testing.T) { _, _, ok = th.App.getLinkMetadataFromDatabase(requestURL, timestamp) require.False(t, ok, "data should not exist in database") - og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false) + og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false, "") assert.Nil(t, og) assert.NotNil(t, img) @@ -1955,7 +2002,7 @@ func TestGetLinkMetadata(t *testing.T) { _, _, ok = th.App.getLinkMetadataFromDatabase(requestURL, timestamp) require.False(t, ok, "data should not exist in database") - og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false) + og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false, "") assert.Nil(t, og) assert.Nil(t, img) @@ -1985,7 +2032,7 @@ func TestGetLinkMetadata(t *testing.T) { _, _, ok = th.App.getLinkMetadataFromDatabase(requestURL, timestamp) require.False(t, ok, "data should not exist in database") - og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false) + og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false, "") assert.Nil(t, og) assert.Nil(t, img) @@ -2019,7 +2066,7 @@ func TestGetLinkMetadata(t *testing.T) { _, _, ok = th.App.getLinkMetadataFromDatabase(requestURL, timestamp) require.False(t, ok, "data should not exist in database") - og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false) + og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false, "") assert.Nil(t, og) assert.Nil(t, img) @@ -2050,7 +2097,7 @@ func TestGetLinkMetadata(t *testing.T) { _, _, ok = th.App.getLinkMetadataFromDatabase(requestURL, timestamp) require.False(t, ok, "data should not exist in database") - _, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false) + _, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false, "") require.NoError(t, err) _, _, _, ok = getLinkMetadataFromCache(requestURL, timestamp) @@ -2072,7 +2119,7 @@ func TestGetLinkMetadata(t *testing.T) { requestURL := server.URL + "/json?name=" + t.Name() timestamp := int64(1547510400000) - og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false) + og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false, "") assert.Nil(t, og) assert.Nil(t, img) assert.NoError(t, err) @@ -2087,7 +2134,7 @@ func TestGetLinkMetadata(t *testing.T) { cacheLinkMetadata(requestURL, timestamp, &opengraph.OpenGraph{Title: "cached"}, nil, nil) - og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, true) + og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, true, "") assert.NotNil(t, og) assert.Nil(t, img) assert.NoError(t, err) @@ -2102,7 +2149,7 @@ func TestGetLinkMetadata(t *testing.T) { th.App.saveLinkMetadataToDatabase(requestURL, timestamp, &opengraph.OpenGraph{Title: "cached"}, nil) - og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, true) + og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, true, "") assert.Nil(t, og) assert.Nil(t, img) assert.NoError(t, err) @@ -2125,7 +2172,7 @@ func TestGetLinkMetadata(t *testing.T) { requestURL := "/image?height=200&width=300&name=" + t.Name() timestamp := int64(1547510400000) - og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false) + og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false, "") assert.Nil(t, og) assert.NotNil(t, img) assert.NoError(t, err) @@ -2153,7 +2200,7 @@ func TestGetLinkMetadata(t *testing.T) { requestURL := server.URL + "/image?height=200&width=300&name=" + t.Name() timestamp := int64(1547510400000) - og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false) + og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, false, "") assert.Nil(t, og) assert.Nil(t, img) assert.Error(t, err) @@ -2163,7 +2210,7 @@ func TestGetLinkMetadata(t *testing.T) { requestURL = th.App.GetSiteURL() + "/api/v4/image?url=" + url.QueryEscape(requestURL) // Note that this request still fails while testing because the request made by the image proxy is blocked - og, img, _, err = th.App.getLinkMetadata(requestURL, timestamp, false) + og, img, _, err = th.App.getLinkMetadata(requestURL, timestamp, false, "") assert.Nil(t, og) assert.Nil(t, img) assert.Error(t, err) @@ -2177,7 +2224,7 @@ func TestGetLinkMetadata(t *testing.T) { requestURL := server.URL + "/mixed?name=" + t.Name() timestamp := int64(1547510400000) - og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, true) + og, img, _, err := th.App.getLinkMetadata(requestURL, timestamp, true, "") assert.Nil(t, og) assert.NotNil(t, img) assert.NoError(t, err) diff --git a/model/post.go b/model/post.go index b81cd8e846..695971cf50 100644 --- a/model/post.go +++ b/model/post.go @@ -696,3 +696,10 @@ func (o *Post) GetPreviewPost() *PreviewPost { } return nil } + +func (o *Post) GetPreviewedPostProp() string { + if val, ok := o.GetProp(PostPropsPreviewedPost).(string); ok { + return val + } + return "" +}