mirrors
/
gotosocial
mirror of
1
Fork 0
Code Issues Releases Activity

[bugfix] Update account `Update` logic (#1984)

This commit is contained in:
tobi 2023-07-12 13:20:15 +02:00 committed by GitHub
parent 8d92b2479f
commit 1951e6c840
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 94 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

106
internal/federation/dereferencing/account.go
View File

@ -472,34 +472,45 @@ func (d *deref) enrichAccount(ctx context.Context, requestUser string, uri *url.
return latestAcc, apubAcc, nil return latestAcc, apubAcc, nil
} }
func (d *deref) fetchRemoteAccountAvatar(ctx context.Context, tsport transport.Transport, existing, account *gtsmodel.Account) error { func (d *deref) fetchRemoteAccountAvatar(ctx context.Context, tsport transport.Transport, existing, latestAcc *gtsmodel.Account) error {
if account.AvatarRemoteURL == "" { if latestAcc.AvatarRemoteURL == "" {
// No fetching to do. // No avatar set on newest model, leave
// latest avatar attachment ID empty.
return nil return nil
} }
// By default we set the original media attachment ID. // By default we keep the previous media attachment ID. This will only
account.AvatarMediaAttachmentID = existing.AvatarMediaAttachmentID // be changed if and when we have the new media loaded into storage.
latestAcc.AvatarMediaAttachmentID = existing.AvatarMediaAttachmentID
if account.AvatarMediaAttachmentID != "" && // If we had a media attachment ID already, and the URL
existing.AvatarRemoteURL == account.AvatarRemoteURL { // of the attachment hasn't changed from existing -> latest,
// Look for an existing media attachment by the known ID. // then we may be able to just keep our existing attachment
// without having to make any remote calls.
if latestAcc.AvatarMediaAttachmentID != "" &&
existing.AvatarRemoteURL == latestAcc.AvatarRemoteURL {
// Ensure we have media attachment with the known ID.
media, err := d.state.DB.GetAttachmentByID(ctx, existing.AvatarMediaAttachmentID) media, err := d.state.DB.GetAttachmentByID(ctx, existing.AvatarMediaAttachmentID)
if err != nil && !errors.Is(err, db.ErrNoEntries) { if err != nil && !errors.Is(err, db.ErrNoEntries) {
return gtserror.Newf("error getting attachment %s: %w", existing.AvatarMediaAttachmentID, err) return gtserror.Newf("error getting attachment %s: %w", existing.AvatarMediaAttachmentID, err)
} }
if media != nil && *media.Cached { // Ensure attachment has correct properties.
// Media already cached, if media != nil && media.RemoteURL == latestAcc.AvatarRemoteURL {
// use this existing. // We already have the most up-to-date
// media attachment, keep using it.
return nil return nil
} }
} }
// If we reach here, we know we need to fetch the most
// up-to-date version of the attachment from remote.
// Parse and validate the newly provided media URL. // Parse and validate the newly provided media URL.
avatarURI, err := url.Parse(account.AvatarRemoteURL) avatarURI, err := url.Parse(latestAcc.AvatarRemoteURL)
if err != nil { if err != nil {
return gtserror.Newf("error parsing url %s: %w", account.AvatarRemoteURL, err) return gtserror.Newf("error parsing url %s: %w", latestAcc.AvatarRemoteURL, err)
} }
// Acquire lock for derefs map. // Acquire lock for derefs map.
@ -507,7 +518,7 @@ func (d *deref) fetchRemoteAccountAvatar(ctx context.Context, tsport transport.T
defer unlock() defer unlock()
// Look for an existing dereference in progress. // Look for an existing dereference in progress.
processing, ok := d.derefAvatars[account.AvatarRemoteURL] processing, ok := d.derefAvatars[latestAcc.AvatarRemoteURL]
if !ok { if !ok {
var err error var err error
@ -518,21 +529,21 @@ func (d *deref) fetchRemoteAccountAvatar(ctx context.Context, tsport transport.T
} }
// Create new media processing request from the media manager instance. // Create new media processing request from the media manager instance.
processing, err = d.mediaManager.PreProcessMedia(ctx, data, account.ID, &media.AdditionalMediaInfo{ processing, err = d.mediaManager.PreProcessMedia(ctx, data, latestAcc.ID, &media.AdditionalMediaInfo{
Avatar: func() *bool { v := true; return &v }(), Avatar: func() *bool { v := true; return &v }(),
RemoteURL: &account.AvatarRemoteURL, RemoteURL: &latestAcc.AvatarRemoteURL,
}) })
if err != nil { if err != nil {
return gtserror.Newf("error preprocessing media for attachment %s: %w", account.AvatarRemoteURL, err) return gtserror.Newf("error preprocessing media for attachment %s: %w", latestAcc.AvatarRemoteURL, err)
} }
// Store media in map to mark as processing. // Store media in map to mark as processing.
d.derefAvatars[account.AvatarRemoteURL] = processing d.derefAvatars[latestAcc.AvatarRemoteURL] = processing
defer func() { defer func() {
// On exit safely remove media from map. // On exit safely remove media from map.
unlock := d.derefAvatarsMu.Lock() unlock := d.derefAvatarsMu.Lock()
delete(d.derefAvatars, account.AvatarRemoteURL) delete(d.derefAvatars, latestAcc.AvatarRemoteURL)
unlock() unlock()
}() }()
} }
@ -542,43 +553,54 @@ func (d *deref) fetchRemoteAccountAvatar(ctx context.Context, tsport transport.T
// Start media attachment loading (blocking call). // Start media attachment loading (blocking call).
if _, err := processing.LoadAttachment(ctx); err != nil { if _, err := processing.LoadAttachment(ctx); err != nil {
return gtserror.Newf("error loading attachment %s: %w", account.AvatarRemoteURL, err) return gtserror.Newf("error loading attachment %s: %w", latestAcc.AvatarRemoteURL, err)
} }
// Set the newly loaded avatar media attachment ID. // Set the newly loaded avatar media attachment ID.
account.AvatarMediaAttachmentID = processing.AttachmentID() latestAcc.AvatarMediaAttachmentID = processing.AttachmentID()
return nil return nil
} }
func (d *deref) fetchRemoteAccountHeader(ctx context.Context, tsport transport.Transport, existing, account *gtsmodel.Account) error { func (d *deref) fetchRemoteAccountHeader(ctx context.Context, tsport transport.Transport, existing, latestAcc *gtsmodel.Account) error {
if account.HeaderRemoteURL == "" { if latestAcc.HeaderRemoteURL == "" {
// No fetching to do. // No header set on newest model, leave
// latest header attachment ID empty.
return nil return nil
} }
// By default we set the original media attachment ID. // By default we keep the previous media attachment ID. This will only
account.HeaderMediaAttachmentID = existing.HeaderMediaAttachmentID // be changed if and when we have the new media loaded into storage.
latestAcc.HeaderMediaAttachmentID = existing.HeaderMediaAttachmentID
if account.HeaderMediaAttachmentID != "" && // If we had a media attachment ID already, and the URL
existing.HeaderRemoteURL == account.HeaderRemoteURL { // of the attachment hasn't changed from existing -> latest,
// Look for an existing media attachment by the known ID. // then we may be able to just keep our existing attachment
// without having to make any remote calls.
if latestAcc.HeaderMediaAttachmentID != "" &&
existing.HeaderRemoteURL == latestAcc.HeaderRemoteURL {
// Ensure we have media attachment with the known ID.
media, err := d.state.DB.GetAttachmentByID(ctx, existing.HeaderMediaAttachmentID) media, err := d.state.DB.GetAttachmentByID(ctx, existing.HeaderMediaAttachmentID)
if err != nil && !errors.Is(err, db.ErrNoEntries) { if err != nil && !errors.Is(err, db.ErrNoEntries) {
return gtserror.Newf("error getting attachment %s: %w", existing.HeaderMediaAttachmentID, err) return gtserror.Newf("error getting attachment %s: %w", existing.HeaderMediaAttachmentID, err)
} }
if media != nil && *media.Cached { // Ensure attachment has correct properties.
// Media already cached, if media != nil && media.RemoteURL == latestAcc.HeaderRemoteURL {
// use this existing. // We already have the most up-to-date
// media attachment, keep using it.
return nil return nil
} }
} }
// If we reach here, we know we need to fetch the most
// up-to-date version of the attachment from remote.
// Parse and validate the newly provided media URL. // Parse and validate the newly provided media URL.
headerURI, err := url.Parse(account.HeaderRemoteURL) headerURI, err := url.Parse(latestAcc.HeaderRemoteURL)
if err != nil { if err != nil {
return gtserror.Newf("error parsing url %s: %w", account.HeaderRemoteURL, err) return gtserror.Newf("error parsing url %s: %w", latestAcc.HeaderRemoteURL, err)
} }
// Acquire lock for derefs map. // Acquire lock for derefs map.
@ -586,7 +608,7 @@ func (d *deref) fetchRemoteAccountHeader(ctx context.Context, tsport transport.T
defer unlock() defer unlock()
// Look for an existing dereference in progress. // Look for an existing dereference in progress.
processing, ok := d.derefHeaders[account.HeaderRemoteURL] processing, ok := d.derefHeaders[latestAcc.HeaderRemoteURL]
if !ok { if !ok {
var err error var err error
@ -597,21 +619,21 @@ func (d *deref) fetchRemoteAccountHeader(ctx context.Context, tsport transport.T
} }
// Create new media processing request from the media manager instance. // Create new media processing request from the media manager instance.
processing, err = d.mediaManager.PreProcessMedia(ctx, data, account.ID, &media.AdditionalMediaInfo{ processing, err = d.mediaManager.PreProcessMedia(ctx, data, latestAcc.ID, &media.AdditionalMediaInfo{
Header: func() *bool { v := true; return &v }(), Header: func() *bool { v := true; return &v }(),
RemoteURL: &account.HeaderRemoteURL, RemoteURL: &latestAcc.HeaderRemoteURL,
}) })
if err != nil { if err != nil {
return gtserror.Newf("error preprocessing media for attachment %s: %w", account.HeaderRemoteURL, err) return gtserror.Newf("error preprocessing media for attachment %s: %w", latestAcc.HeaderRemoteURL, err)
} }
// Store media in map to mark as processing. // Store media in map to mark as processing.
d.derefHeaders[account.HeaderRemoteURL] = processing d.derefHeaders[latestAcc.HeaderRemoteURL] = processing
defer func() { defer func() {
// On exit safely remove media from map. // On exit safely remove media from map.
unlock := d.derefHeadersMu.Lock() unlock := d.derefHeadersMu.Lock()
delete(d.derefHeaders, account.HeaderRemoteURL) delete(d.derefHeaders, latestAcc.HeaderRemoteURL)
unlock() unlock()
}() }()
} }
@ -621,11 +643,11 @@ func (d *deref) fetchRemoteAccountHeader(ctx context.Context, tsport transport.T
// Start media attachment loading (blocking call). // Start media attachment loading (blocking call).
if _, err := processing.LoadAttachment(ctx); err != nil { if _, err := processing.LoadAttachment(ctx); err != nil {
return gtserror.Newf("error loading attachment %s: %w", account.HeaderRemoteURL, err) return gtserror.Newf("error loading attachment %s: %w", latestAcc.HeaderRemoteURL, err)
} }
// Set the newly loaded avatar media attachment ID. // Set the newly loaded avatar media attachment ID.
account.HeaderMediaAttachmentID = processing.AttachmentID() latestAcc.HeaderMediaAttachmentID = processing.AttachmentID()
return nil return nil
} }

43
internal/federation/federatingdb/update.go
View File

@ -19,13 +19,12 @@ package federatingdb
import ( import (
"context" "context"
"errors"
"fmt"
"codeberg.org/gruf/go-logger/v2/level" "codeberg.org/gruf/go-logger/v2/level"
"github.com/superseriousbusiness/activity/streams/vocab" "github.com/superseriousbusiness/activity/streams/vocab"
"github.com/superseriousbusiness/gotosocial/internal/ap" "github.com/superseriousbusiness/gotosocial/internal/ap"
"github.com/superseriousbusiness/gotosocial/internal/config" "github.com/superseriousbusiness/gotosocial/internal/config"
"github.com/superseriousbusiness/gotosocial/internal/gtserror"
"github.com/superseriousbusiness/gotosocial/internal/gtsmodel" "github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
"github.com/superseriousbusiness/gotosocial/internal/log" "github.com/superseriousbusiness/gotosocial/internal/log"
"github.com/superseriousbusiness/gotosocial/internal/messages" "github.com/superseriousbusiness/gotosocial/internal/messages"
@ -41,7 +40,7 @@ import (
// //
// The library makes this call only after acquiring a lock first. // The library makes this call only after acquiring a lock first.
func (f *federatingDB) Update(ctx context.Context, asType vocab.Type) error { func (f *federatingDB) Update(ctx context.Context, asType vocab.Type) error {
l := log.Entry{}.WithContext(ctx) l := log.WithContext(ctx)
if log.Level() >= level.DEBUG { if log.Level() >= level.DEBUG {
i, err := marshalItem(asType) i, err := marshalItem(asType)
@ -66,40 +65,38 @@ func (f *federatingDB) Update(ctx context.Context, asType vocab.Type) error {
} }
func (f *federatingDB) updateAccountable(ctx context.Context, receivingAcct *gtsmodel.Account, requestingAcct *gtsmodel.Account, asType vocab.Type) error { func (f *federatingDB) updateAccountable(ctx context.Context, receivingAcct *gtsmodel.Account, requestingAcct *gtsmodel.Account, asType vocab.Type) error {
// Ensure delivered asType is a valid Accountable model.
accountable, ok := asType.(ap.Accountable) accountable, ok := asType.(ap.Accountable)
if !ok { if !ok {
return errors.New("updateAccountable: could not convert vocab.Type to Accountable") return gtserror.Newf("could not convert vocab.Type %T to Accountable", asType)
} }
updatedAcct, err := f.typeConverter.ASRepresentationToAccount(ctx, accountable, "") // Extract AP URI of the updated Accountable model.
if err != nil { idProp := accountable.GetJSONLDId()
return fmt.Errorf("updateAccountable: error converting to account: %w", err) if idProp == nil || !idProp.IsIRI() {
return gtserror.New("Accountable id prop was nil or not IRI")
} }
updatedAcctURI := idProp.GetIRI()
if updatedAcct.Domain == config.GetHost() || updatedAcct.Domain == config.GetAccountDomain() { // Don't try to update local accounts, it will break things.
// No need to update local accounts; in fact, if we try if updatedAcctURI.Host == config.GetHost() {
// this it will break the shit out of things so do NOT.
return nil return nil
} }
if requestingAcct.URI != updatedAcct.URI { // Ensure Accountable and requesting account are one and the same.
return fmt.Errorf("updateAccountable: update for account %s was requested by account %s, this is not valid", updatedAcct.URI, requestingAcct.URI) if updatedAcctURIStr := updatedAcctURI.String(); requestingAcct.URI != updatedAcctURIStr {
return gtserror.Newf("update for %s was requested by %s, this is not valid", updatedAcctURIStr, requestingAcct.URI)
} }
// Set some basic fields on the updated account // Pass in to the processor the existing version of the requesting
// based on what we already know about the requester. // account that we have, plus the Accountable representation that
updatedAcct.CreatedAt = requestingAcct.CreatedAt // was delivered along with the Update, for further asynchronous
updatedAcct.ID = requestingAcct.ID // updating of eg., avatar/header, emojis, etc. The actual db
updatedAcct.Language = requestingAcct.Language // inserts/updates will take place there.
updatedAcct.AvatarMediaAttachmentID = requestingAcct.AvatarMediaAttachmentID
updatedAcct.HeaderMediaAttachmentID = requestingAcct.HeaderMediaAttachmentID
// Pass to the processor for further updating of eg., avatar/header,
// emojis, etc. The actual db insert/update will take place there.
f.state.Workers.EnqueueFederator(ctx, messages.FromFederator{ f.state.Workers.EnqueueFederator(ctx, messages.FromFederator{
APObjectType: ap.ObjectProfile, APObjectType: ap.ObjectProfile,
APActivityType: ap.ActivityUpdate, APActivityType: ap.ActivityUpdate,
GTSModel: updatedAcct, GTSModel: requestingAcct,
APObjectModel: accountable, APObjectModel: accountable,
ReceivingAccount: receivingAcct, ReceivingAccount: receivingAcct,
}) })

20
internal/processing/fromfederator.go
View File

@ -20,7 +20,6 @@ package processing
import ( import (
"context" "context"
"errors" "errors"
"fmt"
"net/url" "net/url"
"codeberg.org/gruf/go-kv" "codeberg.org/gruf/go-kv"
@ -422,27 +421,28 @@ func (p *Processor) processCreateFlagFromFederator(ctx context.Context, federato
// processUpdateAccountFromFederator handles Activity Update and Object Profile // processUpdateAccountFromFederator handles Activity Update and Object Profile
func (p *Processor) processUpdateAccountFromFederator(ctx context.Context, federatorMsg messages.FromFederator) error { func (p *Processor) processUpdateAccountFromFederator(ctx context.Context, federatorMsg messages.FromFederator) error {
incomingAccount, ok := federatorMsg.GTSModel.(*gtsmodel.Account) // Parse the old/existing account model.
account, ok := federatorMsg.GTSModel.(*gtsmodel.Account)
if !ok { if !ok {
return errors.New("*gtsmodel.Account was not parseable on update account message") return gtserror.New("account was not parseable as *gtsmodel.Account")
} }
// Because this was an Update, the new AP Object should be set on the message. // Because this was an Update, the new Accountable should be set on the message.
incomingAccountable, ok := federatorMsg.APObjectModel.(ap.Accountable) apubAcc, ok := federatorMsg.APObjectModel.(ap.Accountable)
if !ok { if !ok {
return errors.New("Accountable was not parseable on update account message") return gtserror.New("Accountable was not parseable on update account message")
} }
// Fetch up-to-date bio, avatar, header, etc. // Fetch up-to-date bio, avatar, header, etc.
_, _, err := p.federator.RefreshAccount( _, _, err := p.federator.RefreshAccount(
ctx, ctx,
federatorMsg.ReceivingAccount.Username, federatorMsg.ReceivingAccount.Username,
incomingAccount, account,
incomingAccountable, apubAcc,
true, true, // Force refresh.
) )
if err != nil { if err != nil {
return fmt.Errorf("error enriching updated account from federator: %s", err) return gtserror.Newf("error refreshing updated account: %w", err)
} }
return nil return nil