diff --git a/internal/db/bundb/timeline.go b/internal/db/bundb/timeline.go index 229245899..a07f1a844 100644 --- a/internal/db/bundb/timeline.go +++ b/internal/db/bundb/timeline.go @@ -178,19 +178,22 @@ func (t *timelineDB) GetPublicTimeline(ctx context.Context, maxID string, sinceI } // Make educated guess for slice size - statusIDs := make([]string, 0, limit) + var ( + statusIDs = make([]string, 0, limit) + frontToBack = true + ) q := t.db. NewSelect(). TableExpr("? AS ?", bun.Ident("statuses"), bun.Ident("status")). - Column("status.id"). // Public only. Where("? = ?", bun.Ident("status.visibility"), gtsmodel.VisibilityPublic). // Ignore boosts. Where("? IS NULL", bun.Ident("status.boost_of_id")). - Order("status.id DESC") + // Select only IDs from table + Column("status.id") - if maxID == "" { + if maxID == "" || maxID >= id.Highest { const future = 24 * time.Hour var err error @@ -206,27 +209,54 @@ func (t *timelineDB) GetPublicTimeline(ctx context.Context, maxID string, sinceI q = q.Where("? < ?", bun.Ident("status.id"), maxID) if sinceID != "" { + // return only statuses HIGHER (ie., newer) than sinceID q = q.Where("? > ?", bun.Ident("status.id"), sinceID) } if minID != "" { + // return only statuses HIGHER (ie., newer) than minID q = q.Where("? > ?", bun.Ident("status.id"), minID) + + // page up + frontToBack = false } if local { + // return only statuses posted by local account havers q = q.Where("? = ?", bun.Ident("status.local"), local) } if limit > 0 { + // limit amount of statuses returned q = q.Limit(limit) } + if frontToBack { + // Page down. + q = q.Order("status.id DESC") + } else { + // Page up. + q = q.Order("status.id ASC") + } + if err := q.Scan(ctx, &statusIDs); err != nil { return nil, err } - statuses := make([]*gtsmodel.Status, 0, len(statusIDs)) + if len(statusIDs) == 0 { + return nil, nil + } + // If we're paging up, we still want statuses + // to be sorted by ID desc, so reverse ids slice. + // https://zchee.github.io/golang-wiki/SliceTricks/#reversing + if !frontToBack { + for l, r := 0, len(statusIDs)-1; l < r; l, r = l+1, r-1 { + statusIDs[l], statusIDs[r] = statusIDs[r], statusIDs[l] + } + } + + statuses := make([]*gtsmodel.Status, 0, len(statusIDs)) for _, id := range statusIDs { // Fetch status from db for ID status, err := t.state.DB.GetStatusByID(ctx, id) diff --git a/internal/processing/account/bookmarks.go b/internal/processing/account/bookmarks.go index 88a02786e..9cbc3db26 100644 --- a/internal/processing/account/bookmarks.go +++ b/internal/processing/account/bookmarks.go @@ -25,7 +25,6 @@ "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" - "github.com/superseriousbusiness/gotosocial/internal/id" "github.com/superseriousbusiness/gotosocial/internal/log" "github.com/superseriousbusiness/gotosocial/internal/util" ) @@ -34,15 +33,23 @@ // Paging for this response is done based on bookmark ID rather than status ID. func (p *Processor) BookmarksGet(ctx context.Context, requestingAccount *gtsmodel.Account, limit int, maxID string, minID string) (*apimodel.PageableResponse, gtserror.WithCode) { bookmarks, err := p.state.DB.GetStatusBookmarks(ctx, requestingAccount.ID, limit, maxID, minID) - if err != nil { + if err != nil && !errors.Is(err, db.ErrNoEntries) { return nil, gtserror.NewErrorInternalError(err) } + count := len(bookmarks) + if count == 0 { + return util.EmptyPageableResponse(), nil + } + var ( - count = len(bookmarks) - items = make([]interface{}, 0, count) - nextMaxIDValue = id.Highest - prevMinIDValue = id.Lowest + items = make([]interface{}, 0, count) + + // Set next + prev values before filtering and API + // converting, so caller can still page properly. + // Page based on bookmark ID, not status ID. + nextMaxIDValue = bookmarks[count-1].ID + prevMinIDValue = bookmarks[0].ID ) for _, bookmark := range bookmarks { @@ -73,23 +80,6 @@ func (p *Processor) BookmarksGet(ctx context.Context, requestingAccount *gtsmode continue } items = append(items, item) - - // Page based on bookmark ID, not status ID. - // Note that we only set these values here - // when we're certain that the caller is able - // to see the status, *and* we're sure that - // we can produce an api model representation. - if bookmark.ID < nextMaxIDValue { - nextMaxIDValue = bookmark.ID // Lowest ID (for paging down). - } - - if bookmark.ID > prevMinIDValue { - prevMinIDValue = bookmark.ID // Highest ID (for paging up). - } - } - - if len(items) == 0 { - return util.EmptyPageableResponse(), nil } return util.PackagePageableResponse(util.PageableResponseParams{ diff --git a/internal/processing/account/statuses.go b/internal/processing/account/statuses.go index 99e9edbcf..716157a14 100644 --- a/internal/processing/account/statuses.go +++ b/internal/processing/account/statuses.go @@ -74,21 +74,8 @@ func (p *Processor) StatusesGet( return nil, gtserror.NewErrorInternalError(err) } - if len(statuses) == 0 { - return util.EmptyPageableResponse(), nil - } - - // Filtering + serialization process is the same for - // both pinned status queries and 'normal' ones. - filtered, err := p.filter.StatusesVisible(ctx, requestingAccount, statuses) - if err != nil { - return nil, gtserror.NewErrorInternalError(err) - } - - count := len(filtered) + count := len(statuses) if count == 0 { - // After filtering there were - // no statuses left to serve. return util.EmptyPageableResponse(), nil } @@ -97,10 +84,17 @@ func (p *Processor) StatusesGet( // Set next + prev values before filtering and API // converting, so caller can still page properly. - nextMaxIDValue = filtered[count-1].ID - prevMinIDValue = filtered[0].ID + nextMaxIDValue = statuses[count-1].ID + prevMinIDValue = statuses[0].ID ) + // Filtering + serialization process is the same for + // both pinned status queries and 'normal' ones. + filtered, err := p.filter.StatusesVisible(ctx, requestingAccount, statuses) + if err != nil { + return nil, gtserror.NewErrorInternalError(err) + } + for _, s := range filtered { // Convert filtered statuses to API statuses. item, err := p.converter.StatusToAPIStatus(ctx, s, requestingAccount) diff --git a/internal/processing/admin/report.go b/internal/processing/admin/report.go index 0f47cf839..32f05719b 100644 --- a/internal/processing/admin/report.go +++ b/internal/processing/admin/report.go @@ -19,6 +19,7 @@ import ( "context" + "errors" "fmt" "strconv" "time" @@ -45,17 +46,20 @@ func (p *Processor) ReportsGet( limit int, ) (*apimodel.PageableResponse, gtserror.WithCode) { reports, err := p.state.DB.GetReports(ctx, resolved, accountID, targetAccountID, maxID, sinceID, minID, limit) - if err != nil { - if err == db.ErrNoEntries { - return util.EmptyPageableResponse(), nil - } + if err != nil && !errors.Is(err, db.ErrNoEntries) { return nil, gtserror.NewErrorInternalError(err) } count := len(reports) - items := make([]interface{}, 0, count) - nextMaxIDValue := reports[count-1].ID - prevMinIDValue := reports[0].ID + if count == 0 { + return util.EmptyPageableResponse(), nil + } + + var ( + items = make([]interface{}, 0, count) + nextMaxIDValue = reports[count-1].ID + prevMinIDValue = reports[0].ID + ) for _, r := range reports { item, err := p.converter.ReportToAdminAPIReport(ctx, r, account) @@ -65,7 +69,7 @@ func (p *Processor) ReportsGet( items = append(items, item) } - extraQueryParams := []string{} + extraQueryParams := make([]string, 0, 3) if resolved != nil { extraQueryParams = append(extraQueryParams, "resolved="+strconv.FormatBool(*resolved)) } diff --git a/internal/processing/timeline/public.go b/internal/processing/timeline/public.go index eb8c0c381..0ace00c0b 100644 --- a/internal/processing/timeline/public.go +++ b/internal/processing/timeline/public.go @@ -20,7 +20,6 @@ import ( "context" "errors" - "fmt" apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" "github.com/superseriousbusiness/gotosocial/internal/db" @@ -33,7 +32,7 @@ func (p *Processor) PublicTimelineGet(ctx context.Context, authed *oauth.Auth, maxID string, sinceID string, minID string, limit int, local bool) (*apimodel.PageableResponse, gtserror.WithCode) { statuses, err := p.state.DB.GetPublicTimeline(ctx, maxID, sinceID, minID, limit, local) if err != nil && !errors.Is(err, db.ErrNoEntries) { - err = fmt.Errorf("PublicTimelineGet: db error getting statuses: %w", err) + err = gtserror.Newf("db error getting statuses: %w", err) return nil, gtserror.NewErrorInternalError(err) } diff --git a/internal/util/paging.go b/internal/util/paging.go index 0ab4b9567..190f40afd 100644 --- a/internal/util/paging.go +++ b/internal/util/paging.go @@ -48,11 +48,6 @@ type PageableResponseParams struct { // a bunch of pageable items (notifications, statuses, etc), as well // as a Link header to inform callers of where to find next/prev items. func PackagePageableResponse(params PageableResponseParams) (*apimodel.PageableResponse, gtserror.WithCode) { - if len(params.Items) == 0 { - // No items to page through. - return EmptyPageableResponse(), nil - } - // Set default paging values, if // they weren't set by the caller. if params.NextMaxIDKey == "" { diff --git a/internal/util/paging_test.go b/internal/util/paging_test.go index 685db14ba..66c5b2c56 100644 --- a/internal/util/paging_test.go +++ b/internal/util/paging_test.go @@ -118,6 +118,7 @@ func (suite *PagingSuite) TestPagingNoItems() { config.SetHost("example.org") params := util.PageableResponseParams{ + Path: "/api/v1/accounts/01H11KA68PM4NNYJEG0FJQ90R3/statuses", NextMaxIDValue: "01H11KA1DM2VH3747YDE7FV5HN", PrevMinIDValue: "01H11KBBVRRDYYC5KEPME1NP5R", Limit: 10, @@ -129,9 +130,9 @@ func (suite *PagingSuite) TestPagingNoItems() { } suite.Empty(resp.Items) - suite.Empty(resp.LinkHeader) - suite.Empty(resp.NextLink) - suite.Empty(resp.PrevLink) + suite.Equal(`; rel="next", ; rel="prev"`, resp.LinkHeader) + suite.Equal(`https://example.org/api/v1/accounts/01H11KA68PM4NNYJEG0FJQ90R3/statuses?limit=10&max_id=01H11KA1DM2VH3747YDE7FV5HN`, resp.NextLink) + suite.Equal(`https://example.org/api/v1/accounts/01H11KA68PM4NNYJEG0FJQ90R3/statuses?limit=10&min_id=01H11KBBVRRDYYC5KEPME1NP5R`, resp.PrevLink) } func TestPagingSuite(t *testing.T) {