From f40bb02f31ca72f1528cf40291e86024509e34d6 Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Sat, 8 Jul 2023 16:43:12 +0200 Subject: [PATCH] [bugfix] Delete mutual follow (requests) when receiving block from remote (#1960) * [bugfix] Delete mutual follow (requests) on block * fix test --- internal/db/bundb/relationship_follow.go | 23 +++++++++ internal/db/bundb/relationship_follow_req.go | 40 ++++++++------- internal/db/bundb/relationship_test.go | 15 +++++- internal/db/relationship.go | 6 +++ internal/processing/fromfederator.go | 52 +++++++++++++++++--- 5 files changed, 112 insertions(+), 24 deletions(-) diff --git a/internal/db/bundb/relationship_follow.go b/internal/db/bundb/relationship_follow.go index 39b85075c..88850e72a 100644 --- a/internal/db/bundb/relationship_follow.go +++ b/internal/db/bundb/relationship_follow.go @@ -232,6 +232,29 @@ func (r *relationshipDB) deleteFollow(ctx context.Context, id string) error { return nil } +func (r *relationshipDB) DeleteFollow(ctx context.Context, sourceAccountID string, targetAccountID string) error { + defer r.state.Caches.GTS.Follow().Invalidate("AccountID.TargetAccountID", sourceAccountID, targetAccountID) + + // Load follow into cache before attempting a delete, + // as we need it cached in order to trigger the invalidate + // callback. This in turn invalidates others. + follow, err := r.GetFollow( + gtscontext.SetBarebones(ctx), + sourceAccountID, + targetAccountID, + ) + if err != nil { + if errors.Is(err, db.ErrNoEntries) { + // Already gone. + return nil + } + return err + } + + // Finally delete follow from DB. + return r.deleteFollow(ctx, follow.ID) +} + func (r *relationshipDB) DeleteFollowByID(ctx context.Context, id string) error { defer r.state.Caches.GTS.Follow().Invalidate("ID", id) diff --git a/internal/db/bundb/relationship_follow_req.go b/internal/db/bundb/relationship_follow_req.go index c2ac3d2f1..f233bdd58 100644 --- a/internal/db/bundb/relationship_follow_req.go +++ b/internal/db/bundb/relationship_follow_req.go @@ -231,36 +231,42 @@ func (r *relationshipDB) AcceptFollowRequest(ctx context.Context, sourceAccountI } func (r *relationshipDB) RejectFollowRequest(ctx context.Context, sourceAccountID string, targetAccountID string) db.Error { + // Delete follow request first. + if err := r.DeleteFollowRequest(ctx, sourceAccountID, targetAccountID); err != nil { + return err + } + + // Delete follow request notification + return r.state.DB.DeleteNotifications(ctx, []string{ + string(gtsmodel.NotificationFollowRequest), + }, targetAccountID, sourceAccountID) +} + +func (r *relationshipDB) DeleteFollowRequest(ctx context.Context, sourceAccountID string, targetAccountID string) error { defer r.state.Caches.GTS.FollowRequest().Invalidate("AccountID.TargetAccountID", sourceAccountID, targetAccountID) // Load followreq into cache before attempting a delete, // as we need it cached in order to trigger the invalidate // callback. This in turn invalidates others. - _, err := r.GetFollowRequest(gtscontext.SetBarebones(ctx), + follow, err := r.GetFollowRequest( + gtscontext.SetBarebones(ctx), sourceAccountID, targetAccountID, ) if err != nil { + if errors.Is(err, db.ErrNoEntries) { + // Already gone. + return nil + } return err } - // Attempt to delete follow request. - if _, err = r.conn.NewDelete(). + // Finally delete followreq from DB. + _, err = r.conn.NewDelete(). Table("follow_requests"). - Where("? = ? AND ? = ?", - bun.Ident("account_id"), - sourceAccountID, - bun.Ident("target_account_id"), - targetAccountID, - ). - Exec(ctx); err != nil { - return r.conn.ProcessError(err) - } - - // Delete original follow request notification - return r.state.DB.DeleteNotifications(ctx, []string{ - string(gtsmodel.NotificationFollowRequest), - }, targetAccountID, sourceAccountID) + Where("? = ?", bun.Ident("id"), follow.ID). + Exec(ctx) + return r.conn.ProcessError(err) } func (r *relationshipDB) DeleteFollowRequestByID(ctx context.Context, id string) error { diff --git a/internal/db/bundb/relationship_test.go b/internal/db/bundb/relationship_test.go index 63fdb9632..d3f4a31d1 100644 --- a/internal/db/bundb/relationship_test.go +++ b/internal/db/bundb/relationship_test.go @@ -734,7 +734,7 @@ func (suite *RelationshipTestSuite) TestRejectFollowRequestNotExisting() { targetAccount := suite.testAccounts["local_account_2"] err := suite.db.RejectFollowRequest(ctx, account.ID, targetAccount.ID) - suite.ErrorIs(err, db.ErrNoEntries) + suite.NoError(err) } func (suite *RelationshipTestSuite) TestGetAccountFollowRequests() { @@ -836,6 +836,19 @@ func (suite *RelationshipTestSuite) TestGetFollowNotExisting() { suite.Nil(follow) } +func (suite *RelationshipTestSuite) TestDeleteFollow() { + ctx := context.Background() + originAccount := suite.testAccounts["local_account_1"] + targetAccount := suite.testAccounts["admin_account"] + + err := suite.db.DeleteFollow(ctx, originAccount.ID, targetAccount.ID) + suite.NoError(err) + + follow, err := suite.db.GetFollow(ctx, originAccount.ID, targetAccount.ID) + suite.EqualError(err, db.ErrNoEntries.Error()) + suite.Nil(follow) +} + func (suite *RelationshipTestSuite) TestUnfollowRequestExisting() { ctx := context.Background() originAccount := suite.testAccounts["admin_account"] diff --git a/internal/db/relationship.go b/internal/db/relationship.go index 99093591c..e4b81c003 100644 --- a/internal/db/relationship.go +++ b/internal/db/relationship.go @@ -97,12 +97,18 @@ type Relationship interface { // UpdateFollowRequest updates one follow request by ID. UpdateFollowRequest(ctx context.Context, followRequest *gtsmodel.FollowRequest, columns ...string) error + // DeleteFollow deletes a follow if it exists between source and target accounts. + DeleteFollow(ctx context.Context, sourceAccountID string, targetAccountID string) error + // DeleteFollowByID deletes a follow from the database with the given ID. DeleteFollowByID(ctx context.Context, id string) error // DeleteFollowByURI deletes a follow from the database with the given URI. DeleteFollowByURI(ctx context.Context, uri string) error + // DeleteFollowRequest deletes a follow request if it exists between source and target accounts. + DeleteFollowRequest(ctx context.Context, sourceAccountID string, targetAccountID string) error + // DeleteFollowRequestByID deletes a follow request from the database with the given ID. DeleteFollowRequestByID(ctx context.Context, id string) error diff --git a/internal/processing/fromfederator.go b/internal/processing/fromfederator.go index 335b1a102..7152393db 100644 --- a/internal/processing/fromfederator.go +++ b/internal/processing/fromfederator.go @@ -352,18 +352,58 @@ func (p *Processor) processCreateAnnounceFromFederator(ctx context.Context, fede func (p *Processor) processCreateBlockFromFederator(ctx context.Context, federatorMsg messages.FromFederator) error { block, ok := federatorMsg.GTSModel.(*gtsmodel.Block) if !ok { - return errors.New("block was not parseable as *gtsmodel.Block") + return gtserror.New("block was not parseable as *gtsmodel.Block") } - // remove any of the blocking account's statuses from the blocked account's timeline, and vice versa + // Remove each account's posts from the other's timelines. + // + // First home timelines. if err := p.state.Timelines.Home.WipeItemsFromAccountID(ctx, block.AccountID, block.TargetAccountID); err != nil { - return err + return gtserror.Newf("%w", err) } + if err := p.state.Timelines.Home.WipeItemsFromAccountID(ctx, block.TargetAccountID, block.AccountID); err != nil { - return err + return gtserror.Newf("%w", err) + } + + // Now list timelines. + if err := p.state.Timelines.List.WipeItemsFromAccountID(ctx, block.AccountID, block.TargetAccountID); err != nil { + return gtserror.Newf("%w", err) + } + + if err := p.state.Timelines.List.WipeItemsFromAccountID(ctx, block.TargetAccountID, block.AccountID); err != nil { + return gtserror.Newf("%w", err) + } + + // Remove any follows that existed between blocker + blockee. + if err := p.state.DB.DeleteFollowRequest(ctx, block.AccountID, block.TargetAccountID); err != nil { + return gtserror.Newf( + "db error deleting follow from %s targeting %s: %w", + block.AccountID, block.TargetAccountID, err, + ) + } + + if err := p.state.DB.DeleteFollowRequest(ctx, block.TargetAccountID, block.AccountID); err != nil { + return gtserror.Newf( + "db error deleting follow from %s targeting %s: %w", + block.TargetAccountID, block.AccountID, err, + ) + } + + // Remove any follow requests that existed between blocker + blockee. + if err := p.state.DB.DeleteFollowRequest(ctx, block.AccountID, block.TargetAccountID); err != nil { + return gtserror.Newf( + "db error deleting follow request from %s targeting %s: %w", + block.AccountID, block.TargetAccountID, err, + ) + } + + if err := p.state.DB.DeleteFollowRequest(ctx, block.TargetAccountID, block.AccountID); err != nil { + return gtserror.Newf( + "db error deleting follow request from %s targeting %s: %w", + block.TargetAccountID, block.AccountID, err, + ) } - // TODO: same with notifications - // TODO: same with bookmarks return nil }