From 225461fd7ec060c6d4d4c7790e6b3fe91a3585ba Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Mon, 27 Sep 2021 08:35:39 +0530 Subject: [PATCH] Exclude sending file metadata to plugin hooks (#18454) * Exclude sending file metadata to plugin hooks A FileInfo object contained a MiniPreview which is a slice of bytes. This can be particularly costly while marshalling to plugin hooks. We avoid this by refactoring the Embeds and Images population to a separate method and calling that to prevent posts from getting updated. https://community-daily.mattermost.com/boards/workspace/zyoahc9uapdn3xdptac6jb69ic/285b80a3-257d-41f6-8cf4-ed80ca9d92e5/495cdb4d-c13a-4992-8eb9-80cfee2819a4?c=9c0b5413-5401-4ef2-83d5-b9f756585bbc ```release-note NONE ``` * refactor to separate method ```release-note NONE ``` --- api4/post.go | 4 ++-- app/app_iface.go | 3 ++- app/opentracing/opentracing_layer.go | 17 +++++++++++++++++ app/post.go | 8 ++++---- app/post_metadata.go | 25 +++++++++++++++++++------ app/post_metadata_test.go | 2 +- 6 files changed, 45 insertions(+), 14 deletions(-) diff --git a/api4/post.go b/api4/post.go index 5d3dbc055c..218b0f8ef4 100644 --- a/api4/post.go +++ b/api4/post.go @@ -128,7 +128,7 @@ func createEphemeralPost(c *Context, w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusCreated) rp = model.AddPostActionCookies(rp, c.App.PostActionCookieSecret()) - rp = c.App.PreparePostForClient(rp, true, false) + rp = c.App.PreparePostForClientWithEmbedsAndImages(rp, true, false) rp, err := c.App.SanitizePostMetadataForUser(rp, c.AppContext.Session().UserId) if err != nil { c.Err = err @@ -376,7 +376,7 @@ func getPost(c *Context, w http.ResponseWriter, r *http.Request) { return } - post = c.App.PreparePostForClient(post, false, false) + post = c.App.PreparePostForClientWithEmbedsAndImages(post, false, false) post, err = c.App.SanitizePostMetadataForUser(post, c.AppContext.Session().UserId) if err != nil { c.Err = err diff --git a/app/app_iface.go b/app/app_iface.go index 929bc342cc..ed104d831f 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -880,7 +880,8 @@ type AppIface interface { PostUpdateChannelPurposeMessage(c *request.Context, userID string, channel *model.Channel, oldChannelPurpose string, newChannelPurpose string) *model.AppError PostWithProxyAddedToImageURLs(post *model.Post) *model.Post PostWithProxyRemovedFromImageURLs(post *model.Post) *model.Post - PreparePostForClient(originalPost *model.Post, isNewPost bool, isEditPost bool) *model.Post + PreparePostForClient(originalPost *model.Post, isNewPost, isEditPost bool) *model.Post + PreparePostForClientWithEmbedsAndImages(originalPost *model.Post, isNewPost, isEditPost bool) *model.Post PreparePostListForClient(originalList *model.PostList) *model.PostList ProcessSlackText(text string) string Publish(message *model.WebSocketEvent) diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index 30e0eef82f..7c6b9ef2f6 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -12220,6 +12220,23 @@ func (a *OpenTracingAppLayer) PreparePostForClient(originalPost *model.Post, isN return resultVar0 } +func (a *OpenTracingAppLayer) PreparePostForClientWithEmbedsAndImages(originalPost *model.Post, isNewPost bool, isEditPost bool) *model.Post { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.PreparePostForClientWithEmbedsAndImages") + + a.ctx = newCtx + a.app.Srv().Store.SetContext(newCtx) + defer func() { + a.app.Srv().Store.SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0 := a.app.PreparePostForClientWithEmbedsAndImages(originalPost, isNewPost, isEditPost) + + return resultVar0 +} + func (a *OpenTracingAppLayer) PreparePostListForClient(originalList *model.PostList) *model.PostList { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.PreparePostListForClient") diff --git a/app/post.go b/app/post.go index aca7b5a105..4255e6967a 100644 --- a/app/post.go +++ b/app/post.go @@ -297,7 +297,7 @@ func (a *App) CreatePost(c *request.Context, post *model.Post, channel *model.Ch } } - post = a.PreparePostForClient(post, true, false) + post = a.getEmbedsAndImages(post, true) previewPost := post.GetPreviewPost() if previewPost != nil { post.AddProp(model.PostPropsPreviewedPost, previewPost.PostID) @@ -521,7 +521,7 @@ func (a *App) SendEphemeralPost(userID string, post *model.Post) *model.Post { post.GenerateActionIds() message := model.NewWebSocketEvent(model.WebsocketEventEphemeralMessage, "", post.ChannelId, userID, nil) - post = a.PreparePostForClient(post, true, false) + post = a.PreparePostForClientWithEmbedsAndImages(post, true, false) post = model.AddPostActionCookies(post, a.PostActionCookieSecret()) postJSON, jsonErr := post.ToJSON() @@ -544,7 +544,7 @@ func (a *App) UpdateEphemeralPost(userID string, post *model.Post) *model.Post { post.GenerateActionIds() message := model.NewWebSocketEvent(model.WebsocketEventPostEdited, "", post.ChannelId, userID, nil) - post = a.PreparePostForClient(post, true, false) + post = a.PreparePostForClientWithEmbedsAndImages(post, true, false) post = model.AddPostActionCookies(post, a.PostActionCookieSecret()) postJSON, jsonErr := post.ToJSON() if jsonErr != nil { @@ -685,7 +685,7 @@ func (a *App) UpdatePost(c *request.Context, post *model.Post, safeUpdate bool) }) } - rpost = a.PreparePostForClient(rpost, false, true) + rpost = a.PreparePostForClientWithEmbedsAndImages(rpost, false, true) // Ensure IsFollowing is nil since this updated post will be broadcast to all users // and we don't want to have to populate it for every single user and broadcast to each diff --git a/app/post_metadata.go b/app/post_metadata.go index 9e88e34db5..a8f195524c 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -60,7 +60,7 @@ func (a *App) PreparePostListForClient(originalList *model.PostList) *model.Post } for id, originalPost := range originalList.Posts { - post := a.PreparePostForClient(originalPost, false, false) + post := a.PreparePostForClientWithEmbedsAndImages(originalPost, false, false) list.Posts[id] = post } @@ -94,15 +94,16 @@ func (a *App) OverrideIconURLIfEmoji(post *model.Post) { } } -func (a *App) PreparePostForClient(originalPost *model.Post, isNewPost bool, isEditPost bool) *model.Post { +func (a *App) PreparePostForClient(originalPost *model.Post, isNewPost, isEditPost bool) *model.Post { post := originalPost.Clone() // Proxy image links before constructing metadata so that requests go through the proxy post = a.PostWithProxyAddedToImageURLs(post) a.OverrideIconURLIfEmoji(post) - - post.Metadata = &model.PostMetadata{} + if post.Metadata == nil { + post.Metadata = &model.PostMetadata{} + } if post.DeleteAt > 0 { // For deleted posts we don't fill out metadata nor do we return the post content @@ -125,9 +126,22 @@ func (a *App) PreparePostForClient(originalPost *model.Post, isNewPost bool, isE post.Metadata.Files = fileInfos } + return post +} + +func (a *App) PreparePostForClientWithEmbedsAndImages(originalPost *model.Post, isNewPost, isEditPost bool) *model.Post { + post := a.PreparePostForClient(originalPost, isNewPost, isEditPost) + post = a.getEmbedsAndImages(post, true) + return post +} + +func (a *App) getEmbedsAndImages(post *model.Post, isNewPost bool) *model.Post { + if post.Metadata == nil { + post.Metadata = &model.PostMetadata{} + } + // Embeds and image dimensions firstLink, images := a.getFirstLinkAndImages(post.Message) - if embed, err := a.getEmbedForPost(post, firstLink, isNewPost); err != nil { appErr, ok := err.(*model.AppError) isNotFound := ok && appErr.StatusCode == http.StatusNotFound @@ -142,7 +156,6 @@ func (a *App) PreparePostForClient(originalPost *model.Post, isNewPost bool, isE } post.Metadata.Images = a.getImagesForPost(post, images, isNewPost) - return post } diff --git a/app/post_metadata_test.go b/app/post_metadata_test.go index e524799414..17f30ed5d4 100644 --- a/app/post_metadata_test.go +++ b/app/post_metadata_test.go @@ -122,7 +122,7 @@ func TestPreparePostForClient(t *testing.T) { Message: message, } - clientPost := th.App.PreparePostForClient(post, false, false) + clientPost := th.App.PreparePostForClient(post, false, true) t.Run("doesn't mutate provided post", func(t *testing.T) { assert.NotEqual(t, clientPost, post, "should've returned a new post")