From 4641bdbf620160c3adea58974c60eac5c8697fe1 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 7 May 2022 19:52:13 +0200 Subject: [PATCH 01/10] draft1 (has import cycle) --- models/pull.go | 14 ++++++++++++++ models/repo.go | 6 +++++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/models/pull.go b/models/pull.go index 0fa3bdf14f72a..3b9cbd9cac8a5 100644 --- a/models/pull.go +++ b/models/pull.go @@ -12,6 +12,7 @@ import ( "strings" "code.gitea.io/gitea/models/db" + pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" @@ -97,6 +98,19 @@ func init() { db.RegisterModel(new(PullRequest)) } +func deletePullsByBaseRepoID(sess db.Engine, repoID int64) error { + deleteCond := builder.Select("id").From("pull_request").Where(builder.Eq{"pull_request.base_repo_id": repoID}) + + // Delete scheduled auto merges + if _, err := sess.In("pull_id", deleteCond). + Delete(&pull_model.AutoMerge{}); err != nil { + return err + } + + _, err := sess.Delete(&PullRequest{BaseRepoID: repoID}) + return err +} + // MustHeadUserName returns the HeadRepo's username if failed return blank func (pr *PullRequest) MustHeadUserName() string { if err := pr.LoadHeadRepo(); err != nil { diff --git a/models/repo.go b/models/repo.go index fbc766850d954..e20bf90d97f37 100644 --- a/models/repo.go +++ b/models/repo.go @@ -704,7 +704,6 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error { &Notification{RepoID: repoID}, &ProtectedBranch{RepoID: repoID}, &ProtectedTag{RepoID: repoID}, - &PullRequest{BaseRepoID: repoID}, &repo_model.PushMirror{RepoID: repoID}, &Release{RepoID: repoID}, &repo_model.RepoIndexerStatus{RepoID: repoID}, @@ -723,6 +722,11 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error { return err } + // Delete Pulls and related objects + if err := deletePullsByBaseRepoID(sess, repoID); err != nil { + return err + } + // Delete Issues and related objects var attachmentPaths []string if attachmentPaths, err = deleteIssuesByRepoID(sess, repoID); err != nil { From 3903a4bf0c8ba6e604bc752e79ee02f043f8a3d9 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 7 May 2022 21:06:26 +0200 Subject: [PATCH 02/10] add review states (added in #19007) --- models/pull.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/models/pull.go b/models/pull.go index 3b9cbd9cac8a5..5eaef6161749b 100644 --- a/models/pull.go +++ b/models/pull.go @@ -107,6 +107,12 @@ func deletePullsByBaseRepoID(sess db.Engine, repoID int64) error { return err } + // Delete review states + if _, err := sess.In("pull_id", deleteCond). + Delete(&pull_model.ReviewState{}); err != nil { + return err + } + _, err := sess.Delete(&PullRequest{BaseRepoID: repoID}) return err } From 079f871506250d16a704577690e5bb1526c27ba4 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 7 May 2022 21:09:01 +0200 Subject: [PATCH 03/10] rm stuff on user delete too --- models/user.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/models/user.go b/models/user.go index e8307796293f8..11234a881df05 100644 --- a/models/user.go +++ b/models/user.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/issues" "code.gitea.io/gitea/models/organization" + pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/setting" @@ -82,6 +83,8 @@ func DeleteUser(ctx context.Context, u *user_model.User) (err error) { &Collaboration{UserID: u.ID}, &Stopwatch{UserID: u.ID}, &user_model.Setting{UserID: u.ID}, + &pull_model.AutoMerge{DoerID: u.ID}, + &pull_model.ReviewState{UserID: u.ID}, ); err != nil { return fmt.Errorf("deleteBeans: %v", err) } From 9bc0cc1477746e745a08f076560e286f1053573b Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 7 May 2022 21:34:32 +0200 Subject: [PATCH 04/10] resolve import cycle --- models/db/error.go | 15 ++++++ models/error.go | 15 ------ models/issue_comment.go | 19 +++++++ models/issue_tracked_time.go | 6 +-- models/notification.go | 2 +- models/pull/automerge.go | 65 ++++------------------- routers/api/v1/notify/threads.go | 3 +- routers/api/v1/repo/issue_tracked_time.go | 5 +- routers/api/v1/repo/pull.go | 2 +- routers/web/repo/issue_timetrack.go | 3 +- services/automerge/automerge.go | 33 ++++++++---- services/pull/automerge.go | 36 +++++++++++++ services/pull/merge.go | 3 +- 13 files changed, 117 insertions(+), 90 deletions(-) create mode 100644 services/pull/automerge.go diff --git a/models/db/error.go b/models/db/error.go index f20cc9b4cb71e..65572299436a6 100644 --- a/models/db/error.go +++ b/models/db/error.go @@ -42,3 +42,18 @@ func IsErrSSHDisabled(err error) bool { func (err ErrSSHDisabled) Error() string { return "SSH is disabled" } + +// ErrNotExist represents a non-exist error. +type ErrNotExist struct { + ID int64 +} + +// IsErrNotExist checks if an error is an ErrNotExist +func IsErrNotExist(err error) bool { + _, ok := err.(ErrNotExist) + return ok +} + +func (err ErrNotExist) Error() string { + return fmt.Sprintf("record does not exist [id: %d]", err.ID) +} diff --git a/models/error.go b/models/error.go index c29c818589fb4..0dc14c3e318c5 100644 --- a/models/error.go +++ b/models/error.go @@ -13,21 +13,6 @@ import ( "code.gitea.io/gitea/modules/git" ) -// ErrNotExist represents a non-exist error. -type ErrNotExist struct { - ID int64 -} - -// IsErrNotExist checks if an error is an ErrNotExist -func IsErrNotExist(err error) bool { - _, ok := err.(ErrNotExist) - return ok -} - -func (err ErrNotExist) Error() string { - return fmt.Sprintf("record does not exist [id: %d]", err.ID) -} - // ErrUserOwnRepos represents a "UserOwnRepos" kind of error. type ErrUserOwnRepos struct { UID int64 diff --git a/models/issue_comment.go b/models/issue_comment.go index 13b2c62546063..db8ae9dce5386 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -1360,6 +1360,25 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *Pul return } +// CreateAutoMergeComment is a internal function, only use it for CommentTypePRScheduledToAutoMerge and CommentTypePRUnScheduledToAutoMerge CommentTypes +func CreateAutoMergeComment(ctx context.Context, typ CommentType, pr *PullRequest, doer *user_model.User) (comment *Comment, err error) { + if err = pr.LoadIssueCtx(ctx); err != nil { + return + } + + if err = pr.LoadBaseRepoCtx(ctx); err != nil { + return + } + + comment, err = CreateCommentCtx(ctx, &CreateCommentOptions{ + Type: typ, + Doer: doer, + Repo: pr.BaseRepo, + Issue: pr.Issue, + }) + return +} + // getCommitsFromRepo get commit IDs from repo in between oldCommitID and newCommitID // isForcePush will be true if oldCommit isn't on the branch // Commit on baseBranch will skip diff --git a/models/issue_tracked_time.go b/models/issue_tracked_time.go index e675c7919378f..76ff874c59f21 100644 --- a/models/issue_tracked_time.go +++ b/models/issue_tracked_time.go @@ -251,7 +251,7 @@ func DeleteIssueUserTimes(issue *Issue, user *user_model.User) error { return err } if removedTime == 0 { - return ErrNotExist{} + return db.ErrNotExist{} } if err := issue.LoadRepo(ctx); err != nil { @@ -311,7 +311,7 @@ func deleteTimes(e db.Engine, opts FindTrackedTimesOptions) (removedTime int64, func deleteTime(e db.Engine, t *TrackedTime) error { if t.Deleted { - return ErrNotExist{ID: t.ID} + return db.ErrNotExist{ID: t.ID} } t.Deleted = true _, err := e.ID(t.ID).Cols("deleted").Update(t) @@ -325,7 +325,7 @@ func GetTrackedTimeByID(id int64) (*TrackedTime, error) { if err != nil { return nil, err } else if !has { - return nil, ErrNotExist{ID: id} + return nil, db.ErrNotExist{ID: id} } return time, nil } diff --git a/models/notification.go b/models/notification.go index a1248c240b948..d0b7852cd2481 100644 --- a/models/notification.go +++ b/models/notification.go @@ -825,7 +825,7 @@ func getNotificationByID(e db.Engine, notificationID int64) (*Notification, erro } if !ok { - return nil, ErrNotExist{ID: notificationID} + return nil, db.ErrNotExist{ID: notificationID} } return notification, nil diff --git a/models/pull/automerge.go b/models/pull/automerge.go index fd73f2b0fb0fb..8d9c0114b6b0c 100644 --- a/models/pull/automerge.go +++ b/models/pull/automerge.go @@ -8,7 +8,6 @@ import ( "context" "fmt" - "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" @@ -59,21 +58,12 @@ func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pullID int64, return ErrAlreadyScheduledToAutoMerge{PullID: pullID} } - if _, err := db.GetEngine(ctx).Insert(&AutoMerge{ + _, err := db.GetEngine(ctx).Insert(&AutoMerge{ DoerID: doer.ID, PullID: pullID, MergeStyle: style, Message: message, - }); err != nil { - return err - } - - pr, err := models.GetPullRequestByID(ctx, pullID) - if err != nil { - return err - } - - _, err = createAutoMergeComment(ctx, models.CommentTypePRScheduledToAutoMerge, pr, doer) + }) return err } @@ -94,50 +84,15 @@ func GetScheduledMergeByPullID(ctx context.Context, pullID int64) (bool, *AutoMe return true, scheduledPRM, nil } -// RemoveScheduledAutoMerge cancels a previously scheduled pull request -func RemoveScheduledAutoMerge(ctx context.Context, doer *user_model.User, pullID int64, comment bool) error { - return db.WithTx(func(ctx context.Context) error { - exist, scheduledPRM, err := GetScheduledMergeByPullID(ctx, pullID) - if err != nil { - return err - } else if !exist { - return models.ErrNotExist{ID: pullID} - } - - if _, err := db.GetEngine(ctx).ID(scheduledPRM.ID).Delete(&AutoMerge{}); err != nil { - return err - } - - // if pull got merged we don't need to add "auto-merge canceled comment" - if !comment || doer == nil { - return nil - } - - pr, err := models.GetPullRequestByID(ctx, pullID) - if err != nil { - return err - } - - _, err = createAutoMergeComment(ctx, models.CommentTypePRUnScheduledToAutoMerge, pr, doer) +// DeleteScheduledAutoMerge cancels a previously scheduled pull request +func DeleteScheduledAutoMerge(ctx context.Context, pullID int64) error { + exist, scheduledPRM, err := GetScheduledMergeByPullID(ctx, pullID) + if err != nil { return err - }, ctx) -} - -// createAutoMergeComment is a internal function, only use it for CommentTypePRScheduledToAutoMerge and CommentTypePRUnScheduledToAutoMerge CommentTypes -func createAutoMergeComment(ctx context.Context, typ models.CommentType, pr *models.PullRequest, doer *user_model.User) (comment *models.Comment, err error) { - if err = pr.LoadIssueCtx(ctx); err != nil { - return + } else if !exist { + return db.ErrNotExist{ID: pullID} } - if err = pr.LoadBaseRepoCtx(ctx); err != nil { - return - } - - comment, err = models.CreateCommentCtx(ctx, &models.CreateCommentOptions{ - Type: typ, - Doer: doer, - Repo: pr.BaseRepo, - Issue: pr.Issue, - }) - return + _, err = db.GetEngine(ctx).ID(scheduledPRM.ID).Delete(&AutoMerge{}) + return err } diff --git a/routers/api/v1/notify/threads.go b/routers/api/v1/notify/threads.go index fe89304dc878e..4effd6b3e02e0 100644 --- a/routers/api/v1/notify/threads.go +++ b/routers/api/v1/notify/threads.go @@ -9,6 +9,7 @@ import ( "net/http" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/convert" ) @@ -102,7 +103,7 @@ func ReadThread(ctx *context.APIContext) { func getThread(ctx *context.APIContext) *models.Notification { n, err := models.GetNotificationByID(ctx.ParamsInt64(":id")) if err != nil { - if models.IsErrNotExist(err) { + if db.IsErrNotExist(err) { ctx.Error(http.StatusNotFound, "GetNotificationByID", err) } else { ctx.InternalServerError(err) diff --git a/routers/api/v1/repo/issue_tracked_time.go b/routers/api/v1/repo/issue_tracked_time.go index e42dc60a94c81..8ccad87838c56 100644 --- a/routers/api/v1/repo/issue_tracked_time.go +++ b/routers/api/v1/repo/issue_tracked_time.go @@ -10,6 +10,7 @@ import ( "time" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/context" @@ -281,7 +282,7 @@ func ResetIssueTime(ctx *context.APIContext) { err = models.DeleteIssueUserTimes(issue, ctx.Doer) if err != nil { - if models.IsErrNotExist(err) { + if db.IsErrNotExist(err) { ctx.Error(http.StatusNotFound, "DeleteIssueUserTimes", err) } else { ctx.Error(http.StatusInternalServerError, "DeleteIssueUserTimes", err) @@ -352,7 +353,7 @@ func DeleteTime(ctx *context.APIContext) { time, err := models.GetTrackedTimeByID(ctx.ParamsInt64(":id")) if err != nil { - if models.IsErrNotExist(err) { + if db.IsErrNotExist(err) { ctx.NotFound(err) return } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 91bb57f3fd79a..22bb2b02453c5 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1196,7 +1196,7 @@ func CancelScheduledAutoMerge(ctx *context.APIContext) { } } - if err := pull_model.RemoveScheduledAutoMerge(ctx, ctx.Doer, pull.ID, true); err != nil { + if err := pull_service.RemoveScheduledAutoMerge(ctx, ctx.Doer, pull.ID, true); err != nil { ctx.InternalServerError(err) } else { ctx.Status(http.StatusNoContent) diff --git a/routers/web/repo/issue_timetrack.go b/routers/web/repo/issue_timetrack.go index 0809acc2e417f..28274a7f7b88f 100644 --- a/routers/web/repo/issue_timetrack.go +++ b/routers/web/repo/issue_timetrack.go @@ -9,6 +9,7 @@ import ( "time" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" @@ -63,7 +64,7 @@ func DeleteTime(c *context.Context) { t, err := models.GetTrackedTimeByID(c.ParamsInt64(":timeid")) if err != nil { - if models.IsErrNotExist(err) { + if db.IsErrNotExist(err) { c.NotFound("time not found", err) return } diff --git a/services/automerge/automerge.go b/services/automerge/automerge.go index 389546ed5784f..2578a7a70f1b4 100644 --- a/services/automerge/automerge.go +++ b/services/automerge/automerge.go @@ -12,6 +12,7 @@ import ( "strings" "code.gitea.io/gitea/models" + "code.gitea.io/gitea/models/db" pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" @@ -61,17 +62,31 @@ func addToQueue(pr *models.PullRequest, sha string) { // ScheduleAutoMerge if schedule is false and no error, pull can be merged directly func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pull *models.PullRequest, style repo_model.MergeStyle, message string) (scheduled bool, err error) { - lastCommitStatus, err := pull_service.GetPullRequestCommitStatusState(ctx, pull) - if err != nil { - return false, err - } + err = db.WithTx(func(ctx context.Context) error { + lastCommitStatus, err := pull_service.GetPullRequestCommitStatusState(ctx, pull) + if err != nil { + return err + } - // we don't need to schedule - if lastCommitStatus.IsSuccess() { - return false, nil - } + // we don't need to schedule + if lastCommitStatus.IsSuccess() { + return nil + } - return true, pull_model.ScheduleAutoMerge(ctx, doer, pull.ID, style, message) + if err := pull_model.ScheduleAutoMerge(ctx, doer, pull.ID, style, message); err != nil { + return err + } + scheduled = true + + pr, err := models.GetPullRequestByID(ctx, pull.ID) + if err != nil { + return err + } + + _, err = models.CreateAutoMergeComment(ctx, models.CommentTypePRScheduledToAutoMerge, pr, doer) + return err + }, ctx) + return } // MergeScheduledPullRequest merges a previously scheduled pull request when all checks succeeded diff --git a/services/pull/automerge.go b/services/pull/automerge.go new file mode 100644 index 0000000000000..3fe0a2fcf25d1 --- /dev/null +++ b/services/pull/automerge.go @@ -0,0 +1,36 @@ +// Copyright 2022 Gitea. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package pull + +import ( + "context" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/models/db" + pull_model "code.gitea.io/gitea/models/pull" + user_model "code.gitea.io/gitea/models/user" +) + +// RemoveScheduledAutoMerge cancels a previously scheduled pull request +func RemoveScheduledAutoMerge(ctx context.Context, doer *user_model.User, pullID int64, comment bool) error { + return db.WithTx(func(ctx context.Context) error { + if err := pull_model.DeleteScheduledAutoMerge(ctx, pullID); err != nil { + return err + } + + // if pull got merged we don't need to add "auto-merge canceled comment" + if !comment || doer == nil { + return nil + } + + pr, err := models.GetPullRequestByID(ctx, pullID) + if err != nil { + return err + } + + _, err = models.CreateAutoMergeComment(ctx, models.CommentTypePRUnScheduledToAutoMerge, pr, doer) + return err + }, ctx) +} diff --git a/services/pull/merge.go b/services/pull/merge.go index 8cc4d8888845d..b982353d04f57 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -18,7 +18,6 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" - pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" @@ -48,7 +47,7 @@ func Merge(pr *models.PullRequest, doer *user_model.User, baseGitRepo *git.Repos defer pullWorkingPool.CheckOut(fmt.Sprint(pr.ID)) // Removing an auto merge pull and ignore if not exist - if err := pull_model.RemoveScheduledAutoMerge(db.DefaultContext, doer, pr.ID, false); err != nil && !models.IsErrNotExist(err) { + if err := RemoveScheduledAutoMerge(db.DefaultContext, doer, pr.ID, false); err != nil && !db.IsErrNotExist(err) { return err } From e43cbb6e4b551005de8d5a5582585f9c0b15c813 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 7 May 2022 21:39:17 +0200 Subject: [PATCH 05/10] optimize --- routers/api/v1/repo/pull.go | 2 +- services/automerge/automerge.go | 7 +------ services/pull/automerge.go | 11 +++-------- services/pull/merge.go | 2 +- 4 files changed, 6 insertions(+), 16 deletions(-) diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 22bb2b02453c5..a6d499085d7f9 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1196,7 +1196,7 @@ func CancelScheduledAutoMerge(ctx *context.APIContext) { } } - if err := pull_service.RemoveScheduledAutoMerge(ctx, ctx.Doer, pull.ID, true); err != nil { + if err := pull_service.RemoveScheduledAutoMerge(ctx, ctx.Doer, pull, true); err != nil { ctx.InternalServerError(err) } else { ctx.Status(http.StatusNoContent) diff --git a/services/automerge/automerge.go b/services/automerge/automerge.go index 2578a7a70f1b4..d94c1014c8618 100644 --- a/services/automerge/automerge.go +++ b/services/automerge/automerge.go @@ -78,12 +78,7 @@ func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pull *models. } scheduled = true - pr, err := models.GetPullRequestByID(ctx, pull.ID) - if err != nil { - return err - } - - _, err = models.CreateAutoMergeComment(ctx, models.CommentTypePRScheduledToAutoMerge, pr, doer) + _, err = models.CreateAutoMergeComment(ctx, models.CommentTypePRScheduledToAutoMerge, pull, doer) return err }, ctx) return diff --git a/services/pull/automerge.go b/services/pull/automerge.go index 3fe0a2fcf25d1..0f817e426bbe8 100644 --- a/services/pull/automerge.go +++ b/services/pull/automerge.go @@ -14,9 +14,9 @@ import ( ) // RemoveScheduledAutoMerge cancels a previously scheduled pull request -func RemoveScheduledAutoMerge(ctx context.Context, doer *user_model.User, pullID int64, comment bool) error { +func RemoveScheduledAutoMerge(ctx context.Context, doer *user_model.User, pull *models.PullRequest, comment bool) error { return db.WithTx(func(ctx context.Context) error { - if err := pull_model.DeleteScheduledAutoMerge(ctx, pullID); err != nil { + if err := pull_model.DeleteScheduledAutoMerge(ctx, pull.ID); err != nil { return err } @@ -25,12 +25,7 @@ func RemoveScheduledAutoMerge(ctx context.Context, doer *user_model.User, pullID return nil } - pr, err := models.GetPullRequestByID(ctx, pullID) - if err != nil { - return err - } - - _, err = models.CreateAutoMergeComment(ctx, models.CommentTypePRUnScheduledToAutoMerge, pr, doer) + _, err := models.CreateAutoMergeComment(ctx, models.CommentTypePRUnScheduledToAutoMerge, pull, doer) return err }, ctx) } diff --git a/services/pull/merge.go b/services/pull/merge.go index b982353d04f57..142a78cdc8ba9 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -47,7 +47,7 @@ func Merge(pr *models.PullRequest, doer *user_model.User, baseGitRepo *git.Repos defer pullWorkingPool.CheckOut(fmt.Sprint(pr.ID)) // Removing an auto merge pull and ignore if not exist - if err := RemoveScheduledAutoMerge(db.DefaultContext, doer, pr.ID, false); err != nil && !db.IsErrNotExist(err) { + if err := RemoveScheduledAutoMerge(db.DefaultContext, doer, pr, false); err != nil && !db.IsErrNotExist(err) { return err } From 56f3237ac3904da9ae17f22a47baaf3685dab59f Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 7 May 2022 21:46:42 +0200 Subject: [PATCH 06/10] do it right --- routers/api/v1/repo/pull.go | 2 +- services/automerge/automerge.go | 12 ++++++++++++ services/pull/automerge.go | 31 ------------------------------- services/pull/merge.go | 3 ++- 4 files changed, 15 insertions(+), 33 deletions(-) delete mode 100644 services/pull/automerge.go diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index a6d499085d7f9..14469d952aa82 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1196,7 +1196,7 @@ func CancelScheduledAutoMerge(ctx *context.APIContext) { } } - if err := pull_service.RemoveScheduledAutoMerge(ctx, ctx.Doer, pull, true); err != nil { + if err := automerge.RemoveScheduledAutoMerge(ctx, ctx.Doer, pull); err != nil { ctx.InternalServerError(err) } else { ctx.Status(http.StatusNoContent) diff --git a/services/automerge/automerge.go b/services/automerge/automerge.go index d94c1014c8618..684b1cf4bcc47 100644 --- a/services/automerge/automerge.go +++ b/services/automerge/automerge.go @@ -84,6 +84,18 @@ func ScheduleAutoMerge(ctx context.Context, doer *user_model.User, pull *models. return } +// RemoveScheduledAutoMerge cancels a previously scheduled pull request +func RemoveScheduledAutoMerge(ctx context.Context, doer *user_model.User, pull *models.PullRequest) error { + return db.WithTx(func(ctx context.Context) error { + if err := pull_model.DeleteScheduledAutoMerge(ctx, pull.ID); err != nil { + return err + } + + _, err := models.CreateAutoMergeComment(ctx, models.CommentTypePRUnScheduledToAutoMerge, pull, doer) + return err + }, ctx) +} + // MergeScheduledPullRequest merges a previously scheduled pull request when all checks succeeded func MergeScheduledPullRequest(ctx context.Context, sha string, repo *repo_model.Repository) error { pulls, err := getPullRequestsByHeadSHA(ctx, sha, repo, func(pr *models.PullRequest) bool { diff --git a/services/pull/automerge.go b/services/pull/automerge.go deleted file mode 100644 index 0f817e426bbe8..0000000000000 --- a/services/pull/automerge.go +++ /dev/null @@ -1,31 +0,0 @@ -// Copyright 2022 Gitea. All rights reserved. -// Use of this source code is governed by a MIT-style -// license that can be found in the LICENSE file. - -package pull - -import ( - "context" - - "code.gitea.io/gitea/models" - "code.gitea.io/gitea/models/db" - pull_model "code.gitea.io/gitea/models/pull" - user_model "code.gitea.io/gitea/models/user" -) - -// RemoveScheduledAutoMerge cancels a previously scheduled pull request -func RemoveScheduledAutoMerge(ctx context.Context, doer *user_model.User, pull *models.PullRequest, comment bool) error { - return db.WithTx(func(ctx context.Context) error { - if err := pull_model.DeleteScheduledAutoMerge(ctx, pull.ID); err != nil { - return err - } - - // if pull got merged we don't need to add "auto-merge canceled comment" - if !comment || doer == nil { - return nil - } - - _, err := models.CreateAutoMergeComment(ctx, models.CommentTypePRUnScheduledToAutoMerge, pull, doer) - return err - }, ctx) -} diff --git a/services/pull/merge.go b/services/pull/merge.go index 142a78cdc8ba9..5eef60890db53 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/models/db" + pull_model "code.gitea.io/gitea/models/pull" repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unit" user_model "code.gitea.io/gitea/models/user" @@ -47,7 +48,7 @@ func Merge(pr *models.PullRequest, doer *user_model.User, baseGitRepo *git.Repos defer pullWorkingPool.CheckOut(fmt.Sprint(pr.ID)) // Removing an auto merge pull and ignore if not exist - if err := RemoveScheduledAutoMerge(db.DefaultContext, doer, pr, false); err != nil && !db.IsErrNotExist(err) { + if err := pull_model.DeleteScheduledAutoMerge(db.DefaultContext, pr.ID); err != nil && !db.IsErrNotExist(err) { return err } From 8dac07a61b24771a3a04d5dcd69c289c5232561f Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 7 May 2022 21:47:47 +0200 Subject: [PATCH 07/10] Update models/pull/automerge.go --- models/pull/automerge.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/pull/automerge.go b/models/pull/automerge.go index 8d9c0114b6b0c..d0aca2e85f976 100644 --- a/models/pull/automerge.go +++ b/models/pull/automerge.go @@ -84,7 +84,7 @@ func GetScheduledMergeByPullID(ctx context.Context, pullID int64) (bool, *AutoMe return true, scheduledPRM, nil } -// DeleteScheduledAutoMerge cancels a previously scheduled pull request +// DeleteScheduledAutoMerge delete a scheduled pull request func DeleteScheduledAutoMerge(ctx context.Context, pullID int64) error { exist, scheduledPRM, err := GetScheduledMergeByPullID(ctx, pullID) if err != nil { From a94d9c159aebc6f3196cee5bd06a0d8abdbd18ff Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 7 May 2022 22:20:38 +0200 Subject: [PATCH 08/10] add insurance to models/issue_comment.go Co-authored-by: delvh --- models/issue_comment.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/models/issue_comment.go b/models/issue_comment.go index db8ae9dce5386..2dff58c5a08b6 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -1362,6 +1362,9 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *Pul // CreateAutoMergeComment is a internal function, only use it for CommentTypePRScheduledToAutoMerge and CommentTypePRUnScheduledToAutoMerge CommentTypes func CreateAutoMergeComment(ctx context.Context, typ CommentType, pr *PullRequest, doer *user_model.User) (comment *Comment, err error) { + if typ != CommentType.CommentTypePRScheduledToAutoMerge && typ != CommentType.CommentTypePRUnScheduledToAutoMerge { + return nil, fmt.Errorf("comment type %d cannot be used to create an auto merge comment", typ) + } if err = pr.LoadIssueCtx(ctx); err != nil { return } From 9b8abff1a97c55d0b7543c18193eb49c23463d75 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 7 May 2022 22:31:17 +0200 Subject: [PATCH 09/10] fix --- models/issue_comment.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/issue_comment.go b/models/issue_comment.go index 2dff58c5a08b6..2cf3d5a61d50d 100644 --- a/models/issue_comment.go +++ b/models/issue_comment.go @@ -1362,7 +1362,7 @@ func CreatePushPullComment(ctx context.Context, pusher *user_model.User, pr *Pul // CreateAutoMergeComment is a internal function, only use it for CommentTypePRScheduledToAutoMerge and CommentTypePRUnScheduledToAutoMerge CommentTypes func CreateAutoMergeComment(ctx context.Context, typ CommentType, pr *PullRequest, doer *user_model.User) (comment *Comment, err error) { - if typ != CommentType.CommentTypePRScheduledToAutoMerge && typ != CommentType.CommentTypePRUnScheduledToAutoMerge { + if typ != CommentTypePRScheduledToAutoMerge && typ != CommentTypePRUnScheduledToAutoMerge { return nil, fmt.Errorf("comment type %d cannot be used to create an auto merge comment", typ) } if err = pr.LoadIssueCtx(ctx); err != nil { From 45239d655edc65f80c6a56db97ade56b99308cf7 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sun, 8 May 2022 15:18:29 +0200 Subject: [PATCH 10/10] add index --- models/migrations/v215.go | 2 +- models/pull/review_state.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/models/migrations/v215.go b/models/migrations/v215.go index 138917edbe771..d65488a18126f 100644 --- a/models/migrations/v215.go +++ b/models/migrations/v215.go @@ -15,7 +15,7 @@ func addReviewViewedFiles(x *xorm.Engine) error { type ReviewState struct { ID int64 `xorm:"pk autoincr"` UserID int64 `xorm:"NOT NULL UNIQUE(pull_commit_user)"` - PullID int64 `xorm:"NOT NULL UNIQUE(pull_commit_user) DEFAULT 0"` + PullID int64 `xorm:"NOT NULL INDEX UNIQUE(pull_commit_user) DEFAULT 0"` CommitSHA string `xorm:"NOT NULL VARCHAR(40) UNIQUE(pull_commit_user)"` UpdatedFiles map[string]pull.ViewedState `xorm:"NOT NULL LONGTEXT JSON"` UpdatedUnix timeutil.TimeStamp `xorm:"updated"` diff --git a/models/pull/review_state.go b/models/pull/review_state.go index 59a03c20e8e32..1c465bf766772 100644 --- a/models/pull/review_state.go +++ b/models/pull/review_state.go @@ -38,10 +38,10 @@ func (viewedState ViewedState) String() string { type ReviewState struct { ID int64 `xorm:"pk autoincr"` UserID int64 `xorm:"NOT NULL UNIQUE(pull_commit_user)"` - PullID int64 `xorm:"NOT NULL UNIQUE(pull_commit_user) DEFAULT 0"` // Which PR was the review on? - CommitSHA string `xorm:"NOT NULL VARCHAR(40) UNIQUE(pull_commit_user)"` // Which commit was the head commit for the review? - 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 - 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 + PullID int64 `xorm:"NOT NULL INDEX UNIQUE(pull_commit_user) DEFAULT 0"` // Which PR was the review on? + CommitSHA string `xorm:"NOT NULL VARCHAR(40) UNIQUE(pull_commit_user)"` // Which commit was the head commit for the review? + 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 + 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 } func init() {