From 4f87ef246cc15d2b4a3effb3dff5cf3ece9a44b3 Mon Sep 17 00:00:00 2001 From: kim <89579420+NyaaaWhatsUpDoc@users.noreply.github.com> Date: Tue, 30 Apr 2024 11:19:33 +0100 Subject: [PATCH] [bugfix] paging rel links (#2883) * fix paging so it uses correct cursor query parameter name * improved code comment * whoops, flip the cursoring :facepalm: * fix the broken test --- .../api/activitypub/users/repliesget_test.go | 2 +- internal/paging/boundary.go | 48 +++++++++++-------- internal/paging/page.go | 25 +++++++--- 3 files changed, 49 insertions(+), 26 deletions(-) diff --git a/internal/api/activitypub/users/repliesget_test.go b/internal/api/activitypub/users/repliesget_test.go index 757d0bc83..e821bc897 100644 --- a/internal/api/activitypub/users/repliesget_test.go +++ b/internal/api/activitypub/users/repliesget_test.go @@ -162,7 +162,7 @@ func (suite *RepliesGetTestSuite) TestGetRepliesNext() { "id": targetStatus.URI + "/replies?limit=20&only_other_accounts=false", "partOf": targetStatus.URI + "/replies?only_other_accounts=false", "next": "http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies?limit=20&min_id=01FF25D5Q0DH7CHD57CTRS6WK0&only_other_accounts=false", - "prev": "http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies?limit=20&max_id=01FF25D5Q0DH7CHD57CTRS6WK0&only_other_accounts=false", + "prev": "http://localhost:8080/users/the_mighty_zork/statuses/01F8MHAMCHF6Y650WCRSCP4WMY/replies?limit=20&min_id=01FF25D5Q0DH7CHD57CTRS6WK0&only_other_accounts=false", "orderedItems": []string{"http://localhost:8080/users/admin/statuses/01FF25D5Q0DH7CHD57CTRS6WK0"}, "totalItems": 1, }) diff --git a/internal/paging/boundary.go b/internal/paging/boundary.go index 83d265515..bf4508aff 100644 --- a/internal/paging/boundary.go +++ b/internal/paging/boundary.go @@ -23,26 +23,36 @@ package paging func EitherMinID(minID, sinceID string) Boundary { /* - Paging with `since_id` vs `min_id`: + Paging with `since_id` vs `min_id`: - limit = 4 limit = 4 - +----------+ +----------+ - max_id--> |xxxxxxxxxx| | | <-- max_id - +----------+ +----------+ - |xxxxxxxxxx| | | - +----------+ +----------+ - |xxxxxxxxxx| | | - +----------+ +----------+ - |xxxxxxxxxx| |xxxxxxxxxx| - +----------+ +----------+ - | | |xxxxxxxxxx| - +----------+ +----------+ - | | |xxxxxxxxxx| - +----------+ +----------+ - since_id--> | | |xxxxxxxxxx| <-- min_id - +----------+ +----------+ - | | | | - +----------+ +----------+ + limit = 4 limit = 4 + +----------+ +----------+ + max_id--> |xxxxxxxxxx| | | <-- max_id + +----------+ +----------+ + |xxxxxxxxxx| | | + +----------+ +----------+ + |xxxxxxxxxx| | | + +----------+ +----------+ + |xxxxxxxxxx| |xxxxxxxxxx| + +----------+ +----------+ + | | |xxxxxxxxxx| + +----------+ +----------+ + | | |xxxxxxxxxx| + +----------+ +----------+ + since_id--> | | |xxxxxxxxxx| <-- min_id + +----------+ +----------+ + | | | | + +----------+ +----------+ + + To sum it up in words: + + when paging with since_id, max_id is used as + the cursor value, and since_id provides a + limiting value to the results. + + when paging with min_id, min_id is used as + the cursor value, and max_id provides a + limiting value to the results. */ switch { diff --git a/internal/paging/page.go b/internal/paging/page.go index a1cf76c74..8e8261396 100644 --- a/internal/paging/page.go +++ b/internal/paging/page.go @@ -289,14 +289,27 @@ func (p *Page) ToLinkURL(proto, host, path string, queryParams url.Values) *url. queryParams = cloneQuery(queryParams) } - if p.Min.Value != "" { - // A page-minimum query parameter is available. - queryParams.Set(p.Min.Name, p.Min.Value) + var cursor string + + // Depending on page ordering, the + // page will be cursored by either + // the min or max query parameter. + if p.order().Ascending() { + cursor = p.Min.Name + } else { + cursor = p.Max.Name } - if p.Max.Value != "" { - // A page-maximum query parameter is available. - queryParams.Set(p.Max.Name, p.Max.Value) + if cursor != "" { + if p.Min.Value != "" { + // Set page-minimum cursor value. + queryParams.Set(cursor, p.Min.Value) + } + + if p.Max.Value != "" { + // Set page-maximum cursor value. + queryParams.Set(cursor, p.Max.Value) + } } if p.Limit > 0 {