use pointer for freshness window (#2614)

internal/federation/dereferencing/account.go

@@ -40,41 +40,55 @@
 	"github.com/superseriousbusiness/gotosocial/internal/util"
 )
 
-// accountUpToDate returns whether the given account model is both updateable (i.e.
+// accountFresh returns true if the given account is
-// non-instance remote account) and whether it needs an update based on `fetched_at`.
+// still considered "fresh" according to the desired
-func accountUpToDate(account *gtsmodel.Account, force bool) bool {
+// freshness window (falls back to default if nil).
-	if !account.SuspendedAt.IsZero() {
+//
-		// Can't update suspended accounts.
+// Local accounts will always be considered fresh because
-		return true
+// there's no remote state that could have changed.
+//
+// True is also returned for suspended accounts, since
+// we'll never want to try to refresh one of these.
+//
+// Return value of false indicates that the account
+// is not fresh and should be refreshed from remote.
+func accountFresh(
+	account *gtsmodel.Account,
+	window *FreshnessWindow,
+) bool {
+	if window == nil {
+		window = DefaultAccountFreshness
 	}
 
 	if account.IsLocal() {
-		// Can't update local accounts.
+		// Can't refresh
+		// local accounts.
 		return true
 	}
 
-	if account.IsInstance() && !account.IsNew() {
+	if !account.SuspendedAt.IsZero() {
-		// Existing instance account. No need for update.
+		// Can't refresh
+		// suspended accounts.
 		return true
 	}
 
-	// Default limit we allow
+	if account.IsInstance() &&
-	// statuses to be refreshed.
+		!account.IsNew() {
-	limit := 6 * time.Hour
+		// Existing instance account.
-
+		// No need for refresh.
-	if force {
-		// We specifically allow the force flag
-		// to force an early refresh (on a much
-		// smaller cooldown period).
-		limit = 5 * time.Minute
-	}
-
-	// If account was updated recently (within limit), we return as-is.
-	if next := account.FetchedAt.Add(limit); time.Now().Before(next) {
 		return true
 	}
 
-	return false
+	// Moment when the account is
+	// considered stale according to
+	// desired freshness window.
+	staleAt := account.FetchedAt.Add(
+		time.Duration(*window),
+	)
+
+	// It's still fresh if the time now
+	// is not past the point of staleness.
+	return !time.Now().After(staleAt)
 }
 
 // GetAccountByURI will attempt to fetch an accounts by its URI, first checking the database. In the case of a newly-met remote model, or a remote model
@@ -146,7 +160,7 @@ func (d *Dereferencer) getAccountByURI(ctx context.Context, requestUser string,
 		}, nil)
 	}
 
-	if accountUpToDate(account, false) {
+	if accountFresh(account, nil) {
 		// This is an existing account that is up-to-date,
 		// before returning ensure it is fully populated.
 		if err := d.state.DB.PopulateAccount(ctx, account); err != nil {
@@ -248,7 +262,7 @@ func (d *Dereferencer) getAccountByUsernameDomain(
 		requestUser,
 		account,
 		nil,
-		false,
+		nil,
 	)
 	if err != nil {
 		// Fallback to existing.
@@ -265,22 +279,28 @@ func (d *Dereferencer) getAccountByUsernameDomain(
 	return latest, accountable, nil
 }
 
-// RefreshAccount updates the given account if remote and last_fetched is
+// RefreshAccount updates the given account if it's a
-// beyond fetch interval, or if force is set. An updated account model is
+// remote account, and considered stale / not fresh
-// returned, but in the case of dereferencing, some low-priority account info
+// based on Account.FetchedAt and desired freshness.
-// may be enqueued for asynchronous fetching, e.g. featured account statuses (pins).
+//
-// An ActivityPub object indicates the account was dereferenced (i.e. updated).
+// An updated account model is returned, but in the
+// case of dereferencing, some low-priority account
+// info may be enqueued for asynchronous fetching,
+// e.g. featured account statuses (pins).
+//
+// An ActivityPub object indicates the account was
+// dereferenced (i.e. updated).
 func (d *Dereferencer) RefreshAccount(
 	ctx context.Context,
 	requestUser string,
 	account *gtsmodel.Account,
 	accountable ap.Accountable,
-	force bool,
+	window *FreshnessWindow,
 ) (*gtsmodel.Account, ap.Accountable, error) {
 	// If no incoming data is provided,
-	// check whether account needs update.
+	// check whether account needs refresh.
 	if accountable == nil &&
-		accountUpToDate(account, force) {
+		accountFresh(account, window) {
 		return account, nil, nil
 	}
 
@@ -314,21 +334,25 @@ func (d *Dereferencer) RefreshAccount(
 	return latest, accountable, nil
 }
 
-// RefreshAccountAsync enqueues the given account for an asychronous
+// RefreshAccountAsync enqueues the given account for
-// update fetching, if last_fetched is beyond fetch interval, or if force
+// an asychronous update fetching, if it's a remote
-// is set. This is a more optimized form of manually enqueueing .UpdateAccount()
+// account, and considered stale / not fresh based on
-// to the federation worker, since it only enqueues update if necessary.
+// Account.FetchedAt and desired freshness.
+//
+// This is a more optimized form of manually enqueueing
+// .UpdateAccount() to the federation worker, since it
+// only enqueues update if necessary.
 func (d *Dereferencer) RefreshAccountAsync(
 	ctx context.Context,
 	requestUser string,
 	account *gtsmodel.Account,
 	accountable ap.Accountable,
-	force bool,
+	window *FreshnessWindow,
 ) {
 	// If no incoming data is provided,
-	// check whether account needs update.
+	// check whether account needs refresh.
 	if accountable == nil &&
-		accountUpToDate(account, force) {
+		accountFresh(account, window) {
 		return
 	}
 

internal/federation/dereferencing/dereferencer.go

@@ -20,11 +20,49 @@
 import (
 	"net/url"
 	"sync"
+	"time"
 
 	"github.com/superseriousbusiness/gotosocial/internal/media"
 	"github.com/superseriousbusiness/gotosocial/internal/state"
 	"github.com/superseriousbusiness/gotosocial/internal/transport"
 	"github.com/superseriousbusiness/gotosocial/internal/typeutils"
+	"github.com/superseriousbusiness/gotosocial/internal/util"
+)
+
+// FreshnessWindow represents a duration in which a
+// Status or Account is still considered to be "fresh"
+// (ie., not in need of a refresh from remote), if its
+// last FetchedAt value falls within the window.
+//
+// For example, if an Account was FetchedAt 09:00, and it
+// is now 12:00, then it would be considered "fresh"
+// according to DefaultAccountFreshness, but not according
+// to Fresh, which would indicate that the Account requires
+// refreshing from remote.
+type FreshnessWindow time.Duration
+
+var (
+	// 6 hours.
+	//
+	// Default window for doing a
+	// fresh dereference of an Account.
+	DefaultAccountFreshness = util.Ptr(FreshnessWindow(6 * time.Hour))
+
+	// 2 hours.
+	//
+	// Default window for doing a
+	// fresh dereference of a Status.
+	DefaultStatusFreshness = util.Ptr(FreshnessWindow(2 * time.Hour))
+
+	// 5 minutes.
+	//
+	// Fresh is useful when you're wanting
+	// a more up-to-date model of something
+	// that exceeds default freshness windows.
+	//
+	// This is tuned to be quite fresh without
+	// causing loads of dereferencing calls.
+	Fresh = util.Ptr(FreshnessWindow(5 * time.Minute))
 )
 
 // Dereferencer wraps logic and functionality for doing dereferencing

internal/federation/dereferencing/status.go

@@ -38,31 +38,41 @@
 	"github.com/superseriousbusiness/gotosocial/internal/util"
 )
 
-// statusUpToDate returns whether the given status model is both updateable
+// statusFresh returns true if the given status is still
-// (i.e. remote status) and whether it needs an update based on `fetched_at`.
+// considered "fresh" according to the desired freshness
-func statusUpToDate(status *gtsmodel.Status, force bool) bool {
+// window (falls back to default status freshness if nil).
-	if status.Local != nil && *status.Local {
+//
-		// Can't update local statuses.
+// Local statuses will always be considered fresh,
+// because there's no remote state that may have changed.
+//
+// Return value of false indicates that the status
+// is not fresh and should be refreshed from remote.
+func statusFresh(
+	status *gtsmodel.Status,
+	window *FreshnessWindow,
+) bool {
+	// Take default if no
+	// freshness window preferred.
+	if window == nil {
+		window = DefaultStatusFreshness
+	}
+
+	if status.IsLocal() {
+		// Can't refresh
+		// local statuses.
 		return true
 	}
 
-	// Default limit we allow
+	// Moment when the status is
-	// statuses to be refreshed.
+	// considered stale according to
-	limit := 2 * time.Hour
+	// desired freshness window.
+	staleAt := status.FetchedAt.Add(
+		time.Duration(*window),
+	)
 
-	if force {
+	// It's still fresh if the time now
-		// We specifically allow the force flag
+	// is not past the point of staleness.
-		// to force an early refresh (on a much
+	return !time.Now().After(staleAt)
-		// smaller cooldown period).
-		limit = 5 * time.Minute
-	}
-
-	// If this status was updated recently (within limit), return as-is.
-	if next := status.FetchedAt.Add(limit); time.Now().Before(next) {
-		return true
-	}
-
-	return false
 }
 
 // GetStatusByURI will attempt to fetch a status by its URI, first checking the database. In the case of a newly-met remote model, or a remote model whose 'last_fetched' date
@@ -146,7 +156,7 @@ func (d *Dereferencer) getStatusByURI(ctx context.Context, requestUser string, u
 		}, nil)
 	}
 
-	if statusUpToDate(status, false) {
+	if statusFresh(status, DefaultStatusFreshness) {
 		// This is an existing status that is up-to-date,
 		// before returning ensure it is fully populated.
 		if err := d.state.DB.PopulateStatus(ctx, status); err != nil {
@@ -181,12 +191,12 @@ func (d *Dereferencer) RefreshStatus(
 	requestUser string,
 	status *gtsmodel.Status,
 	statusable ap.Statusable,
-	force bool,
+	window *FreshnessWindow,
 ) (*gtsmodel.Status, ap.Statusable, error) {
 	// If no incoming data is provided,
 	// check whether status needs update.
 	if statusable == nil &&
-		statusUpToDate(status, force) {
+		statusFresh(status, window) {
 		return status, nil, nil
 	}
 
@@ -228,12 +238,12 @@ func (d *Dereferencer) RefreshStatusAsync(
 	requestUser string,
 	status *gtsmodel.Status,
 	statusable ap.Statusable,
-	force bool,
+	window *FreshnessWindow,
 ) {
 	// If no incoming data is provided,
 	// check whether status needs update.
 	if statusable == nil &&
-		statusUpToDate(status, force) {
+		statusFresh(status, window) {
 		return
 	}
 

internal/gtsmodel/status.go

@@ -205,6 +205,12 @@ func (s *Status) BelongsToAccount(accountID string) bool {
 	return s.AccountID == accountID
 }
 
+// IsLocal returns true if this is a local
+// status (ie., originating from this instance).
+func (s *Status) IsLocal() bool {
+	return s.Local != nil && *s.Local
+}
+
 // StatusToTag is an intermediate struct to facilitate the many2many relationship between a status and one or more tags.
 type StatusToTag struct {
 	StatusID string  `bun:"type:CHAR(26),unique:statustag,nullzero,notnull"`

internal/processing/common/account.go

@@ -66,7 +66,7 @@ func (p *Processor) GetTargetAccountBy(
 			requester.Username,
 			target,
 			nil,
-			false,
+			nil,
 		)
 	}
 

internal/processing/common/status.go

@@ -23,19 +23,24 @@
 
 	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"
 	"github.com/superseriousbusiness/gotosocial/internal/db"
+	"github.com/superseriousbusiness/gotosocial/internal/federation/dereferencing"
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 	"github.com/superseriousbusiness/gotosocial/internal/log"
 )
 
-// GetTargetStatusBy fetches the target status with db load function, given the authorized (or, nil) requester's
+// GetTargetStatusBy fetches the target status with db load
-// account. This returns an approprate gtserror.WithCode accounting for not found and visibility to requester.
+// function, given the authorized (or, nil) requester's
-// The refresh argument allows specifying whether the returned copy should be force refreshed.
+// account. This returns an approprate gtserror.WithCode
+// accounting for not found and visibility to requester.
+//
+// window can be used to force refresh of the target if it's
+// deemed to be stale. Falls back to default window if nil.
 func (p *Processor) GetTargetStatusBy(
 	ctx context.Context,
 	requester *gtsmodel.Account,
 	getTargetFromDB func() (*gtsmodel.Status, error),
-	refresh bool,
+	window *dereferencing.FreshnessWindow,
 ) (
 	status *gtsmodel.Status,
 	visible bool,
@@ -68,13 +73,15 @@ func (p *Processor) GetTargetStatusBy(
 		// a requester (i.e. request is authorized)
 		// to prevent a possible DOS vector.
 
-		if refresh {
+		if window != nil {
-			// Refresh required, forcibly do synchronously.
+			// Window is explicitly set, so likely
+			// tighter than the default window.
+			// Do refresh synchronously.
 			_, _, err := p.federator.RefreshStatus(ctx,
 				requester.Username,
 				target,
 				nil,
-				true, // force
+				window,
 			)
 			if err != nil {
 				log.Errorf(ctx, "error refreshing status: %v", err)
@@ -85,7 +92,7 @@ func (p *Processor) GetTargetStatusBy(
 				requester.Username,
 				target,
 				nil,
-				false, // force
+				nil,
 			)
 		}
 	}
@@ -95,11 +102,14 @@ func (p *Processor) GetTargetStatusBy(
 
 // GetVisibleTargetStatus calls GetTargetStatusBy(),
 // but converts a non-visible result to not-found error.
+//
+// window can be used to force refresh of the target if it's
+// deemed to be stale. Falls back to default window if nil.
 func (p *Processor) GetVisibleTargetStatusBy(
 	ctx context.Context,
 	requester *gtsmodel.Account,
 	getTargetFromDB func() (*gtsmodel.Status, error),
-	refresh bool,
+	window *dereferencing.FreshnessWindow,
 ) (
 	status *gtsmodel.Status,
 	errWithCode gtserror.WithCode,
@@ -108,7 +118,7 @@ func (p *Processor) GetVisibleTargetStatusBy(
 	target, visible, errWithCode := p.GetTargetStatusBy(ctx,
 		requester,
 		getTargetFromDB,
-		refresh,
+		window,
 	)
 	if errWithCode != nil {
 		return nil, errWithCode
@@ -128,18 +138,21 @@ func (p *Processor) GetVisibleTargetStatusBy(
 
 // GetVisibleTargetStatus calls GetVisibleTargetStatusBy(),
 // passing in a database function that fetches by status ID.
+//
+// window can be used to force refresh of the target if it's
+// deemed to be stale. Falls back to default window if nil.
 func (p *Processor) GetVisibleTargetStatus(
 	ctx context.Context,
 	requester *gtsmodel.Account,
 	targetID string,
-	refresh bool,
+	window *dereferencing.FreshnessWindow,
 ) (
 	status *gtsmodel.Status,
 	errWithCode gtserror.WithCode,
 ) {
 	return p.GetVisibleTargetStatusBy(ctx, requester, func() (*gtsmodel.Status, error) {
 		return p.state.DB.GetStatusByID(ctx, targetID)
-	}, refresh)
+	}, window)
 }
 
 // UnwrapIfBoost "unwraps" the given status if
@@ -158,7 +171,7 @@ func (p *Processor) UnwrapIfBoost(
 	return p.GetVisibleTargetStatus(ctx,
 		requester,
 		status.BoostOfID,
-		false,
+		nil,
 	)
 }
 

internal/processing/fedi/status.go

@@ -100,7 +100,7 @@ func (p *Processor) StatusRepliesGet(
 	status, errWithCode := p.c.GetVisibleTargetStatus(ctx,
 		requester,
 		statusID,
-		false, // refresh
+		nil, // default freshness
 	)
 	if errWithCode != nil {
 		return nil, errWithCode

internal/processing/polls/poll.go

@@ -53,7 +53,7 @@ func (p *Processor) getTargetPoll(ctx context.Context, requester *gtsmodel.Accou
 		func() (*gtsmodel.Status, error) {
 			return p.state.DB.GetStatusByPollID(ctx, targetID)
 		},
-		true, // refresh
+		nil, // default freshness
 	)
 	if errWithCode != nil {
 		return nil, errWithCode

internal/processing/status/bookmark.go

@@ -33,7 +33,7 @@ func (p *Processor) getBookmarkableStatus(ctx context.Context, requestingAccount
 	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx,
 		requestingAccount,
 		targetStatusID,
-		false, // refresh
+		nil, // default freshness
 	)
 	if errWithCode != nil {
 		return nil, "", errWithCode

internal/processing/status/boost.go

@@ -43,7 +43,7 @@ func (p *Processor) BoostCreate(
 		ctx,
 		requester,
 		targetID,
-		false, // refresh
+		nil, // default freshness
 	)
 	if errWithCode != nil {
 		return nil, errWithCode
@@ -113,7 +113,7 @@ func (p *Processor) BoostRemove(
 		ctx,
 		requester,
 		targetID,
-		false, // refresh
+		nil, // default freshness
 	)
 	if errWithCode != nil {
 		return nil, errWithCode

internal/processing/status/fave.go

@@ -47,7 +47,7 @@ func (p *Processor) getFaveableStatus(
 		ctx,
 		requester,
 		targetID,
-		false, // refresh
+		nil, // default freshness
 	)
 	if errWithCode != nil {
 		return nil, nil, errWithCode
@@ -153,7 +153,7 @@ func (p *Processor) FavedBy(ctx context.Context, requestingAccount *gtsmodel.Acc
 	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx,
 		requestingAccount,
 		targetStatusID,
-		false, // refresh
+		nil, // default freshness
 	)
 	if errWithCode != nil {
 		return nil, errWithCode

internal/processing/status/get.go

@@ -32,7 +32,7 @@ func (p *Processor) Get(ctx context.Context, requestingAccount *gtsmodel.Account
 	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx,
 		requestingAccount,
 		targetStatusID,
-		false, // refresh
+		nil, // default freshness
 	)
 	if errWithCode != nil {
 		return nil, errWithCode
@@ -46,7 +46,7 @@ func (p *Processor) WebGet(ctx context.Context, targetStatusID string) (*apimode
 	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx,
 		nil, // requester
 		targetStatusID,
-		false, // refresh
+		nil, // default freshness
 	)
 	if errWithCode != nil {
 		return nil, errWithCode
@@ -69,7 +69,7 @@ func (p *Processor) contextGet(
 	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx,
 		requestingAccount,
 		targetStatusID,
-		false, // refresh
+		nil, // default freshness
 	)
 	if errWithCode != nil {
 		return nil, errWithCode

internal/processing/status/mute.go

@@ -44,7 +44,7 @@ func (p *Processor) getMuteableStatus(
 	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx,
 		requestingAccount,
 		targetStatusID,
-		false, // refresh
+		nil, // default freshness
 	)
 	if errWithCode != nil {
 		return nil, errWithCode

internal/processing/status/pin.go

@@ -42,7 +42,7 @@ func (p *Processor) getPinnableStatus(ctx context.Context, requestingAccount *gt
 	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx,
 		requestingAccount,
 		targetStatusID,
-		false, // refresh
+		nil, // default freshness
 	)
 	if errWithCode != nil {
 		return nil, errWithCode

internal/processing/workers/fromfediapi.go

@@ -23,6 +23,8 @@
 	"codeberg.org/gruf/go-kv"
 	"codeberg.org/gruf/go-logger/v2/level"
 	"github.com/superseriousbusiness/gotosocial/internal/ap"
+	"github.com/superseriousbusiness/gotosocial/internal/federation/dereferencing"
+
 	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
@@ -179,7 +181,8 @@ func (p *fediAPI) CreateStatus(ctx context.Context, fMsg messages.FromFediAPI) e
 			fMsg.ReceivingAccount.Username,
 			bareStatus,
 			statusable,
-			true,
+			// Force refresh within 5min window.
+			dereferencing.Fresh,
 		)
 		if err != nil {
 			return gtserror.Newf("error processing new status %s: %w", bareStatus.URI, err)
@@ -487,7 +490,8 @@ func (p *fediAPI) UpdateAccount(ctx context.Context, fMsg messages.FromFediAPI)
 		fMsg.ReceivingAccount.Username,
 		account,
 		apubAcc,
-		true, // Force refresh.
+		// Force refresh within 5min window.
+		dereferencing.Fresh,
 	)
 	if err != nil {
 		log.Errorf(ctx, "error refreshing account: %v", err)
@@ -512,7 +516,8 @@ func (p *fediAPI) UpdateStatus(ctx context.Context, fMsg messages.FromFediAPI) e
 		fMsg.ReceivingAccount.Username,
 		existing,
 		apStatus,
-		true,
+		// Force refresh within 5min window.
+		dereferencing.Fresh,
 	)
 	if err != nil {
 		log.Errorf(ctx, "error refreshing status: %v", err)