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.
Этот коммит содержится в:
Martin Kraft
2021-09-17 17:47:00 -04:00
коммит произвёл GitHub
родитель fd7bc9b4a0
Коммит 28ef5856ed
4 изменённых файлов: 81 добавлений и 28 удалений

Просмотреть файл

@@ -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)
})
}

Просмотреть файл

@@ -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
}

Просмотреть файл

@@ -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)

Просмотреть файл

@@ -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 ""
}