[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

@@ -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)
 

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 {
+		Where("? = ?", bun.Ident("id"), follow.ID).
+		Exec(ctx)
 	return r.conn.ProcessError(err)
-	}
-
-	// Delete original follow request notification
-	return r.state.DB.DeleteNotifications(ctx, []string{
-		string(gtsmodel.NotificationFollowRequest),
-	}, targetAccountID, sourceAccountID)
 }
 
 func (r *relationshipDB) DeleteFollowRequestByID(ctx context.Context, id string) error {

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"]

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
 

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
 }