Bugfixerino (#133)

* fix some lil bugs

* fmt, lint

internal/federation/dereference.go

@@ -11,6 +11,10 @@ func (f *federator) GetRemoteAccount(username string, remoteAccountID *url.URL,
 	return f.dereferencer.GetRemoteAccount(username, remoteAccountID, refresh)
 }
 
+func (f *federator) EnrichRemoteAccount(username string, account *gtsmodel.Account) (*gtsmodel.Account, error) {
+	return f.dereferencer.EnrichRemoteAccount(username, account)
+}
+
 func (f *federator) GetRemoteStatus(username string, remoteStatusID *url.URL, refresh bool) (*gtsmodel.Status, ap.Statusable, bool, error) {
 	return f.dereferencer.GetRemoteStatus(username, remoteStatusID, refresh)
 }

internal/federation/dereferencing/account.go

@@ -41,7 +41,7 @@ import (
 // EnrichRemoteAccount is mostly useful for calling after an account has been initially created by
 // the federatingDB's Create function, or during the federated authorization flow.
 func (d *deref) EnrichRemoteAccount(username string, account *gtsmodel.Account) (*gtsmodel.Account, error) {
-	if err := d.populateAccountFields(account, username, false); err != nil {
+	if err := d.PopulateAccountFields(account, username, false); err != nil {
 		return nil, err
 	}
 
@@ -69,10 +69,10 @@ func (d *deref) GetRemoteAccount(username string, remoteAccountID *url.URL, refr
 	if err := d.db.GetWhere([]db.Where{{Key: "uri", Value: remoteAccountID.String()}}, maybeAccount); err == nil {
 		// we've seen this account before so it's not new
 		new = false
-
-		// if we're not being asked to refresh, we can just return the maybeAccount as-is and avoid doing any external calls
 		if !refresh {
-			return maybeAccount, new, nil
+			// we're not being asked to refresh, but just in case we don't have the avatar/header cached yet....
+			maybeAccount, err = d.EnrichRemoteAccount(username, maybeAccount)
+			return maybeAccount, new, err
 		}
 	}
 
@@ -81,7 +81,7 @@ func (d *deref) GetRemoteAccount(username string, remoteAccountID *url.URL, refr
 		return nil, new, fmt.Errorf("FullyDereferenceAccount: error dereferencing accountable: %s", err)
 	}
 
-	gtsAccount, err := d.typeConverter.ASRepresentationToAccount(accountable, false)
+	gtsAccount, err := d.typeConverter.ASRepresentationToAccount(accountable, refresh)
 	if err != nil {
 		return nil, new, fmt.Errorf("FullyDereferenceAccount: error converting accountable to account: %s", err)
 	}
@@ -94,7 +94,7 @@ func (d *deref) GetRemoteAccount(username string, remoteAccountID *url.URL, refr
 		}
 		gtsAccount.ID = ulid
 
-		if err := d.populateAccountFields(gtsAccount, username, refresh); err != nil {
+		if err := d.PopulateAccountFields(gtsAccount, username, refresh); err != nil {
 			return nil, new, fmt.Errorf("FullyDereferenceAccount: error populating further account fields: %s", err)
 		}
 
@@ -105,7 +105,7 @@ func (d *deref) GetRemoteAccount(username string, remoteAccountID *url.URL, refr
 		// take the id we already have and do an update
 		gtsAccount.ID = maybeAccount.ID
 
-		if err := d.populateAccountFields(gtsAccount, username, refresh); err != nil {
+		if err := d.PopulateAccountFields(gtsAccount, username, refresh); err != nil {
 			return nil, new, fmt.Errorf("FullyDereferenceAccount: error populating further account fields: %s", err)
 		}
 
@@ -173,9 +173,9 @@ func (d *deref) dereferenceAccountable(username string, remoteAccountID *url.URL
 	return nil, fmt.Errorf("DereferenceAccountable: type name %s not supported", t.GetTypeName())
 }
 
-// populateAccountFields populates any fields on the given account that weren't populated by the initial
+// PopulateAccountFields populates any fields on the given account that weren't populated by the initial
 // dereferencing. This includes things like header and avatar etc.
-func (d *deref) populateAccountFields(account *gtsmodel.Account, requestingUsername string, refresh bool) error {
+func (d *deref) PopulateAccountFields(account *gtsmodel.Account, requestingUsername string, refresh bool) error {
 	l := d.log.WithFields(logrus.Fields{
 		"func":               "PopulateAccountFields",
 		"requestingUsername": requestingUsername,

internal/federation/federator.go

@@ -60,6 +60,7 @@ type Federator interface {
 	DereferenceAnnounce(announce *gtsmodel.Status, requestingUsername string) error
 
 	GetRemoteAccount(username string, remoteAccountID *url.URL, refresh bool) (*gtsmodel.Account, bool, error)
+	EnrichRemoteAccount(username string, account *gtsmodel.Account) (*gtsmodel.Account, error)
 
 	GetRemoteStatus(username string, remoteStatusID *url.URL, refresh bool) (*gtsmodel.Status, ap.Statusable, bool, error)
 	EnrichRemoteStatus(username string, status *gtsmodel.Status) (*gtsmodel.Status, error)

internal/processing/account/get.go

@@ -48,7 +48,21 @@ func (p *processor) Get(requestingAccount *gtsmodel.Account, targetAccountID str
 	var mastoAccount *apimodel.Account
 	if blocked {
 		mastoAccount, err = p.tc.AccountToMastoBlocked(targetAccount)
-	} else if requestingAccount != nil && targetAccount.ID == requestingAccount.ID {
+		if err != nil {
+			return nil, fmt.Errorf("error converting account: %s", err)
+		}
+		return mastoAccount, nil
+	}
+
+	// last-minute check to make sure we have remote account header/avi cached
+	if targetAccount.Domain != "" {
+		a, err := p.federator.EnrichRemoteAccount(requestingAccount.Username, targetAccount)
+		if err == nil {
+			targetAccount = a
+		}
+	}
+
+	if requestingAccount != nil && targetAccount.ID == requestingAccount.ID {
 		mastoAccount, err = p.tc.AccountToMastoSensitive(targetAccount)
 	} else {
 		mastoAccount, err = p.tc.AccountToMastoPublic(targetAccount)

internal/timeline/get.go

@@ -27,6 +27,8 @@ import (
 	apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model"
 )
 
+const retries = 5
+
 func (t *timeline) Get(amount int, maxID string, sinceID string, minID string) ([]*apimodel.Status, error) {
 	l := t.log.WithFields(logrus.Fields{
 		"func":      "Get",
@@ -57,7 +59,8 @@ func (t *timeline) Get(amount int, maxID string, sinceID string, minID string) (
 
 	// maxID is defined but sinceID isn't so take from behind
 	if maxID != "" && sinceID == "" {
-		statuses, err = t.GetXBehindID(amount, maxID)
+		attempts := 0
+		statuses, err = t.GetXBehindID(amount, maxID, &attempts)
 		// aysnchronously prepare the next predicted query so it's ready when the user asks for it
 		if len(statuses) != 0 {
 			nextMaxID := statuses[len(statuses)-1].ID
@@ -79,10 +82,12 @@ func (t *timeline) Get(amount int, maxID string, sinceID string, minID string) (
 
 	// maxID isn't defined, but sinceID || minID are, so take x before
 	if maxID == "" && sinceID != "" {
-		statuses, err = t.GetXBeforeID(amount, sinceID, true)
+		attempts := 0
+		statuses, err = t.GetXBeforeID(amount, sinceID, true, &attempts)
 	}
 	if maxID == "" && minID != "" {
-		statuses, err = t.GetXBeforeID(amount, minID, true)
+		attempts := 0
+		statuses, err = t.GetXBeforeID(amount, minID, true, &attempts)
 	}
 
 	return statuses, err
@@ -120,7 +125,11 @@ func (t *timeline) GetXFromTop(amount int) ([]*apimodel.Status, error) {
 	return statuses, nil
 }
 
-func (t *timeline) GetXBehindID(amount int, behindID string) ([]*apimodel.Status, error) {
+func (t *timeline) GetXBehindID(amount int, behindID string, attempts *int) ([]*apimodel.Status, error) {
+	newAttempts := *attempts
+	newAttempts = newAttempts + 1
+	attempts = &newAttempts
+
 	// make a slice of statuses with the length we need to return
 	statuses := make([]*apimodel.Status, 0, amount)
 
@@ -158,12 +167,13 @@ findMarkLoop:
 		if err != nil {
 			return nil, err
 		}
-		if oldestID == "" || oldestID == behindID {
+		if oldestID == "" || oldestID == behindID || *attempts > retries {
-			// there is no oldest prepared post, or the oldest prepared post is still the post we're looking for entries after
+			// There is no oldest prepared post, or the oldest prepared post is still the post we're looking for entries after,
-			// this means we should just return the empty statuses slice since we don't have any more posts to offer
+			// or we've tried this loop too many times.
+			// This means we should just return the empty statuses slice since we don't have any more posts to offer.
 			return statuses, nil
 		}
-		return t.GetXBehindID(amount, behindID)
+		return t.GetXBehindID(amount, behindID, attempts)
 	}
 
 	// make sure we have enough posts prepared behind it to return what we're being asked for
@@ -193,7 +203,11 @@ serveloop:
 	return statuses, nil
 }
 
-func (t *timeline) GetXBeforeID(amount int, beforeID string, startFromTop bool) ([]*apimodel.Status, error) {
+func (t *timeline) GetXBeforeID(amount int, beforeID string, startFromTop bool, attempts *int) ([]*apimodel.Status, error) {
+	newAttempts := *attempts
+	newAttempts = newAttempts + 1
+	attempts = &newAttempts
+
 	// make a slice of statuses with the length we need to return
 	statuses := make([]*apimodel.Status, 0, amount)
 
@@ -224,7 +238,10 @@ findMarkLoop:
 		if err := t.PrepareBefore(beforeID, true, amount); err != nil {
 			return nil, fmt.Errorf("GetXBeforeID: error preparing before and including ID %s", beforeID)
 		}
-		return t.GetXBeforeID(amount, beforeID, startFromTop)
+		if *attempts > retries {
+			return statuses, nil
+		}
+		return t.GetXBeforeID(amount, beforeID, startFromTop, attempts)
 	}
 
 	var served int

internal/timeline/index.go

@@ -39,8 +39,9 @@ func (t *timeline) IndexBefore(statusID string, include bool, amount int) error
 		filtered = append(filtered, s)
 	}
 
+	i := 0
 grabloop:
-	for len(filtered) < amount {
+	for ; len(filtered) < amount && i < 5; i = i + 1 { // try the grabloop 5 times only
 		statuses, err := t.db.GetHomeTimelineForAccount(t.accountID, "", offsetStatus, "", amount, false)
 		if err != nil {
 			if _, ok := err.(db.ErrNoEntries); ok {
@@ -74,8 +75,9 @@ func (t *timeline) IndexBehind(statusID string, amount int) error {
 	filtered := []*gtsmodel.Status{}
 	offsetStatus := statusID
 
+	i := 0
 grabloop:
-	for len(filtered) < amount {
+	for ; len(filtered) < amount && i < 5; i = i + 1 { // try the grabloop 5 times only
 		statuses, err := t.db.GetHomeTimelineForAccount(t.accountID, offsetStatus, "", "", amount, false)
 		if err != nil {
 			if _, ok := err.(db.ErrNoEntries); ok {

internal/timeline/timeline.go

@@ -45,12 +45,12 @@ type Timeline interface {
 	// This will NOT include the status with the given ID.
 	//
 	// This corresponds to an api call to /timelines/home?max_id=WHATEVER
-	GetXBehindID(amount int, fromID string) ([]*apimodel.Status, error)
+	GetXBehindID(amount int, fromID string, attempts *int) ([]*apimodel.Status, error)
 	// GetXBeforeID returns x amount of posts up to the given id, from newest to oldest.
 	// This will NOT include the status with the given ID.
 	//
 	// This corresponds to an api call to /timelines/home?since_id=WHATEVER
-	GetXBeforeID(amount int, sinceID string, startFromTop bool) ([]*apimodel.Status, error)
+	GetXBeforeID(amount int, sinceID string, startFromTop bool, attempts *int) ([]*apimodel.Status, error)
 	// GetXBetweenID returns x amount of posts from the given maxID, up to the given id, from newest to oldest.
 	// This will NOT include the status with the given IDs.
 	//