[MM-36792] limit number of threads returned from SQL store (#18260)

Summary
Limit the number of threads returned in a single SQL store call by using the per_page query param instead of pageSize. Our param handling code automatically limits the number of records that can be requested. To support older mobile clients we continue to support the pageSize param until version 6.0 of the server is the minimum supported server version on mobile. 

Related PRs:

[MM-36792] Consistent query param names mattermost-webapp#8700
[MM-36792] Consistent query param names mattermost-mobile#5643

Ticket Link
https://mattermost.atlassian.net/browse/MM-36792
Этот коммит содержится в:
Ashish Bhate
2021-08-31 19:16:54 +05:30
коммит произвёл GitHub
родитель 9fb8de7318
Коммит b6564fec6d
5 изменённых файлов: 63 добавлений и 20 удалений

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

@@ -2939,7 +2939,7 @@ func getThreadsForUser(c *Context, w http.ResponseWriter, r *http.Request) {
Since: 0,
Before: "",
After: "",
PageSize: 30,
PageSize: uint64(c.Params.PerPage),
Unread: false,
Extended: false,
Deleted: false,
@@ -2962,15 +2962,6 @@ func getThreadsForUser(c *Context, w http.ResponseWriter, r *http.Request) {
c.Err = model.NewAppError("api.getThreadsForUser", "api.getThreadsForUser.bad_params", nil, "", http.StatusBadRequest)
return
}
pageSizeString := r.URL.Query().Get("pageSize")
if pageSizeString != "" {
pageSize, parseError := strconv.ParseUint(pageSizeString, 10, 64)
if parseError != nil {
c.SetInvalidParam("pageSize")
return
}
options.PageSize = pageSize
}
deletedStr := r.URL.Query().Get("deleted")
unreadStr := r.URL.Query().Get("unread")

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

@@ -5621,7 +5621,8 @@ func TestGetThreadsForUser(t *testing.T) {
defer th.App.Srv().Store.Post().PermanentDeleteByUser(th.BasicUser.Id)
uss, _, err := th.Client.GetUserThreads(th.BasicUser.Id, th.BasicTeam.Id, model.GetUserThreadsOpts{
Deleted: false,
Deleted: false,
PageSize: 30,
})
require.NoError(t, err)
require.Len(t, uss.Threads, 30)

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

@@ -6593,7 +6593,7 @@ func (c *Client4) GetUserThreads(userId, teamId string, options GetUserThreadsOp
v.Set("after", options.After)
}
if options.PageSize != 0 {
v.Set("pageSize", fmt.Sprintf("%d", options.PageSize))
v.Set("per_page", fmt.Sprintf("%d", options.PageSize))
}
if options.Extended {
v.Set("extended", "true")

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

@@ -5,6 +5,7 @@ package web
import (
"net/http"
"net/url"
"strconv"
"strings"
@@ -255,14 +256,7 @@ func ParamsFromRequest(r *http.Request) *Params {
params.Permanent = val
}
if val, err := strconv.Atoi(query.Get("per_page")); err != nil || val < 0 {
params.PerPage = PerPageDefault
} else if val > PerPageMaximum {
params.PerPage = PerPageMaximum
} else {
params.PerPage = val
}
params.PerPage = getPerPageFromQuery(query)
if val, err := strconv.Atoi(query.Get("logs_per_page")); err != nil || val < 0 {
params.LogsPerPage = LogsPerPageDefault
} else if val > LogsPerPageMaximum {
@@ -363,3 +357,26 @@ func ParamsFromRequest(r *http.Request) *Params {
return params
}
// getPerPageFromQuery returns the PerPage value from the given query.
// This function should be removed and the support for `pageSize`
// should be dropped after v1.46 of the mobile app is no longer supported
// https://mattermost.atlassian.net/browse/MM-38131
func getPerPageFromQuery(query url.Values) int {
val, err := strconv.Atoi(query.Get("per_page"))
if err != nil {
val, err = strconv.Atoi(query.Get("pageSize"))
// if err != nil || val < 0 {
// return PerPageDefault
// } else if val > PerPageMaximum {
// return PerPageMaximum
// }
// return val
}
if err != nil || val < 0 {
return PerPageDefault
} else if val > PerPageMaximum {
return PerPageMaximum
}
return val
}

34
web/params_test.go Обычный файл
Просмотреть файл

@@ -0,0 +1,34 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
package web
import (
"net/url"
"testing"
"github.com/stretchr/testify/require"
)
func TestGetPerPageFromQuery(t *testing.T) {
t.Run("defaults should be set", func(t *testing.T) {
query := make(url.Values)
perPage := getPerPageFromQuery(query)
require.Equal(t, PerPageDefault, perPage)
})
t.Run("per_page should take priority", func(t *testing.T) {
query := make(url.Values)
query.Add("pageSize", "100")
query.Add("per_page", "50")
perPage := getPerPageFromQuery(query)
require.Equal(t, 50, perPage)
})
t.Run("pageSize should be used only if per_page is incorrectly set", func(t *testing.T) {
query := make(url.Values)
query.Add("pageSize", "100")
query.Add("per_page", "BAD VALUE")
perPage := getPerPageFromQuery(query)
require.Equal(t, 100, perPage)
})
}