improvements

lunny · lunny · commit 488e6581aff7 · 2025-07-18T18:09:03.000-07:00
diff --git 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
@@ -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
@@ -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
@@ -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,32 +311,29 @@ 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...)
 		}
 
 		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
@@ -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
@@ -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
@@ -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
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-// Copyright 2024 The Gitea Authors. All rights reserved.`
	`1`	`+// Copyright 2025 The Gitea Authors. All rights reserved.`
`2`	`2`	`// SPDX-License-Identifier: MIT`
`3`	`3`
`4`	`4`	`package v1_25`
Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,7 @@ func DeleteAttachments(ctx context.Context, attachments []*repo_model.Attachment`
`79`	`79`	`return 0, err`
`80`	`80`	`}`
`81`	`81`
`82`		`- CleanAttachments(ctx, attachments)`
	`82`	`+ AddAttachmentsToCleanQueue(ctx, attachments)`
`83`	`83`
`84`	`84`	`return int(cnt), nil`
`85`	`85`	`}`
`@@ -94,8 +94,8 @@ func Init() error {`
`94`	`94`	`return nil`
`95`	`95`	`}`
`96`	`96`
`97`		`-// CleanAttachments adds the attachments to the clean queue for deletion.`
`98`		`-func CleanAttachments(ctx context.Context, attachments []*repo_model.Attachment) {`
	`97`	`+// AddAttachmentsToCleanQueue adds the attachments to the clean queue for deletion.`
	`98`	`+func AddAttachmentsToCleanQueue(ctx context.Context, attachments []*repo_model.Attachment) {`
`99`	`99`	`for _, a := range attachments {`
`100`	`100`	`if err := cleanQueue.Push(a.ID); err != nil {`
`101`	`101`	`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 {`
`164`	`164`	`log.Trace("No more attachments to be deleted")`
`165`	`165`	`break`
`166`	`166`	`}`
`167`		`- CleanAttachments(ctx, attachments)`
	`167`	`+ AddAttachmentsToCleanQueue(ctx, attachments)`
`168`	`168`	`lastID = attachments[len(attachments)-1].ID`
`169`	`169`	`}`
`170`	`170`
Original file line number	Diff line number	Diff line change
`@@ -194,7 +194,7 @@ func DeleteIssue(ctx context.Context, doer user_model.User, gitRepo git.Reposi`
`194`	`194`	`return err`
`195`	`195`	`}`
`196`	`196`
`197`		`- attachment_service.CleanAttachments(ctx, toBeCleanedAttachments)`
	`197`	`+ attachment_service.AddAttachmentsToCleanQueue(ctx, toBeCleanedAttachments)`
`198`	`198`
`199`	`199`	`// delete pull request related git data`
`200`	`200`	`if issue.IsPull && gitRepo != nil {`
`@@ -259,36 +259,36 @@ func GetRefEndNamesAndURLs(issues []*issues_model.Issue, repoLink string) (map[i`
`259`	`259`
`260`	`260`	`// deleteIssue deletes the issue`
`261`	`261`	`func deleteIssue(ctx context.Context, issue issues_model.Issue, deleteAttachments bool) ([]repo_model.Attachment, error) {`
`262`		`- toBeCleanedAttachments := make([]*repo_model.Attachment, 0)`
`263`		`- if err := db.WithTx(ctx, func(ctx context.Context) error {`
	`262`	`+ return db.WithTx2(ctx, func(ctx context.Context) ([]*repo_model.Attachment, error) {`
	`263`	`+ toBeCleanedAttachments := make([]*repo_model.Attachment, 0)`
`264`	`264`	`if _, err := db.GetEngine(ctx).ID(issue.ID).NoAutoCondition().Delete(issue); err != nil {`
`265`		`- return err`
	`265`	`+ return nil, err`
`266`	`266`	`}`
`267`	`267`
`268`	`268`	`// update the total issue numbers`
`269`	`269`	`if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil {`
`270`		`- return err`
	`270`	`+ return nil, err`
`271`	`271`	`}`
`272`	`272`	`// if the issue is closed, update the closed issue numbers`
`273`	`273`	`if issue.IsClosed {`
`274`	`274`	`if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil {`
`275`		`- return err`
	`275`	`+ return nil, err`
`276`	`276`	`}`
`277`	`277`	`}`
`278`	`278`
`279`	`279`	`if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil {`
`280`		`- return fmt.Errorf("error updating counters for milestone id %d: %w",`
	`280`	`+ return nil, fmt.Errorf("error updating counters for milestone id %d: %w",`
`281`	`281`	`issue.MilestoneID, err)`
`282`	`282`	`}`
`283`	`283`
`284`	`284`	`if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID, issue.Index); err != nil {`
`285`		`- return err`
	`285`	`+ return nil, err`
`286`	`286`	`}`
`287`	`287`
`288`	`288`	`if deleteAttachments {`
`289`	`289`	`// find attachments related to this issue and remove them`
`290`	`290`	`if err := issue.LoadAttachments(ctx); err != nil {`
`291`		`- return err`
	`291`	`+ return nil, err`
`292`	`292`	`}`
`293`	`293`	`}`
`294`	`294`
`@@ -311,32 +311,29 @@ func deleteIssue(ctx context.Context, issue *issues_model.Issue, deleteAttachmen`
`311`	`311`	`&issues_model.Comment{DependentIssueID: issue.ID},`
`312`	`312`	`&issues_model.IssuePin{IssueID: issue.ID},`
`313`	`313`	`); err != nil {`
`314`		`- return err`
	`314`	`+ return nil, err`
`315`	`315`	`}`
`316`	`316`
`317`	`317`	`for _, comment := range issue.Comments {`
`318`	`318`	`_, err := deleteComment(ctx, comment, deleteAttachments)`
`319`	`319`	`if err != nil {`
`320`		`- return fmt.Errorf("deleteComment [comment_id: %d]: %w", comment.ID, err)`
	`320`	`+ return nil, fmt.Errorf("deleteComment [comment_id: %d]: %w", comment.ID, err)`
`321`	`321`	`}`
`322`	`322`	`toBeCleanedAttachments = append(toBeCleanedAttachments, comment.Attachments...)`
`323`	`323`	`}`
`324`	`324`
`325`	`325`	`if deleteAttachments {`
`326`	`326`	`// delete issue attachments`
`327`	`327`	`if err := issue.LoadAttachments(ctx); err != nil {`
`328`		`- return err`
	`328`	`+ return nil, err`
`329`	`329`	`}`
`330`	`330`	`if _, err := repo_model.MarkAttachmentsDeleted(ctx, issue.Attachments); err != nil {`
`331`		`- return err`
	`331`	`+ return nil, err`
`332`	`332`	`}`
`333`	`333`	`toBeCleanedAttachments = append(toBeCleanedAttachments, issue.Attachments...)`
`334`	`334`	`}`
`335`		`- return nil`
`336`		`- }); err != nil {`
`337`		`- return nil, err`
`338`		`- }`
`339`		`- return toBeCleanedAttachments, nil`
	`335`	`+ return toBeCleanedAttachments, nil`
	`336`	`+ })`
`340`	`337`	`}`
`341`	`338`
`342`	`339`	`// DeleteOrphanedIssues delete issues without a repo`
`@@ -358,7 +355,7 @@ func DeleteOrphanedIssues(ctx context.Context) error {`
`358`	`355`	`}); err != nil {`
`359`	`356`	`return err`
`360`	`357`	`}`
`361`		`- attachment_service.CleanAttachments(ctx, toBeCleanedAttachments)`
	`358`	`+ attachment_service.AddAttachmentsToCleanQueue(ctx, toBeCleanedAttachments)`
`362`	`359`	`return nil`
`363`	`360`	`}`
`364`	`361`
Original file line number	Diff line number	Diff line change
`@@ -341,7 +341,7 @@ func UpdateRelease(ctx context.Context, doer user_model.User, gitRepo git.Repo`
`341`	`341`	`return err`
`342`	`342`	`}`
`343`	`343`
`344`		`- attachment_service.CleanAttachments(ctx, deletedAttachments)`
	`344`	`+ attachment_service.AddAttachmentsToCleanQueue(ctx, deletedAttachments)`
`345`	`345`
`346`	`346`	`if !rel.IsDraft {`
`347`	`347`	`if !isTagCreated && !isConvertedFromTag {`
`@@ -412,7 +412,7 @@ func DeleteReleaseByID(ctx context.Context, repo repo_model.Repository, rel re`
`412`	`412`	`return err`
`413`	`413`	`}`
`414`	`414`
`415`		`- attachment_service.CleanAttachments(ctx, rel.Attachments)`
	`415`	`+ attachment_service.AddAttachmentsToCleanQueue(ctx, rel.Attachments)`
`416`	`416`
`417`	`417`	`if !rel.IsDraft {`
`418`	`418`	`notify_service.DeleteRelease(ctx, doer, rel)`
Original file line number	Diff line number	Diff line change
`@@ -254,7 +254,7 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error {`
`254`	`254`	`}`
`255`	`255`	`_ = committer.Close()`
`256`	`256`
`257`		`- attachment_service.CleanAttachments(ctx, toBeCleanedAttachments)`
	`257`	`+ attachment_service.AddAttachmentsToCleanQueue(ctx, toBeCleanedAttachments)`
`258`	`258`
`259`	`259`	`if err = asymkey_service.RewriteAllPublicKeys(ctx); err != nil {`
`260`	`260`	`return err`