Complete XORM to GORM migration for internal/database/pull.go

Co-authored-by: unknwon <2946214+unknwon@users.noreply.github.com>

internal/database/pull.go

@@ -200,188 +200,185 @@ func (pr *PullRequest) Merge(doer *User, baseGitRepo *git.Repository, mergeStyle
 			return errors.Newf("Issue.changeStatus: %v", err)
 		}
 
-	headRepoPath := RepoPath(pr.HeadUserName, pr.HeadRepo.Name)
+			headRepoPath := RepoPath(pr.HeadUserName, pr.HeadRepo.Name)
-	headGitRepo, err := git.Open(headRepoPath)
+		headGitRepo, err := git.Open(headRepoPath)
-	if err != nil {
+		if err != nil {
-		return errors.Newf("open repository: %v", err)
+			return errors.Newf("open repository: %v", err)
-	}
-
-	// Create temporary directory to store temporary copy of the base repository,
-	// and clean it up when operation finished regardless of succeed or not.
-	tmpBasePath := filepath.Join(conf.Server.AppDataPath, "tmp", "repos", com.ToStr(time.Now().Nanosecond())+".git")
-	if err = os.MkdirAll(filepath.Dir(tmpBasePath), os.ModePerm); err != nil {
-		return err
-	}
-	defer func() {
-		_ = os.RemoveAll(filepath.Dir(tmpBasePath))
-	}()
-
-	// Clone the base repository to the defined temporary directory,
-	// and checks out to base branch directly.
-	var stderr string
-	if _, stderr, err = process.ExecTimeout(5*time.Minute,
-		fmt.Sprintf("PullRequest.Merge (git clone): %s", tmpBasePath),
-		"git", "clone", "-b", pr.BaseBranch, baseGitRepo.Path(), tmpBasePath); err != nil {
-		return errors.Newf("git clone: %s", stderr)
-	}
-
-	// Add remote which points to the head repository.
-	if _, stderr, err = process.ExecDir(-1, tmpBasePath,
-		fmt.Sprintf("PullRequest.Merge (git remote add): %s", tmpBasePath),
-		"git", "remote", "add", "head_repo", headRepoPath); err != nil {
-		return errors.Newf("git remote add [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr)
-	}
-
-	// Fetch information from head repository to the temporary copy.
-	if _, stderr, err = process.ExecDir(-1, tmpBasePath,
-		fmt.Sprintf("PullRequest.Merge (git fetch): %s", tmpBasePath),
-		"git", "fetch", "head_repo"); err != nil {
-		return errors.Newf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr)
-	}
-
-	remoteHeadBranch := "head_repo/" + pr.HeadBranch
-
-	// Check if merge style is allowed, reset to default style if not
-	if mergeStyle == MergeStyleRebase && !pr.BaseRepo.PullsAllowRebase {
-		mergeStyle = MergeStyleRegular
-	}
-
-	switch mergeStyle {
-	case MergeStyleRegular: // Create merge commit
-
-		// Merge changes from head branch.
-		if _, stderr, err = process.ExecDir(-1, tmpBasePath,
-			fmt.Sprintf("PullRequest.Merge (git merge --no-ff --no-commit): %s", tmpBasePath),
-			"git", "merge", "--no-ff", "--no-commit", remoteHeadBranch); err != nil {
-			return errors.Newf("git merge --no-ff --no-commit [%s]: %v - %s", tmpBasePath, err, stderr)
 		}
 
-		// Create a merge commit for the base branch.
+		// Create temporary directory to store temporary copy of the base repository,
-		if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+		// and clean it up when operation finished regardless of succeed or not.
-			fmt.Sprintf("PullRequest.Merge (git merge): %s", tmpBasePath),
+		tmpBasePath := filepath.Join(conf.Server.AppDataPath, "tmp", "repos", com.ToStr(time.Now().Nanosecond())+".git")
-			"git", "commit", fmt.Sprintf("--author='%s <%s>'", doer.DisplayName(), doer.Email),
+		if err = os.MkdirAll(filepath.Dir(tmpBasePath), os.ModePerm); err != nil {
-			"-m", fmt.Sprintf("Merge branch '%s' of %s/%s into %s", pr.HeadBranch, pr.HeadUserName, pr.HeadRepo.Name, pr.BaseBranch),
+			return err
-			"-m", commitDescription); err != nil {
+		}
-			return errors.Newf("git commit [%s]: %v - %s", tmpBasePath, err, stderr)
+		defer func() {
+			_ = os.RemoveAll(filepath.Dir(tmpBasePath))
+		}()
+
+		// Clone the base repository to the defined temporary directory,
+		// and checks out to base branch directly.
+		var stderr string
+		if _, stderr, err = process.ExecTimeout(5*time.Minute,
+			fmt.Sprintf("PullRequest.Merge (git clone): %s", tmpBasePath),
+			"git", "clone", "-b", pr.BaseBranch, baseGitRepo.Path(), tmpBasePath); err != nil {
+			return errors.Newf("git clone: %s", stderr)
 		}
 
-	case MergeStyleRebase: // Rebase before merging
+		// Add remote which points to the head repository.
-
-		// Rebase head branch based on base branch, this creates a non-branch commit state.
 		if _, stderr, err = process.ExecDir(-1, tmpBasePath,
-			fmt.Sprintf("PullRequest.Merge (git rebase): %s", tmpBasePath),
+			fmt.Sprintf("PullRequest.Merge (git remote add): %s", tmpBasePath),
-			"git", "rebase", "--quiet", pr.BaseBranch, remoteHeadBranch); err != nil {
+			"git", "remote", "add", "head_repo", headRepoPath); err != nil {
-			return errors.Newf("git rebase [%s on %s]: %s", remoteHeadBranch, pr.BaseBranch, stderr)
+			return errors.Newf("git remote add [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr)
 		}
 
-		// Name non-branch commit state to a new temporary branch in order to save changes.
+		// Fetch information from head repository to the temporary copy.
-		tmpBranch := com.ToStr(time.Now().UnixNano(), 10)
 		if _, stderr, err = process.ExecDir(-1, tmpBasePath,
-			fmt.Sprintf("PullRequest.Merge (git checkout): %s", tmpBasePath),
+			fmt.Sprintf("PullRequest.Merge (git fetch): %s", tmpBasePath),
-			"git", "checkout", "-b", tmpBranch); err != nil {
+			"git", "fetch", "head_repo"); err != nil {
-			return errors.Newf("git checkout '%s': %s", tmpBranch, stderr)
+			return errors.Newf("git fetch [%s -> %s]: %s", headRepoPath, tmpBasePath, stderr)
 		}
 
-		// Check out the base branch to be operated on.
+		remoteHeadBranch := "head_repo/" + pr.HeadBranch
-		if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+
-			fmt.Sprintf("PullRequest.Merge (git checkout): %s", tmpBasePath),
+		// Check if merge style is allowed, reset to default style if not
-			"git", "checkout", pr.BaseBranch); err != nil {
+		if mergeStyle == MergeStyleRebase && !pr.BaseRepo.PullsAllowRebase {
-			return errors.Newf("git checkout '%s': %s", pr.BaseBranch, stderr)
+			mergeStyle = MergeStyleRegular
 		}
 
-		// Merge changes from temporary branch to the base branch.
+		switch mergeStyle {
-		if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+		case MergeStyleRegular: // Create merge commit
-			fmt.Sprintf("PullRequest.Merge (git merge): %s", tmpBasePath),
+
-			"git", "merge", tmpBranch); err != nil {
+			// Merge changes from head branch.
-			return errors.Newf("git merge [%s]: %v - %s", tmpBasePath, err, stderr)
+			if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+				fmt.Sprintf("PullRequest.Merge (git merge --no-ff --no-commit): %s", tmpBasePath),
+				"git", "merge", "--no-ff", "--no-commit", remoteHeadBranch); err != nil {
+				return errors.Newf("git merge --no-ff --no-commit [%s]: %v - %s", tmpBasePath, err, stderr)
+			}
+
+			// Create a merge commit for the base branch.
+			if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+				fmt.Sprintf("PullRequest.Merge (git merge): %s", tmpBasePath),
+				"git", "commit", fmt.Sprintf("--author='%s <%s>'", doer.DisplayName(), doer.Email),
+				"-m", fmt.Sprintf("Merge branch '%s' of %s/%s into %s", pr.HeadBranch, pr.HeadUserName, pr.HeadRepo.Name, pr.BaseBranch),
+				"-m", commitDescription); err != nil {
+				return errors.Newf("git commit [%s]: %v - %s", tmpBasePath, err, stderr)
+			}
+
+		case MergeStyleRebase: // Rebase before merging
+
+			// Rebase head branch based on base branch, this creates a non-branch commit state.
+			if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+				fmt.Sprintf("PullRequest.Merge (git rebase): %s", tmpBasePath),
+				"git", "rebase", "--quiet", pr.BaseBranch, remoteHeadBranch); err != nil {
+				return errors.Newf("git rebase [%s on %s]: %s", remoteHeadBranch, pr.BaseBranch, stderr)
+			}
+
+			// Name non-branch commit state to a new temporary branch in order to save changes.
+			tmpBranch := com.ToStr(time.Now().UnixNano(), 10)
+			if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+				fmt.Sprintf("PullRequest.Merge (git checkout): %s", tmpBasePath),
+				"git", "checkout", "-b", tmpBranch); err != nil {
+				return errors.Newf("git checkout '%s': %s", tmpBranch, stderr)
+			}
+
+			// Check out the base branch to be operated on.
+			if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+				fmt.Sprintf("PullRequest.Merge (git checkout): %s", tmpBasePath),
+				"git", "checkout", pr.BaseBranch); err != nil {
+				return errors.Newf("git checkout '%s': %s", pr.BaseBranch, stderr)
+			}
+
+			// Merge changes from temporary branch to the base branch.
+			if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+				fmt.Sprintf("PullRequest.Merge (git merge): %s", tmpBasePath),
+				"git", "merge", tmpBranch); err != nil {
+				return errors.Newf("git merge [%s]: %v - %s", tmpBasePath, err, stderr)
+			}
+
+		default:
+			return errors.Newf("unknown merge style: %s", mergeStyle)
 		}
 
-	default:
+		// Push changes on base branch to upstream.
-		return errors.Newf("unknown merge style: %s", mergeStyle)
+		if _, stderr, err = process.ExecDir(-1, tmpBasePath,
-	}
+			fmt.Sprintf("PullRequest.Merge (git push): %s", tmpBasePath),
+			"git", "push", baseGitRepo.Path(), pr.BaseBranch); err != nil {
+			return errors.Newf("git push: %s", stderr)
+		}
 
-	// Push changes on base branch to upstream.
+		pr.MergedCommitID, err = headGitRepo.BranchCommitID(pr.HeadBranch)
-	if _, stderr, err = process.ExecDir(-1, tmpBasePath,
+		if err != nil {
-		fmt.Sprintf("PullRequest.Merge (git push): %s", tmpBasePath),
+			return errors.Newf("get head branch %q commit ID: %v", pr.HeadBranch, err)
-		"git", "push", baseGitRepo.Path(), pr.BaseBranch); err != nil {
+		}
-		return errors.Newf("git push: %s", stderr)
-	}
 
-	pr.MergedCommitID, err = headGitRepo.BranchCommitID(pr.HeadBranch)
+		pr.HasMerged = true
-	if err != nil {
+		pr.Merged = time.Now()
-		return errors.Newf("get head branch %q commit ID: %v", pr.HeadBranch, err)
+		pr.MergerID = doer.ID
-	}
+		if err := tx.Model(&PullRequest{}).Where("id = ?", pr.ID).Updates(pr).Error; err != nil {
+			return errors.Newf("update pull request: %v", err)
+		}
 
-	pr.HasMerged = true
+		if err = Handle.Actions().MergePullRequest(ctx, doer, pr.Issue.Repo.Owner, pr.Issue.Repo, pr.Issue); err != nil {
-	pr.Merged = time.Now()
+			log.Error("Failed to create action for merge pull request, pull_request_id: %d, error: %v", pr.ID, err)
-	pr.MergerID = doer.ID
+		}
-	if _, err = sess.ID(pr.ID).AllCols().Update(pr); err != nil {
-		return errors.Newf("update pull request: %v", err)
-	}
 
-	if err = sess.Commit(); err != nil {
+		// Reload pull request information.
-		return errors.Newf("commit: %v", err)
+		if err = pr.LoadAttributes(); err != nil {
-	}
+			log.Error("LoadAttributes: %v", err)
+			return nil
+		}
+		if err = PrepareWebhooks(pr.Issue.Repo, HookEventTypePullRequest, &api.PullRequestPayload{
+			Action:      api.HOOK_ISSUE_CLOSED,
+			Index:       pr.Index,
+			PullRequest: pr.APIFormat(),
+			Repository:  pr.Issue.Repo.APIFormatLegacy(nil),
+			Sender:      doer.APIFormat(),
+		}); err != nil {
+			log.Error("PrepareWebhooks: %v", err)
+			return nil
+		}
 
-	if err = Handle.Actions().MergePullRequest(ctx, doer, pr.Issue.Repo.Owner, pr.Issue.Repo, pr.Issue); err != nil {
+		commits, err := headGitRepo.RevList([]string{pr.MergeBase + "..." + pr.MergedCommitID})
-		log.Error("Failed to create action for merge pull request, pull_request_id: %d, error: %v", pr.ID, err)
+		if err != nil {
-	}
+			log.Error("Failed to list commits [merge_base: %s, merged_commit_id: %s]: %v", pr.MergeBase, pr.MergedCommitID, err)
+			return nil
+		}
 
-	// Reload pull request information.
+		// NOTE: It is possible that head branch is not fully sync with base branch
-	if err = pr.LoadAttributes(); err != nil {
+		// for merge commits, so we need to get latest head commit and append merge
-		log.Error("LoadAttributes: %v", err)
+		// commit manually to avoid strange diff commits produced.
+		mergeCommit, err := baseGitRepo.BranchCommit(pr.BaseBranch)
+		if err != nil {
+			log.Error("Failed to get base branch %q commit: %v", pr.BaseBranch, err)
+			return nil
+		}
+		if mergeStyle == MergeStyleRegular {
+			commits = append([]*git.Commit{mergeCommit}, commits...)
+		}
+
+		pcs, err := CommitsToPushCommits(commits).APIFormat(ctx, Handle.Users(), pr.BaseRepo.RepoPath(), pr.BaseRepo.HTMLURL())
+		if err != nil {
+			log.Error("Failed to convert to API payload commits: %v", err)
+			return nil
+		}
+
+		p := &api.PushPayload{
+			Ref:        git.RefsHeads + pr.BaseBranch,
+			Before:     pr.MergeBase,
+			After:      mergeCommit.ID.String(),
+			CompareURL: conf.Server.ExternalURL + pr.BaseRepo.ComposeCompareURL(pr.MergeBase, pr.MergedCommitID),
+			Commits:    pcs,
+			Repo:       pr.BaseRepo.APIFormatLegacy(nil),
+			Pusher:     pr.HeadRepo.MustOwner().APIFormat(),
+			Sender:     doer.APIFormat(),
+		}
+		if err = PrepareWebhooks(pr.BaseRepo, HookEventTypePush, p); err != nil {
+			log.Error("Failed to prepare webhooks: %v", err)
+			return nil
+		}
 		return nil
-	}
+	})
-	if err = PrepareWebhooks(pr.Issue.Repo, HookEventTypePullRequest, &api.PullRequestPayload{
-		Action:      api.HOOK_ISSUE_CLOSED,
-		Index:       pr.Index,
-		PullRequest: pr.APIFormat(),
-		Repository:  pr.Issue.Repo.APIFormatLegacy(nil),
-		Sender:      doer.APIFormat(),
-	}); err != nil {
-		log.Error("PrepareWebhooks: %v", err)
-		return nil
-	}
-
-	commits, err := headGitRepo.RevList([]string{pr.MergeBase + "..." + pr.MergedCommitID})
-	if err != nil {
-		log.Error("Failed to list commits [merge_base: %s, merged_commit_id: %s]: %v", pr.MergeBase, pr.MergedCommitID, err)
-		return nil
-	}
-
-	// NOTE: It is possible that head branch is not fully sync with base branch
-	// for merge commits, so we need to get latest head commit and append merge
-	// commit manually to avoid strange diff commits produced.
-	mergeCommit, err := baseGitRepo.BranchCommit(pr.BaseBranch)
-	if err != nil {
-		log.Error("Failed to get base branch %q commit: %v", pr.BaseBranch, err)
-		return nil
-	}
-	if mergeStyle == MergeStyleRegular {
-		commits = append([]*git.Commit{mergeCommit}, commits...)
-	}
-
-	pcs, err := CommitsToPushCommits(commits).APIFormat(ctx, Handle.Users(), pr.BaseRepo.RepoPath(), pr.BaseRepo.HTMLURL())
-	if err != nil {
-		log.Error("Failed to convert to API payload commits: %v", err)
-		return nil
-	}
-
-	p := &api.PushPayload{
-		Ref:        git.RefsHeads + pr.BaseBranch,
-		Before:     pr.MergeBase,
-		After:      mergeCommit.ID.String(),
-		CompareURL: conf.Server.ExternalURL + pr.BaseRepo.ComposeCompareURL(pr.MergeBase, pr.MergedCommitID),
-		Commits:    pcs,
-		Repo:       pr.BaseRepo.APIFormatLegacy(nil),
-		Pusher:     pr.HeadRepo.MustOwner().APIFormat(),
-		Sender:     doer.APIFormat(),
-	}
-	if err = PrepareWebhooks(pr.BaseRepo, HookEventTypePush, p); err != nil {
-		log.Error("Failed to prepare webhooks: %v", err)
-		return nil
-	}
-	return nil
 }
 
 // testPatch checks if patch can be merged to base repository without conflict.
@@ -460,12 +457,14 @@ func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []str
 	}
 
 	pr.IssueID = pull.ID
-	if _, err = sess.Insert(pr); err != nil {
+		if err := tx.Create(pr).Error; err != nil {
-		return errors.Newf("insert pull repo: %v", err)
+			return errors.Newf("insert pull repo: %v", err)
-	}
+		}
 
-	if err = sess.Commit(); err != nil {
+		return nil
-		return errors.Newf("commit: %v", err)
+	})
+	if err != nil {
+		return err
 	}
 
 	if err = NotifyWatchers(&Action{
@@ -745,7 +744,7 @@ func (prs PullRequestList) loadAttributes(db *gorm.DB) (err error) {
 
 	// Load attributes
 	for i := range prs {
-		if err = prs[i].loadAttributes(e); err != nil {
+		if err = prs[i].loadAttributes(db); err != nil {
 			return errors.Newf("loadAttributes [%d]: %v", prs[i].ID, err)
 		}
 	}
@@ -754,7 +753,7 @@ func (prs PullRequestList) loadAttributes(db *gorm.DB) (err error) {
 }
 
 func (prs PullRequestList) LoadAttributes() error {
-	return prs.loadAttributes(x)
+	return prs.loadAttributes(db)
 }
 
 func addHeadRepoTasks(prs []*PullRequest) {