Skip to content

Commit 9c251fd

Browse files
lunnylunny
committed
Fix some problems
1 parent 551f8ba commit 9c251fd

File tree

10 files changed

+139
-46
lines changed

10 files changed

+139
-46
lines changed

‎models/db/file_status.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ package db
77
type FileStatus int
88

99
const (
10-
FileStatusNormal FileStatus = iota // FileStatusNormal indicates the file is normal and exists on disk.
11-
FileStatusToBeDeleted // FileStatusToBeDeleted indicates the file is marked for deletion but still exists on disk.
10+
FileStatusNormal FileStatus = iota + 1 // FileStatusNormal indicates the file is normal and exists on disk.
11+
FileStatusToBeDeleted // FileStatusToBeDeleted indicates the file is marked for deletion but still exists on disk.
1212
)

‎models/issues/comment.go‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1141,26 +1141,36 @@ func DeleteComment(ctx context.Context, comment *Comment) (*Comment, error) {
11411141

11421142
// delete review & review comment if the code comment is the last comment of the review
11431143
if comment.Type == CommentTypeCode && comment.ReviewID > 0 {
1144-
res, err := db.GetEngine(ctx).ID(comment.ReviewID).
1145-
Where("NOT EXISTS (SELECT 1 FROM comment WHERE review_id = ? AND `type` = ?)", comment.ReviewID, CommentTypeCode).
1146-
Delete(new(Review))
1147-
if err != nil {
1144+
if err := comment.LoadReview(ctx); err != nil {
11481145
return nil, err
11491146
}
1150-
if res > 0 {
1151-
var reviewComment Comment
1152-
has, err := db.GetEngine(ctx).Where("review_id = ?", comment.ReviewID).
1153-
And("type = ?", CommentTypeReview).Get(&reviewComment)
1147+
if comment.Review != nil && comment.Review.Type == ReviewTypeComment {
1148+
res, err := db.GetEngine(ctx).ID(comment.ReviewID).
1149+
Where("NOT EXISTS (SELECT 1 FROM comment WHERE review_id = ? AND `type` = ?)", comment.ReviewID, CommentTypeCode).
1150+
Delete(new(Review))
11541151
if err != nil {
11551152
return nil, err
11561153
}
1157-
if has && reviewComment.Content == "" {
1158-
if _, err := db.GetEngine(ctx).ID(reviewComment.ID).Delete(new(Comment)); err != nil {
1154+
if res > 0 {
1155+
var reviewComment Comment
1156+
has, err := db.GetEngine(ctx).Where("review_id = ?", comment.ReviewID).
1157+
And("`type` = ?", CommentTypeReview).Get(&reviewComment)
1158+
if err != nil {
11591159
return nil, err
11601160
}
1161-
deletedReviewComment = &reviewComment
1161+
if has && reviewComment.Content == "" {
1162+
if err := reviewComment.LoadAttachments(ctx); err != nil {
1163+
return nil, err
1164+
}
1165+
if len(reviewComment.Attachments) == 0 {
1166+
if _, err := db.GetEngine(ctx).ID(reviewComment.ID).Delete(new(Comment)); err != nil {
1167+
return nil, err
1168+
}
1169+
deletedReviewComment = &reviewComment
1170+
}
1171+
}
1172+
comment.ReviewID = 0 // reset review ID to 0 for the notification
11621173
}
1163-
comment.ReviewID = 0 // reset review ID to 0 for the notification
11641174
}
11651175
}
11661176

‎models/issues/comment_list.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,10 @@ func (comments CommentList) LoadAttachmentsByIssue(ctx context.Context) error {
349349
}
350350

351351
attachments := make([]*repo_model.Attachment, 0, len(comments)/2)
352-
if err := db.GetEngine(ctx).Where("issue_id=? AND comment_id>0", comments[0].IssueID).Find(&attachments); err != nil {
352+
if err := db.GetEngine(ctx).
353+
Where("issue_id=? AND comment_id>0", comments[0].IssueID).
354+
And("status = ?", db.FileStatusNormal).
355+
Find(&attachments); err != nil {
353356
return err
354357
}
355358

@@ -377,6 +380,7 @@ func (comments CommentList) LoadAttachments(ctx context.Context) (err error) {
377380
limit := min(left, db.DefaultMaxInSize)
378381
rows, err := db.GetEngine(ctx).
379382
In("comment_id", commentsIDs[:limit]).
383+
And("status = ?", db.FileStatusNormal).
380384
Rows(new(repo_model.Attachment))
381385
if err != nil {
382386
return err

‎models/issues/issue_list.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,7 @@ func (issues IssueList) LoadAttachments(ctx context.Context) (err error) {
339339
limit := min(left, db.DefaultMaxInSize)
340340
rows, err := db.GetEngine(ctx).
341341
In("issue_id", issuesIDs[:limit]).
342+
And("status = ?", db.FileStatusNormal).
342343
Rows(new(repo_model.Attachment))
343344
if err != nil {
344345
return err

‎models/migrations/v1_25/v321.go‎

Lines changed: 58 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,34 +8,68 @@ import (
88
"code.gitea.io/gitea/modules/timeutil"
99

1010
"xorm.io/xorm"
11+
"xorm.io/xorm/schemas"
1112
)
1213

13-
func AddFileStatusToAttachment(x *xorm.Engine) error {
14-
type Attachment struct {
15-
ID int64 `xorm:"pk autoincr"`
16-
UUID string `xorm:"uuid UNIQUE"`
17-
RepoID int64 `xorm:"INDEX"` // this should not be zero
18-
IssueID int64 `xorm:"INDEX"` // maybe zero when creating
19-
ReleaseID int64 `xorm:"INDEX"` // maybe zero when creating
20-
UploaderID int64 `xorm:"INDEX DEFAULT 0"` // Notice: will be zero before this column added
21-
CommentID int64 `xorm:"INDEX"`
22-
Name string
23-
DownloadCount int64 `xorm:"DEFAULT 0"`
24-
Status db.FileStatus `xorm:"INDEX DEFAULT 0"`
25-
DeleteFailedCount int `xorm:"DEFAULT 0"` // Number of times the deletion failed, used to prevent infinite loop
26-
LastDeleteFailedTime timeutil.TimeStamp // Last time the deletion failed, used to prevent infinite loop
27-
Size int64 `xorm:"DEFAULT 0"`
28-
CreatedUnix timeutil.TimeStamp `xorm:"created"`
29-
CustomDownloadURL string `xorm:"-"`
30-
}
14+
type Attachment321 struct {
15+
ID int64 `xorm:"pk autoincr"`
16+
UUID string `xorm:"uuid UNIQUE"`
17+
RepoID int64 `xorm:"INDEX"` // this should not be zero
18+
IssueID int64 `xorm:"INDEX"` // maybe zero when creating
19+
ReleaseID int64 `xorm:"INDEX"` // maybe zero when creating
20+
UploaderID int64 `xorm:"INDEX DEFAULT 0"` // Notice: will be zero before this column added
21+
CommentID int64 `xorm:"INDEX"`
22+
Name string
23+
DownloadCount int64 `xorm:"DEFAULT 0"`
24+
Status db.FileStatus `xorm:"INDEX DEFAULT 1 NOT NULL"` // 1 = normal, 2 = to be deleted
25+
DeleteFailedCount int `xorm:"DEFAULT 0"` // Number of times the deletion failed, used to prevent infinite loop
26+
LastDeleteFailedTime timeutil.TimeStamp // Last time the deletion failed, used to prevent infinite loop
27+
Size int64 `xorm:"DEFAULT 0"`
28+
CreatedUnix timeutil.TimeStamp `xorm:"created"`
29+
}
3130

32-
if err := x.Sync(new(Attachment)); err != nil {
33-
return err
34-
}
31+
func (a *Attachment321) TableName() string {
32+
return "attachment"
33+
}
34+
35+
// TableIndices implements xorm's TableIndices interface
36+
func (a *Attachment321) TableIndices() []*schemas.Index {
37+
uuidIndex := schemas.NewIndex("attachment_uuid", schemas.UniqueType)
38+
uuidIndex.AddColumn("uuid")
39+
40+
repoIndex := schemas.NewIndex("attachment_repo_id", schemas.IndexType)
41+
repoIndex.AddColumn("repo_id")
42+
43+
issueIndex := schemas.NewIndex("attachment_issue_id", schemas.IndexType)
44+
issueIndex.AddColumn("issue_id")
3545

36-
if _, err := x.Exec("UPDATE `attachment` SET status = ? WHERE status IS NULL", db.FileStatusNormal); err != nil {
37-
return err
46+
releaseIndex := schemas.NewIndex("attachment_release_id", schemas.IndexType)
47+
releaseIndex.AddColumn("release_id")
48+
49+
uploaderIndex := schemas.NewIndex("attachment_uploader_id", schemas.IndexType)
50+
uploaderIndex.AddColumn("uploader_id")
51+
52+
commentIndex := schemas.NewIndex("attachment_comment_id", schemas.IndexType)
53+
commentIndex.AddColumn("comment_id")
54+
55+
statusIndex := schemas.NewIndex("attachment_status", schemas.IndexType)
56+
statusIndex.AddColumn("status")
57+
58+
statusIDIndex := schemas.NewIndex("attachment_status_id", schemas.IndexType)
59+
statusIDIndex.AddColumn("status_id", "id") // For status = ? AND id > ? query
60+
61+
return []*schemas.Index{
62+
uuidIndex,
63+
repoIndex,
64+
issueIndex,
65+
releaseIndex,
66+
uploaderIndex,
67+
commentIndex,
68+
statusIndex,
69+
statusIDIndex,
3870
}
71+
}
3972

40-
return nil
73+
func AddFileStatusToAttachment(x *xorm.Engine) error {
74+
return x.Sync(new(Attachment321))
4175
}

‎models/repo/attachment.go‎

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"code.gitea.io/gitea/modules/setting"
1515
"code.gitea.io/gitea/modules/timeutil"
1616
"code.gitea.io/gitea/modules/util"
17+
"xorm.io/xorm/schemas"
1718
)
1819

1920
// Attachment represent a attachment of issue/comment/release.
@@ -27,15 +28,53 @@ type Attachment struct {
2728
CommentID int64 `xorm:"INDEX"`
2829
Name string
2930
DownloadCount int64 `xorm:"DEFAULT 0"`
30-
Status db.FileStatus `xorm:"INDEX DEFAULT 0"`
31-
DeleteFailedCount int `xorm:"DEFAULT 0"` // Number of times the deletion failed, used to prevent infinite loop
32-
LastDeleteFailedReason string `xorm:"TEXT"` // Last reason the deletion failed, used to prevent infinite loop
31+
Status db.FileStatus `xorm:"INDEX DEFAULT 1 NOT NULL"` // 1 = normal, 2 = to be deleted
32+
DeleteFailedCount int `xorm:"DEFAULT 0"` // Number of times the deletion failed, used to prevent infinite loop
33+
LastDeleteFailedReason string `xorm:"TEXT"` // Last reason the deletion failed, used to prevent infinite loop
3334
LastDeleteFailedTime timeutil.TimeStamp // Last time the deletion failed, used to prevent infinite loop
3435
Size int64 `xorm:"DEFAULT 0"`
3536
CreatedUnix timeutil.TimeStamp `xorm:"created"`
3637
CustomDownloadURL string `xorm:"-"`
3738
}
3839

40+
// TableIndices implements xorm's TableIndices interface
41+
func (a *Attachment) TableIndices() []*schemas.Index {
42+
uuidIndex := schemas.NewIndex("attachment_uuid", schemas.UniqueType)
43+
uuidIndex.AddColumn("uuid")
44+
45+
repoIndex := schemas.NewIndex("attachment_repo_id", schemas.IndexType)
46+
repoIndex.AddColumn("repo_id")
47+
48+
issueIndex := schemas.NewIndex("attachment_issue_id", schemas.IndexType)
49+
issueIndex.AddColumn("issue_id")
50+
51+
releaseIndex := schemas.NewIndex("attachment_release_id", schemas.IndexType)
52+
releaseIndex.AddColumn("release_id")
53+
54+
uploaderIndex := schemas.NewIndex("attachment_uploader_id", schemas.IndexType)
55+
uploaderIndex.AddColumn("uploader_id")
56+
57+
commentIndex := schemas.NewIndex("attachment_comment_id", schemas.IndexType)
58+
commentIndex.AddColumn("comment_id")
59+
60+
statusIndex := schemas.NewIndex("attachment_status", schemas.IndexType)
61+
statusIndex.AddColumn("status")
62+
63+
statusIDIndex := schemas.NewIndex("attachment_status_id", schemas.IndexType)
64+
statusIDIndex.AddColumn("status_id", "id") // For status = ? AND id > ? query
65+
66+
return []*schemas.Index{
67+
uuidIndex,
68+
repoIndex,
69+
issueIndex,
70+
releaseIndex,
71+
uploaderIndex,
72+
commentIndex,
73+
statusIndex,
74+
statusIDIndex,
75+
}
76+
}
77+
3978
func init() {
4079
db.RegisterModel(new(Attachment))
4180
}

‎services/attachment/attachment.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ func NewAttachment(ctx context.Context, attach *repo_model.Attachment, file io.R
3737
return fmt.Errorf("Create: %w", err)
3838
}
3939
attach.Size = size
40+
attach.Status = db.FileStatusNormal
4041

4142
return db.Insert(ctx, attach)
4243
})
@@ -147,14 +148,16 @@ func cleanAttachments(ctx context.Context, attachmentIDs []int64) []int64 {
147148
return failed
148149
}
149150

150-
// ScanTobeDeletedAttachments scans for attachments that are marked as to be deleted and send to
151+
// ScanToBeDeletedAttachments scans for attachments that are marked as to be deleted and send to
151152
// clean queue
152-
func ScanTobeDeletedAttachments(ctx context.Context) error {
153+
func ScanToBeDeletedAttachments(ctx context.Context) error {
153154
attachments := make([]*repo_model.Attachment, 0, 10)
154155
lastID := int64(0)
155156
for {
156157
if err := db.GetEngine(ctx).
157-
Where("id > ? AND status = ?", lastID, db.FileStatusToBeDeleted).
158+
// use the status and id index to speed up the query
159+
Where("status = ? AND id > ?", db.FileStatusToBeDeleted, lastID).
160+
Asc("id").
158161
Limit(100).
159162
Find(&attachments); err != nil {
160163
return fmt.Errorf("scan to-be-deleted attachments: %w", err)

‎services/cron/tasks_extended.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -230,7 +230,7 @@ func registerCleanAttachments() {
230230
RunAtStart: false,
231231
Schedule: "@every 24h",
232232
}, func(ctx context.Context, _ *user_model.User, _ Config) error {
233-
return attachment_service.ScanTobeDeletedAttachments(ctx)
233+
return attachment_service.ScanToBeDeletedAttachments(ctx)
234234
})
235235
}
236236

‎services/issue/comments.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,7 @@ func deleteComment(ctx context.Context, comment *issues_model.Comment, removeAtt
141141
}
142142
}
143143

144+
// deletedReviewComment should be a review comment with no content and no attachments
144145
deletedReviewComment, err := issues_model.DeleteComment(ctx, comment)
145146
if err != nil {
146147
return nil, err

‎services/migrations/gitea_uploader.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,7 @@ func (g *GiteaLocalUploader) CreateReleases(ctx context.Context, releases ...*ba
323323
DownloadCount: int64(*asset.DownloadCount),
324324
Size: int64(*asset.Size),
325325
CreatedUnix: timeutil.TimeStamp(asset.Created.Unix()),
326+
Status: db.FileStatusNormal,
326327
}
327328

328329
// SECURITY: We cannot check the DownloadURL and DownloadFunc are safe here

0 commit comments

Comments
 (0)