From 1abcd2fda1a149869610ceec3d0ca1c688f90e9e Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Tue, 3 Mar 2020 11:40:39 +0800 Subject: [PATCH] Show deleted pull head information --- models/pull.go | 273 ++++++++++++++++++-------------- models/pull_sign.go | 2 +- models/pull_test.go | 20 ++- routers/api/v1/repo/pull.go | 24 +-- routers/repo/issue.go | 4 +- routers/repo/pull.go | 56 ++++--- routers/user/setting/profile.go | 2 +- services/pull/commit_status.go | 3 + services/pull/merge.go | 13 +- services/pull/pull.go | 5 +- services/pull/review.go | 4 +- services/pull/temp_repo.go | 17 +- 12 files changed, 233 insertions(+), 190 deletions(-) diff --git a/models/pull.go b/models/pull.go index ea9986dcf..ef5f39b2e 100644 --- a/models/pull.go +++ b/models/pull.go @@ -17,6 +17,23 @@ import ( "code.gitea.io/gitea/modules/timeutil" ) +// ErrHeadRepoMissed represents an error to report pull's head repository missed +type ErrHeadRepoMissed struct { + PullID int64 + HeadRepoID int64 +} + +// Error implements the interface +func (err ErrHeadRepoMissed) Error() string { + return fmt.Sprintf("Head repository [%d] of pull id [%d] missed", err.HeadRepoID, err.PullID) +} + +// IsErrHeadRepoMissed returns true if head repository missed +func IsErrHeadRepoMissed(err error) bool { + _, ok := err.(ErrHeadRepoMissed) + return ok +} + // PullRequestType defines pull request type type PullRequestType int @@ -63,6 +80,8 @@ type PullRequest struct { MergerID int64 `xorm:"INDEX"` Merger *User `xorm:"-"` MergedUnix timeutil.TimeStamp `xorm:"updated INDEX"` + + isHeadRepoLoaded bool `xorm:"-"` } // MustHeadUserName returns the HeadRepo's username if failed return blank @@ -75,6 +94,9 @@ func (pr *PullRequest) MustHeadUserName() string { } return "" } + if pr.HeadRepo == nil { + return "" + } return pr.HeadRepo.MustOwnerName() } @@ -98,38 +120,57 @@ func (pr *PullRequest) LoadAttributes() error { return pr.loadAttributes(x) } -// LoadBaseRepo loads pull request base repository from database -func (pr *PullRequest) LoadBaseRepo() error { - if pr.BaseRepo == nil { - if pr.HeadRepoID == pr.BaseRepoID && pr.HeadRepo != nil { - pr.BaseRepo = pr.HeadRepo - return nil +func (pr *PullRequest) loadHeadRepo(e Engine) error { + if !pr.isHeadRepoLoaded && pr.HeadRepo == nil && pr.HeadRepoID > 0 { + if pr.HeadRepoID == pr.BaseRepoID { + if pr.BaseRepo != nil { + pr.HeadRepo = pr.BaseRepo + return nil + } else if pr.Issue != nil && pr.Issue.Repo != nil { + pr.HeadRepo = pr.Issue.Repo + return nil + } } - var repo Repository - if has, err := x.ID(pr.BaseRepoID).Get(&repo); err != nil { - return err - } else if !has { - return ErrRepoNotExist{ID: pr.BaseRepoID} + + var err error + pr.HeadRepo, err = getRepositoryByID(e, pr.HeadRepoID) + if err != nil && !IsErrRepoNotExist(err) { // Head repo maybe deleted, but it should still work + return fmt.Errorf("getRepositoryByID(head): %v", err) + } - pr.BaseRepo = &repo + pr.isHeadRepoLoaded = true } return nil } -// LoadHeadRepo loads pull request head repository from database +// LoadHeadRepo loads the head repository func (pr *PullRequest) LoadHeadRepo() error { - if pr.HeadRepo == nil { - if pr.HeadRepoID == pr.BaseRepoID && pr.BaseRepo != nil { - pr.HeadRepo = pr.BaseRepo - return nil - } - var repo Repository - if has, err := x.ID(pr.HeadRepoID).Get(&repo); err != nil { - return err - } else if !has { - return ErrRepoNotExist{ID: pr.HeadRepoID} - } - pr.HeadRepo = &repo + return pr.loadHeadRepo(x) +} + +// LoadBaseRepo loads the target repository +func (pr *PullRequest) LoadBaseRepo() error { + return pr.loadBaseRepo(x) +} + +func (pr *PullRequest) loadBaseRepo(e Engine) (err error) { + if pr.BaseRepo != nil { + return nil + } + + if pr.HeadRepoID == pr.BaseRepoID && pr.HeadRepo != nil { + pr.BaseRepo = pr.HeadRepo + return nil + } + + if pr.Issue != nil && pr.Issue.Repo != nil { + pr.BaseRepo = pr.Issue.Repo + return nil + } + + pr.BaseRepo, err = getRepositoryByID(e, pr.BaseRepoID) + if err != nil { + return fmt.Errorf("GetRepositoryByID(base): %v", err) } return nil } @@ -425,7 +466,6 @@ func (pr *PullRequest) apiFormat(e Engine) *api.PullRequest { baseBranch *git.Branch headBranch *git.Branch baseCommit *git.Commit - headCommit *git.Commit err error ) if err = pr.Issue.loadRepo(e); err != nil { @@ -433,19 +473,14 @@ func (pr *PullRequest) apiFormat(e Engine) *api.PullRequest { return nil } apiIssue := pr.Issue.apiFormat(e) - if pr.BaseRepo == nil { - pr.BaseRepo, err = getRepositoryByID(e, pr.BaseRepoID) - if err != nil { - log.Error("GetRepositoryById[%d]: %v", pr.ID, err) - return nil - } + if err := pr.loadBaseRepo(e); err != nil { + log.Error("loadBaseRepo[%d]: %v", pr.ID, err) + return nil } - if pr.HeadRepoID != 0 && pr.HeadRepo == nil { - pr.HeadRepo, err = getRepositoryByID(e, pr.HeadRepoID) - if err != nil && !IsErrRepoNotExist(err) { - log.Error("GetRepositoryById[%d]: %v", pr.ID, err) - return nil - } + + if err := pr.loadHeadRepo(e); err != nil { + log.Error("loadHeadRepo[%d]: %v", pr.ID, err) + return nil } apiPullRequest := &api.PullRequest{ @@ -469,71 +504,17 @@ func (pr *PullRequest) apiFormat(e Engine) *api.PullRequest { Deadline: apiIssue.Deadline, Created: pr.Issue.CreatedUnix.AsTimePtr(), Updated: pr.Issue.UpdatedUnix.AsTimePtr(), - } - baseBranch, err = pr.BaseRepo.GetBranch(pr.BaseBranch) - if err != nil { - if git.IsErrBranchNotExist(err) { - apiPullRequest.Base = nil - } else { - log.Error("GetBranch[%s]: %v", pr.BaseBranch, err) - return nil - } - } else { - apiBaseBranchInfo := &api.PRBranchInfo{ + Base: &api.PRBranchInfo{ Name: pr.BaseBranch, Ref: pr.BaseBranch, RepoID: pr.BaseRepoID, Repository: pr.BaseRepo.innerAPIFormat(e, AccessModeNone, false), - } - baseCommit, err = baseBranch.GetCommit() - if err != nil { - if git.IsErrNotExist(err) { - apiBaseBranchInfo.Sha = "" - } else { - log.Error("GetCommit[%s]: %v", baseBranch.Name, err) - return nil - } - } else { - apiBaseBranchInfo.Sha = baseCommit.ID.String() - } - apiPullRequest.Base = apiBaseBranchInfo - } - - if pr.HeadRepo != nil { - headBranch, err = pr.HeadRepo.GetBranch(pr.HeadBranch) - if err != nil { - if git.IsErrBranchNotExist(err) { - apiPullRequest.Head = nil - } else { - log.Error("GetBranch[%s]: %v", pr.HeadBranch, err) - return nil - } - } else { - apiHeadBranchInfo := &api.PRBranchInfo{ - Name: pr.HeadBranch, - Ref: pr.HeadBranch, - RepoID: pr.HeadRepoID, - Repository: pr.HeadRepo.innerAPIFormat(e, AccessModeNone, false), - } - headCommit, err = headBranch.GetCommit() - if err != nil { - if git.IsErrNotExist(err) { - apiHeadBranchInfo.Sha = "" - } else { - log.Error("GetCommit[%s]: %v", headBranch.Name, err) - return nil - } - } else { - apiHeadBranchInfo.Sha = headCommit.ID.String() - } - apiPullRequest.Head = apiHeadBranchInfo - } - } else { - apiPullRequest.Head = &api.PRBranchInfo{ + }, + Head: &api.PRBranchInfo{ Name: pr.HeadBranch, Ref: fmt.Sprintf("refs/pull/%d/head", pr.Index), RepoID: -1, - } + }, } if pr.Status != PullRequestStatusChecking { @@ -546,33 +527,78 @@ func (pr *PullRequest) apiFormat(e Engine) *api.PullRequest { apiPullRequest.MergedBy = pr.Merger.APIFormat() } - return apiPullRequest -} - -func (pr *PullRequest) getHeadRepo(e Engine) (err error) { - pr.HeadRepo, err = getRepositoryByID(e, pr.HeadRepoID) - if err != nil && !IsErrRepoNotExist(err) { - return fmt.Errorf("getRepositoryByID(head): %v", err) + baseRepoPath := pr.BaseRepo.repoPath(e) + baseGitRepo, err := git.OpenRepository(baseRepoPath) + if err != nil { + log.Error("OpenRepository[%s]: %v", baseRepoPath, err) + return nil } - return nil -} + defer baseGitRepo.Close() -// GetHeadRepo loads the head repository -func (pr *PullRequest) GetHeadRepo() error { - return pr.getHeadRepo(x) -} - -// GetBaseRepo loads the target repository -func (pr *PullRequest) GetBaseRepo() (err error) { - if pr.BaseRepo != nil { + baseBranch, err = baseGitRepo.GetBranch(pr.BaseBranch) + if err != nil && !git.IsErrBranchNotExist(err) { + log.Error("GetBranch[%s]: %v", pr.BaseBranch, err) return nil } - pr.BaseRepo, err = GetRepositoryByID(pr.BaseRepoID) - if err != nil { - return fmt.Errorf("GetRepositoryByID(base): %v", err) + if err == nil { + baseCommit, err = baseBranch.GetCommit() + if err != nil && !git.IsErrNotExist(err) { + log.Error("GetCommit[%s]: %v", baseBranch.Name, err) + return nil + } + + if err == nil { + apiPullRequest.Base.Sha = baseCommit.ID.String() + } } - return nil + + if pr.HeadRepo != nil { + apiPullRequest.Head.RepoID = pr.HeadRepo.ID + apiPullRequest.Head.Repository = pr.HeadRepo.innerAPIFormat(e, AccessModeNone, false) + + var headGitRepo *git.Repository + if pr.HeadRepoID == pr.BaseRepoID { + headGitRepo = baseGitRepo + } else { + headRepoPath := pr.HeadRepo.repoPath(e) + headGitRepo, err = git.OpenRepository(headRepoPath) + if err != nil { + log.Error("OpenRepository[%s]: %v", headRepoPath, err) + return nil + } + defer headGitRepo.Close() + } + + headBranch, err = headGitRepo.GetBranch(pr.HeadBranch) + if err != nil && !git.IsErrBranchNotExist(err) { + log.Error("GetBranch[%s]: %v", pr.HeadBranch, err) + return nil + } + + if git.IsErrBranchNotExist(err) { + headCommitID, err := headGitRepo.GetRefCommitID(apiPullRequest.Head.Ref) + if err != nil && !git.IsErrNotExist(err) { + log.Error("GetCommit[%s]: %v", headBranch.Name, err) + return nil + } + if err == nil { + apiPullRequest.Head.Sha = headCommitID + } + } else { + commit, err := headBranch.GetCommit() + if err != nil && !git.IsErrNotExist(err) { + log.Error("GetCommit[%s]: %v", headBranch.Name, err) + return nil + } + if err == nil { + apiPullRequest.Head.Ref = pr.HeadBranch + apiPullRequest.Head.Sha = commit.ID.String() + } + } + } + + return apiPullRequest } // IsChecking returns true if this pull request is still checking conflict. @@ -587,7 +613,7 @@ func (pr *PullRequest) CanAutoMerge() bool { // GetLastCommitStatus returns the last commit status for this pull request. func (pr *PullRequest) GetLastCommitStatus() (status *CommitStatus, err error) { - if err = pr.GetHeadRepo(); err != nil { + if err = pr.LoadHeadRepo(); err != nil { return nil, err } @@ -641,8 +667,8 @@ func (pr *PullRequest) CheckUserAllowedToMerge(doer *User) (err error) { } if pr.BaseRepo == nil { - if err = pr.GetBaseRepo(); err != nil { - return fmt.Errorf("GetBaseRepo: %v", err) + if err = pr.LoadBaseRepo(); err != nil { + return fmt.Errorf("LoadBaseRepo: %v", err) } } @@ -921,7 +947,7 @@ func (pr *PullRequest) GetWorkInProgressPrefix() string { // IsHeadEqualWithBranch returns if the commits of branchName are available in pull request head func (pr *PullRequest) IsHeadEqualWithBranch(branchName string) (bool, error) { var err error - if err = pr.GetBaseRepo(); err != nil { + if err = pr.LoadBaseRepo(); err != nil { return false, err } baseGitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) @@ -933,9 +959,12 @@ func (pr *PullRequest) IsHeadEqualWithBranch(branchName string) (bool, error) { return false, err } - if err = pr.GetHeadRepo(); err != nil { + if err = pr.LoadHeadRepo(); err != nil { return false, err } + if pr.HeadRepo == nil { + return false, ErrHeadRepoMissed{pr.ID, pr.HeadRepoID} + } headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath()) if err != nil { return false, err diff --git a/models/pull_sign.go b/models/pull_sign.go index 19d8907c3..d351a729c 100644 --- a/models/pull_sign.go +++ b/models/pull_sign.go @@ -12,7 +12,7 @@ import ( // SignMerge determines if we should sign a PR merge commit to the base repository func (pr *PullRequest) SignMerge(u *User, tmpBasePath, baseCommit, headCommit string) (bool, string) { - if err := pr.GetBaseRepo(); err != nil { + if err := pr.LoadBaseRepo(); err != nil { log.Error("Unable to get Base Repo for pull request") return false, "" } diff --git a/models/pull_test.go b/models/pull_test.go index 325818e0b..30fa270ea 100644 --- a/models/pull_test.go +++ b/models/pull_test.go @@ -7,6 +7,7 @@ package models import ( "testing" + "code.gitea.io/gitea/modules/structs" "github.com/stretchr/testify/assert" ) @@ -31,29 +32,36 @@ func TestPullRequest_LoadIssue(t *testing.T) { func TestPullRequest_APIFormat(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) + headRepo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository) pr := AssertExistsAndLoadBean(t, &PullRequest{ID: 1}).(*PullRequest) assert.NoError(t, pr.LoadAttributes()) assert.NoError(t, pr.LoadIssue()) apiPullRequest := pr.APIFormat() assert.NotNil(t, apiPullRequest) - assert.Nil(t, apiPullRequest.Head) + assert.EqualValues(t, &structs.PRBranchInfo{ + Name: "branch1", + Ref: "refs/pull/2/head", + Sha: "4a357436d925b5c974181ff12a994538ddc5a269", + RepoID: 1, + Repository: headRepo.APIFormat(models.AccessModeNone), + }, apiPullRequest.Head) } -func TestPullRequest_GetBaseRepo(t *testing.T) { +func TestPullRequest_LoadBaseRepo(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) pr := AssertExistsAndLoadBean(t, &PullRequest{ID: 1}).(*PullRequest) - assert.NoError(t, pr.GetBaseRepo()) + assert.NoError(t, pr.LoadBaseRepo()) assert.NotNil(t, pr.BaseRepo) assert.Equal(t, pr.BaseRepoID, pr.BaseRepo.ID) - assert.NoError(t, pr.GetBaseRepo()) + assert.NoError(t, pr.LoadBaseRepo()) assert.NotNil(t, pr.BaseRepo) assert.Equal(t, pr.BaseRepoID, pr.BaseRepo.ID) } -func TestPullRequest_GetHeadRepo(t *testing.T) { +func TestPullRequest_LoadHeadRepo(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) pr := AssertExistsAndLoadBean(t, &PullRequest{ID: 1}).(*PullRequest) - assert.NoError(t, pr.GetHeadRepo()) + assert.NoError(t, pr.LoadHeadRepo()) assert.NotNil(t, pr.HeadRepo) assert.Equal(t, pr.HeadRepoID, pr.HeadRepo.ID) } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 1f1c3b716..f749f340e 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -93,14 +93,6 @@ func ListPullRequests(ctx *context.APIContext, form api.ListPullRequestsOptions) ctx.Error(http.StatusInternalServerError, "LoadAttributes", err) return } - if err = prs[i].GetBaseRepo(); err != nil { - ctx.Error(http.StatusInternalServerError, "GetBaseRepo", err) - return - } - if err = prs[i].GetHeadRepo(); err != nil { - ctx.Error(http.StatusInternalServerError, "GetHeadRepo", err) - return - } apiPrs[i] = prs[i].APIFormat() } @@ -146,14 +138,6 @@ func GetPullRequest(ctx *context.APIContext) { return } - if err = pr.GetBaseRepo(); err != nil { - ctx.Error(http.StatusInternalServerError, "GetBaseRepo", err) - return - } - if err = pr.GetHeadRepo(); err != nil { - ctx.Error(http.StatusInternalServerError, "GetHeadRepo", err) - return - } ctx.JSON(http.StatusOK, pr.APIFormat()) } @@ -569,8 +553,12 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) { return } - if err = pr.GetHeadRepo(); err != nil { - ctx.ServerError("GetHeadRepo", err) + if err = pr.LoadHeadRepo(); err != nil { + ctx.ServerError("LoadHeadRepo", err) + return + } + if pr.HeadRepo == nil { + ctx.ServerError("LoadHeadRepo", models.ErrHeadRepoMissed{pr.HeadRepoID, pr.ID}) return } diff --git a/routers/repo/issue.go b/routers/repo/issue.go index f69e0bb60..4e921c94a 100644 --- a/routers/repo/issue.go +++ b/routers/repo/issue.go @@ -912,8 +912,8 @@ func ViewIssue(ctx *context.Context) { canDelete := false if ctx.IsSigned { - if err := pull.GetHeadRepo(); err != nil { - log.Error("GetHeadRepo: %v", err) + if err := pull.LoadHeadRepo(); err != nil { + log.Error("LoadHeadRepo: %v", err) } else if pull.HeadRepo != nil && pull.HeadBranch != pull.HeadRepo.DefaultBranch { perm, err := models.GetUserRepoPermission(pull.HeadRepo, ctx.User) if err != nil { diff --git a/routers/repo/pull.go b/routers/repo/pull.go index c8c35ae91..94024ac30 100644 --- a/routers/repo/pull.go +++ b/routers/repo/pull.go @@ -271,8 +271,8 @@ func checkPullInfo(ctx *context.Context) *models.Issue { return nil } - if err = issue.PullRequest.GetHeadRepo(); err != nil { - ctx.ServerError("GetHeadRepo", err) + if err = issue.PullRequest.LoadHeadRepo(); err != nil { + ctx.ServerError("LoadHeadRepo", err) return nil } @@ -330,13 +330,13 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare repo := ctx.Repo.Repository pull := issue.PullRequest - if err := pull.GetHeadRepo(); err != nil { - ctx.ServerError("GetHeadRepo", err) + if err := pull.LoadHeadRepo(); err != nil { + ctx.ServerError("LoadHeadRepo", err) return nil } - if err := pull.GetBaseRepo(); err != nil { - ctx.ServerError("GetBaseRepo", err) + if err := pull.LoadBaseRepo(); err != nil { + ctx.ServerError("LoadBaseRepo", err) return nil } @@ -358,14 +358,17 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare var headBranchSha string // HeadRepo may be missing if pull.HeadRepo != nil { - var err error - - headGitRepo, err := git.OpenRepository(pull.HeadRepo.RepoPath()) - if err != nil { - ctx.ServerError("OpenRepository", err) - return nil + var headGitRepo *git.Repository + if pull.BaseRepoID == pull.HeadRepoID { + headGitRepo = baseGitRepo + } else { + headGitRepo, err = git.OpenRepository(pull.HeadRepo.RepoPath()) + if err != nil { + ctx.ServerError("OpenRepository", err) + return nil + } + defer headGitRepo.Close() } - defer headGitRepo.Close() headBranchExist = headGitRepo.IsBranchExist(pull.HeadBranch) @@ -864,15 +867,15 @@ func CleanUpPullRequest(ctx *context.Context) { return } - if err := pr.GetHeadRepo(); err != nil { - ctx.ServerError("GetHeadRepo", err) + if err := pr.LoadHeadRepo(); err != nil { + ctx.ServerError("LoadHeadRepo", err) return } else if pr.HeadRepo == nil { // Forked repository has already been deleted ctx.NotFound("CleanUpPullRequest", nil) return - } else if err = pr.GetBaseRepo(); err != nil { - ctx.ServerError("GetBaseRepo", err) + } else if err = pr.LoadBaseRepo(); err != nil { + ctx.ServerError("LoadBaseRepo", err) return } else if err = pr.HeadRepo.GetOwner(); err != nil { ctx.ServerError("HeadRepo.GetOwner", err) @@ -891,13 +894,6 @@ func CleanUpPullRequest(ctx *context.Context) { fullBranchName := pr.HeadRepo.Owner.Name + "/" + pr.HeadBranch - gitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath()) - if err != nil { - ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.RepoPath()), err) - return - } - defer gitRepo.Close() - gitBaseRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) if err != nil { ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.BaseRepo.RepoPath()), err) @@ -905,6 +901,18 @@ func CleanUpPullRequest(ctx *context.Context) { } defer gitBaseRepo.Close() + var gitRepo *git.Repository + if pr.HeadRepoID == pr.BaseRepoID { + gitRepo = gitBaseRepo + } else { + gitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath()) + if err != nil { + ctx.ServerError(fmt.Sprintf("OpenRepository[%s]", pr.HeadRepo.RepoPath()), err) + return + } + defer gitRepo.Close() + } + defer func() { ctx.JSON(200, map[string]interface{}{ "redirect": pr.BaseRepo.Link() + "/pulls/" + com.ToStr(issue.Index), diff --git a/routers/user/setting/profile.go b/routers/user/setting/profile.go index 368dc2373..aae3df6be 100644 --- a/routers/user/setting/profile.go +++ b/routers/user/setting/profile.go @@ -209,7 +209,7 @@ func Repos(ctx *context.Context) { if repos[i].IsFork { err := repos[i].GetBaseRepo() if err != nil { - ctx.ServerError("GetBaseRepo", err) + ctx.ServerError("LoadBaseRepo", err) return } err = repos[i].BaseRepo.GetOwner() diff --git a/services/pull/commit_status.go b/services/pull/commit_status.go index a4803bfb9..96f1f9b36 100644 --- a/services/pull/commit_status.go +++ b/services/pull/commit_status.go @@ -95,6 +95,9 @@ func GetPullRequestCommitStatusState(pr *models.PullRequest) (structs.CommitStat if err := pr.LoadHeadRepo(); err != nil { return "", errors.Wrap(err, "LoadHeadRepo") } + if pr.HeadRepo == nil { + return "", models.ErrHeadRepoMissed{pr.ID, pr.HeadRepoID} + } // check if all required status checks are successful headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath()) diff --git a/services/pull/merge.go b/services/pull/merge.go index f9d9a27c9..1653238f7 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -38,12 +38,15 @@ func Merge(pr *models.PullRequest, doer *models.User, baseGitRepo *git.Repositor return fmt.Errorf("Unable to get git version: %v", err) } - if err = pr.GetHeadRepo(); err != nil { - log.Error("GetHeadRepo: %v", err) + if err = pr.LoadBaseRepo(); err != nil { + log.Error("LoadBaseRepo: %v", err) + return fmt.Errorf("LoadBaseRepo: %v", err) + } else if err = pr.LoadHeadRepo(); err != nil { + log.Error("LoadHeadRepo: %v", err) return fmt.Errorf("GetHeadRepo: %v", err) - } else if err = pr.GetBaseRepo(); err != nil { - log.Error("GetBaseRepo: %v", err) - return fmt.Errorf("GetBaseRepo: %v", err) + } + if pr.HeadRepo == nil { + return models.ErrHeadRepoMissed{pr.ID, pr.HeadRepoID} } prUnit, err := pr.BaseRepo.GetUnit(models.UnitTypePullRequests) diff --git a/services/pull/pull.go b/services/pull/pull.go index 2a09c98bf..f6549152a 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -235,8 +235,11 @@ func PushToBaseRepo(pr *models.PullRequest) (err error) { log.Error("Unable to load head repository for PR[%d] Error: %v", pr.ID, err) return err } - headRepoPath := pr.HeadRepo.RepoPath() + if pr.HeadRepo == nil { + return models.ErrHeadRepoMissed{pr.ID, pr.HeadRepoID} + } + headRepoPath := pr.HeadRepo.RepoPath() if err := git.Clone(headRepoPath, tmpBasePath, git.CloneRepoOptions{ Bare: true, Shared: true, diff --git a/services/pull/review.go b/services/pull/review.go index 5b453e87f..d36c5076f 100644 --- a/services/pull/review.go +++ b/services/pull/review.go @@ -111,8 +111,8 @@ func createCodeComment(doer *models.User, repo *models.Repository, issue *models return nil, fmt.Errorf("GetPullRequestByIssueID: %v", err) } pr := issue.PullRequest - if err := pr.GetBaseRepo(); err != nil { - return nil, fmt.Errorf("GetHeadRepo: %v", err) + if err := pr.LoadBaseRepo(); err != nil { + return nil, fmt.Errorf("LoadBaseRepo: %v", err) } gitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath()) if err != nil { diff --git a/services/pull/temp_repo.go b/services/pull/temp_repo.go index bb6ce2921..12ee89ae5 100644 --- a/services/pull/temp_repo.go +++ b/services/pull/temp_repo.go @@ -17,17 +17,18 @@ import ( ) func createTemporaryRepo(pr *models.PullRequest) (string, error) { - if err := pr.GetHeadRepo(); err != nil { - log.Error("GetHeadRepo: %v", err) - return "", fmt.Errorf("GetHeadRepo: %v", err) + if err := pr.LoadHeadRepo(); err != nil { + log.Error("LoadHeadRepo: %v", err) + return "", fmt.Errorf("LoadHeadRepo: %v", err) } else if pr.HeadRepo == nil { log.Error("Pr %d HeadRepo %d does not exist", pr.ID, pr.HeadRepoID) - return "", &models.ErrRepoNotExist{ - ID: pr.HeadRepoID, + return "", &models.ErrHeadRepoMissed{ + PullID: pr.ID, + HeadRepoID: pr.HeadRepoID, } - } else if err := pr.GetBaseRepo(); err != nil { - log.Error("GetBaseRepo: %v", err) - return "", fmt.Errorf("GetBaseRepo: %v", err) + } else if err := pr.LoadBaseRepo(); err != nil { + log.Error("LoadBaseRepo: %v", err) + return "", fmt.Errorf("LoadBaseRepo: %v", err) } else if pr.BaseRepo == nil { log.Error("Pr %d BaseRepo %d does not exist", pr.ID, pr.BaseRepoID) return "", &models.ErrRepoNotExist{