Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move some pull request functions from models to services #9266

Merged
merged 4 commits into from
Dec 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 5 additions & 126 deletions models/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ package models
import (
"bufio"
"fmt"
"io/ioutil"
"os"
"path"
"path/filepath"
Expand All @@ -20,14 +19,11 @@ import (
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/sync"
"code.gitea.io/gitea/modules/timeutil"

"github.com/unknwon/com"
)

var pullRequestQueue = sync.NewUniqueQueue(setting.Repository.PullRequestQueueLength)

// PullRequestType defines pull request type
type PullRequestType int

Expand Down Expand Up @@ -485,102 +481,6 @@ func (pr *PullRequest) SetMerged() (err error) {
return nil
}

// manuallyMerged checks if a pull request got manually merged
// When a pull request got manually merged mark the pull request as merged
func (pr *PullRequest) manuallyMerged() bool {
commit, err := pr.getMergeCommit()
if err != nil {
log.Error("PullRequest[%d].getMergeCommit: %v", pr.ID, err)
return false
}
if commit != nil {
pr.MergedCommitID = commit.ID.String()
pr.MergedUnix = timeutil.TimeStamp(commit.Author.When.Unix())
pr.Status = PullRequestStatusManuallyMerged
merger, _ := GetUserByEmail(commit.Author.Email)

// When the commit author is unknown set the BaseRepo owner as merger
if merger == nil {
if pr.BaseRepo.Owner == nil {
if err = pr.BaseRepo.getOwner(x); err != nil {
log.Error("BaseRepo.getOwner[%d]: %v", pr.ID, err)
return false
}
}
merger = pr.BaseRepo.Owner
}
pr.Merger = merger
pr.MergerID = merger.ID

if err = pr.SetMerged(); err != nil {
log.Error("PullRequest[%d].setMerged : %v", pr.ID, err)
return false
}
log.Info("manuallyMerged[%d]: Marked as manually merged into %s/%s by commit id: %s", pr.ID, pr.BaseRepo.Name, pr.BaseBranch, commit.ID.String())
return true
}
return false
}

// getMergeCommit checks if a pull request got merged
// Returns the git.Commit of the pull request if merged
func (pr *PullRequest) getMergeCommit() (*git.Commit, error) {
if pr.BaseRepo == nil {
var err error
pr.BaseRepo, err = GetRepositoryByID(pr.BaseRepoID)
if err != nil {
return nil, fmt.Errorf("GetRepositoryByID: %v", err)
}
}

indexTmpPath := filepath.Join(os.TempDir(), "gitea-"+pr.BaseRepo.Name+"-"+strconv.Itoa(time.Now().Nanosecond()))
defer os.Remove(indexTmpPath)

headFile := pr.GetGitRefName()

// Check if a pull request is merged into BaseBranch
_, err := git.NewCommand("merge-base", "--is-ancestor", headFile, pr.BaseBranch).RunInDirWithEnv(pr.BaseRepo.RepoPath(), []string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()})
if err != nil {
// Errors are signaled by a non-zero status that is not 1
if strings.Contains(err.Error(), "exit status 1") {
return nil, nil
}
return nil, fmt.Errorf("git merge-base --is-ancestor: %v", err)
}

commitIDBytes, err := ioutil.ReadFile(pr.BaseRepo.RepoPath() + "/" + headFile)
if err != nil {
return nil, fmt.Errorf("ReadFile(%s): %v", headFile, err)
}
commitID := string(commitIDBytes)
if len(commitID) < 40 {
return nil, fmt.Errorf(`ReadFile(%s): invalid commit-ID "%s"`, headFile, commitID)
}
cmd := commitID[:40] + ".." + pr.BaseBranch

// Get the commit from BaseBranch where the pull request got merged
mergeCommit, err := git.NewCommand("rev-list", "--ancestry-path", "--merges", "--reverse", cmd).RunInDirWithEnv("", []string{"GIT_INDEX_FILE=" + indexTmpPath, "GIT_DIR=" + pr.BaseRepo.RepoPath()})
if err != nil {
return nil, fmt.Errorf("git rev-list --ancestry-path --merges --reverse: %v", err)
} else if len(mergeCommit) < 40 {
// PR was fast-forwarded, so just use last commit of PR
mergeCommit = commitID[:40]
}

gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
if err != nil {
return nil, fmt.Errorf("OpenRepository: %v", err)
}
defer gitRepo.Close()

commit, err := gitRepo.GetCommit(mergeCommit[:40])
if err != nil {
return nil, fmt.Errorf("GetCommit: %v", err)
}

return commit, nil
}

// patchConflicts is a list of conflict description from Git.
var patchConflicts = []string{
"patch does not apply",
Expand All @@ -589,6 +489,11 @@ var patchConflicts = []string{
"error:",
}

// TestPatch checks if patch can be merged to base repository without conflict.
func (pr *PullRequest) TestPatch() error {
return pr.testPatch(x)
}

// testPatch checks if patch can be merged to base repository without conflict.
func (pr *PullRequest) testPatch(e Engine) (err error) {
if pr.BaseRepo == nil {
Expand Down Expand Up @@ -949,32 +854,6 @@ func (pr *PullRequest) PushToBaseRepo() (err error) {
return nil
}

// AddToTaskQueue adds itself to pull request test task queue.
func (pr *PullRequest) AddToTaskQueue() {
go pullRequestQueue.AddFunc(pr.ID, func() {
pr.Status = PullRequestStatusChecking
if err := pr.UpdateCols("status"); err != nil {
log.Error("AddToTaskQueue.UpdateCols[%d].(add to queue): %v", pr.ID, err)
}
})
}

// checkAndUpdateStatus checks if pull request is possible to leaving checking status,
// and set to be either conflict or mergeable.
func (pr *PullRequest) checkAndUpdateStatus() {
// Status is not changed to conflict means mergeable.
if pr.Status == PullRequestStatusChecking {
pr.Status = PullRequestStatusMergeable
}

// Make sure there is no waiting test to process before leaving the checking status.
if !pullRequestQueue.Exist(pr.ID) {
if err := pr.UpdateCols("status, conflicted_files"); err != nil {
log.Error("Update[%d]: %v", pr.ID, err)
}
}
}

// IsWorkInProgress determine if the Pull Request is a Work In Progress by its title
func (pr *PullRequest) IsWorkInProgress() bool {
if err := pr.LoadIssue(); err != nil {
Expand Down
71 changes: 9 additions & 62 deletions models/pull_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"github.com/unknwon/com"

"xorm.io/xorm"
)

Expand Down Expand Up @@ -68,6 +68,14 @@ func GetUnmergedPullRequestsByBaseInfo(repoID int64, branch string) ([]*PullRequ
Find(&prs)
}

// GetPullRequestsByCheckStatus returns all pull requests according the special checking status.
func GetPullRequestsByCheckStatus(status PullRequestStatus) ([]*PullRequest, error) {
prs := make([]*PullRequest, 0, 10)
return prs, x.
Where("status=?", status).
Find(&prs)
}

// PullRequests returns all pull requests for a base Repo by the given conditions
func PullRequests(baseRepoID int64, opts *PullRequestsOptions) ([]*PullRequest, int64, error) {
if opts.Page <= 0 {
Expand Down Expand Up @@ -161,64 +169,3 @@ func (prs PullRequestList) invalidateCodeComments(e Engine, doer *User, repo *gi
func (prs PullRequestList) InvalidateCodeComments(doer *User, repo *git.Repository, branch string) error {
return prs.invalidateCodeComments(x, doer, repo, branch)
}

// TestPullRequests checks and tests untested patches of pull requests.
// TODO: test more pull requests at same time.
func TestPullRequests() {
prs := make([]*PullRequest, 0, 10)

err := x.Where("status = ?", PullRequestStatusChecking).Find(&prs)
if err != nil {
log.Error("Find Checking PRs: %v", err)
return
}

var checkedPRs = make(map[int64]struct{})

// Update pull request status.
for _, pr := range prs {
checkedPRs[pr.ID] = struct{}{}
if err := pr.GetBaseRepo(); err != nil {
log.Error("GetBaseRepo: %v", err)
continue
}
if pr.manuallyMerged() {
continue
}
if err := pr.testPatch(x); err != nil {
log.Error("testPatch: %v", err)
continue
}

pr.checkAndUpdateStatus()
}

// Start listening on new test requests.
for prID := range pullRequestQueue.Queue() {
log.Trace("TestPullRequests[%v]: processing test task", prID)
pullRequestQueue.Remove(prID)

id := com.StrTo(prID).MustInt64()
if _, ok := checkedPRs[id]; ok {
continue
}

pr, err := GetPullRequestByID(id)
if err != nil {
log.Error("GetPullRequestByID[%s]: %v", prID, err)
continue
} else if pr.manuallyMerged() {
continue
} else if err = pr.testPatch(x); err != nil {
log.Error("testPatch[%d]: %v", pr.ID, err)
continue
}

pr.checkAndUpdateStatus()
}
}

// InitTestPullRequests runs the task to test all the checking status pull requests
func InitTestPullRequests() {
go TestPullRequests()
}
20 changes: 0 additions & 20 deletions models/pull_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
package models

import (
"strconv"
"testing"
"time"

"github.com/stretchr/testify/assert"
)
Expand Down Expand Up @@ -196,24 +194,6 @@ func TestPullRequest_UpdateCols(t *testing.T) {

// TODO TestPullRequest_PushToBaseRepo

func TestPullRequest_AddToTaskQueue(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())

pr := AssertExistsAndLoadBean(t, &PullRequest{ID: 1}).(*PullRequest)
pr.AddToTaskQueue()

select {
case id := <-pullRequestQueue.Queue():
assert.EqualValues(t, strconv.FormatInt(pr.ID, 10), id)
case <-time.After(time.Second):
assert.Fail(t, "Timeout: nothing was added to pullRequestQueue")
}

assert.True(t, pullRequestQueue.Exist(pr.ID))
pr = AssertExistsAndLoadBean(t, &PullRequest{ID: 1}).(*PullRequest)
assert.Equal(t, PullRequestStatusChecking, pr.Status)
}

func TestPullRequestList_LoadAttributes(t *testing.T) {
assert.NoError(t, PrepareTestDatabase())

Expand Down
3 changes: 2 additions & 1 deletion routers/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"code.gitea.io/gitea/modules/webhook"
"code.gitea.io/gitea/services/mailer"
mirror_service "code.gitea.io/gitea/services/mirror"
pull_service "code.gitea.io/gitea/services/pull"

"gitea.com/macaron/macaron"
)
Expand Down Expand Up @@ -104,7 +105,7 @@ func GlobalInit() {
models.InitRepoIndexer()
mirror_service.InitSyncMirrors()
webhook.InitDeliverHooks()
models.InitTestPullRequests()
pull_service.Init()
if err := task.Init(); err != nil {
log.Fatal("Failed to initialize task scheduler: %v", err)
}
Expand Down
3 changes: 2 additions & 1 deletion routers/repo/issue.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"code.gitea.io/gitea/modules/util"
comment_service "code.gitea.io/gitea/services/comments"
issue_service "code.gitea.io/gitea/services/issue"
pull_service "code.gitea.io/gitea/services/pull"

"github.com/unknwon/com"
)
Expand Down Expand Up @@ -1272,7 +1273,7 @@ func NewComment(ctx *context.Context, form auth.CreateCommentForm) {
return
}

issue.PullRequest.AddToTaskQueue()
pull_service.AddToTaskQueue(issue.PullRequest)
}
}

Expand Down
Loading