From dc366bc1e23bd6c27ade6b225b345dd7b510a258 Mon Sep 17 00:00:00 2001 From: Lev <1187448+levb@users.noreply.github.com> Date: Thu, 21 Jul 2022 07:11:18 -0700 Subject: [PATCH 01/28] MM-45208: strip post meta for plugins (#20686) Try two... * Revert "Revert "Removed the opengraph type dependency for plugins by stripping post Metadata (#20612)" (#20684)" This reverts commit 32dee6d4499b40fded7749a7f9b651ddafffb4b6. * Fixed race condition * PR feedback * lint --- app/plugin_api.go | 60 ++++++++++++++++++++++++++++-------- app/post.go | 27 ++++++++-------- model/post.go | 16 +++------- model/post_list.go | 8 +++++ model/post_search_results.go | 6 ++++ plugin/client_rpc.go | 2 -- 6 files changed, 79 insertions(+), 40 deletions(-) diff --git a/app/plugin_api.go b/app/plugin_api.go index 6e0edeab21..0b1187d250 100644 --- a/app/plugin_api.go +++ b/app/plugin_api.go @@ -513,7 +513,7 @@ func (api *PluginAPI) SearchPostsInTeam(teamID string, paramsList []*model.Searc if err != nil { return nil, err } - return postList.ToSlice(), nil + return postList.ForPlugin().ToSlice(), nil } func (api *PluginAPI) SearchPostsInTeamForUser(teamID string, userID string, searchParams model.SearchParameter) (*model.PostSearchResults, *model.AppError) { @@ -547,7 +547,11 @@ func (api *PluginAPI) SearchPostsInTeamForUser(teamID string, userID string, sea includeDeletedChannels = *searchParams.IncludeDeletedChannels } - return api.app.SearchPostsForUser(api.ctx, terms, userID, teamID, isOrSearch, includeDeletedChannels, timeZoneOffset, page, perPage, model.ModifierMessages) + results, appErr := api.app.SearchPostsForUser(api.ctx, terms, userID, teamID, isOrSearch, includeDeletedChannels, timeZoneOffset, page, perPage, model.ModifierMessages) + if results != nil { + results = results.ForPlugin() + } + return results, appErr } func (api *PluginAPI) AddChannelMember(channelID, userID string) (*model.ChannelMember, *model.AppError) { @@ -627,7 +631,11 @@ func (api *PluginAPI) GetGroupsForUser(userID string) ([]*model.Group, *model.Ap } func (api *PluginAPI) CreatePost(post *model.Post) (*model.Post, *model.AppError) { - return api.app.CreatePostMissingChannel(api.ctx, post, true) + post, appErr := api.app.CreatePostMissingChannel(api.ctx, post, true) + if post != nil { + post = post.ForPlugin() + } + return post, appErr } func (api *PluginAPI) AddReaction(reaction *model.Reaction) (*model.Reaction, *model.AppError) { @@ -643,11 +651,11 @@ func (api *PluginAPI) GetReactions(postID string) ([]*model.Reaction, *model.App } func (api *PluginAPI) SendEphemeralPost(userID string, post *model.Post) *model.Post { - return api.app.SendEphemeralPost(api.ctx, userID, post) + return api.app.SendEphemeralPost(api.ctx, userID, post).ForPlugin() } func (api *PluginAPI) UpdateEphemeralPost(userID string, post *model.Post) *model.Post { - return api.app.UpdateEphemeralPost(api.ctx, userID, post) + return api.app.UpdateEphemeralPost(api.ctx, userID, post).ForPlugin() } func (api *PluginAPI) DeleteEphemeralPost(userID, postID string) { @@ -660,31 +668,59 @@ func (api *PluginAPI) DeletePost(postID string) *model.AppError { } func (api *PluginAPI) GetPostThread(postID string) (*model.PostList, *model.AppError) { - return api.app.GetPostThread(postID, model.GetPostsOptions{}, "") + list, appErr := api.app.GetPostThread(postID, model.GetPostsOptions{}, "") + if list != nil { + list = list.ForPlugin() + } + return list, appErr } func (api *PluginAPI) GetPost(postID string) (*model.Post, *model.AppError) { - return api.app.GetSinglePost(postID, false) + post, appErr := api.app.GetSinglePost(postID, false) + if post != nil { + post = post.ForPlugin() + } + return post, appErr } func (api *PluginAPI) GetPostsSince(channelID string, time int64) (*model.PostList, *model.AppError) { - return api.app.GetPostsSince(model.GetPostsSinceOptions{ChannelId: channelID, Time: time}) + list, appErr := api.app.GetPostsSince(model.GetPostsSinceOptions{ChannelId: channelID, Time: time}) + if list != nil { + list = list.ForPlugin() + } + return list, appErr } func (api *PluginAPI) GetPostsAfter(channelID, postID string, page, perPage int) (*model.PostList, *model.AppError) { - return api.app.GetPostsAfterPost(model.GetPostsOptions{ChannelId: channelID, PostId: postID, Page: page, PerPage: perPage}) + list, appErr := api.app.GetPostsAfterPost(model.GetPostsOptions{ChannelId: channelID, PostId: postID, Page: page, PerPage: perPage}) + if list != nil { + list = list.ForPlugin() + } + return list, appErr } func (api *PluginAPI) GetPostsBefore(channelID, postID string, page, perPage int) (*model.PostList, *model.AppError) { - return api.app.GetPostsBeforePost(model.GetPostsOptions{ChannelId: channelID, PostId: postID, Page: page, PerPage: perPage}) + list, appErr := api.app.GetPostsBeforePost(model.GetPostsOptions{ChannelId: channelID, PostId: postID, Page: page, PerPage: perPage}) + if list != nil { + list = list.ForPlugin() + } + return list, appErr } func (api *PluginAPI) GetPostsForChannel(channelID string, page, perPage int) (*model.PostList, *model.AppError) { - return api.app.GetPostsPage(model.GetPostsOptions{ChannelId: channelID, Page: page, PerPage: perPage}) + list, appErr := api.app.GetPostsPage(model.GetPostsOptions{ChannelId: channelID, Page: page, PerPage: perPage}) + if list != nil { + list = list.ForPlugin() + } + return list, appErr } func (api *PluginAPI) UpdatePost(post *model.Post) (*model.Post, *model.AppError) { - return api.app.UpdatePost(api.ctx, post, false) + post, appErr := api.app.UpdatePost(api.ctx, post, false) + if post != nil { + post = post.ForPlugin() + } + return post, appErr } func (api *PluginAPI) GetProfileImage(userID string) ([]byte, *model.AppError) { diff --git a/app/post.go b/app/post.go index 2379ee300f..487537029d 100644 --- a/app/post.go +++ b/app/post.go @@ -259,7 +259,7 @@ func (a *App) CreatePost(c request.CTX, post *model.Post, channel *model.Channel var rejectionError *model.AppError pluginContext := pluginContext(c) pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { - replacementPost, rejectionReason := hooks.MessageWillBePosted(pluginContext, post) + replacementPost, rejectionReason := hooks.MessageWillBePosted(pluginContext, post.ForPlugin()) if rejectionReason != "" { id := "Post rejected by plugin. " + rejectionReason if rejectionReason == plugin.DismissPostError { @@ -269,6 +269,7 @@ func (a *App) CreatePost(c request.CTX, post *model.Post, channel *model.Channel return false } if replacementPost != nil { + // the original post's metadata (if there ever was any) is lost, and will be rebuilt. post = replacementPost } @@ -309,21 +310,14 @@ func (a *App) CreatePost(c request.CTX, post *model.Post, channel *model.Channel // might be duplicating requests. a.Srv().seenPendingPostIdsCache.SetWithExpiry(post.PendingPostId, rpost.Id, PendingPostIDsCacheTTL) - // We make a copy of the post for the plugin hook to avoid a race condition. - rPostCopy := rpost.Clone() - - // FIXME: Removes PreviewPost from the post payload sent to the MessageHasBeenPosted hook so that plugins compiled with older versions of - // Mattermost—without the gob registration of the PreviewPost struct—won't crash. - if rPostCopy.Metadata != nil { - rPostCopy.Metadata = rPostCopy.Metadata.Copy() - } - rPostCopy.RemovePreviewPost() - + // We make a copy of the post for the plugin hook to avoid a race condition, + // and to remove the non-GOB-encodable Metadata from it. if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { + pluginPost := rpost.ForPlugin() a.Srv().Go(func() { pluginContext := pluginContext(c) pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { - hooks.MessageHasBeenPosted(pluginContext, rPostCopy) + hooks.MessageHasBeenPosted(pluginContext, pluginPost) return true }, plugin.MessageHasBeenPostedID) }) @@ -650,12 +644,15 @@ func (a *App) UpdatePost(c *request.Context, post *model.Post, safeUpdate bool) var rejectionReason string pluginContext := pluginContext(c) pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { - newPost, rejectionReason = hooks.MessageWillBeUpdated(pluginContext, newPost, oldPost) + newPost, rejectionReason = hooks.MessageWillBeUpdated(pluginContext, newPost.ForPlugin(), oldPost.ForPlugin()) return post != nil }, plugin.MessageWillBeUpdatedID) if newPost == nil { return nil, model.NewAppError("UpdatePost", "Post rejected by plugin. "+rejectionReason, nil, "", http.StatusBadRequest) } + // Restore the post metadata that was stripped by the plugin. Set it to + // the last known good. + newPost.Metadata = oldPost.Metadata } rpost, nErr := a.Srv().Store.Post().Update(newPost, oldPost) @@ -670,10 +667,12 @@ func (a *App) UpdatePost(c *request.Context, post *model.Post, safeUpdate bool) } if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { + pluginOldPost := oldPost.ForPlugin() + pluginNewPost := newPost.ForPlugin() a.Srv().Go(func() { pluginContext := pluginContext(c) pluginsEnvironment.RunMultiPluginHook(func(hooks plugin.Hooks) bool { - hooks.MessageHasBeenUpdated(pluginContext, newPost, oldPost) + hooks.MessageHasBeenUpdated(pluginContext, pluginNewPost, pluginOldPost) return true }, plugin.MessageHasBeenUpdatedID) }) diff --git a/model/post.go b/model/post.go index 29ac05b263..f8e3240154 100644 --- a/model/post.go +++ b/model/post.go @@ -736,18 +736,10 @@ func (o *Post) ToNilIfInvalid() *Post { return o } -func (o *Post) RemovePreviewPost() { - if o.Metadata == nil || o.Metadata.Embeds == nil { - return - } - n := 0 - for _, embed := range o.Metadata.Embeds { - if embed.Type != PostEmbedPermalink { - o.Metadata.Embeds[n] = embed - n++ - } - } - o.Metadata.Embeds = o.Metadata.Embeds[:n] +func (o *Post) ForPlugin() *Post { + p := o.Clone() + p.Metadata = nil + return p } func (o *Post) GetPreviewPost() *PreviewPost { diff --git a/model/post_list.go b/model/post_list.go index 0801b5c8e1..34fc031e3f 100644 --- a/model/post_list.go +++ b/model/post_list.go @@ -46,6 +46,14 @@ func (o *PostList) Clone() *PostList { } } +func (o *PostList) ForPlugin() *PostList { + copy := o.Clone() + for k, p := range copy.Posts { + copy.Posts[k] = p.ForPlugin() + } + return copy +} + func (o *PostList) ToSlice() []*Post { var posts []*Post diff --git a/model/post_search_results.go b/model/post_search_results.go index a3afc7231a..23511039a7 100644 --- a/model/post_search_results.go +++ b/model/post_search_results.go @@ -33,3 +33,9 @@ func (o *PostSearchResults) EncodeJSON(w io.Writer) error { o.PostList.StripActionIntegrations() return json.NewEncoder(w).Encode(o) } + +func (o *PostSearchResults) ForPlugin() *PostSearchResults { + copy := *o + copy.PostList = copy.PostList.ForPlugin() + return © +} diff --git a/plugin/client_rpc.go b/plugin/client_rpc.go index aaf4f66a1d..12c54b61ba 100644 --- a/plugin/client_rpc.go +++ b/plugin/client_rpc.go @@ -21,7 +21,6 @@ import ( "reflect" "sync" - "github.com/dyatlov/go-opengraph/opengraph" "github.com/go-sql-driver/mysql" "github.com/hashicorp/go-plugin" "github.com/lib/pq" @@ -164,7 +163,6 @@ func init() { gob.Register(&pq.Error{}) gob.Register(&mysql.MySQLError{}) gob.Register(&ErrorString{}) - gob.Register(&opengraph.OpenGraph{}) gob.Register(&model.AutocompleteDynamicListArg{}) gob.Register(&model.AutocompleteStaticListArg{}) gob.Register(&model.AutocompleteTextArg{}) From 638ee6893562e6148926a084d9d4eeefc4854950 Mon Sep 17 00:00:00 2001 From: Shota Gvinepadze Date: Thu, 21 Jul 2022 18:11:45 +0400 Subject: [PATCH 02/28] [MM-44948] Update sync (#20414) * Update sync * Add enter * Address review suggestion Co-authored-by: Mattermod --- services/sharedchannel/sync_send_remote.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/services/sharedchannel/sync_send_remote.go b/services/sharedchannel/sync_send_remote.go index 0db2250da9..d6efd49abf 100644 --- a/services/sharedchannel/sync_send_remote.go +++ b/services/sharedchannel/sync_send_remote.go @@ -296,11 +296,11 @@ func (scs *Service) fetchPostUsersForSync(sd *syncData) error { } if sync { - sd.users[user.Id] = sanitizeUserForSync(user) + sd.users[user.Id] = user } if syncImage { - sd.profileImages[user.Id] = sanitizeUserForSync(user) + sd.profileImages[user.Id] = user } // if this was a mention then put the real username in place of the username+remotename, but only @@ -369,6 +369,8 @@ func (scs *Service) filterPostsForSync(sd *syncData) { func (scs *Service) sendSyncData(sd *syncData) error { merr := merror.New() + sanitizeSyncData(sd) + // send users if len(sd.users) != 0 { if err := scs.sendUserSyncData(sd); err != nil { @@ -531,3 +533,12 @@ func (scs *Service) sendSyncMsgToRemote(msg *syncMsg, rc *model.RemoteCluster, f wg.Wait() return err } + +func sanitizeSyncData(sd *syncData) { + for id, user := range sd.users { + sd.users[id] = sanitizeUserForSync(user) + } + for id, user := range sd.profileImages { + sd.profileImages[id] = sanitizeUserForSync(user) + } +} From bc7f961d75809d899b6d1ab9733df7ba6258a80c Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Thu, 21 Jul 2022 10:55:57 -0400 Subject: [PATCH 03/28] Store plugin OnActivate errors and include them in PluginStatus response (#20430) * store plugin OnActivate errors, and include in PluginStatus * write test Co-authored-by: Mattermod --- app/plugin_test.go | 38 ++++++++++++++++++++++++++++++++++++++ model/plugin_status.go | 1 + plugin/environment.go | 26 ++++++++++++++++++++++++++ 3 files changed, 65 insertions(+) diff --git a/app/plugin_test.go b/app/plugin_test.go index 3342f12a5b..a5346e270f 100644 --- a/app/plugin_test.go +++ b/app/plugin_test.go @@ -770,6 +770,44 @@ func TestPluginPanicLogs(t *testing.T) { }) } +func TestPluginStatusActivateError(t *testing.T) { + t.Run("should return error from OnActivate in plugin statuses", func(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + pluginSource := ` + package main + + import ( + "errors" + + "github.com/mattermost/mattermost-server/v6/plugin" + ) + + type MyPlugin struct { + plugin.MattermostPlugin + } + + func (p *MyPlugin) OnActivate() error { + return errors.New("sample error") + } + + func main() { + plugin.ClientMain(&MyPlugin{}) + } + ` + + tearDown, _, _ := SetAppEnvironmentWithPlugins(t, []string{pluginSource}, th.App, th.NewPluginAPI) + defer tearDown() + + env := th.App.GetPluginsEnvironment() + pluginStatus, err := env.Statuses() + require.NoError(t, err) + require.Len(t, pluginStatus, 1) + require.Equal(t, "sample error", pluginStatus[0].Error) + }) +} + func TestProcessPrepackagedPlugins(t *testing.T) { th := Setup(t) defer th.TearDown() diff --git a/model/plugin_status.go b/model/plugin_status.go index c206505be3..63e94c1624 100644 --- a/model/plugin_status.go +++ b/model/plugin_status.go @@ -18,6 +18,7 @@ type PluginStatus struct { ClusterId string `json:"cluster_id"` PluginPath string `json:"plugin_path"` State int `json:"state"` + Error string `json:"error"` Name string `json:"name"` Description string `json:"description"` Version string `json:"version"` diff --git a/plugin/environment.go b/plugin/environment.go index a24f1573ed..6469c9352b 100644 --- a/plugin/environment.go +++ b/plugin/environment.go @@ -32,6 +32,7 @@ type apiImplCreatorFunc func(*model.Manifest) API type registeredPlugin struct { BundleInfo *model.BundleInfo State int + Error string supervisor *supervisor } @@ -137,6 +138,22 @@ func (env *Environment) IsActive(id string) bool { return env.GetPluginState(id) == model.PluginStateRunning } +func (env *Environment) setPluginError(id string, err string) { + if rp, ok := env.registeredPlugins.Load(id); ok { + p := rp.(registeredPlugin) + p.Error = err + env.registeredPlugins.Store(id, p) + } +} + +func (env *Environment) getPluginError(id string) string { + if rp, ok := env.registeredPlugins.Load(id); ok { + return rp.(registeredPlugin).Error + } + + return "" +} + // GetPluginState returns the current state of a plugin (disabled, running, or error) func (env *Environment) GetPluginState(id string) int { rp, ok := env.registeredPlugins.Load(id) @@ -185,6 +202,7 @@ func (env *Environment) Statuses() (model.PluginStatuses, error) { PluginId: plugin.Manifest.Id, PluginPath: filepath.Dir(plugin.ManifestPath), State: pluginState, + Error: env.getPluginError(plugin.Manifest.Id), Name: plugin.Manifest.Name, Description: plugin.Manifest.Description, Version: plugin.Manifest.Version, @@ -214,6 +232,14 @@ func (env *Environment) GetManifest(pluginId string) (*model.Manifest, error) { } func (env *Environment) Activate(id string) (manifest *model.Manifest, activated bool, reterr error) { + defer func() { + if reterr != nil { + env.setPluginError(id, reterr.Error()) + } else { + env.setPluginError(id, "") + } + }() + // Check if we are already active if env.IsActive(id) { return nil, false, nil From 3d7859396dd4cf2338be3b4b33e18c9e35402545 Mon Sep 17 00:00:00 2001 From: Riccardo Santoni <45645402+santoniriccardo@users.noreply.github.com> Date: Thu, 21 Jul 2022 17:11:08 +0200 Subject: [PATCH 04/28] [MM-42194] Get file information from a deleted post (#20279) * Introduced inlcude_deleted query parameter to allow admins to retrieve contents of post regardless of deletion status * Introduced new client route and tests for getting file info of deleted posts * Fixed tests due to caching of posts * gofmt * Small formatting updates * Invalidating file infos cache on delete of post if post includes files * Including deleted in migration flow * Moved invalidating of cache Co-authored-by: Mattermod --- api4/post.go | 8 ++++++- api4/post_test.go | 33 ++++++++++++++++++++++++++++ app/app_iface.go | 4 ++-- app/import_functions.go | 2 +- app/opentracing/opentracing_layer.go | 8 +++---- app/post.go | 12 +++++----- app/post_metadata.go | 2 +- app/post_test.go | 4 ++-- model/client4.go | 18 +++++++++++++++ 9 files changed, 75 insertions(+), 16 deletions(-) diff --git a/api4/post.go b/api4/post.go index b5c33488fa..eb2dc1a26b 100644 --- a/api4/post.go +++ b/api4/post.go @@ -898,7 +898,13 @@ func getFileInfosForPost(c *Context, w http.ResponseWriter, r *http.Request) { return } - infos, err := c.App.GetFileInfosForPostWithMigration(c.Params.PostId) + includeDeleted, _ := strconv.ParseBool(r.URL.Query().Get("include_deleted")) + if includeDeleted && !c.App.SessionHasPermissionTo(*c.AppContext.Session(), model.PermissionManageSystem) { + c.SetPermissionError(model.PermissionManageSystem) + return + } + + infos, err := c.App.GetFileInfosForPostWithMigration(c.Params.PostId, includeDeleted) if err != nil { c.Err = err return diff --git a/api4/post_test.go b/api4/post_test.go index 5db9de4b24..22006fb812 100644 --- a/api4/post_test.go +++ b/api4/post_test.go @@ -2623,6 +2623,39 @@ func TestGetFileInfosForPost(t *testing.T) { require.Error(t, err) CheckForbiddenStatus(t, resp) + // Delete post + th.SystemAdminClient.DeletePost(post.Id) + + // Normal client should get 404 when trying to access deleted post normally + _, resp, err = client.GetFileInfosForPost(post.Id, "") + require.Error(t, err) + CheckNotFoundStatus(t, resp) + + // Normal client should get unauthorized when trying to access deleted post + _, resp, err = client.GetFileInfosForPostIncludeDeleted(post.Id, "") + require.Error(t, err) + CheckForbiddenStatus(t, resp) + + // System client should get 404 when trying to access deleted post normally + _, resp, err = th.SystemAdminClient.GetFileInfosForPost(post.Id, "") + require.Error(t, err) + CheckNotFoundStatus(t, resp) + + // System client should be able to access deleted post with include_deleted param + infos, _, err = th.SystemAdminClient.GetFileInfosForPostIncludeDeleted(post.Id, "") + require.NoError(t, err) + + require.Len(t, infos, 3, "missing file infos") + + found = false + for _, info := range infos { + if info.Id == fileIds[0] { + found = true + } + } + + require.True(t, found, "missing file info") + client.Logout() _, resp, err = client.GetFileInfosForPost(model.NewId(), "") require.Error(t, err) diff --git a/app/app_iface.go b/app/app_iface.go index e1e0aeb057..2ac622e3c0 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -630,8 +630,8 @@ type AppIface interface { GetFile(fileID string) ([]byte, *model.AppError) GetFileInfo(fileID string) (*model.FileInfo, *model.AppError) GetFileInfos(page, perPage int, opt *model.GetFileInfosOptions) ([]*model.FileInfo, *model.AppError) - GetFileInfosForPost(postID string, fromMaster bool) ([]*model.FileInfo, *model.AppError) - GetFileInfosForPostWithMigration(postID string) ([]*model.FileInfo, *model.AppError) + GetFileInfosForPost(postID string, fromMaster bool, includeDeleted bool) ([]*model.FileInfo, *model.AppError) + GetFileInfosForPostWithMigration(postID string, includeDeleted bool) ([]*model.FileInfo, *model.AppError) GetFlaggedPosts(userID string, offset int, limit int) (*model.PostList, *model.AppError) GetFlaggedPostsForChannel(userID, channelID string, offset int, limit int) (*model.PostList, *model.AppError) GetFlaggedPostsForTeam(userID, teamID string, offset int, limit int) (*model.PostList, *model.AppError) diff --git a/app/import_functions.go b/app/import_functions.go index 42ba70e4d9..7ab16c3e50 100644 --- a/app/import_functions.go +++ b/app/import_functions.go @@ -1224,7 +1224,7 @@ func (a *App) importAttachment(c *request.Context, data *AttachmentImportData, p // Go over existing files in the post and see if there already exists a file with the same name, size and hash. If so - skip it if post.Id != "" { - oldFiles, err := a.GetFileInfosForPost(post.Id, true) + oldFiles, err := a.GetFileInfosForPost(post.Id, true, false) if err != nil { return nil, model.NewAppError("BulkImport", "app.import.attachment.file_upload.error", map[string]any{"FilePath": *data.Path}, "", http.StatusBadRequest) } diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index d31592efe5..5761473dcf 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -5970,7 +5970,7 @@ func (a *OpenTracingAppLayer) GetFileInfos(page int, perPage int, opt *model.Get return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) GetFileInfosForPost(postID string, fromMaster bool) ([]*model.FileInfo, *model.AppError) { +func (a *OpenTracingAppLayer) GetFileInfosForPost(postID string, fromMaster bool, includeDeleted bool) ([]*model.FileInfo, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetFileInfosForPost") @@ -5982,7 +5982,7 @@ func (a *OpenTracingAppLayer) GetFileInfosForPost(postID string, fromMaster bool }() defer span.Finish() - resultVar0, resultVar1 := a.app.GetFileInfosForPost(postID, fromMaster) + resultVar0, resultVar1 := a.app.GetFileInfosForPost(postID, fromMaster, includeDeleted) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) @@ -5992,7 +5992,7 @@ func (a *OpenTracingAppLayer) GetFileInfosForPost(postID string, fromMaster bool return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) GetFileInfosForPostWithMigration(postID string) ([]*model.FileInfo, *model.AppError) { +func (a *OpenTracingAppLayer) GetFileInfosForPostWithMigration(postID string, includeDeleted bool) ([]*model.FileInfo, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetFileInfosForPostWithMigration") @@ -6004,7 +6004,7 @@ func (a *OpenTracingAppLayer) GetFileInfosForPostWithMigration(postID string) ([ }() defer span.Finish() - resultVar0, resultVar1 := a.app.GetFileInfosForPostWithMigration(postID) + resultVar0, resultVar1 := a.app.GetFileInfosForPostWithMigration(postID, includeDeleted) if resultVar1 != nil { span.LogFields(spanlog.Error(resultVar1)) diff --git a/app/post.go b/app/post.go index 487537029d..de087c4995 100644 --- a/app/post.go +++ b/app/post.go @@ -1269,6 +1269,8 @@ func (a *App) DeletePost(c request.CTX, postID, deleteByID string) (*model.Post, a.Srv().Go(func() { a.deletePostFiles(post.Id) }) + a.Srv().Store.FileInfo().InvalidateFileInfosForPostCache(postID, true) + a.Srv().Store.FileInfo().InvalidateFileInfosForPostCache(postID, false) } a.Srv().Go(func() { a.deleteFlaggedPosts(post.Id) @@ -1541,16 +1543,16 @@ func (a *App) GetRecentSearchesForUser(userID string) ([]*model.SearchParams, *m return searchParams, nil } -func (a *App) GetFileInfosForPostWithMigration(postID string) ([]*model.FileInfo, *model.AppError) { +func (a *App) GetFileInfosForPostWithMigration(postID string, includeDeleted bool) ([]*model.FileInfo, *model.AppError) { pchan := make(chan store.StoreResult, 1) go func() { - post, err := a.Srv().Store.Post().GetSingle(postID, false) + post, err := a.Srv().Store.Post().GetSingle(postID, includeDeleted) pchan <- store.StoreResult{Data: post, NErr: err} close(pchan) }() - infos, err := a.GetFileInfosForPost(postID, false) + infos, err := a.GetFileInfosForPost(postID, false, includeDeleted) if err != nil { return nil, err } @@ -1580,8 +1582,8 @@ func (a *App) GetFileInfosForPostWithMigration(postID string) ([]*model.FileInfo return infos, nil } -func (a *App) GetFileInfosForPost(postID string, fromMaster bool) ([]*model.FileInfo, *model.AppError) { - fileInfos, err := a.Srv().Store.FileInfo().GetForPost(postID, fromMaster, false, true) +func (a *App) GetFileInfosForPost(postID string, fromMaster bool, includeDeleted bool) ([]*model.FileInfo, *model.AppError) { + fileInfos, err := a.Srv().Store.FileInfo().GetForPost(postID, fromMaster, includeDeleted, true) if err != nil { return nil, model.NewAppError("GetFileInfosForPost", "app.file_info.get_for_post.app_error", nil, err.Error(), http.StatusInternalServerError) } diff --git a/app/post_metadata.go b/app/post_metadata.go index f5ff89cb19..80809c5ee4 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -216,7 +216,7 @@ func (a *App) getFileMetadataForPost(post *model.Post, fromMaster bool) ([]*mode return nil, nil } - return a.GetFileInfosForPost(post.Id, fromMaster) + return a.GetFileInfosForPost(post.Id, fromMaster, false) } func (a *App) getEmojisAndReactionsForPost(post *model.Post) ([]*model.Emoji, []*model.Reaction, *model.AppError) { diff --git a/app/post_test.go b/app/post_test.go index bc38d528aa..64c203e8f2 100644 --- a/app/post_test.go +++ b/app/post_test.go @@ -219,7 +219,7 @@ func TestAttachFilesToPost(t *testing.T) { appErr := th.App.attachFilesToPost(post) assert.Nil(t, appErr) - infos, appErr := th.App.GetFileInfosForPost(post.Id, false) + infos, appErr := th.App.GetFileInfosForPost(post.Id, false, false) assert.Nil(t, appErr) assert.Len(t, infos, 2) }) @@ -247,7 +247,7 @@ func TestAttachFilesToPost(t *testing.T) { appErr := th.App.attachFilesToPost(post) assert.Nil(t, appErr) - infos, appErr := th.App.GetFileInfosForPost(post.Id, false) + infos, appErr := th.App.GetFileInfosForPost(post.Id, false, false) assert.Nil(t, appErr) assert.Len(t, infos, 1) assert.Equal(t, info2.Id, infos[0].Id) diff --git a/model/client4.go b/model/client4.go index 76555fd9da..ba374830a1 100644 --- a/model/client4.go +++ b/model/client4.go @@ -4419,6 +4419,24 @@ func (c *Client4) GetFileInfosForPost(postId string, etag string) ([]*FileInfo, return list, BuildResponse(r), nil } +// GetFileInfosForPost gets all the file info objects attached to a post, including deleted +func (c *Client4) GetFileInfosForPostIncludeDeleted(postId string, etag string) ([]*FileInfo, *Response, error) { + r, err := c.DoAPIGet(c.postRoute(postId)+"/files/info"+"?include_deleted="+c.boolString(true), etag) + if err != nil { + return nil, BuildResponse(r), err + } + defer closeBody(r) + + var list []*FileInfo + if r.StatusCode == http.StatusNotModified { + return list, BuildResponse(r), nil + } + if jsonErr := json.NewDecoder(r.Body).Decode(&list); jsonErr != nil { + return nil, nil, NewAppError("GetFileInfosForPostIncludeDeleted", "api.unmarshal_error", nil, jsonErr.Error(), http.StatusInternalServerError) + } + return list, BuildResponse(r), nil +} + // General/System Section // GenerateSupportPacket downloads the generated support packet From 9255671222d154062a2730b1e9ede8be8430f09f Mon Sep 17 00:00:00 2001 From: Claudio Costa Date: Fri, 22 Jul 2022 09:14:01 +0200 Subject: [PATCH 05/28] [MM-45862] Set size and extension to fileinfo (#20681) * Set size and extension to fileinfo * Remove unnecessary check --- api4/upload_test.go | 5 ++++- app/upload.go | 13 +++++++++++-- model/file_info.go | 2 +- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/api4/upload_test.go b/api4/upload_test.go index d01e24bcdd..e373ecf5f3 100644 --- a/api4/upload_test.go +++ b/api4/upload_test.go @@ -207,7 +207,7 @@ func TestUploadData(t *testing.T) { CreateAt: model.GetMillis(), UserId: th.BasicUser2.Id, ChannelId: th.BasicChannel.Id, - Filename: "upload", + Filename: "upload.zip", FileSize: 8 * 1024 * 1024, } us, err := th.App.CreateUploadSession(th.Context, us) @@ -281,6 +281,9 @@ func TestUploadData(t *testing.T) { require.NoError(t, err) require.NotEmpty(t, info) require.Equal(t, u.Filename, info.Name) + require.Equal(t, u.FileSize, info.Size) + require.Equal(t, "zip", info.Extension) + require.Equal(t, "application/zip", info.MimeType) file, _, err := th.Client.GetFile(info.Id) require.NoError(t, err) diff --git a/app/upload.go b/app/upload.go index b1e37fd0b4..319f7e2e34 100644 --- a/app/upload.go +++ b/app/upload.go @@ -24,10 +24,19 @@ const minFirstPartSize = 5 * 1024 * 1024 // 5MB func (a *App) genFileInfoFromReader(name string, file io.ReadSeeker, size int64) (*model.FileInfo, error) { ext := strings.ToLower(filepath.Ext(name)) + info := &model.FileInfo{ - Name: name, - MimeType: mime.TypeByExtension(ext), + Name: name, + MimeType: mime.TypeByExtension(ext), + Size: size, + Extension: ext, } + + if ext != "" { + // The client expects a file extension without the leading period + info.Extension = ext[1:] + } + if info.IsImage() { config, _, err := a.ch.imgDecoder.DecodeConfig(file) if err != nil { diff --git a/model/file_info.go b/model/file_info.go index 723634b6fc..f42d1fbc54 100644 --- a/model/file_info.go +++ b/model/file_info.go @@ -141,7 +141,7 @@ func GetInfoForBytes(name string, data io.ReadSeeker, size int) (*FileInfo, *App extension := strings.ToLower(filepath.Ext(name)) info.MimeType = mime.TypeByExtension(extension) - if extension != "" && extension[0] == '.' { + if extension != "" { // The client expects a file extension without the leading period info.Extension = extension[1:] } else { From 68055bcb100ced5030da7ad4e2ab9dfa3465fb1e Mon Sep 17 00:00:00 2001 From: Michel Engelen <32863416+michelengelen@users.noreply.github.com> Date: Fri, 22 Jul 2022 12:23:14 +0200 Subject: [PATCH 06/28] added a feature-flag and default value for PostForwarding (#20166) Automatic Merge --- model/feature_flags.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/model/feature_flags.go b/model/feature_flags.go index b5b48a77f9..e5ddee2624 100644 --- a/model/feature_flags.go +++ b/model/feature_flags.go @@ -67,6 +67,8 @@ type FeatureFlags struct { CommandPalette bool + PostForwarding bool + AdvancedTextEditor bool // Enable Boards as a product (multi-product architecture) @@ -94,6 +96,7 @@ func (f *FeatureFlags) SetDefaults() { f.GraphQL = false f.InsightsEnabled = true f.CommandPalette = false + f.PostForwarding = false f.AdvancedTextEditor = true f.CallsEnabled = true f.BoardsProduct = false From fa9477d3325744109c744d85f9b03fc8781b1c6e Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Mon, 25 Jul 2022 14:25:32 +0530 Subject: [PATCH 07/28] MM-45000: Upgrade to new opengraph module path (#20701) https://mattermost.atlassian.net/browse/MM-45000 ```release-note NONE ``` --- app/post_metadata_test.go | 9 ++++--- go.mod | 2 +- go.sum | 5 ++-- model/link_metadata.go | 3 ++- model/link_metadata_test.go | 34 +++++++++++++++----------- store/storetest/link_metadata_store.go | 3 ++- 6 files changed, 33 insertions(+), 23 deletions(-) diff --git a/app/post_metadata_test.go b/app/post_metadata_test.go index 5101fa4208..3bf8ebf334 100644 --- a/app/post_metadata_test.go +++ b/app/post_metadata_test.go @@ -19,6 +19,7 @@ import ( "time" "github.com/dyatlov/go-opengraph/opengraph" + ogimage "github.com/dyatlov/go-opengraph/opengraph/types/image" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -1157,7 +1158,7 @@ func TestGetImagesForPost(t *testing.T) { Type: model.PostEmbedOpengraph, URL: ogURL, Data: &opengraph.OpenGraph{ - Images: []*opengraph.Image{ + Images: []*ogimage.Image{ { URL: imageURL, }, @@ -1211,7 +1212,7 @@ func TestGetImagesForPost(t *testing.T) { Type: model.PostEmbedOpengraph, URL: ogURL, Data: &opengraph.OpenGraph{ - Images: []*opengraph.Image{ + Images: []*ogimage.Image{ { SecureURL: imageURL, }, @@ -1264,7 +1265,7 @@ func TestGetImagesForPost(t *testing.T) { Type: model.PostEmbedOpengraph, URL: ogURL, Data: &opengraph.OpenGraph{ - Images: []*opengraph.Image{ + Images: []*ogimage.Image{ { URL: server.URL + "/image.png", SecureURL: imageURL, @@ -2711,7 +2712,7 @@ func TestSanitizePostMetadataForUserAndChannel(t *testing.T) { Type: model.PostEmbedOpengraph, URL: "ogURL", Data: &opengraph.OpenGraph{ - Images: []*opengraph.Image{ + Images: []*ogimage.Image{ { URL: "imageURL", }, diff --git a/go.mod b/go.mod index aa005fe7d9..569c956299 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/dgrijalva/jwt-go v3.2.0+incompatible github.com/dgryski/dgoogauth v0.0.0-20190221195224-5a805980a5f3 github.com/disintegration/imaging v1.6.2 - github.com/dyatlov/go-opengraph v0.0.0-20210112100619-dae8665a5b09 + github.com/dyatlov/go-opengraph/opengraph v0.0.0-20220524092352-606d7b1e5f8a github.com/francoispqt/gojay v1.2.13 github.com/fsnotify/fsnotify v1.5.4 github.com/getsentry/sentry-go v0.13.0 diff --git a/go.sum b/go.sum index e354e9f911..249690ac72 100644 --- a/go.sum +++ b/go.sum @@ -476,8 +476,8 @@ github.com/dsnet/golib v0.0.0-20171103203638-1ea166775780/go.mod h1:Lj+Z9rebOhdf github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo= github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= -github.com/dyatlov/go-opengraph v0.0.0-20210112100619-dae8665a5b09 h1:AQLr//nh20BzN3hIWj2+/Gt3FwSs8Nwo/nz4hMIcLPg= -github.com/dyatlov/go-opengraph v0.0.0-20210112100619-dae8665a5b09/go.mod h1:nYia/MIs9OyvXXYboPmNOj0gVWo97Wx0sde+ZuKkoM4= +github.com/dyatlov/go-opengraph/opengraph v0.0.0-20220524092352-606d7b1e5f8a h1:etIrTD8BQqzColk9nKRusM9um5+1q0iOEJLqfBMIK64= +github.com/dyatlov/go-opengraph/opengraph v0.0.0-20220524092352-606d7b1e5f8a/go.mod h1:emQhSYTXqB0xxjLITTw4EaWZ+8IIQYw+kx9GqNUKdLg= github.com/edsrzf/mmap-go v0.0.0-20170320065105-0bce6a688712/go.mod h1:YO35OhQPt3KJa3ryjFM5Bs14WD66h8eGKpfaBNrHW5M= github.com/elazarl/goproxy v0.0.0-20180725130230-947c36da3153/go.mod h1:/Zj4wYkgs4iZTTu3o/KG3Itv/qCCa8VVMlb3i9OVuzc= github.com/emicklei/go-restful v0.0.0-20170410110728-ff4f55a20633/go.mod h1:otzb+WCGbkyDHkqmQmT5YD2WR4BBwUdeQoFo8l/7tVs= @@ -1694,6 +1694,7 @@ golang.org/x/net v0.0.0-20211216030914-fe4d6282115f/go.mod h1:9nx3DQGgdP8bBQD5qx golang.org/x/net v0.0.0-20220111093109-d55c255bac03/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= golang.org/x/net v0.0.0-20220225172249-27dd8689420f/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= +golang.org/x/net v0.0.0-20220520000938-2e3eb7b945c2/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= golang.org/x/net v0.0.0-20220614195744-fb05da6f9022 h1:0qjDla5xICC2suMtyRH/QqX3B1btXTfNsIt/i4LFgO0= golang.org/x/net v0.0.0-20220614195744-fb05da6f9022/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/oauth2 v0.0.0-20180227000427-d7d64896b5ff/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= diff --git a/model/link_metadata.go b/model/link_metadata.go index 8f5fda1ec9..56f2a8640d 100644 --- a/model/link_metadata.go +++ b/model/link_metadata.go @@ -13,6 +13,7 @@ import ( "unicode/utf8" "github.com/dyatlov/go-opengraph/opengraph" + "github.com/dyatlov/go-opengraph/opengraph/types/image" ) const ( @@ -50,7 +51,7 @@ func truncateText(original string) string { return original } -func firstNImages(images []*opengraph.Image, maxImages int) []*opengraph.Image { +func firstNImages(images []*image.Image, maxImages int) []*image.Image { if maxImages < 0 { // don't break stuff, if it's weird, go for sane defaults maxImages = LinkMetadataMaxImages } diff --git a/model/link_metadata_test.go b/model/link_metadata_test.go index 49419d08fe..c67781503f 100644 --- a/model/link_metadata_test.go +++ b/model/link_metadata_test.go @@ -11,14 +11,20 @@ import ( "unicode/utf8" "github.com/dyatlov/go-opengraph/opengraph" + "github.com/dyatlov/go-opengraph/opengraph/types/article" + "github.com/dyatlov/go-opengraph/opengraph/types/audio" + "github.com/dyatlov/go-opengraph/opengraph/types/book" + "github.com/dyatlov/go-opengraph/opengraph/types/image" + "github.com/dyatlov/go-opengraph/opengraph/types/profile" + "github.com/dyatlov/go-opengraph/opengraph/types/video" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) const BigText = "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Vivamus maximus faucibus ex, vitae placerat neque feugiat ac. Nam tempus libero quis pellentesque feugiat. Cras tristique diam vel condimentum viverra. Proin molestie posuere leo. Nam pulvinar, ex quis tristique cursus, turpis ante commodo elit, a dapibus est ipsum id eros. Mauris tortor dolor, posuere ac velit vitae, faucibus viverra fusce." -func sampleImage(imageName string) *opengraph.Image { - return &opengraph.Image{ +func sampleImage(imageName string) *image.Image { + return &image.Image{ URL: fmt.Sprintf("http://example.com/%s", imageName), SecureURL: fmt.Sprintf("https://example.com/%s", imageName), Type: "png", @@ -180,7 +186,7 @@ func TestLinkMetadataDeserializeDataToConcreteType(t *testing.T) { og := &opengraph.OpenGraph{ URL: "http://example.com", Description: "Hello, world!", - Images: []*opengraph.Image{ + Images: []*image.Image{ { URL: "http://example.com/image.png", }, @@ -260,24 +266,24 @@ func TestTruncateText(t *testing.T) { func TestFirstNImages(t *testing.T) { t.Run("when empty, return an empty one", func(t *testing.T) { - empty := make([]*opengraph.Image, 0) + empty := make([]*image.Image, 0) assert.Exactly(t, firstNImages(empty, 1), empty, "Should be the same element") }) t.Run("when it contains one element, return the same array", func(t *testing.T) { - one := []*opengraph.Image{sampleImage("image.png")} + one := []*image.Image{sampleImage("image.png")} assert.Exactly(t, firstNImages(one, 1), one, "Should be the same element") }) t.Run("when it contains more than one element and asking for only one, return the first one", func(t *testing.T) { - two := []*opengraph.Image{sampleImage("image.png"), sampleImage("notme.png")} + two := []*image.Image{sampleImage("image.png"), sampleImage("notme.png")} assert.True(t, strings.HasSuffix(firstNImages(two, 1)[0].URL, "image.png"), "Should be the image element") }) t.Run("when it contains less than asked, return the original", func(t *testing.T) { - two := []*opengraph.Image{sampleImage("image.png"), sampleImage("notme.png")} + two := []*image.Image{sampleImage("image.png"), sampleImage("notme.png")} assert.Equal(t, two, firstNImages(two, 10), "should be the same pointer") }) t.Run("asking for negative images", func(t *testing.T) { - six := []*opengraph.Image{ + six := []*image.Image{ sampleImage("image.png"), sampleImage("another.png"), sampleImage("yetanother.jpg"), @@ -300,18 +306,18 @@ func TestTruncateOpenGraph(t *testing.T) { SiteName: BigText, Locale: "[EN-en]", LocalesAlternate: []string{"[EN-ca]", "[ES-es]"}, - Images: []*opengraph.Image{ + Images: []*image.Image{ sampleImage("image.png"), sampleImage("another.png"), sampleImage("yetanother.jpg"), sampleImage("metoo.gif"), sampleImage("fifth.ico"), sampleImage("notme.tiff")}, - Audios: []*opengraph.Audio{{}}, - Videos: []*opengraph.Video{{}}, - Article: &opengraph.Article{}, - Book: &opengraph.Book{}, - Profile: &opengraph.Profile{}, + Audios: []*audio.Audio{{}}, + Videos: []*video.Video{{}}, + Article: &article.Article{}, + Book: &book.Book{}, + Profile: &profile.Profile{}, } result := TruncateOpenGraph(&og) assert.Nil(t, result.Article, "No article stored") diff --git a/store/storetest/link_metadata_store.go b/store/storetest/link_metadata_store.go index 0a9376225a..ce7c0d6d7c 100644 --- a/store/storetest/link_metadata_store.go +++ b/store/storetest/link_metadata_store.go @@ -9,6 +9,7 @@ import ( "time" "github.com/dyatlov/go-opengraph/opengraph" + "github.com/dyatlov/go-opengraph/opengraph/types/image" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -205,7 +206,7 @@ func testLinkMetadataStoreTypes(t *testing.T, ss store.Store) { t.Run("should save and get opengraph data", func(t *testing.T) { og := &opengraph.OpenGraph{ URL: "http://example.com", - Images: []*opengraph.Image{ + Images: []*image.Image{ { URL: "http://example.com/image.png", }, From 07623a70fd046c403d9f7bf4ca18406f9261afb1 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Mon, 25 Jul 2022 15:23:14 +0530 Subject: [PATCH 08/28] MM-45875: Apply environment overrides in cluster scenario (#20694) Automatic Merge --- config/store.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/config/store.go b/config/store.go index 69e5bfa778..a3db0d186c 100644 --- a/config/store.go +++ b/config/store.go @@ -184,6 +184,11 @@ func (s *Store) Set(newCfg *model.Config) (*model.Config, *model.Config, error) // data from the existing config as necessary. desanitize(oldCfg, newCfg) + // We apply back environment overrides since the input config may or + // may not have them applied. + newCfg = applyEnvironmentMap(newCfg, GetEnvironment()) + fixConfig(newCfg) + if err := newCfg.IsValid(); err != nil { return nil, nil, errors.Wrap(err, "new configuration is invalid") } @@ -209,14 +214,6 @@ func (s *Store) Set(newCfg *model.Config) (*model.Config, *model.Config, error) return nil, nil, errors.Wrap(err, "failed to persist") } - // We apply back environment overrides since the input config may or - // may not have them applied. - newCfg = applyEnvironmentMap(newCfgNoEnv, GetEnvironment()) - fixConfig(newCfg) - if err := newCfg.IsValid(); err != nil { - return nil, nil, errors.Wrap(err, "new configuration is invalid") - } - hasChanged, err := equal(oldCfg, newCfg) if err != nil { return nil, nil, errors.Wrap(err, "failed to compare configs") From b18a42313b55dd74cf50cfd5f5ff19773f9395ea Mon Sep 17 00:00:00 2001 From: Ibrahim Serdar Acikgoz Date: Mon, 25 Jul 2022 16:54:06 +0300 Subject: [PATCH 09/28] move metrics server into platform service (#20683) move metrics into platform --- api4/apitestlib.go | 4 +- app/config.go | 4 +- app/platform/config.go | 14 ++++ app/platform/metrics.go | 162 ++++++++++++++++++++++++++++++++++++++++ app/platform/service.go | 31 +++++++- app/server.go | 145 +++++------------------------------ 6 files changed, 225 insertions(+), 135 deletions(-) create mode 100644 app/platform/metrics.go diff --git a/api4/apitestlib.go b/api4/apitestlib.go index 5ac2a9bac4..08c53286ac 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -322,8 +322,8 @@ func SetupWithStoreMock(tb testing.TB) *TestHelper { return th } -func SetupEnterpriseWithStoreMock(tb testing.TB) *TestHelper { - th := setupTestHelper(testlib.GetMockStoreForSetupFunctions(), nil, true, false, nil, nil) +func SetupEnterpriseWithStoreMock(tb testing.TB, options ...app.Option) *TestHelper { + th := setupTestHelper(testlib.GetMockStoreForSetupFunctions(), nil, true, false, nil, options) statusMock := mocks.StatusStore{} statusMock.On("UpdateExpiredDNDStatuses").Return([]*model.Status{}, nil) statusMock.On("Get", "user1").Return(&model.Status{UserId: "user1", Status: model.StatusOnline}, nil) diff --git a/app/config.go b/app/config.go index bbbc92ff44..9fb2d92af2 100644 --- a/app/config.go +++ b/app/config.go @@ -82,9 +82,9 @@ func (w *configWrapper) SaveConfig(newCfg *model.Config, sendConfigChangeCluster if w.srv.Metrics != nil { w.srv.Metrics.Register() } - w.srv.SetupMetricsServer() + w.srv.platformService.RestartMetrics() // TODO: remove when this moved to the platform service } else { - w.srv.StopMetricsServer() + w.srv.platformService.ShutdownMetrics() // TODO: remove when this moved to the platform service } if w.srv.Cluster != nil { diff --git a/app/platform/config.go b/app/platform/config.go index 2713166849..37e7286283 100644 --- a/app/platform/config.go +++ b/app/platform/config.go @@ -3,14 +3,28 @@ package platform +import ( + "errors" + + "github.com/mattermost/mattermost-server/v6/config" + "github.com/mattermost/mattermost-server/v6/einterfaces" +) + // ServiceConfig is used to initialize the PlatformService. // The mandatory fields will be checked during the initialization of the service. type ServiceConfig struct { // Mandatory fields + ConfigStore *config.Store + StartMetrics bool // TODO: find an elegant way to start/stop metrics server by default // Optional fields + Metrics einterfaces.MetricsInterface + Cluster einterfaces.ClusterInterface } func (c *ServiceConfig) validate() error { // Mandatory fields need to be checked here + if c.ConfigStore == nil { + return errors.New("ConfigStore is required") + } return nil } diff --git a/app/platform/metrics.go b/app/platform/metrics.go new file mode 100644 index 0000000000..e5daef5457 --- /dev/null +++ b/app/platform/metrics.go @@ -0,0 +1,162 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package platform + +import ( + "context" + "net" + "net/http" + "net/http/pprof" + "runtime" + "sync" + "text/template" + "time" + + "github.com/gorilla/handlers" + "github.com/gorilla/mux" + "github.com/mattermost/mattermost-server/v6/einterfaces" + "github.com/mattermost/mattermost-server/v6/model" + "github.com/mattermost/mattermost-server/v6/shared/mlog" + "github.com/pkg/errors" +) + +const TimeToWaitForConnectionsToCloseOnServerShutdown = time.Second + +type platformMetrics struct { + server *http.Server + router *mux.Router + lock sync.Mutex + + metricsImpl einterfaces.MetricsInterface + + cfgFn func() *model.Config +} + +func newPlatformMetrics(metricsImpl einterfaces.MetricsInterface, cfgFn func() *model.Config) *platformMetrics { + if !*cfgFn().MetricsSettings.Enable { + return nil + } + + pm := &platformMetrics{ + cfgFn: cfgFn, + } + + pm.stopMetricsServer() + + if err := pm.initMetricsRouter(); err != nil { + mlog.Error("Error initiating metrics router.", mlog.Err(err)) + } + + if metricsImpl != nil { + metricsImpl.Register() + } + + pm.startMetricsServer() + + return pm +} + +func (pm *platformMetrics) stopMetricsServer() { + pm.lock.Lock() + defer pm.lock.Unlock() + + if pm.server != nil { + ctx, cancel := context.WithTimeout(context.Background(), TimeToWaitForConnectionsToCloseOnServerShutdown) + defer cancel() + + pm.server.Shutdown(ctx) + mlog.Info("Metrics and profiling server is stopping") + } +} + +func (pm *platformMetrics) startMetricsServer() { + var notify chan struct{} + pm.lock.Lock() + defer func() { + if notify != nil { + <-notify + } + pm.lock.Unlock() + }() + + l, err := net.Listen("tcp", *pm.cfgFn().MetricsSettings.ListenAddress) + if err != nil { + mlog.Error(err.Error()) + return + } + + notify = make(chan struct{}) + pm.server = &http.Server{ + Handler: handlers.RecoveryHandler(handlers.PrintRecoveryStack(true))(pm.router), + ReadTimeout: time.Duration(*pm.cfgFn().ServiceSettings.ReadTimeout) * time.Second, + WriteTimeout: time.Duration(*pm.cfgFn().ServiceSettings.WriteTimeout) * time.Second, + } + + go func() { + close(notify) + if err := pm.server.Serve(l); err != nil && err != http.ErrServerClosed { + mlog.Critical(err.Error()) + } + }() + + mlog.Info("Metrics and profiling server is started", mlog.String("address", l.Addr().String())) +} + +func (pm *platformMetrics) initMetricsRouter() error { + pm.router = mux.NewRouter() + runtime.SetBlockProfileRate(*pm.cfgFn().MetricsSettings.BlockProfileRate) + + metricsPage := ` + + {{if .}} + {{end}} + + + + + + + + + + + + ` + metricsPageTmpl, err := template.New("page").Parse(metricsPage) + if err != nil { + return errors.Wrap(err, "failed to create template") + } + + rootHandler := func(w http.ResponseWriter, r *http.Request) { + metricsPageTmpl.Execute(w, pm.metricsImpl != nil) + } + + pm.router.HandleFunc("/", rootHandler) + pm.router.StrictSlash(true) + + pm.router.Handle("/debug", http.RedirectHandler("/", http.StatusMovedPermanently)) + pm.router.HandleFunc("/debug/pprof/", pprof.Index) + pm.router.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline) + pm.router.HandleFunc("/debug/pprof/profile", pprof.Profile) + pm.router.HandleFunc("/debug/pprof/symbol", pprof.Symbol) + pm.router.HandleFunc("/debug/pprof/trace", pprof.Trace) + + // Manually add support for paths linked to by index page at /debug/pprof/ + pm.router.Handle("/debug/pprof/goroutine", pprof.Handler("goroutine")) + pm.router.Handle("/debug/pprof/heap", pprof.Handler("heap")) + pm.router.Handle("/debug/pprof/threadcreate", pprof.Handler("threadcreate")) + pm.router.Handle("/debug/pprof/block", pprof.Handler("block")) + + return nil +} + +func (ps *PlatformService) HandleMetrics(route string, h http.Handler) { + if ps.metrics.router != nil { + ps.metrics.router.Handle(route, h) + } +} + +func (ps *PlatformService) RestartMetrics() { + ps.metrics = newPlatformMetrics(ps.serviceConfig.Metrics, ps.serviceConfig.ConfigStore.Get) +} diff --git a/app/platform/service.go b/app/platform/service.go index a9eda866f7..7524a3a491 100644 --- a/app/platform/service.go +++ b/app/platform/service.go @@ -3,17 +3,42 @@ package platform +import ( + "github.com/mattermost/mattermost-server/v6/config" + "github.com/mattermost/mattermost-server/v6/einterfaces" +) + // PlatformService is the service for the platform related tasks. It is // responsible for non-entity related functionalities that are required // by a product such as database access, configuration access, licensing etc. type PlatformService struct { + serviceConfig ServiceConfig + configStore *config.Store + + metrics *platformMetrics + + cluster einterfaces.ClusterInterface } // New creates a new PlatformService. -func New(c ServiceConfig) (*PlatformService, error) { - if err := c.validate(); err != nil { +func New(sc ServiceConfig) (*PlatformService, error) { + if err := sc.validate(); err != nil { return nil, err } - return &PlatformService{}, nil + ps := &PlatformService{ + serviceConfig: sc, + configStore: sc.ConfigStore, + cluster: sc.Cluster, + } + + ps.metrics = newPlatformMetrics(sc.Metrics, ps.configStore.Get) + + return ps, nil +} + +func (ps *PlatformService) ShutdownMetrics() { + if ps.metrics != nil { + ps.metrics.stopMetricsServer() + } } diff --git a/app/server.go b/app/server.go index 5b556c55b4..3066ecc858 100644 --- a/app/server.go +++ b/app/server.go @@ -9,10 +9,8 @@ import ( "crypto/tls" "fmt" "hash/maphash" - "html/template" "net" "net/http" - "net/http/pprof" "net/url" "os" "os/exec" @@ -27,7 +25,6 @@ import ( "github.com/getsentry/sentry-go" sentryhttp "github.com/getsentry/sentry-go/http" - "github.com/gorilla/handlers" "github.com/gorilla/mux" "github.com/pkg/errors" "github.com/rs/cors" @@ -35,6 +32,7 @@ import ( "github.com/mattermost/mattermost-server/v6/app/email" "github.com/mattermost/mattermost-server/v6/app/featureflag" + "github.com/mattermost/mattermost-server/v6/app/platform" "github.com/mattermost/mattermost-server/v6/app/request" "github.com/mattermost/mattermost-server/v6/app/teams" "github.com/mattermost/mattermost-server/v6/app/users" @@ -131,10 +129,6 @@ type Server struct { localModeServer *http.Server - metricsServer *http.Server - metricsRouter *mux.Router - metricsLock sync.Mutex - didFinishListen chan struct{} goroutineCount int32 @@ -177,6 +171,7 @@ type Server struct { configStore *configWrapper filestore filestore.FileBackend + platformService *platform.PlatformService telemetryService *telemetry.TelemetryService userService *users.UserService teamService *teams.TeamService @@ -256,6 +251,17 @@ func NewServer(options ...Option) (*Server, error) { s.configStore = &configWrapper{srv: s, Store: configStore} } + ps, sErr := platform.New(platform.ServiceConfig{ + ConfigStore: s.configStore.Store, + StartMetrics: s.startMetrics, + Metrics: s.Metrics, + Cluster: s.Cluster, + }) + if sErr != nil { + return nil, errors.Wrap(sErr, "failed to initialize platform") + } + s.platformService = ps + // Step 2: Logging if err := s.initLogging(); err != nil { mlog.Error("Could not initiate logging", mlog.Err(err)) @@ -619,10 +625,6 @@ func NewServer(options ...Option) (*Server, error) { s.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableDeveloper = true }) } - if s.startMetrics { - s.SetupMetricsServer() - } - s.AddLicenseListener(func(oldLicense, newLicense *model.License) { if (oldLicense == nil && newLicense == nil) || !s.startMetrics { return @@ -632,7 +634,7 @@ func NewServer(options ...Option) (*Server, error) { return } - s.SetupMetricsServer() + s.platformService.RestartMetrics() // TODO: remove when this moved to the platform service }) s.SearchEngine.UpdateConfig(s.Config()) @@ -700,24 +702,6 @@ func NewServer(options ...Option) (*Server, error) { return s, nil } -func (s *Server) SetupMetricsServer() { - if !*s.Config().MetricsSettings.Enable { - return - } - - s.StopMetricsServer() - - if err := s.InitMetricsRouter(); err != nil { - mlog.Error("Error initiating metrics router.", mlog.Err(err)) - } - - if s.Metrics != nil { - s.Metrics.Register() - } - - s.startMetricsServer() -} - func maxInt(a, b int) int { if a > b { return a @@ -1045,7 +1029,7 @@ func (s *Server) Shutdown() { s.Cluster.StopInterNodeCommunication() } - s.StopMetricsServer() + s.platformService.ShutdownMetrics() // This must be done after the cluster is stopped. if s.Jobs != nil { @@ -1629,104 +1613,9 @@ func doConfigCleanup(s *Server) { } } -func (s *Server) StopMetricsServer() { - s.metricsLock.Lock() - defer s.metricsLock.Unlock() - - if s.metricsServer != nil { - ctx, cancel := context.WithTimeout(context.Background(), TimeToWaitForConnectionsToCloseOnServerShutdown) - defer cancel() - - s.metricsServer.Shutdown(ctx) - s.Log.Info("Metrics and profiling server is stopping") - } -} - +// TODO: remove this method when we switch to using platform service. func (s *Server) HandleMetrics(route string, h http.Handler) { - if s.metricsRouter != nil { - s.metricsRouter.Handle(route, h) - } -} - -func (s *Server) InitMetricsRouter() error { - s.metricsRouter = mux.NewRouter() - runtime.SetBlockProfileRate(*s.Config().MetricsSettings.BlockProfileRate) - - metricsPage := ` - - {{if .}} - {{end}} - - - - - - - - - - - - ` - metricsPageTmpl, err := template.New("page").Parse(metricsPage) - if err != nil { - return errors.Wrap(err, "failed to create template") - } - - rootHandler := func(w http.ResponseWriter, r *http.Request) { - metricsPageTmpl.Execute(w, s.Metrics != nil) - } - - s.metricsRouter.HandleFunc("/", rootHandler) - s.metricsRouter.StrictSlash(true) - - s.metricsRouter.Handle("/debug", http.RedirectHandler("/", http.StatusMovedPermanently)) - s.metricsRouter.HandleFunc("/debug/pprof/", pprof.Index) - s.metricsRouter.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline) - s.metricsRouter.HandleFunc("/debug/pprof/profile", pprof.Profile) - s.metricsRouter.HandleFunc("/debug/pprof/symbol", pprof.Symbol) - s.metricsRouter.HandleFunc("/debug/pprof/trace", pprof.Trace) - - // Manually add support for paths linked to by index page at /debug/pprof/ - s.metricsRouter.Handle("/debug/pprof/goroutine", pprof.Handler("goroutine")) - s.metricsRouter.Handle("/debug/pprof/heap", pprof.Handler("heap")) - s.metricsRouter.Handle("/debug/pprof/threadcreate", pprof.Handler("threadcreate")) - s.metricsRouter.Handle("/debug/pprof/block", pprof.Handler("block")) - - return nil -} - -func (s *Server) startMetricsServer() { - var notify chan struct{} - s.metricsLock.Lock() - defer func() { - if notify != nil { - <-notify - } - s.metricsLock.Unlock() - }() - - l, err := net.Listen("tcp", *s.Config().MetricsSettings.ListenAddress) - if err != nil { - mlog.Error(err.Error()) - return - } - - notify = make(chan struct{}) - s.metricsServer = &http.Server{ - Handler: handlers.RecoveryHandler(handlers.PrintRecoveryStack(true))(s.metricsRouter), - ReadTimeout: time.Duration(*s.Config().ServiceSettings.ReadTimeout) * time.Second, - WriteTimeout: time.Duration(*s.Config().ServiceSettings.WriteTimeout) * time.Second, - } - - go func() { - close(notify) - if err := s.metricsServer.Serve(l); err != nil && err != http.ErrServerClosed { - mlog.Critical(err.Error()) - } - }() - - s.Log.Info("Metrics and profiling server is started", mlog.String("address", l.Addr().String())) + s.platformService.HandleMetrics(route, h) } func (s *Server) sendLicenseUpForRenewalEmail(users map[string]*model.User, license *model.License) *model.AppError { From 2388848f4751064c4f44d104be1e94907b967c2b Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Mon, 25 Jul 2022 22:15:24 +0530 Subject: [PATCH 10/28] Killing dyatlov/go-opengraph with fire (#20706) Correctly updating this requires PRs in multiple repositories. Fixing this with a hammer for now to unblock server. ```release-note NONE ``` --- go.mod | 1 + 1 file changed, 1 insertion(+) diff --git a/go.mod b/go.mod index 569c956299..4f0f239fca 100644 --- a/go.mod +++ b/go.mod @@ -185,5 +185,6 @@ require ( exclude ( github.com/RoaringBitmap/roaring v0.7.0 github.com/RoaringBitmap/roaring v0.7.1 + github.com/dyatlov/go-opengraph v0.0.0-20210112100619-dae8665a5b09 github.com/willf/bitset v1.2.0 ) From 478ff50ffdf67ca847079c606d845a8a82f39049 Mon Sep 17 00:00:00 2001 From: Allan Guwatudde Date: Mon, 25 Jul 2022 22:22:07 +0300 Subject: [PATCH 11/28] [MM-45666] - A/B Test: Nav Bar Upgrade button (#20676) * [MM-45666] - A/B Test: Nav Bar Upgrade button * update flag default value * update text --- model/feature_flags.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/model/feature_flags.go b/model/feature_flags.go index e5ddee2624..f2f14bd563 100644 --- a/model/feature_flags.go +++ b/model/feature_flags.go @@ -73,6 +73,8 @@ type FeatureFlags struct { // Enable Boards as a product (multi-product architecture) BoardsProduct bool + + PlanUpgradeButtonText string } func (f *FeatureFlags) SetDefaults() { @@ -100,6 +102,7 @@ func (f *FeatureFlags) SetDefaults() { f.AdvancedTextEditor = true f.CallsEnabled = true f.BoardsProduct = false + f.PlanUpgradeButtonText = "Upgrade" } func (f *FeatureFlags) Plugins() map[string]string { From 77881fc35748aad73aa05733584820e99d52c620 Mon Sep 17 00:00:00 2001 From: Nathaniel Allred Date: Mon, 25 Jul 2022 16:01:59 -0500 Subject: [PATCH 12/28] Mm 43609 (#20657) * return first inaccessible post time instead of has inaccessible posts --- api4/post.go | 6 +-- app/app_iface.go | 2 +- app/opentracing/opentracing_layer.go | 2 +- app/post.go | 16 +++--- app/post_helpers.go | 80 ++++++++++++++++++++-------- app/post_helpers_test.go | 45 +++++++++++++--- app/post_metadata.go | 12 ++--- model/client4.go | 48 ++++++++--------- model/post_list.go | 16 +++--- 9 files changed, 147 insertions(+), 80 deletions(-) diff --git a/api4/post.go b/api4/post.go index eb2dc1a26b..c5a8c5f0b3 100644 --- a/api4/post.go +++ b/api4/post.go @@ -401,7 +401,7 @@ func getPost(c *Context, w http.ResponseWriter, r *http.Request) { // Post is inaccessible due to cloud plan's limit. if err.Id == "app.post.cloud.get.app_error" { - w.Header().Set(model.HeaderHasInaccessiblePosts, "true") + w.Header().Set(model.HeaderFirstInaccessiblePostTime, "1") } return @@ -438,7 +438,7 @@ func getPostsByIds(c *Context, w http.ResponseWriter, r *http.Request) { return } - postsList, hasInaccessiblePosts, err := c.App.GetPostsByIds(postIDs) + postsList, firstInaccessiblePostTime, err := c.App.GetPostsByIds(postIDs) if err != nil { c.Err = err return @@ -471,7 +471,7 @@ func getPostsByIds(c *Context, w http.ResponseWriter, r *http.Request) { posts = append(posts, post) } - w.Header().Set(model.HeaderHasInaccessiblePosts, strconv.FormatBool(hasInaccessiblePosts)) + w.Header().Set(model.HeaderFirstInaccessiblePostTime, strconv.FormatInt(firstInaccessiblePostTime, 10)) if err := json.NewEncoder(w).Encode(posts); err != nil { mlog.Warn("Error while writing response", mlog.Err(err)) diff --git a/app/app_iface.go b/app/app_iface.go index 2ac622e3c0..fcce9b66e3 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -202,7 +202,7 @@ type AppIface interface { // lock instead. GetPluginsEnvironment() *plugin.Environment // GetPostsByIds response bool value indicates, if the post is inaccessible due to cloud plan's limit. - GetPostsByIds(postIDs []string) ([]*model.Post, bool, *model.AppError) + GetPostsByIds(postIDs []string) ([]*model.Post, int64, *model.AppError) // GetPostsUsage returns the total posts count rounded down to the most // significant digit GetPostsUsage() (int64, *model.AppError) diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index 5761473dcf..28a25fdcd6 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -7881,7 +7881,7 @@ func (a *OpenTracingAppLayer) GetPostsBeforePost(options model.GetPostsOptions) return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) GetPostsByIds(postIDs []string) ([]*model.Post, bool, *model.AppError) { +func (a *OpenTracingAppLayer) GetPostsByIds(postIDs []string) ([]*model.Post, int64, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetPostsByIds") diff --git a/app/post.go b/app/post.go index de087c4995..adfa7302ec 100644 --- a/app/post.go +++ b/app/post.go @@ -870,11 +870,11 @@ func (a *App) GetSinglePost(postID string, includeDeleted bool) (*model.Post, *m } } - isInaccessible, appErr := a.isInaccessiblePost(post) + firstInaccessiblePostTime, appErr := a.isInaccessiblePost(post) if appErr != nil { return nil, appErr } - if isInaccessible { + if firstInaccessiblePostTime != 0 { return nil, model.NewAppError("GetSinglePost", "app.post.cloud.get.app_error", nil, "", http.StatusForbidden) } @@ -1881,24 +1881,24 @@ func (a *App) GetPostIfAuthorized(c request.CTX, postID string, session *model.S } // GetPostsByIds response bool value indicates, if the post is inaccessible due to cloud plan's limit. -func (a *App) GetPostsByIds(postIDs []string) ([]*model.Post, bool, *model.AppError) { +func (a *App) GetPostsByIds(postIDs []string) ([]*model.Post, int64, *model.AppError) { posts, err := a.Srv().Store.Post().GetPostsByIds(postIDs) if err != nil { var nfErr *store.ErrNotFound switch { case errors.As(err, &nfErr): - return nil, false, model.NewAppError("GetPostsByIds", "app.post.get.app_error", nil, nfErr.Error(), http.StatusNotFound) + return nil, 0, model.NewAppError("GetPostsByIds", "app.post.get.app_error", nil, nfErr.Error(), http.StatusNotFound) default: - return nil, false, model.NewAppError("GetPostsByIds", "app.post.get.app_error", nil, err.Error(), http.StatusInternalServerError) + return nil, 0, model.NewAppError("GetPostsByIds", "app.post.get.app_error", nil, err.Error(), http.StatusInternalServerError) } } - posts, hasInaccessiblePosts, appErr := a.getFilteredAccessiblePosts(posts, filterPostOptions{assumeSortedCreatedAt: true}) + posts, firstInaccessiblePostTime, appErr := a.getFilteredAccessiblePosts(posts, filterPostOptions{assumeSortedCreatedAt: true}) if appErr != nil { - return nil, false, appErr + return nil, 0, appErr } - return posts, hasInaccessiblePosts, nil + return posts, firstInaccessiblePostTime, nil } func (a *App) GetTopThreadsForTeamSince(c request.CTX, teamID, userID string, opts *model.InsightsOpts) (*model.TopThreadList, *model.AppError) { diff --git a/app/post_helpers.go b/app/post_helpers.go index 88f9487829..4ae91bf60f 100644 --- a/app/post_helpers.go +++ b/app/post_helpers.go @@ -27,6 +27,19 @@ func (b accessibleBounds) noAccessible() bool { return b.start == noAccessibleBounds.start && b.end == noAccessibleBounds.end } +// assumes checking was already performed that at least one post is inaccessible +func (b accessibleBounds) getInaccessibleRange(listLength int) (int, int) { + var start, end int + if b.start == 0 { + start = b.end + 1 + end = listLength - 1 + } else { + start = 0 + end = b.start - 1 + } + return start, end +} + var noAccessibleBounds = accessibleBounds{start: -1, end: -1} var allAccessibleBounds = func(lenPosts int) accessibleBounds { return accessibleBounds{start: 0, end: lenPosts - 1} } @@ -82,11 +95,13 @@ func linearFilterPostList(postList *model.PostList, earliestAccessibleTime int64 n := 0 for i, postId := range order { - if posts[postId].CreateAt >= earliestAccessibleTime { + if createAt := posts[postId].CreateAt; createAt >= earliestAccessibleTime { order[n] = order[i] n++ } else { - postList.HasInaccessiblePosts = true + if createAt > postList.FirstInaccessiblePostTime { + postList.FirstInaccessiblePostTime = createAt + } delete(posts, postId) } } @@ -96,8 +111,10 @@ func linearFilterPostList(postList *model.PostList, earliestAccessibleTime int64 // for example GetPosts in the CollapsedThreads = false path, parents are not added // to Order for postId := range posts { - if posts[postId].CreateAt < earliestAccessibleTime { - postList.HasInaccessiblePosts = true + if createAt := posts[postId].CreateAt; createAt < earliestAccessibleTime { + if createAt > postList.FirstInaccessiblePostTime { + postList.FirstInaccessiblePostTime = createAt + } delete(posts, postId) } } @@ -106,18 +123,20 @@ func linearFilterPostList(postList *model.PostList, earliestAccessibleTime int64 // linearFilterPostsSlice make no assumptions about ordering, go through posts one by one // this is the slower fallback that is still safe if we can not // assume posts are ordered by CreatedAt -func linearFilterPostsSlice(posts []*model.Post, earliestAccessibleTime int64) ([]*model.Post, bool) { - hasInaccessiblePosts := false +func linearFilterPostsSlice(posts []*model.Post, earliestAccessibleTime int64) ([]*model.Post, int64) { + var firstInaccessiblePostTime int64 = 0 n := 0 for i := range posts { - if posts[i].CreateAt >= earliestAccessibleTime { + if createAt := posts[i].CreateAt; createAt >= earliestAccessibleTime { posts[n] = posts[i] n++ } else { - hasInaccessiblePosts = true + if createAt > firstInaccessiblePostTime { + firstInaccessiblePostTime = createAt + } } } - return posts[:n], hasInaccessiblePosts + return posts[:n], firstInaccessiblePostTime } // filterInaccessiblePosts filters out the posts, past the cloud limit @@ -146,13 +165,18 @@ func (a *App) filterInaccessiblePosts(postList *model.PostList, options filterPo } if bounds.noAccessible() { if lenPosts > 0 { - postList.HasInaccessiblePosts = true + firstPostCreatedAt := postList.Posts[postList.Order[0]].CreateAt + lastPostCreatedAt := postList.Posts[postList.Order[len(postList.Order)-1]].CreateAt + postList.FirstInaccessiblePostTime = max(firstPostCreatedAt, lastPostCreatedAt) } postList.Posts = map[string]*model.Post{} postList.Order = []string{} return nil } - postList.HasInaccessiblePosts = true + startInaccessibleIndex, endInaccessibleIndex := bounds.getInaccessibleRange(len(postList.Order)) + startInaccessibleCreatedAt := postList.Posts[postList.Order[startInaccessibleIndex]].CreateAt + endInaccessibleCreatedAt := postList.Posts[postList.Order[endInaccessibleIndex]].CreateAt + postList.FirstInaccessiblePostTime = max(startInaccessibleCreatedAt, endInaccessibleCreatedAt) posts := postList.Posts order := postList.Order @@ -183,9 +207,9 @@ func (a *App) filterInaccessiblePosts(postList *model.PostList, options filterPo } // isInaccessiblePost indicates if the post is past the cloud plan's limit. -func (a *App) isInaccessiblePost(post *model.Post) (bool, *model.AppError) { +func (a *App) isInaccessiblePost(post *model.Post) (int64, *model.AppError) { if post == nil { - return false, nil + return 0, nil } pl := &model.PostList{ @@ -193,22 +217,22 @@ func (a *App) isInaccessiblePost(post *model.Post) (bool, *model.AppError) { Posts: map[string]*model.Post{post.Id: post}, } - return pl.HasInaccessiblePosts, a.filterInaccessiblePosts(pl, filterPostOptions{assumeSortedCreatedAt: true}) + return pl.FirstInaccessiblePostTime, a.filterInaccessiblePosts(pl, filterPostOptions{assumeSortedCreatedAt: true}) } // getFilteredAccessiblePosts returns accessible posts filtered as per the cloud plan's limit and also indicates if there were any inaccessible posts -func (a *App) getFilteredAccessiblePosts(posts []*model.Post, options filterPostOptions) ([]*model.Post, bool, *model.AppError) { +func (a *App) getFilteredAccessiblePosts(posts []*model.Post, options filterPostOptions) ([]*model.Post, int64, *model.AppError) { if len(posts) == 0 { - return posts, false, nil + return posts, 0, nil } filteredPosts := []*model.Post{} lastAccessiblePostTime, appErr := a.GetLastAccessiblePostTime() if appErr != nil { - return filteredPosts, false, model.NewAppError("getFilteredAccessiblePosts", "app.last_accessible_post.app_error", nil, appErr.Error(), http.StatusInternalServerError) + return filteredPosts, 0, model.NewAppError("getFilteredAccessiblePosts", "app.last_accessible_post.app_error", nil, appErr.Error(), http.StatusInternalServerError) } else if lastAccessiblePostTime == 0 { // No need to filter, all posts are accessible - return posts, false, nil + return posts, 0, nil } if options.assumeSortedCreatedAt { @@ -216,16 +240,26 @@ func (a *App) getFilteredAccessiblePosts(posts []*model.Post, options filterPost getCreateAt := func(i int) int64 { return posts[i].CreateAt } bounds := getTimeSortedPostAccessibleBounds(lastAccessiblePostTime, lenPosts, getCreateAt) if bounds.allAccessible(lenPosts) { - return posts, false, nil + return posts, 0, nil } if bounds.noAccessible() { - return filteredPosts, lenPosts > 0, nil + var firstInaccessiblePostTime int64 = 0 + if lenPosts > 0 { + firstPostCreatedAt := posts[0].CreateAt + lastPostCreatedAt := posts[len(posts)-1].CreateAt + firstInaccessiblePostTime = max(firstPostCreatedAt, lastPostCreatedAt) + } + return filteredPosts, firstInaccessiblePostTime, nil } + startInaccessibleIndex, endInaccessibleIndex := bounds.getInaccessibleRange(len(posts)) + firstPostCreatedAt := posts[startInaccessibleIndex].CreateAt + lastPostCreatedAt := posts[endInaccessibleIndex].CreateAt + firstInaccessiblePostTime := max(firstPostCreatedAt, lastPostCreatedAt) filteredPosts = posts[bounds.start : bounds.end+1] - return filteredPosts, true, nil + return filteredPosts, firstInaccessiblePostTime, nil } - filteredPosts, hasInaccessiblePosts := linearFilterPostsSlice(posts, lastAccessiblePostTime) - return filteredPosts, hasInaccessiblePosts, nil + filteredPosts, firstInaccessiblePostTime := linearFilterPostsSlice(posts, lastAccessiblePostTime) + return filteredPosts, firstInaccessiblePostTime, nil } diff --git a/app/post_helpers_test.go b/app/post_helpers_test.go index ebde2f2e1a..34b17e9e34 100644 --- a/app/post_helpers_test.go +++ b/app/post_helpers_test.go @@ -231,6 +231,7 @@ func TestFilterInaccessiblePosts(t *testing.T) { "post_d", "post_e", }, postList.Order) + assert.Equal(t, int64(1), postList.FirstInaccessiblePostTime) }) t.Run("descending order returns correct posts", func(t *testing.T) { @@ -259,6 +260,8 @@ func TestFilterInaccessiblePosts(t *testing.T) { "post_d", "post_c", }, postList.Order) + + assert.Equal(t, int64(1), postList.FirstInaccessiblePostTime) }) t.Run("handles mixed create at ordering correctly if correct options given", func(t *testing.T) { @@ -332,18 +335,20 @@ func TestGetFilteredAccessiblePosts(t *testing.T) { t.Run("ascending order returns correct posts", func(t *testing.T) { posts := []*model.Post{postFromCreateAt(0), postFromCreateAt(1), postFromCreateAt(2), postFromCreateAt(3), postFromCreateAt(4)} - filteredPosts, _, appErr := th.App.getFilteredAccessiblePosts(posts, filterPostOptions{assumeSortedCreatedAt: true}) + filteredPosts, firstInaccessiblePostTime, appErr := th.App.getFilteredAccessiblePosts(posts, filterPostOptions{assumeSortedCreatedAt: true}) assert.Nil(t, appErr) assert.Equal(t, []*model.Post{postFromCreateAt(2), postFromCreateAt(3), postFromCreateAt(4)}, filteredPosts) + assert.Equal(t, int64(1), firstInaccessiblePostTime) }) t.Run("descending order returns correct posts", func(t *testing.T) { posts := []*model.Post{postFromCreateAt(4), postFromCreateAt(3), postFromCreateAt(2), postFromCreateAt(1), postFromCreateAt(0)} - filteredPosts, _, appErr := th.App.getFilteredAccessiblePosts(posts, filterPostOptions{assumeSortedCreatedAt: true}) + filteredPosts, firstInaccessiblePostTime, appErr := th.App.getFilteredAccessiblePosts(posts, filterPostOptions{assumeSortedCreatedAt: true}) assert.Nil(t, appErr) assert.Equal(t, []*model.Post{postFromCreateAt(4), postFromCreateAt(3), postFromCreateAt(2)}, filteredPosts) + assert.Equal(t, int64(1), firstInaccessiblePostTime) }) t.Run("handles mixed create at ordering correctly if correct options given", func(t *testing.T) { @@ -366,12 +371,40 @@ func TestIsInaccessiblePost(t *testing.T) { defer th.TearDown() post := &model.Post{CreateAt: 3} - r, appErr := th.App.isInaccessiblePost(post) + firstInaccessiblePostTime, appErr := th.App.isInaccessiblePost(post) assert.Nil(t, appErr) - assert.Equal(t, false, r) + assert.Equal(t, int64(0), firstInaccessiblePostTime) post = &model.Post{CreateAt: 1} - r, appErr = th.App.isInaccessiblePost(post) + firstInaccessiblePostTime, appErr = th.App.isInaccessiblePost(post) assert.Nil(t, appErr) - assert.Equal(t, true, r) + assert.Equal(t, int64(1), firstInaccessiblePostTime) +} + +func Test_getInaccessibleRange(t *testing.T) { + type test struct { + label string + bounds accessibleBounds + listLength int + expectedStart int + expectedEnd int + } + tests := []test{ + { + label: "inaccessible at end", + bounds: accessibleBounds{start: 0, end: 3}, + listLength: 6, + expectedStart: 4, + expectedEnd: 5, + }, + } + + for _, test := range tests { + t.Run(test.label, func(t *testing.T) { + start, end := test.bounds.getInaccessibleRange(test.listLength) + + assert.Equal(t, test.expectedStart, start) + assert.Equal(t, test.expectedEnd, end) + }) + } } diff --git a/app/post_metadata.go b/app/post_metadata.go index 80809c5ee4..cbc920d506 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -54,12 +54,12 @@ func (s *Server) initPostMetadata() { func (a *App) PreparePostListForClient(c request.CTX, originalList *model.PostList) *model.PostList { list := &model.PostList{ - Posts: make(map[string]*model.Post, len(originalList.Posts)), - Order: originalList.Order, - NextPostId: originalList.NextPostId, - PrevPostId: originalList.PrevPostId, - HasNext: originalList.HasNext, - HasInaccessiblePosts: originalList.HasInaccessiblePosts, + Posts: make(map[string]*model.Post, len(originalList.Posts)), + Order: originalList.Order, + NextPostId: originalList.NextPostId, + PrevPostId: originalList.PrevPostId, + HasNext: originalList.HasNext, + FirstInaccessiblePostTime: originalList.FirstInaccessiblePostTime, } for id, originalPost := range originalList.Posts { diff --git a/model/client4.go b/model/client4.go index ba374830a1..1dc5f0aa8d 100644 --- a/model/client4.go +++ b/model/client4.go @@ -18,30 +18,30 @@ import ( ) const ( - HeaderRequestId = "X-Request-ID" - HeaderVersionId = "X-Version-ID" - HeaderClusterId = "X-Cluster-ID" - HeaderEtagServer = "ETag" - HeaderEtagClient = "If-None-Match" - HeaderForwarded = "X-Forwarded-For" - HeaderRealIP = "X-Real-IP" - HeaderForwardedProto = "X-Forwarded-Proto" - HeaderToken = "token" - HeaderCsrfToken = "X-CSRF-Token" - HeaderBearer = "BEARER" - HeaderAuth = "Authorization" - HeaderCloudToken = "X-Cloud-Token" - HeaderRemoteclusterToken = "X-RemoteCluster-Token" - HeaderRemoteclusterId = "X-RemoteCluster-Id" - HeaderRequestedWith = "X-Requested-With" - HeaderRequestedWithXML = "XMLHttpRequest" - HeaderHasInaccessiblePosts = "Has-Inaccessible-Posts" - HeaderRange = "Range" - STATUS = "status" - StatusOk = "OK" - StatusFail = "FAIL" - StatusUnhealthy = "UNHEALTHY" - StatusRemove = "REMOVE" + HeaderRequestId = "X-Request-ID" + HeaderVersionId = "X-Version-ID" + HeaderClusterId = "X-Cluster-ID" + HeaderEtagServer = "ETag" + HeaderEtagClient = "If-None-Match" + HeaderForwarded = "X-Forwarded-For" + HeaderRealIP = "X-Real-IP" + HeaderForwardedProto = "X-Forwarded-Proto" + HeaderToken = "token" + HeaderCsrfToken = "X-CSRF-Token" + HeaderBearer = "BEARER" + HeaderAuth = "Authorization" + HeaderCloudToken = "X-Cloud-Token" + HeaderRemoteclusterToken = "X-RemoteCluster-Token" + HeaderRemoteclusterId = "X-RemoteCluster-Id" + HeaderRequestedWith = "X-Requested-With" + HeaderRequestedWithXML = "XMLHttpRequest" + HeaderFirstInaccessiblePostTime = "First-Inaccessible-Post-Time" + HeaderRange = "Range" + STATUS = "status" + StatusOk = "OK" + StatusFail = "FAIL" + StatusUnhealthy = "UNHEALTHY" + StatusRemove = "REMOVE" ClientDir = "client" diff --git a/model/post_list.go b/model/post_list.go index 34fc031e3f..093ead6d50 100644 --- a/model/post_list.go +++ b/model/post_list.go @@ -16,8 +16,8 @@ type PostList struct { PrevPostId string `json:"prev_post_id"` // HasNext indicates whether there are more items to be fetched or not. HasNext bool `json:"has_next"` - // HasInaccessiblePosts tells if there are inaccessible posts, past the cloud limit. - HasInaccessiblePosts bool `json:"has_inaccessible_posts"` + // If there are inaccessible posts, FirstInaccessiblePostTime is the time of the latest inaccessible post + FirstInaccessiblePostTime int64 `json:"first_inaccessible_post_time"` } func NewPostList() *PostList { @@ -37,12 +37,12 @@ func (o *PostList) Clone() *PostList { postsCopy[k] = v.Clone() } return &PostList{ - Order: orderCopy, - Posts: postsCopy, - NextPostId: o.NextPostId, - PrevPostId: o.PrevPostId, - HasNext: o.HasNext, - HasInaccessiblePosts: o.HasInaccessiblePosts, + Order: orderCopy, + Posts: postsCopy, + NextPostId: o.NextPostId, + PrevPostId: o.PrevPostId, + HasNext: o.HasNext, + FirstInaccessiblePostTime: o.FirstInaccessiblePostTime, } } From 7441a26b6d6a67c62797c3a0ebdda86327a51591 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Tue, 26 Jul 2022 12:17:23 +0530 Subject: [PATCH 13/28] MM-45871: Do not try to extract content from images (#20698) This creates faulty requests to Bifrost and results in errors and warnings in the logs. Even without Bifrost, this would make unnecessary requests to S3. We only extract info from documents and therefore we can safely avoid this. ```release-note NONE ``` --- app/file.go | 5 +++++ app/file_test.go | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/app/file.go b/app/file.go index 9d7a382563..12c954de33 100644 --- a/app/file.go +++ b/app/file.go @@ -1327,6 +1327,11 @@ func (a *App) SearchFilesInTeamForUser(c *request.Context, terms string, userId } func (a *App) ExtractContentFromFileInfo(fileInfo *model.FileInfo) error { + // We don't process images. + if fileInfo.IsImage() { + return nil + } + file, aerr := a.FileReader(fileInfo.Path) if aerr != nil { return errors.Wrap(aerr, "failed to open file for extract file content") diff --git a/app/file_test.go b/app/file_test.go index d234e5eb83..6f84894b8f 100644 --- a/app/file_test.go +++ b/app/file_test.go @@ -543,3 +543,13 @@ func TestSearchFilesInTeamForUser(t *testing.T) { es.AssertExpectations(t) }) } + +func TestExtractContentFromFileInfo(t *testing.T) { + app := &App{} + fi := &model.FileInfo{ + MimeType: "image/jpeg", + } + + // Test that we don't process images. + require.NoError(t, app.ExtractContentFromFileInfo(fi)) +} From 4870e20ef16a6eb4dcdbc07eb5bb0fc0ab898bcd Mon Sep 17 00:00:00 2001 From: Kaya Zeren Date: Mon, 25 Jul 2022 10:55:41 +0200 Subject: [PATCH 14/28] Translated using Weblate (Turkish) Currently translated at 100.0% (2339 of 2339 strings) Translation: mattermost-languages-shipped/mattermost-server Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/tr/ --- i18n/tr.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/i18n/tr.json b/i18n/tr.json index e8b1828585..df86bd936e 100644 --- a/i18n/tr.json +++ b/i18n/tr.json @@ -9365,5 +9365,9 @@ { "id": "app.upload.upload_data.gen_info.app_error", "translation": "Yüklenen verilerden dosya bilgileri oluşturulamadı." + }, + { + "id": "model.config.is_valid.amazons3_timeout.app_error", + "translation": "{{.Value}} zaman aşımı değeri geçersiz. Bir pozitif tamsayı olmalıdır." } ] From cfa7a2bc463aeb2609f0ebe2ecbfc6d854f88bdf Mon Sep 17 00:00:00 2001 From: jprusch Date: Mon, 25 Jul 2022 10:55:42 +0200 Subject: [PATCH 15/28] Translated using Weblate (German) Currently translated at 100.0% (2339 of 2339 strings) Translation: mattermost-languages-shipped/mattermost-server Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/de/ --- i18n/de.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/i18n/de.json b/i18n/de.json index d4d25aa8d4..36ca2630f0 100644 --- a/i18n/de.json +++ b/i18n/de.json @@ -9365,5 +9365,9 @@ { "id": "app.upload.upload_data.gen_info.app_error", "translation": "Fehlschlag bei Erstellen von Dateiinformationen aus hochgeladenen Daten." + }, + { + "id": "model.config.is_valid.amazons3_timeout.app_error", + "translation": "Ungültiger Timeoutwert {{.Value}}. Muss eine positive Zahl sein." } ] From 25ff5c17a8274bfb4e7db22fd91de96aaa7826d7 Mon Sep 17 00:00:00 2001 From: "yeongeun.seo" Date: Mon, 25 Jul 2022 10:55:42 +0200 Subject: [PATCH 16/28] Translated using Weblate (Korean) Currently translated at 84.5% (1978 of 2339 strings) Translation: mattermost-languages-shipped/mattermost-server Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/ko/ Translated using Weblate (Korean) Currently translated at 83.9% (1963 of 2339 strings) Translation: mattermost-languages-shipped/mattermost-server Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/ko/ --- i18n/ko.json | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/i18n/ko.json b/i18n/ko.json index 6d956bfd94..eb2eb7fce5 100644 --- a/i18n/ko.json +++ b/i18n/ko.json @@ -7886,5 +7886,69 @@ { "id": "bleveengine.delete_post_files.error", "translation": "게시 된 파일을 삭제하지 못했습니다." + }, + { + "id": "Boards", + "translation": "게시판" + }, + { + "id": "api.elasticsearch.test_elasticsearch_settings_nil.app_error", + "translation": "Elasticsearch 설정에 설정되지 않은 값이 있습니다." + }, + { + "id": "api.custom_status.set_custom_statuses.update.app_error", + "translation": "커스텀 상태를 갱신하지 못했습니다. 이모티콘이나 사용자 지정 텍스트 상태 또는 둘 다 추가하십시오." + }, + { + "id": "api.custom_groups.no_remote_id", + "translation": "커스텀 그룹의 경우 remote_id는 공백이어야 합니다" + }, + { + "id": "api.custom_groups.must_be_referenceable", + "translation": "커스텀 그룹의 경우 allow_reference는 'true'이어야 합니다" + }, + { + "id": "api.custom_groups.license_error", + "translation": "커스텀 그룹을 위한 라이센스가 없습니다" + }, + { + "id": "api.cloud.upgrade_plan_bot_message", + "translation": "{{.TeamName}} 멤버로부터 이 작업 영역을 업그레이드하라는 통지가 있었습니다." + }, + { + "id": "api.cloud.subscription.update_error", + "translation": "웹 훅에서 구독을 업데이트하는 동안 오류가 발생했습니다." + }, + { + "id": "api.cloud.notify_admin_to_upgrade_error.already_notified", + "translation": "관리자에게 통지됨" + }, + { + "id": "api.authorization_error.guest", + "translation": " " + }, + { + "id": "Playbooks", + "translation": "플레이북" + }, + { + "id": "Channels", + "translation": "채널" + }, + { + "id": "api.custom_groups.feature_disabled", + "translation": "커스텀 그룹 기능은 비활성화되어 있습니다" + }, + { + "id": "api.custom_groups.count_err", + "translation": "그룹 카운트 중 오류가 발생했습니다" + }, + { + "id": "api.cloud.teams_limit_reached.restore", + "translation": "팀 제한에 도달했기 때문에 팀을 복원할 수 없습니다" + }, + { + "id": "api.cloud.teams_limit_reached.create", + "translation": "팀 제한에 도달했기 때문에 팀을 만들 수 없습니다" } ] From e1aa25e5fc54ee627be5c12ce5a56c7a116a1a42 Mon Sep 17 00:00:00 2001 From: Weblate Date: Mon, 25 Jul 2022 10:55:42 +0200 Subject: [PATCH 17/28] Added translation using Weblate (Vietnamese) --- i18n/vi.json | 1 + 1 file changed, 1 insertion(+) create mode 100644 i18n/vi.json diff --git a/i18n/vi.json b/i18n/vi.json new file mode 100644 index 0000000000..0967ef424b --- /dev/null +++ b/i18n/vi.json @@ -0,0 +1 @@ +{} From adcf6a1a765b17bb57ac7a0af854d35651eb1854 Mon Sep 17 00:00:00 2001 From: Tom De Moor Date: Mon, 25 Jul 2022 10:55:43 +0200 Subject: [PATCH 18/28] Deleted translation using Weblate (Vietnamese) --- i18n/vi.json | 1 - 1 file changed, 1 deletion(-) delete mode 100644 i18n/vi.json diff --git a/i18n/vi.json b/i18n/vi.json deleted file mode 100644 index 0967ef424b..0000000000 --- a/i18n/vi.json +++ /dev/null @@ -1 +0,0 @@ -{} From 2e027cafbb9ddad61485c49af379ec7d51adbbb7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?T=C3=B3th=20Csaba=20//=20Online=20ERP=20Hungary=20Kft?= Date: Mon, 25 Jul 2022 10:55:43 +0200 Subject: [PATCH 19/28] Translated using Weblate (Hungarian) Currently translated at 99.5% (2328 of 2339 strings) Translation: mattermost-languages-shipped/mattermost-server Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/hu/ --- i18n/hu.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/i18n/hu.json b/i18n/hu.json index d1eac31064..c353efa8af 100644 --- a/i18n/hu.json +++ b/i18n/hu.json @@ -9318,5 +9318,9 @@ { "id": "app.upload.upload_data.gen_info.app_error", "translation": "Nem sikerült legenerálni a fájl információt a feltöltött adatból." + }, + { + "id": "model.config.is_valid.amazons3_timeout.app_error", + "translation": "Érvénytelen az időtúllépés értéke {{.Value}}. Pozitív számnak kell lennie." } ] From 53c9e051ca835a1405cf19a506a5648a1acb9ae1 Mon Sep 17 00:00:00 2001 From: Matthew Williams Date: Mon, 25 Jul 2022 10:55:43 +0200 Subject: [PATCH 20/28] Translated using Weblate (English (Australia)) Currently translated at 99.2% (2322 of 2339 strings) Translation: mattermost-languages-shipped/mattermost-server Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/en_AU/ --- i18n/en_AU.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/i18n/en_AU.json b/i18n/en_AU.json index e45ec3aa38..303d4e1e45 100644 --- a/i18n/en_AU.json +++ b/i18n/en_AU.json @@ -9297,5 +9297,9 @@ { "id": "app.upload.upload_data.gen_info.app_error", "translation": "Failed to generate file info from uploaded data." + }, + { + "id": "model.config.is_valid.amazons3_timeout.app_error", + "translation": "Invalid timeout value {{.Value}}. Should be a positive number." } ] From 0916ec1d257f2294c31536c088c26fb32f349e5e Mon Sep 17 00:00:00 2001 From: Pierre JENICOT Date: Mon, 25 Jul 2022 10:55:44 +0200 Subject: [PATCH 21/28] Translated using Weblate (French) Currently translated at 100.0% (2339 of 2339 strings) Translation: mattermost-languages-shipped/mattermost-server Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/fr/ --- i18n/fr.json | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/i18n/fr.json b/i18n/fr.json index 6120133483..8a05a6e0a8 100644 --- a/i18n/fr.json +++ b/i18n/fr.json @@ -1857,7 +1857,7 @@ }, { "id": "api.templates.post_body.button", - "translation": "Afficher le message" + "translation": "Répondre dans Mattermost" }, { "id": "api.templates.reset_body.button", @@ -4745,7 +4745,7 @@ }, { "id": "web.error.unsupported_browser.min_os_version.windows", - "translation": "Windows 7+" + "translation": "Windows 8.1+" }, { "id": "web.error.unsupported_browser.min_os_version.mac", @@ -4753,11 +4753,11 @@ }, { "id": "web.error.unsupported_browser.min_browser_version.safari", - "translation": "Version 12+" + "translation": "Version 14.1+" }, { "id": "web.error.unsupported_browser.min_browser_version.firefox", - "translation": "Version 78+" + "translation": "Version 91+" }, { "id": "web.error.unsupported_browser.min_browser_version.edge", @@ -4765,7 +4765,7 @@ }, { "id": "web.error.unsupported_browser.min_browser_version.chrome", - "translation": "Version 89+" + "translation": "Version 100+" }, { "id": "web.error.unsupported_browser.learn_more", @@ -6933,7 +6933,7 @@ }, { "id": "api.push_notifications.session.expired", - "translation": "La session a expiré : Veuillez vous connecter pour continuer à recevoir des notifications. Les sessions pour {{.siteName}} sont configurées par votre administrateur système pour expirer tous les {{.daysCount}} jours." + "translation": "La session a expiré : Veuillez vous connecter pour continuer à recevoir des notifications. Les sessions pour {{.siteName}} sont configurées par votre administrateur système pour expirer toutes les {{.hoursCount}} heures." }, { "id": "api.preference.update_preferences.update_sidebar.app_error", @@ -7157,7 +7157,7 @@ }, { "id": "api.templates.cloud_welcome_email.title", - "translation": "Votre essai de 14 jours de l'espace de travail {{.WorkSpace}} est prêt à démarrer !" + "translation": "Votre espace de travail est prêt à fonctionner !" }, { "id": "api.templates.cloud_welcome_email.subtitle_info", @@ -7921,7 +7921,7 @@ }, { "id": "api.templates.questions_footer.info", - "translation": "Envoyez-nous un e-mail à tout moment à " + "translation": "Vous avez besoin d'aide ou vous avez des questions ? Envoyez-nous un courriel à " }, { "id": "api.templates.payment_failed_no_card.info3", @@ -9365,5 +9365,9 @@ { "id": "ent.ldap_id_migrate.app_error", "translation": " " + }, + { + "id": "model.config.is_valid.amazons3_timeout.app_error", + "translation": "Valeur de délai d'attente non valide {{.Value}}. Doit être un nombre positif." } ] From 4453e365ec6598e8bb422998300ace95a540da41 Mon Sep 17 00:00:00 2001 From: kaakaa Date: Mon, 25 Jul 2022 10:55:44 +0200 Subject: [PATCH 22/28] Translated using Weblate (Japanese) Currently translated at 100.0% (2339 of 2339 strings) Translation: mattermost-languages-shipped/mattermost-server Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/ja/ --- i18n/ja.json | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/i18n/ja.json b/i18n/ja.json index a32fe5cb4f..26c6bf178c 100644 --- a/i18n/ja.json +++ b/i18n/ja.json @@ -9358,5 +9358,9 @@ { "id": "app.post.cloud.get.app_error", "translation": "クラウドプランの制限を超えているため投稿を取得できません。" + }, + { + "id": "model.config.is_valid.amazons3_timeout.app_error", + "translation": "不正なタイムアウト値 {{.Value}} 。正の数にする必要があります。" } ] From e6459b97de7e3f9cc56626a337c86bf510795186 Mon Sep 17 00:00:00 2001 From: master7 Date: Mon, 25 Jul 2022 10:55:44 +0200 Subject: [PATCH 23/28] Translated using Weblate (Polish) Currently translated at 100.0% (2339 of 2339 strings) Translation: mattermost-languages-shipped/mattermost-server Translate-URL: https://translate.mattermost.com/projects/mattermost/mattermost-server_master/pl/ --- i18n/pl.json | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/i18n/pl.json b/i18n/pl.json index 19dc931e3b..841e0ca77c 100644 --- a/i18n/pl.json +++ b/i18n/pl.json @@ -9362,5 +9362,13 @@ { "id": "app.last_accessible_post.app_error", "translation": "Błąd pobierania ostatniego dostępnego postu" + }, + { + "id": "model.config.is_valid.amazons3_timeout.app_error", + "translation": "Nieprawidłowa wartość limitu czasu {{.Value}}. Powinna być liczbą dodatnią." + }, + { + "id": "app.upload.upload_data.gen_info.app_error", + "translation": "Nie udało się wygenerować informacji o pliku z przesłanych danych." } ] From 20cb04236254389b09a0ffffb45bb49d4462c572 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Tue, 26 Jul 2022 16:12:56 +0530 Subject: [PATCH 24/28] Hackathon: Post Reminders (#20555) This PR adds the post reminder backend work. We add a new API endpoint via which a user can set a reminder for a post. An ephemeral message will be sent down the line to let the user know about the action. And then after the time is over, the system admin bot will send a DM message to the user about the reminder post. --- api4/post.go | 32 +++++ api4/post_test.go | 60 ++++++++ app/app_iface.go | 2 + app/channels.go | 3 + app/opentracing/opentracing_layer.go | 37 +++++ app/post.go | 120 ++++++++++++++++ app/server.go | 49 +++++-- db/migrations/migrations.list | 4 + .../000091_create_post_reminder.down.sql | 16 +++ .../mysql/000091_create_post_reminder.up.sql | 21 +++ .../000091_create_post_reminder.down.sql | 3 + .../000091_create_post_reminder.up.sql | 8 ++ i18n/en.json | 8 ++ model/client4.go | 17 +++ model/post.go | 8 ++ store/layer_generators/main.go | 16 ++- store/opentracinglayer/opentracinglayer.go | 54 ++++++++ store/retrylayer/retrylayer.go | 63 +++++++++ store/sqlstore/post_store.go | 94 +++++++++++++ store/store.go | 12 ++ store/storetest/mocks/PostStore.go | 62 +++++++++ store/storetest/post_store.go | 128 ++++++++++++++++++ store/timerlayer/timerlayer.go | 48 +++++++ 23 files changed, 849 insertions(+), 16 deletions(-) create mode 100644 db/migrations/mysql/000091_create_post_reminder.down.sql create mode 100644 db/migrations/mysql/000091_create_post_reminder.up.sql create mode 100644 db/migrations/postgres/000091_create_post_reminder.down.sql create mode 100644 db/migrations/postgres/000091_create_post_reminder.up.sql diff --git a/api4/post.go b/api4/post.go index c5a8c5f0b3..024bcdc873 100644 --- a/api4/post.go +++ b/api4/post.go @@ -34,6 +34,8 @@ func (api *API) InitPost() { api.BaseRoutes.Post.Handle("", api.APISessionRequired(updatePost)).Methods("PUT") api.BaseRoutes.Post.Handle("/patch", api.APISessionRequired(patchPost)).Methods("PUT") api.BaseRoutes.PostForUser.Handle("/set_unread", api.APISessionRequired(setPostUnread)).Methods("POST") + api.BaseRoutes.PostForUser.Handle("/reminder", api.APISessionRequired(setPostReminder)).Methods("POST") + api.BaseRoutes.Post.Handle("/pin", api.APISessionRequired(pinPost)).Methods("POST") api.BaseRoutes.Post.Handle("/unpin", api.APISessionRequired(unpinPost)).Methods("POST") } @@ -842,6 +844,36 @@ func setPostUnread(c *Context, w http.ResponseWriter, r *http.Request) { } } +func setPostReminder(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequirePostId().RequireUserId() + if c.Err != nil { + return + } + + if c.AppContext.Session().UserId != c.Params.UserId && !c.App.SessionHasPermissionToUser(*c.AppContext.Session(), c.Params.UserId) { + c.SetPermissionError(model.PermissionEditOtherUsers) + return + } + if !c.App.SessionHasPermissionToChannelByPost(*c.AppContext.Session(), c.Params.PostId, model.PermissionReadChannel) { + c.SetPermissionError(model.PermissionReadChannel) + return + } + + var reminder model.PostReminder + if jsonErr := json.NewDecoder(r.Body).Decode(&reminder); jsonErr != nil { + c.SetInvalidParam("target_time") + return + } + + appErr := c.App.SetPostReminder(c.Params.PostId, c.Params.UserId, reminder.TargetTime) + if appErr != nil { + c.Err = appErr + return + } + + ReturnStatusOK(w) +} + func saveIsPinnedPost(c *Context, w http.ResponseWriter, isPinned bool) { c.RequirePostId() if c.Err != nil { diff --git a/api4/post_test.go b/api4/post_test.go index 22006fb812..a7550edffc 100644 --- a/api4/post_test.go +++ b/api4/post_test.go @@ -3164,3 +3164,63 @@ func TestGetPostStripActionIntegrations(t *testing.T) { // integration must be omitted require.Nil(t, action["integration"]) } + +func TestPostReminder(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + client := th.Client + userWSClient, err := th.CreateWebSocketClient() + require.NoError(t, err) + defer userWSClient.Close() + userWSClient.Listen() + + targetTime := time.Now().UTC().Unix() + resp, err := client.SetPostReminder(&model.PostReminder{ + TargetTime: targetTime, + PostId: th.BasicPost.Id, + UserId: th.BasicUser.Id, + }) + require.NoError(t, err) + CheckOKStatus(t, resp) + + post, _, err := client.GetPost(th.BasicPost.Id, "") + require.NoError(t, err) + + user, _, err := client.GetUser(post.UserId, "") + require.NoError(t, err) + + var caught bool + func() { + for { + select { + case ev := <-userWSClient.EventChannel: + if ev.EventType() == model.WebsocketEventEphemeralMessage { + caught = true + data := ev.GetData() + + post, ok := data["post"].(string) + require.True(t, ok) + + var parsedPost model.Post + err := json.Unmarshal([]byte(post), &parsedPost) + require.NoError(t, err) + + assert.Equal(t, model.PostTypeEphemeral, parsedPost.Type) + assert.Equal(t, th.BasicUser.Id, parsedPost.UserId) + assert.Equal(t, th.BasicPost.Id, parsedPost.RootId) + + require.Equal(t, float64(targetTime), parsedPost.GetProp("target_time").(float64)) + require.Equal(t, th.BasicPost.Id, parsedPost.GetProp("post_id").(string)) + require.Equal(t, user.Username, parsedPost.GetProp("username").(string)) + require.Equal(t, th.BasicTeam.Name, parsedPost.GetProp("team_name").(string)) + return + } + case <-time.After(1 * time.Second): + return + } + } + }() + + require.Truef(t, caught, "User should have received %s event", model.WebsocketEventEphemeralMessage) +} diff --git a/app/app_iface.go b/app/app_iface.go index fcce9b66e3..49f298ab40 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -446,6 +446,7 @@ type AppIface interface { CheckIntegrity() <-chan model.IntegrityCheckResult CheckMandatoryS3Fields(settings *model.FileSettings) *model.AppError CheckPasswordAndAllCriteria(user *model.User, password string, mfaToken string) *model.AppError + CheckPostReminders() CheckRolesExist(roleNames []string) *model.AppError CheckUserAllAuthenticationCriteria(user *model.User, mfaToken string) *model.AppError CheckUserMfa(user *model.User, token string) *model.AppError @@ -1041,6 +1042,7 @@ type AppIface interface { SetPluginKey(pluginID string, key string, value []byte) *model.AppError SetPluginKeyWithExpiry(pluginID string, key string, value []byte, expireInSeconds int64) *model.AppError SetPluginKeyWithOptions(pluginID string, key string, value []byte, options model.PluginKVSetOptions) (bool, *model.AppError) + SetPostReminder(postID, userID string, targetTime int64) *model.AppError SetProfileImage(userID string, imageData *multipart.FileHeader) *model.AppError SetProfileImageFromFile(userID string, file io.Reader) *model.AppError SetProfileImageFromMultiPartFile(userID string, file multipart.File) *model.AppError diff --git a/app/channels.go b/app/channels.go index 24a2f460d1..48aba3297c 100644 --- a/app/channels.go +++ b/app/channels.go @@ -86,6 +86,9 @@ type Channels struct { dndTaskMut sync.Mutex dndTask *model.ScheduledTask + + postReminderMut sync.Mutex + postReminderTask *model.ScheduledTask } func init() { diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index 28a25fdcd6..c8eaca45fe 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -1310,6 +1310,21 @@ func (a *OpenTracingAppLayer) CheckPasswordAndAllCriteria(user *model.User, pass return resultVar0 } +func (a *OpenTracingAppLayer) CheckPostReminders() { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.CheckPostReminders") + + a.ctx = newCtx + a.app.Srv().Store.SetContext(newCtx) + defer func() { + a.app.Srv().Store.SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + a.app.CheckPostReminders() +} + func (a *OpenTracingAppLayer) CheckProviderAttributes(user *model.User, patch *model.UserPatch) string { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.CheckProviderAttributes") @@ -15592,6 +15607,28 @@ func (a *OpenTracingAppLayer) SetPluginKeyWithOptions(pluginID string, key strin return resultVar0, resultVar1 } +func (a *OpenTracingAppLayer) SetPostReminder(postID string, userID string, targetTime int64) *model.AppError { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.SetPostReminder") + + 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.SetPostReminder(postID, userID, targetTime) + + if resultVar0 != nil { + span.LogFields(spanlog.Error(resultVar0)) + ext.Error.Set(span, true) + } + + return resultVar0 +} + func (a *OpenTracingAppLayer) SetProfileImage(userID string, imageData *multipart.FileHeader) *model.AppError { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.SetProfileImage") diff --git a/app/post.go b/app/post.go index adfa7302ec..797f1b6861 100644 --- a/app/post.go +++ b/app/post.go @@ -1933,6 +1933,126 @@ func (a *App) GetTopThreadsForUserSince(c request.CTX, teamID, userID string, op return topThreadsWithEmbedAndImage, nil } +func (a *App) SetPostReminder(postID, userID string, targetTime int64) *model.AppError { + // Store the reminder in the DB + reminder := &model.PostReminder{ + PostId: postID, + UserId: userID, + TargetTime: targetTime, + } + err := a.Srv().Store.Post().SetPostReminder(reminder) + if err != nil { + return model.NewAppError("SetPostReminder", "app.post_reminder.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + metadata, err := a.Srv().Store.Post().GetPostReminderMetadata(postID) + if err != nil { + return model.NewAppError("SetPostReminder", "app.post_reminder.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + parsed := time.Unix(targetTime, 0).UTC().Format(time.RFC822) + siteURL := *a.Config().ServiceSettings.SiteURL + // Send an ack message. + ephemeralPost := &model.Post{ + Type: model.PostTypeEphemeral, + Id: model.NewId(), + CreateAt: model.GetMillis(), + UserId: userID, + RootId: postID, + ChannelId: metadata.ChannelId, + // It's okay to keep this non-translated. This is just a fallback. + // The webapp will parse the timestamp and show that in user's local timezone. + Message: fmt.Sprintf("You will be reminded about %s/%s/pl/%s by @%s at %s", siteURL, metadata.TeamName, postID, metadata.Username, parsed), + Props: model.StringInterface{ + "target_time": targetTime, + "team_name": metadata.TeamName, + "post_id": postID, + "username": metadata.Username, + "type": model.PostTypeReminder, + }, + } + + message := model.NewWebSocketEvent(model.WebsocketEventEphemeralMessage, "", ephemeralPost.ChannelId, userID, nil) + ephemeralPost = a.PreparePostForClientWithEmbedsAndImages(request.EmptyContext(a.Log()), ephemeralPost, true, false) + ephemeralPost = model.AddPostActionCookies(ephemeralPost, a.PostActionCookieSecret()) + + postJSON, jsonErr := ephemeralPost.ToJSON() + if jsonErr != nil { + mlog.Warn("Failed to encode post to JSON", mlog.Err(jsonErr)) + } + message.Add("post", postJSON) + a.Publish(message) + + return nil +} + +func (a *App) CheckPostReminders() { + systemBot, appErr := a.GetSystemBot() + if appErr != nil { + mlog.Error("Failed to get system bot", mlog.Err(appErr)) + return + } + + // This will return the reminders and also delete them from the DB. + // In case, any of the next steps fail, those reminders would be lost. + // Alternatively, if we delete those reminders _after_ it has been sent, + // then in case of any temporary failure, they would get sent in the next batch. + // MM-45595. + reminders, err := a.Srv().Store.Post().GetPostReminders(time.Now().UTC().Unix()) + if err != nil { + mlog.Error("Failed to get post reminders", mlog.Err(err)) + return + } + + // We group multiple reminders for a single user. + groupedReminders := make(map[string][]string) + for _, r := range reminders { + if groupedReminders[r.UserId] == nil { + groupedReminders[r.UserId] = []string{r.PostId} + } else { + groupedReminders[r.UserId] = append(groupedReminders[r.UserId], r.PostId) + } + } + + siteURL := *a.Config().ServiceSettings.SiteURL + for userID, postIDs := range groupedReminders { + ch, appErr := a.GetOrCreateDirectChannel(request.EmptyContext(a.Log()), userID, systemBot.UserId) + if appErr != nil { + mlog.Error("Failed to get direct channel", mlog.Err(appErr)) + return + } + + for _, postID := range postIDs { + metadata, err := a.Srv().Store.Post().GetPostReminderMetadata(postID) + if err != nil { + mlog.Error("Failed to get post reminder metadata", mlog.Err(err)) + continue + } + + T := i18n.GetUserTranslations(metadata.UserLocale) + dm := &model.Post{ + ChannelId: ch.Id, + Message: T("app.post_reminder_dm", model.StringInterface{ + "SiteURL": siteURL, + "TeamName": metadata.TeamName, + "PostId": postID, + "Username": metadata.Username, + }), + Type: model.PostTypeDefault, + UserId: systemBot.UserId, + Props: model.StringInterface{ + "username": systemBot.Username, + }, + } + + if _, err := a.CreatePost(request.EmptyContext(a.Log()), dm, ch, false, true); err != nil { + mlog.Error("Failed to post reminder message", mlog.Err(err)) + } + } + } + +} + func includeEmbedsAndImages(a *App, c request.CTX, topThreadList *model.TopThreadList, userID string) (*model.TopThreadList, error) { for _, topThread := range topThreadList.Items { topThread.Post = a.PreparePostForClientWithEmbedsAndImages(c, topThread.Post, false, false) diff --git a/app/server.go b/app/server.go index 3066ecc858..fddb9ef1c5 100644 --- a/app/server.go +++ b/app/server.go @@ -681,6 +681,7 @@ func NewServer(options ...Option) (*Server, error) { s.runLicenseExpirationCheckJob() s.runInactivityCheckJob() runDNDStatusExpireJob(appInstance) + runPostReminderJob(appInstance) }) s.runJobs() } @@ -2066,31 +2067,53 @@ func (s *Server) ReadFile(path string) ([]byte, *model.AppError) { return result, nil } -func createDNDStatusExpirationRecurringTask(a *App) { - a.ch.dndTaskMut.Lock() - a.ch.dndTask = model.CreateRecurringTaskFromNextIntervalTime("Unset DND Statuses", a.UpdateDNDStatusOfUsers, 5*time.Minute) - a.ch.dndTaskMut.Unlock() +func withMut(mut *sync.Mutex, f func()) { + mut.Lock() + defer mut.Unlock() + f() } -func cancelDNDStatusExpirationRecurringTask(a *App) { - a.ch.dndTaskMut.Lock() - if a.ch.dndTask != nil { - a.ch.dndTask.Cancel() - a.ch.dndTask = nil +func cancelTask(mut *sync.Mutex, task *model.ScheduledTask) { + mut.Lock() + defer mut.Unlock() + if task != nil { + task.Cancel() + task = nil } - a.ch.dndTaskMut.Unlock() } func runDNDStatusExpireJob(a *App) { if a.IsLeader() { - createDNDStatusExpirationRecurringTask(a) + withMut(&a.ch.dndTaskMut, func() { + a.ch.dndTask = model.CreateRecurringTaskFromNextIntervalTime("Unset DND Statuses", a.UpdateDNDStatusOfUsers, 5*time.Minute) + }) } a.ch.srv.AddClusterLeaderChangedListener(func() { mlog.Info("Cluster leader changed. Determining if unset DNS status task should be running", mlog.Bool("isLeader", a.IsLeader())) if a.IsLeader() { - createDNDStatusExpirationRecurringTask(a) + withMut(&a.ch.dndTaskMut, func() { + a.ch.dndTask = model.CreateRecurringTaskFromNextIntervalTime("Unset DND Statuses", a.UpdateDNDStatusOfUsers, 5*time.Minute) + }) } else { - cancelDNDStatusExpirationRecurringTask(a) + cancelTask(&a.ch.dndTaskMut, a.ch.dndTask) + } + }) +} + +func runPostReminderJob(a *App) { + if a.IsLeader() { + withMut(&a.ch.postReminderMut, func() { + a.ch.postReminderTask = model.CreateRecurringTaskFromNextIntervalTime("Check Post reminders", a.CheckPostReminders, 5*time.Minute) + }) + } + a.ch.srv.AddClusterLeaderChangedListener(func() { + mlog.Info("Cluster leader changed. Determining if post reminder task should be running", mlog.Bool("isLeader", a.IsLeader())) + if a.IsLeader() { + withMut(&a.ch.postReminderMut, func() { + a.ch.postReminderTask = model.CreateRecurringTaskFromNextIntervalTime("Check Post reminders", a.CheckPostReminders, 5*time.Minute) + }) + } else { + cancelTask(&a.ch.postReminderMut, a.ch.postReminderTask) } }) } diff --git a/db/migrations/migrations.list b/db/migrations/migrations.list index 6f8dd5ac80..1caa7a23a7 100644 --- a/db/migrations/migrations.list +++ b/db/migrations/migrations.list @@ -180,6 +180,8 @@ db/migrations/mysql/000089_add-channelid-to-reaction.down.sql db/migrations/mysql/000089_add-channelid-to-reaction.up.sql db/migrations/mysql/000090_create_enums.down.sql db/migrations/mysql/000090_create_enums.up.sql +db/migrations/mysql/000091_create_post_reminder.down.sql +db/migrations/mysql/000091_create_post_reminder.up.sql db/migrations/postgres/000001_create_teams.down.sql db/migrations/postgres/000001_create_teams.up.sql db/migrations/postgres/000002_create_team_members.down.sql @@ -360,3 +362,5 @@ db/migrations/postgres/000089_add-channelid-to-reaction.down.sql db/migrations/postgres/000089_add-channelid-to-reaction.up.sql db/migrations/postgres/000090_create_enums.down.sql db/migrations/postgres/000090_create_enums.up.sql +db/migrations/postgres/000091_create_post_reminder.down.sql +db/migrations/postgres/000091_create_post_reminder.up.sql diff --git a/db/migrations/mysql/000091_create_post_reminder.down.sql b/db/migrations/mysql/000091_create_post_reminder.down.sql new file mode 100644 index 0000000000..40aeec0bf9 --- /dev/null +++ b/db/migrations/mysql/000091_create_post_reminder.down.sql @@ -0,0 +1,16 @@ +SET @preparedStatement = (SELECT IF( + ( + SELECT COUNT(*) FROM INFORMATION_SCHEMA.STATISTICS + WHERE table_name = 'PostReminders' + AND table_schema = DATABASE() + AND index_name = 'idx_postreminders_targettime' + ) > 0, + 'DROP INDEX idx_postreminders_targettime ON PostReminders;', + 'SELECT 1' +)); + +PREPARE removeIndexIfExists FROM @preparedStatement; +EXECUTE removeIndexIfExists; +DEALLOCATE PREPARE removeIndexIfExists; + +DROP TABLE IF EXISTS PostReminders; \ No newline at end of file diff --git a/db/migrations/mysql/000091_create_post_reminder.up.sql b/db/migrations/mysql/000091_create_post_reminder.up.sql new file mode 100644 index 0000000000..7cea8025b0 --- /dev/null +++ b/db/migrations/mysql/000091_create_post_reminder.up.sql @@ -0,0 +1,21 @@ +CREATE TABLE IF NOT EXISTS PostReminders ( + PostId varchar(26) NOT NULL, + UserId varchar(26) NOT NULL, + TargetTime bigint, + PRIMARY KEY (PostId, UserId) +); + +SET @preparedStatement = (SELECT IF( + ( + SELECT COUNT(*) FROM INFORMATION_SCHEMA.STATISTICS + WHERE table_name = 'PostReminders' + AND table_schema = DATABASE() + AND index_name = 'idx_postreminders_targettime' + ) > 0, + 'SELECT 1', + 'CREATE INDEX idx_postreminders_targettime ON PostReminders(TargetTime);' +)); + +PREPARE createIndexIfNotExists FROM @preparedStatement; +EXECUTE createIndexIfNotExists; +DEALLOCATE PREPARE createIndexIfNotExists; \ No newline at end of file diff --git a/db/migrations/postgres/000091_create_post_reminder.down.sql b/db/migrations/postgres/000091_create_post_reminder.down.sql new file mode 100644 index 0000000000..268d3908e5 --- /dev/null +++ b/db/migrations/postgres/000091_create_post_reminder.down.sql @@ -0,0 +1,3 @@ +DROP INDEX IF EXISTS idx_postreminders_targettime; + +DROP TABLE IF EXISTS postreminders; \ No newline at end of file diff --git a/db/migrations/postgres/000091_create_post_reminder.up.sql b/db/migrations/postgres/000091_create_post_reminder.up.sql new file mode 100644 index 0000000000..cdc87f66de --- /dev/null +++ b/db/migrations/postgres/000091_create_post_reminder.up.sql @@ -0,0 +1,8 @@ +CREATE TABLE IF NOT EXISTS postreminders ( + postid varchar(26) NOT NULL, + userid varchar(26) NOT NULL, + targettime bigint, + PRIMARY KEY (postid, userid) +); + +CREATE INDEX IF NOT EXISTS idx_postreminders_targettime ON postreminders(targettime); \ No newline at end of file diff --git a/i18n/en.json b/i18n/en.json index a86209588a..5ad9f74c1d 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -5911,6 +5911,14 @@ "id": "app.post.update.app_error", "translation": "Unable to update the Post." }, + { + "id": "app.post_reminder.app_error", + "translation": " " + }, + { + "id": "app.post_reminder_dm", + "translation": "Hi there, you asked me to remind you about {{.SiteURL}}/{{.TeamName}}/pl/{{.PostId}} by @{{.Username}}" + }, { "id": "app.preference.delete.app_error", "translation": "We encountered an error while deleting preferences." diff --git a/model/client4.go b/model/client4.go index 1dc5f0aa8d..98cd3c739e 100644 --- a/model/client4.go +++ b/model/client4.go @@ -3743,6 +3743,23 @@ func (c *Client4) SetPostUnread(userId string, postId string, collapsedThreadsSu return BuildResponse(r), nil } +// SetPostReminder creates a post reminder for a given post at a specified time. +// The time needs to be in UTC epoch in seconds. It is always truncated to a +// 5 minute resolution minimum. +func (c *Client4) SetPostReminder(reminder *PostReminder) (*Response, error) { + b, err := json.Marshal(reminder) + if err != nil { + return nil, NewAppError("SetPostReminder", "api.marshal_error", nil, err.Error(), http.StatusInternalServerError) + } + + r, err := c.DoAPIPostBytes(c.userRoute(reminder.UserId)+c.postRoute(reminder.PostId)+"/reminder", b) + if err != nil { + return BuildResponse(r), err + } + defer closeBody(r) + return BuildResponse(r), nil +} + // PinPost pin a post based on provided post id string. func (c *Client4) PinPost(postId string) (*Response, error) { r, err := c.DoAPIPost(c.postRoute(postId)+"/pin", "") diff --git a/model/post.go b/model/post.go index f8e3240154..f0ebcfbea2 100644 --- a/model/post.go +++ b/model/post.go @@ -48,6 +48,7 @@ const ( PostTypeSystemWarnMetricStatus = "warn_metric_status" PostTypeMe = "me" PostCustomTypePrefix = "custom_" + PostTypeReminder = "reminder" PostFileidsMaxRunes = 300 PostFilenamesMaxRunes = 4000 @@ -149,6 +150,13 @@ type PostPatch struct { HasReactions *bool `json:"has_reactions"` } +type PostReminder struct { + TargetTime int64 `json:"target_time"` + // These fields are only used internally for interacting with DB. + PostId string `json:",omitempty"` + UserId string `json:",omitempty"` +} + type SearchParameter struct { Terms *string `json:"terms"` IsOrSearch *bool `json:"is_or_search"` diff --git a/store/layer_generators/main.go b/store/layer_generators/main.go index 3f9089246e..28b015ac0c 100644 --- a/store/layer_generators/main.go +++ b/store/layer_generators/main.go @@ -221,10 +221,20 @@ func generateLayer(name, templateFile string) ([]byte, error) { if len(results) == 0 { return "" } - if len(results) == 1 { - return strings.Join(results, ", ") + returns := []string{} + for _, result := range results { + switch result { + case "*PostReminderMetadata": + returns = append(returns, fmt.Sprintf("*store.%s", strings.TrimPrefix(result, "*"))) + default: + returns = append(returns, result) + } } - return fmt.Sprintf("(%s)", strings.Join(results, ", ")) + + if len(returns) == 1 { + return strings.Join(returns, ", ") + } + return fmt.Sprintf("(%s)", strings.Join(returns, ", ")) }, "genResultsVars": func(results []string, withNilError bool) string { vars := []string{} diff --git a/store/opentracinglayer/opentracinglayer.go b/store/opentracinglayer/opentracinglayer.go index 85c4829f58..ee89078792 100644 --- a/store/opentracinglayer/opentracinglayer.go +++ b/store/opentracinglayer/opentracinglayer.go @@ -5809,6 +5809,42 @@ func (s *OpenTracingLayerPostStore) GetPostIdBeforeTime(channelID string, timest return result, err } +func (s *OpenTracingLayerPostStore) GetPostReminderMetadata(postID string) (*store.PostReminderMetadata, error) { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PostStore.GetPostReminderMetadata") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + result, err := s.PostStore.GetPostReminderMetadata(postID) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return result, err +} + +func (s *OpenTracingLayerPostStore) GetPostReminders(now int64) ([]*model.PostReminder, error) { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PostStore.GetPostReminders") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + result, err := s.PostStore.GetPostReminders(now) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return result, err +} + func (s *OpenTracingLayerPostStore) GetPosts(options model.GetPostsOptions, allowFromCache bool, sanitizeOptions map[string]bool) (*model.PostList, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PostStore.GetPosts") @@ -6236,6 +6272,24 @@ func (s *OpenTracingLayerPostStore) SearchPostsForUser(paramsList []*model.Searc return result, err } +func (s *OpenTracingLayerPostStore) SetPostReminder(reminder *model.PostReminder) error { + origCtx := s.Root.Store.Context() + span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PostStore.SetPostReminder") + s.Root.Store.SetContext(newCtx) + defer func() { + s.Root.Store.SetContext(origCtx) + }() + + defer span.Finish() + err := s.PostStore.SetPostReminder(reminder) + if err != nil { + span.LogFields(spanlog.Error(err)) + ext.Error.Set(span, true) + } + + return err +} + func (s *OpenTracingLayerPostStore) Update(newPost *model.Post, oldPost *model.Post) (*model.Post, error) { origCtx := s.Root.Store.Context() span, newCtx := tracing.StartSpanWithParentByContext(s.Root.Store.Context(), "PostStore.Update") diff --git a/store/retrylayer/retrylayer.go b/store/retrylayer/retrylayer.go index 5094c41fcb..ebd66004f5 100644 --- a/store/retrylayer/retrylayer.go +++ b/store/retrylayer/retrylayer.go @@ -6575,6 +6575,48 @@ func (s *RetryLayerPostStore) GetPostIdBeforeTime(channelID string, timestamp in } +func (s *RetryLayerPostStore) GetPostReminderMetadata(postID string) (*store.PostReminderMetadata, error) { + + tries := 0 + for { + result, err := s.PostStore.GetPostReminderMetadata(postID) + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + +func (s *RetryLayerPostStore) GetPostReminders(now int64) ([]*model.PostReminder, error) { + + tries := 0 + for { + result, err := s.PostStore.GetPostReminders(now) + if err == nil { + return result, nil + } + if !isRepeatableError(err) { + return result, err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return result, err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerPostStore) GetPosts(options model.GetPostsOptions, allowFromCache bool, sanitizeOptions map[string]bool) (*model.PostList, error) { tries := 0 @@ -7064,6 +7106,27 @@ func (s *RetryLayerPostStore) SearchPostsForUser(paramsList []*model.SearchParam } +func (s *RetryLayerPostStore) SetPostReminder(reminder *model.PostReminder) error { + + tries := 0 + for { + err := s.PostStore.SetPostReminder(reminder) + if err == nil { + return nil + } + if !isRepeatableError(err) { + return err + } + tries++ + if tries >= 3 { + err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures") + return err + } + timepkg.Sleep(100 * timepkg.Millisecond) + } + +} + func (s *RetryLayerPostStore) Update(newPost *model.Post, oldPost *model.Post) (*model.Post, error) { tries := 0 diff --git a/store/sqlstore/post_store.go b/store/sqlstore/post_store.go index af7b11ca01..03ee58888e 100644 --- a/store/sqlstore/post_store.go +++ b/store/sqlstore/post_store.go @@ -2963,3 +2963,97 @@ func (s *SqlPostStore) updateThreadsFromPosts(transaction *sqlxTxWrapper, posts } return nil } + +func (s *SqlPostStore) SetPostReminder(reminder *model.PostReminder) error { + transaction, err := s.GetMasterX().Beginx() + if err != nil { + return errors.Wrap(err, "begin_transaction") + } + defer finalizeTransactionX(transaction) + + sql := `SELECT EXISTS (SELECT 1 FROM Posts WHERE Id=?)` + var exist bool + err = transaction.Get(&exist, sql, reminder.PostId) + if err != nil { + return errors.Wrap(err, "failed to check for post") + } + if !exist { + return store.NewErrNotFound("Post", reminder.PostId) + } + + query := s.getQueryBuilder(). + Insert("PostReminders"). + Columns("PostId", "UserId", "TargetTime"). + Values(reminder.PostId, reminder.UserId, reminder.TargetTime) + + if s.DriverName() == model.DatabaseDriverMysql { + query = query.SuffixExpr(sq.Expr("ON DUPLICATE KEY UPDATE TargetTime = ?", reminder.TargetTime)) + } else { + query = query.SuffixExpr(sq.Expr("ON CONFLICT (postid, userid) DO UPDATE SET TargetTime = ?", reminder.TargetTime)) + } + + sql, args, err := query.ToSql() + if err != nil { + return errors.Wrap(err, "setPostReminder_tosql") + } + if _, err2 := transaction.Exec(sql, args...); err2 != nil { + return errors.Wrap(err2, "failed to insert post reminder") + } + if err = transaction.Commit(); err != nil { + return errors.Wrap(err, "commit_transaction") + } + return nil +} + +func (s *SqlPostStore) GetPostReminders(now int64) ([]*model.PostReminder, error) { + reminders := []*model.PostReminder{} + + transaction, err := s.GetMasterX().Beginx() + if err != nil { + return nil, errors.Wrap(err, "begin_transaction") + } + defer finalizeTransactionX(transaction) + + err = transaction.Select(&reminders, `SELECT PostId, UserId + FROM PostReminders + WHERE TargetTime < ?`, now) + if err != nil && err != sql.ErrNoRows { + return nil, errors.Wrap(err, "failed to get post reminders") + } + + if err == sql.ErrNoRows { + // No need to execute delete statement if there's nothing to delete. + return reminders, nil + } + + // Postgres supports RETURNING * in a DELETE statement, but MySQL doesn't. + // So we are stuck with 2 queries. Not taking separate paths for Postgres + // and MySQL for simplicity. + _, err = transaction.Exec(`DELETE from PostReminders WHERE TargetTime < ?`, now) + if err != nil { + return nil, errors.Wrap(err, "failed to delete post reminders") + } + + if err = transaction.Commit(); err != nil { + return nil, errors.Wrap(err, "commit_transaction") + } + + return reminders, nil +} + +func (s *SqlPostStore) GetPostReminderMetadata(postID string) (*store.PostReminderMetadata, error) { + meta := &store.PostReminderMetadata{} + err := s.GetReplicaX().Get(meta, `SELECT c.id as ChannelId, + t.name as TeamName, + u.locale as UserLocale, u.username as Username + FROM Posts p, Channels c, Teams t, Users u + WHERE p.ChannelId=c.Id + AND c.TeamId=t.Id + AND p.UserId=u.Id + AND p.Id=?`, postID) + if err != nil { + return nil, errors.Wrap(err, "failed to get post reminder metadata") + } + + return meta, nil +} diff --git a/store/store.go b/store/store.go index 370dad74dc..911c06191f 100644 --- a/store/store.go +++ b/store/store.go @@ -386,6 +386,9 @@ type PostStore interface { GetOldestEntityCreationTime() (int64, error) HasAutoResponsePostByUserSince(options model.GetPostsSinceOptions, userId string) (bool, error) GetPostsSinceForSync(options model.GetPostsSinceForSyncOptions, cursor model.GetPostsSinceForSyncCursor, limit int) ([]*model.Post, model.GetPostsSinceForSyncCursor, error) + SetPostReminder(reminder *model.PostReminder) error + GetPostReminders(now int64) ([]*model.PostReminder, error) + GetPostReminderMetadata(postID string) (*PostReminderMetadata, error) // GetNthRecentPostTime returns the CreateAt time of the nth most recent post. GetNthRecentPostTime(n int64) (int64, error) } @@ -1019,6 +1022,15 @@ type ChannelMemberGraphQLSearchOpts struct { ExcludeTeam bool } +// PostReminderMetadata contains some info needed to send +// the reminder message to the user. +type PostReminderMetadata struct { + ChannelId string + TeamName string + UserLocale string + Username string +} + // SidebarCategorySearchOpts contains the options for a graphQL query // to get the sidebar categories. type SidebarCategorySearchOpts struct { diff --git a/store/storetest/mocks/PostStore.go b/store/storetest/mocks/PostStore.go index 0f24d48db0..42977fb9e3 100644 --- a/store/storetest/mocks/PostStore.go +++ b/store/storetest/mocks/PostStore.go @@ -9,6 +9,8 @@ import ( model "github.com/mattermost/mattermost-server/v6/model" mock "github.com/stretchr/testify/mock" + + store "github.com/mattermost/mattermost-server/v6/store" ) // PostStore is an autogenerated mock type for the PostStore type @@ -440,6 +442,52 @@ func (_m *PostStore) GetPostIdBeforeTime(channelID string, timestamp int64, coll return r0, r1 } +// GetPostReminderMetadata provides a mock function with given fields: postID +func (_m *PostStore) GetPostReminderMetadata(postID string) (*store.PostReminderMetadata, error) { + ret := _m.Called(postID) + + var r0 *store.PostReminderMetadata + if rf, ok := ret.Get(0).(func(string) *store.PostReminderMetadata); ok { + r0 = rf(postID) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*store.PostReminderMetadata) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(string) error); ok { + r1 = rf(postID) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + +// GetPostReminders provides a mock function with given fields: now +func (_m *PostStore) GetPostReminders(now int64) ([]*model.PostReminder, error) { + ret := _m.Called(now) + + var r0 []*model.PostReminder + if rf, ok := ret.Get(0).(func(int64) []*model.PostReminder); ok { + r0 = rf(now) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]*model.PostReminder) + } + } + + var r1 error + if rf, ok := ret.Get(1).(func(int64) error); ok { + r1 = rf(now) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetPosts provides a mock function with given fields: options, allowFromCache, sanitizeOptions func (_m *PostStore) GetPosts(options model.GetPostsOptions, allowFromCache bool, sanitizeOptions map[string]bool) (*model.PostList, error) { ret := _m.Called(options, allowFromCache, sanitizeOptions) @@ -969,6 +1017,20 @@ func (_m *PostStore) SearchPostsForUser(paramsList []*model.SearchParams, userID return r0, r1 } +// SetPostReminder provides a mock function with given fields: reminder +func (_m *PostStore) SetPostReminder(reminder *model.PostReminder) error { + ret := _m.Called(reminder) + + var r0 error + if rf, ok := ret.Get(0).(func(*model.PostReminder) error); ok { + r0 = rf(reminder) + } else { + r0 = ret.Error(0) + } + + return r0 +} + // Update provides a mock function with given fields: newPost, oldPost func (_m *PostStore) Update(newPost *model.Post, oldPost *model.Post) (*model.Post, error) { ret := _m.Called(newPost, oldPost) diff --git a/store/storetest/post_store.go b/store/storetest/post_store.go index 15147cbfb6..98efff5321 100644 --- a/store/storetest/post_store.go +++ b/store/storetest/post_store.go @@ -5,6 +5,7 @@ package storetest import ( "context" + "errors" "fmt" "sort" "strings" @@ -57,6 +58,9 @@ func TestPostStore(t *testing.T, ss store.Store, s SqlStore) { t.Run("GetForThread", func(t *testing.T) { testPostStoreGetForThread(t, ss) }) t.Run("HasAutoResponsePostByUserSince", func(t *testing.T) { testHasAutoResponsePostByUserSince(t, ss) }) t.Run("GetPostsSinceForSync", func(t *testing.T) { testGetPostsSinceForSync(t, ss, s) }) + t.Run("SetPostReminder", func(t *testing.T) { testSetPostReminder(t, ss, s) }) + t.Run("GetPostReminders", func(t *testing.T) { testGetPostReminders(t, ss, s) }) + t.Run("GetPostReminderMetadata", func(t *testing.T) { testGetPostReminderMetadata(t, ss, s) }) t.Run("GetNthRecentPostTime", func(t *testing.T) { testGetNthRecentPostTime(t, ss) }) } @@ -3756,6 +3760,130 @@ func testGetPostsSinceForSync(t *testing.T, ss store.Store, s SqlStore) { }) } +func testSetPostReminder(t *testing.T, ss store.Store, s SqlStore) { + // Basic + userID := NewTestId() + + p1 := &model.Post{ + UserId: userID, + ChannelId: NewTestId(), + Message: "hi there", + Type: model.PostTypeDefault, + } + p1, err := ss.Post().Save(p1) + require.NoError(t, err) + + reminder := &model.PostReminder{ + TargetTime: 1234, + PostId: p1.Id, + UserId: userID, + } + + require.NoError(t, ss.Post().SetPostReminder(reminder)) + + out := model.PostReminder{} + require.NoError(t, s.GetMasterX().Get(&out, `SELECT PostId, UserId, TargetTime FROM PostReminders WHERE PostId=? AND UserId=?`, reminder.PostId, reminder.UserId)) + assert.Equal(t, reminder, &out) + + reminder.PostId = "notfound" + err = ss.Post().SetPostReminder(reminder) + var nfErr *store.ErrNotFound + require.True(t, errors.As(err, &nfErr)) + + // Upsert + reminder = &model.PostReminder{ + TargetTime: 12345, + PostId: p1.Id, + UserId: userID, + } + + require.NoError(t, ss.Post().SetPostReminder(reminder)) + require.NoError(t, s.GetMasterX().Get(&out, `SELECT PostId, UserId, TargetTime FROM PostReminders WHERE PostId=? AND UserId=?`, reminder.PostId, reminder.UserId)) + assert.Equal(t, reminder, &out) +} + +func testGetPostReminders(t *testing.T, ss store.Store, s SqlStore) { + times := []int64{100, 101, 102} + for _, tt := range times { + userID := NewTestId() + + p1 := &model.Post{ + UserId: userID, + ChannelId: NewTestId(), + Message: "hi there", + Type: model.PostTypeDefault, + } + p1, err := ss.Post().Save(p1) + require.NoError(t, err) + + reminder := &model.PostReminder{ + TargetTime: tt, + PostId: p1.Id, + UserId: userID, + } + + require.NoError(t, ss.Post().SetPostReminder(reminder)) + } + + reminders, err := ss.Post().GetPostReminders(102) + require.NoError(t, err) + require.Len(t, reminders, 2) + + // assert one reminder is left + reminders, err = ss.Post().GetPostReminders(103) + require.NoError(t, err) + require.Len(t, reminders, 1) + + // assert everything is deleted. + reminders, err = ss.Post().GetPostReminders(103) + require.NoError(t, err) + require.Len(t, reminders, 0) +} + +func testGetPostReminderMetadata(t *testing.T, ss store.Store, s SqlStore) { + team := &model.Team{ + Name: "teamname", + DisplayName: "display", + Type: model.TeamOpen, + } + team, err := ss.Team().Save(team) + require.NoError(t, err) + + ch := &model.Channel{ + TeamId: team.Id, + DisplayName: "channeldisplay", + Name: NewTestId(), + Type: model.ChannelTypeOpen, + } + ch, err = ss.Channel().Save(ch, -1) + require.NoError(t, err) + + u1 := &model.User{ + Email: MakeEmail(), + Username: model.NewId(), + Locale: "es", + } + + u1, err = ss.User().Save(u1) + require.NoError(t, err) + + p1 := &model.Post{ + UserId: u1.Id, + ChannelId: ch.Id, + Message: "hi there", + Type: model.PostTypeDefault, + } + p1, err = ss.Post().Save(p1) + require.NoError(t, err) + + meta, err := ss.Post().GetPostReminderMetadata(p1.Id) + require.NoError(t, err) + assert.Equal(t, meta.ChannelId, ch.Id) + assert.Equal(t, meta.TeamName, team.Name) + assert.Equal(t, meta.Username, u1.Username) + assert.Equal(t, meta.UserLocale, u1.Locale) +} + func getPostIds(posts []*model.Post, morePosts ...*model.Post) []string { ids := make([]string, 0, len(posts)+len(morePosts)) for _, p := range posts { diff --git a/store/timerlayer/timerlayer.go b/store/timerlayer/timerlayer.go index eb920da051..3066b77559 100644 --- a/store/timerlayer/timerlayer.go +++ b/store/timerlayer/timerlayer.go @@ -5258,6 +5258,38 @@ func (s *TimerLayerPostStore) GetPostIdBeforeTime(channelID string, timestamp in return result, err } +func (s *TimerLayerPostStore) GetPostReminderMetadata(postID string) (*store.PostReminderMetadata, error) { + start := time.Now() + + result, err := s.PostStore.GetPostReminderMetadata(postID) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("PostStore.GetPostReminderMetadata", success, elapsed) + } + return result, err +} + +func (s *TimerLayerPostStore) GetPostReminders(now int64) ([]*model.PostReminder, error) { + start := time.Now() + + result, err := s.PostStore.GetPostReminders(now) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("PostStore.GetPostReminders", success, elapsed) + } + return result, err +} + func (s *TimerLayerPostStore) GetPosts(options model.GetPostsOptions, allowFromCache bool, sanitizeOptions map[string]bool) (*model.PostList, error) { start := time.Now() @@ -5641,6 +5673,22 @@ func (s *TimerLayerPostStore) SearchPostsForUser(paramsList []*model.SearchParam return result, err } +func (s *TimerLayerPostStore) SetPostReminder(reminder *model.PostReminder) error { + start := time.Now() + + err := s.PostStore.SetPostReminder(reminder) + + elapsed := float64(time.Since(start)) / float64(time.Second) + if s.Root.Metrics != nil { + success := "false" + if err == nil { + success = "true" + } + s.Root.Metrics.ObserveStoreMethodDuration("PostStore.SetPostReminder", success, elapsed) + } + return err +} + func (s *TimerLayerPostStore) Update(newPost *model.Post, oldPost *model.Post) (*model.Post, error) { start := time.Now() From 1044fba4491f01b84e836edf9dbbc0696b365f67 Mon Sep 17 00:00:00 2001 From: Julien Tant <785518+JulienTant@users.noreply.github.com> Date: Tue, 26 Jul 2022 07:47:09 -0700 Subject: [PATCH 25/28] [MM-45817] Set cloud cookies whenever user gets logged in (#20692) --- api4/user.go | 5 ---- api4/user_test.go | 58 +++++++++++++++++++++++++++++++++++++++++++---- app/login.go | 5 ++++ 3 files changed, 58 insertions(+), 10 deletions(-) diff --git a/api4/user.go b/api4/user.go index d65018e1f5..6eee9d8db3 100644 --- a/api4/user.go +++ b/api4/user.go @@ -1881,11 +1881,6 @@ func login(c *Context, w http.ResponseWriter, r *http.Request) { c.App.AttachSessionCookies(c.AppContext, w, r) } - // For context see: https://mattermost.atlassian.net/browse/MM-39583 - if c.App.Channels().License() != nil && *c.App.Channels().License().Features.Cloud { - c.App.AttachCloudSessionCookie(c.AppContext, w, r) - } - userTermsOfService, err := c.App.GetUserTermsOfService(user.Id) if err != nil && err.StatusCode != http.StatusNotFound { c.Err = err diff --git a/api4/user_test.go b/api4/user_test.go index 4788fe570b..658ebf4d4b 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -7,6 +7,7 @@ import ( "encoding/json" "fmt" "net/http" + "net/url" "os" "regexp" "strings" @@ -3610,14 +3611,61 @@ func TestLoginCookies(t *testing.T) { th.App.Srv().SetLicense(model.NewTestLicense("cloud")) + th.Client.HTTPHeader[model.HeaderRequestedWith] = model.HeaderRequestedWithXML _, resp, _ := th.Client.Login(th.BasicUser.Email, th.BasicUser.Password) - val := strings.Split(resp.Header["Set-Cookie"][0], ";") - cloudSessionCookie := strings.Split(val[0], "=")[1] - domain := strings.Split(val[2], "=")[1] + found := false + cookies := resp.Header.Values("Set-Cookie") + for i := range cookies { + if strings.Contains(cookies[i], "MMCLOUDURL") { + found = true + assert.Contains(t, cookies[i], "MMCLOUDURL=testchips;", "should contain MMCLOUDURL") + assert.Contains(t, cookies[i], "Domain=mattermost.com;", "should contain Domain=mattermost.com") + break + } + } + assert.True(t, found, "Did not find MMCLOUDURL cookie") + }) - assert.Equal(t, "testchips", cloudSessionCookie) - assert.Equal(t, "mattermost.com", domain) + t.Run("should return cookie with MMCLOUDURL for cloud installations when doing cws login", func(t *testing.T) { + token := model.NewRandomString(64) + os.Setenv("CWS_CLOUD_TOKEN", token) + + updateConfig := func(cfg *model.Config) { + *cfg.ServiceSettings.SiteURL = "https://testchips.cloud.mattermost.com" + } + th := SetupAndApplyConfigBeforeLogin(t, updateConfig).InitBasic() + defer th.TearDown() + + th.App.Srv().SetLicense(model.NewTestLicense("cloud")) + + form := url.Values{} + form.Add("login_id", th.SystemAdminUser.Email) + form.Add("cws_token", token) + + th.Client.HTTPClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { + return http.ErrUseLastResponse + } + + r, _ := th.Client.DoAPIRequestWithHeaders( + http.MethodPost, + th.Client.APIURL+"/users/login/cws", + form.Encode(), + map[string]string{ + "Content-Type": "application/x-www-form-urlencoded", + }, + ) + defer closeBody(r) + + cookies := r.Cookies() + found := false + for i := range cookies { + if cookies[i].Name == model.SessionCookieCloudUrl { + found = true + assert.Equal(t, "testchips", cookies[i].Value) + } + } + assert.True(t, found, "should have found cookie") }) t.Run("should NOT return cookie with MMCLOUDURL for cloud installations without expected format of cloud URL", func(t *testing.T) { diff --git a/app/login.go b/app/login.go index e67db2fe38..25d3ede871 100644 --- a/app/login.go +++ b/app/login.go @@ -331,6 +331,11 @@ func (a *App) AttachSessionCookies(c *request.Context, w http.ResponseWriter, r http.SetCookie(w, sessionCookie) http.SetCookie(w, userCookie) http.SetCookie(w, csrfCookie) + + // For context see: https://mattermost.atlassian.net/browse/MM-39583 + if a.Channels().License() != nil && *a.Channels().License().Features.Cloud { + a.AttachCloudSessionCookie(c, w, r) + } } func GetProtocol(r *http.Request) string { From c6913f7d9e636420ccc0d521ad8bdcfa375cfd44 Mon Sep 17 00:00:00 2001 From: Michael Kochell <6913320+mickmister@users.noreply.github.com> Date: Tue, 26 Jul 2022 11:31:34 -0400 Subject: [PATCH 26/28] support plugin setting property `hosting` (#20677) Co-authored-by: Mattermod --- model/manifest.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/model/manifest.go b/model/manifest.go index d017df164e..6b7ddbd84d 100644 --- a/model/manifest.go +++ b/model/manifest.go @@ -84,6 +84,11 @@ type PluginSetting struct { // For "radio" or "dropdown" settings, this is the list of pre-defined options that the user can choose // from. Options []*PluginOption `json:"options,omitempty" yaml:"options,omitempty"` + + // The intended hosting environment for this plugin setting. Can be "cloud" or "on-prem". When this field is set, + // and the opposite environment is running the plugin, the setting will be hidden in the admin console UI. + // Note that this functionality is entirely client-side, so the plugin needs to handle the case of invalid submissions. + Hosting string `json:"hosting"` } type PluginSettingsSchema struct { From c00609ab8e756d9fb525e781d6a5ed582bb69547 Mon Sep 17 00:00:00 2001 From: "Carrie Warner (Mattermost)" <74422101+cwarnermm@users.noreply.github.com> Date: Tue, 26 Jul 2022 16:17:50 -0400 Subject: [PATCH 27/28] Added schema migration template to PR template (#20717) A schema migration template is now available to help guide release note development. --- .github/PULL_REQUEST_TEMPLATE.md | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 09cd0f7cb3..548bb4a16c 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -23,16 +23,17 @@ Otherwise, link the JIRA ticket.