From ca5492b65f45c7db8a9cfb767b0b48aa6cf6fe24 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Mon, 10 Jul 2023 17:05:59 +0200 Subject: [bugfix] Tidy up rss feed serving; don't error on empty feed (#1970) * [bugfix] Tidy up rss feed serving; don't error on empty feed * fall back to account creation time as rss feed update time * return feed early when account has no eligible statuses --- internal/processing/account/rss.go | 163 ++++++++++++++++++++++++++----------- 1 file changed, 116 insertions(+), 47 deletions(-) (limited to 'internal/processing/account/rss.go') diff --git a/internal/processing/account/rss.go b/internal/processing/account/rss.go index 9b5773809..ddc07b9ad 100644 --- a/internal/processing/account/rss.go +++ b/internal/processing/account/rss.go @@ -27,82 +27,151 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/config" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtserror" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" ) -const rssFeedLength = 20 +const ( + rssFeedLength = 20 +) + +type GetRSSFeed func() (string, gtserror.WithCode) + +// GetRSSFeedForUsername returns a function to return the RSS feed of a local account +// with the given username, and the last-modified time (time that the account last +// posted a status eligible to be included in the rss feed). +// +// To save db calls, callers to this function should only call the returned GetRSSFeed +// func if the last-modified time is newer than the last-modified time they have cached. +// +// If the account has not yet posted an RSS-eligible status, the returned last-modified +// time will be zero, and the GetRSSFeed func will return a valid RSS xml with no items. +func (p *Processor) GetRSSFeedForUsername(ctx context.Context, username string) (GetRSSFeed, time.Time, gtserror.WithCode) { + var ( + never = time.Time{} + ) -// GetRSSFeedForUsername returns RSS feed for the given local username. -func (p *Processor) GetRSSFeedForUsername(ctx context.Context, username string) (func() (string, gtserror.WithCode), time.Time, gtserror.WithCode) { account, err := p.state.DB.GetAccountByUsernameDomain(ctx, username, "") if err != nil { - if err == db.ErrNoEntries { - return nil, time.Time{}, gtserror.NewErrorNotFound(errors.New("GetRSSFeedForUsername: account not found")) + if errors.Is(err, db.ErrNoEntries) { + // Simply no account with this username. + err = gtserror.New("account not found") + return nil, never, gtserror.NewErrorNotFound(err) } - return nil, time.Time{}, gtserror.NewErrorInternalError(fmt.Errorf("GetRSSFeedForUsername: db error: %s", err)) + + // Real db error. + err = gtserror.Newf("db error getting account %s: %w", username, err) + return nil, never, gtserror.NewErrorInternalError(err) } + // Ensure account has rss feed enabled. if !*account.EnableRSS { - return nil, time.Time{}, gtserror.NewErrorNotFound(errors.New("GetRSSFeedForUsername: account RSS feed not enabled")) + err = gtserror.New("account RSS feed not enabled") + return nil, never, gtserror.NewErrorNotFound(err) } - lastModified, err := p.state.DB.GetAccountLastPosted(ctx, account.ID, true) - if err != nil { - return nil, time.Time{}, gtserror.NewErrorInternalError(fmt.Errorf("GetRSSFeedForUsername: db error: %s", err)) + // LastModified time is needed by callers to check freshness for cacheing. + // This might be a zero time.Time if account has never posted a status that's + // eligible to appear in the RSS feed; that's fine. + lastPostAt, err := p.state.DB.GetAccountLastPosted(ctx, account.ID, true) + if err != nil && !errors.Is(err, db.ErrNoEntries) { + err = gtserror.Newf("db error getting account %s last posted: %w", username, err) + return nil, never, gtserror.NewErrorInternalError(err) } return func() (string, gtserror.WithCode) { - statuses, err := p.state.DB.GetAccountWebStatuses(ctx, account.ID, rssFeedLength, "") - if err != nil && err != db.ErrNoEntries { - return "", gtserror.NewErrorInternalError(fmt.Errorf("GetRSSFeedForUsername: db error: %s", err)) - } - + // Assemble author namestring once only. author := "@" + account.Username + "@" + config.GetAccountDomain() - title := "Posts from " + author - description := "Posts from " + author - link := &feeds.Link{Href: account.URL} - - var image *feeds.Image - if account.AvatarMediaAttachmentID != "" { - if account.AvatarMediaAttachment == nil { - avatar, err := p.state.DB.GetAttachmentByID(ctx, account.AvatarMediaAttachmentID) - if err != nil { - return "", gtserror.NewErrorInternalError(fmt.Errorf("GetRSSFeedForUsername: db error fetching avatar attachment: %s", err)) - } - account.AvatarMediaAttachment = avatar - } - image = &feeds.Image{ - Url: account.AvatarMediaAttachment.Thumbnail.URL, - Title: "Avatar for " + author, - Link: account.URL, - } + + // Derive image/thumbnail for this account (may be nil). + image, errWithCode := p.rssImageForAccount(ctx, account, author) + if errWithCode != nil { + return "", errWithCode } feed := &feeds.Feed{ - Title: title, - Description: description, - Link: link, + Title: "Posts from " + author, + Description: "Posts from " + author, + Link: &feeds.Link{Href: account.URL}, Image: image, } - for i, s := range statuses { - // take the date of the first (ie., latest) status as feed updated value - if i == 0 { - feed.Updated = s.UpdatedAt - } + // If the account has never posted anything, just use + // account creation time as Updated value for the feed; + // we could use time.Now() here but this would likely + // mess up cacheing; we want something determinate. + // + // We can also return early rather than wasting a db call, + // since we already know there's no eligible statuses. + if lastPostAt.IsZero() { + feed.Updated = account.CreatedAt + return stringifyFeed(feed) + } + + // Account has posted at least one status that's + // eligible to appear in the RSS feed. + // + // Reuse the lastPostAt value for feed.Updated. + feed.Updated = lastPostAt - item, err := p.tc.StatusToRSSItem(ctx, s) + // Retrieve latest statuses as they'd be shown on the web view of the account profile. + statuses, err := p.state.DB.GetAccountWebStatuses(ctx, account.ID, rssFeedLength, "") + if err != nil && !errors.Is(err, db.ErrNoEntries) { + err = fmt.Errorf("db error getting account web statuses: %w", err) + return "", gtserror.NewErrorInternalError(err) + } + + // Add each status to the rss feed. + for _, status := range statuses { + item, err := p.tc.StatusToRSSItem(ctx, status) if err != nil { - return "", gtserror.NewErrorInternalError(fmt.Errorf("GetRSSFeedForUsername: error converting status to feed item: %s", err)) + err = gtserror.Newf("error converting status to feed item: %w", err) + return "", gtserror.NewErrorInternalError(err) } feed.Add(item) } - rss, err := feed.ToRss() + return stringifyFeed(feed) + }, lastPostAt, nil +} + +func (p *Processor) rssImageForAccount(ctx context.Context, account *gtsmodel.Account, author string) (*feeds.Image, gtserror.WithCode) { + if account.AvatarMediaAttachmentID == "" { + // No image, no problem! + return nil, nil + } + + // Ensure account avatar attachment populated. + if account.AvatarMediaAttachment == nil { + var err error + account.AvatarMediaAttachment, err = p.state.DB.GetAttachmentByID(ctx, account.AvatarMediaAttachmentID) if err != nil { - return "", gtserror.NewErrorInternalError(fmt.Errorf("GetRSSFeedForUsername: error converting feed to rss string: %s", err)) + if errors.Is(err, db.ErrNoEntries) { + // No attachment found with this ID (race condition?). + return nil, nil + } + + // Real db error. + err = gtserror.Newf("db error fetching avatar media attachment: %w", err) + return nil, gtserror.NewErrorInternalError(err) } + } + + return &feeds.Image{ + Url: account.AvatarMediaAttachment.Thumbnail.URL, + Title: "Avatar for " + author, + Link: account.URL, + }, nil +} + +func stringifyFeed(feed *feeds.Feed) (string, gtserror.WithCode) { + // Stringify the feed. Even with no statuses, + // this will still produce valid rss xml. + rss, err := feed.ToRss() + if err != nil { + err := gtserror.Newf("error converting feed to rss string: %w", err) + return "", gtserror.NewErrorInternalError(err) + } - return rss, nil - }, lastModified, nil + return rss, nil } -- cgit v1.2.3