From 0846b76e9343fd90a33f5a8864f39cf071d3841e Mon Sep 17 00:00:00 2001 From: tobi <31960611+tsmethurst@users.noreply.github.com> Date: Fri, 24 Jun 2022 17:17:40 +0200 Subject: [PATCH] [bugfix] Fix 404 on status delete redraft (#668) * add unattach function to media processor * call delete or unattach appropriately unattach from client api, delete from federated api * typo fix --- internal/processing/fromclientapi.go | 7 ++- internal/processing/fromcommon.go | 22 ++++++-- internal/processing/fromfederator.go | 6 ++- internal/processing/media/media.go | 3 ++ internal/processing/media/unattach.go | 59 ++++++++++++++++++++++ internal/processing/media/unattach_test.go | 53 +++++++++++++++++++ 6 files changed, 143 insertions(+), 7 deletions(-) create mode 100644 internal/processing/media/unattach.go create mode 100644 internal/processing/media/unattach_test.go diff --git a/internal/processing/fromclientapi.go b/internal/processing/fromclientapi.go index 13be84305..24465a059 100644 --- a/internal/processing/fromclientapi.go +++ b/internal/processing/fromclientapi.go @@ -284,7 +284,12 @@ func (p *processor) processDeleteStatusFromClientAPI(ctx context.Context, client statusToDelete.Account = clientMsg.OriginAccount } - if err := p.wipeStatus(ctx, statusToDelete); err != nil { + // don't delete attachments, just unattach them; + // since this request comes from the client API + // and the poster might want to use the attachments + // again in a new post + deleteAttachments := false + if err := p.wipeStatus(ctx, statusToDelete, deleteAttachments); err != nil { return err } diff --git a/internal/processing/fromcommon.go b/internal/processing/fromcommon.go index e9a2e4994..2cac20193 100644 --- a/internal/processing/fromcommon.go +++ b/internal/processing/fromcommon.go @@ -444,11 +444,23 @@ func (p *processor) deleteStatusFromTimelines(ctx context.Context, status *gtsmo // 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 +func (p *processor) wipeStatus(ctx context.Context, statusToDelete *gtsmodel.Status, deleteAttachments bool) error { + // either delete all attachments for this status, or simply + // unattach all attachments for this status, so they'll be + // cleaned later by a separate process; reason to unattach rather + // than delete is that the poster might want to reattach them + // to another status immediately (in case of delete + redraft) + if deleteAttachments { + for _, a := range statusToDelete.AttachmentIDs { + if err := p.mediaProcessor.Delete(ctx, a); err != nil { + return err + } + } + } else { + for _, a := range statusToDelete.AttachmentIDs { + if _, err := p.mediaProcessor.Unattach(ctx, statusToDelete.Account, a); err != nil { + return err + } } } diff --git a/internal/processing/fromfederator.go b/internal/processing/fromfederator.go index 60f5cc787..e39a6b4e8 100644 --- a/internal/processing/fromfederator.go +++ b/internal/processing/fromfederator.go @@ -367,7 +367,11 @@ func (p *processor) processDeleteStatusFromFederator(ctx context.Context, federa return errors.New("note was not parseable as *gtsmodel.Status") } - return p.wipeStatus(ctx, statusToDelete) + // delete attachments from this status since this request + // comes from the federating API, and there's no way the + // poster can do a delete + redraft for it on our instance + deleteAttachments := true + return p.wipeStatus(ctx, statusToDelete, deleteAttachments) } // processDeleteAccountFromFederator handles Activity Delete and Object Profile diff --git a/internal/processing/media/media.go b/internal/processing/media/media.go index 05bea615f..50cbc1b3c 100644 --- a/internal/processing/media/media.go +++ b/internal/processing/media/media.go @@ -37,6 +37,9 @@ type Processor interface { Create(ctx context.Context, account *gtsmodel.Account, form *apimodel.AttachmentRequest) (*apimodel.Attachment, gtserror.WithCode) // Delete deletes the media attachment with the given ID, including all files pertaining to that attachment. Delete(ctx context.Context, mediaAttachmentID string) gtserror.WithCode + // Unattach unattaches the media attachment with the given ID from any statuses it was attached to, making it available + // for reattachment again. + Unattach(ctx context.Context, account *gtsmodel.Account, mediaAttachmentID string) (*apimodel.Attachment, gtserror.WithCode) // GetFile retrieves a file from storage and streams it back to the caller via an io.reader embedded in *apimodel.Content. GetFile(ctx context.Context, account *gtsmodel.Account, form *apimodel.GetContentRequestForm) (*apimodel.Content, gtserror.WithCode) GetCustomEmojis(ctx context.Context) ([]*apimodel.Emoji, gtserror.WithCode) diff --git a/internal/processing/media/unattach.go b/internal/processing/media/unattach.go new file mode 100644 index 000000000..bb09525fe --- /dev/null +++ b/internal/processing/media/unattach.go @@ -0,0 +1,59 @@ +/* + GoToSocial + Copyright (C) 2021-2022 GoToSocial Authors admin@gotosocial.org + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU Affero General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Affero General Public License for more details. + + You should have received a copy of the GNU Affero General Public License + along with this program. If not, see . +*/ + +package media + +import ( + "context" + "errors" + "fmt" + "time" + + apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" + "github.com/superseriousbusiness/gotosocial/internal/db" + "github.com/superseriousbusiness/gotosocial/internal/gtserror" + "github.com/superseriousbusiness/gotosocial/internal/gtsmodel" +) + +func (p *processor) Unattach(ctx context.Context, account *gtsmodel.Account, mediaAttachmentID string) (*apimodel.Attachment, gtserror.WithCode) { + attachment, err := p.db.GetAttachmentByID(ctx, mediaAttachmentID) + if err != nil { + if err == db.ErrNoEntries { + return nil, gtserror.NewErrorNotFound(errors.New("attachment doesn't exist in the db")) + } + return nil, gtserror.NewErrorNotFound(fmt.Errorf("db error getting attachment: %s", err)) + } + + if attachment.AccountID != account.ID { + return nil, gtserror.NewErrorNotFound(errors.New("attachment not owned by requesting account")) + } + + attachment.UpdatedAt = time.Now() + attachment.StatusID = "" + + if err := p.db.UpdateByPrimaryKey(ctx, attachment); err != nil { + return nil, gtserror.NewErrorNotFound(fmt.Errorf("db error updating attachment: %s", err)) + } + + a, err := p.tc.AttachmentToAPIAttachment(ctx, attachment) + if err != nil { + return nil, gtserror.NewErrorNotFound(fmt.Errorf("error converting attachment: %s", err)) + } + + return &a, nil +} diff --git a/internal/processing/media/unattach_test.go b/internal/processing/media/unattach_test.go new file mode 100644 index 000000000..60efc2688 --- /dev/null +++ b/internal/processing/media/unattach_test.go @@ -0,0 +1,53 @@ +/* + GoToSocial + Copyright (C) 2021-2022 GoToSocial Authors admin@gotosocial.org + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU Affero General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU Affero General Public License for more details. + + You should have received a copy of the GNU Affero General Public License + along with this program. If not, see . +*/ + +package media_test + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/suite" +) + +type UnattachTestSuite struct { + MediaStandardTestSuite +} + +func (suite *GetFileTestSuite) TestUnattachMedia() { + ctx := context.Background() + + testAttachment := suite.testAttachments["admin_account_status_1_attachment_1"] + testAccount := suite.testAccounts["admin_account"] + suite.NotEmpty(testAttachment.StatusID) + + a, err := suite.mediaProcessor.Unattach(ctx, testAccount, testAttachment.ID) + suite.NoError(err) + suite.NotNil(a) + + dbAttachment, errWithCode := suite.db.GetAttachmentByID(ctx, a.ID) + suite.NoError(errWithCode) + + suite.WithinDuration(dbAttachment.UpdatedAt, time.Now(), 1*time.Minute) + suite.Empty(dbAttachment.StatusID) +} + +func TestUnattachTestSuite(t *testing.T) { + suite.Run(t, &UnattachTestSuite{}) +}