diff --git a/api4/post.go b/api4/post.go index 8509e1c1be..7b9f41c4af 100644 --- a/api4/post.go +++ b/api4/post.go @@ -97,7 +97,7 @@ func createEphemeralPost(c *Context, w http.ResponseWriter, r *http.Request) { rp := c.App.SendEphemeralPost(ephRequest.UserID, c.App.PostWithProxyRemovedFromImageURLs(ephRequest.Post)) w.WriteHeader(http.StatusCreated) - w.Write([]byte(c.App.PreparePostForClient(rp).ToJson())) + w.Write([]byte(c.App.PreparePostForClient(rp, true).ToJson())) } func getPostsForChannel(c *Context, w http.ResponseWriter, r *http.Request) { @@ -259,7 +259,7 @@ func getPost(c *Context, w http.ResponseWriter, r *http.Request) { } } - post = c.App.PreparePostForClient(post) + post = c.App.PreparePostForClient(post, false) if c.HandleEtag(post.Etag(), "Get Post", w, r) { return diff --git a/app/post.go b/app/post.go index 1144dee6c5..5a4570d3ee 100644 --- a/app/post.go +++ b/app/post.go @@ -281,7 +281,7 @@ func (a *App) CreatePost(post *model.Post, channel *model.Channel, triggerWebhoo // Normally, we would let the API layer call PreparePostForClient, but we do it here since it also needs // to be done when we send the post over the websocket in handlePostEvents - rpost = a.PreparePostForClient(rpost) + rpost = a.PreparePostForClient(rpost, true) if err := a.handlePostEvents(rpost, user, channel, triggerWebhooks, parentPostList); err != nil { mlog.Error("Failed to handle post events", mlog.Err(err)) @@ -401,7 +401,7 @@ func (a *App) SendEphemeralPost(userId string, post *model.Post) *model.Post { } message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_EPHEMERAL_MESSAGE, "", post.ChannelId, userId, nil) - message.Add("post", a.PreparePostForClient(post).ToJson()) + message.Add("post", a.PreparePostForClient(post, true).ToJson()) a.Publish(message) return post @@ -498,7 +498,7 @@ func (a *App) UpdatePost(post *model.Post, safeUpdate bool) (*model.Post, *model }) } - rpost = a.PreparePostForClient(rpost) + rpost = a.PreparePostForClient(rpost, false) a.sendUpdatedPostEvent(rpost) @@ -665,7 +665,7 @@ func (a *App) DeletePost(postId, deleteByID string) (*model.Post, *model.AppErro } message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_POST_DELETED, "", post.ChannelId, "", nil) - message.Add("post", a.PreparePostForClient(post).ToJson()) + message.Add("post", a.PreparePostForClient(post, false).ToJson()) a.Publish(message) a.Srv.Go(func() { diff --git a/app/post_metadata.go b/app/post_metadata.go index a92f3c08c4..da61a8b76b 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -41,7 +41,7 @@ func (a *App) PreparePostListForClient(originalList *model.PostList) *model.Post } for id, originalPost := range originalList.Posts { - post := a.PreparePostForClient(originalPost) + post := a.PreparePostForClient(originalPost, false) list.Posts[id] = post } @@ -49,7 +49,7 @@ func (a *App) PreparePostListForClient(originalList *model.PostList) *model.Post return list } -func (a *App) PreparePostForClient(originalPost *model.Post) *model.Post { +func (a *App) PreparePostForClient(originalPost *model.Post, isNewPost bool) *model.Post { post := originalPost.Clone() // Proxy image links before constructing metadata so that requests go through the proxy @@ -79,7 +79,7 @@ func (a *App) PreparePostForClient(originalPost *model.Post) *model.Post { // Embeds and image dimensions firstLink, images := getFirstLinkAndImages(post.Message) - if embed, err := a.getEmbedForPost(post, firstLink); err != nil { + if embed, err := a.getEmbedForPost(post, firstLink, isNewPost); 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{} @@ -87,7 +87,7 @@ func (a *App) PreparePostForClient(originalPost *model.Post) *model.Post { post.Metadata.Embeds = []*model.PostEmbed{embed} } - post.Metadata.Images = a.getImagesForPost(post, images) + post.Metadata.Images = a.getImagesForPost(post, images, isNewPost) return post } @@ -118,7 +118,7 @@ func (a *App) getEmojisAndReactionsForPost(post *model.Post) ([]*model.Emoji, [] return emojis, reactions, nil } -func (a *App) getEmbedForPost(post *model.Post, firstLink string) (*model.PostEmbed, error) { +func (a *App) getEmbedForPost(post *model.Post, firstLink string, isNewPost bool) (*model.PostEmbed, error) { if _, ok := post.Props["attachments"]; ok { return &model.PostEmbed{ Type: model.POST_EMBED_MESSAGE_ATTACHMENT, @@ -129,7 +129,7 @@ func (a *App) getEmbedForPost(post *model.Post, firstLink string) (*model.PostEm return nil, nil } - og, image, err := a.getLinkMetadata(firstLink, true) + og, image, err := a.getLinkMetadata(firstLink, post.CreateAt, isNewPost) if err != nil { return nil, err } @@ -153,7 +153,7 @@ func (a *App) getEmbedForPost(post *model.Post, firstLink string) (*model.PostEm return nil, nil } -func (a *App) getImagesForPost(post *model.Post, imageURLs []string) map[string]*model.PostImage { +func (a *App) getImagesForPost(post *model.Post, imageURLs []string, isNewPost bool) map[string]*model.PostImage { images := map[string]*model.PostImage{} for _, embed := range post.Metadata.Embeds { @@ -187,7 +187,7 @@ func (a *App) getImagesForPost(post *model.Post, imageURLs []string) map[string] } for _, imageURL := range imageURLs { - if _, image, err := a.getLinkMetadata(imageURL, true); err != nil { + if _, image, err := a.getLinkMetadata(imageURL, post.CreateAt, isNewPost); 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 { @@ -317,14 +317,23 @@ func getImagesInMessageAttachments(post *model.Post) []string { return images } -func (a *App) getLinkMetadata(requestURL string, useCache bool) (*opengraph.OpenGraph, *model.PostImage, error) { +func (a *App) getLinkMetadata(requestURL string, timestamp int64, isNewPost bool) (*opengraph.OpenGraph, *model.PostImage, error) { requestURL = resolveMetadataURL(requestURL, a.GetSiteURL()) - // Check cache - if useCache { - og, image, ok := getLinkMetadataFromCache(requestURL) + timestamp = model.FloorToNearestHour(timestamp) + // Check cache + og, image, ok := getLinkMetadataFromCache(requestURL, timestamp) + if ok { + return og, image, 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 { + cacheLinkMetadata(requestURL, timestamp, og, image) + return og, image, nil } } @@ -345,12 +354,12 @@ func (a *App) getLinkMetadata(requestURL string, useCache bool) (*opengraph.Open defer res.Body.Close() // Parse the data - og, image, 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, image) - } + // Write back to cache and database + cacheLinkMetadata(requestURL, timestamp, og, image) + + a.saveLinkMetadataToDatabase(requestURL, timestamp, og, image) return og, image, err } @@ -370,8 +379,8 @@ func resolveMetadataURL(requestURL string, siteURL string) string { return resolved.String() } -func getLinkMetadataFromCache(requestURL string) (*opengraph.OpenGraph, *model.PostImage, bool) { - cached, ok := linkCache.Get(requestURL) +func getLinkMetadataFromCache(requestURL string, timestamp int64) (*opengraph.OpenGraph, *model.PostImage, bool) { + cached, ok := linkCache.Get(model.GenerateLinkMetadataHash(requestURL, timestamp)) if !ok { return nil, nil, false } @@ -386,7 +395,47 @@ func getLinkMetadataFromCache(requestURL string) (*opengraph.OpenGraph, *model.P } } -func cacheLinkMetadata(requestURL string, og *opengraph.OpenGraph, image *model.PostImage) { +func (a *App) getLinkMetadataFromDatabase(requestURL string, timestamp int64) (*opengraph.OpenGraph, *model.PostImage, bool) { + result := <-a.Srv.Store.LinkMetadata().Get(requestURL, timestamp) + if result.Err != nil { + return nil, nil, false + } + + data := result.Data.(*model.LinkMetadata).Data + + switch v := data.(type) { + case *opengraph.OpenGraph: + return v, nil, true + case *model.PostImage: + return nil, v, true + default: + return nil, nil, true + } +} + +func (a *App) saveLinkMetadataToDatabase(requestURL string, timestamp int64, og *opengraph.OpenGraph, image *model.PostImage) { + metadata := &model.LinkMetadata{ + URL: requestURL, + Timestamp: timestamp, + } + + if og != nil { + metadata.Type = model.LINK_METADATA_TYPE_OPENGRAPH + metadata.Data = og + } else if image != nil { + metadata.Type = model.LINK_METADATA_TYPE_IMAGE + metadata.Data = image + } else { + metadata.Type = model.LINK_METADATA_TYPE_NONE + } + + result := <-a.Srv.Store.LinkMetadata().Save(metadata) + if result.Err != nil { + mlog.Warn("Failed to write link metadata", mlog.String("request_url", requestURL), mlog.Err(result.Err)) + } +} + +func cacheLinkMetadata(requestURL string, timestamp int64, og *opengraph.OpenGraph, image *model.PostImage) { var val interface{} if og != nil { val = og @@ -394,7 +443,7 @@ func cacheLinkMetadata(requestURL string, og *opengraph.OpenGraph, image *model. val = image } - linkCache.AddWithExpiresInSecs(requestURL, val, LINK_CACHE_DURATION) + linkCache.AddWithExpiresInSecs(model.GenerateLinkMetadataHash(requestURL, timestamp), val, LINK_CACHE_DURATION) } func (a *App) parseLinkMetadata(requestURL string, body io.Reader, contentType string) (*opengraph.OpenGraph, *model.PostImage, error) { diff --git a/app/post_metadata_test.go b/app/post_metadata_test.go index b543fb7f6f..10488c05ab 100644 --- a/app/post_metadata_test.go +++ b/app/post_metadata_test.go @@ -6,7 +6,12 @@ package app import ( "bytes" "fmt" + "image" + "image/png" "io" + "net/http" + "net/http/httptest" + "strconv" "strings" "testing" "time" @@ -74,7 +79,7 @@ func TestPreparePostForClient(t *testing.T) { Message: message, } - clientPost := th.App.PreparePostForClient(post) + clientPost := th.App.PreparePostForClient(post, false) t.Run("doesn't mutate provided post", func(t *testing.T) { assert.NotEqual(t, clientPost, post, "should've returned a new post") @@ -100,7 +105,7 @@ func TestPreparePostForClient(t *testing.T) { post := th.CreatePost(th.BasicChannel) - clientPost := th.App.PreparePostForClient(post) + clientPost := th.App.PreparePostForClient(post, false) assert.False(t, clientPost == post, "should've returned a new post") assert.Equal(t, clientPost, post, "shouldn't have changed any metadata") @@ -116,7 +121,7 @@ func TestPreparePostForClient(t *testing.T) { reaction3 := th.AddReactionToPost(post, th.BasicUser2, "ice_cream") post.HasReactions = true - clientPost := th.App.PreparePostForClient(post) + clientPost := th.App.PreparePostForClient(post, false) assert.Len(t, clientPost.Metadata.Reactions, 3, "should've populated Reactions") assert.Equal(t, reaction1, clientPost.Metadata.Reactions[0], "first reaction is incorrect") @@ -140,7 +145,7 @@ func TestPreparePostForClient(t *testing.T) { fileInfo.PostId = post.Id - clientPost := th.App.PreparePostForClient(post) + clientPost := th.App.PreparePostForClient(post, false) assert.Equal(t, []*model.FileInfo{fileInfo}, clientPost.Metadata.Files, "should've populated Files") }) @@ -174,7 +179,7 @@ func TestPreparePostForClient(t *testing.T) { th.AddReactionToPost(post, th.BasicUser2, "angry") post.HasReactions = true - clientPost := th.App.PreparePostForClient(post) + clientPost := th.App.PreparePostForClient(post, false) t.Run("populates emojis", func(t *testing.T) { assert.ElementsMatch(t, []*model.Emoji{}, clientPost.Metadata.Emojis, "should've populated empty Emojis") @@ -219,7 +224,7 @@ func TestPreparePostForClient(t *testing.T) { th.AddReactionToPost(post, th.BasicUser2, "angry") post.HasReactions = true - clientPost := th.App.PreparePostForClient(post) + clientPost := th.App.PreparePostForClient(post, false) t.Run("pupulates emojis", func(t *testing.T) { assert.ElementsMatch(t, []*model.Emoji{emoji1, emoji2, emoji3, emoji4}, clientPost.Metadata.Emojis, "should've populated post.Emojis") @@ -242,7 +247,7 @@ func TestPreparePostForClient(t *testing.T) { }, th.BasicChannel, false) require.Nil(t, err) - clientPost := th.App.PreparePostForClient(post) + clientPost := th.App.PreparePostForClient(post, false) t.Run("populates image dimensions", func(t *testing.T) { imageDimensions := clientPost.Metadata.Images @@ -284,7 +289,7 @@ func TestPreparePostForClient(t *testing.T) { }, th.BasicChannel, false) require.Nil(t, err) - clientPost := th.App.PreparePostForClient(post) + clientPost := th.App.PreparePostForClient(post, false) // Reminder that only the first link gets an embed and dimensions @@ -318,7 +323,7 @@ func TestPreparePostForClient(t *testing.T) { }, th.BasicChannel, false) require.Nil(t, err) - clientPost := th.App.PreparePostForClient(post) + clientPost := th.App.PreparePostForClient(post, false) t.Run("populates embeds", func(t *testing.T) { assert.ElementsMatch(t, []*model.PostEmbed{ @@ -368,7 +373,7 @@ func TestPreparePostForClient(t *testing.T) { }, th.BasicChannel, false) require.Nil(t, err) - clientPost := th.App.PreparePostForClient(post) + clientPost := th.App.PreparePostForClient(post, false) t.Run("populates embeds", func(t *testing.T) { assert.ElementsMatch(t, []*model.PostEmbed{ @@ -397,7 +402,7 @@ func TestPreparePostForClient(t *testing.T) { }) post := th.CreatePost(th.BasicChannel) - post = th.App.PreparePostForClient(post) + post = th.App.PreparePostForClient(post, false) assert.Nil(t, post.Metadata) @@ -449,7 +454,7 @@ func testProxyLinkedImage(t *testing.T, th *TestHelper, shouldProxy bool) { Message: fmt.Sprintf(postTemplate, imageURL), } - clientPost := th.App.PreparePostForClient(post) + clientPost := th.App.PreparePostForClient(post, false) if shouldProxy { assert.Equal(t, fmt.Sprintf(postTemplate, imageURL), post.Message, "should not have mutated original post") @@ -467,7 +472,7 @@ func testProxyOpenGraphImage(t *testing.T, th *TestHelper, shouldProxy bool) { }, th.BasicChannel, false) require.Nil(t, err) - embeds := th.App.PreparePostForClient(post).Metadata.Embeds + embeds := th.App.PreparePostForClient(post, false).Metadata.Embeds require.Len(t, embeds, 1, "should have one embed") embed := embeds[0] @@ -991,6 +996,404 @@ func TestGetImagesInMessageAttachments(t *testing.T) { } } +func TestGetLinkMetadata(t *testing.T) { + setup := func() *TestHelper { + th := Setup().InitBasic() + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "127.0.0.1" + }) + + linkCache.Purge() + + return th + } + th := Setup().InitBasic() + defer th.TearDown() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + params := r.URL.Query() + + if strings.HasPrefix(r.URL.Path, "/image") { + height, _ := strconv.ParseInt(params["height"][0], 10, 0) + width, _ := strconv.ParseInt(params["width"][0], 10, 0) + + img := image.NewGray(image.Rect(0, 0, int(width), int(height))) + + var encoder png.Encoder + + encoder.Encode(w, img) + } else if strings.HasPrefix(r.URL.Path, "/opengraph") { + w.Header().Set("Content-Type", "text/html") + + w.Write([]byte(` + + + + + + + `)) + } else if strings.HasPrefix(r.URL.Path, "/json") { + w.Header().Set("Content-Type", "application/json") + + w.Write([]byte("true")) + } else { + w.WriteHeader(http.StatusInternalServerError) + } + })) + defer server.Close() + + t.Run("in-memory cache", func(t *testing.T) { + th := setup() + defer th.TearDown() + + requestURL := server.URL + "/cached" + timestamp := int64(1547510400000) + title := "from cache" + + cacheLinkMetadata(requestURL, timestamp, &opengraph.OpenGraph{Title: title}, nil) + + t.Run("should use cache if cached entry exists", func(t *testing.T) { + _, _, ok := getLinkMetadataFromCache(requestURL, timestamp) + require.True(t, ok, "data should already exist in in-memory cache") + + _, _, 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) + + require.NotNil(t, og) + assert.Nil(t, img) + assert.Nil(t, err) + assert.Equal(t, title, og.Title) + }) + + t.Run("should use cache if cached entry exists near time", func(t *testing.T) { + _, _, ok := getLinkMetadataFromCache(requestURL, timestamp) + require.True(t, ok, "data should already exist in in-memory cache") + + _, _, 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) + + require.NotNil(t, og) + assert.Nil(t, img) + assert.Nil(t, err) + assert.Equal(t, title, og.Title) + }) + + t.Run("should not use cache if URL is different", func(t *testing.T) { + differentURL := server.URL + "/other" + + _, _, ok := getLinkMetadataFromCache(differentURL, timestamp) + require.False(t, ok, "data should not exist in in-memory cache") + + _, _, 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) + + assert.Nil(t, og) + assert.Nil(t, img) + assert.Nil(t, err) + }) + + t.Run("should not use cache if timestamp is different", func(t *testing.T) { + differentTimestamp := timestamp + 60*60*1000 + + _, _, ok := getLinkMetadataFromCache(requestURL, differentTimestamp) + require.False(t, ok, "data should not exist in in-memory cache") + + _, _, 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) + + assert.Nil(t, og) + assert.Nil(t, img) + assert.Nil(t, err) + }) + }) + + t.Run("database cache", func(t *testing.T) { + th := setup() + defer th.TearDown() + + requestURL := server.URL + timestamp := int64(1547510400000) + title := "from database" + + th.App.saveLinkMetadataToDatabase(requestURL, timestamp, &opengraph.OpenGraph{Title: title}, nil) + + t.Run("should use database if saved entry exists", func(t *testing.T) { + linkCache.Purge() + + _, _, ok := getLinkMetadataFromCache(requestURL, timestamp) + require.False(t, ok, "data should not exist in in-memory cache") + + _, _, 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) + + require.NotNil(t, og) + assert.Nil(t, img) + assert.Nil(t, err) + assert.Equal(t, title, og.Title) + }) + + t.Run("should use database if saved entry exists near time", func(t *testing.T) { + linkCache.Purge() + + _, _, ok := getLinkMetadataFromCache(requestURL, timestamp) + require.False(t, ok, "data should not exist in in-memory cache") + + _, _, 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) + + require.NotNil(t, og) + assert.Nil(t, img) + assert.Nil(t, err) + assert.Equal(t, title, og.Title) + }) + + t.Run("should not use database if URL is different", func(t *testing.T) { + linkCache.Purge() + + differentURL := requestURL + "/other" + + _, _, ok := getLinkMetadataFromCache(requestURL, timestamp) + require.False(t, ok, "data should not exist in in-memory cache") + + _, _, 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) + + assert.Nil(t, og) + assert.Nil(t, img) + assert.Nil(t, err) + }) + + t.Run("should not use database if timestamp is different", func(t *testing.T) { + linkCache.Purge() + + differentTimestamp := timestamp + 60*60*1000 + + _, _, ok := getLinkMetadataFromCache(requestURL, timestamp) + require.False(t, ok, "data should not exist in in-memory cache") + + _, _, 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) + + assert.Nil(t, og) + assert.Nil(t, img) + assert.Nil(t, err) + }) + }) + + t.Run("should get data from remote source", func(t *testing.T) { + th := setup() + defer th.TearDown() + + requestURL := server.URL + "/opengraph?title=Remote&name=" + t.Name() + timestamp := int64(1547510400000) + + _, _, ok := getLinkMetadataFromCache(requestURL, timestamp) + require.False(t, ok, "data should not exist in in-memory cache") + + _, _, 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) + + assert.NotNil(t, og) + assert.Nil(t, img) + assert.Nil(t, err) + }) + + t.Run("should cache OpenGraph results", func(t *testing.T) { + th := setup() + defer th.TearDown() + + requestURL := server.URL + "/opengraph?title=Remote&name=" + t.Name() + timestamp := int64(1547510400000) + + _, _, ok := getLinkMetadataFromCache(requestURL, timestamp) + require.False(t, ok, "data should not exist in in-memory cache") + + _, _, 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) + + assert.NotNil(t, og) + assert.Nil(t, img) + assert.Nil(t, err) + + fromCache, _, ok := getLinkMetadataFromCache(requestURL, timestamp) + assert.True(t, ok) + assert.Exactly(t, og, fromCache) + + fromDatabase, _, ok := th.App.getLinkMetadataFromDatabase(requestURL, timestamp) + assert.True(t, ok) + assert.Exactly(t, og, fromDatabase) + }) + + t.Run("should cache image results", func(t *testing.T) { + th := setup() + defer th.TearDown() + + requestURL := server.URL + "/image?height=300&width=400&name=" + t.Name() + timestamp := int64(1547510400000) + + _, _, ok := getLinkMetadataFromCache(requestURL, timestamp) + require.False(t, ok, "data should not exist in in-memory cache") + + _, _, 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) + + assert.Nil(t, og) + assert.NotNil(t, img) + assert.Nil(t, err) + + _, fromCache, ok := getLinkMetadataFromCache(requestURL, timestamp) + assert.True(t, ok) + assert.Exactly(t, img, fromCache) + + _, fromDatabase, ok := th.App.getLinkMetadataFromDatabase(requestURL, timestamp) + assert.True(t, ok) + assert.Exactly(t, img, fromDatabase) + }) + + t.Run("should cache error results", func(t *testing.T) { + th := setup() + defer th.TearDown() + + requestURL := server.URL + "/error" + timestamp := int64(1547510400000) + + _, _, ok := getLinkMetadataFromCache(requestURL, timestamp) + require.False(t, ok, "data should not exist in in-memory cache") + + _, _, 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) + + assert.Nil(t, og) + assert.Nil(t, img) + assert.Nil(t, err) + + ogFromCache, imgFromCache, ok := getLinkMetadataFromCache(requestURL, timestamp) + assert.True(t, ok) + assert.Nil(t, ogFromCache) + assert.Nil(t, imgFromCache) + + ogFromDatabase, imageFromDatabase, ok := th.App.getLinkMetadataFromDatabase(requestURL, timestamp) + assert.True(t, ok) + assert.Nil(t, ogFromDatabase) + assert.Nil(t, imageFromDatabase) + }) + + t.Run("should cache database results in memory", func(t *testing.T) { + th := setup() + defer th.TearDown() + + requestURL := server.URL + "/image?height=300&width=400&name=" + t.Name() + timestamp := int64(1547510400000) + + _, _, ok := getLinkMetadataFromCache(requestURL, timestamp) + require.False(t, ok, "data should not exist in in-memory cache") + + _, _, ok = th.App.getLinkMetadataFromDatabase(requestURL, timestamp) + require.False(t, ok, "data should not exist in database") + + _, img, err := th.App.getLinkMetadata(requestURL, timestamp, false) + require.Nil(t, err) + + _, _, ok = getLinkMetadataFromCache(requestURL, timestamp) + require.True(t, ok, "data should now exist in in-memory cache") + + linkCache.Purge() + _, _, ok = getLinkMetadataFromCache(requestURL, timestamp) + require.False(t, ok, "data should no longer exist in in-memory cache") + + _, fromDatabase, ok := th.App.getLinkMetadataFromDatabase(requestURL, timestamp) + assert.True(t, ok, "data should be be in in-memory cache again") + assert.Exactly(t, img, fromDatabase) + }) + + t.Run("should reject non-html, non-image response", func(t *testing.T) { + th := setup() + defer th.TearDown() + + requestURL := server.URL + "/json?name=" + t.Name() + timestamp := int64(1547510400000) + + og, img, err := th.App.getLinkMetadata(requestURL, timestamp, false) + assert.Nil(t, og) + assert.Nil(t, img) + assert.Nil(t, err) + }) + + t.Run("should check in-memory cache for new post", func(t *testing.T) { + th := setup() + defer th.TearDown() + + requestURL := server.URL + "/error?name=" + t.Name() + timestamp := int64(1547510400000) + + cacheLinkMetadata(requestURL, timestamp, &opengraph.OpenGraph{Title: "cached"}, nil) + + og, img, err := th.App.getLinkMetadata(requestURL, timestamp, true) + assert.NotNil(t, og) + assert.Nil(t, img) + assert.Nil(t, err) + }) + + t.Run("should skip database cache for new post", func(t *testing.T) { + th := setup() + defer th.TearDown() + + requestURL := server.URL + "/error?name=" + t.Name() + timestamp := int64(1547510400000) + + th.App.saveLinkMetadataToDatabase(requestURL, timestamp, &opengraph.OpenGraph{Title: "cached"}, nil) + + og, img, err := th.App.getLinkMetadata(requestURL, timestamp, true) + assert.Nil(t, og) + assert.Nil(t, img) + assert.Nil(t, err) + }) + + t.Run("should resolve relative URL", func(t *testing.T) { + th := setup() + defer th.TearDown() + + // Fake the SiteURL to have the relative URL resolve to the external server + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.SiteURL = server.URL + }) + + requestURL := "/image?height=200&width=300&name=" + t.Name() + timestamp := int64(1547510400000) + + og, img, err := th.App.getLinkMetadata(requestURL, timestamp, false) + assert.Nil(t, og) + assert.NotNil(t, img) + assert.Nil(t, err) + }) +} + func TestResolveMetadataURL(t *testing.T) { for _, test := range []struct { Name string diff --git a/app/reaction.go b/app/reaction.go index 5b4e916222..c447fa8b5e 100644 --- a/app/reaction.go +++ b/app/reaction.go @@ -143,7 +143,7 @@ func (a *App) sendReactionEvent(event string, reaction *model.Reaction, post *mo post.HasReactions = hasReactions post.UpdateAt = model.GetMillis() - clientPost := a.PreparePostForClient(post) + clientPost := a.PreparePostForClient(post, false) umessage := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_POST_EDITED, "", post.ChannelId, "", nil) umessage.Add("post", clientPost.ToJson()) diff --git a/i18n/en.json b/i18n/en.json index c792c341ee..42fc5b1ef4 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -4522,6 +4522,30 @@ "id": "model.license_record.is_valid.id.app_error", "translation": "Invalid value for id when uploading a license." }, + { + "id": "model.link_metadata.is_valid.data.app_error", + "translation": "Link metadata data cannot be nil" + }, + { + "id": "model.link_metadata.is_valid.data_type.app_error", + "translation": "Link metadata data does not match the given type" + }, + { + "id": "model.link_metadata.is_valid.id.app_error", + "translation": "Link metadata id invalid" + }, + { + "id": "model.link_metadata.is_valid.timestamp.app_error", + "translation": "Link metadata timestamp must be nonzero and rounded to the nearest hour" + }, + { + "id": "model.link_metadata.is_valid.type.app_error", + "translation": "Invalid link metadata type" + }, + { + "id": "model.link_metadata.is_valid.url.app_error", + "translation": "Link metadata URL must be set" + }, { "id": "model.oauth.is_valid.app_id.app_error", "translation": "Invalid app id" diff --git a/model/link_metadata.go b/model/link_metadata.go new file mode 100644 index 0000000000..0488289295 --- /dev/null +++ b/model/link_metadata.go @@ -0,0 +1,148 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package model + +import ( + "encoding/binary" + "encoding/json" + "hash/fnv" + "net/http" + "time" + + "github.com/dyatlov/go-opengraph/opengraph" +) + +const ( + LINK_METADATA_TYPE_IMAGE LinkMetadataType = "image" + LINK_METADATA_TYPE_NONE LinkMetadataType = "none" + LINK_METADATA_TYPE_OPENGRAPH LinkMetadataType = "opengraph" +) + +type LinkMetadataType string + +// LinkMetadata stores arbitrary data about a link posted in a message. This includes dimensions of linked images +// and OpenGraph metadata. +type LinkMetadata struct { + // Hash is a value computed from the URL and Timestamp for use as a primary key in the database. + Hash int64 + + URL string + Timestamp int64 + Type LinkMetadataType + + // Data is the actual metadata for the link. It should contain data of one of the following types: + // - *model.PostImage if the linked content is an image + // - *opengraph.OpenGraph if the linked content is an HTML document + // - nil if the linked content has no metadata + Data interface{} +} + +func (o *LinkMetadata) PreSave() { + o.Hash = GenerateLinkMetadataHash(o.URL, o.Timestamp) +} + +func (o *LinkMetadata) IsValid() *AppError { + if o.URL == "" { + return NewAppError("LinkMetadata.IsValid", "model.link_metadata.is_valid.url.app_error", nil, "", http.StatusBadRequest) + } + + if o.Timestamp == 0 || !isRoundedToNearestHour(o.Timestamp) { + return NewAppError("LinkMetadata.IsValid", "model.link_metadata.is_valid.timestamp.app_error", nil, "", http.StatusBadRequest) + } + + switch o.Type { + case LINK_METADATA_TYPE_IMAGE: + if o.Data == nil { + return NewAppError("LinkMetadata.IsValid", "model.link_metadata.is_valid.data.app_error", nil, "", http.StatusBadRequest) + } + + if _, ok := o.Data.(*PostImage); !ok { + return NewAppError("LinkMetadata.IsValid", "model.link_metadata.is_valid.data_type.app_error", nil, "", http.StatusBadRequest) + } + case LINK_METADATA_TYPE_NONE: + if o.Data != nil { + return NewAppError("LinkMetadata.IsValid", "model.link_metadata.is_valid.data_type.app_error", nil, "", http.StatusBadRequest) + } + case LINK_METADATA_TYPE_OPENGRAPH: + if o.Data == nil { + return NewAppError("LinkMetadata.IsValid", "model.link_metadata.is_valid.data.app_error", nil, "", http.StatusBadRequest) + } + + if _, ok := o.Data.(*opengraph.OpenGraph); !ok { + return NewAppError("LinkMetadata.IsValid", "model.link_metadata.is_valid.data_type.app_error", nil, "", http.StatusBadRequest) + } + default: + return NewAppError("LinkMetadata.IsValid", "model.link_metadata.is_valid.type.app_error", nil, "", http.StatusBadRequest) + } + + return nil +} + +// DeserializeDataToConcreteType converts o.Data from JSON into properly structured data. This is intended to be used +// after getting a LinkMetadata object that has been stored in the database. +func (o *LinkMetadata) DeserializeDataToConcreteType() error { + var b []byte + switch t := o.Data.(type) { + case []byte: + // MySQL uses a byte slice for JSON + b = t + case string: + // Postgres uses a string for JSON + b = []byte(t) + } + + if b == nil { + // Data doesn't need to be fixed + return nil + } + + var data interface{} + var err error + + switch o.Type { + case LINK_METADATA_TYPE_IMAGE: + image := &PostImage{} + + err = json.Unmarshal(b, &image) + + data = image + case LINK_METADATA_TYPE_OPENGRAPH: + og := &opengraph.OpenGraph{} + + json.Unmarshal(b, &og) + + data = og + } + + if err != nil { + return err + } + + o.Data = data + + return nil +} + +// FloorToNearestHour takes a timestamp (in milliseconds) and returns it rounded to the previous hour in UTC. +func FloorToNearestHour(ms int64) int64 { + t := time.Unix(0, ms*int64(1000*1000)) + + return time.Date(t.Year(), t.Month(), t.Day(), t.Hour(), 0, 0, 0, t.Location()).UnixNano() / int64(time.Millisecond) +} + +// isRoundedToNearestHour returns true if the given timestamp (in milliseconds) has been rounded to the nearest hour in UTC. +func isRoundedToNearestHour(ms int64) bool { + return FloorToNearestHour(ms) == ms +} + +// GenerateLinkMetadataHash generates a unique hash for a given URL and timestamp for use as a database key. +func GenerateLinkMetadataHash(url string, timestamp int64) int64 { + hash := fnv.New32() + + // Note that we ignore write errors here because the Hash interface says that its Write will never return an error + binary.Write(hash, binary.LittleEndian, timestamp) + hash.Write([]byte(url)) + + return int64(hash.Sum32()) +} diff --git a/model/link_metadata_test.go b/model/link_metadata_test.go new file mode 100644 index 0000000000..1d9e095c83 --- /dev/null +++ b/model/link_metadata_test.go @@ -0,0 +1,227 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package model + +import ( + "encoding/json" + "testing" + + "github.com/dyatlov/go-opengraph/opengraph" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestLinkMetadataIsValid(t *testing.T) { + for _, test := range []struct { + Name string + Metadata *LinkMetadata + Expected bool + }{ + { + Name: "should be valid image metadata", + Metadata: &LinkMetadata{ + URL: "http://example.com", + Timestamp: 1546300800000, + Type: LINK_METADATA_TYPE_IMAGE, + Data: &PostImage{}, + }, + Expected: true, + }, + { + Name: "should be valid opengraph metadata", + Metadata: &LinkMetadata{ + URL: "http://example.com", + Timestamp: 1546300800000, + Type: LINK_METADATA_TYPE_OPENGRAPH, + Data: &opengraph.OpenGraph{}, + }, + Expected: true, + }, + { + Name: "should be valid with no metadata", + Metadata: &LinkMetadata{ + URL: "http://example.com", + Timestamp: 1546300800000, + Type: LINK_METADATA_TYPE_NONE, + Data: nil, + }, + Expected: true, + }, + { + Name: "should be invalid because of empty URL", + Metadata: &LinkMetadata{ + Timestamp: 1546300800000, + Type: LINK_METADATA_TYPE_IMAGE, + Data: &PostImage{}, + }, + Expected: false, + }, + { + Name: "should be invalid because of empty timestamp", + Metadata: &LinkMetadata{ + URL: "http://example.com", + Type: LINK_METADATA_TYPE_IMAGE, + Data: &PostImage{}, + }, + Expected: false, + }, + { + Name: "should be invalid because of unrounded timestamp", + Metadata: &LinkMetadata{ + URL: "http://example.com", + Timestamp: 1546300800001, + Type: LINK_METADATA_TYPE_IMAGE, + Data: &PostImage{}, + }, + Expected: false, + }, + { + Name: "should be invalid because of invalid type", + Metadata: &LinkMetadata{ + URL: "http://example.com", + Timestamp: 1546300800000, + Type: "garbage", + Data: &PostImage{}, + }, + Expected: false, + }, + { + Name: "should be invalid because of empty data", + Metadata: &LinkMetadata{ + URL: "http://example.com", + Timestamp: 1546300800000, + Type: LINK_METADATA_TYPE_IMAGE, + }, + Expected: false, + }, + { + Name: "should be invalid because of mismatched data and type, image type and opengraph data", + Metadata: &LinkMetadata{ + URL: "http://example.com", + Timestamp: 1546300800000, + Type: LINK_METADATA_TYPE_IMAGE, + Data: &opengraph.OpenGraph{}, + }, + Expected: false, + }, + { + Name: "should be invalid because of mismatched data and type, opengraph type and image data", + Metadata: &LinkMetadata{ + URL: "http://example.com", + Timestamp: 1546300800000, + Type: LINK_METADATA_TYPE_OPENGRAPH, + Data: &PostImage{}, + }, + Expected: false, + }, + { + Name: "should be invalid because of mismatched data and type, image type and random data", + Metadata: &LinkMetadata{ + URL: "http://example.com", + Timestamp: 1546300800000, + Type: LINK_METADATA_TYPE_OPENGRAPH, + Data: &Channel{}, + }, + Expected: false, + }, + } { + t.Run(test.Name, func(t *testing.T) { + err := test.Metadata.IsValid() + + if test.Expected { + assert.Nil(t, err) + } else { + assert.NotNil(t, err) + } + }) + } +} + +func TestLinkMetadataDeserializeDataToConcreteType(t *testing.T) { + t.Run("should convert []byte to PostImage", func(t *testing.T) { + image := &PostImage{ + Height: 400, + Width: 500, + } + + metadata := &LinkMetadata{ + Type: LINK_METADATA_TYPE_IMAGE, + Data: []byte(image.ToJson()), + } + + require.IsType(t, []byte{}, metadata.Data) + + err := metadata.DeserializeDataToConcreteType() + + assert.Nil(t, err) + assert.IsType(t, &PostImage{}, metadata.Data) + assert.Equal(t, *image, *metadata.Data.(*PostImage)) + }) + + t.Run("should convert string to OpenGraph", func(t *testing.T) { + og := &opengraph.OpenGraph{ + URL: "http://example.com", + Description: "Hello, world!", + Images: []*opengraph.Image{ + { + URL: "http://example.com/image.png", + }, + }, + } + + b, err := json.Marshal(og) + + require.Nil(t, err) + + metadata := &LinkMetadata{ + Type: LINK_METADATA_TYPE_OPENGRAPH, + Data: b, + } + + require.IsType(t, []byte{}, metadata.Data) + + err = metadata.DeserializeDataToConcreteType() + + assert.Nil(t, err) + assert.IsType(t, &opengraph.OpenGraph{}, metadata.Data) + assert.Equal(t, *og, *metadata.Data.(*opengraph.OpenGraph)) + }) + + t.Run("should ignore data of the correct type", func(t *testing.T) { + metadata := &LinkMetadata{ + Type: LINK_METADATA_TYPE_OPENGRAPH, + Data: 1234, + } + + err := metadata.DeserializeDataToConcreteType() + + assert.Nil(t, err) + }) + + t.Run("should ignore an invalid type", func(t *testing.T) { + metadata := &LinkMetadata{ + Type: "garbage", + Data: "garbage", + } + + err := metadata.DeserializeDataToConcreteType() + + assert.Nil(t, err) + }) + + t.Run("should return error for invalid data", func(t *testing.T) { + metadata := &LinkMetadata{ + Type: LINK_METADATA_TYPE_IMAGE, + Data: "garbage", + } + + err := metadata.DeserializeDataToConcreteType() + + assert.NotNil(t, err) + }) +} + +func TestFloorToNearestHour(t *testing.T) { + assert.True(t, isRoundedToNearestHour(FloorToNearestHour(1546346096000))) +} diff --git a/model/post_metadata.go b/model/post_metadata.go index a337b2f8bf..40712d44e7 100644 --- a/model/post_metadata.go +++ b/model/post_metadata.go @@ -3,6 +3,10 @@ package model +import ( + "encoding/json" +) + type PostMetadata struct { // Embeds holds information required to render content embedded in the post. This includes the OpenGraph metadata // for links in the post. @@ -28,3 +32,8 @@ type PostImage struct { Width int `json:"width"` Height int `json:"height"` } + +func (o *PostImage) ToJson() string { + b, _ := json.Marshal(o) + return string(b) +} diff --git a/store/layered_store.go b/store/layered_store.go index a9ed76ed82..fc47aff26e 100644 --- a/store/layered_store.go +++ b/store/layered_store.go @@ -187,6 +187,10 @@ func (s *LayeredStore) Group() GroupStore { return s.GroupStore } +func (s *LayeredStore) LinkMetadata() LinkMetadataStore { + return s.DatabaseLayer.LinkMetadata() +} + func (s *LayeredStore) MarkSystemRanUnitTests() { s.DatabaseLayer.MarkSystemRanUnitTests() } diff --git a/store/sqlstore/link_metadata_store.go b/store/sqlstore/link_metadata_store.go new file mode 100644 index 0000000000..dcc77c664f --- /dev/null +++ b/store/sqlstore/link_metadata_store.go @@ -0,0 +1,88 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package sqlstore + +import ( + "database/sql" + "net/http" + + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/store" +) + +type SqlLinkMetadataStore struct { + SqlStore +} + +func NewSqlLinkMetadataStore(sqlStore SqlStore) store.LinkMetadataStore { + s := &SqlLinkMetadataStore{sqlStore} + + for _, db := range sqlStore.GetAllConns() { + table := db.AddTableWithName(model.LinkMetadata{}, "LinkMetadata").SetKeys(false, "Hash") + table.ColMap("URL").SetMaxSize(2048) + table.ColMap("Type").SetMaxSize(16) + table.ColMap("Data").SetMaxSize(4096) + } + + return s +} + +func (s SqlLinkMetadataStore) CreateIndexesIfNotExists() { + if s.DriverName() == model.DATABASE_DRIVER_MYSQL { + s.CreateCompositeIndexIfNotExists("idx_link_metadata_url_timestamp", "LinkMetadata", []string{"URL(512)", "Timestamp"}) + } else { + s.CreateCompositeIndexIfNotExists("idx_link_metadata_url_timestamp", "LinkMetadata", []string{"URL", "Timestamp"}) + } +} + +func (s SqlLinkMetadataStore) Save(metadata *model.LinkMetadata) store.StoreChannel { + return store.Do(func(result *store.StoreResult) { + if result.Err = metadata.IsValid(); result.Err != nil { + return + } + + metadata.PreSave() + + err := s.GetMaster().Insert(metadata) + if err != nil { + result.Err = model.NewAppError("SqlLinkMetadataStore.Save", "store.sql_link_metadata.save.app_error", nil, "url="+metadata.URL+", "+err.Error(), http.StatusInternalServerError) + return + } + + result.Data = metadata + }) +} + +func (s SqlLinkMetadataStore) Get(url string, timestamp int64) store.StoreChannel { + return store.Do(func(result *store.StoreResult) { + var metadata *model.LinkMetadata + + err := s.GetReplica().SelectOne(&metadata, + `SELECT + * + FROM + LinkMetadata + WHERE + URL = :URL + AND Timestamp = :Timestamp`, map[string]interface{}{"URL": url, "Timestamp": timestamp}) + if err != nil { + result.Err = model.NewAppError("SqlLinkMetadataStore.Get", "store.sql_link_metadata.get.app_error", nil, "url="+url+", "+err.Error(), http.StatusInternalServerError) + + if err == sql.ErrNoRows { + result.Err.StatusCode = http.StatusNotFound + } + + return + } + + err = metadata.DeserializeDataToConcreteType() + if err != nil { + result.Err = model.NewAppError("SqlLinkMetadataStore.Get", "store.sql_link_metadata.get.app_error", nil, "url="+url+", "+err.Error(), http.StatusInternalServerError) + + return + } + + result.Data = metadata + }) +} diff --git a/store/sqlstore/link_metadata_store_test.go b/store/sqlstore/link_metadata_store_test.go new file mode 100644 index 0000000000..a915740f71 --- /dev/null +++ b/store/sqlstore/link_metadata_store_test.go @@ -0,0 +1,14 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package sqlstore + +import ( + "testing" + + "github.com/mattermost/mattermost-server/store/storetest" +) + +func TestLinkMetadataStore(t *testing.T) { + StoreTest(t, storetest.TestLinkMetadataStore) +} diff --git a/store/sqlstore/store.go b/store/sqlstore/store.go index 8193c7ad0a..945a8fb7e6 100644 --- a/store/sqlstore/store.go +++ b/store/sqlstore/store.go @@ -96,4 +96,5 @@ type SqlStore interface { Scheme() store.SchemeStore TermsOfService() store.TermsOfServiceStore UserTermsOfService() store.UserTermsOfServiceStore + LinkMetadata() store.LinkMetadataStore } diff --git a/store/sqlstore/supplier.go b/store/sqlstore/supplier.go index 663e644e12..2147b7ae87 100644 --- a/store/sqlstore/supplier.go +++ b/store/sqlstore/supplier.go @@ -15,6 +15,7 @@ import ( "sync/atomic" "time" + "github.com/dyatlov/go-opengraph/opengraph" "github.com/go-sql-driver/mysql" "github.com/lib/pq" "github.com/mattermost/gorp" @@ -95,6 +96,7 @@ type SqlSupplierOldStores struct { TermsOfService store.TermsOfServiceStore group store.GroupStore UserTermsOfService store.UserTermsOfServiceStore + linkMetadata store.LinkMetadataStore } type SqlSupplier struct { @@ -145,6 +147,7 @@ func NewSqlSupplier(settings model.SqlSettings, metrics einterfaces.MetricsInter supplier.oldStores.plugin = NewSqlPluginStore(supplier) supplier.oldStores.TermsOfService = NewSqlTermsOfServiceStore(supplier, metrics) supplier.oldStores.UserTermsOfService = NewSqlUserTermsOfServiceStore(supplier) + supplier.oldStores.linkMetadata = NewSqlLinkMetadataStore(supplier) initSqlSupplierReactions(supplier) initSqlSupplierRoles(supplier) @@ -183,6 +186,7 @@ func NewSqlSupplier(settings model.SqlSettings, metrics einterfaces.MetricsInter supplier.oldStores.plugin.(*SqlPluginStore).CreateIndexesIfNotExists() supplier.oldStores.TermsOfService.(SqlTermsOfServiceStore).CreateIndexesIfNotExists() supplier.oldStores.UserTermsOfService.(SqlUserTermsOfServiceStore).CreateIndexesIfNotExists() + supplier.oldStores.linkMetadata.(*SqlLinkMetadataStore).CreateIndexesIfNotExists() supplier.CreateIndexesIfNotExistsGroups() @@ -1032,6 +1036,10 @@ func (ss *SqlSupplier) Group() store.GroupStore { return ss.oldStores.group } +func (ss *SqlSupplier) LinkMetadata() store.LinkMetadataStore { + return ss.oldStores.linkMetadata +} + func (ss *SqlSupplier) DropAllTables() { ss.master.TruncateTables() } @@ -1051,6 +1059,10 @@ func (me mattermConverter) ToDb(val interface{}) (interface{}, error) { return model.StringInterfaceToJson(t), nil case map[string]interface{}: return model.StringInterfaceToJson(model.StringInterface(t)), nil + case JSONSerializable: + return t.ToJson(), nil + case *opengraph.OpenGraph: + return json.Marshal(t) } return val, nil @@ -1113,6 +1125,10 @@ func (me mattermConverter) FromDb(target interface{}) (gorp.CustomScanner, bool) return gorp.CustomScanner{}, false } +type JSONSerializable interface { + ToJson() string +} + func convertMySQLFullTextColumnsToPostgres(columnNames string) string { columns := strings.Split(columnNames, ", ") concatenatedColumnNames := "" diff --git a/store/store.go b/store/store.go index f16c6cbb1e..88098b98be 100644 --- a/store/store.go +++ b/store/store.go @@ -68,6 +68,7 @@ type Store interface { TermsOfService() TermsOfServiceStore Group() GroupStore UserTermsOfService() UserTermsOfServiceStore + LinkMetadata() LinkMetadataStore MarkSystemRanUnitTests() Close() LockToMaster() @@ -563,3 +564,8 @@ type GroupStore interface { PendingAutoAddTeamMembers(minGroupMembersCreateAt int64) StoreChannel PendingAutoAddChannelMembers(minGroupMembersCreateAt int64) StoreChannel } + +type LinkMetadataStore interface { + Save(linkMetadata *model.LinkMetadata) StoreChannel + Get(url string, timestamp int64) StoreChannel +} diff --git a/store/storetest/link_metadata_store.go b/store/storetest/link_metadata_store.go new file mode 100644 index 0000000000..2521f294a4 --- /dev/null +++ b/store/storetest/link_metadata_store.go @@ -0,0 +1,251 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package storetest + +import ( + "net/http" + "testing" + "time" + + "github.com/dyatlov/go-opengraph/opengraph" + "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/store" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// These tests are ran on the same store instance, so this provides easier unique, valid timestamps +var linkMetadataTimestamp int64 = 1546300800000 + +func getNextLinkMetadataTimestamp() int64 { + linkMetadataTimestamp += int64(time.Hour) / 1000 + return linkMetadataTimestamp +} + +func TestLinkMetadataStore(t *testing.T, ss store.Store) { + t.Run("Save", func(t *testing.T) { testLinkMetadataStoreSave(t, ss) }) + t.Run("Get", func(t *testing.T) { testLinkMetadataStoreGet(t, ss) }) + t.Run("Types", func(t *testing.T) { testLinkMetadataStoreTypes(t, ss) }) +} + +func testLinkMetadataStoreSave(t *testing.T, ss store.Store) { + t.Run("should save item", func(t *testing.T) { + metadata := &model.LinkMetadata{ + URL: "http://example.com", + Timestamp: getNextLinkMetadataTimestamp(), + Type: model.LINK_METADATA_TYPE_IMAGE, + Data: &model.PostImage{}, + } + + result := <-ss.LinkMetadata().Save(metadata) + + require.Nil(t, result.Err) + require.IsType(t, metadata, result.Data) + assert.Equal(t, *metadata, *result.Data.(*model.LinkMetadata)) + }) + + t.Run("should fail to save invalid item", func(t *testing.T) { + metadata := &model.LinkMetadata{ + URL: "", + Timestamp: 0, + Type: "garbage", + Data: nil, + } + + result := <-ss.LinkMetadata().Save(metadata) + + assert.NotNil(t, result.Err) + }) + + t.Run("should save with duplicate URL and different timestamp", func(t *testing.T) { + metadata := &model.LinkMetadata{ + URL: "http://example.com", + Timestamp: getNextLinkMetadataTimestamp(), + Type: model.LINK_METADATA_TYPE_IMAGE, + Data: &model.PostImage{}, + } + + result := <-ss.LinkMetadata().Save(metadata) + require.Nil(t, result.Err) + + metadata.Timestamp = getNextLinkMetadataTimestamp() + + result = <-ss.LinkMetadata().Save(metadata) + + require.Nil(t, result.Err) + require.IsType(t, metadata, result.Data) + assert.Equal(t, *metadata, *result.Data.(*model.LinkMetadata)) + }) + + t.Run("should save with duplicate timestamp and different URL", func(t *testing.T) { + metadata := &model.LinkMetadata{ + URL: "http://example.com", + Timestamp: getNextLinkMetadataTimestamp(), + Type: model.LINK_METADATA_TYPE_IMAGE, + Data: &model.PostImage{}, + } + + result := <-ss.LinkMetadata().Save(metadata) + require.Nil(t, result.Err) + + metadata.URL = "http://example.com/another/page" + + result = <-ss.LinkMetadata().Save(metadata) + + require.Nil(t, result.Err) + require.IsType(t, metadata, result.Data) + assert.Equal(t, *metadata, *result.Data.(*model.LinkMetadata)) + }) + + t.Run("should fail to save with duplicate URL and timestamp", func(t *testing.T) { + metadata := &model.LinkMetadata{ + URL: "http://example.com", + Timestamp: getNextLinkMetadataTimestamp(), + Type: model.LINK_METADATA_TYPE_IMAGE, + Data: &model.PostImage{}, + } + + result := <-ss.LinkMetadata().Save(metadata) + require.Nil(t, result.Err) + + result = <-ss.LinkMetadata().Save(metadata) + + assert.NotNil(t, result.Err) + }) +} + +func testLinkMetadataStoreGet(t *testing.T, ss store.Store) { + t.Run("should get value", func(t *testing.T) { + metadata := &model.LinkMetadata{ + URL: "http://example.com", + Timestamp: getNextLinkMetadataTimestamp(), + Type: model.LINK_METADATA_TYPE_IMAGE, + Data: &model.PostImage{}, + } + + result := <-ss.LinkMetadata().Save(metadata) + require.Nil(t, result.Err) + + result = <-ss.LinkMetadata().Get(metadata.URL, metadata.Timestamp) + + require.Nil(t, result.Err) + require.IsType(t, metadata, result.Data) + assert.Equal(t, *metadata, *result.Data.(*model.LinkMetadata)) + }) + + t.Run("should return not found with incorrect URL", func(t *testing.T) { + metadata := &model.LinkMetadata{ + URL: "http://example.com", + Timestamp: getNextLinkMetadataTimestamp(), + Type: model.LINK_METADATA_TYPE_IMAGE, + Data: &model.PostImage{}, + } + + result := <-ss.LinkMetadata().Save(metadata) + require.Nil(t, result.Err) + + result = <-ss.LinkMetadata().Get("http://example.com/another_page", metadata.Timestamp) + + require.NotNil(t, result.Err) + assert.Equal(t, http.StatusNotFound, result.Err.StatusCode) + }) + + t.Run("should return not found with incorrect timestamp", func(t *testing.T) { + metadata := &model.LinkMetadata{ + URL: "http://example.com", + Timestamp: getNextLinkMetadataTimestamp(), + Type: model.LINK_METADATA_TYPE_IMAGE, + Data: &model.PostImage{}, + } + + result := <-ss.LinkMetadata().Save(metadata) + require.Nil(t, result.Err) + + result = <-ss.LinkMetadata().Get(metadata.URL, getNextLinkMetadataTimestamp()) + + require.NotNil(t, result.Err) + assert.Equal(t, http.StatusNotFound, result.Err.StatusCode) + }) +} + +func testLinkMetadataStoreTypes(t *testing.T, ss store.Store) { + t.Run("should save and get image metadata", func(t *testing.T) { + metadata := &model.LinkMetadata{ + URL: "http://example.com", + Timestamp: getNextLinkMetadataTimestamp(), + Type: model.LINK_METADATA_TYPE_IMAGE, + Data: &model.PostImage{ + Width: 123, + Height: 456, + }, + } + + result := <-ss.LinkMetadata().Save(metadata) + require.Nil(t, result.Err) + + received := result.Data.(*model.LinkMetadata) + require.IsType(t, &model.PostImage{}, received.Data) + assert.Equal(t, *(metadata.Data.(*model.PostImage)), *(received.Data.(*model.PostImage))) + + result = <-ss.LinkMetadata().Get(metadata.URL, metadata.Timestamp) + require.Nil(t, result.Err) + + received = result.Data.(*model.LinkMetadata) + require.IsType(t, &model.PostImage{}, received.Data) + assert.Equal(t, *(metadata.Data.(*model.PostImage)), *(received.Data.(*model.PostImage))) + }) + + t.Run("should save and get opengraph data", func(t *testing.T) { + og := &opengraph.OpenGraph{ + URL: "http://example.com", + Images: []*opengraph.Image{ + { + URL: "http://example.com/image.png", + }, + }, + } + + metadata := &model.LinkMetadata{ + URL: "http://example.com", + Timestamp: getNextLinkMetadataTimestamp(), + Type: model.LINK_METADATA_TYPE_OPENGRAPH, + Data: og, + } + + result := <-ss.LinkMetadata().Save(metadata) + require.Nil(t, result.Err) + + received := result.Data.(*model.LinkMetadata) + require.IsType(t, &opengraph.OpenGraph{}, received.Data) + assert.Equal(t, *(metadata.Data.(*opengraph.OpenGraph)), *(received.Data.(*opengraph.OpenGraph))) + + result = <-ss.LinkMetadata().Get(metadata.URL, metadata.Timestamp) + require.Nil(t, result.Err) + + received = result.Data.(*model.LinkMetadata) + require.IsType(t, &opengraph.OpenGraph{}, received.Data) + assert.Equal(t, *(metadata.Data.(*opengraph.OpenGraph)), *(received.Data.(*opengraph.OpenGraph))) + }) + + t.Run("should save and get nil", func(t *testing.T) { + metadata := &model.LinkMetadata{ + URL: "http://example.com", + Timestamp: getNextLinkMetadataTimestamp(), + Type: model.LINK_METADATA_TYPE_NONE, + Data: nil, + } + + result := <-ss.LinkMetadata().Save(metadata) + require.Nil(t, result.Err) + + received := result.Data.(*model.LinkMetadata) + assert.Nil(t, received.Data) + + result = <-ss.LinkMetadata().Get(metadata.URL, metadata.Timestamp) + require.Nil(t, result.Err) + + received = result.Data.(*model.LinkMetadata) + require.Nil(t, received.Data) + }) +} diff --git a/store/storetest/mocks/LayeredStoreDatabaseLayer.go b/store/storetest/mocks/LayeredStoreDatabaseLayer.go index 8234c0594d..99c846bd15 100644 --- a/store/storetest/mocks/LayeredStoreDatabaseLayer.go +++ b/store/storetest/mocks/LayeredStoreDatabaseLayer.go @@ -584,6 +584,22 @@ func (_m *LayeredStoreDatabaseLayer) License() store.LicenseStore { return r0 } +// LinkMetadata provides a mock function with given fields: +func (_m *LayeredStoreDatabaseLayer) LinkMetadata() store.LinkMetadataStore { + ret := _m.Called() + + var r0 store.LinkMetadataStore + if rf, ok := ret.Get(0).(func() store.LinkMetadataStore); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(store.LinkMetadataStore) + } + } + + return r0 +} + // LockToMaster provides a mock function with given fields: func (_m *LayeredStoreDatabaseLayer) LockToMaster() { _m.Called() diff --git a/store/storetest/mocks/LinkMetadataStore.go b/store/storetest/mocks/LinkMetadataStore.go new file mode 100644 index 0000000000..d018d6ab08 --- /dev/null +++ b/store/storetest/mocks/LinkMetadataStore.go @@ -0,0 +1,46 @@ +// Code generated by mockery v1.0.0. DO NOT EDIT. + +// Regenerate this file using `make store-mocks`. + +package mocks + +import mock "github.com/stretchr/testify/mock" +import model "github.com/mattermost/mattermost-server/model" +import store "github.com/mattermost/mattermost-server/store" + +// LinkMetadataStore is an autogenerated mock type for the LinkMetadataStore type +type LinkMetadataStore struct { + mock.Mock +} + +// Get provides a mock function with given fields: url, timestamp +func (_m *LinkMetadataStore) Get(url string, timestamp int64) store.StoreChannel { + ret := _m.Called(url, timestamp) + + var r0 store.StoreChannel + if rf, ok := ret.Get(0).(func(string, int64) store.StoreChannel); ok { + r0 = rf(url, timestamp) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(store.StoreChannel) + } + } + + return r0 +} + +// Save provides a mock function with given fields: linkMetadata +func (_m *LinkMetadataStore) Save(linkMetadata *model.LinkMetadata) store.StoreChannel { + ret := _m.Called(linkMetadata) + + var r0 store.StoreChannel + if rf, ok := ret.Get(0).(func(*model.LinkMetadata) store.StoreChannel); ok { + r0 = rf(linkMetadata) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(store.StoreChannel) + } + } + + return r0 +} diff --git a/store/storetest/mocks/ObjectCache.go b/store/storetest/mocks/ObjectCache.go new file mode 100644 index 0000000000..33877564e2 --- /dev/null +++ b/store/storetest/mocks/ObjectCache.go @@ -0,0 +1,97 @@ +// Code generated by mockery v1.0.0. DO NOT EDIT. + +// Regenerate this file using `make store-mocks`. + +package mocks + +import mock "github.com/stretchr/testify/mock" + +// ObjectCache is an autogenerated mock type for the ObjectCache type +type ObjectCache struct { + mock.Mock +} + +// AddWithDefaultExpires provides a mock function with given fields: key, value +func (_m *ObjectCache) AddWithDefaultExpires(key interface{}, value interface{}) { + _m.Called(key, value) +} + +// AddWithExpiresInSecs provides a mock function with given fields: key, value, expireAtSecs +func (_m *ObjectCache) AddWithExpiresInSecs(key interface{}, value interface{}, expireAtSecs int64) { + _m.Called(key, value, expireAtSecs) +} + +// Get provides a mock function with given fields: key +func (_m *ObjectCache) Get(key interface{}) (interface{}, bool) { + ret := _m.Called(key) + + var r0 interface{} + if rf, ok := ret.Get(0).(func(interface{}) interface{}); ok { + r0 = rf(key) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(interface{}) + } + } + + var r1 bool + if rf, ok := ret.Get(1).(func(interface{}) bool); ok { + r1 = rf(key) + } else { + r1 = ret.Get(1).(bool) + } + + return r0, r1 +} + +// GetInvalidateClusterEvent provides a mock function with given fields: +func (_m *ObjectCache) GetInvalidateClusterEvent() string { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + +// Len provides a mock function with given fields: +func (_m *ObjectCache) Len() int { + ret := _m.Called() + + var r0 int + if rf, ok := ret.Get(0).(func() int); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(int) + } + + return r0 +} + +// Name provides a mock function with given fields: +func (_m *ObjectCache) Name() string { + ret := _m.Called() + + var r0 string + if rf, ok := ret.Get(0).(func() string); ok { + r0 = rf() + } else { + r0 = ret.Get(0).(string) + } + + return r0 +} + +// Purge provides a mock function with given fields: +func (_m *ObjectCache) Purge() { + _m.Called() +} + +// Remove provides a mock function with given fields: key +func (_m *ObjectCache) Remove(key interface{}) { + _m.Called(key) +} diff --git a/store/storetest/mocks/SqlStore.go b/store/storetest/mocks/SqlStore.go index 4ccc53dd79..e2c15a5fbd 100644 --- a/store/storetest/mocks/SqlStore.go +++ b/store/storetest/mocks/SqlStore.go @@ -439,6 +439,22 @@ func (_m *SqlStore) License() store.LicenseStore { return r0 } +// LinkMetadata provides a mock function with given fields: +func (_m *SqlStore) LinkMetadata() store.LinkMetadataStore { + ret := _m.Called() + + var r0 store.LinkMetadataStore + if rf, ok := ret.Get(0).(func() store.LinkMetadataStore); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(store.LinkMetadataStore) + } + } + + return r0 +} + // LockToMaster provides a mock function with given fields: func (_m *SqlStore) LockToMaster() { _m.Called() diff --git a/store/storetest/mocks/Store.go b/store/storetest/mocks/Store.go index 10282660ed..600c9e3f22 100644 --- a/store/storetest/mocks/Store.go +++ b/store/storetest/mocks/Store.go @@ -214,6 +214,22 @@ func (_m *Store) License() store.LicenseStore { return r0 } +// LinkMetadata provides a mock function with given fields: +func (_m *Store) LinkMetadata() store.LinkMetadataStore { + ret := _m.Called() + + var r0 store.LinkMetadataStore + if rf, ok := ret.Get(0).(func() store.LinkMetadataStore); ok { + r0 = rf() + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(store.LinkMetadataStore) + } + } + + return r0 +} + // LockToMaster provides a mock function with given fields: func (_m *Store) LockToMaster() { _m.Called() diff --git a/store/storetest/store.go b/store/storetest/store.go index 4d5d5e0428..76ec1c474d 100644 --- a/store/storetest/store.go +++ b/store/storetest/store.go @@ -48,6 +48,7 @@ type Store struct { TermsOfServiceStore mocks.TermsOfServiceStore GroupStore mocks.GroupStore UserTermsOfServiceStore mocks.UserTermsOfServiceStore + LinkMetadataStore mocks.LinkMetadataStore } func (s *Store) Team() store.TeamStore { return &s.TeamStore } @@ -80,15 +81,16 @@ func (s *Store) UserTermsOfService() store.UserTermsOfServiceStore { return &s.U func (s *Store) ChannelMemberHistory() store.ChannelMemberHistoryStore { return &s.ChannelMemberHistoryStore } -func (s *Store) Group() store.GroupStore { return &s.GroupStore } -func (s *Store) MarkSystemRanUnitTests() { /* do nothing */ } -func (s *Store) Close() { /* do nothing */ } -func (s *Store) LockToMaster() { /* do nothing */ } -func (s *Store) UnlockFromMaster() { /* do nothing */ } -func (s *Store) DropAllTables() { /* do nothing */ } -func (s *Store) TotalMasterDbConnections() int { return 1 } -func (s *Store) TotalReadDbConnections() int { return 1 } -func (s *Store) TotalSearchDbConnections() int { return 1 } +func (s *Store) Group() store.GroupStore { return &s.GroupStore } +func (s *Store) LinkMetadata() store.LinkMetadataStore { return &s.LinkMetadataStore } +func (s *Store) MarkSystemRanUnitTests() { /* do nothing */ } +func (s *Store) Close() { /* do nothing */ } +func (s *Store) LockToMaster() { /* do nothing */ } +func (s *Store) UnlockFromMaster() { /* do nothing */ } +func (s *Store) DropAllTables() { /* do nothing */ } +func (s *Store) TotalMasterDbConnections() int { return 1 } +func (s *Store) TotalReadDbConnections() int { return 1 } +func (s *Store) TotalSearchDbConnections() int { return 1 } func (s *Store) AssertExpectations(t mock.TestingT) bool { return mock.AssertExpectationsForObjects(t,