[bugfix] use a much shorter refresh limit for statuses with polls (#2453)

* specifically use a much shorter refresh limit for statuses with polls

* allow specifying whether status must be upToDate in calls to Get(Visible)?TargetStatusBy_(), limit force refresh to 5 minute cooldown

* remove the PollID check from statusUpToDate()

* remove unnecessary force flag checks

* remove unused field

* check refresh status error

* use argument name 'refresh' instead of 'upToDate' to better fit with the codebase

* add statuses_poll_id_idx

* remove the definitely-not copy-pasted comment i accidentally typed out in full

* only synchronously refresh if the refresh flag is provided, otherwise do async

* fix wrong force value being provided for async

---------

Co-authored-by: tobi <tobi.smethurst@protonmail.com>

internal/processing/common/status.go

@@ -30,10 +30,12 @@ import (
 
 // GetTargetStatusBy fetches the target status with db load function, given the authorized (or, nil) requester's
 // account. This returns an approprate gtserror.WithCode accounting for not found and visibility to requester.
+// The refresh argument allows specifying whether the returned copy should be force refreshed.
 func (p *Processor) GetTargetStatusBy(
 	ctx context.Context,
 	requester *gtsmodel.Account,
 	getTargetFromDB func() (*gtsmodel.Status, error),
+	refresh bool,
 ) (
 	status *gtsmodel.Status,
 	visible bool,
@@ -61,47 +63,52 @@ func (p *Processor) GetTargetStatusBy(
 	}
 
 	if requester != nil && visible {
-		// Ensure remote status is up-to-date.
-		p.federator.RefreshStatusAsync(ctx,
-			requester.Username,
-			target,
-			nil,
-			false,
-		)
+		// We only bother refreshing if this status
+		// is visible to requester, AND there *is*
+		// a requester (i.e. request is authorized)
+		// to prevent a possible DOS vector.
+
+		if refresh {
+			// Refresh required, forcibly do synchronously.
+			_, _, err := p.federator.RefreshStatus(ctx,
+				requester.Username,
+				target,
+				nil,
+				true, // force
+			)
+			if err != nil {
+				log.Errorf(ctx, "error refreshing status: %v", err)
+			}
+		} else {
+			// Only refresh async *if* out-of-date.
+			p.federator.RefreshStatusAsync(ctx,
+				requester.Username,
+				target,
+				nil,
+				false, // force
+			)
+		}
 	}
 
 	return target, visible, nil
 }
 
-// GetTargetStatusByID is a call-through to GetTargetStatus() using the db GetStatusByID() function.
-func (p *Processor) GetTargetStatusByID(
-	ctx context.Context,
-	requester *gtsmodel.Account,
-	targetID string,
-) (
-	status *gtsmodel.Status,
-	visible bool,
-	errWithCode gtserror.WithCode,
-) {
-	return p.GetTargetStatusBy(ctx, requester, func() (*gtsmodel.Status, error) {
-		return p.state.DB.GetStatusByID(ctx, targetID)
-	})
-}
-
-// GetVisibleTargetStatus calls GetTargetStatusByID(),
+// GetVisibleTargetStatus calls GetTargetStatusBy(),
 // but converts a non-visible result to not-found error.
-func (p *Processor) GetVisibleTargetStatus(
+func (p *Processor) GetVisibleTargetStatusBy(
 	ctx context.Context,
 	requester *gtsmodel.Account,
-	targetID string,
+	getTargetFromDB func() (*gtsmodel.Status, error),
+	refresh bool,
 ) (
 	status *gtsmodel.Status,
 	errWithCode gtserror.WithCode,
 ) {
 	// Fetch the target status by ID from the database.
-	target, visible, errWithCode := p.GetTargetStatusByID(ctx,
+	target, visible, errWithCode := p.GetTargetStatusBy(ctx,
 		requester,
-		targetID,
+		getTargetFromDB,
+		refresh,
 	)
 	if errWithCode != nil {
 		return nil, errWithCode
@@ -119,6 +126,22 @@ func (p *Processor) GetVisibleTargetStatus(
 	return target, nil
 }
 
+// GetVisibleTargetStatus calls GetVisibleTargetStatusBy(),
+// passing in a database function that fetches by status ID.
+func (p *Processor) GetVisibleTargetStatus(
+	ctx context.Context,
+	requester *gtsmodel.Account,
+	targetID string,
+	refresh bool,
+) (
+	status *gtsmodel.Status,
+	errWithCode gtserror.WithCode,
+) {
+	return p.GetVisibleTargetStatusBy(ctx, requester, func() (*gtsmodel.Status, error) {
+		return p.state.DB.GetStatusByID(ctx, targetID)
+	}, refresh)
+}
+
 // UnwrapIfBoost "unwraps" the given status if
 // it's a boost wrapper, by returning the boosted
 // status it targets (pending visibility checks).
@@ -132,9 +155,10 @@ func (p *Processor) UnwrapIfBoost(
 	if status.BoostOfID == "" {
 		return status, nil
 	}
-
 	return p.GetVisibleTargetStatus(ctx,
-		requester, status.BoostOfID,
+		requester,
+		status.BoostOfID,
+		false,
 	)
 }
 

internal/processing/fedi/status.go

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

internal/processing/polls/poll.go

@@ -19,11 +19,8 @@ package polls
 
 import (
 	"context"
-	"errors"
 
 	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"
-	"github.com/superseriousbusiness/gotosocial/internal/db"
-	"github.com/superseriousbusiness/gotosocial/internal/gtscontext"
 	"github.com/superseriousbusiness/gotosocial/internal/gtserror"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 	"github.com/superseriousbusiness/gotosocial/internal/processing/common"
@@ -48,35 +45,24 @@ func New(common *common.Processor, state *state.State, converter *typeutils.Conv
 }
 
 // getTargetPoll fetches a target poll ID for requesting account, taking visibility of the poll's originating status into account.
-func (p *Processor) getTargetPoll(ctx context.Context, requestingAccount *gtsmodel.Account, targetID string) (*gtsmodel.Poll, gtserror.WithCode) {
-	// Load the requested poll with ID.
-	// (barebones as we fetch status below)
-	poll, err := p.state.DB.GetPollByID(
-		gtscontext.SetBarebones(ctx),
-		targetID,
+func (p *Processor) getTargetPoll(ctx context.Context, requester *gtsmodel.Account, targetID string) (*gtsmodel.Poll, gtserror.WithCode) {
+	// Load the status the poll is attached to by the poll ID,
+	// checking for visibility and ensuring it is up-to-date.
+	status, errWithCode := p.c.GetVisibleTargetStatusBy(ctx,
+		requester,
+		func() (*gtsmodel.Status, error) {
+			return p.state.DB.GetStatusByPollID(ctx, targetID)
+		},
+		true, // refresh
 	)
-	if err != nil && !errors.Is(err, db.ErrNoEntries) {
-		return nil, gtserror.NewErrorInternalError(err)
-	}
-
-	if poll == nil {
-		// No poll could be found for given ID.
-		const text = "target poll not found"
-		return nil, gtserror.NewErrorNotFound(
-			errors.New(text),
-			text,
-		)
-	}
-
-	// Check that we can see + fetch the originating status for requesting account.
-	status, errWithCode := p.c.GetVisibleTargetStatus(ctx, requestingAccount, poll.StatusID)
 	if errWithCode != nil {
 		return nil, errWithCode
 	}
 
-	// Update poll status.
+	// Return most up-to-date
+	// copy of the status poll.
+	poll := status.Poll
 	poll.Status = status
-
 	return poll, nil
 }
 

internal/processing/status/bookmark.go

@@ -30,7 +30,11 @@ import (
 )
 
 func (p *Processor) getBookmarkableStatus(ctx context.Context, requestingAccount *gtsmodel.Account, targetStatusID string) (*gtsmodel.Status, string, gtserror.WithCode) {
-	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx, requestingAccount, targetStatusID)
+	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx,
+		requestingAccount,
+		targetStatusID,
+		false, // refresh
+	)
 	if errWithCode != nil {
 		return nil, "", errWithCode
 	}

internal/processing/status/boost.go

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

internal/processing/status/fave.go

@@ -47,6 +47,7 @@ func (p *Processor) getFaveableStatus(
 		ctx,
 		requester,
 		targetID,
+		false, // refresh
 	)
 	if errWithCode != nil {
 		return nil, nil, errWithCode
@@ -149,7 +150,11 @@ func (p *Processor) FaveRemove(ctx context.Context, requestingAccount *gtsmodel.
 
 // FavedBy returns a slice of accounts that have liked the given status, filtered according to privacy settings.
 func (p *Processor) FavedBy(ctx context.Context, requestingAccount *gtsmodel.Account, targetStatusID string) ([]*apimodel.Account, gtserror.WithCode) {
-	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx, requestingAccount, targetStatusID)
+	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx,
+		requestingAccount,
+		targetStatusID,
+		false, // refresh
+	)
 	if errWithCode != nil {
 		return nil, errWithCode
 	}

internal/processing/status/get.go

@@ -28,7 +28,11 @@ import (
 
 // Get gets the given status, taking account of privacy settings and blocks etc.
 func (p *Processor) Get(ctx context.Context, requestingAccount *gtsmodel.Account, targetStatusID string) (*apimodel.Status, gtserror.WithCode) {
-	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx, requestingAccount, targetStatusID)
+	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx,
+		requestingAccount,
+		targetStatusID,
+		false, // refresh
+	)
 	if errWithCode != nil {
 		return nil, errWithCode
 	}
@@ -38,7 +42,11 @@ func (p *Processor) Get(ctx context.Context, requestingAccount *gtsmodel.Account
 
 // WebGet gets the given status for web use, taking account of privacy settings.
 func (p *Processor) WebGet(ctx context.Context, targetStatusID string) (*apimodel.Status, gtserror.WithCode) {
-	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx, nil, targetStatusID)
+	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx,
+		nil, // requester
+		targetStatusID,
+		false, // refresh
+	)
 	if errWithCode != nil {
 		return nil, errWithCode
 	}
@@ -57,7 +65,11 @@ func (p *Processor) contextGet(
 	targetStatusID string,
 	convert func(context.Context, *gtsmodel.Status, *gtsmodel.Account) (*apimodel.Status, error),
 ) (*apimodel.Context, gtserror.WithCode) {
-	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx, requestingAccount, targetStatusID)
+	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx,
+		requestingAccount,
+		targetStatusID,
+		false, // refresh
+	)
 	if errWithCode != nil {
 		return nil, errWithCode
 	}

internal/processing/status/mute.go

@@ -41,7 +41,11 @@ func (p *Processor) getMuteableStatus(
 	requestingAccount *gtsmodel.Account,
 	targetStatusID string,
 ) (*gtsmodel.Status, gtserror.WithCode) {
-	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx, requestingAccount, targetStatusID)
+	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx,
+		requestingAccount,
+		targetStatusID,
+		false, // refresh
+	)
 	if errWithCode != nil {
 		return nil, errWithCode
 	}

internal/processing/status/pin.go

@@ -39,7 +39,11 @@ const allowedPinnedCount = 10
 //   - Status is public, unlisted, or followers-only.
 //   - Status is not a boost.
 func (p *Processor) getPinnableStatus(ctx context.Context, requestingAccount *gtsmodel.Account, targetStatusID string) (*gtsmodel.Status, gtserror.WithCode) {
-	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx, requestingAccount, targetStatusID)
+	targetStatus, errWithCode := p.c.GetVisibleTargetStatus(ctx,
+		requestingAccount,
+		targetStatusID,
+		false, // refresh
+	)
 	if errWithCode != nil {
 		return nil, errWithCode
 	}