Skip to content

Commit

Permalink
Display pull request head branch even the branch deleted or repositor…
Browse files Browse the repository at this point in the history 
…y deleted (#10413)

* Display pull request head branch even the branch deleted or repository deleted

* Merge getHeadRepo/loadHeadRepo and getBaseRepo/loadBaseRepo on pull and fill repo when pr.Issue.Repo is available

* retrieve sha from pull head when pull request branch deleted and fix tests

* Fix test

* Ensure MustHeadRepoName returns empty string if no head repo

Co-authored-by: zeripath <[email protected]>
  • Loading branch information
@lunny @zeripath
lunny and zeripath committed Mar 2, 2020
1 parent 22b7507 commit 5abe1c5
Show file tree
Hide file tree
Showing 12 changed files with 184 additions and 180 deletions.
104 changes: 50 additions & 54 deletions models/pull.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,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
Expand All @@ -74,6 +76,9 @@ func (pr *PullRequest) MustHeadUserName() string {
}
return ""
}
if pr.HeadRepo == nil {
return ""
}
return pr.HeadRepo.OwnerName
}

Expand All @@ -97,38 +102,55 @@ 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) (err 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}

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
}
Expand Down Expand Up @@ -414,32 +436,6 @@ func (pr *PullRequest) GetGitRefName() string {
return fmt.Sprintf("refs/pull/%d/head", pr.Index)
}

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)
}
return nil
}

// 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 {
return nil
}

pr.BaseRepo, err = GetRepositoryByID(pr.BaseRepoID)
if err != nil {
return fmt.Errorf("GetRepositoryByID(base): %v", err)
}
return nil
}

// IsChecking returns true if this pull request is still checking conflict.
func (pr *PullRequest) IsChecking() bool {
return pr.Status == PullRequestStatusChecking
Expand All @@ -452,7 +448,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
}

Expand Down Expand Up @@ -774,7 +770,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())
Expand All @@ -786,7 +782,7 @@ 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
}
headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath())
Expand Down
2 changes: 1 addition & 1 deletion models/pull_sign.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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, error) {
if err := pr.GetBaseRepo(); err != nil {
if err := pr.LoadBaseRepo(); err != nil {
log.Error("Unable to get Base Repo for pull request")
return false, "", err
}
Expand Down
10 changes: 5 additions & 5 deletions models/pull_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,21 +29,21 @@ func TestPullRequest_LoadIssue(t *testing.T) {
assert.Equal(t, int64(2), pr.Issue.ID)
}

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)
}
Expand Down
121 changes: 59 additions & 62 deletions modules/convert/pull.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
baseBranch *git.Branch
headBranch *git.Branch
baseCommit *git.Commit
headCommit *git.Commit
err error
)

Expand All @@ -32,20 +31,14 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
}

apiIssue := ToAPIIssue(pr.Issue)
if pr.BaseRepo == nil {
pr.BaseRepo, err = models.GetRepositoryByID(pr.BaseRepoID)
if err != nil {
log.Error("GetRepositoryById[%d]: %v", pr.ID, err)
return nil
}
if err := pr.LoadBaseRepo(); err != nil {
log.Error("GetRepositoryById[%d]: %v", pr.ID, err)
return nil
}
if pr.HeadRepoID != 0 && pr.HeadRepo == nil {
pr.HeadRepo, err = models.GetRepositoryByID(pr.HeadRepoID)
if err != nil && !models.IsErrRepoNotExist(err) {
log.Error("GetRepositoryById[%d]: %v", pr.ID, err)
return nil

}
if err := pr.LoadHeadRepo(); err != nil {
log.Error("GetRepositoryById[%d]: %v", pr.ID, err)
return nil
}

apiPullRequest := &api.PullRequest{
Expand All @@ -69,70 +62,74 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
Deadline: apiIssue.Deadline,
Created: pr.Issue.CreatedUnix.AsTimePtr(),
Updated: pr.Issue.UpdatedUnix.AsTimePtr(),
}
baseBranch, err = repo_module.GetBranch(pr.BaseRepo, 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.APIFormat(models.AccessModeNone),
}
},
Head: &api.PRBranchInfo{
Name: pr.HeadBranch,
Ref: fmt.Sprintf("refs/pull/%d/head", pr.Index),
RepoID: -1,
},
}

baseBranch, err = repo_module.GetBranch(pr.BaseRepo, pr.BaseBranch)
if err != nil && !git.IsErrBranchNotExist(err) {
log.Error("GetBranch[%s]: %v", pr.BaseBranch, err)
return nil
}

if err == nil {
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()
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()
}
apiPullRequest.Base = apiBaseBranchInfo
}

if pr.HeadRepo != nil {
headBranch, err = repo_module.GetBranch(pr.HeadRepo, pr.HeadBranch)
apiPullRequest.Head.RepoID = pr.HeadRepo.ID
apiPullRequest.Head.Repository = pr.HeadRepo.APIFormat(models.AccessModeNone)

headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath())
if err != nil {
if git.IsErrBranchNotExist(err) {
apiPullRequest.Head = nil
} else {
log.Error("GetBranch[%s]: %v", pr.HeadBranch, err)
log.Error("OpenRepository[%s]: %v", pr.HeadRepo.RepoPath(), 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 {
apiHeadBranchInfo := &api.PRBranchInfo{
Name: pr.HeadBranch,
Ref: pr.HeadBranch,
RepoID: pr.HeadRepoID,
Repository: pr.HeadRepo.APIFormat(models.AccessModeNone),
commit, err := headBranch.GetCommit()
if err != nil && !git.IsErrNotExist(err) {
log.Error("GetCommit[%s]: %v", headBranch.Name, err)
return nil
}
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()
if err == nil {
apiPullRequest.Head.Ref = pr.HeadBranch
apiPullRequest.Head.Sha = commit.ID.String()
}
apiPullRequest.Head = apiHeadBranchInfo
}
} else {
apiPullRequest.Head = &api.PRBranchInfo{
Name: pr.HeadBranch,
Ref: fmt.Sprintf("refs/pull/%d/head", pr.Index),
RepoID: -1,
}
}

Expand Down
10 changes: 9 additions & 1 deletion modules/convert/pull_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,27 @@ import (
"testing"

"code.gitea.io/gitea/models"
"code.gitea.io/gitea/modules/structs"

"github.com/stretchr/testify/assert"
)

func TestPullRequest_APIFormat(t *testing.T) {
//with HeadRepo
assert.NoError(t, models.PrepareTestDatabase())
headRepo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 1}).(*models.Repository)
pr := models.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 1}).(*models.PullRequest)
assert.NoError(t, pr.LoadAttributes())
assert.NoError(t, pr.LoadIssue())
apiPullRequest := ToAPIPullRequest(pr)
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)

//withOut HeadRepo
pr = models.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 1}).(*models.PullRequest)
Expand Down

0 comments on commit 5abe1c5

Please sign in to comment.