Skip to content

Commit f977c1e

Browse files
lunnylunny
committed
Fix possible bug
1 parent 675ba42 commit f977c1e

File tree

4 files changed

+57
-123
lines changed

4 files changed

+57
-123
lines changed

‎models/issues/issue_update.go‎

Lines changed: 0 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"code.gitea.io/gitea/models/db"
1313
"code.gitea.io/gitea/models/organization"
1414
access_model "code.gitea.io/gitea/models/perm/access"
15-
project_model "code.gitea.io/gitea/models/project"
1615
repo_model "code.gitea.io/gitea/models/repo"
1716
"code.gitea.io/gitea/models/unit"
1817
user_model "code.gitea.io/gitea/models/user"
@@ -714,107 +713,6 @@ func UpdateReactionsMigrationsByType(ctx context.Context, gitServiceType api.Git
714713
return err
715714
}
716715

717-
// DeleteIssuesByRepoID deletes issues by repositories id
718-
func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths []string, err error) {
719-
// MariaDB has a performance bug: https://jira.mariadb.org/browse/MDEV-16289
720-
// so here it uses "DELETE ... WHERE IN" with pre-queried IDs.
721-
sess := db.GetEngine(ctx)
722-
723-
for {
724-
issueIDs := make([]int64, 0, db.DefaultMaxInSize)
725-
726-
err := sess.Table(&Issue{}).Where("repo_id = ?", repoID).OrderBy("id").Limit(db.DefaultMaxInSize).Cols("id").Find(&issueIDs)
727-
if err != nil {
728-
return nil, err
729-
}
730-
731-
if len(issueIDs) == 0 {
732-
break
733-
}
734-
735-
// Delete content histories
736-
_, err = sess.In("issue_id", issueIDs).Delete(&ContentHistory{})
737-
if err != nil {
738-
return nil, err
739-
}
740-
741-
// Delete comments and attachments
742-
_, err = sess.In("issue_id", issueIDs).Delete(&Comment{})
743-
if err != nil {
744-
return nil, err
745-
}
746-
747-
// Dependencies for issues in this repository
748-
_, err = sess.In("issue_id", issueIDs).Delete(&IssueDependency{})
749-
if err != nil {
750-
return nil, err
751-
}
752-
753-
// Delete dependencies for issues in other repositories
754-
_, err = sess.In("dependency_id", issueIDs).Delete(&IssueDependency{})
755-
if err != nil {
756-
return nil, err
757-
}
758-
759-
_, err = sess.In("issue_id", issueIDs).Delete(&IssueUser{})
760-
if err != nil {
761-
return nil, err
762-
}
763-
764-
_, err = sess.In("issue_id", issueIDs).Delete(&Reaction{})
765-
if err != nil {
766-
return nil, err
767-
}
768-
769-
_, err = sess.In("issue_id", issueIDs).Delete(&IssueWatch{})
770-
if err != nil {
771-
return nil, err
772-
}
773-
774-
_, err = sess.In("issue_id", issueIDs).Delete(&Stopwatch{})
775-
if err != nil {
776-
return nil, err
777-
}
778-
779-
_, err = sess.In("issue_id", issueIDs).Delete(&TrackedTime{})
780-
if err != nil {
781-
return nil, err
782-
}
783-
784-
_, err = sess.In("issue_id", issueIDs).Delete(&project_model.ProjectIssue{})
785-
if err != nil {
786-
return nil, err
787-
}
788-
789-
_, err = sess.In("dependent_issue_id", issueIDs).Delete(&Comment{})
790-
if err != nil {
791-
return nil, err
792-
}
793-
794-
var attachments []*repo_model.Attachment
795-
err = sess.In("issue_id", issueIDs).Find(&attachments)
796-
if err != nil {
797-
return nil, err
798-
}
799-
800-
for j := range attachments {
801-
attachmentPaths = append(attachmentPaths, attachments[j].RelativePath())
802-
}
803-
804-
_, err = sess.In("issue_id", issueIDs).Delete(&repo_model.Attachment{})
805-
if err != nil {
806-
return nil, err
807-
}
808-
809-
_, err = sess.In("id", issueIDs).Delete(&Issue{})
810-
if err != nil {
811-
return nil, err
812-
}
813-
}
814-
815-
return attachmentPaths, err
816-
}
817-
818716
func GetOrphanedIssueRepoIDs(ctx context.Context) ([]int64, error) {
819717
var repoIDs []int64
820718
if err := db.GetEngine(ctx).Table("issue").Distinct("issue.repo_id").

‎services/issue/issue.go‎

Lines changed: 52 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -190,9 +190,13 @@ func DeleteIssue(ctx context.Context, doer *user_model.User, gitRepo *git.Reposi
190190
}
191191

192192
// delete entries in database
193-
if err := deleteIssue(ctx, issue); err != nil {
193+
attachmentPaths, err := deleteIssue(ctx, issue)
194+
if err != nil {
194195
return err
195196
}
197+
for _, attachmentPath := range attachmentPaths {
198+
system_model.RemoveStorageWithNotice(ctx, storage.Attachments, "Delete issue attachment", attachmentPath)
199+
}
196200

197201
// delete pull request related git data
198202
if issue.IsPull && gitRepo != nil {
@@ -256,45 +260,45 @@ func GetRefEndNamesAndURLs(issues []*issues_model.Issue, repoLink string) (map[i
256260
}
257261

258262
// deleteIssue deletes the issue
259-
func deleteIssue(ctx context.Context, issue *issues_model.Issue) error {
263+
func deleteIssue(ctx context.Context, issue *issues_model.Issue) ([]string, error) {
260264
ctx, committer, err := db.TxContext(ctx)
261265
if err != nil {
262-
return err
266+
return nil, err
263267
}
264268
defer committer.Close()
265269

266-
e := db.GetEngine(ctx)
267-
if _, err := e.ID(issue.ID).NoAutoCondition().Delete(issue); err != nil {
268-
return err
270+
if _, err := db.GetEngine(ctx).ID(issue.ID).NoAutoCondition().Delete(issue); err != nil {
271+
return nil, err
269272
}
270273

271274
// update the total issue numbers
272275
if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, false); err != nil {
273-
return err
276+
return nil, err
274277
}
275278
// if the issue is closed, update the closed issue numbers
276279
if issue.IsClosed {
277280
if err := repo_model.UpdateRepoIssueNumbers(ctx, issue.RepoID, issue.IsPull, true); err != nil {
278-
return err
281+
return nil, err
279282
}
280283
}
281284

282285
if err := issues_model.UpdateMilestoneCounters(ctx, issue.MilestoneID); err != nil {
283-
return fmt.Errorf("error updating counters for milestone id %d: %w",
286+
return nil, fmt.Errorf("error updating counters for milestone id %d: %w",
284287
issue.MilestoneID, err)
285288
}
286289

287290
if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID, issue.Index); err != nil {
288-
return err
291+
return nil, err
289292
}
290293

291294
// find attachments related to this issue and remove them
292-
if err := issue.LoadAttributes(ctx); err != nil {
293-
return err
295+
if err := issue.LoadAttachments(ctx); err != nil {
296+
return nil, err
294297
}
295298

299+
var attachmentPaths []string
296300
for i := range issue.Attachments {
297-
system_model.RemoveStorageWithNotice(ctx, storage.Attachments, "Delete issue attachment", issue.Attachments[i].RelativePath())
301+
attachmentPaths = append(attachmentPaths, issue.Attachments[i].RelativePath())
298302
}
299303

300304
// delete all database data still assigned to this issue
@@ -318,10 +322,13 @@ func deleteIssue(ctx context.Context, issue *issues_model.Issue) error {
318322
&issues_model.Comment{DependentIssueID: issue.ID},
319323
&issues_model.IssuePin{IssueID: issue.ID},
320324
); err != nil {
321-
return err
325+
return nil, err
322326
}
323327

324-
return committer.Commit()
328+
if err := committer.Commit(); err != nil {
329+
return nil, err
330+
}
331+
return attachmentPaths, nil
325332
}
326333

327334
// DeleteOrphanedIssues delete issues without a repo
@@ -333,13 +340,12 @@ func DeleteOrphanedIssues(ctx context.Context) error {
333340
return err
334341
}
335342
for i := range repoIDs {
336-
paths, err := issues_model.DeleteIssuesByRepoID(ctx, repoIDs[i])
343+
paths, err := DeleteIssuesByRepoID(ctx, repoIDs[i])
337344
if err != nil {
338345
return err
339346
}
340347
attachmentPaths = append(attachmentPaths, paths...)
341348
}
342-
343349
return nil
344350
})
345351
if err != nil {
@@ -352,3 +358,32 @@ func DeleteOrphanedIssues(ctx context.Context) error {
352358
}
353359
return nil
354360
}
361+
362+
// DeleteIssuesByRepoID deletes issues by repositories id
363+
func DeleteIssuesByRepoID(ctx context.Context, repoID int64) (attachmentPaths []string, err error) {
364+
for {
365+
issues := make([]*issues_model.Issue, 0, db.DefaultMaxInSize)
366+
if err := db.GetEngine(ctx).
367+
Where("repo_id = ?", repoID).
368+
OrderBy("id").
369+
Limit(db.DefaultMaxInSize).
370+
Find(&issues); err != nil {
371+
return nil, err
372+
}
373+
374+
if len(issues) == 0 {
375+
break
376+
}
377+
378+
for _, issue := range issues {
379+
issueAttachPaths, err := deleteIssue(ctx, issue)
380+
if err != nil {
381+
return nil, fmt.Errorf("deleteIssue [issue_id: %d]: %w", issue.ID, err)
382+
}
383+
384+
attachmentPaths = append(attachmentPaths, issueAttachPaths...)
385+
}
386+
}
387+
388+
return attachmentPaths, err
389+
}

‎services/issue/issue_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func TestIssue_DeleteIssue(t *testing.T) {
4444
ID: issueIDs[2],
4545
}
4646

47-
err = deleteIssue(db.DefaultContext, issue)
47+
_, err = deleteIssue(db.DefaultContext, issue)
4848
assert.NoError(t, err)
4949
issueIDs, err = issues_model.GetIssueIDsByRepoID(db.DefaultContext, 1)
5050
assert.NoError(t, err)
@@ -55,7 +55,7 @@ func TestIssue_DeleteIssue(t *testing.T) {
5555
assert.NoError(t, err)
5656
issue, err = issues_model.GetIssueByID(db.DefaultContext, 4)
5757
assert.NoError(t, err)
58-
err = deleteIssue(db.DefaultContext, issue)
58+
_, err = deleteIssue(db.DefaultContext, issue)
5959
assert.NoError(t, err)
6060
assert.Len(t, attachments, 2)
6161
for i := range attachments {
@@ -78,7 +78,7 @@ func TestIssue_DeleteIssue(t *testing.T) {
7878
assert.NoError(t, err)
7979
assert.False(t, left)
8080

81-
err = deleteIssue(db.DefaultContext, issue2)
81+
_, err = deleteIssue(db.DefaultContext, issue2)
8282
assert.NoError(t, err)
8383
left, err = issues_model.IssueNoDependenciesLeft(db.DefaultContext, issue1)
8484
assert.NoError(t, err)

‎services/repository/delete.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"code.gitea.io/gitea/modules/log"
2929
"code.gitea.io/gitea/modules/storage"
3030
asymkey_service "code.gitea.io/gitea/services/asymkey"
31+
issue_service "code.gitea.io/gitea/services/issue"
3132

3233
"xorm.io/builder"
3334
)
@@ -184,7 +185,7 @@ func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, repoID
184185

185186
// Delete Issues and related objects
186187
var attachmentPaths []string
187-
if attachmentPaths, err = issues_model.DeleteIssuesByRepoID(ctx, repoID); err != nil {
188+
if attachmentPaths, err = issue_service.DeleteIssuesByRepoID(ctx, repoID); err != nil {
188189
return err
189190
}
190191

0 commit comments

Comments
 (0)