From f4319740ab02d680961781861335285f618f5f48 Mon Sep 17 00:00:00 2001 From: kim <89579420+NyaaaWhatsUpDoc@users.noreply.github.com> Date: Tue, 18 Jul 2023 09:43:17 +0100 Subject: [PATCH] [bugfix] more robust list timeline invalidation (#1995) --- internal/api/util/parsequery.go | 9 +- internal/cache/cache.go | 14 +- internal/cache/gts.go | 3 + internal/db/bundb/list.go | 264 ++++++++++++++--------- internal/db/bundb/relationship_follow.go | 3 +- internal/processing/account/bookmarks.go | 1 + internal/processing/account/statuses.go | 36 ++-- internal/processing/admin/report.go | 16 +- internal/processing/list/delete.go | 5 +- internal/processing/list/get.go | 38 ++-- internal/processing/report/get.go | 26 +-- internal/processing/timeline/faved.go | 6 +- internal/processing/timeline/home.go | 18 +- internal/processing/timeline/list.go | 18 +- internal/processing/timeline/public.go | 23 +- 15 files changed, 254 insertions(+), 226 deletions(-) diff --git a/internal/api/util/parsequery.go b/internal/api/util/parsequery.go index 59b07f7e..92105ef8 100644 --- a/internal/api/util/parsequery.go +++ b/internal/api/util/parsequery.go @@ -73,7 +73,14 @@ func requiredError(key string) gtserror.WithCode { */ func ParseLimit(value string, defaultValue int, max, min int) (int, gtserror.WithCode) { - return parseInt(value, defaultValue, max, min, LimitKey) + i, err := parseInt(value, defaultValue, max, min, LimitKey) + if err != nil { + return 0, err + } else if i == 0 { + // treat 0 as an empty query + return defaultValue, nil + } + return i, nil } func ParseLocal(value string, defaultValue bool) (bool, gtserror.WithCode) { diff --git a/internal/cache/cache.go b/internal/cache/cache.go index df7d9a7a..63564935 100644 --- a/internal/cache/cache.go +++ b/internal/cache/cache.go @@ -93,13 +93,14 @@ func (c *Caches) setuphooks() { }) c.GTS.EmojiCategory().SetInvalidateCallback(func(category *gtsmodel.EmojiCategory) { - // Invalidate entire emoji cache, - // as we can't know which emojis - // specifically this will effect. - c.GTS.Emoji().Clear() + // Invalidate any emoji in this category. + c.GTS.Emoji().Invalidate("CategoryID", category.ID) }) c.GTS.Follow().SetInvalidateCallback(func(follow *gtsmodel.Follow) { + // Invalidate any related list entries. + c.GTS.ListEntry().Invalidate("FollowID", follow.ID) + // Invalidate follow origin account ID cached visibility. c.Visibility.Invalidate("ItemID", follow.AccountID) c.Visibility.Invalidate("RequesterID", follow.AccountID) @@ -122,6 +123,11 @@ func (c *Caches) setuphooks() { c.GTS.Follow().Invalidate("ID", followReq.ID) }) + c.GTS.List().SetInvalidateCallback(func(list *gtsmodel.List) { + // Invalidate all cached entries of this list. + c.GTS.ListEntry().Invalidate("ListID", list.ID) + }) + c.GTS.Status().SetInvalidateCallback(func(status *gtsmodel.Status) { // Invalidate status ID cached visibility. c.Visibility.Invalidate("ItemID", status.ID) diff --git a/internal/cache/gts.go b/internal/cache/gts.go index 4b2e65b9..8082a9fd 100644 --- a/internal/cache/gts.go +++ b/internal/cache/gts.go @@ -262,6 +262,7 @@ func (c *GTSCaches) initEmoji() { {Name: "URI"}, {Name: "Shortcode.Domain"}, {Name: "ImageStaticURL"}, + {Name: "CategoryID", Multi: true}, }, func(e1 *gtsmodel.Emoji) *gtsmodel.Emoji { e2 := new(gtsmodel.Emoji) *e2 = *e1 @@ -338,6 +339,8 @@ func (c *GTSCaches) initList() { func (c *GTSCaches) initListEntry() { c.listEntry = result.New([]result.Lookup{ {Name: "ID"}, + {Name: "ListID", Multi: true}, + {Name: "FollowID", Multi: true}, }, func(l1 *gtsmodel.ListEntry) *gtsmodel.ListEntry { l2 := new(gtsmodel.ListEntry) *l2 = *l1 diff --git a/internal/db/bundb/list.go b/internal/db/bundb/list.go index 38701cc0..837dfac2 100644 --- a/internal/db/bundb/list.go +++ b/internal/db/bundb/list.go @@ -41,33 +41,6 @@ type listDB struct { LIST FUNCTIONS */ -func (l *listDB) getList(ctx context.Context, lookup string, dbQuery func(*gtsmodel.List) error, keyParts ...any) (*gtsmodel.List, error) { - list, err := l.state.Caches.GTS.List().Load(lookup, func() (*gtsmodel.List, error) { - var list gtsmodel.List - - // Not cached! Perform database query. - if err := dbQuery(&list); err != nil { - return nil, l.conn.ProcessError(err) - } - - return &list, nil - }, keyParts...) - if err != nil { - return nil, err // already processed - } - - if gtscontext.Barebones(ctx) { - // Only a barebones model was requested. - return list, nil - } - - if err := l.state.DB.PopulateList(ctx, list); err != nil { - return nil, err - } - - return list, nil -} - func (l *listDB) GetListByID(ctx context.Context, id string) (*gtsmodel.List, error) { return l.getList( ctx, @@ -82,6 +55,34 @@ func (l *listDB) GetListByID(ctx context.Context, id string) (*gtsmodel.List, er ) } +func (l *listDB) getList(ctx context.Context, lookup string, dbQuery func(*gtsmodel.List) error, keyParts ...any) (*gtsmodel.List, error) { + list, err := l.state.Caches.GTS.List().Load(lookup, func() (*gtsmodel.List, error) { + var list gtsmodel.List + + // Not cached! Perform database query. + if err := dbQuery(&list); err != nil { + return nil, l.conn.ProcessError(err) + } + + return &list, nil + }, keyParts...) + if err != nil { + // already processed + return nil, err + } + + if gtscontext.Barebones(ctx) { + // Only a barebones model was requested. + return list, nil + } + + if err := l.state.DB.PopulateList(ctx, list); err != nil { + return nil, err + } + + return list, nil +} + func (l *listDB) GetListsForAccountID(ctx context.Context, accountID string) ([]*gtsmodel.List, error) { // Fetch IDs of all lists owned by this account. var listIDs []string @@ -107,8 +108,6 @@ func (l *listDB) GetListsForAccountID(ctx context.Context, accountID string) ([] log.Errorf(ctx, "error fetching list %q: %v", id, err) continue } - - // Append list. lists = append(lists, list) } @@ -161,49 +160,89 @@ func (l *listDB) UpdateList(ctx context.Context, list *gtsmodel.List, columns .. columns = append(columns, "updated_at") } + defer func() { + // Invalidate all entries for this list ID. + l.state.Caches.GTS.ListEntry().Invalidate("ListID", list.ID) + + // Invalidate this entire list's timeline. + if err := l.state.Timelines.List.RemoveTimeline(ctx, list.ID); err != nil { + log.Errorf(ctx, "error invalidating list timeline: %q", err) + } + }() + return l.state.Caches.GTS.List().Store(list, func() error { - if _, err := l.conn.NewUpdate(). + _, err := l.conn.NewUpdate(). Model(list). Where("? = ?", bun.Ident("list.id"), list.ID). Column(columns...). - Exec(ctx); err != nil { - return l.conn.ProcessError(err) - } - - return nil + Exec(ctx) + return l.conn.ProcessError(err) }) } func (l *listDB) DeleteListByID(ctx context.Context, id string) error { - defer l.state.Caches.GTS.List().Invalidate("ID", id) - - // Select all entries that belong to this list. - listEntries, err := l.state.DB.GetListEntries(ctx, id, "", "", "", 0) + // Load list by ID into cache to ensure we can perform + // all necessary cache invalidation hooks on removal. + _, err := l.GetListByID( + // Don't populate the entry; + // we only want the list ID. + gtscontext.SetBarebones(ctx), + id, + ) if err != nil { - return fmt.Errorf("error selecting entries from list %q: %w", id, err) + if errors.Is(err, db.ErrNoEntries) { + // Already gone. + return nil + } + return err } - // Delete each list entry. This will - // invalidate the list timeline too. - for _, listEntry := range listEntries { - err := l.state.DB.DeleteListEntry(ctx, listEntry.ID) - if err != nil && !errors.Is(err, db.ErrNoEntries) { + defer func() { + // Invalidate this list from cache. + l.state.Caches.GTS.List().Invalidate("ID", id) + + // Invalidate this entire list's timeline. + if err := l.state.Timelines.List.RemoveTimeline(ctx, id); err != nil { + log.Errorf(ctx, "error invalidating list timeline: %q", err) + } + }() + + return l.conn.RunInTx(ctx, func(tx bun.Tx) error { + // Delete all entries attached to list. + if _, err := tx.NewDelete(). + Table("list_entries"). + Where("? = ?", bun.Ident("list_id"), id). + Exec(ctx); err != nil { return err } - } - // Finally delete list itself from DB. - _, err = l.conn.NewDelete(). - Table("lists"). - Where("? = ?", bun.Ident("id"), id). - Exec(ctx) - return l.conn.ProcessError(err) + // Delete the list itself. + _, err := tx.NewDelete(). + Table("lists"). + Where("? = ?", bun.Ident("id"), id). + Exec(ctx) + return err + }) } /* LIST ENTRY functions */ +func (l *listDB) GetListEntryByID(ctx context.Context, id string) (*gtsmodel.ListEntry, error) { + return l.getListEntry( + ctx, + "ID", + func(listEntry *gtsmodel.ListEntry) error { + return l.conn.NewSelect(). + Model(listEntry). + Where("? = ?", bun.Ident("list_entry.id"), id). + Scan(ctx) + }, + id, + ) +} + func (l *listDB) getListEntry(ctx context.Context, lookup string, dbQuery func(*gtsmodel.ListEntry) error, keyParts ...any) (*gtsmodel.ListEntry, error) { listEntry, err := l.state.Caches.GTS.ListEntry().Load(lookup, func() (*gtsmodel.ListEntry, error) { var listEntry gtsmodel.ListEntry @@ -232,20 +271,6 @@ func (l *listDB) getListEntry(ctx context.Context, lookup string, dbQuery func(* return listEntry, nil } -func (l *listDB) GetListEntryByID(ctx context.Context, id string) (*gtsmodel.ListEntry, error) { - return l.getListEntry( - ctx, - "ID", - func(listEntry *gtsmodel.ListEntry) error { - return l.conn.NewSelect(). - Model(listEntry). - Where("? = ?", bun.Ident("list_entry.id"), id). - Scan(ctx) - }, - id, - ) -} - func (l *listDB) GetListEntries(ctx context.Context, listID string, maxID string, @@ -328,8 +353,6 @@ func (l *listDB) GetListEntries(ctx context.Context, log.Errorf(ctx, "error fetching list entry %q: %v", id, err) continue } - - // Append list entries. listEntries = append(listEntries, listEntry) } @@ -337,7 +360,7 @@ func (l *listDB) GetListEntries(ctx context.Context, } func (l *listDB) GetListEntriesForFollowID(ctx context.Context, followID string) ([]*gtsmodel.ListEntry, error) { - entryIDs := []string{} + var entryIDs []string if err := l.conn. NewSelect(). @@ -362,8 +385,6 @@ func (l *listDB) GetListEntriesForFollowID(ctx context.Context, followID string) log.Errorf(ctx, "error fetching list entry %q: %v", id, err) continue } - - // Append list entries. listEntries = append(listEntries, listEntry) } @@ -387,33 +408,42 @@ func (l *listDB) PopulateListEntry(ctx context.Context, listEntry *gtsmodel.List return nil } -func (l *listDB) PutListEntries(ctx context.Context, listEntries []*gtsmodel.ListEntry) error { - return l.conn.RunInTx(ctx, func(tx bun.Tx) error { - for _, listEntry := range listEntries { - if _, err := tx. - NewInsert(). - Model(listEntry). - Exec(ctx); err != nil { - return err - } +func (l *listDB) PutListEntries(ctx context.Context, entries []*gtsmodel.ListEntry) error { + defer func() { + // Collect unique list IDs from the entries. + listIDs := collate(func(i int) string { + return entries[i].ListID + }, len(entries)) + for _, id := range listIDs { // Invalidate the timeline for the list this entry belongs to. - if err := l.state.Timelines.List.RemoveTimeline(ctx, listEntry.ListID); err != nil { - log.Errorf(ctx, "PutListEntries: error invalidating list timeline: %q", err) + if err := l.state.Timelines.List.RemoveTimeline(ctx, id); err != nil { + log.Errorf(ctx, "error invalidating list timeline: %q", err) } } + }() + // Finally, insert each list entry into the database. + return l.conn.RunInTx(ctx, func(tx bun.Tx) error { + for _, entry := range entries { + if err := l.state.Caches.GTS.ListEntry().Store(entry, func() error { + _, err := tx. + NewInsert(). + Model(entry). + Exec(ctx) + return err + }); err != nil { + return err + } + } return nil }) } func (l *listDB) DeleteListEntry(ctx context.Context, id string) error { - defer l.state.Caches.GTS.ListEntry().Invalidate("ID", id) - - // Load list entry into cache before attempting a delete, - // as we need the followID from it in order to trigger - // timeline invalidation. - listEntry, err := l.GetListEntryByID( + // Load list entry into cache to ensure we can perform + // all necessary cache invalidation hooks on removal. + entry, err := l.GetListEntryByID( // Don't populate the entry; // we only want the list ID. gtscontext.SetBarebones(ctx), @@ -428,36 +458,39 @@ func (l *listDB) DeleteListEntry(ctx context.Context, id string) error { } defer func() { + // Invalidate this list entry upon delete. + l.state.Caches.GTS.ListEntry().Invalidate("ID", id) + // Invalidate the timeline for the list this entry belongs to. - if err := l.state.Timelines.List.RemoveTimeline(ctx, listEntry.ListID); err != nil { - log.Errorf(ctx, "DeleteListEntry: error invalidating list timeline: %q", err) + if err := l.state.Timelines.List.RemoveTimeline(ctx, entry.ListID); err != nil { + log.Errorf(ctx, "error invalidating list timeline: %q", err) } }() - if _, err := l.conn.NewDelete(). + // Finally delete the list entry. + _, err = l.conn.NewDelete(). Table("list_entries"). - Where("? = ?", bun.Ident("id"), listEntry.ID). - Exec(ctx); err != nil { - return l.conn.ProcessError(err) - } - - return nil + Where("? = ?", bun.Ident("id"), id). + Exec(ctx) + return err } func (l *listDB) DeleteListEntriesForFollowID(ctx context.Context, followID string) error { - // Fetch IDs of all entries that pertain to this follow. - var listEntryIDs []string + var entryIDs []string + + // Fetch entry IDs for follow ID. if err := l.conn. NewSelect(). - TableExpr("? AS ?", bun.Ident("list_entries"), bun.Ident("list_entry")). - Column("list_entry.id"). - Where("? = ?", bun.Ident("list_entry.follow_id"), followID). - Order("list_entry.id DESC"). - Scan(ctx, &listEntryIDs); err != nil { + Table("list_entries"). + Column("id"). + Where("? = ?", bun.Ident("follow_id"), followID). + Order("id DESC"). + Scan(ctx, &entryIDs); err != nil { return l.conn.ProcessError(err) } - for _, id := range listEntryIDs { + for _, id := range entryIDs { + // Delete each separately to trigger cache invalidations. if err := l.DeleteListEntry(ctx, id); err != nil { return err } @@ -465,3 +498,24 @@ func (l *listDB) DeleteListEntriesForFollowID(ctx context.Context, followID stri return nil } + +// collate will collect the values of type T from an expected slice of length 'len', +// passing the expected index to each call of 'get' and deduplicating the end result. +func collate[T comparable](get func(int) T, len int) []T { + ts := make([]T, 0, len) + tm := make(map[T]struct{}, len) + + for i := 0; i < len; i++ { + // Get next. + t := get(i) + + if _, ok := tm[t]; !ok { + // New value, add + // to map + slice. + ts = append(ts, t) + tm[t] = struct{}{} + } + } + + return ts +} diff --git a/internal/db/bundb/relationship_follow.go b/internal/db/bundb/relationship_follow.go index 88850e72..349c1ef4 100644 --- a/internal/db/bundb/relationship_follow.go +++ b/internal/db/bundb/relationship_follow.go @@ -328,7 +328,8 @@ func (r *relationshipDB) DeleteAccountFollows(ctx context.Context, accountID str } // Delete each follow from DB. - if err := r.deleteFollow(ctx, follow.ID); err != nil && !errors.Is(err, db.ErrNoEntries) { + if err := r.deleteFollow(ctx, follow.ID); err != nil && + !errors.Is(err, db.ErrNoEntries) { return err } } diff --git a/internal/processing/account/bookmarks.go b/internal/processing/account/bookmarks.go index 32075f59..c6b0c14c 100644 --- a/internal/processing/account/bookmarks.go +++ b/internal/processing/account/bookmarks.go @@ -82,6 +82,7 @@ func (p *Processor) BookmarksGet(ctx context.Context, requestingAccount *gtsmode if bookmark.ID < nextMaxIDValue { nextMaxIDValue = bookmark.ID // Lowest ID (for paging down). } + if bookmark.ID > prevMinIDValue { prevMinIDValue = bookmark.ID // Highest ID (for paging up). } diff --git a/internal/processing/account/statuses.go b/internal/processing/account/statuses.go index df7064b7..26684265 100644 --- a/internal/processing/account/statuses.go +++ b/internal/processing/account/statuses.go @@ -93,28 +93,21 @@ func (p *Processor) StatusesGet( } var ( - items = make([]interface{}, 0, count) - nextMaxIDValue string - prevMinIDValue string - ) + items = make([]interface{}, 0, count) - for i, s := range filtered { // Set next + prev values before filtering and API // converting, so caller can still page properly. - if i == count-1 { - nextMaxIDValue = s.ID - } - - if i == 0 { - prevMinIDValue = s.ID - } + nextMaxIDValue = filtered[count-1].ID + prevMinIDValue = filtered[0].ID + ) + for _, s := range filtered { + // Convert filtered statuses to API statuses. item, err := p.tc.StatusToAPIStatus(ctx, s, requestingAccount) if err != nil { - log.Debugf(ctx, "skipping status %s because it couldn't be converted to its api representation: %s", s.ID, err) + log.Errorf(ctx, "error convering to api status: %v", err) continue } - items = append(items, item) } @@ -171,23 +164,20 @@ func (p *Processor) WebStatusesGet(ctx context.Context, targetAccountID string, } var ( - items = make([]interface{}, 0, count) - nextMaxIDValue string - ) + items = make([]interface{}, 0, count) - for i, s := range statuses { // Set next value before API converting, // so caller can still page properly. - if i == count-1 { - nextMaxIDValue = s.ID - } + nextMaxIDValue = statuses[count-1].ID + ) + for _, s := range statuses { + // Convert fetched statuses to API statuses. item, err := p.tc.StatusToAPIStatus(ctx, s, nil) if err != nil { - log.Debugf(ctx, "skipping status %s because it couldn't be converted to its api representation: %s", s.ID, err) + log.Errorf(ctx, "error convering to api status: %v", err) continue } - items = append(items, item) } diff --git a/internal/processing/admin/report.go b/internal/processing/admin/report.go index 174bd3c2..e99cc2ec 100644 --- a/internal/processing/admin/report.go +++ b/internal/processing/admin/report.go @@ -54,22 +54,14 @@ func (p *Processor) ReportsGet( count := len(reports) items := make([]interface{}, 0, count) - nextMaxIDValue := "" - prevMinIDValue := "" - for i, r := range reports { + nextMaxIDValue := reports[count-1].ID + prevMinIDValue := reports[0].ID + + for _, r := range reports { item, err := p.tc.ReportToAdminAPIReport(ctx, r, account) if err != nil { return nil, gtserror.NewErrorInternalError(fmt.Errorf("error converting report to api: %s", err)) } - - if i == count-1 { - nextMaxIDValue = item.ID - } - - if i == 0 { - prevMinIDValue = item.ID - } - items = append(items, item) } diff --git a/internal/processing/list/delete.go b/internal/processing/list/delete.go index 1c8ee570..327ac9d1 100644 --- a/internal/processing/list/delete.go +++ b/internal/processing/list/delete.go @@ -27,7 +27,8 @@ import ( // Delete deletes one list for the given account. func (p *Processor) Delete(ctx context.Context, account *gtsmodel.Account, id string) gtserror.WithCode { - list, errWithCode := p.getList( + // Ensure list exists + is owned by requesting account. + _, errWithCode := p.getList( // Use barebones ctx; no embedded // structs necessary for this call. gtscontext.SetBarebones(ctx), @@ -38,7 +39,7 @@ func (p *Processor) Delete(ctx context.Context, account *gtsmodel.Account, id st return errWithCode } - if err := p.state.DB.DeleteListByID(ctx, list.ID); err != nil { + if err := p.state.DB.DeleteListByID(ctx, id); err != nil { return gtserror.NewErrorInternalError(err) } diff --git a/internal/processing/list/get.go b/internal/processing/list/get.go index 3f124fe7..0fc14f93 100644 --- a/internal/processing/list/get.go +++ b/internal/processing/list/get.go @@ -87,7 +87,14 @@ func (p *Processor) GetListAccounts( limit int, ) (*apimodel.PageableResponse, gtserror.WithCode) { // Ensure list exists + is owned by requesting account. - if _, errWithCode := p.getList(ctx, account.ID, listID); errWithCode != nil { + _, errWithCode := p.getList( + // Use barebones ctx; no embedded + // structs necessary for this call. + gtscontext.SetBarebones(ctx), + account.ID, + listID, + ) + if errWithCode != nil { return nil, errWithCode } @@ -106,9 +113,12 @@ func (p *Processor) GetListAccounts( } var ( - items = make([]interface{}, count) - nextMaxIDValue string - prevMinIDValue string + items = make([]interface{}, 0, count) + + // Set next + prev values before filtering and API + // converting, so caller can still page properly. + nextMaxIDValue = listEntries[count-1].ID + prevMinIDValue = listEntries[0].ID ) // For each list entry, we want the account it points to. @@ -117,37 +127,29 @@ func (p *Processor) GetListAccounts( // from that follow. // // We do paging not by account ID, but by list entry ID. - for i, listEntry := range listEntries { - if i == count-1 { - nextMaxIDValue = listEntry.ID - } - - if i == 0 { - prevMinIDValue = listEntry.ID - } - + for _, listEntry := range listEntries { if err := p.state.DB.PopulateListEntry(ctx, listEntry); err != nil { - log.Debugf(ctx, "skipping list entry because of error populating it: %q", err) + log.Errorf(ctx, "error populating list entry: %v", err) continue } if err := p.state.DB.PopulateFollow(ctx, listEntry.Follow); err != nil { - log.Debugf(ctx, "skipping list entry because of error populating follow: %q", err) + log.Errorf(ctx, "error populating follow: %v", err) continue } apiAccount, err := p.tc.AccountToAPIAccountPublic(ctx, listEntry.Follow.TargetAccount) if err != nil { - log.Debugf(ctx, "skipping list entry because of error converting follow target account: %q", err) + log.Errorf(ctx, "error converting to public api account: %v", err) continue } - items[i] = apiAccount + items = append(items, apiAccount) } return util.PackagePageableResponse(util.PageableResponseParams{ Items: items, - Path: "api/v1/lists/" + listID + "/accounts", + Path: "/api/v1/lists/" + listID + "/accounts", NextMaxIDValue: nextMaxIDValue, PrevMinIDValue: prevMinIDValue, Limit: limit, diff --git a/internal/processing/report/get.go b/internal/processing/report/get.go index bfe7dedf..f3964883 100644 --- a/internal/processing/report/get.go +++ b/internal/processing/report/get.go @@ -19,6 +19,7 @@ package report import ( "context" + "errors" "fmt" "strconv" @@ -64,31 +65,24 @@ func (p *Processor) GetMultiple( limit int, ) (*apimodel.PageableResponse, gtserror.WithCode) { reports, err := p.state.DB.GetReports(ctx, resolved, account.ID, 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) + if count == 0 { + return util.EmptyPageableResponse(), nil + } + items := make([]interface{}, 0, count) - nextMaxIDValue := "" - prevMinIDValue := "" - for i, r := range reports { + nextMaxIDValue := reports[count-1].ID + prevMinIDValue := reports[0].ID + + for _, r := range reports { item, err := p.tc.ReportToAPIReport(ctx, r) if err != nil { return nil, gtserror.NewErrorInternalError(fmt.Errorf("error converting report to api: %s", err)) } - - if i == count-1 { - nextMaxIDValue = item.ID - } - - if i == 0 { - prevMinIDValue = item.ID - } - items = append(items, item) } diff --git a/internal/processing/timeline/faved.go b/internal/processing/timeline/faved.go index 0fc92d8f..556ced4c 100644 --- a/internal/processing/timeline/faved.go +++ b/internal/processing/timeline/faved.go @@ -46,7 +46,7 @@ func (p *Processor) FavedTimelineGet(ctx context.Context, authed *oauth.Auth, ma for _, s := range statuses { visible, err := p.filter.StatusVisible(ctx, authed.Account, s) if err != nil { - log.Debugf(ctx, "skipping status %s because of an error checking status visibility: %s", s.ID, err) + log.Errorf(ctx, "error checking status visibility: %v", err) continue } @@ -56,7 +56,7 @@ func (p *Processor) FavedTimelineGet(ctx context.Context, authed *oauth.Auth, ma apiStatus, err := p.tc.StatusToAPIStatus(ctx, s, authed.Account) if err != nil { - log.Debugf(ctx, "skipping status %s because it couldn't be converted to its api representation: %s", s.ID, err) + log.Errorf(ctx, "error convering to api status: %v", err) continue } @@ -65,7 +65,7 @@ func (p *Processor) FavedTimelineGet(ctx context.Context, authed *oauth.Auth, ma return util.PackagePageableResponse(util.PageableResponseParams{ Items: items, - Path: "api/v1/favourites", + Path: "/api/v1/favourites", NextMaxIDValue: nextMaxID, PrevMinIDValue: prevMinID, Limit: limit, diff --git a/internal/processing/timeline/home.go b/internal/processing/timeline/home.go index 80718949..72940175 100644 --- a/internal/processing/timeline/home.go +++ b/internal/processing/timeline/home.go @@ -116,25 +116,17 @@ func (p *Processor) HomeTimelineGet(ctx context.Context, authed *oauth.Auth, max var ( items = make([]interface{}, count) - nextMaxIDValue string - prevMinIDValue string + nextMaxIDValue = statuses[count-1].GetID() + prevMinIDValue = statuses[0].GetID() ) - for i, item := range statuses { - if i == count-1 { - nextMaxIDValue = item.GetID() - } - - if i == 0 { - prevMinIDValue = item.GetID() - } - - items[i] = item + for i := range statuses { + items[i] = statuses[i] } return util.PackagePageableResponse(util.PageableResponseParams{ Items: items, - Path: "api/v1/timelines/home", + Path: "/api/v1/timelines/home", NextMaxIDValue: nextMaxIDValue, PrevMinIDValue: prevMinIDValue, Limit: limit, diff --git a/internal/processing/timeline/list.go b/internal/processing/timeline/list.go index 830c874a..80744df1 100644 --- a/internal/processing/timeline/list.go +++ b/internal/processing/timeline/list.go @@ -142,25 +142,17 @@ func (p *Processor) ListTimelineGet(ctx context.Context, authed *oauth.Auth, lis var ( items = make([]interface{}, count) - nextMaxIDValue string - prevMinIDValue string + nextMaxIDValue = statuses[count-1].GetID() + prevMinIDValue = statuses[0].GetID() ) - for i, item := range statuses { - if i == count-1 { - nextMaxIDValue = item.GetID() - } - - if i == 0 { - prevMinIDValue = item.GetID() - } - - items[i] = item + for i := range statuses { + items[i] = statuses[i] } return util.PackagePageableResponse(util.PageableResponseParams{ Items: items, - Path: "api/v1/timelines/list/" + listID, + Path: "/api/v1/timelines/list/" + listID, NextMaxIDValue: nextMaxIDValue, PrevMinIDValue: prevMinIDValue, Limit: limit, diff --git a/internal/processing/timeline/public.go b/internal/processing/timeline/public.go index 67893ecf..78ca5673 100644 --- a/internal/processing/timeline/public.go +++ b/internal/processing/timeline/public.go @@ -43,25 +43,18 @@ func (p *Processor) PublicTimelineGet(ctx context.Context, authed *oauth.Auth, m } var ( - items = make([]interface{}, 0, count) - nextMaxIDValue string - prevMinIDValue string - ) + items = make([]interface{}, 0, count) - for i, s := range statuses { // Set next + prev values before filtering and API // converting, so caller can still page properly. - if i == count-1 { - nextMaxIDValue = s.ID - } - - if i == 0 { - prevMinIDValue = s.ID - } + nextMaxIDValue = statuses[count-1].ID + prevMinIDValue = statuses[0].ID + ) + for _, s := range statuses { timelineable, err := p.filter.StatusPublicTimelineable(ctx, authed.Account, s) if err != nil { - log.Debugf(ctx, "skipping status %s because of an error checking StatusPublicTimelineable: %s", s.ID, err) + log.Errorf(ctx, "error checking status visibility: %v", err) continue } @@ -71,7 +64,7 @@ func (p *Processor) PublicTimelineGet(ctx context.Context, authed *oauth.Auth, m apiStatus, err := p.tc.StatusToAPIStatus(ctx, s, authed.Account) if err != nil { - log.Debugf(ctx, "skipping status %s because it couldn't be converted to its api representation: %s", s.ID, err) + log.Errorf(ctx, "error convert to api status: %v", err) continue } @@ -80,7 +73,7 @@ func (p *Processor) PublicTimelineGet(ctx context.Context, authed *oauth.Auth, m return util.PackagePageableResponse(util.PageableResponseParams{ Items: items, - Path: "api/v1/timelines/public", + Path: "/api/v1/timelines/public", NextMaxIDValue: nextMaxIDValue, PrevMinIDValue: prevMinIDValue, Limit: limit,