improvements

This commit is contained in:
Lunny Xiao 2025-07-16 19:33:20 -07:00
parent 2d423dc78c
commit 3abb7571a8
11 changed files with 177 additions and 85 deletions

17
modules/util/cleanup.go Normal file
View File

@ -0,0 +1,17 @@
// Copyright 2025 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package util
type CleanUpFunc func()
func NewCleanUpFunc() CleanUpFunc {
return func() {}
}
func (f CleanUpFunc) Append(newF CleanUpFunc) CleanUpFunc {
return func() {
f()
newF()
}
}

View File

@ -726,7 +726,7 @@ func deleteIssueComment(ctx *context.APIContext) {
return return
} }
if _, err = issue_service.DeleteComment(ctx, ctx.Doer, comment, true); err != nil { if _, err = issue_service.DeleteComment(ctx, ctx.Doer, comment); err != nil {
ctx.APIErrorInternal(err) ctx.APIErrorInternal(err)
return return
} }

View File

@ -325,7 +325,7 @@ func DeleteComment(ctx *context.Context) {
return return
} }
deletedReviewComment, err := issue_service.DeleteComment(ctx, ctx.Doer, comment, true) deletedReviewComment, err := issue_service.DeleteComment(ctx, ctx.Doer, comment)
if err != nil { if err != nil {
ctx.ServerError("DeleteComment", err) ctx.ServerError("DeleteComment", err)
return return

View File

@ -7,6 +7,7 @@ import (
"context" "context"
"errors" "errors"
"fmt" "fmt"
"os"
"code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/db"
issues_model "code.gitea.io/gitea/models/issues" issues_model "code.gitea.io/gitea/models/issues"
@ -15,8 +16,10 @@ import (
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/gitrepo" "code.gitea.io/gitea/modules/gitrepo"
"code.gitea.io/gitea/modules/json" "code.gitea.io/gitea/modules/json"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/storage"
"code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/timeutil"
attachment_service "code.gitea.io/gitea/services/attachment" "code.gitea.io/gitea/modules/util"
git_service "code.gitea.io/gitea/services/git" git_service "code.gitea.io/gitea/services/git"
notify_service "code.gitea.io/gitea/services/notify" notify_service "code.gitea.io/gitea/services/notify"
) )
@ -132,9 +135,11 @@ func UpdateComment(ctx context.Context, c *issues_model.Comment, contentVersion
} }
// deleteComment deletes the comment // deleteComment deletes the comment
func deleteComment(ctx context.Context, comment *issues_model.Comment, removeAttachments bool) (*issues_model.Comment, error) { func deleteComment(ctx context.Context, comment *issues_model.Comment, removeAttachments bool) (*issues_model.Comment, func(), error) {
return db.WithTx2(ctx, func(ctx context.Context) (*issues_model.Comment, error) { storageCleanup := util.NewCleanUpFunc()
deletedReviewComment, err := db.WithTx2(ctx, func(ctx context.Context) (*issues_model.Comment, error) {
if removeAttachments { if removeAttachments {
// load attachments before deleting the comment
if err := comment.LoadAttachments(ctx); err != nil { if err := comment.LoadAttachments(ctx); err != nil {
return nil, err return nil, err
} }
@ -147,20 +152,42 @@ func deleteComment(ctx context.Context, comment *issues_model.Comment, removeAtt
if removeAttachments { if removeAttachments {
// delete comment attachments // delete comment attachments
if _, err := attachment_service.DeleteAttachments(ctx, comment.Attachments); err != nil { _, err := db.GetEngine(ctx).Where("comment_id = ?", comment.ID).NoAutoCondition().Delete(&repo_model.Attachment{})
if err != nil {
return nil, err return nil, err
} }
}
// the storage cleanup function to remove attachments could be called after all transactions are committed
storageCleanup = storageCleanup.Append(func() {
for _, a := range comment.Attachments {
if err := storage.Attachments.Delete(a.RelativePath()); err != nil {
if !errors.Is(err, os.ErrNotExist) {
// 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", a.UUID, err)
} else {
log.Warn("Attachment file not found when deleting: %s", a.RelativePath())
}
}
}
})
}
return deletedReviewComment, nil return deletedReviewComment, nil
}) })
if err != nil {
return nil, nil, err
}
return deletedReviewComment, storageCleanup, nil
} }
func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment, removeAttachments bool) (*issues_model.Comment, error) { func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) (*issues_model.Comment, error) {
deletedReviewComment, err := deleteComment(ctx, comment, removeAttachments) deletedReviewComment, cleanup, err := deleteComment(ctx, comment, false)
if err != nil { if err != nil {
return nil, err return nil, err
} }
cleanup()
notify_service.DeleteComment(ctx, doer, comment) notify_service.DeleteComment(ctx, doer, comment)

View File

@ -27,7 +27,7 @@ func Test_DeleteCommentWithReview(t *testing.T) {
user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1}) user1 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
// since this is the last comment of the review, it should be deleted when the comment is deleted // since this is the last comment of the review, it should be deleted when the comment is deleted
deletedReviewComment, err := DeleteComment(db.DefaultContext, user1, comment, true) deletedReviewComment, err := DeleteComment(db.DefaultContext, user1, comment)
assert.NoError(t, err) assert.NoError(t, err)
assert.NotNil(t, deletedReviewComment) assert.NotNil(t, deletedReviewComment)

View File

@ -19,6 +19,7 @@ import (
"code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/storage"
"code.gitea.io/gitea/modules/util"
notify_service "code.gitea.io/gitea/services/notify" notify_service "code.gitea.io/gitea/services/notify"
) )
@ -190,9 +191,11 @@ func DeleteIssue(ctx context.Context, doer *user_model.User, gitRepo *git.Reposi
} }
// delete entries in database // delete entries in database
if err := deleteIssue(ctx, issue, true); err != nil { cleanup, err := deleteIssue(ctx, issue, true)
if err != nil {
return err return err
} }
cleanup()
// delete pull request related git data // delete pull request related git data
if issue.IsPull && gitRepo != nil { if issue.IsPull && gitRepo != nil {
@ -256,8 +259,9 @@ func GetRefEndNamesAndURLs(issues []*issues_model.Issue, repoLink string) (map[i
} }
// deleteIssue deletes the issue // deleteIssue deletes the issue
func deleteIssue(ctx context.Context, issue *issues_model.Issue, deleteAttachments bool) error { func deleteIssue(ctx context.Context, issue *issues_model.Issue, deleteAttachments bool) (util.CleanUpFunc, error) {
return db.WithTx(ctx, func(ctx context.Context) error { cleanup := util.NewCleanUpFunc()
if err := db.WithTx(ctx, func(ctx context.Context) error {
if _, err := db.GetEngine(ctx).ID(issue.ID).NoAutoCondition().Delete(issue); err != nil { if _, err := db.GetEngine(ctx).ID(issue.ID).NoAutoCondition().Delete(issue); err != nil {
return err return err
} }
@ -302,7 +306,6 @@ func deleteIssue(ctx context.Context, issue *issues_model.Issue, deleteAttachmen
&issues_model.Stopwatch{IssueID: issue.ID}, &issues_model.Stopwatch{IssueID: issue.ID},
&issues_model.TrackedTime{IssueID: issue.ID}, &issues_model.TrackedTime{IssueID: issue.ID},
&project_model.ProjectIssue{IssueID: issue.ID}, &project_model.ProjectIssue{IssueID: issue.ID},
&repo_model.Attachment{IssueID: issue.ID},
&issues_model.PullRequest{IssueID: issue.ID}, &issues_model.PullRequest{IssueID: issue.ID},
&issues_model.Comment{RefIssueID: issue.ID}, &issues_model.Comment{RefIssueID: issue.ID},
&issues_model.IssueDependency{DependencyID: issue.ID}, &issues_model.IssueDependency{DependencyID: issue.ID},
@ -313,19 +316,32 @@ func deleteIssue(ctx context.Context, issue *issues_model.Issue, deleteAttachmen
} }
for _, comment := range issue.Comments { for _, comment := range issue.Comments {
if _, err := deleteComment(ctx, comment, deleteAttachments); err != nil { _, cleanupDeleteComment, err := deleteComment(ctx, comment, deleteAttachments)
if err != nil {
return fmt.Errorf("deleteComment [comment_id: %d]: %w", comment.ID, err) return fmt.Errorf("deleteComment [comment_id: %d]: %w", comment.ID, err)
} }
cleanup = cleanup.Append(cleanupDeleteComment)
} }
if deleteAttachments { if deleteAttachments {
// Remove issue attachment files. // delete issue attachments
for i := range issue.Attachments { _, err := db.GetEngine(ctx).Where("issue_id = ? AND comment_id = 0", issue.ID).NoAutoCondition().Delete(&issues_model.Issue{})
system_model.RemoveStorageWithNotice(ctx, storage.Attachments, "Delete issue attachment", issue.Attachments[i].RelativePath()) if err != nil {
return err
} }
// the storage cleanup function to remove attachments could be called after all transactions are committed
cleanup = cleanup.Append(func() {
// Remove issue attachment files.
for i := range issue.Attachments {
system_model.RemoveStorageWithNotice(ctx, storage.Attachments, "Delete issue attachment", issue.Attachments[i].RelativePath())
}
})
} }
return nil return nil
}) }); err != nil {
return nil, err
}
return cleanup, nil
} }
// DeleteOrphanedIssues delete issues without a repo // DeleteOrphanedIssues delete issues without a repo
@ -361,9 +377,11 @@ func DeleteIssuesByRepoID(ctx context.Context, repoID int64, deleteAttachments b
} }
for _, issue := range issues { for _, issue := range issues {
if err := deleteIssue(ctx, issue, deleteAttachments); err != nil { cleanup, err := deleteIssue(ctx, issue, deleteAttachments)
if err != nil {
return fmt.Errorf("deleteIssue [issue_id: %d]: %w", issue.ID, err) return fmt.Errorf("deleteIssue [issue_id: %d]: %w", issue.ID, err)
} }
cleanup()
} }
} }

View File

@ -44,8 +44,9 @@ func TestIssue_DeleteIssue(t *testing.T) {
ID: issueIDs[2], ID: issueIDs[2],
} }
err = deleteIssue(db.DefaultContext, issue, true) cleanup, err := deleteIssue(db.DefaultContext, issue, true)
assert.NoError(t, err) assert.NoError(t, err)
cleanup()
issueIDs, err = issues_model.GetIssueIDsByRepoID(db.DefaultContext, 1) issueIDs, err = issues_model.GetIssueIDsByRepoID(db.DefaultContext, 1)
assert.NoError(t, err) assert.NoError(t, err)
assert.Len(t, issueIDs, 4) assert.Len(t, issueIDs, 4)
@ -55,8 +56,9 @@ func TestIssue_DeleteIssue(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
issue, err = issues_model.GetIssueByID(db.DefaultContext, 4) issue, err = issues_model.GetIssueByID(db.DefaultContext, 4)
assert.NoError(t, err) assert.NoError(t, err)
err = deleteIssue(db.DefaultContext, issue, true) cleanup, err = deleteIssue(db.DefaultContext, issue, true)
assert.NoError(t, err) assert.NoError(t, err)
cleanup()
assert.Len(t, attachments, 2) assert.Len(t, attachments, 2)
for i := range attachments { for i := range attachments {
attachment, err := repo_model.GetAttachmentByUUID(db.DefaultContext, attachments[i].UUID) attachment, err := repo_model.GetAttachmentByUUID(db.DefaultContext, attachments[i].UUID)
@ -78,8 +80,9 @@ func TestIssue_DeleteIssue(t *testing.T) {
assert.NoError(t, err) assert.NoError(t, err)
assert.False(t, left) assert.False(t, left)
err = deleteIssue(db.DefaultContext, issue2, true) cleanup, err = deleteIssue(db.DefaultContext, issue2, true)
assert.NoError(t, err) assert.NoError(t, err)
cleanup()
left, err = issues_model.IssueNoDependenciesLeft(db.DefaultContext, issue1) left, err = issues_model.IssueNoDependenciesLeft(db.DefaultContext, issue1)
assert.NoError(t, err) assert.NoError(t, err)
assert.True(t, left) assert.True(t, left)

View File

@ -22,7 +22,6 @@ import (
"code.gitea.io/gitea/modules/storage" "code.gitea.io/gitea/modules/storage"
"code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/timeutil"
"code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/util"
attachment_service "code.gitea.io/gitea/services/attachment"
notify_service "code.gitea.io/gitea/services/notify" notify_service "code.gitea.io/gitea/services/notify"
) )
@ -302,8 +301,9 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo
deletedUUIDs.Add(attach.UUID) deletedUUIDs.Add(attach.UUID)
} }
if _, err := attachment_service.DeleteAttachments(ctx, attachments); err != nil { _, err = db.GetEngine(ctx).In("uuid", deletedUUIDs.Values()).NoAutoCondition().Delete(&repo_model.Attachment{})
return fmt.Errorf("DeleteAttachments [uuids: %v]: %w", delAttachmentUUIDs, err) if err != nil {
return err
} }
} }
@ -339,7 +339,7 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo
return err return err
} }
for _, uuid := range delAttachmentUUIDs { for _, uuid := range deletedUUIDs.Values() {
if err := storage.Attachments.Delete(repo_model.AttachmentRelativePath(uuid)); err != nil { 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 // 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. // should not return error but only record the error on logs.

View File

@ -115,15 +115,22 @@ func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, repoID
} }
} }
attachments := make([]*repo_model.Attachment, 0, 20) // some attachments have release_id but repo_id = 0
if err = sess.Join("INNER", "`release`", "`release`.id = `attachment`.release_id"). releaseAttachments := make([]*repo_model.Attachment, 0, 20)
if err = db.GetEngine(ctx).Join("INNER", "`release`", "`release`.id = `attachment`.release_id").
Where("`release`.repo_id = ?", repoID). Where("`release`.repo_id = ?", repoID).
Find(&attachments); err != nil { Find(&releaseAttachments); err != nil {
return err return err
} }
releaseAttachments := make([]string, 0, len(attachments)) // Delete attachments with release_id but repo_id = 0
for i := 0; i < len(attachments); i++ { if len(releaseAttachments) > 0 {
releaseAttachments = append(releaseAttachments, attachments[i].RelativePath()) ids := make([]int64, 0, len(releaseAttachments))
for _, attach := range releaseAttachments {
ids = append(ids, attach.ID)
}
if _, err := db.GetEngine(ctx).In("id", ids).Delete(&repo_model.Attachment{}); err != nil {
return fmt.Errorf("delete release attachments failed: %w", 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 { 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 {
@ -267,21 +274,14 @@ func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, repoID
} }
} }
// Get all attachments with both issue_id and release_id are zero // Get all attachments with repo_id = repo.ID. some release attachments have repo_id = 0 should be deleted before
var newAttachments []*repo_model.Attachment var repoAttachments []*repo_model.Attachment
if err := sess.Where(builder.Eq{ if err := sess.Where(builder.Eq{
"repo_id": repo.ID, "repo_id": repo.ID,
"issue_id": 0, }).Find(&repoAttachments); err != nil {
"release_id": 0,
}).Find(&newAttachments); err != nil {
return err 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 { if _, err := sess.Where("repo_id=?", repo.ID).Delete(new(repo_model.Attachment)); err != nil {
return err return err
} }
@ -330,14 +330,13 @@ func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, repoID
system_model.RemoveStorageWithNotice(ctx, storage.LFS, "Delete orphaned LFS file", lfsObj) system_model.RemoveStorageWithNotice(ctx, storage.LFS, "Delete orphaned LFS file", lfsObj)
} }
// Remove release attachment files. // Remove release attachments
for _, releaseAttachment := range releaseAttachments { for _, attachment := range releaseAttachments {
system_model.RemoveStorageWithNotice(ctx, storage.Attachments, "Delete release attachment", releaseAttachment) system_model.RemoveStorageWithNotice(ctx, storage.Attachments, "Delete release attachment", attachment.RelativePath())
} }
// Remove attachment with repo_id = repo.ID.
// Remove attachment with no issue_id and release_id. for _, attachment := range repoAttachments {
for _, newAttachment := range newAttachmentPaths { system_model.RemoveStorageWithNotice(ctx, storage.Attachments, "Delete repo attachment", attachment.RelativePath())
system_model.RemoveStorageWithNotice(ctx, storage.Attachments, "Delete issue attachment", newAttachment)
} }
if len(repo.Avatar) > 0 { if len(repo.Avatar) > 0 {

View File

@ -5,7 +5,9 @@ package user
import ( import (
"context" "context"
"errors"
"fmt" "fmt"
"os"
"time" "time"
_ "image/jpeg" // Needed for jpeg support _ "image/jpeg" // Needed for jpeg support
@ -22,25 +24,27 @@ import (
pull_model "code.gitea.io/gitea/models/pull" pull_model "code.gitea.io/gitea/models/pull"
repo_model "code.gitea.io/gitea/models/repo" repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user" user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
attachment_service "code.gitea.io/gitea/services/attachment" "code.gitea.io/gitea/modules/storage"
"code.gitea.io/gitea/modules/util"
"xorm.io/builder" "xorm.io/builder"
) )
// deleteUser deletes models associated to an user. // deleteUser deletes models associated to an user.
func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error) { func deleteUser(ctx context.Context, u *user_model.User, purge bool) (cleanup util.CleanUpFunc, err error) {
e := db.GetEngine(ctx) cleanup = util.NewCleanUpFunc()
// ***** START: Watch ***** // ***** START: Watch *****
watchedRepoIDs, err := db.FindIDs(ctx, "watch", "watch.repo_id", watchedRepoIDs, err := db.FindIDs(ctx, "watch", "watch.repo_id",
builder.Eq{"watch.user_id": u.ID}. builder.Eq{"watch.user_id": u.ID}.
And(builder.Neq{"watch.mode": repo_model.WatchModeDont})) And(builder.Neq{"watch.mode": repo_model.WatchModeDont}))
if err != nil { 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 { 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 ***** // ***** END: Watch *****
@ -48,9 +52,9 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
starredRepoIDs, err := db.FindIDs(ctx, "star", "star.repo_id", starredRepoIDs, err := db.FindIDs(ctx, "star", "star.repo_id",
builder.Eq{"star.uid": u.ID}) builder.Eq{"star.uid": u.ID})
if err != nil { 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 { } 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 ***** // ***** END: Star *****
@ -58,17 +62,17 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
followeeIDs, err := db.FindIDs(ctx, "follow", "follow.follow_id", followeeIDs, err := db.FindIDs(ctx, "follow", "follow.follow_id",
builder.Eq{"follow.user_id": u.ID}) builder.Eq{"follow.user_id": u.ID})
if err != nil { 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 { } 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", followerIDs, err := db.FindIDs(ctx, "follow", "follow.user_id",
builder.Eq{"follow.follow_id": u.ID}) builder.Eq{"follow.follow_id": u.ID})
if err != nil { 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 { } 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 ***** // ***** END: Follow *****
@ -97,11 +101,11 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
&user_model.Blocking{BlockeeID: u.ID}, &user_model.Blocking{BlockeeID: u.ID},
&actions_model.ActionRunnerToken{OwnerID: u.ID}, &actions_model.ActionRunnerToken{OwnerID: u.ID},
); err != nil { ); 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 { if err := auth_model.DeleteOAuth2RelictsByUserID(ctx, u.ID); err != nil {
return err return nil, fmt.Errorf("deleteOAuth2RelictsByUserID: %w", err)
} }
if purge || (setting.Service.UserDeleteWithCommentsMaxTime != 0 && if purge || (setting.Service.UserDeleteWithCommentsMaxTime != 0 &&
@ -110,8 +114,8 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
const batchSize = 50 const batchSize = 50
for { for {
comments := make([]*issues_model.Comment, 0, batchSize) 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 return nil, err
} }
if len(comments) == 0 { if len(comments) == 0 {
break break
@ -120,23 +124,44 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
for _, comment := range comments { for _, comment := range comments {
// Delete attachments of the comments // Delete attachments of the comments
if err := comment.LoadAttachments(ctx); err != nil { if err := comment.LoadAttachments(ctx); err != nil {
return err return nil, err
} }
if _, err = issues_model.DeleteComment(ctx, comment); err != nil { if _, err = issues_model.DeleteComment(ctx, comment); err != nil {
return err return nil, err
} }
// delete comment attachments ids := make([]int64, 0, len(comment.Attachments))
if _, err := attachment_service.DeleteAttachments(ctx, comment.Attachments); err != nil { for _, a := range comment.Attachments {
return fmt.Errorf("delete attachments: %w", err) ids = append(ids, a.ID)
} }
_, err := db.GetEngine(ctx).In("id", ids).NoAutoCondition().Delete(&repo_model.Attachment{})
if err != nil {
return nil, err
}
cleanup = cleanup.Append(func() {
for _, a := range comment.Attachments {
if err := storage.Attachments.Delete(a.RelativePath()); err != nil {
if !errors.Is(err, os.ErrNotExist) {
// 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", a.UUID, err)
} else {
log.Warn("Attachment file not found when deleting: %s", a.RelativePath())
}
}
}
})
} }
} }
// Delete Reactions // Delete Reactions
if err = issues_model.DeleteReaction(ctx, &issues_model.ReactionOptions{DoerID: u.ID}); err != nil { if err = issues_model.DeleteReaction(ctx, &issues_model.ReactionOptions{DoerID: u.ID}); err != nil {
return err return nil, err
} }
} }
@ -150,15 +175,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()`). // 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, // Also, as we didn't update branch protections when removing entries from `access` table,
// it's safer to iterate all protected branches. // 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) return nil, fmt.Errorf("findProtectedBranches: %w", err)
} }
if len(protections) == 0 { if len(protections) == 0 {
break break
} }
for _, p := range protections { for _, p := range protections {
if err := git_model.RemoveUserIDFromProtectedBranch(ctx, p, u.ID); err != nil { if err := git_model.RemoveUserIDFromProtectedBranch(ctx, p, u.ID); err != nil {
return err return nil, err
} }
} }
} }
@ -167,7 +192,7 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
// ***** START: PublicKey ***** // ***** START: PublicKey *****
if _, err = db.DeleteByBean(ctx, &asymkey_model.PublicKey{OwnerID: u.ID}); err != nil { 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 ***** // ***** END: PublicKey *****
@ -176,37 +201,37 @@ func deleteUser(ctx context.Context, u *user_model.User, purge bool) (err error)
OwnerID: u.ID, OwnerID: u.ID,
}) })
if err != nil { if err != nil {
return fmt.Errorf("ListGPGKeys: %w", err) return nil, fmt.Errorf("ListGPGKeys: %w", err)
} }
// Delete GPGKeyImport(s). // Delete GPGKeyImport(s).
for _, key := range keys { for _, key := range keys {
if _, err = db.DeleteByBean(ctx, &asymkey_model.GPGKeyImport{KeyID: key.KeyID}); err != nil { 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 { 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 ***** // ***** END: GPGPublicKey *****
// Clear assignee. // Clear assignee.
if _, err = db.DeleteByBean(ctx, &issues_model.IssueAssignees{AssigneeID: u.ID}); err != nil { 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 ***** // ***** START: ExternalLoginUser *****
if err = user_model.RemoveAllAccountLinks(ctx, u); err != nil { if err = user_model.RemoveAllAccountLinks(ctx, u); err != nil {
return fmt.Errorf("ExternalLoginUser: %w", err) return nil, fmt.Errorf("ExternalLoginUser: %w", err)
} }
// ***** END: ExternalLoginUser ***** // ***** END: ExternalLoginUser *****
if err := auth_model.DeleteAuthTokensByUserID(ctx, u.ID); err != nil { 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 { 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 cleanup, nil
} }

View File

@ -243,7 +243,8 @@ func DeleteUser(ctx context.Context, doer, u *user_model.User, purge bool) error
return packages_model.ErrUserOwnPackages{UID: u.ID} return packages_model.ErrUserOwnPackages{UID: u.ID}
} }
if err := deleteUser(ctx, u, purge); err != nil { cleanup, err := deleteUser(ctx, u, purge)
if err != nil {
return fmt.Errorf("DeleteUser: %w", err) return fmt.Errorf("DeleteUser: %w", err)
} }
@ -252,6 +253,8 @@ func DeleteUser(ctx context.Context, doer, u *user_model.User, purge bool) error
} }
_ = committer.Close() _ = committer.Close()
cleanup()
if err = asymkey_service.RewriteAllPublicKeys(ctx); err != nil { if err = asymkey_service.RewriteAllPublicKeys(ctx); err != nil {
return err return err
} }