improvements

models/migrations/v1_25/main_test.go

@@ -0,0 +1,14 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_25
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/migrations/base"
+)
+
+func TestMain(m *testing.M) {
+	base.MainTest(m)
+}

models/migrations/v1_25/v321.go

@@ -1,7 +1,7 @@
 // Copyright 2024 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-package v1_24
+package v1_25
 
 import (
 	"code.gitea.io/gitea/models/db"

models/migrations/v1_25/v321_test.go

@@ -0,0 +1,36 @@
+// Copyright 2025 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_25
+
+import (
+	"testing"
+
+	"code.gitea.io/gitea/models/migrations/base"
+	"code.gitea.io/gitea/modules/timeutil"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_AddFileStatusToAttachment(t *testing.T) {
+	type Attachment struct {
+		ID                int64  `xorm:"pk autoincr"`
+		UUID              string `xorm:"uuid UNIQUE"`
+		RepoID            int64  `xorm:"INDEX"`           // this should not be zero
+		IssueID           int64  `xorm:"INDEX"`           // maybe zero when creating
+		ReleaseID         int64  `xorm:"INDEX"`           // maybe zero when creating
+		UploaderID        int64  `xorm:"INDEX DEFAULT 0"` // Notice: will be zero before this column added
+		CommentID         int64  `xorm:"INDEX"`
+		Name              string
+		DownloadCount     int64              `xorm:"DEFAULT 0"`
+		Size              int64              `xorm:"DEFAULT 0"`
+		CreatedUnix       timeutil.TimeStamp `xorm:"created"`
+		CustomDownloadURL string             `xorm:"-"`
+	}
+
+	// Prepare and load the testing database
+	x, deferable := base.PrepareTestEnv(t, 0, new(Attachment))
+	defer deferable()
+
+	assert.NoError(t, AddFileStatusToAttachment(x))
+}

models/repo/attachment.go

@@ -227,8 +227,8 @@ func MarkAttachmentsDeletedByRelease(ctx context.Context, releaseID int64) error
 	return err
 }
 
-// DeleteAttachmentByID deletes the attachment which has been marked as deleted by given id
+// DeleteMarkedAttachmentByID deletes the attachment which has been marked as deleted by given id
-func DeleteAttachmentByID(ctx context.Context, id int64) error {
+func DeleteMarkedAttachmentByID(ctx context.Context, id int64) error {
 	cnt, err := db.GetEngine(ctx).ID(id).Where("status = ?", db.FileStatusToBeDeleted).Delete(new(Attachment))
 	if err != nil {
 		return fmt.Errorf("delete attachment by id: %w", err)
@@ -239,7 +239,7 @@ func DeleteAttachmentByID(ctx context.Context, id int64) error {
 	return nil
 }
 
-func UpdateAttachmentFailure(ctx context.Context, attachment *Attachment, err error) error {
+func UpdateMarkedAttachmentFailure(ctx context.Context, attachment *Attachment, err error) error {
 	attachment.DeleteFailedCount++
 	_, updateErr := db.GetEngine(ctx).Table("attachment").ID(attachment.ID).Update(map[string]any{
 		"delete_failed_count":       attachment.DeleteFailedCount,

routers/api/v1/repo/issue_attachment.go

@@ -6,7 +6,6 @@ package repo
 import (
 	"net/http"
 
-	"code.gitea.io/gitea/models/db"
 	issues_model "code.gitea.io/gitea/models/issues"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/modules/log"
@@ -361,10 +360,6 @@ func getIssueAttachmentSafeRead(ctx *context.APIContext, issue *issues_model.Iss
 	if !attachmentBelongsToRepoOrIssue(ctx, attachment, issue) {
 		return nil
 	}
-	if attachment.Status != db.FileStatusNormal {
-		ctx.APIErrorNotFound()
-		return nil
-	}
 	return attachment
 }
 

services/attachment/attachment.go

@@ -13,6 +13,7 @@ import (
 
 	"code.gitea.io/gitea/models/db"
 	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/models/system"
 	"code.gitea.io/gitea/modules/graceful"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/queue"
@@ -127,13 +128,16 @@ func cleanAttachments(ctx context.Context, attachmentIDs []int64) []int64 {
 			if !errors.Is(err, os.ErrNotExist) {
 				log.Error("delete attachment[uuid: %s] failed: %v", attachment.UUID, err)
 				failed = append(failed, attachment.ID)
-				if err := repo_model.UpdateAttachmentFailure(ctx, attachment, err); err != nil {
+				if attachment.DeleteFailedCount%3 == 0 {
+					_ = system.CreateNotice(ctx, system.NoticeRepository, fmt.Sprintf("Failed to delete attachment %s (%d times): %v", attachment.RelativePath(), attachment.DeleteFailedCount+1, err))
+				}
+				if err := repo_model.UpdateMarkedAttachmentFailure(ctx, attachment, err); err != nil {
 					log.Error("Failed to update attachment failure for ID %d: %v", attachment.ID, err)
 				}
 				continue
 			}
 		}
-		if err := repo_model.DeleteAttachmentByID(ctx, attachment.ID); err != nil {
+		if err := repo_model.DeleteMarkedAttachmentByID(ctx, attachment.ID); err != nil {
 			log.Error("Failed to delete attachment by ID %d(will be tried later): %v", attachment.ID, err)
 			failed = append(failed, attachment.ID)
 		} else {