Delete related PullAutoMerge and ReviewState on User/Repo Deletion (#19649)

* delete pullautomerges on repo/user deletion
* delete reviewstates on repo/user deletion
* optimize automerhe code
* add index to reviewstate

11 files changed, 84 insertions, 80 deletions

diff --git a/models/db/error.go b/models/db/error.go
index f20cc9b4cb..6557229943 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 c29c818589..0dc14c3e31 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 13b2c62546..2cf3d5a61d 100644
--- a/models/issue_comment.go
+++ b/models/issue_comment.go
@@ -1360,6 +1360,28 @@ 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 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 {
+		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 e675c79193..76ff874c59 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/migrations/v215.go b/models/migrations/v215.go
index 138917edbe..d65488a181 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/notification.go b/models/notification.go
index a1248c240b..d0b7852cd2 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.go b/models/pull.go
index fc5c0d61b3..8eab7569cd 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"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/git"
@@ -96,6 +97,25 @@ 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
+	}
+
+	// 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
+}
+
 // 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/pull/automerge.go b/models/pull/automerge.go
index fd73f2b0fb..d0aca2e85f 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 delete a 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/models/pull/review_state.go b/models/pull/review_state.go
index 59a03c20e8..1c465bf766 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() {
diff --git a/models/repo.go b/models/repo.go
index fbc766850d..e20bf90d97 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 {
diff --git a/models/user.go b/models/user.go
index e830779629..11234a881d 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)
 	}