diff --git a/models/fixtures/attachment.yml b/models/fixtures/attachment.yml index 7882d8bff2089..1c1eb402befc8 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/fixtures/storage_path_deletion.yml b/models/fixtures/storage_path_deletion.yml new file mode 100644 index 0000000000000..476bd96da7326 --- /dev/null +++ b/models/fixtures/storage_path_deletion.yml @@ -0,0 +1,6 @@ +- + id: 1 + storage_name: attachment + path_type: 1 + relative_path: "1/2/1213b3ce-b1af-4a82-9279-bc30ea91aa45" + created_unix: 1234567890 diff --git a/models/issues/comment.go b/models/issues/comment.go index d22f08fa87621..f393e4838786d 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 4f899453b5f57..b1f07db50cea4 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 0000000000000..a804f4ea7bf5a --- /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 835bee540250d..955398235372d 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) error { if len(attachments) == 0 { - return 0, nil + return nil } ids := make([]int64, 0, len(attachments)) @@ -183,42 +176,25 @@ 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 - } + return db.WithTx(ctx, func(ctx context.Context) error { + // delete attachments from database + if _, err := db.GetEngine(ctx).Table("attachment").In("id", ids).Delete(); err != nil { + return 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()) + // add disk files to pending deletion table as well + 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 fmt.Errorf("insert pending deletion: %w", 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 - } - - 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) + return nil + }) } // UpdateAttachmentByUUID Updates attachment via uuid @@ -243,12 +219,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 c059ffd39a91e..91d44159f2269 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_path_deletion.go b/models/system/storage_path_deletion.go new file mode 100644 index 0000000000000..3151b0a62ae52 --- /dev/null +++ b/models/system/storage_path_deletion.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/modules/storage/storage.go b/modules/storage/storage.go index 1868817c057cf..c017838be1b64 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 53fbf0af767ad..37c495ef8e760 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 3f751a295c37e..550abf4a7b373 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 5f660c57504dd..704db1c7a3a83 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 defde81a1d2ae..fa25c0cdfddaf 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/web/repo/attachment.go b/routers/web/repo/attachment.go index f696669196100..ff8d098c1f2a6 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 54b7e5df2a01c..84035b5983569 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 ccb97c66c82b1..115480bed05bc 100644 --- a/services/attachment/attachment.go +++ b/services/attachment/attachment.go @@ -59,3 +59,19 @@ 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) { + err := repo_model.DeleteAttachments(ctx, attachments) + if err != nil { + return 0, err + } + + return len(attachments), nil +} diff --git a/services/attachment/attachment_test.go b/services/attachment/attachment_test.go index 65475836becab..0e5a31cff127e 100644 --- a/services/attachment/attachment_test.go +++ b/services/attachment/attachment_test.go @@ -6,12 +6,16 @@ package attachment import ( "os" "path/filepath" + "strings" "testing" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/system" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/storage" + storage_service "code.gitea.io/gitea/services/storage" _ "code.gitea.io/gitea/models/actions" @@ -44,3 +48,42 @@ 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()) + + deletionsTotal, err := db.Count[system.StoragePathDeletion](t.Context(), db.ListOptionsAll) + assert.NoError(t, err) + assert.Equal(t, int64(1), deletionsTotal) + + err = DeleteAttachment(t.Context(), attachment8) + assert.NoError(t, err) + + attachment, err := repo_model.GetAttachmentByUUID(t.Context(), attachment8.UUID) + assert.Error(t, err) + assert.True(t, repo_model.IsErrAttachmentNotExist(err)) + assert.Nil(t, attachment) + + deletions, err := db.Find[system.StoragePathDeletion](t.Context(), db.ListOptionsAll) + assert.NoError(t, err) + assert.Len(t, deletions, int(deletionsTotal)+1) + assert.Equal(t, attachment8.RelativePath(), deletions[deletionsTotal].RelativePath) + + _, err = storage.Attachments.Stat(attachment8.RelativePath()) + assert.NoError(t, err) + + err = storage_service.ScanToBeDeletedFilesOrDir(t.Context()) + assert.NoError(t, err) + + _, 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 0018c5facc5d7..43b361fa923ab 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" + storage_service "code.gitea.io/gitea/services/storage" user_service "code.gitea.io/gitea/services/user" ) @@ -223,6 +224,16 @@ func registerRebuildIssueIndexer() { }) } +func registerCleanStorage() { + RegisterTaskFatal("cleanup_storage", &BaseConfig{ + Enabled: true, + RunAtStart: false, + Schedule: "@every 1m", // Run every minute to check for deletions + }, func(ctx context.Context, _ *user_model.User, _ Config) error { + return storage_service.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 10c81198d57e2..1017840c220a8 100644 --- a/services/issue/comments.go +++ b/services/issue/comments.go @@ -130,12 +130,29 @@ 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) error { + return db.WithTx(ctx, func(ctx context.Context) error { + if removeAttachments { + // load attachments before deleting the comment + if err := comment.LoadAttachments(ctx); err != nil { + return err + } + } + + if err := issues_model.DeleteComment(ctx, comment); err != nil { + return err + } + + if removeAttachments { + return repo_model.DeleteAttachments(ctx, comment.Attachments) + } + return nil }) - if err != nil { +} + +func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) error { + if err := deleteComment(ctx, comment, true); err != nil { return err } diff --git a/services/issue/issue.go b/services/issue/issue.go index f03be3e18f6c7..71a022c85bdd1 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -13,12 +13,10 @@ 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" ) @@ -190,18 +188,14 @@ func DeleteIssue(ctx context.Context, doer *user_model.User, gitRepo *git.Reposi } // delete entries in database - attachmentPaths, err := deleteIssue(ctx, issue) - if err != nil { + if err := deleteIssue(ctx, issue, true); err != nil { return err } - for _, attachmentPath := range attachmentPaths { - system_model.RemoveStorageWithNotice(ctx, storage.Attachments, "Delete issue attachment", attachmentPath) - } // 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 +254,98 @@ 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 - } +func deleteIssue(ctx context.Context, issue *issues_model.Issue, deleteAttachments bool) error { + return db.WithTx(ctx, func(ctx context.Context) error { + if _, err := db.GetEngine(ctx).ID(issue.ID).NoAutoCondition().Delete(issue); err != nil { + return 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 + // update the total issue numbers + if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil { + return 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 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) - } + if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil { + return fmt.Errorf("error updating counters for milestone id %d: %w", + issue.MilestoneID, err) + } - if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID, issue.Index); err != nil { - return nil, err - } + if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID, issue.Index); err != nil { + return err + } - // find attachments related to this issue and remove them - if err := issue.LoadAttachments(ctx); err != nil { - return nil, err - } + if deleteAttachments { + // find attachments related to this issue and remove them + if err := issue.LoadAttachments(ctx); err != nil { + return err + } + } - var attachmentPaths []string - for i := range issue.Attachments { - attachmentPaths = append(attachmentPaths, issue.Attachments[i].RelativePath()) - } + // 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 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 - } + for _, comment := range issue.Comments { + if err := deleteComment(ctx, comment, deleteAttachments); err != nil { + return fmt.Errorf("deleteComment [comment_id: %d]: %w", comment.ID, err) + } + } - if err := committer.Commit(); err != nil { - return nil, err - } - return attachmentPaths, nil + if deleteAttachments { + // delete issue attachments + if err := issue.LoadAttachments(ctx); err != nil { + return err + } + if err := repo_model.DeleteAttachments(ctx, issue.Attachments); err != nil { + return err + } + } + return 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 { + return 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]) - if err != nil { + if err := DeleteIssuesByRepoID(ctx, repoIDs[i], true); err != nil { return err } - attachmentPaths = append(attachmentPaths, paths...) } return nil }) - if err != nil { - return err - } - - // Remove issue attachment files. - for i := range attachmentPaths { - system_model.RemoveStorageWithNotice(ctx, storage.Attachments, "Delete issue attachment", attachmentPaths[i]) - } - 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) error { for { issues := make([]*issues_model.Issue, 0, db.DefaultMaxInSize) if err := db.GetEngine(ctx). @@ -368,7 +353,7 @@ func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths [] OrderBy("id"). Limit(db.DefaultMaxInSize). Find(&issues); err != nil { - return nil, err + return err } if len(issues) == 0 { @@ -376,14 +361,11 @@ func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths [] } for _, issue := range issues { - issueAttachPaths, err := deleteIssue(ctx, issue) - if err != nil { - return nil, fmt.Errorf("deleteIssue [issue_id: %d]: %w", issue.ID, err) + if err := deleteIssue(ctx, issue, deleteAttachments); err != nil { + return fmt.Errorf("deleteIssue [issue_id: %d]: %w", issue.ID, err) } - - attachmentPaths = append(attachmentPaths, issueAttachPaths...) } } - return attachmentPaths, err + return nil } diff --git a/services/issue/issue_test.go b/services/issue/issue_test.go index bad0d65d1ed8f..a6bc1014ad580 100644 --- a/services/issue/issue_test.go +++ b/services/issue/issue_test.go @@ -44,7 +44,7 @@ func TestIssue_DeleteIssue(t *testing.T) { ID: issueIDs[2], } - _, err = deleteIssue(db.DefaultContext, issue) + err = deleteIssue(db.DefaultContext, issue, true) assert.NoError(t, err) issueIDs, err = issues_model.GetIssueIDsByRepoID(db.DefaultContext, 1) assert.NoError(t, err) @@ -55,7 +55,7 @@ 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) + err = deleteIssue(db.DefaultContext, issue, true) assert.NoError(t, err) assert.Len(t, attachments, 2) for i := range attachments { @@ -78,7 +78,7 @@ func TestIssue_DeleteIssue(t *testing.T) { assert.NoError(t, err) assert.False(t, left) - _, err = deleteIssue(db.DefaultContext, issue2) + err = deleteIssue(db.DefaultContext, issue2, true) assert.NoError(t, err) left, err = issues_model.IssueNoDependenciesLeft(db.DefaultContext, issue1) assert.NoError(t, err) diff --git a/services/org/org.go b/services/org/org.go index 3d30ae21a39d3..43aa32485c875 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}) - 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} - } + 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} + } - if err := deleteOrganization(ctx, org); err != nil { - return fmt.Errorf("DeleteOrganization: %w", err) - } + // 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 := committer.Commit(); err != nil { + if err := deleteOrganization(ctx, org); err != nil { + return fmt.Errorf("DeleteOrganization: %w", err) + } + return nil + }) + if err != nil { return err } diff --git a/services/release/release.go b/services/release/release.go index 0b8a74252a08d..f9eb3de7c6454 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -19,7 +19,6 @@ 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" @@ -288,6 +287,7 @@ 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)) if len(delAttachmentUUIDs) > 0 { // Check attachments attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, delAttachmentUUIDs) @@ -299,10 +299,11 @@ 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) + if err := repo_model.DeleteAttachments(ctx, deletedAttachments); err != nil { + return fmt.Errorf("DeleteAttachments [uuids: %v]: %w", deletedUUIDs.Values(), err) } } @@ -338,16 +339,6 @@ 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) - } - } - if !rel.IsDraft { if !isTagCreated && !isConvertedFromTag { notify_service.UpdateRelease(gitRepo.Ctx, doer, rel) @@ -360,63 +351,61 @@ 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, + 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) - } + 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 + 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 err := repo_model.UpdateRelease(ctx, rel); err != nil { + return fmt.Errorf("Update: %w", 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) - } + rel.Repo = repo + if err := rel.LoadAttributes(ctx); err != nil { + return fmt.Errorf("LoadAttributes: %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) + if err := repo_model.DeleteAttachments(ctx, rel.Attachments); err != nil { + return fmt.Errorf("DeleteAttachments: %w", err) } + return nil + }); err != nil { + return err } if !rel.IsDraft { diff --git a/services/repository/delete.go b/services/repository/delete.go index c48d6e1d56e94..706ded8f31588 100644 --- a/services/repository/delete.go +++ b/services/repository/delete.go @@ -50,15 +50,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 +74,216 @@ 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 - // 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 - } + 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 + } + } - if org != nil && org.IsOrganization() { - teams, err := organization.FindOrgTeams(ctx, org.ID) + // Delete Deploy Keys + deleted, err := asymkey_service.DeleteRepoDeployKeys(ctx, repoID) 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 { + 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 { + 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) - } + 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 + } - 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) - } + if err := repo_model.DeleteAttachments(ctx, releaseAttachments); err != nil { + return fmt.Errorf("delete release attachments: %w", err) + } - // Delete Labels and related objects - if err := issues_model.DeleteLabelsByRepoID(ctx, repoID); err != nil { - return err - } + 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 + } - // Delete Pulls and related objects - if err := issues_model.DeletePullsByBaseRepoID(ctx, repoID); 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 + } - // Delete Issues and related objects - var attachmentPaths []string - if attachmentPaths, err = issue_service.DeleteIssuesByRepoID(ctx, repoID); 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) + } - // Delete issue index - if err := db.DeleteResourceIndex(ctx, "issue_index", repoID); err != nil { - return 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) + } - 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) + // Delete Labels and related objects + if err := issues_model.DeleteLabelsByRepoID(ctx, repoID); err != nil { + return err } - } - if _, err := db.Exec(ctx, "UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", repo.OwnerID); err != nil { - return err - } + // Delete Pulls and related objects + if err := issues_model.DeletePullsByBaseRepoID(ctx, repoID); err != nil { + return err + } - if len(repo.Topics) > 0 { - if err := repo_model.RemoveTopicsFromRepo(ctx, repo.ID); err != nil { + // 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 } - } - if err := project_model.DeleteProjectByRepoID(ctx, repoID); err != nil { - return fmt.Errorf("unable to delete projects for repo[%d]: %w", repoID, err) - } + // Delete issue index + if err := db.DeleteResourceIndex(ctx, "issue_index", repoID); err != nil { + return err + } - // Remove LFS objects - var lfsObjects []*git_model.LFSMetaObject - if err = sess.Where("repository_id=?", repoID).Find(&lfsObjects); 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) + } + } - 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 { + if _, err := db.Exec(ctx, "UPDATE `user` SET num_repos=num_repos-1 WHERE id=?", repo.OwnerID); err != nil { return err } - if count > 1 { - continue + + if len(repo.Topics) > 0 { + if err := repo_model.RemoveTopicsFromRepo(ctx, repo.ID); err != nil { + return err + } } - lfsPaths = append(lfsPaths, v.RelativePath()) - } + if err := project_model.DeleteProjectByRepoID(ctx, repoID); err != nil { + return fmt.Errorf("unable to delete projects for repo[%d]: %w", repoID, err) + } - if _, err := db.DeleteByBean(ctx, &git_model.LFSMetaObject{RepositoryID: repoID}); err != nil { - return err - } + // Remove LFS objects + var lfsObjects []*git_model.LFSMetaObject + if err = db.GetEngine(ctx).Where("repository_id=?", repoID).Find(&lfsObjects); err != nil { + return err + } - // Remove archives - var archives []*repo_model.RepoArchiver - if err = sess.Where("repo_id=?", repoID).Find(&archives); 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 + } - archivePaths := make([]string, 0, len(archives)) - for _, v := range archives { - archivePaths = append(archivePaths, v.RelativePath()) - } + lfsPaths = append(lfsPaths, v.RelativePath()) + } - if _, err := db.DeleteByBean(ctx, &repo_model.RepoArchiver{RepoID: repoID}); err != nil { - return err - } + if _, err := db.DeleteByBean(ctx, &git_model.LFSMetaObject{RepositoryID: 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) + // Remove archives + var archives []*repo_model.RepoArchiver + if err = db.GetEngine(ctx).Where("repo_id=?", repoID).Find(&archives); 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 { - return err - } + archivePaths = make([]string, 0, len(archives)) + for _, v := range archives { + archivePaths = append(archivePaths, v.RelativePath()) + } - newAttachmentPaths := make([]string, 0, len(newAttachments)) - for _, attach := range newAttachments { - newAttachmentPaths = append(newAttachmentPaths, attach.RelativePath()) - } + if _, err := db.DeleteByBean(ctx, &repo_model.RepoArchiver{RepoID: repoID}); err != nil { + return err + } - if _, err := sess.Where("repo_id=?", repo.ID).Delete(new(repo_model.Attachment)); 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) + } + } - // unlink packages linked to this repository - if err = packages_model.UnlinkRepositoryFromAllPackages(ctx, repoID); err != nil { - return 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 + } + if err = repo_model.DeleteAttachments(ctx, repoAttachments); err != nil { + return err + } - if err = committer.Commit(); err != nil { + // unlink packages linked to this repository + return packages_model.UnlinkRepositoryFromAllPackages(ctx, repoID) + }) + if err != nil { return err } - committer.Close() - if needRewriteKeysFile { if err := asymkey_service.RewriteAllPublicKeys(ctx); err != nil { log.Error("RewriteAllPublicKeys failed: %v", err) @@ -330,21 +317,6 @@ 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) - } - if len(repo.Avatar) > 0 { if err := storage.RepoAvatars.Delete(repo.CustomAvatarRelativePath()); err != nil { log.Error("remove avatar file %q: %v", repo.CustomAvatarRelativePath(), err) diff --git a/services/storage/cleanup.go b/services/storage/cleanup.go new file mode 100644 index 0000000000000..752359680fd92 --- /dev/null +++ b/services/storage/cleanup.go @@ -0,0 +1,81 @@ +// Copyright 2025 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package storage + +import ( + "context" + "errors" + "fmt" + "os" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/models/system" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/storage" +) + +func ProcessDeletions(ctx context.Context, deletionIDs []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) + 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) + 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) + 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) + } else { + log.Trace("Pending deletion %s deleted from database", deletion.RelativePath) + } + } +} + +// ScanToBeDeletedFilesOrDir scans for files or directories that are marked as to +// be deleted and processes them in batches. +func ScanToBeDeletedFilesOrDir(ctx context.Context) error { + deletionIDs := make([]int64, 0, 100) + lastID := int64(0) + for { + if err := db.GetEngine(ctx). + Table("storage_path_deletion"). + 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 + } + ProcessDeletions(ctx, deletionIDs) + + lastID = deletionIDs[len(deletionIDs)-1] + deletionIDs = deletionIDs[0:0] + } + + return nil +} diff --git a/services/user/delete.go b/services/user/delete.go index 39c6ef052dca7..07e904db36225 100644 --- a/services/user/delete.go +++ b/services/user/delete.go @@ -29,8 +29,6 @@ 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) - // ***** START: Watch ***** watchedRepoIDs, err := db.FindIDs(ctx, "watch", "watch.repo_id", builder.Eq{"watch.user_id": u.ID}. @@ -109,7 +107,7 @@ 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 { + if err = db.GetEngine(ctx).Where("type=? AND poster_id=?", issues_model.CommentTypeComment, u.ID).Limit(batchSize, 0).Find(&comments); err != nil { return err } if len(comments) == 0 { @@ -117,9 +115,18 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error) } for _, comment := range comments { + // Delete attachments of the comments + if err := comment.LoadAttachments(ctx); err != nil { + return err + } + if err = issues_model.DeleteComment(ctx, comment); err != nil { return err } + + if err := repo_model.DeleteAttachments(ctx, comment.Attachments); err != nil { + return err + } } } @@ -139,7 +146,7 @@ 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 { + if err = db.GetEngine(ctx).Limit(batchSize, start).Find(&protections); err != nil { return fmt.Errorf("findProtectedBranches: %w", err) } if len(protections) == 0 { diff --git a/services/user/user.go b/services/user/user.go index c7252430dea03..7982649c8fa01 100644 --- a/services/user/user.go +++ b/services/user/user.go @@ -210,47 +210,42 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error { } } - ctx, committer, err := db.TxContext(ctx) - 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 + err := db.WithTx(ctx, func(ctx context.Context) 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 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 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 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} - } + // 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} + } - if err := deleteUser(ctx, u, purge); err != nil { - return fmt.Errorf("DeleteUser: %w", err) - } + // 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 := committer.Commit(); err != nil { + if err := deleteUser(ctx, u, purge); err != nil { + return fmt.Errorf("DeleteUser: %w", err) + } + return nil + }) + if err != nil { return err } - _ = committer.Close() if err = asymkey_service.RewriteAllPublicKeys(ctx); err != nil { return err diff --git a/tests/integration/api_admin_test.go b/tests/integration/api_admin_test.go index d28a103e596f8..1cceacefbad98 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) {