[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

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

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 {
+func (p *processor) wipeStatus(ctx context.Context, statusToDelete *gtsmodel.Status, deleteAttachments bool) error {
-	// delete all attachments for this status
+	// either delete all attachments for this status, or simply
-	for _, a := range statusToDelete.AttachmentIDs {
+	// unattach all attachments for this status, so they'll be
-		if err := p.mediaProcessor.Delete(ctx, a); err != nil {
+	// cleaned later by a separate process; reason to unattach rather
-			return err
+	// 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
+			}
 		}
 	}
 

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

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)

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 <http://www.gnu.org/licenses/>.
+*/
+
+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
+}

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 <http://www.gnu.org/licenses/>.
+*/
+
+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{})
+}