Skip to content

Commit

Permalink
[bugfix] Return all accounts when list accounts limit <= 0 (#2014)
Browse files Browse the repository at this point in the history
  • Loading branch information
tsmethurst authored Jul 21, 2023
1 parent d6fa74e commit 89ee9d5
Show file tree
Hide file tree
Showing 6 changed files with 451 additions and 23 deletions.
6 changes: 4 additions & 2 deletions docs/api/swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4873,9 +4873,11 @@ paths:
in: query
name: min_id
type: string
- default: 20
description: Number of accounts to return.
- default: 40
description: 'Number of accounts to return. If set to 0 explicitly, all accounts in the list will be returned, and pagination headers will not be used. This is a workaround for Mastodon API peculiarities: https://docs.joinmastodon.org/methods/lists/#query-parameters.'
in: query
maximum: 80
minimum: 0
name: limit
type: integer
produces:
Expand Down
30 changes: 26 additions & 4 deletions internal/api/client/lists/listaccounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,13 @@ import (
// -
// name: limit
// type: integer
// description: Number of accounts to return.
// default: 20
// description: >-
// Number of accounts to return.
// If set to 0 explicitly, all accounts in the list will be returned, and pagination headers will not be used.
// This is a workaround for Mastodon API peculiarities: https://docs.joinmastodon.org/methods/lists/#query-parameters.
// default: 40
// minimum: 0
// maximum: 80
// in: query
// required: false
//
Expand Down Expand Up @@ -129,14 +134,31 @@ func (m *Module) ListAccountsGETHandler(c *gin.Context) {
return
}

limit, errWithCode := apiutil.ParseLimit(c.Query(apiutil.LimitKey), 20, 40, 1)
limit, errWithCode := apiutil.ParseLimit(c.Query(apiutil.LimitKey), 40, 80, 0)
if errWithCode != nil {
apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
return
}

var (
ctx = c.Request.Context()
)

if limit == 0 {
// Return all accounts in the list without pagination.
accounts, errWithCode := m.processor.List().GetAllListAccounts(ctx, authed.Account, targetListID)
if errWithCode != nil {
apiutil.ErrorHandler(c, errWithCode, m.processor.InstanceGetV1)
return
}

c.JSON(http.StatusOK, accounts)
return
}

// Return subset of accounts in the list with pagination.
resp, errWithCode := m.processor.List().GetListAccounts(
c.Request.Context(),
ctx,
authed.Account,
targetListID,
c.Query(MaxIDKey),
Expand Down
246 changes: 246 additions & 0 deletions internal/api/client/lists/listaccounts_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,246 @@
// GoToSocial
// Copyright (C) GoToSocial Authors [email protected]
// SPDX-License-Identifier: AGPL-3.0-or-later
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <http://www.gnu.org/licenses/>.

package lists_test

import (
"encoding/json"
"fmt"
"io/ioutil"
"net/http"
"net/http/httptest"
"strconv"
"testing"

"github.com/stretchr/testify/suite"
"github.com/superseriousbusiness/gotosocial/internal/api/client/lists"
apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"
apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util"
"github.com/superseriousbusiness/gotosocial/internal/config"
"github.com/superseriousbusiness/gotosocial/internal/gtserror"
"github.com/superseriousbusiness/gotosocial/internal/oauth"
"github.com/superseriousbusiness/gotosocial/testrig"
)

type ListAccountsTestSuite struct {
ListsStandardTestSuite
}

func (suite *ListAccountsTestSuite) getListAccounts(
expectedHTTPStatus int,
expectedBody string,
listID string,
maxID string,
sinceID string,
minID string,
limit *int,
) (
[]*apimodel.Account,
string, // Link header
error,
) {

var (
recorder = httptest.NewRecorder()
ctx, _ = testrig.CreateGinTestContext(recorder, nil)
)

// Prepare test context.
ctx.Set(oauth.SessionAuthorizedAccount, suite.testAccounts["local_account_1"])
ctx.Set(oauth.SessionAuthorizedToken, oauth.DBTokenToToken(suite.testTokens["local_account_1"]))
ctx.Set(oauth.SessionAuthorizedApplication, suite.testApplications["application_1"])
ctx.Set(oauth.SessionAuthorizedUser, suite.testUsers["local_account_1"])

// Inject path parameters.
ctx.AddParam("id", listID)

// Inject query parameters.
requestPath := config.GetProtocol() + "://" + config.GetHost() + "/api/" + lists.BasePath + "/" + listID + "/accounts"

if limit != nil {
requestPath += "?limit=" + strconv.Itoa(*limit)
} else {
requestPath += "?limit=40"
}
if maxID != "" {
requestPath += "&" + apiutil.MaxIDKey + "=" + maxID
}
if sinceID != "" {
requestPath += "&" + apiutil.SinceIDKey + "=" + sinceID
}
if minID != "" {
requestPath += "&" + apiutil.MinIDKey + "=" + minID
}

// Prepare test context request.
request := httptest.NewRequest(http.MethodGet, requestPath, nil)
request.Header.Set("accept", "application/json")
ctx.Request = request

// trigger the handler
suite.listsModule.ListAccountsGETHandler(ctx)

// read the response
result := recorder.Result()
defer result.Body.Close()

b, err := ioutil.ReadAll(result.Body)
if err != nil {
return nil, "", err
}

errs := gtserror.MultiError{}

// check code + body
if resultCode := recorder.Code; expectedHTTPStatus != resultCode {
errs = append(errs, fmt.Sprintf("expected %d got %d", expectedHTTPStatus, resultCode))
}

// if we got an expected body, return early
if expectedBody != "" {
if string(b) != expectedBody {
errs = append(errs, fmt.Sprintf("expected %s got %s", expectedBody, string(b)))
}
return nil, "", errs.Combine()
}

resp := []*apimodel.Account{}
if err := json.Unmarshal(b, &resp); err != nil {
return nil, "", err
}

return resp, result.Header.Get("Link"), nil
}

func (suite *ListAccountsTestSuite) TestGetListAccountsPaginatedDefaultLimit() {
var (
expectedHTTPStatus = 200
expectedBody = ""
listID = suite.testLists["local_account_1_list_1"].ID
maxID = ""
minID = ""
sinceID = ""
limit *int = nil
)

accounts, link, err := suite.getListAccounts(
expectedHTTPStatus,
expectedBody,
listID,
maxID,
sinceID,
minID,
limit,
)
if err != nil {
suite.FailNow(err.Error())
}

suite.Len(accounts, 2)
suite.Equal(
`<http://localhost:8080/api/v1/lists/01H0G8E4Q2J3FE3JDWJVWEDCD1/accounts?limit=40&max_id=01H0G89MWVQE0M58VD2HQYMQWH>; rel="next", `+
`<http://localhost:8080/api/v1/lists/01H0G8E4Q2J3FE3JDWJVWEDCD1/accounts?limit=40&min_id=01H0G8FFM1AGQDRNGBGGX8CYJQ>; rel="prev"`,
link,
)
}

func (suite *ListAccountsTestSuite) TestGetListAccountsPaginatedNextPage() {
var (
expectedHTTPStatus = 200
expectedBody = ""
listID = suite.testLists["local_account_1_list_1"].ID
maxID = ""
minID = ""
sinceID = ""
limit *int = func() *int { l := 1; return &l }() // Set to 1.
)

// First response, ask for 1 account.
accounts, link, err := suite.getListAccounts(
expectedHTTPStatus,
expectedBody,
listID,
maxID,
sinceID,
minID,
limit,
)
if err != nil {
suite.FailNow(err.Error())
}

suite.Len(accounts, 1)
suite.Equal(
`<http://localhost:8080/api/v1/lists/01H0G8E4Q2J3FE3JDWJVWEDCD1/accounts?limit=1&max_id=01H0G8FFM1AGQDRNGBGGX8CYJQ>; rel="next", `+
`<http://localhost:8080/api/v1/lists/01H0G8E4Q2J3FE3JDWJVWEDCD1/accounts?limit=1&min_id=01H0G8FFM1AGQDRNGBGGX8CYJQ>; rel="prev"`,
link,
)

// Next response, ask for next 1 account.
maxID = "01H0G8FFM1AGQDRNGBGGX8CYJQ"
accounts, link, err = suite.getListAccounts(
expectedHTTPStatus,
expectedBody,
listID,
maxID,
sinceID,
minID,
limit,
)
if err != nil {
suite.FailNow(err.Error())
}

suite.Len(accounts, 1)
suite.Equal(
`<http://localhost:8080/api/v1/lists/01H0G8E4Q2J3FE3JDWJVWEDCD1/accounts?limit=1&max_id=01H0G89MWVQE0M58VD2HQYMQWH>; rel="next", `+
`<http://localhost:8080/api/v1/lists/01H0G8E4Q2J3FE3JDWJVWEDCD1/accounts?limit=1&min_id=01H0G89MWVQE0M58VD2HQYMQWH>; rel="prev"`,
link,
)
}

func (suite *ListAccountsTestSuite) TestGetListAccountsUnpaginated() {
var (
expectedHTTPStatus = 200
expectedBody = ""
listID = suite.testLists["local_account_1_list_1"].ID
maxID = ""
minID = ""
sinceID = ""
limit *int = func() *int { l := 0; return &l }() // Set to 0 explicitly.
)

accounts, link, err := suite.getListAccounts(
expectedHTTPStatus,
expectedBody,
listID,
maxID,
sinceID,
minID,
limit,
)
if err != nil {
suite.FailNow(err.Error())
}

suite.Len(accounts, 2)
suite.Empty(link)
}

func TestListAccountsTestSuite(t *testing.T) {
suite.Run(t, new(ListAccountsTestSuite))
}
Loading

0 comments on commit 89ee9d5

Please sign in to comment.