diff --git a/models/fixtures/attachment.yml b/models/fixtures/attachment.yml index 7882d8bff20..1c1eb402bef 100644 --- a/models/fixtures/attachment.yml +++ b/models/fixtures/attachment.yml @@ -99,7 +99,7 @@ comment_id: 0 name: attach1 download_count: 0 - size: 0 + size: 29 created_unix: 946684800 - diff --git a/models/issues/comment.go b/models/issues/comment.go index d22f08fa876..f393e483878 100644 --- a/models/issues/comment.go +++ b/models/issues/comment.go @@ -390,18 +390,6 @@ func (c *Comment) LoadPoster(ctx context.Context) (err error) { return err } -// AfterDelete is invoked from XORM after the object is deleted. -func (c *Comment) AfterDelete(ctx context.Context) { - if c.ID <= 0 { - return - } - - _, err := repo_model.DeleteAttachmentsByComment(ctx, c.ID, true) - if err != nil { - log.Info("Could not delete files for comment %d on issue #%d: %s", c.ID, c.IssueID, err) - } -} - // HTMLURL formats a URL-string to the issue-comment func (c *Comment) HTMLURL(ctx context.Context) string { err := c.LoadIssue(ctx) @@ -611,6 +599,11 @@ func UpdateCommentAttachments(ctx context.Context, c *Comment, uuids []string) e return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err) } for i := range attachments { + if attachments[i].CommentID == c.ID && attachments[i].IssueID == c.IssueID { + continue + } else if attachments[i].IssueID != 0 || attachments[i].CommentID != 0 { + return util.NewPermissionDeniedErrorf("update comment attachments permission denied") + } attachments[i].IssueID = c.IssueID attachments[i].CommentID = c.ID if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil { diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 4f899453b5f..b1f07db50ce 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -386,6 +386,7 @@ func prepareMigrationTasks() []*migration { // Gitea 1.24.0 ends at database version 321 newMigration(321, "Use LONGTEXT for some columns and fix review_state.updated_files column", v1_25.UseLongTextInSomeColumnsAndFixBugs), + newMigration(322, "Add storage_path_deletion table", v1_25.AddStoragePathDeletion), } return preparedMigrations } diff --git a/models/migrations/v1_25/v322.go b/models/migrations/v1_25/v322.go new file mode 100644 index 00000000000..a804f4ea7bf --- /dev/null +++ b/models/migrations/v1_25/v322.go @@ -0,0 +1,26 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_25 + +import ( + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" +) + +func AddStoragePathDeletion(x *xorm.Engine) error { + // StoragePathDeletion represents a file or directory that is pending deletion. + type StoragePathDeletion struct { + ID int64 + StorageName string // storage name defines in storage module + PathType int // 1 for file, 2 for directory + RelativePath string `xorm:"TEXT"` + DeleteFailedCount int `xorm:"DEFAULT 0 NOT NULL"` // Number of times the deletion failed, used to prevent infinite loop + LastDeleteFailedReason string `xorm:"TEXT"` // Last reason the deletion failed, used to prevent infinite loop + LastDeleteFailedTime timeutil.TimeStamp // Last time the deletion failed, used to prevent infinite loop + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + } + + return x.Sync(new(StoragePathDeletion)) +} diff --git a/models/repo/attachment.go b/models/repo/attachment.go index 835bee54025..b82317d47ba 100644 --- a/models/repo/attachment.go +++ b/models/repo/attachment.go @@ -8,11 +8,10 @@ import ( "errors" "fmt" "net/url" - "os" "path" "code.gitea.io/gitea/models/db" - "code.gitea.io/gitea/modules/log" + system_model "code.gitea.io/gitea/models/system" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/timeutil" @@ -166,16 +165,10 @@ func GetAttachmentByReleaseIDFileName(ctx context.Context, releaseID int64, file return attach, nil } -// DeleteAttachment deletes the given attachment and optionally the associated file. -func DeleteAttachment(ctx context.Context, a *Attachment, remove bool) error { - _, err := DeleteAttachments(ctx, []*Attachment{a}, remove) - return err -} - -// DeleteAttachments deletes the given attachments and optionally the associated files. -func DeleteAttachments(ctx context.Context, attachments []*Attachment, remove bool) (int, error) { +// DeleteAttachments delete the given attachments and add disk files to pending deletion +func DeleteAttachments(ctx context.Context, attachments []*Attachment) ([]int64, error) { if len(attachments) == 0 { - return 0, nil + return nil, nil } ids := make([]int64, 0, len(attachments)) @@ -183,42 +176,28 @@ func DeleteAttachments(ctx context.Context, attachments []*Attachment, remove bo ids = append(ids, a.ID) } - cnt, err := db.GetEngine(ctx).In("id", ids).NoAutoCondition().Delete(attachments[0]) - if err != nil { - return 0, err - } - - if remove { - for i, a := range attachments { - if err := storage.Attachments.Delete(a.RelativePath()); err != nil { - if !errors.Is(err, os.ErrNotExist) { - return i, err - } - log.Warn("Attachment file not found when deleting: %s", a.RelativePath()) - } + return db.WithTx2(ctx, func(ctx context.Context) ([]int64, error) { + // delete attachments from database + if _, err := db.GetEngine(ctx).Table("attachment").In("id", ids).Delete(); err != nil { + return nil, err } - } - return int(cnt), nil -} -// DeleteAttachmentsByIssue deletes all attachments associated with the given issue. -func DeleteAttachmentsByIssue(ctx context.Context, issueID int64, remove bool) (int, error) { - attachments, err := GetAttachmentsByIssueID(ctx, issueID) - if err != nil { - return 0, err - } + // add disk files to pending deletion table as well + var deletionIDs []int64 + for _, a := range attachments { + pendingDeletion := &system_model.StoragePathDeletion{ + StorageName: storage.AttachmentStorageName, + PathType: system_model.PathFile, + RelativePath: a.RelativePath(), + } + if err := db.Insert(ctx, pendingDeletion); err != nil { + return nil, fmt.Errorf("insert pending deletion: %w", err) + } - return DeleteAttachments(ctx, attachments, remove) -} - -// DeleteAttachmentsByComment deletes all attachments associated with the given comment. -func DeleteAttachmentsByComment(ctx context.Context, commentID int64, remove bool) (int, error) { - attachments, err := GetAttachmentsByCommentID(ctx, commentID) - if err != nil { - return 0, err - } - - return DeleteAttachments(ctx, attachments, remove) + deletionIDs = append(deletionIDs, pendingDeletion.ID) // Collect pending deletions + } + return deletionIDs, nil + }) } // UpdateAttachmentByUUID Updates attachment via uuid @@ -243,12 +222,6 @@ func UpdateAttachment(ctx context.Context, atta *Attachment) error { return err } -// DeleteAttachmentsByRelease deletes all attachments associated with the given release. -func DeleteAttachmentsByRelease(ctx context.Context, releaseID int64) error { - _, err := db.GetEngine(ctx).Where("release_id = ?", releaseID).Delete(&Attachment{}) - return err -} - // CountOrphanedAttachments returns the number of bad attachments func CountOrphanedAttachments(ctx context.Context) (int64, error) { return db.GetEngine(ctx).Where("(issue_id > 0 and issue_id not in (select id from issue)) or (release_id > 0 and release_id not in (select id from `release`))"). diff --git a/models/repo/attachment_test.go b/models/repo/attachment_test.go index c059ffd39a9..91d44159f22 100644 --- a/models/repo/attachment_test.go +++ b/models/repo/attachment_test.go @@ -42,26 +42,6 @@ func TestGetByCommentOrIssueID(t *testing.T) { assert.Len(t, attachments, 2) } -func TestDeleteAttachments(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - - count, err := repo_model.DeleteAttachmentsByIssue(db.DefaultContext, 4, false) - assert.NoError(t, err) - assert.Equal(t, 2, count) - - count, err = repo_model.DeleteAttachmentsByComment(db.DefaultContext, 2, false) - assert.NoError(t, err) - assert.Equal(t, 2, count) - - err = repo_model.DeleteAttachment(db.DefaultContext, &repo_model.Attachment{ID: 8}, false) - assert.NoError(t, err) - - attachment, err := repo_model.GetAttachmentByUUID(db.DefaultContext, "a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a18") - assert.Error(t, err) - assert.True(t, repo_model.IsErrAttachmentNotExist(err)) - assert.Nil(t, attachment) -} - func TestGetAttachmentByID(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) diff --git a/models/system/storage_cleanup.go b/models/system/storage_cleanup.go new file mode 100644 index 00000000000..3151b0a62ae --- /dev/null +++ b/models/system/storage_cleanup.go @@ -0,0 +1,42 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package system + +import ( + "context" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/timeutil" +) + +const ( + PathFile = 1 // PathTypeFile represents a file + PathDir = 2 // PathTypeDir represents a directory +) + +// StoragePathDeletion represents a file or directory that is pending deletion. +type StoragePathDeletion struct { + ID int64 + StorageName string // storage name defines in storage module + PathType int // 1 for file, 2 for directory + RelativePath string `xorm:"TEXT"` + DeleteFailedCount int `xorm:"DEFAULT 0 NOT NULL"` // Number of times the deletion failed, used to prevent infinite loop + LastDeleteFailedReason string `xorm:"TEXT"` // Last reason the deletion failed, used to prevent infinite loop + LastDeleteFailedTime timeutil.TimeStamp // Last time the deletion failed, used to prevent infinite loop + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` +} + +func init() { + db.RegisterModel(new(StoragePathDeletion)) +} + +func UpdateDeletionFailure(ctx context.Context, deletion *StoragePathDeletion, err error) error { + deletion.DeleteFailedCount++ + _, updateErr := db.GetEngine(ctx).Table("storage_path_deletion").ID(deletion.ID).Update(map[string]any{ + "delete_failed_count": deletion.DeleteFailedCount, + "last_delete_failed_reason": err.Error(), + "last_delete_failed_time": timeutil.TimeStampNow(), + }) + return updateErr +} diff --git a/models/user/main_test.go b/models/user/main_test.go index a626d323a71..db60a281465 100644 --- a/models/user/main_test.go +++ b/models/user/main_test.go @@ -7,6 +7,8 @@ import ( "testing" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/services/storagecleanup" _ "code.gitea.io/gitea/models" _ "code.gitea.io/gitea/models/actions" @@ -15,5 +17,10 @@ import ( ) func TestMain(m *testing.M) { - unittest.MainTest(m) + unittest.MainTest(m, &unittest.TestOptions{ + SetUp: func() error { + setting.LoadQueueSettings() + return storagecleanup.Init() + }, + }) } diff --git a/modules/storage/storage.go b/modules/storage/storage.go index 1868817c057..c017838be1b 100644 --- a/modules/storage/storage.go +++ b/modules/storage/storage.go @@ -166,6 +166,40 @@ func NewStorage(typStr Type, cfg *setting.Storage) (ObjectStorage, error) { return fn(context.Background(), cfg) } +const ( + AttachmentStorageName = "attachment" + AvatarStorageName = "avatar" + RepoAvatarStorageName = "repo_avatar" + LFSStorageName = "lfs" + RepoArchiveStorageName = "repo_archive" + PackagesStorageName = "packages" + ActionsLogStorageName = "actions_logs" + ActionsArtifactsStorageName = "actions_artifacts" +) + +func GetStorageByName(name string) (ObjectStorage, error) { + switch name { + case AttachmentStorageName: + return Attachments, nil + case AvatarStorageName: + return Avatars, nil + case RepoAvatarStorageName: + return RepoAvatars, nil + case LFSStorageName: + return LFS, nil + case RepoArchiveStorageName: + return RepoArchives, nil + case PackagesStorageName: + return Packages, nil + case ActionsLogStorageName: + return Actions, nil + case ActionsArtifactsStorageName: + return ActionsArtifacts, nil + default: + return nil, fmt.Errorf("Unknown storage name: %s", name) + } +} + func initAvatars() (err error) { log.Info("Initialising Avatar storage with type: %s", setting.Avatar.Storage.Type) Avatars, err = NewStorage(setting.Avatar.Storage.Type, setting.Avatar.Storage) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 65ba4e9cd3f..5006cab0f68 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -3065,6 +3065,7 @@ dashboard.sync_branch.started = Branches Sync started dashboard.sync_tag.started = Tags Sync started dashboard.rebuild_issue_indexer = Rebuild issue indexer dashboard.sync_repo_licenses = Sync repo licenses +dashboard.cleanup_storage = Clean up deleted storage files users.user_manage_panel = User Account Management users.new_account = Create User Account diff --git a/routers/api/v1/repo/issue_attachment.go b/routers/api/v1/repo/issue_attachment.go index 3f751a295c3..550abf4a7b3 100644 --- a/routers/api/v1/repo/issue_attachment.go +++ b/routers/api/v1/repo/issue_attachment.go @@ -318,7 +318,7 @@ func DeleteIssueAttachment(ctx *context.APIContext) { return } - if err := repo_model.DeleteAttachment(ctx, attachment, true); err != nil { + if err := attachment_service.DeleteAttachment(ctx, attachment); err != nil { ctx.APIErrorInternal(err) return } diff --git a/routers/api/v1/repo/issue_comment_attachment.go b/routers/api/v1/repo/issue_comment_attachment.go index 5f660c57504..704db1c7a3a 100644 --- a/routers/api/v1/repo/issue_comment_attachment.go +++ b/routers/api/v1/repo/issue_comment_attachment.go @@ -330,7 +330,7 @@ func DeleteIssueCommentAttachment(ctx *context.APIContext) { return } - if err := repo_model.DeleteAttachment(ctx, attach, true); err != nil { + if err := attachment_service.DeleteAttachment(ctx, attach); err != nil { ctx.APIErrorInternal(err) return } diff --git a/routers/api/v1/repo/release_attachment.go b/routers/api/v1/repo/release_attachment.go index defde81a1d2..fa25c0cdfdd 100644 --- a/routers/api/v1/repo/release_attachment.go +++ b/routers/api/v1/repo/release_attachment.go @@ -393,8 +393,7 @@ func DeleteReleaseAttachment(ctx *context.APIContext) { return } // FIXME Should prove the existence of the given repo, but results in unnecessary database requests - - if err := repo_model.DeleteAttachment(ctx, attach, true); err != nil { + if err := attachment_service.DeleteAttachment(ctx, attach); err != nil { ctx.APIErrorInternal(err) return } diff --git a/routers/init.go b/routers/init.go index 744feee2f0d..39c34bf3a37 100644 --- a/routers/init.go +++ b/routers/init.go @@ -52,6 +52,7 @@ import ( release_service "code.gitea.io/gitea/services/release" repo_service "code.gitea.io/gitea/services/repository" "code.gitea.io/gitea/services/repository/archiver" + "code.gitea.io/gitea/services/storagecleanup" "code.gitea.io/gitea/services/task" "code.gitea.io/gitea/services/uinotification" "code.gitea.io/gitea/services/webhook" @@ -174,6 +175,7 @@ func InitWebInstalled(ctx context.Context) { mustInitCtx(ctx, actions_service.Init) mustInit(repo_service.InitLicenseClassifier) + mustInit(storagecleanup.Init) // Finally start up the cron cron.NewContext(ctx) diff --git a/routers/web/repo/attachment.go b/routers/web/repo/attachment.go index f6966691961..ff8d098c1f2 100644 --- a/routers/web/repo/attachment.go +++ b/routers/web/repo/attachment.go @@ -15,7 +15,7 @@ import ( "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/routers/common" - "code.gitea.io/gitea/services/attachment" + attachment_service "code.gitea.io/gitea/services/attachment" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/context/upload" repo_service "code.gitea.io/gitea/services/repository" @@ -45,7 +45,7 @@ func uploadAttachment(ctx *context.Context, repoID int64, allowedTypes string) { } defer file.Close() - attach, err := attachment.UploadAttachment(ctx, file, allowedTypes, header.Size, &repo_model.Attachment{ + attach, err := attachment_service.UploadAttachment(ctx, file, allowedTypes, header.Size, &repo_model.Attachment{ Name: header.Filename, UploaderID: ctx.Doer.ID, RepoID: repoID, @@ -70,14 +70,18 @@ func DeleteAttachment(ctx *context.Context) { file := ctx.FormString("file") attach, err := repo_model.GetAttachmentByUUID(ctx, file) if err != nil { - ctx.HTTPError(http.StatusBadRequest, err.Error()) + if repo_model.IsErrAttachmentNotExist(err) { + ctx.HTTPError(http.StatusNotFound) + } else { + ctx.ServerError("GetAttachmentByUUID", err) + } return } if !ctx.IsSigned || (ctx.Doer.ID != attach.UploaderID) { ctx.HTTPError(http.StatusForbidden) return } - err = repo_model.DeleteAttachment(ctx, attach, true) + err = attachment_service.DeleteAttachment(ctx, attach) if err != nil { ctx.HTTPError(http.StatusInternalServerError, fmt.Sprintf("DeleteAttachment: %v", err)) return diff --git a/routers/web/repo/issue.go b/routers/web/repo/issue.go index 54b7e5df2a0..84035b59835 100644 --- a/routers/web/repo/issue.go +++ b/routers/web/repo/issue.go @@ -29,6 +29,7 @@ import ( "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/common" + attachment_service "code.gitea.io/gitea/services/attachment" "code.gitea.io/gitea/services/context" "code.gitea.io/gitea/services/convert" "code.gitea.io/gitea/services/forms" @@ -605,7 +606,7 @@ func updateAttachments(ctx *context.Context, item any, files []string) error { if util.SliceContainsString(files, attachments[i].UUID) { continue } - if err := repo_model.DeleteAttachment(ctx, attachments[i], true); err != nil { + if err := attachment_service.DeleteAttachment(ctx, attachments[i]); err != nil { return err } } diff --git a/services/attachment/attachment.go b/services/attachment/attachment.go index ccb97c66c82..65ff3629ec3 100644 --- a/services/attachment/attachment.go +++ b/services/attachment/attachment.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/context/upload" + "code.gitea.io/gitea/services/storagecleanup" "github.com/google/uuid" ) @@ -59,3 +60,21 @@ func UpdateAttachment(ctx context.Context, allowedTypes string, attach *repo_mod return repo_model.UpdateAttachment(ctx, attach) } + +// DeleteAttachment deletes the given attachment and optionally the associated file. +func DeleteAttachment(ctx context.Context, a *repo_model.Attachment) error { + _, err := DeleteAttachments(ctx, []*repo_model.Attachment{a}) + return err +} + +// DeleteAttachments deletes the given attachments and optionally the associated files. +func DeleteAttachments(ctx context.Context, attachments []*repo_model.Attachment) (int, error) { + deletions, err := repo_model.DeleteAttachments(ctx, attachments) + if err != nil { + return 0, err + } + + storagecleanup.AddDeletionsToCleanQueue(ctx, deletions) + + return len(deletions), nil +} diff --git a/services/attachment/attachment_test.go b/services/attachment/attachment_test.go index 65475836bec..da45a8079f3 100644 --- a/services/attachment/attachment_test.go +++ b/services/attachment/attachment_test.go @@ -6,12 +6,17 @@ package attachment import ( "os" "path/filepath" + "strings" "testing" + "time" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/storage" + "code.gitea.io/gitea/services/storagecleanup" _ "code.gitea.io/gitea/models/actions" @@ -19,7 +24,12 @@ import ( ) func TestMain(m *testing.M) { - unittest.MainTest(m) + unittest.MainTest(m, &unittest.TestOptions{ + SetUp: func() error { + setting.LoadQueueSettings() + return storagecleanup.Init() + }, + }) } func TestUploadAttachment(t *testing.T) { @@ -44,3 +54,30 @@ func TestUploadAttachment(t *testing.T) { assert.Equal(t, user.ID, attachment.UploaderID) assert.Equal(t, int64(0), attachment.DownloadCount) } + +func TestDeleteAttachments(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + attachment8 := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 8}) + const attachment8Content = "test content for attachment 8" // 29 bytes + _, err := storage.Attachments.Save(attachment8.RelativePath(), strings.NewReader(attachment8Content), int64(len(attachment8Content))) + assert.NoError(t, err) + + fileInfo, err := storage.Attachments.Stat(attachment8.RelativePath()) + assert.NoError(t, err) + assert.Equal(t, attachment8.Size, fileInfo.Size()) + + err = DeleteAttachment(db.DefaultContext, attachment8) + assert.NoError(t, err) + + attachment, err := repo_model.GetAttachmentByUUID(db.DefaultContext, attachment8.UUID) + assert.Error(t, err) + assert.True(t, repo_model.IsErrAttachmentNotExist(err)) + assert.Nil(t, attachment) + + // allow the queue to process the deletion + time.Sleep(1 * time.Second) + + _, err = storage.Attachments.Stat(attachment8.RelativePath()) + assert.Error(t, err) + assert.True(t, os.IsNotExist(err)) +} diff --git a/services/cron/tasks_extended.go b/services/cron/tasks_extended.go index 0018c5facc5..4252d98b402 100644 --- a/services/cron/tasks_extended.go +++ b/services/cron/tasks_extended.go @@ -17,6 +17,7 @@ import ( asymkey_service "code.gitea.io/gitea/services/asymkey" repo_service "code.gitea.io/gitea/services/repository" archiver_service "code.gitea.io/gitea/services/repository/archiver" + "code.gitea.io/gitea/services/storagecleanup" user_service "code.gitea.io/gitea/services/user" ) @@ -223,6 +224,16 @@ func registerRebuildIssueIndexer() { }) } +func registerCleanStorage() { + RegisterTaskFatal("cleanup_storage", &BaseConfig{ + Enabled: false, + RunAtStart: false, + Schedule: "@every 24h", + }, func(ctx context.Context, _ *user_model.User, _ Config) error { + return storagecleanup.ScanToBeDeletedFilesOrDir(ctx) + }) +} + func initExtendedTasks() { registerDeleteInactiveUsers() registerDeleteRepositoryArchives() @@ -238,4 +249,5 @@ func initExtendedTasks() { registerDeleteOldSystemNotices() registerGCLFS() registerRebuildIssueIndexer() + registerCleanStorage() } diff --git a/services/issue/comments.go b/services/issue/comments.go index 10c81198d57..b9213a10fa4 100644 --- a/services/issue/comments.go +++ b/services/issue/comments.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/modules/timeutil" git_service "code.gitea.io/gitea/services/git" notify_service "code.gitea.io/gitea/services/notify" + "code.gitea.io/gitea/services/storagecleanup" ) // CreateRefComment creates a commit reference comment to issue. @@ -130,15 +131,35 @@ func UpdateComment(ctx context.Context, c *issues_model.Comment, contentVersion return nil } -// DeleteComment deletes the comment -func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) error { - err := db.WithTx(ctx, func(ctx context.Context) error { - return issues_model.DeleteComment(ctx, comment) +// deleteComment deletes the comment +func deleteComment(ctx context.Context, comment *issues_model.Comment, removeAttachments bool) ([]int64, error) { + return db.WithTx2(ctx, func(ctx context.Context) ([]int64, error) { + if removeAttachments { + // load attachments before deleting the comment + if err := comment.LoadAttachments(ctx); err != nil { + return nil, err + } + } + + if err := issues_model.DeleteComment(ctx, comment); err != nil { + return nil, err + } + + if removeAttachments { + return repo_model.DeleteAttachments(ctx, comment.Attachments) + } + return nil, nil }) +} + +func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) error { + deletions, err := deleteComment(ctx, comment, true) if err != nil { return err } + storagecleanup.AddDeletionsToCleanQueue(ctx, deletions) + notify_service.DeleteComment(ctx, doer, comment) return nil diff --git a/services/issue/issue.go b/services/issue/issue.go index f03be3e18f6..c0f61859805 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -13,13 +13,12 @@ import ( access_model "code.gitea.io/gitea/models/perm/access" project_model "code.gitea.io/gitea/models/project" repo_model "code.gitea.io/gitea/models/repo" - system_model "code.gitea.io/gitea/models/system" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/container" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/storage" notify_service "code.gitea.io/gitea/services/notify" + "code.gitea.io/gitea/services/storagecleanup" ) // NewIssue creates new issue with labels for repository. @@ -190,18 +189,17 @@ func DeleteIssue(ctx context.Context, doer *user_model.User, gitRepo *git.Reposi } // delete entries in database - attachmentPaths, err := deleteIssue(ctx, issue) + toBeCleanedDeletions, err := deleteIssue(ctx, issue, true) if err != nil { return err } - for _, attachmentPath := range attachmentPaths { - system_model.RemoveStorageWithNotice(ctx, storage.Attachments, "Delete issue attachment", attachmentPath) - } + + storagecleanup.AddDeletionsToCleanQueue(ctx, toBeCleanedDeletions) // delete pull request related git data if issue.IsPull && gitRepo != nil { if err := gitRepo.RemoveReference(issue.PullRequest.GetGitHeadRefName()); err != nil { - return err + log.Error("DeleteIssue: RemoveReference %s: %v", issue.PullRequest.GetGitHeadRefName(), err) } } @@ -260,107 +258,113 @@ func GetRefEndNamesAndURLs(issues []*issues_model.Issue, repoLink string) (map[i } // deleteIssue deletes the issue -func deleteIssue(ctx context.Context, issue *issues_model.Issue) ([]string, error) { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return nil, err - } - defer committer.Close() - - if _, err := db.GetEngine(ctx).ID(issue.ID).NoAutoCondition().Delete(issue); err != nil { - return nil, err - } - - // update the total issue numbers - if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil { - return nil, err - } - // if the issue is closed, update the closed issue numbers - if issue.IsClosed { - if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil { +func deleteIssue(ctx context.Context, issue *issues_model.Issue, deleteAttachments bool) ([]int64, error) { + return db.WithTx2(ctx, func(ctx context.Context) ([]int64, error) { + toBeCleanedDeletions := make([]int64, 0) + if _, err := db.GetEngine(ctx).ID(issue.ID).NoAutoCondition().Delete(issue); err != nil { return nil, err } - } - if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil { - return nil, fmt.Errorf("error updating counters for milestone id %d: %w", - issue.MilestoneID, err) - } + // update the total issue numbers + if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil { + return nil, err + } + // if the issue is closed, update the closed issue numbers + if issue.IsClosed { + if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil { + return nil, err + } + } - if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID, issue.Index); err != nil { - return nil, err - } + if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil { + return nil, fmt.Errorf("error updating counters for milestone id %d: %w", + issue.MilestoneID, err) + } - // find attachments related to this issue and remove them - if err := issue.LoadAttachments(ctx); err != nil { - return nil, err - } + if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID, issue.Index); err != nil { + return nil, err + } - var attachmentPaths []string - for i := range issue.Attachments { - attachmentPaths = append(attachmentPaths, issue.Attachments[i].RelativePath()) - } + if deleteAttachments { + // find attachments related to this issue and remove them + if err := issue.LoadAttachments(ctx); err != nil { + return nil, err + } + } - // delete all database data still assigned to this issue - if err := db.DeleteBeans(ctx, - &issues_model.ContentHistory{IssueID: issue.ID}, - &issues_model.Comment{IssueID: issue.ID}, - &issues_model.IssueLabel{IssueID: issue.ID}, - &issues_model.IssueDependency{IssueID: issue.ID}, - &issues_model.IssueAssignees{IssueID: issue.ID}, - &issues_model.IssueUser{IssueID: issue.ID}, - &activities_model.Notification{IssueID: issue.ID}, - &issues_model.Reaction{IssueID: issue.ID}, - &issues_model.IssueWatch{IssueID: issue.ID}, - &issues_model.Stopwatch{IssueID: issue.ID}, - &issues_model.TrackedTime{IssueID: issue.ID}, - &project_model.ProjectIssue{IssueID: issue.ID}, - &repo_model.Attachment{IssueID: issue.ID}, - &issues_model.PullRequest{IssueID: issue.ID}, - &issues_model.Comment{RefIssueID: issue.ID}, - &issues_model.IssueDependency{DependencyID: issue.ID}, - &issues_model.Comment{DependentIssueID: issue.ID}, - &issues_model.IssuePin{IssueID: issue.ID}, - ); err != nil { - return nil, err - } + // delete all database data still assigned to this issue + if err := db.DeleteBeans(ctx, + &issues_model.ContentHistory{IssueID: issue.ID}, + &issues_model.IssueLabel{IssueID: issue.ID}, + &issues_model.IssueDependency{IssueID: issue.ID}, + &issues_model.IssueAssignees{IssueID: issue.ID}, + &issues_model.IssueUser{IssueID: issue.ID}, + &activities_model.Notification{IssueID: issue.ID}, + &issues_model.Reaction{IssueID: issue.ID}, + &issues_model.IssueWatch{IssueID: issue.ID}, + &issues_model.Stopwatch{IssueID: issue.ID}, + &issues_model.TrackedTime{IssueID: issue.ID}, + &project_model.ProjectIssue{IssueID: issue.ID}, + &issues_model.PullRequest{IssueID: issue.ID}, + &issues_model.Comment{RefIssueID: issue.ID}, + &issues_model.IssueDependency{DependencyID: issue.ID}, + &issues_model.Comment{DependentIssueID: issue.ID}, + &issues_model.IssuePin{IssueID: issue.ID}, + ); err != nil { + return nil, err + } - if err := committer.Commit(); err != nil { - return nil, err - } - return attachmentPaths, nil + for _, comment := range issue.Comments { + deletions, err := deleteComment(ctx, comment, deleteAttachments) + if err != nil { + return nil, fmt.Errorf("deleteComment [comment_id: %d]: %w", comment.ID, err) + } + if deleteAttachments { + toBeCleanedDeletions = append(toBeCleanedDeletions, deletions...) + } + } + + if deleteAttachments { + // delete issue attachments + if err := issue.LoadAttachments(ctx); err != nil { + return nil, err + } + deletions, err := repo_model.DeleteAttachments(ctx, issue.Attachments) + if err != nil { + return nil, err + } + toBeCleanedDeletions = append(toBeCleanedDeletions, deletions...) + } + return toBeCleanedDeletions, nil + }) } // DeleteOrphanedIssues delete issues without a repo func DeleteOrphanedIssues(ctx context.Context) error { - var attachmentPaths []string - err := db.WithTx(ctx, func(ctx context.Context) error { + toBeCleanedDeletions := make([]int64, 0) + if err := db.WithTx(ctx, func(ctx context.Context) error { repoIDs, err := issues_model.GetOrphanedIssueRepoIDs(ctx) if err != nil { return err } for i := range repoIDs { - paths, err := DeleteIssuesByRepoID(ctx, repoIDs[i]) + deletions, err := DeleteIssuesByRepoID(ctx, repoIDs[i], true) if err != nil { return err } - attachmentPaths = append(attachmentPaths, paths...) + toBeCleanedDeletions = append(toBeCleanedDeletions, deletions...) } return nil - }) - if err != nil { + }); err != nil { return err } - - // Remove issue attachment files. - for i := range attachmentPaths { - system_model.RemoveStorageWithNotice(ctx, storage.Attachments, "Delete issue attachment", attachmentPaths[i]) - } + storagecleanup.AddDeletionsToCleanQueue(ctx, toBeCleanedDeletions) return nil } // DeleteIssuesByRepoID deletes issues by repositories id -func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths []string, err error) { +func DeleteIssuesByRepoID(ctx context.Context, repoID int64, deleteAttachments bool) ([]int64, error) { + toBeCleanedDeletions := make([]int64, 0) for { issues := make([]*issues_model.Issue, 0, db.DefaultMaxInSize) if err := db.GetEngine(ctx). @@ -376,14 +380,13 @@ func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths [] } for _, issue := range issues { - issueAttachPaths, err := deleteIssue(ctx, issue) + deletions, err := deleteIssue(ctx, issue, deleteAttachments) if err != nil { return nil, fmt.Errorf("deleteIssue [issue_id: %d]: %w", issue.ID, err) } - - attachmentPaths = append(attachmentPaths, issueAttachPaths...) + toBeCleanedDeletions = append(toBeCleanedDeletions, deletions...) } } - return attachmentPaths, err + return toBeCleanedDeletions, nil } diff --git a/services/issue/issue_test.go b/services/issue/issue_test.go index bad0d65d1ed..780f25ad009 100644 --- a/services/issue/issue_test.go +++ b/services/issue/issue_test.go @@ -11,6 +11,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/services/storagecleanup" "github.com/stretchr/testify/assert" ) @@ -44,8 +45,9 @@ func TestIssue_DeleteIssue(t *testing.T) { ID: issueIDs[2], } - _, err = deleteIssue(db.DefaultContext, issue) + toBeCleanedDeletions, err := deleteIssue(db.DefaultContext, issue, true) assert.NoError(t, err) + storagecleanup.AddDeletionsToCleanQueue(db.DefaultContext, toBeCleanedDeletions) issueIDs, err = issues_model.GetIssueIDsByRepoID(db.DefaultContext, 1) assert.NoError(t, err) assert.Len(t, issueIDs, 4) @@ -55,8 +57,9 @@ func TestIssue_DeleteIssue(t *testing.T) { assert.NoError(t, err) issue, err = issues_model.GetIssueByID(db.DefaultContext, 4) assert.NoError(t, err) - _, err = deleteIssue(db.DefaultContext, issue) + toBeCleanedDeletions, err = deleteIssue(db.DefaultContext, issue, true) assert.NoError(t, err) + storagecleanup.AddDeletionsToCleanQueue(db.DefaultContext, toBeCleanedDeletions) assert.Len(t, attachments, 2) for i := range attachments { attachment, err := repo_model.GetAttachmentByUUID(db.DefaultContext, attachments[i].UUID) @@ -78,8 +81,9 @@ func TestIssue_DeleteIssue(t *testing.T) { assert.NoError(t, err) assert.False(t, left) - _, err = deleteIssue(db.DefaultContext, issue2) + toBeCleanedDeletions, err = deleteIssue(db.DefaultContext, issue2, true) assert.NoError(t, err) + storagecleanup.AddDeletionsToCleanQueue(db.DefaultContext, toBeCleanedDeletions) left, err = issues_model.IssueNoDependenciesLeft(db.DefaultContext, issue1) assert.NoError(t, err) assert.True(t, left) diff --git a/services/issue/main_test.go b/services/issue/main_test.go index 819c5d98c3f..2a57a09a879 100644 --- a/services/issue/main_test.go +++ b/services/issue/main_test.go @@ -7,11 +7,18 @@ import ( "testing" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/services/storagecleanup" _ "code.gitea.io/gitea/models" _ "code.gitea.io/gitea/models/actions" ) func TestMain(m *testing.M) { - unittest.MainTest(m) + unittest.MainTest(m, &unittest.TestOptions{ + SetUp: func() error { + setting.LoadQueueSettings() + return storagecleanup.Init() + }, + }) } diff --git a/services/org/org.go b/services/org/org.go index 3d30ae21a39..43aa32485c8 100644 --- a/services/org/org.go +++ b/services/org/org.go @@ -48,12 +48,14 @@ func deleteOrganization(ctx context.Context, org *org_model.Organization) error // DeleteOrganization completely and permanently deletes everything of organization. func DeleteOrganization(ctx context.Context, org *org_model.Organization, purge bool) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - + // Deleting repositories under the organization cannot be wrapped in a transaction at the moment, + // because the associated disk content is permanently deleted by the DeleteOwnerRepositoriesDirectly function, + // which cannot be rolled back. + // + // Even if some repositories fail to delete, the organization will still be deleted. + // + // TODO: Consider marking repositories as "deleted" first, + // and handling the actual deletion in a background job for better reliability and rollback support. if purge { err := repo_service.DeleteOwnerRepositoriesDirectly(ctx, org.AsUser()) if err != nil { @@ -61,26 +63,28 @@ func DeleteOrganization(ctx context.Context, org *org_model.Organization, purge } } - // Check ownership of repository. - count, err := repo_model.CountRepositories(ctx, repo_model.CountRepositoryOptions{OwnerID: org.ID}) + err := db.WithTx(ctx, func(ctx context.Context) error { + // Check ownership of repository. + count, err := repo_model.CountRepositories(ctx, repo_model.CountRepositoryOptions{OwnerID: org.ID}) + if err != nil { + return fmt.Errorf("GetRepositoryCount: %w", err) + } else if count > 0 { + return repo_model.ErrUserOwnRepos{UID: org.ID} + } + + // Check ownership of packages. + if ownsPackages, err := packages_model.HasOwnerPackages(ctx, org.ID); err != nil { + return fmt.Errorf("HasOwnerPackages: %w", err) + } else if ownsPackages { + return packages_model.ErrUserOwnPackages{UID: org.ID} + } + + if err := deleteOrganization(ctx, org); err != nil { + return fmt.Errorf("DeleteOrganization: %w", err) + } + return nil + }) if err != nil { - return fmt.Errorf("GetRepositoryCount: %w", err) - } else if count > 0 { - return repo_model.ErrUserOwnRepos{UID: org.ID} - } - - // Check ownership of packages. - if ownsPackages, err := packages_model.HasOwnerPackages(ctx, org.ID); err != nil { - return fmt.Errorf("HasOwnerPackages: %w", err) - } else if ownsPackages { - return packages_model.ErrUserOwnPackages{UID: org.ID} - } - - if err := deleteOrganization(ctx, org); err != nil { - return fmt.Errorf("DeleteOrganization: %w", err) - } - - if err := committer.Commit(); err != nil { return err } diff --git a/services/release/release.go b/services/release/release.go index 0b8a74252a0..f6502a30767 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -19,10 +19,10 @@ import ( "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/repository" - "code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/util" notify_service "code.gitea.io/gitea/services/notify" + "code.gitea.io/gitea/services/storagecleanup" ) // ErrInvalidTagName represents a "InvalidTagName" kind of error. @@ -288,6 +288,8 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo } deletedUUIDs := make(container.Set[string]) + deletedAttachments := make([]*repo_model.Attachment, 0, len(delAttachmentUUIDs)) + toBeCleanedDeletions := make([]int64, 0, len(delAttachmentUUIDs)) if len(delAttachmentUUIDs) > 0 { // Check attachments attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, delAttachmentUUIDs) @@ -299,11 +301,15 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo return util.NewPermissionDeniedErrorf("delete attachment of release permission denied") } deletedUUIDs.Add(attach.UUID) + deletedAttachments = append(deletedAttachments, attach) } - if _, err := repo_model.DeleteAttachments(ctx, attachments, true); err != nil { - return fmt.Errorf("DeleteAttachments [uuids: %v]: %w", delAttachmentUUIDs, err) + deletions, err := repo_model.DeleteAttachments(ctx, deletedAttachments) + if err != nil { + return fmt.Errorf("DeleteAttachments [uuids: %v]: %w", deletedUUIDs.Values(), err) } + toBeCleanedDeletions = append(toBeCleanedDeletions, deletions...) + // files will be deleted after database transaction is committed successfully } if len(editAttachments) > 0 { @@ -338,15 +344,7 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo return err } - for _, uuid := range delAttachmentUUIDs { - if err := storage.Attachments.Delete(repo_model.AttachmentRelativePath(uuid)); err != nil { - // Even delete files failed, but the attachments has been removed from database, so we - // should not return error but only record the error on logs. - // users have to delete this attachments manually or we should have a - // synchronize between database attachment table and attachment storage - log.Error("delete attachment[uuid: %s] failed: %v", uuid, err) - } - } + storagecleanup.AddDeletionsToCleanQueue(ctx, toBeCleanedDeletions) if !rel.IsDraft { if !isTagCreated && !isConvertedFromTag { @@ -360,64 +358,67 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo // DeleteReleaseByID deletes a release and corresponding Git tag by given ID. func DeleteReleaseByID(ctx context.Context, repo *repo_model.Repository, rel *repo_model.Release, doer *user_model.User, delTag bool) error { - if delTag { - protectedTags, err := git_model.GetProtectedTags(ctx, rel.RepoID) - if err != nil { - return fmt.Errorf("GetProtectedTags: %w", err) - } - isAllowed, err := git_model.IsUserAllowedToControlTag(ctx, protectedTags, rel.TagName, rel.PublisherID) - if err != nil { - return err - } - if !isAllowed { - return ErrProtectedTagName{ - TagName: rel.TagName, + var toBeCleanedDeletions []int64 + if err := db.WithTx(ctx, func(ctx context.Context) error { + if delTag { + protectedTags, err := git_model.GetProtectedTags(ctx, rel.RepoID) + if err != nil { + return fmt.Errorf("GetProtectedTags: %w", err) + } + isAllowed, err := git_model.IsUserAllowedToControlTag(ctx, protectedTags, rel.TagName, rel.PublisherID) + if err != nil { + return err + } + if !isAllowed { + return ErrProtectedTagName{ + TagName: rel.TagName, + } + } + + if stdout, _, err := git.NewCommand("tag", "-d").AddDashesAndList(rel.TagName). + RunStdString(ctx, &git.RunOpts{Dir: repo.RepoPath()}); err != nil && !strings.Contains(err.Error(), "not found") { + log.Error("DeleteReleaseByID (git tag -d): %d in %v Failed:\nStdout: %s\nError: %v", rel.ID, repo, stdout, err) + return fmt.Errorf("git tag -d: %w", err) + } + + refName := git.RefNameFromTag(rel.TagName) + objectFormat := git.ObjectFormatFromName(repo.ObjectFormatName) + notify_service.PushCommits( + ctx, doer, repo, + &repository.PushUpdateOptions{ + RefFullName: refName, + OldCommitID: rel.Sha1, + NewCommitID: objectFormat.EmptyObjectID().String(), + }, repository.NewPushCommits()) + notify_service.DeleteRef(ctx, doer, repo, refName) + + if _, err := db.DeleteByID[repo_model.Release](ctx, rel.ID); err != nil { + return fmt.Errorf("DeleteReleaseByID: %w", err) + } + } else { + rel.IsTag = true + + if err := repo_model.UpdateRelease(ctx, rel); err != nil { + return fmt.Errorf("Update: %w", err) } } - if stdout, _, err := git.NewCommand("tag", "-d").AddDashesAndList(rel.TagName). - RunStdString(ctx, &git.RunOpts{Dir: repo.RepoPath()}); err != nil && !strings.Contains(err.Error(), "not found") { - log.Error("DeleteReleaseByID (git tag -d): %d in %v Failed:\nStdout: %s\nError: %v", rel.ID, repo, stdout, err) - return fmt.Errorf("git tag -d: %w", err) + rel.Repo = repo + if err := rel.LoadAttributes(ctx); err != nil { + return fmt.Errorf("LoadAttributes: %w", err) } - refName := git.RefNameFromTag(rel.TagName) - objectFormat := git.ObjectFormatFromName(repo.ObjectFormatName) - notify_service.PushCommits( - ctx, doer, repo, - &repository.PushUpdateOptions{ - RefFullName: refName, - OldCommitID: rel.Sha1, - NewCommitID: objectFormat.EmptyObjectID().String(), - }, repository.NewPushCommits()) - notify_service.DeleteRef(ctx, doer, repo, refName) - - if _, err := db.DeleteByID[repo_model.Release](ctx, rel.ID); err != nil { - return fmt.Errorf("DeleteReleaseByID: %w", err) - } - } else { - rel.IsTag = true - - if err := repo_model.UpdateRelease(ctx, rel); err != nil { - return fmt.Errorf("Update: %w", err) + deletions, err := repo_model.DeleteAttachments(ctx, rel.Attachments) + if err != nil { + return fmt.Errorf("DeleteAttachments: %w", err) } + toBeCleanedDeletions = append(toBeCleanedDeletions, deletions...) + return nil + }); err != nil { + return err } - rel.Repo = repo - if err := rel.LoadAttributes(ctx); err != nil { - return fmt.Errorf("LoadAttributes: %w", err) - } - - if err := repo_model.DeleteAttachmentsByRelease(ctx, rel.ID); err != nil { - return fmt.Errorf("DeleteAttachments: %w", err) - } - - for i := range rel.Attachments { - attachment := rel.Attachments[i] - if err := storage.Attachments.Delete(attachment.RelativePath()); err != nil { - log.Error("Delete attachment %s of release %s failed: %v", attachment.UUID, rel.ID, err) - } - } + storagecleanup.AddDeletionsToCleanQueue(ctx, toBeCleanedDeletions) if !rel.IsDraft { notify_service.DeleteRelease(ctx, doer, rel) diff --git a/services/release/release_test.go b/services/release/release_test.go index 36a9f667d69..69b8384d1e3 100644 --- a/services/release/release_test.go +++ b/services/release/release_test.go @@ -14,7 +14,9 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/gitrepo" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/services/attachment" + "code.gitea.io/gitea/services/storagecleanup" _ "code.gitea.io/gitea/models/actions" @@ -22,7 +24,12 @@ import ( ) func TestMain(m *testing.M) { - unittest.MainTest(m) + unittest.MainTest(m, &unittest.TestOptions{ + SetUp: func() error { + setting.LoadQueueSettings() + return storagecleanup.Init() + }, + }) } func TestRelease_Create(t *testing.T) { diff --git a/services/repository/delete.go b/services/repository/delete.go index c48d6e1d56e..ea281c44208 100644 --- a/services/repository/delete.go +++ b/services/repository/delete.go @@ -30,6 +30,7 @@ import ( actions_service "code.gitea.io/gitea/services/actions" asymkey_service "code.gitea.io/gitea/services/asymkey" issue_service "code.gitea.io/gitea/services/issue" + "code.gitea.io/gitea/services/storagecleanup" "xorm.io/builder" ) @@ -50,15 +51,8 @@ func deleteDBRepository(ctx context.Context, repoID int64) error { // DeleteRepository deletes a repository for a user or organization. // make sure if you call this func to close open sessions (sqlite will otherwise get a deadlock) func DeleteRepositoryDirectly(ctx context.Context, repoID int64, ignoreOrgTeams ...bool) error { - ctx, committer, err := db.TxContext(ctx) - if err != nil { - return err - } - defer committer.Close() - sess := db.GetEngine(ctx) - repo := &repo_model.Repository{} - has, err := sess.ID(repoID).Get(repo) + has, err := db.GetEngine(ctx).ID(repoID).Get(repo) if err != nil { return err } else if !has { @@ -81,222 +75,221 @@ func DeleteRepositoryDirectly(ctx context.Context, repoID int64, ignoreOrgTeams return fmt.Errorf("list actions artifacts of repo %v: %w", repoID, err) } - // In case owner is a organization, we have to change repo specific teams - // if ignoreOrgTeams is not true - var org *user_model.User - if len(ignoreOrgTeams) == 0 || !ignoreOrgTeams[0] { - if org, err = user_model.GetUserByID(ctx, repo.OwnerID); err != nil { - return err - } - } + var needRewriteKeysFile bool + var archivePaths []string + var lfsPaths []string + toBeCleanedDeletions := make([]int64, 0, 20) - // Delete Deploy Keys - deleted, err := asymkey_service.DeleteRepoDeployKeys(ctx, repoID) - if err != nil { - return err - } - needRewriteKeysFile := deleted > 0 - - if err := deleteDBRepository(ctx, repoID); err != nil { - return err - } - - if org != nil && org.IsOrganization() { - teams, err := organization.FindOrgTeams(ctx, org.ID) - if err != nil { - return err - } - for _, t := range teams { - if !organization.HasTeamRepo(ctx, t.OrgID, t.ID, repoID) { - continue - } else if err = removeRepositoryFromTeam(ctx, t, repo, false); err != nil { + err = db.WithTx(ctx, func(ctx context.Context) error { + // In case owner is a organization, we have to change repo specific teams + // if ignoreOrgTeams is not true + var org *user_model.User + if len(ignoreOrgTeams) == 0 || !ignoreOrgTeams[0] { + if org, err = user_model.GetUserByID(ctx, repo.OwnerID); err != nil { return err } } - } - attachments := make([]*repo_model.Attachment, 0, 20) - if err = sess.Join("INNER", "`release`", "`release`.id = `attachment`.release_id"). - Where("`release`.repo_id = ?", repoID). - Find(&attachments); err != nil { - return err - } - releaseAttachments := make([]string, 0, len(attachments)) - for i := 0; i < len(attachments); i++ { - releaseAttachments = append(releaseAttachments, attachments[i].RelativePath()) - } - - if _, err := db.Exec(ctx, "UPDATE `user` SET num_stars=num_stars-1 WHERE id IN (SELECT `uid` FROM `star` WHERE repo_id = ?)", repo.ID); err != nil { - return err - } - - if _, err := db.GetEngine(ctx).In("hook_id", builder.Select("id").From("webhook").Where(builder.Eq{"webhook.repo_id": repo.ID})). - Delete(&webhook.HookTask{}); err != nil { - return err - } - - // CleanupEphemeralRunnersByPickedTaskOfRepo deletes ephemeral global/org/user that have started any task of this repo - // The cannot pick a second task hardening for ephemeral runners expect that task objects remain available until runner deletion - // This method will delete affected ephemeral global/org/user runners - // &actions_model.ActionRunner{RepoID: repoID} does only handle ephemeral repository runners - if err := actions_service.CleanupEphemeralRunnersByPickedTaskOfRepo(ctx, repoID); err != nil { - return fmt.Errorf("cleanupEphemeralRunners: %w", err) - } - - if err := db.DeleteBeans(ctx, - &access_model.Access{RepoID: repo.ID}, - &activities_model.Action{RepoID: repo.ID}, - &repo_model.Collaboration{RepoID: repoID}, - &issues_model.Comment{RefRepoID: repoID}, - &git_model.CommitStatus{RepoID: repoID}, - &git_model.Branch{RepoID: repoID}, - &git_model.LFSLock{RepoID: repoID}, - &repo_model.LanguageStat{RepoID: repoID}, - &repo_model.RepoLicense{RepoID: repoID}, - &issues_model.Milestone{RepoID: repoID}, - &repo_model.Mirror{RepoID: repoID}, - &activities_model.Notification{RepoID: repoID}, - &git_model.ProtectedBranch{RepoID: repoID}, - &git_model.ProtectedTag{RepoID: repoID}, - &repo_model.PushMirror{RepoID: repoID}, - &repo_model.Release{RepoID: repoID}, - &repo_model.RepoIndexerStatus{RepoID: repoID}, - &repo_model.Redirect{RedirectRepoID: repoID}, - &repo_model.RepoUnit{RepoID: repoID}, - &repo_model.Star{RepoID: repoID}, - &admin_model.Task{RepoID: repoID}, - &repo_model.Watch{RepoID: repoID}, - &webhook.Webhook{RepoID: repoID}, - &secret_model.Secret{RepoID: repoID}, - &actions_model.ActionTaskStep{RepoID: repoID}, - &actions_model.ActionTask{RepoID: repoID}, - &actions_model.ActionRunJob{RepoID: repoID}, - &actions_model.ActionRun{RepoID: repoID}, - &actions_model.ActionRunner{RepoID: repoID}, - &actions_model.ActionScheduleSpec{RepoID: repoID}, - &actions_model.ActionSchedule{RepoID: repoID}, - &actions_model.ActionArtifact{RepoID: repoID}, - &actions_model.ActionRunnerToken{RepoID: repoID}, - &issues_model.IssuePin{RepoID: repoID}, - ); err != nil { - return fmt.Errorf("deleteBeans: %w", err) - } - - // Delete Labels and related objects - if err := issues_model.DeleteLabelsByRepoID(ctx, repoID); err != nil { - return err - } - - // Delete Pulls and related objects - if err := issues_model.DeletePullsByBaseRepoID(ctx, repoID); err != nil { - return err - } - - // Delete Issues and related objects - var attachmentPaths []string - if attachmentPaths, err = issue_service.DeleteIssuesByRepoID(ctx, repoID); err != nil { - return err - } - - // Delete issue index - if err := db.DeleteResourceIndex(ctx, "issue_index", repoID); err != nil { - return err - } - - if repo.IsFork { - if _, err := db.Exec(ctx, "UPDATE `repository` SET num_forks=num_forks-1 WHERE id=?", repo.ForkID); err != nil { - return fmt.Errorf("decrease fork count: %w", err) - } - } - - if _, err := db.Exec(ctx, "UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", repo.OwnerID); err != nil { - return err - } - - if len(repo.Topics) > 0 { - if err := repo_model.RemoveTopicsFromRepo(ctx, repo.ID); err != nil { - return err - } - } - - if err := project_model.DeleteProjectByRepoID(ctx, repoID); err != nil { - return fmt.Errorf("unable to delete projects for repo[%d]: %w", repoID, err) - } - - // Remove LFS objects - var lfsObjects []*git_model.LFSMetaObject - if err = sess.Where("repository_id=?", repoID).Find(&lfsObjects); err != nil { - return err - } - - lfsPaths := make([]string, 0, len(lfsObjects)) - for _, v := range lfsObjects { - count, err := db.CountByBean(ctx, &git_model.LFSMetaObject{Pointer: lfs.Pointer{Oid: v.Oid}}) + // Delete Deploy Keys + deleted, err := asymkey_service.DeleteRepoDeployKeys(ctx, repoID) if err != nil { return err } - if count > 1 { - continue + needRewriteKeysFile = deleted > 0 + + if err := deleteDBRepository(ctx, repoID); err != nil { + return err } - lfsPaths = append(lfsPaths, v.RelativePath()) - } - - if _, err := db.DeleteByBean(ctx, &git_model.LFSMetaObject{RepositoryID: repoID}); err != nil { - return err - } - - // Remove archives - var archives []*repo_model.RepoArchiver - if err = sess.Where("repo_id=?", repoID).Find(&archives); err != nil { - return err - } - - archivePaths := make([]string, 0, len(archives)) - for _, v := range archives { - archivePaths = append(archivePaths, v.RelativePath()) - } - - if _, err := db.DeleteByBean(ctx, &repo_model.RepoArchiver{RepoID: repoID}); err != nil { - return err - } - - if repo.NumForks > 0 { - if _, err = sess.Exec("UPDATE `repository` SET fork_id=0,is_fork=? WHERE fork_id=?", false, repo.ID); err != nil { - log.Error("reset 'fork_id' and 'is_fork': %v", err) + if org != nil && org.IsOrganization() { + teams, err := organization.FindOrgTeams(ctx, org.ID) + if err != nil { + return err + } + for _, t := range teams { + if !organization.HasTeamRepo(ctx, t.OrgID, t.ID, repoID) { + continue + } else if err = removeRepositoryFromTeam(ctx, t, repo, false); err != nil { + return err + } + } } - } - // Get all attachments with both issue_id and release_id are zero - var newAttachments []*repo_model.Attachment - if err := sess.Where(builder.Eq{ - "repo_id": repo.ID, - "issue_id": 0, - "release_id": 0, - }).Find(&newAttachments); err != nil { + releaseAttachments := make([]*repo_model.Attachment, 0, 20) + // some attachments have release_id but repo_id = 0 + if err = db.GetEngine(ctx).Join("INNER", "`release`", "`release`.id = `attachment`.release_id"). + Where("`release`.repo_id = ?", repoID). + Find(&releaseAttachments); err != nil { + return err + } + + deletions, err := repo_model.DeleteAttachments(ctx, releaseAttachments) + if err != nil { + return fmt.Errorf("delete release attachments: %w", err) + } + toBeCleanedDeletions = append(toBeCleanedDeletions, deletions...) + + if _, err := db.Exec(ctx, "UPDATE `user` SET num_stars=num_stars-1 WHERE id IN (SELECT `uid` FROM `star` WHERE repo_id = ?)", repo.ID); err != nil { + return err + } + + if _, err := db.GetEngine(ctx).In("hook_id", builder.Select("id").From("webhook").Where(builder.Eq{"webhook.repo_id": repo.ID})). + Delete(&webhook.HookTask{}); err != nil { + return err + } + + // CleanupEphemeralRunnersByPickedTaskOfRepo deletes ephemeral global/org/user that have started any task of this repo + // The cannot pick a second task hardening for ephemeral runners expect that task objects remain available until runner deletion + // This method will delete affected ephemeral global/org/user runners + // &actions_model.ActionRunner{RepoID: repoID} does only handle ephemeral repository runners + if err := actions_service.CleanupEphemeralRunnersByPickedTaskOfRepo(ctx, repoID); err != nil { + return fmt.Errorf("cleanupEphemeralRunners: %w", err) + } + + if err := db.DeleteBeans(ctx, + &access_model.Access{RepoID: repo.ID}, + &activities_model.Action{RepoID: repo.ID}, + &repo_model.Collaboration{RepoID: repoID}, + &issues_model.Comment{RefRepoID: repoID}, + &git_model.CommitStatus{RepoID: repoID}, + &git_model.Branch{RepoID: repoID}, + &git_model.LFSLock{RepoID: repoID}, + &repo_model.LanguageStat{RepoID: repoID}, + &repo_model.RepoLicense{RepoID: repoID}, + &issues_model.Milestone{RepoID: repoID}, + &repo_model.Mirror{RepoID: repoID}, + &activities_model.Notification{RepoID: repoID}, + &git_model.ProtectedBranch{RepoID: repoID}, + &git_model.ProtectedTag{RepoID: repoID}, + &repo_model.PushMirror{RepoID: repoID}, + &repo_model.Release{RepoID: repoID}, + &repo_model.RepoIndexerStatus{RepoID: repoID}, + &repo_model.Redirect{RedirectRepoID: repoID}, + &repo_model.RepoUnit{RepoID: repoID}, + &repo_model.Star{RepoID: repoID}, + &admin_model.Task{RepoID: repoID}, + &repo_model.Watch{RepoID: repoID}, + &webhook.Webhook{RepoID: repoID}, + &secret_model.Secret{RepoID: repoID}, + &actions_model.ActionTaskStep{RepoID: repoID}, + &actions_model.ActionTask{RepoID: repoID}, + &actions_model.ActionRunJob{RepoID: repoID}, + &actions_model.ActionRun{RepoID: repoID}, + &actions_model.ActionRunner{RepoID: repoID}, + &actions_model.ActionScheduleSpec{RepoID: repoID}, + &actions_model.ActionSchedule{RepoID: repoID}, + &actions_model.ActionArtifact{RepoID: repoID}, + &actions_model.ActionRunnerToken{RepoID: repoID}, + &issues_model.IssuePin{RepoID: repoID}, + ); err != nil { + return fmt.Errorf("deleteBeans: %w", err) + } + + // Delete Labels and related objects + if err := issues_model.DeleteLabelsByRepoID(ctx, repoID); err != nil { + return err + } + + // Delete Pulls and related objects + if err := issues_model.DeletePullsByBaseRepoID(ctx, repoID); err != nil { + return err + } + + // Delete Issues and related objects + // attachments will be deleted later with repo_id, so we don't need to delete them here + if _, err := issue_service.DeleteIssuesByRepoID(ctx, repoID, false); err != nil { + return err + } + + // Delete issue index + if err := db.DeleteResourceIndex(ctx, "issue_index", repoID); err != nil { + return err + } + + if repo.IsFork { + if _, err := db.Exec(ctx, "UPDATE `repository` SET num_forks=num_forks-1 WHERE id=?", repo.ForkID); err != nil { + return fmt.Errorf("decrease fork count: %w", err) + } + } + + if _, err := db.Exec(ctx, "UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", repo.OwnerID); err != nil { + return err + } + + if len(repo.Topics) > 0 { + if err := repo_model.RemoveTopicsFromRepo(ctx, repo.ID); err != nil { + return err + } + } + + if err := project_model.DeleteProjectByRepoID(ctx, repoID); err != nil { + return fmt.Errorf("unable to delete projects for repo[%d]: %w", repoID, err) + } + + // Remove LFS objects + var lfsObjects []*git_model.LFSMetaObject + if err = db.GetEngine(ctx).Where("repository_id=?", repoID).Find(&lfsObjects); err != nil { + return err + } + + lfsPaths = make([]string, 0, len(lfsObjects)) + for _, v := range lfsObjects { + count, err := db.CountByBean(ctx, &git_model.LFSMetaObject{Pointer: lfs.Pointer{Oid: v.Oid}}) + if err != nil { + return err + } + if count > 1 { + continue + } + + lfsPaths = append(lfsPaths, v.RelativePath()) + } + + if _, err := db.DeleteByBean(ctx, &git_model.LFSMetaObject{RepositoryID: repoID}); err != nil { + return err + } + + // Remove archives + var archives []*repo_model.RepoArchiver + if err = db.GetEngine(ctx).Where("repo_id=?", repoID).Find(&archives); err != nil { + return err + } + + archivePaths = make([]string, 0, len(archives)) + for _, v := range archives { + archivePaths = append(archivePaths, v.RelativePath()) + } + + if _, err := db.DeleteByBean(ctx, &repo_model.RepoArchiver{RepoID: repoID}); err != nil { + return err + } + + if repo.NumForks > 0 { + if _, err = db.GetEngine(ctx).Exec("UPDATE `repository` SET fork_id=0,is_fork=? WHERE fork_id=?", false, repo.ID); err != nil { + log.Error("reset 'fork_id' and 'is_fork': %v", err) + } + } + + var repoAttachments []*repo_model.Attachment + // Get all attachments with repo_id = repo.ID. some release attachments have repo_id = 0 should be deleted before + if err := db.GetEngine(ctx).Where(builder.Eq{ + "repo_id": repo.ID, + }).Find(&repoAttachments); err != nil { + return err + } + deletions, err = repo_model.DeleteAttachments(ctx, repoAttachments) + if err != nil { + return err + } + toBeCleanedDeletions = append(toBeCleanedDeletions, deletions...) + + // unlink packages linked to this repository + return packages_model.UnlinkRepositoryFromAllPackages(ctx, repoID) + }) + if err != nil { return err } - newAttachmentPaths := make([]string, 0, len(newAttachments)) - for _, attach := range newAttachments { - newAttachmentPaths = append(newAttachmentPaths, attach.RelativePath()) - } - - if _, err := sess.Where("repo_id=?", repo.ID).Delete(new(repo_model.Attachment)); err != nil { - return err - } - - // unlink packages linked to this repository - if err = packages_model.UnlinkRepositoryFromAllPackages(ctx, repoID); err != nil { - return err - } - - if err = committer.Commit(); err != nil { - return err - } - - committer.Close() - if needRewriteKeysFile { if err := asymkey_service.RewriteAllPublicKeys(ctx); err != nil { log.Error("RewriteAllPublicKeys failed: %v", err) @@ -330,20 +323,7 @@ func DeleteRepositoryDirectly(ctx context.Context, repoID int64, ignoreOrgTeams system_model.RemoveStorageWithNotice(ctx, storage.LFS, "Delete orphaned LFS file", lfsObj) } - // Remove issue attachment files. - for _, attachment := range attachmentPaths { - system_model.RemoveStorageWithNotice(ctx, storage.Attachments, "Delete issue attachment", attachment) - } - - // Remove release attachment files. - for _, releaseAttachment := range releaseAttachments { - system_model.RemoveStorageWithNotice(ctx, storage.Attachments, "Delete release attachment", releaseAttachment) - } - - // Remove attachment with no issue_id and release_id. - for _, newAttachment := range newAttachmentPaths { - system_model.RemoveStorageWithNotice(ctx, storage.Attachments, "Delete issue attachment", newAttachment) - } + storagecleanup.AddDeletionsToCleanQueue(ctx, toBeCleanedDeletions) if len(repo.Avatar) > 0 { if err := storage.RepoAvatars.Delete(repo.CustomAvatarRelativePath()); err != nil { diff --git a/services/repository/main_test.go b/services/repository/main_test.go index 7ad1540aee4..a3a9e8774ec 100644 --- a/services/repository/main_test.go +++ b/services/repository/main_test.go @@ -7,8 +7,15 @@ import ( "testing" "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/services/storagecleanup" ) func TestMain(m *testing.M) { - unittest.MainTest(m) + unittest.MainTest(m, &unittest.TestOptions{ + SetUp: func() error { + setting.LoadQueueSettings() + return storagecleanup.Init() + }, + }) } diff --git a/services/storagecleanup/storagecleanup.go b/services/storagecleanup/storagecleanup.go new file mode 100644 index 00000000000..ffa9f2164bf --- /dev/null +++ b/services/storagecleanup/storagecleanup.go @@ -0,0 +1,117 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package storagecleanup + +import ( + "context" + "errors" + "fmt" + "os" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/system" + "code.gitea.io/gitea/modules/graceful" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/queue" + "code.gitea.io/gitea/modules/storage" +) + +var cleanQueue *queue.WorkerPoolQueue[int64] + +func Init() error { + cleanQueue = queue.CreateSimpleQueue(graceful.GetManager().ShutdownContext(), "storage-cleanup", handler) + if cleanQueue == nil { + return errors.New("Unable to create storage-cleanup queue") + } + go graceful.GetManager().RunWithCancel(cleanQueue) + return nil +} + +// AddDeletionsToCleanQueue adds the attachments to the clean queue for deletion. +func AddDeletionsToCleanQueue(ctx context.Context, deletionIDs []int64) { + for _, id := range deletionIDs { + if err := cleanQueue.Push(id); err != nil { + log.Error("Failed to push deletion ID %d to clean queue: %v", id, err) + continue + } + } +} + +func handler(deletionIDs ...int64) []int64 { + return cleanupDeletions(graceful.GetManager().ShutdownContext(), deletionIDs) +} + +func cleanupDeletions(ctx context.Context, deletionIDs []int64) []int64 { + var failed []int64 + for _, deletionID := range deletionIDs { + deletion, exist, err := db.GetByID[system.StoragePathDeletion](ctx, deletionID) + if err != nil { + log.Error("Failed to get deletion by ID %d: %v", deletionID, err) + failed = append(failed, deletionID) + continue + } + if !exist { + continue + } + + theStorage, err := storage.GetStorageByName(deletion.StorageName) + if err != nil { + log.Error("Failed to get storage by name %s: %v", deletion.StorageName, err) + failed = append(failed, deletionID) + continue + } + if err := theStorage.Delete(deletion.RelativePath); err != nil { + if !errors.Is(err, os.ErrNotExist) { + log.Error("delete pending deletion[relative path: %s] failed: %v", deletion.RelativePath, err) + failed = append(failed, deletion.ID) + if deletion.DeleteFailedCount%3 == 0 { + _ = system.CreateNotice(ctx, system.NoticeRepository, fmt.Sprintf("Failed to delete pending deletion %s (%d times): %v", deletion.RelativePath, deletion.DeleteFailedCount+1, err)) + } + if err := system.UpdateDeletionFailure(ctx, deletion, err); err != nil { + log.Error("Failed to update deletion failure for ID %d: %v", deletion.ID, err) + } + continue + } + } + if _, err := db.DeleteByID[system.StoragePathDeletion](ctx, deletion.ID); err != nil { + log.Error("Failed to delete pending deletion by ID %d(will be tried later): %v", deletion.ID, err) + failed = append(failed, deletion.ID) + } else { + log.Trace("Pending deletion %s deleted from database", deletion.RelativePath) + } + } + return failed +} + +// ScanToBeDeletedFilesOrDir scans for files or directories that are marked as to be deleted and send to +// clean queue +func ScanToBeDeletedFilesOrDir(ctx context.Context) error { + deletionIDs := make([]int64, 0, 100) + lastID := int64(0) + for { + if err := db.GetEngine(ctx). + Select("id"). + Where("id > ?", lastID). + Asc("id"). + Limit(100). + Find(&deletionIDs); err != nil { + return fmt.Errorf("scan to-be-deleted files or directories: %w", err) + } + + if len(deletionIDs) == 0 { + log.Trace("No more files or directories to be deleted") + break + } + for _, id := range deletionIDs { + if err := cleanQueue.Push(id); err != nil { + log.Error("Failed to push deletion ID %d to clean queue: %v", id, err) + } + } + + lastID = deletionIDs[len(deletionIDs)-1] + deletionIDs = deletionIDs[0:0] + } + + return nil +} diff --git a/services/user/delete.go b/services/user/delete.go index 39c6ef052dc..ef58db1ed38 100644 --- a/services/user/delete.go +++ b/services/user/delete.go @@ -28,18 +28,18 @@ import ( ) // deleteUser deletes models associated to an user. -func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error) { - e := db.GetEngine(ctx) +func deleteUser(ctx context.Context, u *user_model.User, purge bool) (toBeCleanedDeletions []int64, err error) { + toBeCleanedDeletions = make([]int64, 0) // ***** START: Watch ***** watchedRepoIDs, err := db.FindIDs(ctx, "watch", "watch.repo_id", builder.Eq{"watch.user_id": u.ID}. And(builder.Neq{"watch.mode": repo_model.WatchModeDont})) if err != nil { - return fmt.Errorf("get all watches: %w", err) + return nil, fmt.Errorf("get all watches: %w", err) } if err = db.DecrByIDs(ctx, watchedRepoIDs, "num_watches", new(repo_model.Repository)); err != nil { - return fmt.Errorf("decrease repository num_watches: %w", err) + return nil, fmt.Errorf("decrease repository num_watches: %w", err) } // ***** END: Watch ***** @@ -47,9 +47,9 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error) starredRepoIDs, err := db.FindIDs(ctx, "star", "star.repo_id", builder.Eq{"star.uid": u.ID}) if err != nil { - return fmt.Errorf("get all stars: %w", err) + return nil, fmt.Errorf("get all stars: %w", err) } else if err = db.DecrByIDs(ctx, starredRepoIDs, "num_stars", new(repo_model.Repository)); err != nil { - return fmt.Errorf("decrease repository num_stars: %w", err) + return nil, fmt.Errorf("decrease repository num_stars: %w", err) } // ***** END: Star ***** @@ -57,17 +57,17 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error) followeeIDs, err := db.FindIDs(ctx, "follow", "follow.follow_id", builder.Eq{"follow.user_id": u.ID}) if err != nil { - return fmt.Errorf("get all followees: %w", err) + return nil, fmt.Errorf("get all followees: %w", err) } else if err = db.DecrByIDs(ctx, followeeIDs, "num_followers", new(user_model.User)); err != nil { - return fmt.Errorf("decrease user num_followers: %w", err) + return nil, fmt.Errorf("decrease user num_followers: %w", err) } followerIDs, err := db.FindIDs(ctx, "follow", "follow.user_id", builder.Eq{"follow.follow_id": u.ID}) if err != nil { - return fmt.Errorf("get all followers: %w", err) + return nil, fmt.Errorf("get all followers: %w", err) } else if err = db.DecrByIDs(ctx, followerIDs, "num_following", new(user_model.User)); err != nil { - return fmt.Errorf("decrease user num_following: %w", err) + return nil, fmt.Errorf("decrease user num_following: %w", err) } // ***** END: Follow ***** @@ -96,11 +96,11 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error) &user_model.Blocking{BlockeeID: u.ID}, &actions_model.ActionRunnerToken{OwnerID: u.ID}, ); err != nil { - return fmt.Errorf("deleteBeans: %w", err) + return nil, fmt.Errorf("deleteBeans: %w", err) } if err := auth_model.DeleteOAuth2RelictsByUserID(ctx, u.ID); err != nil { - return err + return nil, err } if purge || (setting.Service.UserDeleteWithCommentsMaxTime != 0 && @@ -109,23 +109,34 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error) const batchSize = 50 for { comments := make([]*issues_model.Comment, 0, batchSize) - if err = e.Where("type=? AND poster_id=?", issues_model.CommentTypeComment, u.ID).Limit(batchSize, 0).Find(&comments); err != nil { - return err + if err = db.GetEngine(ctx).Where("type=? AND poster_id=?", issues_model.CommentTypeComment, u.ID).Limit(batchSize, 0).Find(&comments); err != nil { + return nil, err } if len(comments) == 0 { break } for _, comment := range comments { - if err = issues_model.DeleteComment(ctx, comment); err != nil { - return err + // Delete attachments of the comments + if err := comment.LoadAttachments(ctx); err != nil { + return nil, err } + + if err = issues_model.DeleteComment(ctx, comment); err != nil { + return nil, err + } + + pendingDeletions, err := repo_model.DeleteAttachments(ctx, comment.Attachments) + if err != nil { + return nil, err + } + toBeCleanedDeletions = append(toBeCleanedDeletions, pendingDeletions...) } } // Delete Reactions if err = issues_model.DeleteReaction(ctx, &issues_model.ReactionOptions{DoerID: u.ID}); err != nil { - return err + return nil, err } } @@ -139,15 +150,15 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error) // though that query will be quite complex and tricky to maintain (compare `getRepoAssignees()`). // Also, as we didn't update branch protections when removing entries from `access` table, // it's safer to iterate all protected branches. - if err = e.Limit(batchSize, start).Find(&protections); err != nil { - return fmt.Errorf("findProtectedBranches: %w", err) + if err = db.GetEngine(ctx).Limit(batchSize, start).Find(&protections); err != nil { + return nil, fmt.Errorf("findProtectedBranches: %w", err) } if len(protections) == 0 { break } for _, p := range protections { if err := git_model.RemoveUserIDFromProtectedBranch(ctx, p, u.ID); err != nil { - return err + return nil, err } } } @@ -156,7 +167,7 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error) // ***** START: PublicKey ***** if _, err = db.DeleteByBean(ctx, &asymkey_model.PublicKey{OwnerID: u.ID}); err != nil { - return fmt.Errorf("deletePublicKeys: %w", err) + return nil, fmt.Errorf("deletePublicKeys: %w", err) } // ***** END: PublicKey ***** @@ -165,37 +176,37 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error) OwnerID: u.ID, }) if err != nil { - return fmt.Errorf("ListGPGKeys: %w", err) + return nil, fmt.Errorf("ListGPGKeys: %w", err) } // Delete GPGKeyImport(s). for _, key := range keys { if _, err = db.DeleteByBean(ctx, &asymkey_model.GPGKeyImport{KeyID: key.KeyID}); err != nil { - return fmt.Errorf("deleteGPGKeyImports: %w", err) + return nil, fmt.Errorf("deleteGPGKeyImports: %w", err) } } if _, err = db.DeleteByBean(ctx, &asymkey_model.GPGKey{OwnerID: u.ID}); err != nil { - return fmt.Errorf("deleteGPGKeys: %w", err) + return nil, fmt.Errorf("deleteGPGKeys: %w", err) } // ***** END: GPGPublicKey ***** // Clear assignee. if _, err = db.DeleteByBean(ctx, &issues_model.IssueAssignees{AssigneeID: u.ID}); err != nil { - return fmt.Errorf("clear assignee: %w", err) + return nil, fmt.Errorf("clear assignee: %w", err) } // ***** START: ExternalLoginUser ***** if err = user_model.RemoveAllAccountLinks(ctx, u); err != nil { - return fmt.Errorf("ExternalLoginUser: %w", err) + return nil, fmt.Errorf("ExternalLoginUser: %w", err) } // ***** END: ExternalLoginUser ***** if err := auth_model.DeleteAuthTokensByUserID(ctx, u.ID); err != nil { - return fmt.Errorf("DeleteAuthTokensByUserID: %w", err) + return nil, fmt.Errorf("DeleteAuthTokensByUserID: %w", err) } if _, err = db.DeleteByID[user_model.User](ctx, u.ID); err != nil { - return fmt.Errorf("delete: %w", err) + return nil, fmt.Errorf("delete: %w", err) } - return nil + return toBeCleanedDeletions, nil } diff --git a/services/user/user.go b/services/user/user.go index c7252430dea..c9aab51ecd9 100644 --- a/services/user/user.go +++ b/services/user/user.go @@ -28,6 +28,7 @@ import ( "code.gitea.io/gitea/services/packages" container_service "code.gitea.io/gitea/services/packages/container" repo_service "code.gitea.io/gitea/services/repository" + "code.gitea.io/gitea/services/storagecleanup" ) // RenameUser renames a user @@ -210,47 +211,45 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error { } } - ctx, committer, err := db.TxContext(ctx) + toBeCleanedDeletions, err := db.WithTx2(ctx, func(ctx context.Context) ([]int64, error) { + // Note: A user owns any repository or belongs to any organization + // cannot perform delete operation. This causes a race with the purge above + // however consistency requires that we ensure that this is the case + + // Check ownership of repository. + count, err := repo_model.CountRepositories(ctx, repo_model.CountRepositoryOptions{OwnerID: u.ID}) + if err != nil { + return nil, fmt.Errorf("GetRepositoryCount: %w", err) + } else if count > 0 { + return nil, repo_model.ErrUserOwnRepos{UID: u.ID} + } + + // Check membership of organization. + count, err = organization.GetOrganizationCount(ctx, u) + if err != nil { + return nil, fmt.Errorf("GetOrganizationCount: %w", err) + } else if count > 0 { + return nil, organization.ErrUserHasOrgs{UID: u.ID} + } + + // Check ownership of packages. + if ownsPackages, err := packages_model.HasOwnerPackages(ctx, u.ID); err != nil { + return nil, fmt.Errorf("HasOwnerPackages: %w", err) + } else if ownsPackages { + return nil, packages_model.ErrUserOwnPackages{UID: u.ID} + } + + toBeCleanedDeletions, err := deleteUser(ctx, u, purge) + if err != nil { + return nil, fmt.Errorf("DeleteUser: %w", err) + } + return toBeCleanedDeletions, nil + }) if err != nil { return err } - defer committer.Close() - // Note: A user owns any repository or belongs to any organization - // cannot perform delete operation. This causes a race with the purge above - // however consistency requires that we ensure that this is the case - - // Check ownership of repository. - count, err := repo_model.CountRepositories(ctx, repo_model.CountRepositoryOptions{OwnerID: u.ID}) - if err != nil { - return fmt.Errorf("GetRepositoryCount: %w", err) - } else if count > 0 { - return repo_model.ErrUserOwnRepos{UID: u.ID} - } - - // Check membership of organization. - count, err = organization.GetOrganizationCount(ctx, u) - if err != nil { - return fmt.Errorf("GetOrganizationCount: %w", err) - } else if count > 0 { - return organization.ErrUserHasOrgs{UID: u.ID} - } - - // Check ownership of packages. - if ownsPackages, err := packages_model.HasOwnerPackages(ctx, u.ID); err != nil { - return fmt.Errorf("HasOwnerPackages: %w", err) - } else if ownsPackages { - return packages_model.ErrUserOwnPackages{UID: u.ID} - } - - if err := deleteUser(ctx, u, purge); err != nil { - return fmt.Errorf("DeleteUser: %w", err) - } - - if err := committer.Commit(); err != nil { - return err - } - _ = committer.Close() + storagecleanup.AddDeletionsToCleanQueue(ctx, toBeCleanedDeletions) if err = asymkey_service.RewriteAllPublicKeys(ctx); err != nil { return err diff --git a/services/user/user_test.go b/services/user/user_test.go index 28a0df8628f..868cd08c4ea 100644 --- a/services/user/user_test.go +++ b/services/user/user_test.go @@ -18,12 +18,18 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" org_service "code.gitea.io/gitea/services/org" + "code.gitea.io/gitea/services/storagecleanup" "github.com/stretchr/testify/assert" ) func TestMain(m *testing.M) { - unittest.MainTest(m) + unittest.MainTest(m, &unittest.TestOptions{ + SetUp: func() error { + setting.LoadQueueSettings() + return storagecleanup.Init() + }, + }) } func TestDeleteUser(t *testing.T) { diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index d28a103e596..1cceacefbad 100644 --- a/tests/integration/api_admin_test.go +++ b/tests/integration/api_admin_test.go @@ -304,11 +304,11 @@ func TestAPICron(t *testing.T) { AddTokenAuth(token) resp := MakeRequest(t, req, http.StatusOK) - assert.Equal(t, "29", resp.Header().Get("X-Total-Count")) + assert.Equal(t, "30", resp.Header().Get("X-Total-Count")) var crons []api.Cron DecodeJSON(t, resp, &crons) - assert.Len(t, crons, 29) + assert.Len(t, crons, 30) }) t.Run("Execute", func(t *testing.T) {