mirrors/gotosocial
1
0
Fork 0
mirror of https://github.com/superseriousbusiness/gotosocial.git synced 2025-12-02 20:48:07 -06:00
Code Issues Projects Releases Packages Wiki Activity Actions

[bugfix] Fix multiple dereferences of boosted status causing media duplication (#589)

Browse source 
* add some announces to test models

* start on announce test logic

* test federatingDB.Announce

* change signature of GetRemoteStatus

* remove 'refresh' logic and replace it with refetch

* go fmt

* remove timeline manager from processor test

* make zork created at determinate

* test get account statuses

* test get + serialize zork

* make account keys determinate

* make admin accountCreate time determinate

* test account to as

* init test config before test log

* test status to frontend

* remove daft Within check

* hack around a bit

* use index of slice
This commit is contained in:
tobi 2022-05-23 17:40:03 +02:00 • committed by GitHub
commit f0c9f4169b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
18 changed files with 324 additions and 99 deletions
Download patch file Download diff file Expand all files Collapse all files

2
internal/federation/dereferencing/announce.go
View file

@ -46,7 +46,7 @@ func (d *deref) DereferenceAnnounce(ctx context.Context, announce *gtsmodel.Stat
return fmt.Errorf("DereferenceAnnounce: error dereferencing thread of boosted status: %s", err)
}
boostedStatus, _, _, err := d.GetRemoteStatus(ctx, requestingUsername, boostedStatusURI, false, true)
boostedStatus, _, err := d.GetRemoteStatus(ctx, requestingUsername, boostedStatusURI, false, true)
if err != nil {
return fmt.Errorf("DereferenceAnnounce: error dereferencing remote status with id %s: %s", announce.BoostOf.URI, err)
}

2
internal/federation/dereferencing/dereferencer.go
View file

@ -35,7 +35,7 @@ import (
type Dereferencer interface {
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)
GetRemoteStatus(ctx context.Context, username string, remoteStatusID *url.URL, refetch, includeParent bool) (*gtsmodel.Status, ap.Statusable, error)
EnrichRemoteStatus(ctx context.Context, username string, status *gtsmodel.Status, includeParent bool) (*gtsmodel.Status, error)
GetRemoteInstance(ctx context.Context, username string, remoteInstanceURI *url.URL) (*gtsmodel.Instance, error)

87
internal/federation/dereferencing/status.go
View file

@ -53,79 +53,63 @@ func (d *deref) EnrichRemoteStatus(ctx context.Context, username string, status
}
// GetRemoteStatus completely dereferences a remote status, converts it to a GtS model status,
// puts it in the database, and returns it to a caller. The boolean indicates whether the status is new
// to us or not. If we haven't seen the status before, bool will be true. If we have seen the status before,
// it will be false.
// puts it in the database, and returns it to a caller.
//
// If refresh is true, then even if we have the status in our database already, it will be dereferenced from its
// remote representation, as will its owner.
// If refetch is true, then regardless of whether we have the original status in the database or not,
// the ap.Statusable representation of the status will be dereferenced and returned.
//
// If a dereference was performed, then the function also returns the ap.Statusable representation for further processing.
// If refetch is false, the ap.Statusable will only be returned if this is a new status, so callers
// should check whether or not this is nil.
//
// SIDE EFFECTS: remote status will be stored in the database, and the remote status owner will also be stored.
func (d *deref) GetRemoteStatus(ctx context.Context, username string, remoteStatusID *url.URL, refresh, includeParent bool) (*gtsmodel.Status, ap.Statusable, bool, error) {
new := true
// check if we already have the status in our db
func (d *deref) GetRemoteStatus(ctx context.Context, username string, remoteStatusID *url.URL, refetch, includeParent bool) (*gtsmodel.Status, ap.Statusable, error) {
maybeStatus, err := d.db.GetStatusByURI(ctx, remoteStatusID.String())
if err == nil {
// we've seen this status before so it's not new
new = false
// if we're not being asked to refresh, we can just return the maybeStatus as-is and avoid doing any external calls
if !refresh {
return maybeStatus, nil, new, nil
}
if err == nil && !refetch {
// we already had the status and we aren't being asked to refetch the AP representation
return maybeStatus, nil, nil
}
statusable, err := d.dereferenceStatusable(ctx, username, remoteStatusID)
if err != nil {
return nil, statusable, new, fmt.Errorf("GetRemoteStatus: error dereferencing statusable: %s", err)
return nil, nil, fmt.Errorf("GetRemoteStatus: error dereferencing statusable: %s", err)
}
if maybeStatus != nil && refetch {
// we already had the status and we've successfully fetched the AP representation as requested
return maybeStatus, statusable, nil
}
// from here on out we can consider this to be a 'new' status because we didn't have the status in the db already
accountURI, err := ap.ExtractAttributedTo(statusable)
if err != nil {
return nil, statusable, new, fmt.Errorf("GetRemoteStatus: error extracting attributedTo: %s", err)
return nil, nil, fmt.Errorf("GetRemoteStatus: error extracting attributedTo: %s", err)
}
// do this so we know we have the remote account of the status in the db
_, err = d.GetRemoteAccount(ctx, username, accountURI, true, false)
if err != nil {
return nil, statusable, new, fmt.Errorf("GetRemoteStatus: couldn't derive status author: %s", err)
return nil, nil, fmt.Errorf("GetRemoteStatus: couldn't get status author: %s", err)
}
gtsStatus, err := d.typeConverter.ASStatusToStatus(ctx, statusable)
if err != nil {
return nil, statusable, new, fmt.Errorf("GetRemoteStatus: error converting statusable to status: %s", err)
return nil, statusable, fmt.Errorf("GetRemoteStatus: error converting statusable to status: %s", err)
}
if new {
ulid, err := id.NewULIDFromTime(gtsStatus.CreatedAt)
if err != nil {
return nil, statusable, new, fmt.Errorf("GetRemoteStatus: error generating new id for status: %s", err)
}
gtsStatus.ID = ulid
ulid, err := id.NewULIDFromTime(gtsStatus.CreatedAt)
if err != nil {
return nil, nil, fmt.Errorf("GetRemoteStatus: error generating new id for status: %s", err)
}
gtsStatus.ID = ulid
if err := d.populateStatusFields(ctx, gtsStatus, username, includeParent); err != nil {
return nil, statusable, new, fmt.Errorf("GetRemoteStatus: error populating status fields: %s", err)
}
if err := d.db.PutStatus(ctx, gtsStatus); err != nil {
return nil, statusable, new, fmt.Errorf("GetRemoteStatus: error putting new status: %s", err)
}
} else {
gtsStatus.ID = maybeStatus.ID
if err := d.populateStatusFields(ctx, gtsStatus, username, includeParent); err != nil {
return nil, statusable, new, fmt.Errorf("GetRemoteStatus: error populating status fields: %s", err)
}
if err := d.db.UpdateByPrimaryKey(ctx, gtsStatus); err != nil {
return nil, statusable, new, fmt.Errorf("GetRemoteStatus: error updating status: %s", err)
}
if err := d.populateStatusFields(ctx, gtsStatus, username, includeParent); err != nil {
return nil, nil, fmt.Errorf("GetRemoteStatus: error populating status fields: %s", err)
}
return gtsStatus, statusable, new, nil
if err := d.db.PutStatus(ctx, gtsStatus); err != nil {
return nil, nil, fmt.Errorf("GetRemoteStatus: error putting new status: %s", err)
}
return gtsStatus, statusable, nil
}
func (d *deref) dereferenceStatusable(ctx context.Context, username string, remoteStatusID *url.URL) (ap.Statusable, error) {
@ -429,14 +413,9 @@ func (d *deref) populateStatusRepliedTo(ctx context.Context, status *gtsmodel.St
return err
}
// see if we have the status in our db already
replyToStatus, err := d.db.GetStatusByURI(ctx, status.InReplyToURI)
replyToStatus, _, err := d.GetRemoteStatus(ctx, requestingUsername, statusURI, false, false)
if err != nil {
// Status was not in the DB, try fetch
replyToStatus, _, _, err = d.GetRemoteStatus(ctx, requestingUsername, statusURI, false, false)
if err != nil {
return fmt.Errorf("populateStatusRepliedTo: couldn't get reply to status with uri %s: %s", status.InReplyToURI, err)
}
return fmt.Errorf("populateStatusRepliedTo: couldn't get reply to status with uri %s: %s", status.InReplyToURI, err)
}
// we have the status

12
internal/federation/dereferencing/status_test.go
View file

@ -38,11 +38,9 @@ func (suite *StatusTestSuite) TestDereferenceSimpleStatus() {
fetchingAccount := suite.testAccounts["local_account_1"]
statusURL := testrig.URLMustParse("https://unknown-instance.com/users/brand_new_person/statuses/01FE4NTHKWW7THT67EF10EB839")
status, statusable, new, err := suite.dereferencer.GetRemoteStatus(context.Background(), fetchingAccount.Username, statusURL, false, false)
status, _, err := suite.dereferencer.GetRemoteStatus(context.Background(), fetchingAccount.Username, statusURL, false, false)
suite.NoError(err)
suite.NotNil(status)
suite.NotNil(statusable)
suite.True(new)
// status values should be set
suite.Equal("https://unknown-instance.com/users/brand_new_person/statuses/01FE4NTHKWW7THT67EF10EB839", status.URI)
@ -80,11 +78,9 @@ func (suite *StatusTestSuite) TestDereferenceStatusWithMention() {
fetchingAccount := suite.testAccounts["local_account_1"]
statusURL := testrig.URLMustParse("https://unknown-instance.com/users/brand_new_person/statuses/01FE5Y30E3W4P7TRE0R98KAYQV")
status, statusable, new, err := suite.dereferencer.GetRemoteStatus(context.Background(), fetchingAccount.Username, statusURL, false, false)
status, _, err := suite.dereferencer.GetRemoteStatus(context.Background(), fetchingAccount.Username, statusURL, false, false)
suite.NoError(err)
suite.NotNil(status)
suite.NotNil(statusable)
suite.True(new)
// status values should be set
suite.Equal("https://unknown-instance.com/users/brand_new_person/statuses/01FE5Y30E3W4P7TRE0R98KAYQV", status.URI)
@ -135,11 +131,9 @@ func (suite *StatusTestSuite) TestDereferenceStatusWithImageAndNoContent() {
fetchingAccount := suite.testAccounts["local_account_1"]
statusURL := testrig.URLMustParse("https://turnip.farm/users/turniplover6969/statuses/70c53e54-3146-42d5-a630-83c8b6c7c042")
status, statusable, new, err := suite.dereferencer.GetRemoteStatus(context.Background(), fetchingAccount.Username, statusURL, false, false)
status, _, err := suite.dereferencer.GetRemoteStatus(context.Background(), fetchingAccount.Username, statusURL, false, false)
suite.NoError(err)
suite.NotNil(status)
suite.NotNil(statusable)
suite.True(new)
// status values should be set
suite.Equal("https://turnip.farm/users/turniplover6969/statuses/70c53e54-3146-42d5-a630-83c8b6c7c042", status.URI)

13
internal/federation/dereferencing/thread.go
View file

@ -52,9 +52,9 @@ func (d *deref) DereferenceThread(ctx context.Context, username string, statusIR
}
// first make sure we have this status in our db
_, statusable, _, err := d.GetRemoteStatus(ctx, username, statusIRI, true, false)
_, statusable, err := d.GetRemoteStatus(ctx, username, statusIRI, true, false)
if err != nil {
return fmt.Errorf("DereferenceThread: error getting status with id %s: %s", statusIRI.String(), err)
return fmt.Errorf("DereferenceThread: error getting initial status with id %s: %s", statusIRI.String(), err)
}
// first iterate up through ancestors, dereferencing if necessary as we go
@ -106,9 +106,8 @@ func (d *deref) iterateAncestors(ctx context.Context, username string, statusIRI
return d.iterateAncestors(ctx, username, *nextIRI)
}
// If we reach here, we're looking at a remote status -- make sure we have it in our db by calling GetRemoteStatus
// We call it with refresh to true because we want the statusable representation to parse inReplyTo from.
_, statusable, _, err := d.GetRemoteStatus(ctx, username, &statusIRI, true, false)
// If we reach here, we're looking at a remote status
_, statusable, err := d.GetRemoteStatus(ctx, username, &statusIRI, true, false)
if err != nil {
l.Debugf("error getting remote status: %s", err)
return nil
@ -220,8 +219,8 @@ pageLoop:
foundReplies++
// get the remote statusable and put it in the db
_, statusable, new, err := d.GetRemoteStatus(ctx, username, itemURI, false, false)
if new && err == nil && statusable != nil {
_, statusable, err := d.GetRemoteStatus(ctx, username, itemURI, false, false)
if err == nil {
// now iterate descendants of *that* status
if err := d.iterateDescendants(ctx, username, *itemURI, statusable); err != nil {
continue