Skip to content

Commit ac5de13

Browse files
lunnylunny
committed
Merge getHeadRepo/loadHeadRepo and getBaseRepo/loadBaseRepo on pull and fill repo when pr.Issue.Repo is available
1 parent f386cca commit ac5de13

File tree

11 files changed

+148
-177
lines changed

11 files changed

+148
-177
lines changed

‎models/pull.go

Lines changed: 47 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ type PullRequest struct {
6262
MergerID int64 `xorm:"INDEX"`
6363
Merger *User `xorm:"-"`
6464
MergedUnix timeutil.TimeStamp `xorm:"updated INDEX"`
65+
66+
isHeadRepoLoaded bool `xorm:"-"`
6567
}
6668

6769
// MustHeadUserName returns the HeadRepo's username if failed return blank
@@ -97,38 +99,55 @@ func (pr *PullRequest) LoadAttributes() error {
9799
return pr.loadAttributes(x)
98100
}
99101

100-
// LoadBaseRepo loads pull request base repository from database
101-
func (pr *PullRequest) LoadBaseRepo() error {
102-
if pr.BaseRepo == nil {
103-
if pr.HeadRepoID == pr.BaseRepoID && pr.HeadRepo != nil {
104-
pr.BaseRepo = pr.HeadRepo
105-
return nil
102+
func (pr *PullRequest) loadHeadRepo(e Engine) (err error) {
103+
if !pr.isHeadRepoLoaded && pr.HeadRepo == nil && pr.HeadRepoID > 0 {
104+
if pr.HeadRepoID == pr.BaseRepoID {
105+
if pr.BaseRepo != nil {
106+
pr.HeadRepo = pr.BaseRepo
107+
return nil
108+
} else if pr.Issue != nil && pr.Issue.Repo != nil {
109+
pr.HeadRepo = pr.Issue.Repo
110+
return nil
111+
}
106112
}
107-
var repo Repository
108-
if has, err := x.ID(pr.BaseRepoID).Get(&repo); err != nil {
109-
return err
110-
} else if !has {
111-
return ErrRepoNotExist{ID: pr.BaseRepoID}
113+
114+
pr.HeadRepo, err = getRepositoryByID(e, pr.HeadRepoID)
115+
if err != nil && !IsErrRepoNotExist(err) { // Head repo maybe deleted, but it should still work
116+
return fmt.Errorf("getRepositoryByID(head): %v", err)
112117
}
113-
pr.BaseRepo = &repo
118+
pr.isHeadRepoLoaded = true
114119
}
115120
return nil
116121
}
117122

118-
// LoadHeadRepo loads pull request head repository from database
123+
// LoadHeadRepo loads the head repository
119124
func (pr *PullRequest) LoadHeadRepo() error {
120-
if pr.HeadRepo == nil {
121-
if pr.HeadRepoID == pr.BaseRepoID && pr.BaseRepo != nil {
122-
pr.HeadRepo = pr.BaseRepo
123-
return nil
124-
}
125-
var repo Repository
126-
if has, err := x.ID(pr.HeadRepoID).Get(&repo); err != nil {
127-
return err
128-
} else if !has {
129-
return ErrRepoNotExist{ID: pr.HeadRepoID}
130-
}
131-
pr.HeadRepo = &repo
125+
return pr.loadHeadRepo(x)
126+
}
127+
128+
// LoadBaseRepo loads the target repository
129+
func (pr *PullRequest) LoadBaseRepo() error {
130+
return pr.loadBaseRepo(x)
131+
}
132+
133+
func (pr *PullRequest) loadBaseRepo(e Engine) (err error) {
134+
if pr.BaseRepo != nil {
135+
return nil
136+
}
137+
138+
if pr.HeadRepoID == pr.BaseRepoID && pr.HeadRepo != nil {
139+
pr.BaseRepo = pr.HeadRepo
140+
return nil
141+
}
142+
143+
if pr.Issue != nil && pr.Issue.Repo != nil {
144+
pr.BaseRepo = pr.Issue.Repo
145+
return nil
146+
}
147+
148+
pr.BaseRepo, err = getRepositoryByID(e, pr.BaseRepoID)
149+
if err != nil {
150+
return fmt.Errorf("GetRepositoryByID(base): %v", err)
132151
}
133152
return nil
134153
}
@@ -414,32 +433,6 @@ func (pr *PullRequest) GetGitRefName() string {
414433
return fmt.Sprintf("refs/pull/%d/head", pr.Index)
415434
}
416435

417-
func (pr *PullRequest) getHeadRepo(e Engine) (err error) {
418-
pr.HeadRepo, err = getRepositoryByID(e, pr.HeadRepoID)
419-
if err != nil && !IsErrRepoNotExist(err) {
420-
return fmt.Errorf("getRepositoryByID(head): %v", err)
421-
}
422-
return nil
423-
}
424-
425-
// GetHeadRepo loads the head repository
426-
func (pr *PullRequest) GetHeadRepo() error {
427-
return pr.getHeadRepo(x)
428-
}
429-
430-
// GetBaseRepo loads the target repository
431-
func (pr *PullRequest) GetBaseRepo() (err error) {
432-
if pr.BaseRepo != nil {
433-
return nil
434-
}
435-
436-
pr.BaseRepo, err = GetRepositoryByID(pr.BaseRepoID)
437-
if err != nil {
438-
return fmt.Errorf("GetRepositoryByID(base): %v", err)
439-
}
440-
return nil
441-
}
442-
443436
// IsChecking returns true if this pull request is still checking conflict.
444437
func (pr *PullRequest) IsChecking() bool {
445438
return pr.Status == PullRequestStatusChecking
@@ -452,7 +445,7 @@ func (pr *PullRequest) CanAutoMerge() bool {
452445

453446
// GetLastCommitStatus returns the last commit status for this pull request.
454447
func (pr *PullRequest) GetLastCommitStatus() (status *CommitStatus, err error) {
455-
if err = pr.GetHeadRepo(); err != nil {
448+
if err = pr.LoadHeadRepo(); err != nil {
456449
return nil, err
457450
}
458451

@@ -774,7 +767,7 @@ func (pr *PullRequest) GetWorkInProgressPrefix() string {
774767
// IsHeadEqualWithBranch returns if the commits of branchName are available in pull request head
775768
func (pr *PullRequest) IsHeadEqualWithBranch(branchName string) (bool, error) {
776769
var err error
777-
if err = pr.GetBaseRepo(); err != nil {
770+
if err = pr.LoadBaseRepo(); err != nil {
778771
return false, err
779772
}
780773
baseGitRepo, err := git.OpenRepository(pr.BaseRepo.RepoPath())
@@ -786,7 +779,7 @@ func (pr *PullRequest) IsHeadEqualWithBranch(branchName string) (bool, error) {
786779
return false, err
787780
}
788781

789-
if err = pr.GetHeadRepo(); err != nil {
782+
if err = pr.LoadHeadRepo(); err != nil {
790783
return false, err
791784
}
792785
headGitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath())

‎models/pull_sign.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212

1313
// SignMerge determines if we should sign a PR merge commit to the base repository
1414
func (pr *PullRequest) SignMerge(u *User, tmpBasePath, baseCommit, headCommit string) (bool, string, error) {
15-
if err := pr.GetBaseRepo(); err != nil {
15+
if err := pr.LoadBaseRepo(); err != nil {
1616
log.Error("Unable to get Base Repo for pull request")
1717
return false, "", err
1818
}

‎models/pull_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,21 +29,21 @@ func TestPullRequest_LoadIssue(t *testing.T) {
2929
assert.Equal(t, int64(2), pr.Issue.ID)
3030
}
3131

32-
func TestPullRequest_GetBaseRepo(t *testing.T) {
32+
func TestPullRequest_LoadBaseRepo(t *testing.T) {
3333
assert.NoError(t, PrepareTestDatabase())
3434
pr := AssertExistsAndLoadBean(t, &PullRequest{ID: 1}).(*PullRequest)
35-
assert.NoError(t, pr.GetBaseRepo())
35+
assert.NoError(t, pr.LoadBaseRepo())
3636
assert.NotNil(t, pr.BaseRepo)
3737
assert.Equal(t, pr.BaseRepoID, pr.BaseRepo.ID)
38-
assert.NoError(t, pr.GetBaseRepo())
38+
assert.NoError(t, pr.LoadBaseRepo())
3939
assert.NotNil(t, pr.BaseRepo)
4040
assert.Equal(t, pr.BaseRepoID, pr.BaseRepo.ID)
4141
}
4242

43-
func TestPullRequest_GetHeadRepo(t *testing.T) {
43+
func TestPullRequest_LoadHeadRepo(t *testing.T) {
4444
assert.NoError(t, PrepareTestDatabase())
4545
pr := AssertExistsAndLoadBean(t, &PullRequest{ID: 1}).(*PullRequest)
46-
assert.NoError(t, pr.GetHeadRepo())
46+
assert.NoError(t, pr.LoadHeadRepo())
4747
assert.NotNil(t, pr.HeadRepo)
4848
assert.Equal(t, pr.HeadRepoID, pr.HeadRepo.ID)
4949
}

‎modules/convert/pull.go

Lines changed: 46 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -32,20 +32,14 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
3232
}
3333

3434
apiIssue := pr.Issue.APIFormat()
35-
if pr.BaseRepo == nil {
36-
pr.BaseRepo, err = models.GetRepositoryByID(pr.BaseRepoID)
37-
if err != nil {
38-
log.Error("GetRepositoryById[%d]: %v", pr.ID, err)
39-
return nil
40-
}
35+
if err := pr.LoadBaseRepo(); err != nil {
36+
log.Error("GetRepositoryById[%d]: %v", pr.ID, err)
37+
return nil
4138
}
42-
if pr.HeadRepoID != 0 && pr.HeadRepo == nil {
43-
pr.HeadRepo, err = models.GetRepositoryByID(pr.HeadRepoID)
44-
if err != nil && !models.IsErrRepoNotExist(err) {
45-
log.Error("GetRepositoryById[%d]: %v", pr.ID, err)
46-
return nil
4739

48-
}
40+
if err := pr.LoadHeadRepo(); err != nil {
41+
log.Error("GetRepositoryById[%d]: %v", pr.ID, err)
42+
return nil
4943
}
5044

5145
apiPullRequest := &api.PullRequest{
@@ -69,70 +63,59 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
6963
Deadline: apiIssue.Deadline,
7064
Created: pr.Issue.CreatedUnix.AsTimePtr(),
7165
Updated: pr.Issue.UpdatedUnix.AsTimePtr(),
72-
}
73-
baseBranch, err = repo_module.GetBranch(pr.BaseRepo, pr.BaseBranch)
74-
if err != nil {
75-
if git.IsErrBranchNotExist(err) {
76-
apiPullRequest.Base = nil
77-
} else {
78-
log.Error("GetBranch[%s]: %v", pr.BaseBranch, err)
79-
return nil
80-
}
81-
} else {
82-
apiBaseBranchInfo := &api.PRBranchInfo{
66+
67+
Base: &api.PRBranchInfo{
8368
Name: pr.BaseBranch,
8469
Ref: pr.BaseBranch,
8570
RepoID: pr.BaseRepoID,
8671
Repository: pr.BaseRepo.APIFormat(models.AccessModeNone),
87-
}
72+
},
73+
Head: &api.PRBranchInfo{
74+
Name: pr.HeadBranch,
75+
Ref: fmt.Sprintf("refs/pull/%d/head", pr.Index),
76+
RepoID: -1,
77+
},
78+
}
79+
80+
baseBranch, err = repo_module.GetBranch(pr.BaseRepo, pr.BaseBranch)
81+
if err != nil && !git.IsErrBranchNotExist(err) {
82+
log.Error("GetBranch[%s]: %v", pr.BaseBranch, err)
83+
return nil
84+
}
85+
86+
if err == nil {
8887
baseCommit, err = baseBranch.GetCommit()
89-
if err != nil {
90-
if git.IsErrNotExist(err) {
91-
apiBaseBranchInfo.Sha = ""
92-
} else {
93-
log.Error("GetCommit[%s]: %v", baseBranch.Name, err)
94-
return nil
95-
}
96-
} else {
97-
apiBaseBranchInfo.Sha = baseCommit.ID.String()
88+
if err != nil && !git.IsErrNotExist(err) {
89+
log.Error("GetCommit[%s]: %v", baseBranch.Name, err)
90+
return nil
91+
}
92+
93+
if err == nil {
94+
apiPullRequest.Base.Sha = baseCommit.ID.String()
9895
}
99-
apiPullRequest.Base = apiBaseBranchInfo
10096
}
10197

10298
if pr.HeadRepo != nil {
99+
apiPullRequest.Head.RepoID = pr.HeadRepo.ID
100+
apiPullRequest.Head.Repository = pr.HeadRepo.APIFormat(models.AccessModeNone)
101+
103102
headBranch, err = repo_module.GetBranch(pr.HeadRepo, pr.HeadBranch)
104-
if err != nil {
105-
if git.IsErrBranchNotExist(err) {
106-
apiPullRequest.Head = nil
107-
} else {
108-
log.Error("GetBranch[%s]: %v", pr.HeadBranch, err)
103+
if err != nil && !git.IsErrBranchNotExist(err) {
104+
log.Error("GetBranch[%s]: %v", pr.HeadBranch, err)
105+
return nil
106+
}
107+
108+
if err == nil {
109+
headCommit, err = headBranch.GetCommit()
110+
if err != nil && !git.IsErrNotExist(err) {
111+
log.Error("GetCommit[%s]: %v", headBranch.Name, err)
109112
return nil
110113
}
111-
} else {
112-
apiHeadBranchInfo := &api.PRBranchInfo{
113-
Name: pr.HeadBranch,
114-
Ref: pr.HeadBranch,
115-
RepoID: pr.HeadRepoID,
116-
Repository: pr.HeadRepo.APIFormat(models.AccessModeNone),
117-
}
118-
headCommit, err = headBranch.GetCommit()
119-
if err != nil {
120-
if git.IsErrNotExist(err) {
121-
apiHeadBranchInfo.Sha = ""
122-
} else {
123-
log.Error("GetCommit[%s]: %v", headBranch.Name, err)
124-
return nil
125-
}
126-
} else {
127-
apiHeadBranchInfo.Sha = headCommit.ID.String()
114+
115+
if err == nil {
116+
apiPullRequest.Head.Ref = pr.HeadBranch
117+
apiPullRequest.Head.Sha = headCommit.ID.String()
128118
}
129-
apiPullRequest.Head = apiHeadBranchInfo
130-
}
131-
} else {
132-
apiPullRequest.Head = &api.PRBranchInfo{
133-
Name: pr.HeadBranch,
134-
Ref: fmt.Sprintf("refs/pull/%d/head", pr.Index),
135-
RepoID: -1,
136119
}
137120
}
138121

‎routers/api/v1/repo/pull.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,12 @@ func ListPullRequests(ctx *context.APIContext, form api.ListPullRequestsOptions)
102102
ctx.Error(http.StatusInternalServerError, "LoadAttributes", err)
103103
return
104104
}
105-
if err = prs[i].GetBaseRepo(); err != nil {
106-
ctx.Error(http.StatusInternalServerError, "GetBaseRepo", err)
105+
if err = prs[i].LoadBaseRepo(); err != nil {
106+
ctx.Error(http.StatusInternalServerError, "LoadBaseRepo", err)
107107
return
108108
}
109-
if err = prs[i].GetHeadRepo(); err != nil {
110-
ctx.Error(http.StatusInternalServerError, "GetHeadRepo", err)
109+
if err = prs[i].LoadHeadRepo(); err != nil {
110+
ctx.Error(http.StatusInternalServerError, "LoadHeadRepo", err)
111111
return
112112
}
113113
apiPrs[i] = convert.ToAPIPullRequest(prs[i])
@@ -157,12 +157,12 @@ func GetPullRequest(ctx *context.APIContext) {
157157
return
158158
}
159159

160-
if err = pr.GetBaseRepo(); err != nil {
161-
ctx.Error(http.StatusInternalServerError, "GetBaseRepo", err)
160+
if err = pr.LoadBaseRepo(); err != nil {
161+
ctx.Error(http.StatusInternalServerError, "LoadBaseRepo", err)
162162
return
163163
}
164-
if err = pr.GetHeadRepo(); err != nil {
165-
ctx.Error(http.StatusInternalServerError, "GetHeadRepo", err)
164+
if err = pr.LoadHeadRepo(); err != nil {
165+
ctx.Error(http.StatusInternalServerError, "LoadHeadRepo", err)
166166
return
167167
}
168168
ctx.JSON(http.StatusOK, convert.ToAPIPullRequest(pr))
@@ -582,8 +582,8 @@ func MergePullRequest(ctx *context.APIContext, form auth.MergePullRequestForm) {
582582
return
583583
}
584584

585-
if err = pr.GetHeadRepo(); err != nil {
586-
ctx.ServerError("GetHeadRepo", err)
585+
if err = pr.LoadHeadRepo(); err != nil {
586+
ctx.ServerError("LoadHeadRepo", err)
587587
return
588588
}
589589

‎routers/repo/issue.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -910,8 +910,8 @@ func ViewIssue(ctx *context.Context) {
910910
ctx.Data["AllowMerge"] = false
911911

912912
if ctx.IsSigned {
913-
if err := pull.GetHeadRepo(); err != nil {
914-
log.Error("GetHeadRepo: %v", err)
913+
if err := pull.LoadHeadRepo(); err != nil {
914+
log.Error("LoadHeadRepo: %v", err)
915915
} else if pull.HeadRepo != nil && pull.HeadBranch != pull.HeadRepo.DefaultBranch {
916916
perm, err := models.GetUserRepoPermission(pull.HeadRepo, ctx.User)
917917
if err != nil {
@@ -929,8 +929,8 @@ func ViewIssue(ctx *context.Context) {
929929
}
930930
}
931931

932-
if err := pull.GetBaseRepo(); err != nil {
933-
log.Error("GetBaseRepo: %v", err)
932+
if err := pull.LoadBaseRepo(); err != nil {
933+
log.Error("LoadBaseRepo: %v", err)
934934
}
935935
perm, err := models.GetUserRepoPermission(pull.BaseRepo, ctx.User)
936936
if err != nil {

0 commit comments

Comments
 (0)