From 9521797b15881c6cfdcbbf6b6d170fcd40d945a6 Mon Sep 17 00:00:00 2001 From: Ashish Bhate Date: Mon, 4 Apr 2022 17:50:13 +0530 Subject: [PATCH] MM-40148: threadsOnly query param for user threads (#19833) * MM-40148: threadsOnly query param for user threads Currently we always calculate counts when fetching user threads. Those counts include total, unread replies, and unread mentions, and are potentially expensive to calculate. This commit adds a new query param 'threadsOnly' which won't calculate any counts and just return threads. Co-authored-by: koox00 <3829551+koox00@users.noreply.github.com> Co-authored-by: Mattermod --- api4/user.go | 30 ++++++++++------ api4/user_test.go | 90 +++++++++++++++++++++++++++++++++++++++++++++++ app/user.go | 50 +++++++++++++------------- i18n/en.json | 4 +++ model/client4.go | 6 ++++ model/thread.go | 3 ++ 6 files changed, 149 insertions(+), 34 deletions(-) diff --git a/api4/user.go b/api4/user.go index 81baaabe2a..a2bb38d9d1 100644 --- a/api4/user.go +++ b/api4/user.go @@ -3008,14 +3008,15 @@ func getThreadsForUser(c *Context, w http.ResponseWriter, r *http.Request) { } options := model.GetUserThreadsOpts{ - Since: 0, - Before: "", - After: "", - PageSize: uint64(c.Params.PerPage), - Unread: false, - Extended: false, - Deleted: false, - TotalsOnly: false, + Since: 0, + Before: "", + After: "", + PageSize: uint64(c.Params.PerPage), + Unread: false, + Extended: false, + Deleted: false, + TotalsOnly: false, + ThreadsOnly: false, } sinceString := r.URL.Query().Get("since") @@ -3030,21 +3031,30 @@ func getThreadsForUser(c *Context, w http.ResponseWriter, r *http.Request) { options.Before = r.URL.Query().Get("before") options.After = r.URL.Query().Get("after") + totalsOnlyStr := r.URL.Query().Get("totalsOnly") + threadsOnlyStr := r.URL.Query().Get("threadsOnly") + options.TotalsOnly, _ = strconv.ParseBool(totalsOnlyStr) + options.ThreadsOnly, _ = strconv.ParseBool(threadsOnlyStr) + // parameters are mutually exclusive if options.Before != "" && options.After != "" { c.Err = model.NewAppError("api.getThreadsForUser", "api.getThreadsForUser.bad_params", nil, "", http.StatusBadRequest) return } + // parameters are mutually exclusive + if options.TotalsOnly && options.ThreadsOnly { + c.Err = model.NewAppError("api.getThreadsForUser", "api.getThreadsForUser.bad_only_params", nil, "", http.StatusBadRequest) + return + } + deletedStr := r.URL.Query().Get("deleted") unreadStr := r.URL.Query().Get("unread") extendedStr := r.URL.Query().Get("extended") - totalsOnlyStr := r.URL.Query().Get("totalsOnly") options.Deleted, _ = strconv.ParseBool(deletedStr) options.Unread, _ = strconv.ParseBool(unreadStr) options.Extended, _ = strconv.ParseBool(extendedStr) - options.TotalsOnly, _ = strconv.ParseBool(totalsOnlyStr) threads, err := c.App.GetThreadsForUser(c.Params.UserId, c.Params.TeamId, options) if err != nil { diff --git a/api4/user_test.go b/api4/user_test.go index d98b3f0ccb..e8485217ae 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -5754,6 +5754,96 @@ func TestGetThreadsForUser(t *testing.T) { require.Len(t, uss3.Threads, 0) }) + t.Run("totalsOnly param", func(t *testing.T) { + client := th.Client + sysadminClient := th.SystemAdminClient + + var rootIds []*model.Post + for i := 0; i < 10; i++ { + rpost, resp, err := client.CreatePost(&model.Post{ChannelId: th.BasicChannel.Id, Message: "testMsg"}) + require.NoError(t, err) + CheckCreatedStatus(t, resp) + rootIds = append(rootIds, rpost) + if i%2 == 0 { + _, resp, err = client.CreatePost(&model.Post{ChannelId: th.BasicChannel.Id, Message: "testReply", RootId: rpost.Id}) + } else { + _, resp, err = sysadminClient.CreatePost(&model.Post{ChannelId: th.BasicChannel.Id, Message: "testReply @" + th.BasicUser.Username, RootId: rpost.Id}) + } + require.NoError(t, err) + CheckCreatedStatus(t, resp) + } + + defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.BasicUser.Id) + defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.SystemAdminUser.Id) + + uss, _, err := th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{ + Deleted: false, + TotalsOnly: true, + PageSize: 30, + }) + require.NoError(t, err) + require.Len(t, uss.Threads, 0) + require.Len(t, rootIds, 10) + require.Equal(t, int64(10), uss.Total) + require.Equal(t, int64(5), uss.TotalUnreadThreads) + require.Equal(t, int64(5), uss.TotalUnreadMentions) + }) + + t.Run("threadsOnly param", func(t *testing.T) { + client := th.Client + sysadminClient := th.SystemAdminClient + + var rootIds []*model.Post + for i := 0; i < 10; i++ { + rpost, resp, err := client.CreatePost(&model.Post{ChannelId: th.BasicChannel.Id, Message: "testMsg"}) + require.NoError(t, err) + CheckCreatedStatus(t, resp) + rootIds = append(rootIds, rpost) + if i%2 == 0 { + _, resp, err = client.CreatePost(&model.Post{ChannelId: th.BasicChannel.Id, Message: "testReply", RootId: rpost.Id}) + } else { + _, resp, err = sysadminClient.CreatePost(&model.Post{ChannelId: th.BasicChannel.Id, Message: "testReply @" + th.BasicUser.Username, RootId: rpost.Id}) + } + + require.NoError(t, err) + CheckCreatedStatus(t, resp) + } + + defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.BasicUser.Id) + defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.SystemAdminUser.Id) + + uss, _, err := th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{ + Deleted: false, + ThreadsOnly: true, + PageSize: 30, + }) + + require.NoError(t, err) + require.Len(t, rootIds, 10) + require.Len(t, uss.Threads, 10) + require.Equal(t, int64(0), uss.Total) + require.Equal(t, int64(0), uss.TotalUnreadThreads) + require.Equal(t, int64(0), uss.TotalUnreadMentions) + require.Equal(t, int64(1), uss.Threads[0].ReplyCount) + + require.Equal(t, rootIds[9].Id, uss.Threads[0].PostId) + require.Equal(t, th.SystemAdminUser.Id, uss.Threads[0].Participants[0].Id) + require.Equal(t, th.BasicUser.Id, uss.Threads[1].Participants[0].Id) + }) + + t.Run("setting both threadsOnly, and totalsOnly params is not allowed", func(t *testing.T) { + defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.BasicUser.Id) + + _, resp, err := th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{ + ThreadsOnly: true, + TotalsOnly: true, + PageSize: 30, + }) + + require.Error(t, err) + checkHTTPStatus(t, resp, http.StatusBadRequest) + }) + t.Run("editing or reacting to reply post does not make thread unread", func(t *testing.T) { client := th.Client diff --git a/app/user.go b/app/user.go index 3d01fea64e..3bfd1778b4 100644 --- a/app/user.go +++ b/app/user.go @@ -2296,35 +2296,37 @@ func (a *App) GetThreadsForUser(userID, teamID string, options model.GetUserThre var result model.Threads var eg errgroup.Group - eg.Go(func() error { - totalUnreadThreads, err := a.Srv().Store.Thread().GetTotalUnreadThreads(userID, teamID, options) - if err != nil { - return errors.Wrapf(err, "failed to count unread threads for user id=%s", userID) - } - result.TotalUnreadThreads = totalUnreadThreads + if !options.ThreadsOnly { + eg.Go(func() error { + totalUnreadThreads, err := a.Srv().Store.Thread().GetTotalUnreadThreads(userID, teamID, options) + if err != nil { + return errors.Wrapf(err, "failed to count unread threads for user id=%s", userID) + } + result.TotalUnreadThreads = totalUnreadThreads - return nil - }) + return nil + }) - eg.Go(func() error { - totalCount, err := a.Srv().Store.Thread().GetTotalThreads(userID, teamID, options) - if err != nil { - return errors.Wrapf(err, "failed to count threads for user id=%s", userID) - } - result.Total = totalCount + eg.Go(func() error { + totalCount, err := a.Srv().Store.Thread().GetTotalThreads(userID, teamID, options) + if err != nil { + return errors.Wrapf(err, "failed to count threads for user id=%s", userID) + } + result.Total = totalCount - return nil - }) + return nil + }) - eg.Go(func() error { - totalUnreadMentions, err := a.Srv().Store.Thread().GetTotalUnreadMentions(userID, teamID, options) - if err != nil { - return errors.Wrapf(err, "failed to count threads for user id=%s", userID) - } - result.TotalUnreadMentions = totalUnreadMentions + eg.Go(func() error { + totalUnreadMentions, err := a.Srv().Store.Thread().GetTotalUnreadMentions(userID, teamID, options) + if err != nil { + return errors.Wrapf(err, "failed to count threads for user id=%s", userID) + } + result.TotalUnreadMentions = totalUnreadMentions - return nil - }) + return nil + }) + } if !options.TotalsOnly { eg.Go(func() error { diff --git a/i18n/en.json b/i18n/en.json index f30a4f4287..03c5a400ba 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1861,6 +1861,10 @@ "id": "api.file.write_file.app_error", "translation": "Unable to write the file." }, + { + "id": "api.getThreadsForUser.bad_only_params", + "translation": "OnlyThreads and OnlyTotals parameters to getThreadsForUser are mutually exclusive" + }, { "id": "api.getThreadsForUser.bad_params", "translation": "Before and After parameters to getThreadsForUser are mutually exclusive" diff --git a/model/client4.go b/model/client4.go index 75d339af1c..ba51b5ce01 100644 --- a/model/client4.go +++ b/model/client4.go @@ -7813,6 +7813,12 @@ func (c *Client4) GetUserThreads(userId, teamId string, options GetUserThreadsOp if options.Unread { v.Set("unread", "true") } + if options.ThreadsOnly { + v.Set("threadsOnly", "true") + } + if options.TotalsOnly { + v.Set("totalsOnly", "true") + } url := c.userThreadsRoute(userId, teamId) if len(v) > 0 { url += "?" + v.Encode() diff --git a/model/thread.go b/model/thread.go index 8998570979..17fffdddb3 100644 --- a/model/thread.go +++ b/model/thread.go @@ -67,6 +67,9 @@ type GetUserThreadsOpts struct { // TotalsOnly will not fetch any threads and just fetch the total counts TotalsOnly bool + // ThreadsOnly will fetch threads but not calculate totals and will return 0 + ThreadsOnly bool + // TeamOnly will only fetch threads and unreads for the specified team and excludes DMs/GMs TeamOnly bool }