From 8d43563a320ded0ffc0b9d66020ed3451c187259 Mon Sep 17 00:00:00 2001
From: 6543 <6543@obermui.de>
Date: Fri, 31 Jan 2020 22:13:51 +0100
Subject: [PATCH] [BugFix] [API] Pull.API.Convert: Only try to get HeadBranch
 if HeadRepo exist (#10029)

* only try to get HeadBranch if HeadRepo exist

* impruve

* no nil error

* add TEST

* correct error msg
---
 models/user.go               |  3 ++
 modules/convert/pull.go      | 66 ++++++++++++++++++++----------------
 modules/convert/pull_test.go | 13 +++++++
 3 files changed, 53 insertions(+), 29 deletions(-)

diff --git a/models/user.go b/models/user.go
index d10d2cbff..ee66acf1a 100644
--- a/models/user.go
+++ b/models/user.go
@@ -234,6 +234,9 @@ func (u *User) GetEmail() string {
 
 // APIFormat converts a User to api.User
 func (u *User) APIFormat() *api.User {
+	if u == nil {
+		return nil
+	}
 	return &api.User{
 		ID:        u.ID,
 		UserName:  u.Name,
diff --git a/modules/convert/pull.go b/modules/convert/pull.go
index 18b4bc8de..fa22977d0 100644
--- a/modules/convert/pull.go
+++ b/modules/convert/pull.go
@@ -5,6 +5,8 @@
 package convert
 
 import (
+	"fmt"
+
 	"code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/modules/git"
 	"code.gitea.io/gitea/modules/log"
@@ -23,10 +25,12 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
 		headCommit *git.Commit
 		err        error
 	)
+
 	if err = pr.Issue.LoadRepo(); err != nil {
-		log.Error("loadRepo[%d]: %v", pr.ID, err)
+		log.Error("pr.Issue.LoadRepo[%d]: %v", pr.ID, err)
 		return nil
 	}
+
 	apiIssue := pr.Issue.APIFormat()
 	if pr.BaseRepo == nil {
 		pr.BaseRepo, err = models.GetRepositoryByID(pr.BaseRepoID)
@@ -35,17 +39,13 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
 			return nil
 		}
 	}
-	if pr.HeadRepo == nil {
+	if pr.HeadRepoID != 0 && pr.HeadRepo == nil {
 		pr.HeadRepo, err = models.GetRepositoryByID(pr.HeadRepoID)
-		if err != nil {
+		if err != nil && !models.IsErrRepoNotExist(err) {
 			log.Error("GetRepositoryById[%d]: %v", pr.ID, err)
 			return nil
-		}
-	}
 
-	if err = pr.Issue.LoadRepo(); err != nil {
-		log.Error("pr.Issue.loadRepo[%d]: %v", pr.ID, err)
-		return nil
+		}
 	}
 
 	apiPullRequest := &api.PullRequest{
@@ -99,33 +99,41 @@ func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
 		apiPullRequest.Base = apiBaseBranchInfo
 	}
 
-	headBranch, err = repo_module.GetBranch(pr.HeadRepo, 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.APIFormat(models.AccessModeNone),
-		}
-		headCommit, err = headBranch.GetCommit()
+	if pr.HeadRepo != nil {
+		headBranch, err = repo_module.GetBranch(pr.HeadRepo, pr.HeadBranch)
 		if err != nil {
-			if git.IsErrNotExist(err) {
-				apiHeadBranchInfo.Sha = ""
+			if git.IsErrBranchNotExist(err) {
+				apiPullRequest.Head = nil
 			} else {
-				log.Error("GetCommit[%s]: %v", headBranch.Name, err)
+				log.Error("GetBranch[%s]: %v", pr.HeadBranch, err)
 				return nil
 			}
 		} else {
-			apiHeadBranchInfo.Sha = headCommit.ID.String()
+			apiHeadBranchInfo := &api.PRBranchInfo{
+				Name:       pr.HeadBranch,
+				Ref:        pr.HeadBranch,
+				RepoID:     pr.HeadRepoID,
+				Repository: pr.HeadRepo.APIFormat(models.AccessModeNone),
+			}
+			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{
+			Name:   pr.HeadBranch,
+			Ref:    fmt.Sprintf("refs/pull/%d/head", pr.Index),
+			RepoID: -1,
 		}
-		apiPullRequest.Head = apiHeadBranchInfo
 	}
 
 	if pr.Status != models.PullRequestStatusChecking {
diff --git a/modules/convert/pull_test.go b/modules/convert/pull_test.go
index 9055d555e..fe82a61bd 100644
--- a/modules/convert/pull_test.go
+++ b/modules/convert/pull_test.go
@@ -13,6 +13,7 @@ import (
 )
 
 func TestPullRequest_APIFormat(t *testing.T) {
+	//with HeadRepo
 	assert.NoError(t, models.PrepareTestDatabase())
 	pr := models.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 1}).(*models.PullRequest)
 	assert.NoError(t, pr.LoadAttributes())
@@ -20,4 +21,16 @@ func TestPullRequest_APIFormat(t *testing.T) {
 	apiPullRequest := ToAPIPullRequest(pr)
 	assert.NotNil(t, apiPullRequest)
 	assert.Nil(t, apiPullRequest.Head)
+
+	//withOut HeadRepo
+	pr = models.AssertExistsAndLoadBean(t, &models.PullRequest{ID: 1}).(*models.PullRequest)
+	assert.NoError(t, pr.LoadIssue())
+	assert.NoError(t, pr.LoadAttributes())
+	// simulate fork deletion
+	pr.HeadRepo = nil
+	pr.HeadRepoID = 100000
+	apiPullRequest = ToAPIPullRequest(pr)
+	assert.NotNil(t, apiPullRequest)
+	assert.Nil(t, apiPullRequest.Head.Repository)
+	assert.EqualValues(t, -1, apiPullRequest.Head.RepoID)
 }