Use cron only for deleting the files need to be cleanup

Author: lunny

models/repo/attachment.go

@@ -166,37 +166,34 @@ func GetAttachmentByReleaseIDFileName(ctx context.Context, releaseID int64, file
 }
 
 // DeleteAttachments delete the given attachments and add disk files to pending deletion
-func DeleteAttachments(ctx context.Context, attachments []*Attachment) ([]int64, error) {
+func DeleteAttachments(ctx context.Context, attachments []*Attachment) error {
 	if len(attachments) == 0 {
-		return nil, nil
+		return nil
 	}
 
 	ids := make([]int64, 0, len(attachments))
 	for _, a := range attachments {
 		ids = append(ids, a.ID)
 	}
 
-	return db.WithTx2(ctx, func(ctx context.Context) ([]int64, error) {
+	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 nil, err
+			return err
 		}
 
 		// add disk files to pending deletion table as well
-		var deletionIDs []int64
 		for _, a := range attachments {
 			pendingDeletion := &system_model.StoragePathDeletion{
 				StorageName:  storage.AttachmentStorageName,
 				PathType:     system_model.PathFile,
 				RelativePath: a.RelativePath(),
 			}
 			if err := db.Insert(ctx, pendingDeletion); err != nil {
-				return nil, fmt.Errorf("insert pending deletion: %w", err)
+				return fmt.Errorf("insert pending deletion: %w", err)
 			}
-
-			deletionIDs = append(deletionIDs, pendingDeletion.ID) // Collect pending deletions
 		}
-		return deletionIDs, nil
+		return nil
 	})
 }
 

models/user/main_test.go

@@ -7,8 +7,6 @@ import (
 	"testing"
 
 	"code.gitea.io/gitea/models/unittest"
-	"code.gitea.io/gitea/modules/setting"
-	"code.gitea.io/gitea/services/storagecleanup"
 
 	_ "code.gitea.io/gitea/models"
 	_ "code.gitea.io/gitea/models/actions"
@@ -17,10 +15,5 @@ import (
 )
 
 func TestMain(m *testing.M) {
-	unittest.MainTest(m, &unittest.TestOptions{
-		SetUp: func() error {
-			setting.LoadQueueSettings()
-			return storagecleanup.Init()
-		},
-	})
+	unittest.MainTest(m)
 }

routers/init.go

@@ -52,7 +52,6 @@ import (
 	release_service "code.gitea.io/gitea/services/release"
 	repo_service "code.gitea.io/gitea/services/repository"
 	"code.gitea.io/gitea/services/repository/archiver"
-	"code.gitea.io/gitea/services/storagecleanup"
 	"code.gitea.io/gitea/services/task"
 	"code.gitea.io/gitea/services/uinotification"
 	"code.gitea.io/gitea/services/webhook"
@@ -175,7 +174,6 @@ func InitWebInstalled(ctx context.Context) {
 	mustInitCtx(ctx, actions_service.Init)
 
 	mustInit(repo_service.InitLicenseClassifier)
-	mustInit(storagecleanup.Init)
 
 	// Finally start up the cron
 	cron.NewContext(ctx)

services/attachment/attachment.go

@@ -14,7 +14,6 @@ import (
 	"code.gitea.io/gitea/modules/storage"
 	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/services/context/upload"
-	"code.gitea.io/gitea/services/storagecleanup"
 
 	"github.com/google/uuid"
 )
@@ -69,12 +68,10 @@ func DeleteAttachment(ctx context.Context, a *repo_model.Attachment) error {
 
 // DeleteAttachments deletes the given attachments and optionally the associated files.
 func DeleteAttachments(ctx context.Context, attachments []*repo_model.Attachment) (int, error) {
-	deletions, err := repo_model.DeleteAttachments(ctx, attachments)
+	err := repo_model.DeleteAttachments(ctx, attachments)
 	if err != nil {
 		return 0, err
 	}
 
-	storagecleanup.AddDeletionsToCleanQueue(ctx, deletions)
-
-	return len(deletions), nil
+	return len(attachments), nil
 }

services/attachment/attachment_test.go

@@ -14,22 +14,15 @@ import (
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
-	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/storage"
-	"code.gitea.io/gitea/services/storagecleanup"
 
 	_ "code.gitea.io/gitea/models/actions"
 
 	"github.com/stretchr/testify/assert"
 )
 
 func TestMain(m *testing.M) {
-	unittest.MainTest(m, &unittest.TestOptions{
-		SetUp: func() error {
-			setting.LoadQueueSettings()
-			return storagecleanup.Init()
-		},
-	})
+	unittest.MainTest(m)
 }
 
 func TestUploadAttachment(t *testing.T) {

services/cron/tasks_extended.go

@@ -17,7 +17,7 @@ import (
 	asymkey_service "code.gitea.io/gitea/services/asymkey"
 	repo_service "code.gitea.io/gitea/services/repository"
 	archiver_service "code.gitea.io/gitea/services/repository/archiver"
-	"code.gitea.io/gitea/services/storagecleanup"
+	storage_service "code.gitea.io/gitea/services/storage"
 	user_service "code.gitea.io/gitea/services/user"
 )
 
@@ -228,9 +228,9 @@ func registerCleanStorage() {
 	RegisterTaskFatal("cleanup_storage", &BaseConfig{
 		Enabled:    false,
 		RunAtStart: false,
-		Schedule:   "@every 24h",
+		Schedule:   "@every 1m", // Run every minute to check for deletions
 	}, func(ctx context.Context, _ *user_model.User, _ Config) error {
-		return storagecleanup.ScanToBeDeletedFilesOrDir(ctx)
+		return storage_service.ScanToBeDeletedFilesOrDir(ctx)
 	})
 }
 

services/issue/comments.go

@@ -18,7 +18,6 @@ import (
 	"code.gitea.io/gitea/modules/timeutil"
 	git_service "code.gitea.io/gitea/services/git"
 	notify_service "code.gitea.io/gitea/services/notify"
-	"code.gitea.io/gitea/services/storagecleanup"
 )
 
 // CreateRefComment creates a commit reference comment to issue.
@@ -132,34 +131,31 @@ 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) ([]int64, error) {
-	return db.WithTx2(ctx, func(ctx context.Context) ([]int64, error) {
+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 nil, err
+				return err
 			}
 		}
 
 		if err := issues_model.DeleteComment(ctx, comment); err != nil {
-			return nil, err
+			return err
 		}
 
 		if removeAttachments {
 			return repo_model.DeleteAttachments(ctx, comment.Attachments)
 		}
-		return nil, nil
+		return nil
 	})
 }
 
 func DeleteComment(ctx context.Context, doer *user_model.User, comment *issues_model.Comment) error {
-	deletions, err := deleteComment(ctx, comment, true)
-	if err != nil {
+	if err := deleteComment(ctx, comment, true); err != nil {
 		return err
 	}
 
-	storagecleanup.AddDeletionsToCleanQueue(ctx, deletions)
-
 	notify_service.DeleteComment(ctx, doer, comment)
 
 	return nil

services/issue/issue.go

@@ -18,7 +18,6 @@ import (
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
 	notify_service "code.gitea.io/gitea/services/notify"
-	"code.gitea.io/gitea/services/storagecleanup"
 )
 
 // NewIssue creates new issue with labels for repository.
@@ -189,13 +188,10 @@ func DeleteIssue(ctx context.Context, doer *user_model.User, gitRepo *git.Reposi
 	}
 
 	// delete entries in database
-	toBeCleanedDeletions, err := deleteIssue(ctx, issue, true)
-	if err != nil {
+	if err := deleteIssue(ctx, issue, true); err != nil {
 		return err
 	}
 
-	storagecleanup.AddDeletionsToCleanQueue(ctx, toBeCleanedDeletions)
-
 	// delete pull request related git data
 	if issue.IsPull && gitRepo != nil {
 		if err := gitRepo.RemoveReference(issue.PullRequest.GetGitHeadRefName()); err != nil {
@@ -258,37 +254,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) ([]int64, error) {
-	return db.WithTx2(ctx, func(ctx context.Context) ([]int64, error) {
-		toBeCleanedDeletions := make([]int64, 0)
+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 nil, err
+			return err
 		}
 
 		// update the total issue numbers
 		if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil {
-			return nil, err
+			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 nil, err
+				return err
 			}
 		}
 
 		if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil {
-			return nil, fmt.Errorf("error updating counters for milestone id %d: %w",
+			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
+			return err
 		}
 
 		if deleteAttachments {
 			// find attachments related to this issue and remove them
 			if err := issue.LoadAttachments(ctx); err != nil {
-				return nil, err
+				return err
 			}
 		}
 
@@ -311,82 +306,66 @@ 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 nil, err
+			return err
 		}
 
 		for _, comment := range issue.Comments {
-			deletions, err := deleteComment(ctx, comment, deleteAttachments)
-			if err != nil {
-				return nil, fmt.Errorf("deleteComment [comment_id: %d]: %w", comment.ID, err)
-			}
-			if deleteAttachments {
-				toBeCleanedDeletions = append(toBeCleanedDeletions, deletions...)
+			if err := deleteComment(ctx, comment, deleteAttachments); err != nil {
+				return fmt.Errorf("deleteComment [comment_id: %d]: %w", comment.ID, err)
 			}
 		}
 
 		if deleteAttachments {
 			// delete issue attachments
 			if err := issue.LoadAttachments(ctx); err != nil {
-				return nil, err
+				return err
 			}
-			deletions, err := repo_model.DeleteAttachments(ctx, issue.Attachments)
-			if err != nil {
-				return nil, err
+			if err := repo_model.DeleteAttachments(ctx, issue.Attachments); err != nil {
+				return err
 			}
-			toBeCleanedDeletions = append(toBeCleanedDeletions, deletions...)
 		}
-		return toBeCleanedDeletions, nil
+		return nil
 	})
 }
 
 // DeleteOrphanedIssues delete issues without a repo
 func DeleteOrphanedIssues(ctx context.Context) error {
-	toBeCleanedDeletions := make([]int64, 0)
-	if 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 {
-			deletions, err := DeleteIssuesByRepoID(ctx, repoIDs[i], true)
-			if err != nil {
+			if err := DeleteIssuesByRepoID(ctx, repoIDs[i], true); err != nil {
 				return err
 			}
-			toBeCleanedDeletions = append(toBeCleanedDeletions, deletions...)
 		}
 		return nil
-	}); err != nil {
-		return err
-	}
-	storagecleanup.AddDeletionsToCleanQueue(ctx, toBeCleanedDeletions)
-	return nil
+	})
 }
 
 // DeleteIssuesByRepoID deletes issues by repositories id
-func DeleteIssuesByRepoID(ctx context.Context, repoID int64, deleteAttachments bool) ([]int64, error) {
-	toBeCleanedDeletions := make([]int64, 0)
+func DeleteIssuesByRepoID(ctx context.Context, repoID int64, deleteAttachments bool) error {
 	for {
 		issues := make([]*issues_model.Issue, 0, db.DefaultMaxInSize)
 		if err := db.GetEngine(ctx).
 			Where("repo_id = ?", repoID).
 			OrderBy("id").
 			Limit(db.DefaultMaxInSize).
 			Find(&issues); err != nil {
-			return nil, err
+			return err
 		}
 
 		if len(issues) == 0 {
 			break
 		}
 
 		for _, issue := range issues {
-			deletions, err := deleteIssue(ctx, issue, deleteAttachments)
-			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)
 			}
-			toBeCleanedDeletions = append(toBeCleanedDeletions, deletions...)
 		}
 	}
 
-	return toBeCleanedDeletions, nil
+	return nil
 }