From 488e6581aff7656cd2ea25a03172793f1fa827af Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 18 Jul 2025 18:09:03 -0700 Subject: [PATCH] improvements --- models/migrations/v1_25/v321.go | 2 +- services/attachment/attachment.go | 8 +++---- services/issue/comments.go | 8 ++----- services/issue/issue.go | 35 ++++++++++++++----------------- services/issue/issue_test.go | 6 +++--- services/release/release.go | 4 ++-- services/repository/delete.go | 4 ++-- services/user/user.go | 2 +- 8 files changed, 31 insertions(+), 38 deletions(-) diff --git a/models/migrations/v1_25/v321.go b/models/migrations/v1_25/v321.go index 3d8e8f7e3fe..174e6c40c91 100644 --- a/models/migrations/v1_25/v321.go +++ b/models/migrations/v1_25/v321.go @@ -1,4 +1,4 @@ -// Copyright 2024 The Gitea Authors. All rights reserved. +// Copyright 2025 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT package v1_25 diff --git a/services/attachment/attachment.go b/services/attachment/attachment.go index 18e3d384448..8a214ea2f53 100644 --- a/services/attachment/attachment.go +++ b/services/attachment/attachment.go @@ -79,7 +79,7 @@ func DeleteAttachments(ctx context.Context, attachments []*repo_model.Attachment return 0, err } - CleanAttachments(ctx, attachments) + AddAttachmentsToCleanQueue(ctx, attachments) return int(cnt), nil } @@ -94,8 +94,8 @@ func Init() error { return nil } -// CleanAttachments adds the attachments to the clean queue for deletion. -func CleanAttachments(ctx context.Context, attachments []*repo_model.Attachment) { +// AddAttachmentsToCleanQueue adds the attachments to the clean queue for deletion. +func AddAttachmentsToCleanQueue(ctx context.Context, attachments []*repo_model.Attachment) { for _, a := range attachments { if err := cleanQueue.Push(a.ID); err != nil { log.Error("Failed to push attachment ID %d to clean queue: %v", a.ID, err) @@ -164,7 +164,7 @@ func ScanTobeDeletedAttachments(ctx context.Context) error { log.Trace("No more attachments to be deleted") break } - CleanAttachments(ctx, attachments) + AddAttachmentsToCleanQueue(ctx, attachments) lastID = attachments[len(attachments)-1].ID } diff --git a/services/issue/comments.go b/services/issue/comments.go index a2c5e28d234..3b6a7af2cd6 100644 --- a/services/issue/comments.go +++ b/services/issue/comments.go @@ -133,7 +133,7 @@ func UpdateComment(ctx context.Context, c *issues_model.Comment, contentVersion // deleteComment deletes the comment func deleteComment(ctx context.Context, comment *issues_model.Comment, removeAttachments bool) (*issues_model.Comment, error) { - deletedReviewComment, err := db.WithTx2(ctx, func(ctx context.Context) (*issues_model.Comment, error) { + return db.WithTx2(ctx, func(ctx context.Context) (*issues_model.Comment, error) { if removeAttachments { // load attachments before deleting the comment if err := comment.LoadAttachments(ctx); err != nil { @@ -154,10 +154,6 @@ func deleteComment(ctx context.Context, comment *issues_model.Comment, removeAtt } return deletedReviewComment, nil }) - if err != nil { - return nil, err - } - return deletedReviewComment, nil } func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) (*issues_model.Comment, error) { @@ -166,7 +162,7 @@ func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_m return nil, err } - attachment.CleanAttachments(ctx, comment.Attachments) + attachment.AddAttachmentsToCleanQueue(ctx, comment.Attachments) notify_service.DeleteComment(ctx, doer, comment) diff --git a/services/issue/issue.go b/services/issue/issue.go index 54f5fd9e1a4..fe369fe1e68 100644 --- a/services/issue/issue.go +++ b/services/issue/issue.go @@ -194,7 +194,7 @@ func DeleteIssue(ctx context.Context, doer *user_model.User, gitRepo *git.Reposi return err } - attachment_service.CleanAttachments(ctx, toBeCleanedAttachments) + attachment_service.AddAttachmentsToCleanQueue(ctx, toBeCleanedAttachments) // delete pull request related git data if issue.IsPull && gitRepo != nil { @@ -259,36 +259,36 @@ func GetRefEndNamesAndURLs(issues []*issues_model.Issue, repoLink string) (map[i // deleteIssue deletes the issue func deleteIssue(ctx context.Context, issue *issues_model.Issue, deleteAttachments bool) ([]*repo_model.Attachment, error) { - toBeCleanedAttachments := make([]*repo_model.Attachment, 0) - if err := db.WithTx(ctx, func(ctx context.Context) error { + return db.WithTx2(ctx, func(ctx context.Context) ([]*repo_model.Attachment, error) { + toBeCleanedAttachments := make([]*repo_model.Attachment, 0) if _, err := db.GetEngine(ctx).ID(issue.ID).NoAutoCondition().Delete(issue); err != nil { - return err + return nil, err } // update the total issue numbers if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil { - return err + 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 err + return nil, err } } if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil { - return fmt.Errorf("error updating counters for milestone id %d: %w", + return nil, 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 err + return nil, err } if deleteAttachments { // find attachments related to this issue and remove them if err := issue.LoadAttachments(ctx); err != nil { - return err + return nil, err } } @@ -311,13 +311,13 @@ func deleteIssue(ctx context.Context, issue *issues_model.Issue, deleteAttachmen &issues_model.Comment{DependentIssueID: issue.ID}, &issues_model.IssuePin{IssueID: issue.ID}, ); err != nil { - return err + return nil, err } for _, comment := range issue.Comments { _, err := deleteComment(ctx, comment, deleteAttachments) if err != nil { - return fmt.Errorf("deleteComment [comment_id: %d]: %w", comment.ID, err) + return nil, fmt.Errorf("deleteComment [comment_id: %d]: %w", comment.ID, err) } toBeCleanedAttachments = append(toBeCleanedAttachments, comment.Attachments...) } @@ -325,18 +325,15 @@ func deleteIssue(ctx context.Context, issue *issues_model.Issue, deleteAttachmen if deleteAttachments { // delete issue attachments if err := issue.LoadAttachments(ctx); err != nil { - return err + return nil, err } if _, err := repo_model.MarkAttachmentsDeleted(ctx, issue.Attachments); err != nil { - return err + return nil, err } toBeCleanedAttachments = append(toBeCleanedAttachments, issue.Attachments...) } - return nil - }); err != nil { - return nil, err - } - return toBeCleanedAttachments, nil + return toBeCleanedAttachments, nil + }) } // DeleteOrphanedIssues delete issues without a repo @@ -358,7 +355,7 @@ func DeleteOrphanedIssues(ctx context.Context) error { }); err != nil { return err } - attachment_service.CleanAttachments(ctx, toBeCleanedAttachments) + attachment_service.AddAttachmentsToCleanQueue(ctx, toBeCleanedAttachments) return nil } diff --git a/services/issue/issue_test.go b/services/issue/issue_test.go index ec15c0a5e01..5bf8426d7b9 100644 --- a/services/issue/issue_test.go +++ b/services/issue/issue_test.go @@ -47,7 +47,7 @@ func TestIssue_DeleteIssue(t *testing.T) { toBeCleanedAttachments, err := deleteIssue(db.DefaultContext, issue, true) assert.NoError(t, err) - attachment_service.CleanAttachments(db.DefaultContext, toBeCleanedAttachments) + attachment_service.AddAttachmentsToCleanQueue(db.DefaultContext, toBeCleanedAttachments) issueIDs, err = issues_model.GetIssueIDsByRepoID(db.DefaultContext, 1) assert.NoError(t, err) assert.Len(t, issueIDs, 4) @@ -59,7 +59,7 @@ func TestIssue_DeleteIssue(t *testing.T) { assert.NoError(t, err) toBeCleanedAttachments, err = deleteIssue(db.DefaultContext, issue, true) assert.NoError(t, err) - attachment_service.CleanAttachments(db.DefaultContext, toBeCleanedAttachments) + attachment_service.AddAttachmentsToCleanQueue(db.DefaultContext, toBeCleanedAttachments) assert.Len(t, attachments, 2) for i := range attachments { attachment, err := repo_model.GetAttachmentByUUID(db.DefaultContext, attachments[i].UUID) @@ -83,7 +83,7 @@ func TestIssue_DeleteIssue(t *testing.T) { toBeCleanedAttachments, err = deleteIssue(db.DefaultContext, issue2, true) assert.NoError(t, err) - attachment_service.CleanAttachments(db.DefaultContext, toBeCleanedAttachments) + attachment_service.AddAttachmentsToCleanQueue(db.DefaultContext, toBeCleanedAttachments) left, err = issues_model.IssueNoDependenciesLeft(db.DefaultContext, issue1) assert.NoError(t, err) assert.True(t, left) diff --git a/services/release/release.go b/services/release/release.go index ae840836d25..417b16ce51e 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -341,7 +341,7 @@ func UpdateRelease(ctx context.Context, doer *user_model.User, gitRepo *git.Repo return err } - attachment_service.CleanAttachments(ctx, deletedAttachments) + attachment_service.AddAttachmentsToCleanQueue(ctx, deletedAttachments) if !rel.IsDraft { if !isTagCreated && !isConvertedFromTag { @@ -412,7 +412,7 @@ func DeleteReleaseByID(ctx context.Context, repo *repo_model.Repository, rel *re return err } - attachment_service.CleanAttachments(ctx, rel.Attachments) + attachment_service.AddAttachmentsToCleanQueue(ctx, rel.Attachments) if !rel.IsDraft { notify_service.DeleteRelease(ctx, doer, rel) diff --git a/services/repository/delete.go b/services/repository/delete.go index 84f7fecbc73..e0669ec9ca5 100644 --- a/services/repository/delete.go +++ b/services/repository/delete.go @@ -291,8 +291,8 @@ func DeleteRepositoryDirectly(ctx context.Context, repoID int64, ignoreOrgTeams committer.Close() - attachment_service.CleanAttachments(ctx, releaseAttachments) - attachment_service.CleanAttachments(ctx, repoAttachments) + attachment_service.AddAttachmentsToCleanQueue(ctx, releaseAttachments) + attachment_service.AddAttachmentsToCleanQueue(ctx, repoAttachments) if needRewriteKeysFile { if err := asymkey_service.RewriteAllPublicKeys(ctx); err != nil { diff --git a/services/user/user.go b/services/user/user.go index 7f43d3aa6d5..ad17732df15 100644 --- a/services/user/user.go +++ b/services/user/user.go @@ -254,7 +254,7 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error { } _ = committer.Close() - attachment_service.CleanAttachments(ctx, toBeCleanedAttachments) + attachment_service.AddAttachmentsToCleanQueue(ctx, toBeCleanedAttachments) if err = asymkey_service.RewriteAllPublicKeys(ctx); err != nil { return err