only update account in db if changed

This commit is contained in:
tsmethurst 2022-01-25 11:21:22 +01:00
parent 926b37e271
commit eba66d3a88

View file

@ -66,17 +66,21 @@ func (d *deref) GetRemoteAccount(ctx context.Context, username string, remoteAcc
// make sure the account fields are populated before returning: // make sure the account fields are populated before returning:
// even if we're not doing a refresh, the caller might want to block // even if we're not doing a refresh, the caller might want to block
// until everything is loaded // until everything is loaded
if err := d.populateAccountFields(ctx, remoteAccount, username, refresh, blocking); err != nil { changed, err := d.populateAccountFields(ctx, remoteAccount, username, refresh, blocking)
if err != nil {
return nil, fmt.Errorf("GetRemoteAccount: error populating remoteAccount fields: %s", err) return nil, fmt.Errorf("GetRemoteAccount: error populating remoteAccount fields: %s", err)
} }
if changed {
updatedAccount, err := d.db.UpdateAccount(ctx, remoteAccount) updatedAccount, err := d.db.UpdateAccount(ctx, remoteAccount)
if err != nil { if err != nil {
return nil, fmt.Errorf("GetRemoteAccount: error updating remoteAccount: %s", err) return nil, fmt.Errorf("GetRemoteAccount: error updating remoteAccount: %s", err)
} }
return updatedAccount, err return updatedAccount, err
} }
return remoteAccount, nil
}
} }
if new { if new {
@ -97,7 +101,7 @@ func (d *deref) GetRemoteAccount(ctx context.Context, username string, remoteAcc
} }
newAccount.ID = ulid newAccount.ID = ulid
if err := d.populateAccountFields(ctx, newAccount, username, refresh, blocking); err != nil { if _, err := d.populateAccountFields(ctx, newAccount, username, refresh, blocking); err != nil {
return nil, fmt.Errorf("GetRemoteAccount: error populating further account fields: %s", err) return nil, fmt.Errorf("GetRemoteAccount: error populating further account fields: %s", err)
} }
@ -120,18 +124,22 @@ func (d *deref) GetRemoteAccount(ctx context.Context, username string, remoteAcc
} }
refreshedAccount.ID = remoteAccount.ID refreshedAccount.ID = remoteAccount.ID
if err := d.populateAccountFields(ctx, refreshedAccount, username, refresh, blocking); err != nil { changed, err := d.populateAccountFields(ctx, refreshedAccount, username, refresh, blocking)
if err != nil {
return nil, fmt.Errorf("GetRemoteAccount: error populating further refreshedAccount fields: %s", err) return nil, fmt.Errorf("GetRemoteAccount: error populating further refreshedAccount fields: %s", err)
} }
if changed {
updatedAccount, err := d.db.UpdateAccount(ctx, refreshedAccount) updatedAccount, err := d.db.UpdateAccount(ctx, refreshedAccount)
if err != nil { if err != nil {
return nil, fmt.Errorf("GetRemoteAccount: error updating refreshedAccount: %s", err) return nil, fmt.Errorf("GetRemoteAccount: error updating refreshedAccount: %s", err)
} }
return updatedAccount, nil return updatedAccount, nil
} }
return refreshedAccount, nil
}
// dereferenceAccountable calls remoteAccountID with a GET request, and tries to parse whatever // dereferenceAccountable calls remoteAccountID with a GET request, and tries to parse whatever
// it finds as something that an account model can be constructed out of. // it finds as something that an account model can be constructed out of.
// //
@ -202,51 +210,57 @@ 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 // populateAccountFields populates any fields on the given account that weren't populated by the initial
// dereferencing. This includes things like header and avatar etc. // 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) error { func (d *deref) populateAccountFields(ctx context.Context, account *gtsmodel.Account, requestingUsername string, blocking bool, refresh bool) (bool, error) {
// if we're dealing with an instance account, just bail, we don't need to do anything // if we're dealing with an instance account, just bail, we don't need to do anything
if instanceAccount(account) { if instanceAccount(account) {
return nil return false, nil
} }
accountURI, err := url.Parse(account.URI) accountURI, err := url.Parse(account.URI)
if err != nil { if err != nil {
return fmt.Errorf("populateAccountFields: couldn't parse account URI %s: %s", account.URI, err) return false, 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 { if blocked, err := d.db.IsDomainBlocked(ctx, accountURI.Host); blocked || err != nil {
return fmt.Errorf("populateAccountFields: domain %s is blocked", accountURI.Host) return false, fmt.Errorf("populateAccountFields: domain %s is blocked", accountURI.Host)
} }
t, err := d.transportController.NewTransportForUsername(ctx, requestingUsername) t, err := d.transportController.NewTransportForUsername(ctx, requestingUsername)
if err != nil { if err != nil {
return fmt.Errorf("populateAccountFields: error getting transport for user: %s", err) return false, fmt.Errorf("populateAccountFields: error getting transport for user: %s", err)
} }
// fetch the header and avatar // fetch the header and avatar
if err := d.fetchRemoteAccountMedia(ctx, account, t, refresh, blocking); err != nil { changed, err := d.fetchRemoteAccountMedia(ctx, account, t, refresh, blocking)
return fmt.Errorf("populateAccountFields: error fetching header/avi for account: %s", err) if err != nil {
return false, fmt.Errorf("populateAccountFields: error fetching header/avi for account: %s", err)
} }
return nil return changed, nil
} }
// fetchRemoteAccountMedia fetches and stores the header and avatar for a remote account, // fetchRemoteAccountMedia fetches and stores the header and avatar for a remote account,
// using a transport on behalf of requestingUsername. // using a transport on behalf of requestingUsername.
// //
// The returned boolean indicates whether anything changed -- in other words, whether the
// account should be updated in the database.
//
// targetAccount's AvatarMediaAttachmentID and HeaderMediaAttachmentID will be updated as necessary. // targetAccount's AvatarMediaAttachmentID and HeaderMediaAttachmentID will be updated as necessary.
// //
// If refresh is true, then the media will be fetched again even if it's already been fetched before. // 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: // 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. // 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) error { func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsmodel.Account, t transport.Transport, blocking bool, refresh bool) (bool, error) {
changed := false
accountURI, err := url.Parse(targetAccount.URI) accountURI, err := url.Parse(targetAccount.URI)
if err != nil { if err != nil {
return fmt.Errorf("fetchRemoteAccountMedia: couldn't parse account URI %s: %s", targetAccount.URI, err) return changed, 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 { if blocked, err := d.db.IsDomainBlocked(ctx, accountURI.Host); blocked || err != nil {
return fmt.Errorf("fetchRemoteAccountMedia: domain %s is blocked", accountURI.Host) return changed, fmt.Errorf("fetchRemoteAccountMedia: domain %s is blocked", accountURI.Host)
} }
if targetAccount.AvatarRemoteURL != "" && (targetAccount.AvatarMediaAttachmentID == "" || refresh) { if targetAccount.AvatarRemoteURL != "" && (targetAccount.AvatarMediaAttachmentID == "" || refresh) {
@ -264,7 +278,7 @@ func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsm
// we're not already processing it so start now // we're not already processing it so start now
avatarIRI, err := url.Parse(targetAccount.AvatarRemoteURL) avatarIRI, err := url.Parse(targetAccount.AvatarRemoteURL)
if err != nil { if err != nil {
return err return changed, err
} }
data := func(innerCtx context.Context) (io.Reader, int, error) { data := func(innerCtx context.Context) (io.Reader, int, error) {
@ -277,7 +291,7 @@ func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsm
Avatar: &avatar, Avatar: &avatar,
}) })
if err != nil { if err != nil {
return err return changed, err
} }
// store it in our map to indicate it's in process // store it in our map to indicate it's in process
@ -291,7 +305,7 @@ func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsm
// block until loaded if required... // block until loaded if required...
if blocking { if blocking {
if err := lockAndLoad(ctx, d.dereferencingAvatarsLock, processingMedia, d.dereferencingAvatars, targetAccount.ID); err != nil { if err := lockAndLoad(ctx, d.dereferencingAvatarsLock, processingMedia, d.dereferencingAvatars, targetAccount.ID); err != nil {
return err return changed, err
} }
} else { } else {
// ...otherwise do it async // ...otherwise do it async
@ -305,6 +319,7 @@ func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsm
} }
targetAccount.AvatarMediaAttachmentID = processingMedia.AttachmentID() targetAccount.AvatarMediaAttachmentID = processingMedia.AttachmentID()
changed = true
} }
if targetAccount.HeaderRemoteURL != "" && (targetAccount.HeaderMediaAttachmentID == "" || refresh) { if targetAccount.HeaderRemoteURL != "" && (targetAccount.HeaderMediaAttachmentID == "" || refresh) {
@ -322,7 +337,7 @@ func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsm
// we're not already processing it so start now // we're not already processing it so start now
headerIRI, err := url.Parse(targetAccount.HeaderRemoteURL) headerIRI, err := url.Parse(targetAccount.HeaderRemoteURL)
if err != nil { if err != nil {
return err return changed, err
} }
data := func(innerCtx context.Context) (io.Reader, int, error) { data := func(innerCtx context.Context) (io.Reader, int, error) {
@ -335,7 +350,7 @@ func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsm
Header: &header, Header: &header,
}) })
if err != nil { if err != nil {
return err return changed, err
} }
// store it in our map to indicate it's in process // store it in our map to indicate it's in process
@ -349,7 +364,7 @@ func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsm
// block until loaded if required... // block until loaded if required...
if blocking { if blocking {
if err := lockAndLoad(ctx, d.dereferencingHeadersLock, processingMedia, d.dereferencingHeaders, targetAccount.ID); err != nil { if err := lockAndLoad(ctx, d.dereferencingHeadersLock, processingMedia, d.dereferencingHeaders, targetAccount.ID); err != nil {
return err return changed, err
} }
} else { } else {
// ...otherwise do it async // ...otherwise do it async
@ -363,9 +378,10 @@ func (d *deref) fetchRemoteAccountMedia(ctx context.Context, targetAccount *gtsm
} }
targetAccount.HeaderMediaAttachmentID = processingMedia.AttachmentID() targetAccount.HeaderMediaAttachmentID = processingMedia.AttachmentID()
changed = true
} }
return nil return changed, nil
} }
func lockAndLoad(ctx context.Context, lock *sync.Mutex, processing *media.ProcessingMedia, processingMap map[string]*media.ProcessingMedia, accountID string) error { func lockAndLoad(ctx context.Context, lock *sync.Mutex, processing *media.ProcessingMedia, processingMap map[string]*media.ProcessingMedia, accountID string) error {