Skip to content

Commit b80b656

Browse files
6543AbdulrhmnGhanem
6543
authored and
AbdulrhmnGhanem
committed
Delete related PullAutoMerge and ReviewState on User/Repo Deletion (go-gitea#19649)
* delete pullautomerges on repo/user deletion * delete reviewstates on repo/user deletion * optimize automerhe code * add index to reviewstate
1 parent 4d4015a commit b80b656

File tree

17 files changed

+124
-95
lines changed

17 files changed

+124
-95
lines changed

‎models/db/error.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,3 +42,18 @@ func IsErrSSHDisabled(err error) bool {
4242
func (err ErrSSHDisabled) Error() string {
4343
return "SSH is disabled"
4444
}
45+
46+
// ErrNotExist represents a non-exist error.
47+
type ErrNotExist struct {
48+
ID int64
49+
}
50+
51+
// IsErrNotExist checks if an error is an ErrNotExist
52+
func IsErrNotExist(err error) bool {
53+
_, ok := err.(ErrNotExist)
54+
return ok
55+
}
56+
57+
func (err ErrNotExist) Error() string {
58+
return fmt.Sprintf("record does not exist [id: %d]", err.ID)
59+
}

‎models/error.go

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,21 +13,6 @@ import (
1313
"code.gitea.io/gitea/modules/git"
1414
)
1515

16-
// ErrNotExist represents a non-exist error.
17-
type ErrNotExist struct {
18-
ID int64
19-
}
20-
21-
// IsErrNotExist checks if an error is an ErrNotExist
22-
func IsErrNotExist(err error) bool {
23-
_, ok := err.(ErrNotExist)
24-
return ok
25-
}
26-
27-
func (err ErrNotExist) Error() string {
28-
return fmt.Sprintf("record does not exist [id: %d]", err.ID)
29-
}
30-
3116
// ErrUserOwnRepos represents a "UserOwnRepos" kind of error.
3217
type ErrUserOwnRepos struct {
3318
UID int64

‎models/issue_comment.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1360,6 +1360,28 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *Pul
13601360
return
13611361
}
13621362

1363+
// CreateAutoMergeComment is a internal function, only use it for CommentTypePRScheduledToAutoMerge and CommentTypePRUnScheduledToAutoMerge CommentTypes
1364+
func CreateAutoMergeComment(ctx context.Context, typ CommentType, pr *PullRequest, doer *user_model.User) (comment *Comment, err error) {
1365+
if typ != CommentTypePRScheduledToAutoMerge && typ != CommentTypePRUnScheduledToAutoMerge {
1366+
return nil, fmt.Errorf("comment type %d cannot be used to create an auto merge comment", typ)
1367+
}
1368+
if err = pr.LoadIssueCtx(ctx); err != nil {
1369+
return
1370+
}
1371+
1372+
if err = pr.LoadBaseRepoCtx(ctx); err != nil {
1373+
return
1374+
}
1375+
1376+
comment, err = CreateCommentCtx(ctx, &CreateCommentOptions{
1377+
Type: typ,
1378+
Doer: doer,
1379+
Repo: pr.BaseRepo,
1380+
Issue: pr.Issue,
1381+
})
1382+
return
1383+
}
1384+
13631385
// getCommitsFromRepo get commit IDs from repo in between oldCommitID and newCommitID
13641386
// isForcePush will be true if oldCommit isn't on the branch
13651387
// Commit on baseBranch will skip

‎models/issue_tracked_time.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ func DeleteIssueUserTimes(issue *Issue, user *user_model.User) error {
251251
return err
252252
}
253253
if removedTime == 0 {
254-
return ErrNotExist{}
254+
return db.ErrNotExist{}
255255
}
256256

257257
if err := issue.LoadRepo(ctx); err != nil {
@@ -311,7 +311,7 @@ func deleteTimes(e db.Engine, opts FindTrackedTimesOptions) (removedTime int64,
311311

312312
func deleteTime(e db.Engine, t *TrackedTime) error {
313313
if t.Deleted {
314-
return ErrNotExist{ID: t.ID}
314+
return db.ErrNotExist{ID: t.ID}
315315
}
316316
t.Deleted = true
317317
_, err := e.ID(t.ID).Cols("deleted").Update(t)
@@ -325,7 +325,7 @@ func GetTrackedTimeByID(id int64) (*TrackedTime, error) {
325325
if err != nil {
326326
return nil, err
327327
} else if !has {
328-
return nil, ErrNotExist{ID: id}
328+
return nil, db.ErrNotExist{ID: id}
329329
}
330330
return time, nil
331331
}

‎models/migrations/v215.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ func addReviewViewedFiles(x *xorm.Engine) error {
1515
type ReviewState struct {
1616
ID int64 `xorm:"pk autoincr"`
1717
UserID int64 `xorm:"NOT NULL UNIQUE(pull_commit_user)"`
18-
PullID int64 `xorm:"NOT NULL UNIQUE(pull_commit_user) DEFAULT 0"`
18+
PullID int64 `xorm:"NOT NULL INDEX UNIQUE(pull_commit_user) DEFAULT 0"`
1919
CommitSHA string `xorm:"NOT NULL VARCHAR(40) UNIQUE(pull_commit_user)"`
2020
UpdatedFiles map[string]pull.ViewedState `xorm:"NOT NULL LONGTEXT JSON"`
2121
UpdatedUnix timeutil.TimeStamp `xorm:"updated"`

‎models/notification.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -825,7 +825,7 @@ func getNotificationByID(e db.Engine, notificationID int64) (*Notification, erro
825825
}
826826

827827
if !ok {
828-
return nil, ErrNotExist{ID: notificationID}
828+
return nil, db.ErrNotExist{ID: notificationID}
829829
}
830830

831831
return notification, nil

‎models/pull.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"strings"
1313

1414
"code.gitea.io/gitea/models/db"
15+
pull_model "code.gitea.io/gitea/models/pull"
1516
repo_model "code.gitea.io/gitea/models/repo"
1617
user_model "code.gitea.io/gitea/models/user"
1718
"code.gitea.io/gitea/modules/git"
@@ -96,6 +97,25 @@ func init() {
9697
db.RegisterModel(new(PullRequest))
9798
}
9899

100+
func deletePullsByBaseRepoID(sess db.Engine, repoID int64) error {
101+
deleteCond := builder.Select("id").From("pull_request").Where(builder.Eq{"pull_request.base_repo_id": repoID})
102+
103+
// Delete scheduled auto merges
104+
if _, err := sess.In("pull_id", deleteCond).
105+
Delete(&pull_model.AutoMerge{}); err != nil {
106+
return err
107+
}
108+
109+
// Delete review states
110+
if _, err := sess.In("pull_id", deleteCond).
111+
Delete(&pull_model.ReviewState{}); err != nil {
112+
return err
113+
}
114+
115+
_, err := sess.Delete(&PullRequest{BaseRepoID: repoID})
116+
return err
117+
}
118+
99119
// MustHeadUserName returns the HeadRepo's username if failed return blank
100120
func (pr *PullRequest) MustHeadUserName() string {
101121
if err := pr.LoadHeadRepo(); err != nil {

‎models/pull/automerge.go

Lines changed: 10 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"context"
99
"fmt"
1010

11-
"code.gitea.io/gitea/models"
1211
"code.gitea.io/gitea/models/db"
1312
repo_model "code.gitea.io/gitea/models/repo"
1413
user_model "code.gitea.io/gitea/models/user"
@@ -59,21 +58,12 @@ func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pullID int64,
5958
return ErrAlreadyScheduledToAutoMerge{PullID: pullID}
6059
}
6160

62-
if _, err := db.GetEngine(ctx).Insert(&AutoMerge{
61+
_, err := db.GetEngine(ctx).Insert(&AutoMerge{
6362
DoerID: doer.ID,
6463
PullID: pullID,
6564
MergeStyle: style,
6665
Message: message,
67-
}); err != nil {
68-
return err
69-
}
70-
71-
pr, err := models.GetPullRequestByID(ctx, pullID)
72-
if err != nil {
73-
return err
74-
}
75-
76-
_, err = createAutoMergeComment(ctx, models.CommentTypePRScheduledToAutoMerge, pr, doer)
66+
})
7767
return err
7868
}
7969

@@ -94,50 +84,15 @@ func GetScheduledMergeByPullID(ctx context.Context, pullID int64) (bool, *AutoMe
9484
return true, scheduledPRM, nil
9585
}
9686

97-
// RemoveScheduledAutoMerge cancels a previously scheduled pull request
98-
func RemoveScheduledAutoMerge(ctx context.Context, doer *user_model.User, pullID int64, comment bool) error {
99-
return db.WithTx(func(ctx context.Context) error {
100-
exist, scheduledPRM, err := GetScheduledMergeByPullID(ctx, pullID)
101-
if err != nil {
102-
return err
103-
} else if !exist {
104-
return models.ErrNotExist{ID: pullID}
105-
}
106-
107-
if _, err := db.GetEngine(ctx).ID(scheduledPRM.ID).Delete(&AutoMerge{}); err != nil {
108-
return err
109-
}
110-
111-
// if pull got merged we don't need to add "auto-merge canceled comment"
112-
if !comment || doer == nil {
113-
return nil
114-
}
115-
116-
pr, err := models.GetPullRequestByID(ctx, pullID)
117-
if err != nil {
118-
return err
119-
}
120-
121-
_, err = createAutoMergeComment(ctx, models.CommentTypePRUnScheduledToAutoMerge, pr, doer)
87+
// DeleteScheduledAutoMerge delete a scheduled pull request
88+
func DeleteScheduledAutoMerge(ctx context.Context, pullID int64) error {
89+
exist, scheduledPRM, err := GetScheduledMergeByPullID(ctx, pullID)
90+
if err != nil {
12291
return err
123-
}, ctx)
124-
}
125-
126-
// createAutoMergeComment is a internal function, only use it for CommentTypePRScheduledToAutoMerge and CommentTypePRUnScheduledToAutoMerge CommentTypes
127-
func createAutoMergeComment(ctx context.Context, typ models.CommentType, pr *models.PullRequest, doer *user_model.User) (comment *models.Comment, err error) {
128-
if err = pr.LoadIssueCtx(ctx); err != nil {
129-
return
92+
} else if !exist {
93+
return db.ErrNotExist{ID: pullID}
13094
}
13195

132-
if err = pr.LoadBaseRepoCtx(ctx); err != nil {
133-
return
134-
}
135-
136-
comment, err = models.CreateCommentCtx(ctx, &models.CreateCommentOptions{
137-
Type: typ,
138-
Doer: doer,
139-
Repo: pr.BaseRepo,
140-
Issue: pr.Issue,
141-
})
142-
return
96+
_, err = db.GetEngine(ctx).ID(scheduledPRM.ID).Delete(&AutoMerge{})
97+
return err
14398
}

‎models/pull/review_state.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@ func (viewedState ViewedState) String() string {
3838
type ReviewState struct {
3939
ID int64 `xorm:"pk autoincr"`
4040
UserID int64 `xorm:"NOT NULL UNIQUE(pull_commit_user)"`
41-
PullID int64 `xorm:"NOT NULL UNIQUE(pull_commit_user) DEFAULT 0"` // Which PR was the review on?
42-
CommitSHA string `xorm:"NOT NULL VARCHAR(40) UNIQUE(pull_commit_user)"` // Which commit was the head commit for the review?
43-
UpdatedFiles map[string]ViewedState `xorm:"NOT NULL LONGTEXT JSON"` // Stores for each of the changed files of a PR whether they have been viewed, changed since last viewed, or not viewed
44-
UpdatedUnix timeutil.TimeStamp `xorm:"updated"` // Is an accurate indicator of the order of commits as we do not expect it to be possible to make reviews on previous commits
41+
PullID int64 `xorm:"NOT NULL INDEX UNIQUE(pull_commit_user) DEFAULT 0"` // Which PR was the review on?
42+
CommitSHA string `xorm:"NOT NULL VARCHAR(40) UNIQUE(pull_commit_user)"` // Which commit was the head commit for the review?
43+
UpdatedFiles map[string]ViewedState `xorm:"NOT NULL LONGTEXT JSON"` // Stores for each of the changed files of a PR whether they have been viewed, changed since last viewed, or not viewed
44+
UpdatedUnix timeutil.TimeStamp `xorm:"updated"` // Is an accurate indicator of the order of commits as we do not expect it to be possible to make reviews on previous commits
4545
}
4646

4747
func init() {

‎models/repo.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,6 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error {
704704
&Notification{RepoID: repoID},
705705
&ProtectedBranch{RepoID: repoID},
706706
&ProtectedTag{RepoID: repoID},
707-
&PullRequest{BaseRepoID: repoID},
708707
&repo_model.PushMirror{RepoID: repoID},
709708
&Release{RepoID: repoID},
710709
&repo_model.RepoIndexerStatus{RepoID: repoID},
@@ -723,6 +722,11 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error {
723722
return err
724723
}
725724

725+
// Delete Pulls and related objects
726+
if err := deletePullsByBaseRepoID(sess, repoID); err != nil {
727+
return err
728+
}
729+
726730
// Delete Issues and related objects
727731
var attachmentPaths []string
728732
if attachmentPaths, err = deleteIssuesByRepoID(sess, repoID); err != nil {

‎models/user.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"code.gitea.io/gitea/models/db"
1717
"code.gitea.io/gitea/models/issues"
1818
"code.gitea.io/gitea/models/organization"
19+
pull_model "code.gitea.io/gitea/models/pull"
1920
repo_model "code.gitea.io/gitea/models/repo"
2021
user_model "code.gitea.io/gitea/models/user"
2122
"code.gitea.io/gitea/modules/setting"
@@ -82,6 +83,8 @@ func DeleteUser(ctx context.Context, u *user_model.User) (err error) {
8283
&Collaboration{UserID: u.ID},
8384
&Stopwatch{UserID: u.ID},
8485
&user_model.Setting{UserID: u.ID},
86+
&pull_model.AutoMerge{DoerID: u.ID},
87+
&pull_model.ReviewState{UserID: u.ID},
8588
); err != nil {
8689
return fmt.Errorf("deleteBeans: %v", err)
8790
}

‎routers/api/v1/notify/threads.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"net/http"
1010

1111
"code.gitea.io/gitea/models"
12+
"code.gitea.io/gitea/models/db"
1213
"code.gitea.io/gitea/modules/context"
1314
"code.gitea.io/gitea/modules/convert"
1415
)
@@ -102,7 +103,7 @@ func ReadThread(ctx *context.APIContext) {
102103
func getThread(ctx *context.APIContext) *models.Notification {
103104
n, err := models.GetNotificationByID(ctx.ParamsInt64(":id"))
104105
if err != nil {
105-
if models.IsErrNotExist(err) {
106+
if db.IsErrNotExist(err) {
106107
ctx.Error(http.StatusNotFound, "GetNotificationByID", err)
107108
} else {
108109
ctx.InternalServerError(err)

‎routers/api/v1/repo/issue_tracked_time.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"time"
1111

1212
"code.gitea.io/gitea/models"
13+
"code.gitea.io/gitea/models/db"
1314
"code.gitea.io/gitea/models/unit"
1415
user_model "code.gitea.io/gitea/models/user"
1516
"code.gitea.io/gitea/modules/context"
@@ -281,7 +282,7 @@ func ResetIssueTime(ctx *context.APIContext) {
281282

282283
err = models.DeleteIssueUserTimes(issue, ctx.Doer)
283284
if err != nil {
284-
if models.IsErrNotExist(err) {
285+
if db.IsErrNotExist(err) {
285286
ctx.Error(http.StatusNotFound, "DeleteIssueUserTimes", err)
286287
} else {
287288
ctx.Error(http.StatusInternalServerError, "DeleteIssueUserTimes", err)
@@ -352,7 +353,7 @@ func DeleteTime(ctx *context.APIContext) {
352353

353354
time, err := models.GetTrackedTimeByID(ctx.ParamsInt64(":id"))
354355
if err != nil {
355-
if models.IsErrNotExist(err) {
356+
if db.IsErrNotExist(err) {
356357
ctx.NotFound(err)
357358
return
358359
}

‎routers/api/v1/repo/pull.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1208,7 +1208,7 @@ func CancelScheduledAutoMerge(ctx *context.APIContext) {
12081208
}
12091209
}
12101210

1211-
if err := pull_model.RemoveScheduledAutoMerge(ctx, ctx.Doer, pull.ID, true); err != nil {
1211+
if err := automerge.RemoveScheduledAutoMerge(ctx, ctx.Doer, pull); err != nil {
12121212
ctx.InternalServerError(err)
12131213
} else {
12141214
ctx.Status(http.StatusNoContent)

‎routers/web/repo/issue_timetrack.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"time"
1010

1111
"code.gitea.io/gitea/models"
12+
"code.gitea.io/gitea/models/db"
1213
"code.gitea.io/gitea/modules/context"
1314
"code.gitea.io/gitea/modules/util"
1415
"code.gitea.io/gitea/modules/web"
@@ -63,7 +64,7 @@ func DeleteTime(c *context.Context) {
6364

6465
t, err := models.GetTrackedTimeByID(c.ParamsInt64(":timeid"))
6566
if err != nil {
66-
if models.IsErrNotExist(err) {
67+
if db.IsErrNotExist(err) {
6768
c.NotFound("time not found", err)
6869
return
6970
}

0 commit comments

Comments
 (0)