[bugfix] Clean up boosts of status when the status itself is deleted (#579)

* move status wiping logic to fromcommon.go

* delete reblogs of status when a status is deleted

* add admin boost of zork to test model

* update tests to make them more determinate

* Merge branch 'main' into status_reblog_cleanup

* move status wiping logic to fromcommon.go

* delete reblogs of status when a status is deleted

* add admin boost of zork to test model

* update tests to make them more determinate

* Merge branch 'main' into status_reblog_cleanup

* test status delete via client api

* go fmt

internal/processing/fromclientapi.go

@@ -284,27 +284,7 @@ func (p *processor) processDeleteStatusFromClientAPI(ctx context.Context, client
 		statusToDelete.Account = clientMsg.OriginAccount
 	}
 
-	// delete all attachments for this status
+	if err := p.wipeStatus(ctx, statusToDelete); err != nil {
-	for _, a := range statusToDelete.AttachmentIDs {
-		if err := p.mediaProcessor.Delete(ctx, a); err != nil {
-			return err
-		}
-	}
-
-	// delete all mentions for this status
-	for _, m := range statusToDelete.MentionIDs {
-		if err := p.db.DeleteByID(ctx, m, &gtsmodel.Mention{}); err != nil {
-			return err
-		}
-	}
-
-	// delete all notifications for this status
-	if err := p.db.DeleteWhere(ctx, []db.Where{{Key: "status_id", Value: statusToDelete.ID}}, &[]*gtsmodel.Notification{}); err != nil {
-		return err
-	}
-
-	// delete this status from any and all timelines
-	if err := p.deleteStatusFromTimelines(ctx, statusToDelete); err != nil {
 		return err
 	}
 

internal/processing/fromclientapi_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/stretchr/testify/suite"
 	"github.com/superseriousbusiness/gotosocial/internal/ap"
 	"github.com/superseriousbusiness/gotosocial/internal/api/model"
+	"github.com/superseriousbusiness/gotosocial/internal/db"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 	"github.com/superseriousbusiness/gotosocial/internal/messages"
 	"github.com/superseriousbusiness/gotosocial/internal/stream"
@@ -113,6 +114,52 @@ func (suite *FromClientAPITestSuite) TestProcessStreamNewStatus() {
 	suite.Empty(irrelevantStream.Messages)
 }
 
+func (suite *FromClientAPITestSuite) TestProcessStatusDelete() {
+	ctx := context.Background()
+
+	deletingAccount := suite.testAccounts["local_account_1"]
+	receivingAccount := suite.testAccounts["local_account_2"]
+
+	deletedStatus := suite.testStatuses["local_account_1_status_1"]
+	boostOfDeletedStatus := suite.testStatuses["admin_account_status_4"]
+
+	// open a home timeline stream for turtle, who follows zork
+	wssStream, errWithCode := suite.processor.OpenStreamForAccount(ctx, receivingAccount, stream.TimelineHome)
+	suite.NoError(errWithCode)
+
+	// delete the status from the db first, to mimic what would have already happened earlier up the flow
+	err := suite.db.DeleteByID(ctx, deletedStatus.ID, &gtsmodel.Status{})
+	suite.NoError(err)
+
+	// process the status delete
+	err = suite.processor.ProcessFromClientAPI(ctx, messages.FromClientAPI{
+		APObjectType:   ap.ObjectNote,
+		APActivityType: ap.ActivityDelete,
+		GTSModel:       deletedStatus,
+		OriginAccount:  deletingAccount,
+	})
+	suite.NoError(err)
+
+	// turtle's stream should have the delete of admin's boost in it now
+	msg := <-wssStream.Messages
+	suite.Equal(stream.EventTypeDelete, msg.Event)
+	suite.Equal(boostOfDeletedStatus.ID, msg.Payload)
+	suite.EqualValues([]string{stream.TimelineHome}, msg.Stream)
+
+	// turtle's stream should also have the delete of the message itself in it
+	msg = <-wssStream.Messages
+	suite.Equal(stream.EventTypeDelete, msg.Event)
+	suite.Equal(deletedStatus.ID, msg.Payload)
+	suite.EqualValues([]string{stream.TimelineHome}, msg.Stream)
+
+	// stream should now be empty
+	suite.Empty(wssStream.Messages)
+
+	// the boost should no longer be in the database
+	_, err = suite.db.GetStatusByID(ctx, boostOfDeletedStatus.ID)
+	suite.ErrorIs(err, db.ErrNoEntries)
+}
+
 func TestFromClientAPITestSuite(t *testing.T) {
 	suite.Run(t, &FromClientAPITestSuite{})
 }

internal/processing/fromcommon.go

@@ -441,3 +441,47 @@ func (p *processor) deleteStatusFromTimelines(ctx context.Context, status *gtsmo
 
 	return p.streamingProcessor.StreamDelete(status.ID)
 }
+
+// wipeStatus contains common logic used to totally delete a status
+// + all its attachments, notifications, boosts, and timeline entries.
+func (p *processor) wipeStatus(ctx context.Context, statusToDelete *gtsmodel.Status) error {
+	// delete all attachments for this status
+	for _, a := range statusToDelete.AttachmentIDs {
+		if err := p.mediaProcessor.Delete(ctx, a); err != nil {
+			return err
+		}
+	}
+
+	// delete all mentions for this status
+	for _, m := range statusToDelete.MentionIDs {
+		if err := p.db.DeleteByID(ctx, m, &gtsmodel.Mention{}); err != nil {
+			return err
+		}
+	}
+
+	// delete all notifications for this status
+	if err := p.db.DeleteWhere(ctx, []db.Where{{Key: "status_id", Value: statusToDelete.ID}}, &[]*gtsmodel.Notification{}); err != nil {
+		return err
+	}
+
+	// delete all boosts for this status + remove them from timelines
+	boosts, err := p.db.GetStatusReblogs(ctx, statusToDelete)
+	if err != nil {
+		return err
+	}
+	for _, b := range boosts {
+		if err := p.deleteStatusFromTimelines(ctx, b); err != nil {
+			return err
+		}
+		if err := p.db.DeleteByID(ctx, b.ID, b); err != nil {
+			return err
+		}
+	}
+
+	// delete this status from any and all timelines
+	if err := p.deleteStatusFromTimelines(ctx, statusToDelete); err != nil {
+		return err
+	}
+
+	return nil
+}

internal/processing/fromfederator.go

@@ -26,7 +26,6 @@ import (
 
 	"github.com/sirupsen/logrus"
 	"github.com/superseriousbusiness/gotosocial/internal/ap"
-	"github.com/superseriousbusiness/gotosocial/internal/db"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 	"github.com/superseriousbusiness/gotosocial/internal/id"
 	"github.com/superseriousbusiness/gotosocial/internal/messages"
@@ -342,36 +341,12 @@ func (p *processor) processUpdateAccountFromFederator(ctx context.Context, feder
 
 // processDeleteStatusFromFederator handles Activity Delete and Object Note
 func (p *processor) processDeleteStatusFromFederator(ctx context.Context, federatorMsg messages.FromFederator) error {
-	// TODO: handle side effects of status deletion here:
-	// 1. delete all media associated with status
-	// 2. delete boosts of status
-	// 3. etc etc etc
 	statusToDelete, ok := federatorMsg.GTSModel.(*gtsmodel.Status)
 	if !ok {
 		return errors.New("note was not parseable as *gtsmodel.Status")
 	}
 
-	// delete all attachments for this status
+	return p.wipeStatus(ctx, statusToDelete)
-	for _, a := range statusToDelete.AttachmentIDs {
-		if err := p.mediaProcessor.Delete(ctx, a); err != nil {
-			return err
-		}
-	}
-
-	// delete all mentions for this status
-	for _, m := range statusToDelete.MentionIDs {
-		if err := p.db.DeleteByID(ctx, m, &gtsmodel.Mention{}); err != nil {
-			return err
-		}
-	}
-
-	// delete all notifications for this status
-	if err := p.db.DeleteWhere(ctx, []db.Where{{Key: "status_id", Value: statusToDelete.ID}}, &[]*gtsmodel.Notification{}); err != nil {
-		return err
-	}
-
-	// remove this status from any and all timelines
-	return p.deleteStatusFromTimelines(ctx, statusToDelete)
 }
 
 // processDeleteAccountFromFederator handles Activity Delete and Object Profile