From 0f734a24100383171a866fccc194dea578141d74 Mon Sep 17 00:00:00 2001 From: kim <89579420+NyaaaWhatsUpDoc@users.noreply.github.com> Date: Fri, 2 Aug 2024 12:15:11 +0000 Subject: [PATCH] [chore] move PopulateAccountStats() nil check often performed into function itself (#3158) * move PopulateAccountStats() nil check often performed into function itself * fix test to take in mind we don't repopulate account stats if not-nil --- internal/db/bundb/account.go | 13 ++++-- internal/db/bundb/account_test.go | 4 ++ internal/federation/dereferencing/account.go | 6 +-- internal/processing/account/rss.go | 8 ++-- internal/processing/fedi/collections.go | 24 ++++------ internal/processing/status/pin.go | 16 +++---- internal/processing/workers/util.go | 48 +++++++------------- internal/typeutils/internaltofrontend.go | 12 ++--- 8 files changed, 53 insertions(+), 78 deletions(-) diff --git a/internal/db/bundb/account.go b/internal/db/bundb/account.go index 90bf3eb65..94fd054bf 100644 --- a/internal/db/bundb/account.go +++ b/internal/db/bundb/account.go @@ -712,11 +712,9 @@ func (a *accountDB) PopulateAccount(ctx context.Context, account *gtsmodel.Accou } } - if account.Stats == nil { - // Get / Create stats for this account. - if err := a.state.DB.PopulateAccountStats(ctx, account); err != nil { - errs.Appendf("error populating account stats: %w", err) - } + // Get / Create stats for this account (handles case of already set). + if err := a.state.DB.PopulateAccountStats(ctx, account); err != nil { + errs.Appendf("error populating account stats: %w", err) } return errs.Combine() @@ -1160,6 +1158,11 @@ func (a *accountDB) UpdateAccountSettings( } func (a *accountDB) PopulateAccountStats(ctx context.Context, account *gtsmodel.Account) error { + if account.Stats != nil { + // Already populated! + return nil + } + // Fetch stats from db cache with loader callback. stats, err := a.state.Caches.DB.AccountStats.LoadOne( "AccountID", diff --git a/internal/db/bundb/account_test.go b/internal/db/bundb/account_test.go index 116ea19f0..28c9274f7 100644 --- a/internal/db/bundb/account_test.go +++ b/internal/db/bundb/account_test.go @@ -743,6 +743,10 @@ func (suite *AccountTestSuite) TestAccountStatsAll() { suite.FailNow(err.Error()) } + // Nil out account stats to allow + // db to refetch + regenerate them. + account.Stats = nil + // Get stats for a third time, they // should get regenerated now, but // only for local accounts. diff --git a/internal/federation/dereferencing/account.go b/internal/federation/dereferencing/account.go index 65436f9ea..3abf628d1 100644 --- a/internal/federation/dereferencing/account.go +++ b/internal/federation/dereferencing/account.go @@ -984,10 +984,8 @@ func (d *Dereferencer) dereferenceAccountStats( account *gtsmodel.Account, ) error { // Ensure we have a stats model for this account. - if account.Stats == nil { - if err := d.state.DB.PopulateAccountStats(ctx, account); err != nil { - return gtserror.Newf("db error getting account stats: %w", err) - } + if err := d.state.DB.PopulateAccountStats(ctx, account); err != nil { + return gtserror.Newf("db error getting account stats: %w", err) } // We want to update stats by getting remote diff --git a/internal/processing/account/rss.go b/internal/processing/account/rss.go index cbbb4875b..60f93b012 100644 --- a/internal/processing/account/rss.go +++ b/internal/processing/account/rss.go @@ -70,11 +70,9 @@ func (p *Processor) GetRSSFeedForUsername(ctx context.Context, username string) } // Ensure account stats populated. - if account.Stats == nil { - if err := p.state.DB.PopulateAccountStats(ctx, account); err != nil { - err = gtserror.Newf("db error getting account stats %s: %w", username, err) - return nil, never, gtserror.NewErrorInternalError(err) - } + if err := p.state.DB.PopulateAccountStats(ctx, account); err != nil { + err = gtserror.Newf("db error getting account stats %s: %w", username, err) + return nil, never, gtserror.NewErrorInternalError(err) } // LastModified time is needed by callers to check freshness for cacheing. diff --git a/internal/processing/fedi/collections.go b/internal/processing/fedi/collections.go index a1fc5e1f4..e81d3e269 100644 --- a/internal/processing/fedi/collections.go +++ b/internal/processing/fedi/collections.go @@ -70,11 +70,9 @@ func (p *Processor) OutboxGet( } // Ensure we have stats for this account. - if receivingAcct.Stats == nil { - if err := p.state.DB.PopulateAccountStats(ctx, receivingAcct); err != nil { - err := gtserror.Newf("error getting stats for account %s: %w", receivingAcct.ID, err) - return nil, gtserror.NewErrorInternalError(err) - } + if err := p.state.DB.PopulateAccountStats(ctx, receivingAcct); err != nil { + err := gtserror.Newf("error getting stats for account %s: %w", receivingAcct.ID, err) + return nil, gtserror.NewErrorInternalError(err) } var obj vocab.Type @@ -200,11 +198,9 @@ func (p *Processor) FollowersGet( } // Ensure we have stats for this account. - if receivingAcct.Stats == nil { - if err := p.state.DB.PopulateAccountStats(ctx, receivingAcct); err != nil { - err := gtserror.Newf("error getting stats for account %s: %w", receivingAcct.ID, err) - return nil, gtserror.NewErrorInternalError(err) - } + if err := p.state.DB.PopulateAccountStats(ctx, receivingAcct); err != nil { + err := gtserror.Newf("error getting stats for account %s: %w", receivingAcct.ID, err) + return nil, gtserror.NewErrorInternalError(err) } var obj vocab.Type @@ -314,11 +310,9 @@ func (p *Processor) FollowingGet(ctx context.Context, requestedUser string, page } // Ensure we have stats for this account. - if receivingAcct.Stats == nil { - if err := p.state.DB.PopulateAccountStats(ctx, receivingAcct); err != nil { - err := gtserror.Newf("error getting stats for account %s: %w", receivingAcct.ID, err) - return nil, gtserror.NewErrorInternalError(err) - } + if err := p.state.DB.PopulateAccountStats(ctx, receivingAcct); err != nil { + err := gtserror.Newf("error getting stats for account %s: %w", receivingAcct.ID, err) + return nil, gtserror.NewErrorInternalError(err) } var obj vocab.Type diff --git a/internal/processing/status/pin.go b/internal/processing/status/pin.go index c4a6fc3b8..3f8435e52 100644 --- a/internal/processing/status/pin.go +++ b/internal/processing/status/pin.go @@ -92,11 +92,9 @@ func (p *Processor) PinCreate(ctx context.Context, requestingAccount *gtsmodel.A } // Ensure account stats populated. - if requestingAccount.Stats == nil { - if err := p.state.DB.PopulateAccountStats(ctx, requestingAccount); err != nil { - err = gtserror.Newf("db error getting account stats: %w", err) - return nil, gtserror.NewErrorInternalError(err) - } + if err := p.state.DB.PopulateAccountStats(ctx, requestingAccount); err != nil { + err = gtserror.Newf("db error getting account stats: %w", err) + return nil, gtserror.NewErrorInternalError(err) } pinnedCount := *requestingAccount.Stats.StatusesPinnedCount @@ -157,11 +155,9 @@ func (p *Processor) PinRemove(ctx context.Context, requestingAccount *gtsmodel.A } // Ensure account stats populated. - if requestingAccount.Stats == nil { - if err := p.state.DB.PopulateAccountStats(ctx, requestingAccount); err != nil { - err = gtserror.Newf("db error getting account stats: %w", err) - return nil, gtserror.NewErrorInternalError(err) - } + if err := p.state.DB.PopulateAccountStats(ctx, requestingAccount); err != nil { + err = gtserror.Newf("db error getting account stats: %w", err) + return nil, gtserror.NewErrorInternalError(err) } targetStatus.PinnedAt = time.Time{} diff --git a/internal/processing/workers/util.go b/internal/processing/workers/util.go index 994242d37..7f6c259de 100644 --- a/internal/processing/workers/util.go +++ b/internal/processing/workers/util.go @@ -257,10 +257,8 @@ func (u *utils) incrementStatusesCount( defer unlock() // Populate stats. - if account.Stats == nil { - if err := u.state.DB.PopulateAccountStats(ctx, account); err != nil { - return gtserror.Newf("db error getting account stats: %w", err) - } + if err := u.state.DB.PopulateAccountStats(ctx, account); err != nil { + return gtserror.Newf("db error getting account stats: %w", err) } // Update stats by incrementing status @@ -288,10 +286,8 @@ func (u *utils) decrementStatusesCount( defer unlock() // Populate stats. - if account.Stats == nil { - if err := u.state.DB.PopulateAccountStats(ctx, account); err != nil { - return gtserror.Newf("db error getting account stats: %w", err) - } + if err := u.state.DB.PopulateAccountStats(ctx, account); err != nil { + return gtserror.Newf("db error getting account stats: %w", err) } // Update stats by decrementing @@ -322,10 +318,8 @@ func (u *utils) incrementFollowersCount( defer unlock() // Populate stats. - if account.Stats == nil { - if err := u.state.DB.PopulateAccountStats(ctx, account); err != nil { - return gtserror.Newf("db error getting account stats: %w", err) - } + if err := u.state.DB.PopulateAccountStats(ctx, account); err != nil { + return gtserror.Newf("db error getting account stats: %w", err) } // Update stats by incrementing followers @@ -351,10 +345,8 @@ func (u *utils) decrementFollowersCount( defer unlock() // Populate stats. - if account.Stats == nil { - if err := u.state.DB.PopulateAccountStats(ctx, account); err != nil { - return gtserror.Newf("db error getting account stats: %w", err) - } + if err := u.state.DB.PopulateAccountStats(ctx, account); err != nil { + return gtserror.Newf("db error getting account stats: %w", err) } // Update stats by decrementing @@ -385,10 +377,8 @@ func (u *utils) incrementFollowingCount( defer unlock() // Populate stats. - if account.Stats == nil { - if err := u.state.DB.PopulateAccountStats(ctx, account); err != nil { - return gtserror.Newf("db error getting account stats: %w", err) - } + if err := u.state.DB.PopulateAccountStats(ctx, account); err != nil { + return gtserror.Newf("db error getting account stats: %w", err) } // Update stats by incrementing @@ -414,10 +404,8 @@ func (u *utils) decrementFollowingCount( defer unlock() // Populate stats. - if account.Stats == nil { - if err := u.state.DB.PopulateAccountStats(ctx, account); err != nil { - return gtserror.Newf("db error getting account stats: %w", err) - } + if err := u.state.DB.PopulateAccountStats(ctx, account); err != nil { + return gtserror.Newf("db error getting account stats: %w", err) } // Update stats by decrementing @@ -448,10 +436,8 @@ func (u *utils) incrementFollowRequestsCount( defer unlock() // Populate stats. - if account.Stats == nil { - if err := u.state.DB.PopulateAccountStats(ctx, account); err != nil { - return gtserror.Newf("db error getting account stats: %w", err) - } + if err := u.state.DB.PopulateAccountStats(ctx, account); err != nil { + return gtserror.Newf("db error getting account stats: %w", err) } // Update stats by incrementing @@ -477,10 +463,8 @@ func (u *utils) decrementFollowRequestsCount( defer unlock() // Populate stats. - if account.Stats == nil { - if err := u.state.DB.PopulateAccountStats(ctx, account); err != nil { - return gtserror.Newf("db error getting account stats: %w", err) - } + if err := u.state.DB.PopulateAccountStats(ctx, account); err != nil { + return gtserror.Newf("db error getting account stats: %w", err) } // Update stats by decrementing diff --git a/internal/typeutils/internaltofrontend.go b/internal/typeutils/internaltofrontend.go index b85d24eee..682d76775 100644 --- a/internal/typeutils/internaltofrontend.go +++ b/internal/typeutils/internaltofrontend.go @@ -113,13 +113,11 @@ func (c *Converter) AccountToAPIAccountSensitive(ctx context.Context, a *gtsmode } // Ensure account stats populated. - if a.Stats == nil { - if err := c.state.DB.PopulateAccountStats(ctx, a); err != nil { - return nil, gtserror.Newf( - "error getting stats for account %s: %w", - a.ID, err, - ) - } + if err := c.state.DB.PopulateAccountStats(ctx, a); err != nil { + return nil, gtserror.Newf( + "error getting stats for account %s: %w", + a.ID, err, + ) } // Populate the account's role permissions bitmap and highlightedness from its public role.