From e8595f0c64f527af0913d1a426b697e67ff74ac9 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Mon, 20 Mar 2023 19:10:08 +0100 Subject: [chore] Refactor account deleting/block logic, tidy up some other processing things (#1599) * start refactoring account deletion * update to use state.DB * further messing about * some more tidying up * more tidying, cleaning, nice-making * further adventures in refactoring and the woes of technical debt * update fr accept/reject * poking + prodding * fix up deleting * create fave uri * don't log using requestingAccount.ID because it might be nil * move getBookmarks function * use exists query to check for status bookmark * use deletenotifications func * fiddle * delete follow request notif * split up some db functions * Fix possible nil pointer panic * fix more possible nil pointers * fix license headers * warn when follow missing (target) account * return wrapped err when bookmark/fave models can't be retrieved * simplify self account delete * warn log likely race condition * de-sillify status delete loop * move error check due north * warn when unfollowSideEffects has no target account * warn when no boost account is found * warn + dump follow when no account * more warnings * warn on fave account not set * move for loop inside anonymous function * fix funky logic * don't remove mutual account items on block; do make sure unfollow occurs in both directions! --- .../federation/federatingdb/federatingdb_test.go | 6 ++-- internal/federation/federatingdb/followers.go | 33 ++++++--------------- internal/federation/federatingdb/following.go | 34 ++++++---------------- internal/federation/federatingdb/inbox.go | 14 +++------ 4 files changed, 26 insertions(+), 61 deletions(-) (limited to 'internal/federation') diff --git a/internal/federation/federatingdb/federatingdb_test.go b/internal/federation/federatingdb/federatingdb_test.go index 99935ef55..32495a2f8 100644 --- a/internal/federation/federatingdb/federatingdb_test.go +++ b/internal/federation/federatingdb/federatingdb_test.go @@ -69,8 +69,10 @@ func (suite *FederatingDBTestSuite) SetupTest() { testrig.StartWorkers(&suite.state) suite.fromFederator = make(chan messages.FromFederator, 10) - suite.state.Workers.EnqueueFederator = func(ctx context.Context, msg messages.FromFederator) { - suite.fromFederator <- msg + suite.state.Workers.EnqueueFederator = func(ctx context.Context, msgs ...messages.FromFederator) { + for _, msg := range msgs { + suite.fromFederator <- msg + } } suite.db = testrig.NewTestDB(&suite.state) diff --git a/internal/federation/federatingdb/followers.go b/internal/federation/federatingdb/followers.go index d9a359227..d829a304c 100644 --- a/internal/federation/federatingdb/followers.go +++ b/internal/federation/federatingdb/followers.go @@ -22,9 +22,7 @@ import ( "fmt" "net/url" - "codeberg.org/gruf/go-kv" "github.com/superseriousbusiness/activity/streams/vocab" - "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/log" ) @@ -35,43 +33,30 @@ import ( // // The library makes this call only after acquiring a lock first. func (f *federatingDB) Followers(ctx context.Context, actorIRI *url.URL) (followers vocab.ActivityStreamsCollection, err error) { - l := log.WithContext(ctx). - WithFields(kv.Fields{ - {"id", actorIRI}, - }...) - l.Debug("entering Followers") - acct, err := f.getAccountForIRI(ctx, actorIRI) if err != nil { return nil, err } - acctFollowers, err := f.state.DB.GetAccountFollowedBy(ctx, acct.ID, false) + follows, err := f.state.DB.GetFollows(ctx, "", acct.ID) if err != nil { return nil, fmt.Errorf("Followers: db error getting followers for account id %s: %s", acct.ID, err) } - iris := []*url.URL{} - for _, follow := range acctFollowers { + iris := make([]*url.URL, 0, len(follows)) + for _, follow := range follows { if follow.Account == nil { - a, err := f.state.DB.GetAccountByID(ctx, follow.AccountID) - if err != nil { - errWrapped := fmt.Errorf("Followers: db error getting account id %s: %s", follow.AccountID, err) - if err == db.ErrNoEntries { - // no entry for this account id so it's probably been deleted and we haven't caught up yet - l.Error(errWrapped) - continue - } else { - // proper error - return nil, errWrapped - } - } - follow.Account = a + // Follow account no longer exists, + // for some reason. Skip this one. + log.WithContext(ctx).WithField("follow", follow).Warnf("follow missing account %s", follow.AccountID) + continue } + u, err := url.Parse(follow.Account.URI) if err != nil { return nil, err } + iris = append(iris, u) } diff --git a/internal/federation/federatingdb/following.go b/internal/federation/federatingdb/following.go index e42f90ad1..7b0db5660 100644 --- a/internal/federation/federatingdb/following.go +++ b/internal/federation/federatingdb/following.go @@ -22,9 +22,7 @@ import ( "fmt" "net/url" - "codeberg.org/gruf/go-kv" "github.com/superseriousbusiness/activity/streams/vocab" - "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/log" ) @@ -35,39 +33,25 @@ import ( // // The library makes this call only after acquiring a lock first. func (f *federatingDB) Following(ctx context.Context, actorIRI *url.URL) (following vocab.ActivityStreamsCollection, err error) { - l := log.WithContext(ctx). - WithFields(kv.Fields{ - {"id", actorIRI}, - }...) - l.Debug("entering Following") - acct, err := f.getAccountForIRI(ctx, actorIRI) if err != nil { return nil, err } - acctFollowing, err := f.state.DB.GetAccountFollows(ctx, acct.ID) + follows, err := f.state.DB.GetFollows(ctx, acct.ID, "") if err != nil { - return nil, fmt.Errorf("Following: db error getting following for account id %s: %s", acct.ID, err) + return nil, fmt.Errorf("Following: db error getting following for account id %s: %w", acct.ID, err) } - iris := []*url.URL{} - for _, follow := range acctFollowing { + iris := make([]*url.URL, 0, len(follows)) + for _, follow := range follows { if follow.TargetAccount == nil { - a, err := f.state.DB.GetAccountByID(ctx, follow.TargetAccountID) - if err != nil { - errWrapped := fmt.Errorf("Following: db error getting account id %s: %s", follow.TargetAccountID, err) - if err == db.ErrNoEntries { - // no entry for this account id so it's probably been deleted and we haven't caught up yet - l.Error(errWrapped) - continue - } else { - // proper error - return nil, errWrapped - } - } - follow.TargetAccount = a + // Follow target account no longer exists, + // for some reason. Skip this one. + log.WithContext(ctx).WithField("follow", follow).Warnf("follow missing target account %s", follow.TargetAccountID) + continue } + u, err := url.Parse(follow.TargetAccount.URI) if err != nil { return nil, err diff --git a/internal/federation/federatingdb/inbox.go b/internal/federation/federatingdb/inbox.go index 28664cbaf..954a192e4 100644 --- a/internal/federation/federatingdb/inbox.go +++ b/internal/federation/federatingdb/inbox.go @@ -89,22 +89,16 @@ func (f *federatingDB) InboxesForIRI(c context.Context, iri *url.URL) (inboxIRIs return nil, fmt.Errorf("couldn't find local account with username %s: %s", localAccountUsername, err) } - follows, err := f.state.DB.GetAccountFollowedBy(c, account.ID, false) + follows, err := f.state.DB.GetFollows(c, "", account.ID) if err != nil { return nil, fmt.Errorf("couldn't get followers of local account %s: %s", localAccountUsername, err) } for _, follow := range follows { - // make sure we retrieved the following account from the db if follow.Account == nil { - followingAccount, err := f.state.DB.GetAccountByID(c, follow.AccountID) - if err != nil { - if err == db.ErrNoEntries { - continue - } - return nil, fmt.Errorf("error retrieving account with id %s: %s", follow.AccountID, err) - } - follow.Account = followingAccount + // No account exists for this follow, + // for some reason. Just skip it. + continue } // deliver to a shared inbox if we have that option -- cgit v1.2.3