From 667e7f112ce7b5b7452c392bbbe393a4c998508d Mon Sep 17 00:00:00 2001 From: tsmethurst Date: Mon, 24 Jan 2022 13:12:17 +0100 Subject: [PATCH] update remote account get/deref logic --- internal/federation/dereference.go | 8 +- internal/federation/dereferencing/account.go | 292 +++++++++++------- .../federation/dereferencing/account_test.go | 3 +- .../federation/dereferencing/dereferencer.go | 33 +- internal/federation/dereferencing/status.go | 8 +- internal/federation/federatingprotocol.go | 2 +- internal/federation/federator.go | 3 +- internal/processing/account/get.go | 8 +- .../processing/federation/getfollowers.go | 2 +- .../processing/federation/getfollowing.go | 2 +- internal/processing/federation/getoutbox.go | 2 +- internal/processing/federation/getstatus.go | 2 +- .../processing/federation/getstatusreplies.go | 2 +- internal/processing/federation/getuser.go | 2 +- internal/processing/fromfederator.go | 8 +- internal/processing/search.go | 4 +- internal/typeutils/internaltofrontend.go | 23 +- 17 files changed, 244 insertions(+), 160 deletions(-) diff --git a/internal/federation/dereference.go b/internal/federation/dereference.go index 343ddadb7..8cb23a91f 100644 --- a/internal/federation/dereference.go +++ b/internal/federation/dereference.go @@ -26,12 +26,8 @@ import ( "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" ) -func (f *federator) GetRemoteAccount(ctx context.Context, username string, remoteAccountID *url.URL, refresh bool) (*gtsmodel.Account, bool, error) { - return f.dereferencer.GetRemoteAccount(ctx, username, remoteAccountID, refresh) -} - -func (f *federator) EnrichRemoteAccount(ctx context.Context, username string, account *gtsmodel.Account) (*gtsmodel.Account, error) { - return f.dereferencer.EnrichRemoteAccount(ctx, username, account) +func (f *federator) GetRemoteAccount(ctx context.Context, username string, remoteAccountID *url.URL, blocking bool, refresh bool) (*gtsmodel.Account, error) { + return f.dereferencer.GetRemoteAccount(ctx, username, remoteAccountID, blocking, refresh) } func (f *federator) GetRemoteStatus(ctx context.Context, username string, remoteStatusID *url.URL, refresh, includeParent bool) (*gtsmodel.Status, ap.Statusable, bool, error) { diff --git a/internal/federation/dereferencing/account.go b/internal/federation/dereferencing/account.go index 581c95de2..27591d857 100644 --- a/internal/federation/dereferencing/account.go +++ b/internal/federation/dereferencing/account.go @@ -26,6 +26,7 @@ import ( "io" "net/url" "strings" + "sync" "github.com/sirupsen/logrus" "github.com/superseriousbusiness/activity/streams" @@ -44,30 +45,6 @@ func instanceAccount(account *gtsmodel.Account) bool { (account.Username == "internal.fetch" && strings.Contains(account.Note, "internal service actor")) } -// EnrichRemoteAccount takes an account that's already been inserted into the database in a minimal form, -// and populates it with additional fields, media, etc. -// -// 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(ctx context.Context, username string, account *gtsmodel.Account) (*gtsmodel.Account, error) { - // if we're dealing with an instance account, we don't need to update anything - if instanceAccount(account) { - return account, nil - } - - if err := d.PopulateAccountFields(ctx, account, username, false); err != nil { - return nil, err - } - - updated, err := d.db.UpdateAccount(ctx, account) - if err != nil { - logrus.Errorf("EnrichRemoteAccount: error updating account: %s", err) - return account, nil - } - - return updated, nil -} - // GetRemoteAccount completely dereferences a remote account, converts it to a GtS model account, // puts it in the database, and returns it to a caller. The boolean indicates whether the account is new // to us or not. If we haven't seen the account before, bool will be true. If we have seen the account before, @@ -77,60 +54,73 @@ func (d *deref) EnrichRemoteAccount(ctx context.Context, username string, accoun // the remote instance again. // // SIDE EFFECTS: remote account will be stored in the database, or updated if it already exists (and refresh is true). -func (d *deref) GetRemoteAccount(ctx context.Context, username string, remoteAccountID *url.URL, refresh bool) (*gtsmodel.Account, bool, error) { +func (d *deref) GetRemoteAccount(ctx context.Context, username string, remoteAccountID *url.URL, refresh bool, blocking bool) (*gtsmodel.Account, error) { new := true - // check if we already have the account in our db - maybeAccount, err := d.db.GetAccountByURI(ctx, remoteAccountID.String()) + // check if we already have the account in our db, and just return it unless we'd doing a refresh + remoteAccount, err := d.db.GetAccountByURI(ctx, remoteAccountID.String()) if err == nil { - // we've seen this account before so it's not new new = false if !refresh { - // we're not being asked to refresh, but just in case we don't have the avatar/header cached yet.... - maybeAccount, err = d.EnrichRemoteAccount(ctx, username, maybeAccount) - return maybeAccount, new, err + // make sure the account fields are populated before returning: + // even if we're not doing a refresh, the caller might want to block + // until everything is loaded + err = d.populateAccountFields(ctx, remoteAccount, username, refresh, blocking) + return remoteAccount, err } } - accountable, err := d.dereferenceAccountable(ctx, username, remoteAccountID) - if err != nil { - return nil, new, fmt.Errorf("FullyDereferenceAccount: error dereferencing accountable: %s", err) - } - - gtsAccount, err := d.typeConverter.ASRepresentationToAccount(ctx, accountable, refresh) - if err != nil { - return nil, new, fmt.Errorf("FullyDereferenceAccount: error converting accountable to account: %s", err) - } - if new { - // generate a new id since we haven't seen this account before, and do a put + // we haven't seen this account before: dereference it from remote + accountable, err := d.dereferenceAccountable(ctx, username, remoteAccountID) + if err != nil { + return nil, fmt.Errorf("GetRemoteAccount: error dereferencing accountable: %s", err) + } + + newAccount, err := d.typeConverter.ASRepresentationToAccount(ctx, accountable, refresh) + if err != nil { + return nil, fmt.Errorf("GetRemoteAccount: error converting accountable to account: %s", err) + } + ulid, err := id.NewRandomULID() if err != nil { - return nil, new, fmt.Errorf("FullyDereferenceAccount: error generating new id for account: %s", err) + return nil, fmt.Errorf("GetRemoteAccount: error generating new id for account: %s", err) } - gtsAccount.ID = ulid + newAccount.ID = ulid - if err := d.PopulateAccountFields(ctx, gtsAccount, username, refresh); err != nil { - return nil, new, fmt.Errorf("FullyDereferenceAccount: error populating further account fields: %s", err) + if err := d.populateAccountFields(ctx, newAccount, username, refresh, blocking); err != nil { + return nil, fmt.Errorf("GetRemoteAccount: error populating further account fields: %s", err) } - if err := d.db.Put(ctx, gtsAccount); err != nil { - return nil, new, fmt.Errorf("FullyDereferenceAccount: error putting new account: %s", err) - } - } else { - // take the id we already have and do an update - gtsAccount.ID = maybeAccount.ID - if err := d.PopulateAccountFields(ctx, gtsAccount, username, refresh); err != nil { - return nil, new, fmt.Errorf("FullyDereferenceAccount: error populating further account fields: %s", err) + if err := d.db.Put(ctx, newAccount); err != nil { + return nil, fmt.Errorf("GetRemoteAccount: error putting new account: %s", err) } - gtsAccount, err = d.db.UpdateAccount(ctx, gtsAccount) - if err != nil { - return nil, false, fmt.Errorf("EnrichRemoteAccount: error updating account: %s", err) - } + return newAccount, nil } - return gtsAccount, new, nil + // we have seen this account before, but we have to refresh it + refreshedAccountable, err := d.dereferenceAccountable(ctx, username, remoteAccountID) + if err != nil { + return nil, fmt.Errorf("GetRemoteAccount: error dereferencing refreshedAccountable: %s", err) + } + + refreshedAccount, err := d.typeConverter.ASRepresentationToAccount(ctx, refreshedAccountable, refresh) + if err != nil { + return nil, fmt.Errorf("GetRemoteAccount: error converting refreshedAccountable to refreshedAccount: %s", err) + } + refreshedAccount.ID = remoteAccount.ID + + if err := d.populateAccountFields(ctx, refreshedAccount, username, refresh, blocking); err != nil { + return nil, fmt.Errorf("GetRemoteAccount: error populating further refreshedAccount fields: %s", err) + } + + updatedAccount, err := d.db.UpdateAccount(ctx, refreshedAccount) + if err != nil { + return nil, fmt.Errorf("GetRemoteAccount: error updating refreshedAccount: %s", err) + } + + return updatedAccount, nil } // dereferenceAccountable calls remoteAccountID with a GET request, and tries to parse whatever @@ -201,93 +191,177 @@ func (d *deref) dereferenceAccountable(ctx context.Context, username string, rem 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(ctx context.Context, account *gtsmodel.Account, requestingUsername string, refresh bool) error { - l := logrus.WithFields(logrus.Fields{ - "func": "PopulateAccountFields", - "requestingUsername": requestingUsername, - }) +func (d *deref) populateAccountFields(ctx context.Context, account *gtsmodel.Account, requestingUsername string, refresh bool, blocking bool) error { + // if we're dealing with an instance account, just bail, we don't need to do anything + if instanceAccount(account) { + return nil + } accountURI, err := url.Parse(account.URI) if err != nil { - return fmt.Errorf("PopulateAccountFields: couldn't parse account URI %s: %s", account.URI, err) + return fmt.Errorf("populateAccountFields: couldn't parse account URI %s: %s", account.URI, err) } + if blocked, err := d.db.IsDomainBlocked(ctx, accountURI.Host); blocked || err != nil { - return fmt.Errorf("PopulateAccountFields: domain %s is blocked", accountURI.Host) + return fmt.Errorf("populateAccountFields: domain %s is blocked", accountURI.Host) } t, err := d.transportController.NewTransportForUsername(ctx, requestingUsername) if err != nil { - return fmt.Errorf("PopulateAccountFields: error getting transport for user: %s", err) + return fmt.Errorf("populateAccountFields: error getting transport for user: %s", err) } // fetch the header and avatar - if err := d.fetchHeaderAndAviForAccount(ctx, account, t, refresh); err != nil { - // if this doesn't work, just skip it -- we can do it later - l.Debugf("error fetching header/avi for account: %s", err) + if err := d.fetchRemoteAccountMedia(ctx, account, t, refresh, blocking); err != nil { + return fmt.Errorf("populateAccountFields: error fetching header/avi for account: %s", err) } return nil } -// fetchHeaderAndAviForAccount fetches the header and avatar for a remote account, using a transport -// on behalf of requestingUsername. +// fetchRemoteAccountMedia fetches and stores the header and avatar for a remote account, +// using a transport on behalf of requestingUsername. // // targetAccount's AvatarMediaAttachmentID and HeaderMediaAttachmentID will be updated as necessary. // -// SIDE EFFECTS: remote header and avatar will be stored in local storage. -func (d *deref) fetchHeaderAndAviForAccount(ctx context.Context, targetAccount *gtsmodel.Account, t transport.Transport, refresh bool) error { +// If refresh is true, then the media will be fetched again even if it's already been fetched before. +// +// 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, refresh bool, blocking bool) error { accountURI, err := url.Parse(targetAccount.URI) if err != nil { - return fmt.Errorf("fetchHeaderAndAviForAccount: couldn't parse account URI %s: %s", targetAccount.URI, err) + return fmt.Errorf("fetchRemoteAccountMedia: couldn't parse account URI %s: %s", targetAccount.URI, err) } + if blocked, err := d.db.IsDomainBlocked(ctx, accountURI.Host); blocked || err != nil { - return fmt.Errorf("fetchHeaderAndAviForAccount: domain %s is blocked", accountURI.Host) + return fmt.Errorf("fetchRemoteAccountMedia: domain %s is blocked", accountURI.Host) } if targetAccount.AvatarRemoteURL != "" && (targetAccount.AvatarMediaAttachmentID == "" || refresh) { - avatarIRI, err := url.Parse(targetAccount.AvatarRemoteURL) - if err != nil { - return err + var processingMedia *media.ProcessingMedia + + // first check if we're already processing this media + d.dereferencingAvatarsLock.Lock() + if alreadyProcessing, ok := d.dereferencingAvatars[targetAccount.ID]; ok { + // we're already on it, no worries + processingMedia = alreadyProcessing + } + d.dereferencingAvatarsLock.Unlock() + + if processingMedia == nil { + // we're not already processing it so start now + avatarIRI, err := url.Parse(targetAccount.AvatarRemoteURL) + if err != nil { + return err + } + + data := func(innerCtx context.Context) (io.Reader, int, error) { + return t.DereferenceMedia(innerCtx, avatarIRI) + } + + avatar := true + newProcessing, err := d.mediaManager.ProcessMedia(ctx, data, targetAccount.ID, &media.AdditionalMediaInfo{ + RemoteURL: &targetAccount.AvatarRemoteURL, + Avatar: &avatar, + }) + if err != nil { + return err + } + targetAccount.AvatarMediaAttachmentID = newProcessing.AttachmentID() + + // store it in our map to indicate it's in process + d.dereferencingAvatarsLock.Lock() + d.dereferencingAvatars[targetAccount.ID] = newProcessing + d.dereferencingAvatarsLock.Unlock() + + processingMedia = newProcessing } - data := func(innerCtx context.Context) (io.Reader, int, error) { - return t.DereferenceMedia(innerCtx, avatarIRI) + // block until loaded if required... + if blocking { + if err := lockAndLoad(ctx, d.dereferencingAvatarsLock, processingMedia, d.dereferencingAvatars, targetAccount.ID); err != nil { + return err + } + } else { + // ...otherwise do it async + go func() { + if err := lockAndLoad(ctx, d.dereferencingAvatarsLock, processingMedia, d.dereferencingAvatars, targetAccount.ID); err != nil { + logrus.Errorf("fetchRemoteAccountMedia: error during async lock and load of avatar: %s", err) + } + }() } - - avatar := true - processingMedia, err := d.mediaManager.ProcessMedia(ctx, data, targetAccount.ID, &media.AdditionalMediaInfo{ - RemoteURL: &targetAccount.AvatarRemoteURL, - Avatar: &avatar, - }) - if err != nil { - return err - } - - targetAccount.AvatarMediaAttachmentID = processingMedia.AttachmentID() } if targetAccount.HeaderRemoteURL != "" && (targetAccount.HeaderMediaAttachmentID == "" || refresh) { - headerIRI, err := url.Parse(targetAccount.HeaderRemoteURL) - if err != nil { - return err + var processingMedia *media.ProcessingMedia + + // first check if we're already processing this media + d.dereferencingHeadersLock.Lock() + if alreadyProcessing, ok := d.dereferencingHeaders[targetAccount.ID]; ok { + // we're already on it, no worries + processingMedia = alreadyProcessing + } + d.dereferencingHeadersLock.Unlock() + + if processingMedia == nil { + // we're not already processing it so start now + headerIRI, err := url.Parse(targetAccount.HeaderRemoteURL) + if err != nil { + return err + } + + data := func(innerCtx context.Context) (io.Reader, int, error) { + return t.DereferenceMedia(innerCtx, headerIRI) + } + + header := true + newProcessing, err := d.mediaManager.ProcessMedia(ctx, data, targetAccount.ID, &media.AdditionalMediaInfo{ + RemoteURL: &targetAccount.HeaderRemoteURL, + Header: &header, + }) + if err != nil { + return err + } + targetAccount.HeaderMediaAttachmentID = newProcessing.AttachmentID() + + // store it in our map to indicate it's in process + d.dereferencingHeadersLock.Lock() + d.dereferencingHeaders[targetAccount.ID] = newProcessing + d.dereferencingHeadersLock.Unlock() + + processingMedia = newProcessing } - data := func(innerCtx context.Context) (io.Reader, int, error) { - return t.DereferenceMedia(innerCtx, headerIRI) + // block until loaded if required... + if blocking { + if err := lockAndLoad(ctx, d.dereferencingHeadersLock, processingMedia, d.dereferencingHeaders, targetAccount.ID); err != nil { + return err + } + } else { + // ...otherwise do it async + go func() { + if err := lockAndLoad(ctx, d.dereferencingHeadersLock, processingMedia, d.dereferencingHeaders, targetAccount.ID); err != nil { + logrus.Errorf("fetchRemoteAccountMedia: error during async lock and load of header: %s", err) + } + }() } - - header := true - processingMedia, err := d.mediaManager.ProcessMedia(ctx, data, targetAccount.ID, &media.AdditionalMediaInfo{ - RemoteURL: &targetAccount.HeaderRemoteURL, - Header: &header, - }) - if err != nil { - return err - } - - targetAccount.HeaderMediaAttachmentID = processingMedia.AttachmentID() } + return nil } + +func lockAndLoad(ctx context.Context, lock *sync.Mutex, processing *media.ProcessingMedia, processingMap map[string]*media.ProcessingMedia, accountID string) error { + // whatever happens, remove the in-process media from the map + defer func() { + lock.Lock() + delete(processingMap, accountID) + lock.Unlock() + }() + + // try and load it + _, err := processing.LoadAttachment(ctx) + return err +} diff --git a/internal/federation/dereferencing/account_test.go b/internal/federation/dereferencing/account_test.go index 593ad341c..cb6f9588c 100644 --- a/internal/federation/dereferencing/account_test.go +++ b/internal/federation/dereferencing/account_test.go @@ -35,11 +35,10 @@ func (suite *AccountTestSuite) TestDereferenceGroup() { fetchingAccount := suite.testAccounts["local_account_1"] groupURL := testrig.URLMustParse("https://unknown-instance.com/groups/some_group") - group, new, err := suite.dereferencer.GetRemoteAccount(context.Background(), fetchingAccount.Username, groupURL, false) + group, err := suite.dereferencer.GetRemoteAccount(context.Background(), fetchingAccount.Username, groupURL, false, false) suite.NoError(err) suite.NotNil(group) suite.NotNil(group) - suite.True(new) // group values should be set suite.Equal("https://unknown-instance.com/groups/some_group", group.URI) diff --git a/internal/federation/dereferencing/dereferencer.go b/internal/federation/dereferencing/dereferencer.go index 800da6c04..daf82b91e 100644 --- a/internal/federation/dereferencing/dereferencer.go +++ b/internal/federation/dereferencing/dereferencer.go @@ -33,8 +33,7 @@ import ( // Dereferencer wraps logic and functionality for doing dereferencing of remote accounts, statuses, etc, from federated instances. type Dereferencer interface { - GetRemoteAccount(ctx context.Context, username string, remoteAccountID *url.URL, refresh bool) (*gtsmodel.Account, bool, error) - EnrichRemoteAccount(ctx context.Context, username string, account *gtsmodel.Account) (*gtsmodel.Account, error) + GetRemoteAccount(ctx context.Context, username string, remoteAccountID *url.URL, refresh bool, blocking bool) (*gtsmodel.Account, error) GetRemoteStatus(ctx context.Context, username string, remoteStatusID *url.URL, refresh, includeParent bool) (*gtsmodel.Status, ap.Statusable, bool, error) EnrichRemoteStatus(ctx context.Context, username string, status *gtsmodel.Status, includeParent bool) (*gtsmodel.Status, error) @@ -50,21 +49,29 @@ type Dereferencer interface { } type deref struct { - db db.DB - typeConverter typeutils.TypeConverter - transportController transport.Controller - mediaManager media.Manager - handshakes map[string][]*url.URL - handshakeSync *sync.Mutex // mutex to lock/unlock when checking or updating the handshakes map + db db.DB + typeConverter typeutils.TypeConverter + transportController transport.Controller + mediaManager media.Manager + dereferencingAvatars map[string]*media.ProcessingMedia + dereferencingAvatarsLock *sync.Mutex + dereferencingHeaders map[string]*media.ProcessingMedia + dereferencingHeadersLock *sync.Mutex + handshakes map[string][]*url.URL + handshakeSync *sync.Mutex // mutex to lock/unlock when checking or updating the handshakes map } // NewDereferencer returns a Dereferencer initialized with the given parameters. func NewDereferencer(db db.DB, typeConverter typeutils.TypeConverter, transportController transport.Controller, mediaManager media.Manager) Dereferencer { return &deref{ - db: db, - typeConverter: typeConverter, - transportController: transportController, - mediaManager: mediaManager, - handshakeSync: &sync.Mutex{}, + db: db, + typeConverter: typeConverter, + transportController: transportController, + mediaManager: mediaManager, + dereferencingAvatars: make(map[string]*media.ProcessingMedia), + dereferencingAvatarsLock: &sync.Mutex{}, + dereferencingHeaders: make(map[string]*media.ProcessingMedia), + dereferencingHeadersLock: &sync.Mutex{}, + handshakeSync: &sync.Mutex{}, } } diff --git a/internal/federation/dereferencing/status.go b/internal/federation/dereferencing/status.go index 004d648b5..34310f4aa 100644 --- a/internal/federation/dereferencing/status.go +++ b/internal/federation/dereferencing/status.go @@ -89,7 +89,7 @@ func (d *deref) GetRemoteStatus(ctx context.Context, username string, remoteStat } // do this so we know we have the remote account of the status in the db - _, _, err = d.GetRemoteAccount(ctx, username, accountURI, false) + _, err = d.GetRemoteAccount(ctx, username, accountURI, false, false) if err != nil { return nil, statusable, new, fmt.Errorf("GetRemoteStatus: couldn't derive status author: %s", err) } @@ -332,7 +332,7 @@ func (d *deref) populateStatusMentions(ctx context.Context, status *gtsmodel.Sta if targetAccount == nil { // we didn't find the account in our database already // check if we can get the account remotely (dereference it) - if a, _, err := d.GetRemoteAccount(ctx, requestingUsername, targetAccountURI, false); err != nil { + if a, err := d.GetRemoteAccount(ctx, requestingUsername, targetAccountURI, false, false); err != nil { errs = append(errs, err.Error()) } else { logrus.Debugf("populateStatusMentions: got target account %s with id %s through GetRemoteAccount", targetAccountURI, a.ID) @@ -394,7 +394,7 @@ func (d *deref) populateStatusAttachments(ctx context.Context, status *gtsmodel. a.AccountID = status.AccountID a.StatusID = status.ID - media, err := d.GetRemoteMedia(ctx, requestingUsername, a.AccountID, a.RemoteURL, &media.AdditionalMediaInfo{ + processingMedia, err := d.GetRemoteMedia(ctx, requestingUsername, a.AccountID, a.RemoteURL, &media.AdditionalMediaInfo{ CreatedAt: &a.CreatedAt, StatusID: &a.StatusID, RemoteURL: &a.RemoteURL, @@ -406,7 +406,7 @@ func (d *deref) populateStatusAttachments(ctx context.Context, status *gtsmodel. continue } - attachment, err := media.LoadAttachment(ctx) + attachment, err := processingMedia.LoadAttachment(ctx) if err != nil { logrus.Errorf("populateStatusAttachments: couldn't load remote attachment %s: %s", a.RemoteURL, err) continue diff --git a/internal/federation/federatingprotocol.go b/internal/federation/federatingprotocol.go index f5d42a8e3..789959810 100644 --- a/internal/federation/federatingprotocol.go +++ b/internal/federation/federatingprotocol.go @@ -153,7 +153,7 @@ func (f *federator) AuthenticatePostInbox(ctx context.Context, w http.ResponseWr } } - requestingAccount, _, err := f.GetRemoteAccount(ctx, username, publicKeyOwnerURI, false) + requestingAccount, err := f.GetRemoteAccount(ctx, username, publicKeyOwnerURI, false, false) if err != nil { return nil, false, fmt.Errorf("couldn't get requesting account %s: %s", publicKeyOwnerURI, err) } diff --git a/internal/federation/federator.go b/internal/federation/federator.go index 7edff1118..cb63084db 100644 --- a/internal/federation/federator.go +++ b/internal/federation/federator.go @@ -57,8 +57,7 @@ type Federator interface { DereferenceRemoteThread(ctx context.Context, username string, statusURI *url.URL) error DereferenceAnnounce(ctx context.Context, announce *gtsmodel.Status, requestingUsername string) error - GetRemoteAccount(ctx context.Context, username string, remoteAccountID *url.URL, refresh bool) (*gtsmodel.Account, bool, error) - EnrichRemoteAccount(ctx context.Context, username string, account *gtsmodel.Account) (*gtsmodel.Account, error) + GetRemoteAccount(ctx context.Context, username string, remoteAccountID *url.URL, blocking bool, refresh bool) (*gtsmodel.Account, error) GetRemoteStatus(ctx context.Context, username string, remoteStatusID *url.URL, refresh, includeParent bool) (*gtsmodel.Status, ap.Statusable, bool, error) EnrichRemoteStatus(ctx context.Context, username string, status *gtsmodel.Status, includeParent bool) (*gtsmodel.Status, error) diff --git a/internal/processing/account/get.go b/internal/processing/account/get.go index e96040db7..2571d7af1 100644 --- a/internal/processing/account/get.go +++ b/internal/processing/account/get.go @@ -22,6 +22,7 @@ import ( "context" "errors" "fmt" + "net/url" apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" "github.com/superseriousbusiness/gotosocial/internal/db" @@ -56,7 +57,12 @@ func (p *processor) Get(ctx context.Context, requestingAccount *gtsmodel.Account // last-minute check to make sure we have remote account header/avi cached if targetAccount.Domain != "" { - a, err := p.federator.EnrichRemoteAccount(ctx, requestingAccount.Username, targetAccount) + targetAccountURI, err := url.Parse(targetAccount.URI) + if err != nil { + return nil, fmt.Errorf("error parsing url %s: %s", targetAccount.URI, err) + } + + a, err := p.federator.GetRemoteAccount(ctx, requestingAccount.Username, targetAccountURI, true, false) if err == nil { targetAccount = a } diff --git a/internal/processing/federation/getfollowers.go b/internal/processing/federation/getfollowers.go index 9153cde1e..c15b2b6c4 100644 --- a/internal/processing/federation/getfollowers.go +++ b/internal/processing/federation/getfollowers.go @@ -41,7 +41,7 @@ func (p *processor) GetFollowers(ctx context.Context, requestedUsername string, return nil, gtserror.NewErrorNotAuthorized(errors.New("not authorized"), "not authorized") } - requestingAccount, _, err := p.federator.GetRemoteAccount(ctx, requestedUsername, requestingAccountURI, false) + requestingAccount, err := p.federator.GetRemoteAccount(ctx, requestedUsername, requestingAccountURI, false, false) if err != nil { return nil, gtserror.NewErrorNotAuthorized(err) } diff --git a/internal/processing/federation/getfollowing.go b/internal/processing/federation/getfollowing.go index 8a3025154..d2beaada0 100644 --- a/internal/processing/federation/getfollowing.go +++ b/internal/processing/federation/getfollowing.go @@ -41,7 +41,7 @@ func (p *processor) GetFollowing(ctx context.Context, requestedUsername string, return nil, gtserror.NewErrorNotAuthorized(errors.New("not authorized"), "not authorized") } - requestingAccount, _, err := p.federator.GetRemoteAccount(ctx, requestedUsername, requestingAccountURI, false) + requestingAccount, err := p.federator.GetRemoteAccount(ctx, requestedUsername, requestingAccountURI, false, false) if err != nil { return nil, gtserror.NewErrorNotAuthorized(err) } diff --git a/internal/processing/federation/getoutbox.go b/internal/processing/federation/getoutbox.go index 0f2043447..944c0b571 100644 --- a/internal/processing/federation/getoutbox.go +++ b/internal/processing/federation/getoutbox.go @@ -42,7 +42,7 @@ func (p *processor) GetOutbox(ctx context.Context, requestedUsername string, pag return nil, gtserror.NewErrorNotAuthorized(errors.New("not authorized"), "not authorized") } - requestingAccount, _, err := p.federator.GetRemoteAccount(ctx, requestedUsername, requestingAccountURI, false) + requestingAccount, err := p.federator.GetRemoteAccount(ctx, requestedUsername, requestingAccountURI, false, false) if err != nil { return nil, gtserror.NewErrorNotAuthorized(err) } diff --git a/internal/processing/federation/getstatus.go b/internal/processing/federation/getstatus.go index f065eaa71..1651516b5 100644 --- a/internal/processing/federation/getstatus.go +++ b/internal/processing/federation/getstatus.go @@ -43,7 +43,7 @@ func (p *processor) GetStatus(ctx context.Context, requestedUsername string, req return nil, gtserror.NewErrorNotAuthorized(errors.New("not authorized"), "not authorized") } - requestingAccount, _, err := p.federator.GetRemoteAccount(ctx, requestedUsername, requestingAccountURI, false) + requestingAccount, err := p.federator.GetRemoteAccount(ctx, requestedUsername, requestingAccountURI, false, false) if err != nil { return nil, gtserror.NewErrorNotAuthorized(err) } diff --git a/internal/processing/federation/getstatusreplies.go b/internal/processing/federation/getstatusreplies.go index 4fc21e3ad..c6db4dd3e 100644 --- a/internal/processing/federation/getstatusreplies.go +++ b/internal/processing/federation/getstatusreplies.go @@ -43,7 +43,7 @@ func (p *processor) GetStatusReplies(ctx context.Context, requestedUsername stri return nil, gtserror.NewErrorNotAuthorized(errors.New("not authorized"), "not authorized") } - requestingAccount, _, err := p.federator.GetRemoteAccount(ctx, requestedUsername, requestingAccountURI, false) + requestingAccount, err := p.federator.GetRemoteAccount(ctx, requestedUsername, requestingAccountURI, false, false) if err != nil { return nil, gtserror.NewErrorNotAuthorized(err) } diff --git a/internal/processing/federation/getuser.go b/internal/processing/federation/getuser.go index a8d6bcf38..6d5b8463f 100644 --- a/internal/processing/federation/getuser.go +++ b/internal/processing/federation/getuser.go @@ -54,7 +54,7 @@ func (p *processor) GetUser(ctx context.Context, requestedUsername string, reque // if we're not already handshaking/dereferencing a remote account, dereference it now if !p.federator.Handshaking(ctx, requestedUsername, requestingAccountURI) { - requestingAccount, _, err := p.federator.GetRemoteAccount(ctx, requestedUsername, requestingAccountURI, false) + requestingAccount, err := p.federator.GetRemoteAccount(ctx, requestedUsername, requestingAccountURI, false, false) if err != nil { return nil, gtserror.NewErrorNotAuthorized(err) } diff --git a/internal/processing/fromfederator.go b/internal/processing/fromfederator.go index 533d00242..8b575dda8 100644 --- a/internal/processing/fromfederator.go +++ b/internal/processing/fromfederator.go @@ -22,6 +22,7 @@ import ( "context" "errors" "fmt" + "net/url" "github.com/sirupsen/logrus" "github.com/superseriousbusiness/gotosocial/internal/ap" @@ -232,7 +233,12 @@ func (p *processor) processUpdateAccountFromFederator(ctx context.Context, feder return errors.New("profile was not parseable as *gtsmodel.Account") } - if _, err := p.federator.EnrichRemoteAccount(ctx, federatorMsg.ReceivingAccount.Username, incomingAccount); err != nil { + incomingAccountURL, err := url.Parse(incomingAccount.URI) + if err != nil { + return err + } + + if _, err := p.federator.GetRemoteAccount(ctx, federatorMsg.ReceivingAccount.Username, incomingAccountURL, false, true); err != nil { return fmt.Errorf("error enriching updated account from federator: %s", err) } diff --git a/internal/processing/search.go b/internal/processing/search.go index b03ced831..c8c302857 100644 --- a/internal/processing/search.go +++ b/internal/processing/search.go @@ -148,7 +148,7 @@ func (p *processor) searchAccountByURI(ctx context.Context, authed *oauth.Auth, if resolve { // we don't have it locally so try and dereference it - account, _, err := p.federator.GetRemoteAccount(ctx, authed.Account.Username, uri, true) + account, err := p.federator.GetRemoteAccount(ctx, authed.Account.Username, uri, true, true) if err != nil { return nil, fmt.Errorf("searchAccountByURI: error dereferencing account with uri %s: %s", uri.String(), err) } @@ -203,7 +203,7 @@ func (p *processor) searchAccountByMention(ctx context.Context, authed *oauth.Au } // we don't have it locally so try and dereference it - account, _, err := p.federator.GetRemoteAccount(ctx, authed.Account.Username, acctURI, true) + account, err := p.federator.GetRemoteAccount(ctx, authed.Account.Username, acctURI, true, true) if err != nil { return nil, fmt.Errorf("searchAccountByMention: error dereferencing account with uri %s: %s", acctURI.String(), err) } diff --git a/internal/typeutils/internaltofrontend.go b/internal/typeutils/internaltofrontend.go index 8236fb3ae..52e89b7d2 100644 --- a/internal/typeutils/internaltofrontend.go +++ b/internal/typeutils/internaltofrontend.go @@ -96,35 +96,32 @@ func (c *converter) AccountToAPIAccountPublic(ctx context.Context, a *gtsmodel.A lastStatusAt = lastPosted.Format(time.RFC3339) } - // build the avatar and header URLs + // set account avatar fields if available var aviURL string var aviURLStatic string if a.AvatarMediaAttachmentID != "" { - // make sure avi is pinned to this account if a.AvatarMediaAttachment == nil { avi, err := c.db.GetAttachmentByID(ctx, a.AvatarMediaAttachmentID) - if err != nil { - return nil, fmt.Errorf("error retrieving avatar: %s", err) + if err == nil { + a.AvatarMediaAttachment = avi + aviURL = a.AvatarMediaAttachment.URL + aviURLStatic = a.AvatarMediaAttachment.Thumbnail.URL } - a.AvatarMediaAttachment = avi } - aviURL = a.AvatarMediaAttachment.URL - aviURLStatic = a.AvatarMediaAttachment.Thumbnail.URL } + // set account header fields if available var headerURL string var headerURLStatic string if a.HeaderMediaAttachmentID != "" { - // make sure header is pinned to this account if a.HeaderMediaAttachment == nil { avi, err := c.db.GetAttachmentByID(ctx, a.HeaderMediaAttachmentID) - if err != nil { - return nil, fmt.Errorf("error retrieving avatar: %s", err) + if err == nil { + a.HeaderMediaAttachment = avi + headerURL = a.HeaderMediaAttachment.URL + headerURLStatic = a.HeaderMediaAttachment.Thumbnail.URL } - a.HeaderMediaAttachment = avi } - headerURL = a.HeaderMediaAttachment.URL - headerURLStatic = a.HeaderMediaAttachment.Thumbnail.URL } // get the fields set on this account