From b2810fedf201dc9ae80ea8ec6d48e5cf7e21e6ea Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Wed, 18 May 2022 23:13:03 +0200 Subject: [PATCH] [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 | 22 +---------- internal/processing/fromclientapi_test.go | 47 +++++++++++++++++++++++ internal/processing/fromcommon.go | 44 +++++++++++++++++++++ internal/processing/fromfederator.go | 27 +------------ 4 files changed, 93 insertions(+), 47 deletions(-) diff --git a/internal/processing/fromclientapi.go b/internal/processing/fromclientapi.go index 4935f5627..13be84305 100644 --- a/internal/processing/fromclientapi.go +++ b/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 - 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, >smodel.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 { + if err := p.wipeStatus(ctx, statusToDelete); err != nil { return err } diff --git a/internal/processing/fromclientapi_test.go b/internal/processing/fromclientapi_test.go index fd89be270..014266b37 100644 --- a/internal/processing/fromclientapi_test.go +++ b/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, >smodel.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{}) } diff --git a/internal/processing/fromcommon.go b/internal/processing/fromcommon.go index 1b470918d..e9a2e4994 100644 --- a/internal/processing/fromcommon.go +++ b/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, >smodel.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 +} diff --git a/internal/processing/fromfederator.go b/internal/processing/fromfederator.go index bb2cb5323..06df17d91 100644 --- a/internal/processing/fromfederator.go +++ b/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 - 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, >smodel.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) + return p.wipeStatus(ctx, statusToDelete) } // processDeleteAccountFromFederator handles Activity Delete and Object Profile