rework GetRemoteAccount

internal/federation/dereferencing/account.go

@@ -33,6 +33,7 @@
 	"github.com/superseriousbusiness/activity/streams"
 	"github.com/superseriousbusiness/activity/streams/vocab"
 	"github.com/superseriousbusiness/gotosocial/internal/ap"
+	"github.com/superseriousbusiness/gotosocial/internal/db"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 	"github.com/superseriousbusiness/gotosocial/internal/id"
 	"github.com/superseriousbusiness/gotosocial/internal/media"
@@ -67,25 +68,10 @@ type GetRemoteAccountParams struct {
 	// If false, then the account's media and other fields will be dereferenced in the background,
 	// so only a minimal account representation will be returned by GetRemoteAccount.
 	Blocking bool
-}
+	// Whether to skip making calls to remote instances. This is useful when you want to
-
+	// quickly fetch a remote account from the database or fail, and don't want to cause
-func (d *deref) populateAccountBeforeReturn(ctx context.Context, params GetRemoteAccountParams, remoteAccount *gtsmodel.Account) (*gtsmodel.Account, error) {
+	// http requests to go flying around.
-	// make sure the account fields are populated before returning:
+	SkipResolve bool
-	// even if we're not doing a refresh, the caller might want to block
-	// until everything is loaded
-	changed, err := d.populateAccountFields(ctx, remoteAccount, params.RequestingUsername, false, params.Blocking)
-	if err != nil {
-		return nil, fmt.Errorf("GetRemoteAccount: error populating remoteAccount fields: %s", err)
-	}
-
-	if changed {
-		remoteAccount, err = d.db.UpdateAccount(ctx, remoteAccount)
-		if err != nil {
-			return nil, fmt.Errorf("GetRemoteAccount: error updating remoteAccount: %s", err)
-		}
-	}
-
-	return remoteAccount, nil
 }
 
 // GetRemoteAccount completely dereferences a remote account, converts it to a GtS model account,
@@ -97,34 +83,81 @@ func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountPar
 		accountDomain set, while making as few calls to remote instances as possible to save time and bandwidth.
 
 		There are a few different paths through this function, and the path taken depends on how much
-		initial information we are provided with via parameters, and how much information we already have stored.
+		initial information we are provided with via parameters, how much information we already have stored,
+		and what we're allowed to do according to the parameters we've been passed.
+
+		Scenario 1: We're not allowed to resolve remotely, but we've got either the account URI or the
+		            account username + host, so we can check in our database and return if possible.
+
+		Scenario 2: We are allowed to resolve remotely, and we have an account URI but no username or host.
+		            In this case, we can use the URI to resolve the remote account and find the username,
+					and then we can webfinger the account to discover the accountDomain.
+
+		Scenario 3: We are allowed to resolve remotely, and we have the username and host but no URI.
+		            In this case, we webfinger the account to discover the URI. We then check in the db
+
 	*/
 
-	var accountDomain string
+	// first check if we can retrieve the account locally just with what we've been given
-	var accountable ap.Accountable
+	if params.RemoteAccountID != nil {
-
+		// try with uri
-	if params.RemoteAccountID == nil && (params.RemoteAccountUsername == "" || params.RemoteAccountHost == "") {
+		if a, dbErr := d.db.GetAccountByURI(ctx, params.RemoteAccountID.String()); dbErr == nil {
-		err = errors.New("GetRemoteAccount: no identifying parameters were set so we cannot dereference account")
+			remoteAccount = a
+		} else if dbErr != db.ErrNoEntries {
+			err = fmt.Errorf("GetRemoteAccount: database error looking for account %s: %s", params.RemoteAccountID, err)
+		}
+	} else if params.RemoteAccountUsername != "" && params.RemoteAccountHost != "" {
+		// try with username/host
+		a := &gtsmodel.Account{}
+		where := []db.Where{{Key: "username", Value: params.RemoteAccountUsername}, {Key: "domain", Value: params.RemoteAccountHost}}
+		if dbErr := d.db.GetWhere(ctx, where, a); dbErr == nil {
+			remoteAccount = a
+		} else if dbErr != db.ErrNoEntries {
+			err = fmt.Errorf("GetRemoteAccount: database error looking for account with username %s and host %s: %s", params.RemoteAccountUsername, params.RemoteAccountHost, err)
+		}
+	} else {
+		err = errors.New("GetRemoteAccount: no identifying parameters were set so we cannot get account")
 		return
 	}
 
+	if params.SkipResolve {
+		// if we can't resolve, return already since there's nothing more we can do
+		if remoteAccount == nil {
+			err = errors.New("GetRemoteAccount: error retrieving account with skipResolve set true")
+		}
+		return
+	}
+
+	var accountable ap.Accountable
 	if params.RemoteAccountUsername == "" || params.RemoteAccountHost == "" {
+		// try to populate the missing params
+		// the first one is easy ...
+		params.RemoteAccountHost = params.RemoteAccountID.Host
+		// ... but we still need the username so we can do a finger for the accountDomain
+
+		// check if we had the account stored already and got it earlier
+		if remoteAccount != nil {
+			params.RemoteAccountUsername = remoteAccount.Username
+		} else {
+			// if we didn't already have it, we have dereference it from remote and just...
 			accountable, err = d.dereferenceAccountable(ctx, params.RequestingUsername, params.RemoteAccountID)
 			if err != nil {
 				err = fmt.Errorf("GetRemoteAccount: error dereferencing accountable: %s", err)
 				return
 			}
 
-		params.RemoteAccountHost = params.RemoteAccountID.Host
+			// ... take the username (for now)
 			params.RemoteAccountUsername, err = ap.ExtractPreferredUsername(accountable)
 			if err != nil {
 				err = fmt.Errorf("GetRemoteAccount: error extracting accountable username: %s", err)
 				return
 			}
 		}
+	}
 
 	// if we reach this point, params.RemoteAccountHost and params.RemoteAccountUsername must be set
-	// params.RemoteAccountID may or may not be set, but we have enough information to fetch it again in any case
+	// params.RemoteAccountID may or may not be set, but we have enough information to fetch it in any case
+	var accountDomain string
 	accountDomain, params.RemoteAccountID, err = d.fingerRemoteAccount(ctx, params.RequestingUsername, params.RemoteAccountUsername, params.RemoteAccountHost)
 	if err != nil {
 		err = fmt.Errorf("GetRemoteAccount: error while fingering: %s", err)
@@ -134,40 +167,75 @@ func (d *deref) GetRemoteAccount(ctx context.Context, params GetRemoteAccountPar
 	// if we reach here then all params must now be set,
 	// either in original params or through fingering
 
-	// see if we have this account stored already and just return it if so
+	// we may also have some extra information already, like the account we had in the db, or the
-	if a, dbErr := d.db.GetAccountByURI(ctx, params.RemoteAccountID.String()); dbErr == nil {
+	// accountable representation that we dereferenced from remote
-		remoteAccount, err = d.populateAccountBeforeReturn(ctx, params, a)
+	if remoteAccount == nil {
-		return
+		// we still don't have the account, so deference it if we didn't earlier
-	}
-
-	// if we reach here then  we haven't seen this account before: dereference it from remote (f we haven't already)
 		if accountable == nil {
 			accountable, err = d.dereferenceAccountable(ctx, params.RequestingUsername, params.RemoteAccountID)
 			if err != nil {
-			return nil, fmt.Errorf("GetRemoteAccount: error dereferencing accountable: %s", err)
+				err = fmt.Errorf("GetRemoteAccount: error dereferencing accountable: %s", err)
+				return
 			}
 		}
 
-	newAccount, err := d.typeConverter.ASRepresentationToAccount(ctx, accountable, accountDomain, false)
+		// then convert
+		remoteAccount, err = d.typeConverter.ASRepresentationToAccount(ctx, accountable, accountDomain, false)
 		if err != nil {
-		return nil, fmt.Errorf("GetRemoteAccount: error converting accountable to account: %s", err)
+			err = fmt.Errorf("GetRemoteAccount: error converting accountable to account: %s", err)
+			return
 		}
 
-	ulid, err := id.NewRandomULID()
+		// this is a new account so we need to generate a new ID for it
+		var ulid string
+		ulid, err = id.NewRandomULID()
 		if err != nil {
-		return nil, fmt.Errorf("GetRemoteAccount: error generating new id for account: %s", err)
+			err = fmt.Errorf("GetRemoteAccount: error generating new id for account: %s", err)
+			return
 		}
-	newAccount.ID = ulid
+		remoteAccount.ID = ulid
 
-	if _, err := d.populateAccountFields(ctx, newAccount, params.RequestingUsername, params.Blocking, false); err != nil {
+		_, err = d.populateAccountFields(ctx, remoteAccount, params.RequestingUsername, params.Blocking)
-		return nil, fmt.Errorf("GetRemoteAccount: error populating further account fields: %s", err)
+		if err != nil {
+			err = fmt.Errorf("GetRemoteAccount: error populating further account fields: %s", err)
+			return
 		}
 
-	if err := d.db.Put(ctx, newAccount); err != nil {
+		err = d.db.Put(ctx, remoteAccount)
-		return nil, fmt.Errorf("GetRemoteAccount: error putting new account: %s", err)
+		if err != nil {
+			err = fmt.Errorf("GetRemoteAccount: error putting new account: %s", err)
+			return
 		}
 
-	return newAccount, nil
+		return // the new account
+	} else {
+		// we had the account already, but now we know the account domain, so update it if it's different
+		if !strings.EqualFold(remoteAccount.Domain, accountDomain) {
+			remoteAccount.Domain = accountDomain
+			remoteAccount, err = d.db.UpdateAccount(ctx, remoteAccount)
+			if err != nil {
+				err = fmt.Errorf("GetRemoteAccount: error updating account: %s", err)
+				return
+			}
+		}
+
+		// make sure the account fields are populated before returning:
+		// the caller might want to block until everything is loaded
+		var changed bool
+		changed, err = d.populateAccountFields(ctx, remoteAccount, params.RequestingUsername, params.Blocking)
+		if err != nil {
+			return nil, fmt.Errorf("GetRemoteAccount: error populating remoteAccount fields: %s", err)
+		}
+
+		if changed {
+			remoteAccount, err = d.db.UpdateAccount(ctx, remoteAccount)
+			if err != nil {
+				return nil, fmt.Errorf("GetRemoteAccount: error updating remoteAccount: %s", err)
+			}
+		}
+
+		return // the account we already had + possibly updated
+	}
 }
 
 // dereferenceAccountable calls remoteAccountID with a GET request, and tries to parse whatever
@@ -240,7 +308,7 @@ func (d *deref) dereferenceAccountable(ctx context.Context, username string, rem
 
 // 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(ctx context.Context, account *gtsmodel.Account, requestingUsername string, blocking bool, refresh bool) (bool, error) {
+func (d *deref) populateAccountFields(ctx context.Context, account *gtsmodel.Account, requestingUsername string, blocking bool) (bool, error) {
 	// if we're dealing with an instance account, just bail, we don't need to do anything
 	if instanceAccount(account) {
 		return false, nil
@@ -261,7 +329,7 @@ func (d *deref) populateAccountFields(ctx context.Context, account *gtsmodel.Acc
 	}
 
 	// fetch the header and avatar
-	changed, err := d.fetchRemoteAccountMedia(ctx, account, t, refresh, blocking)
+	changed, err := d.fetchRemoteAccountMedia(ctx, account, t, blocking)
 	if err != nil {
 		return false, fmt.Errorf("populateAccountFields: error fetching header/avi for account: %s", err)
 	}
@@ -281,7 +349,7 @@ func (d *deref) populateAccountFields(ctx context.Context, account *gtsmodel.Acc
 //
 // If blocking is true, then the calls to the media manager made by this function will be blocking:
 // in other words, the function won't return until the header and the avatar have been fully processed.
-func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsmodel.Account, t transport.Transport, blocking bool, refresh bool) (bool, error) {
+func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsmodel.Account, t transport.Transport, blocking bool) (bool, error) {
 	changed := false
 
 	accountURI, err := url.Parse(targetAccount.URI)
@@ -293,7 +361,7 @@ func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsm
 		return changed, fmt.Errorf("fetchRemoteAccountMedia: domain %s is blocked", accountURI.Host)
 	}
 
-	if targetAccount.AvatarRemoteURL != "" && (targetAccount.AvatarMediaAttachmentID == "" || refresh) {
+	if targetAccount.AvatarRemoteURL != "" && (targetAccount.AvatarMediaAttachmentID == "") {
 		var processingMedia *media.ProcessingMedia
 
 		d.dereferencingAvatarsLock.Lock() // LOCK HERE
@@ -351,7 +419,7 @@ func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsm
 		changed = true
 	}
 
-	if targetAccount.HeaderRemoteURL != "" && (targetAccount.HeaderMediaAttachmentID == "" || refresh) {
+	if targetAccount.HeaderRemoteURL != "" && (targetAccount.HeaderMediaAttachmentID == "") {
 		var processingMedia *media.ProcessingMedia
 
 		d.dereferencingHeadersLock.Lock() // LOCK HERE