Move pull request api convert to convert package (#9664)

* Move pull request api convert to convert package

* Rename ToPullRequest to ToAPIPullRequest

diff --git a/models/pull.go b/models/pull.go
index a7f51683db435c557aa921afd0ef26a0db54e433..e2a35c59200a784ea7e2ae7148ec3c4b8ee507b8 100644 (file)
--- a/models/pull.go
+++ b/models/pull.go
@@ -13,7 +13,6 @@ import (
        "code.gitea.io/gitea/modules/git"
        "code.gitea.io/gitea/modules/log"
        "code.gitea.io/gitea/modules/setting"
-       api "code.gitea.io/gitea/modules/structs"
        "code.gitea.io/gitea/modules/timeutil"
 )
 
@@ -397,139 +396,6 @@ func (pr *PullRequest) GetGitRefName() string {
        return fmt.Sprintf("refs/pull/%d/head", pr.Index)
 }
 
-// APIFormat assumes following fields have been assigned with valid values:
-// Required - Issue
-// Optional - Merger
-func (pr *PullRequest) APIFormat() *api.PullRequest {
-       return pr.apiFormat(x)
-}
-
-func (pr *PullRequest) apiFormat(e Engine) *api.PullRequest {
-       var (
-               baseBranch *git.Branch
-               headBranch *git.Branch
-               baseCommit *git.Commit
-               headCommit *git.Commit
-               err        error
-       )
-       if err = pr.Issue.loadRepo(e); err != nil {
-               log.Error("loadRepo[%d]: %v", pr.ID, err)
-               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 pr.HeadRepo == nil {
-               pr.HeadRepo, err = getRepositoryByID(e, pr.HeadRepoID)
-               if err != nil {
-                       log.Error("GetRepositoryById[%d]: %v", pr.ID, err)
-                       return nil
-               }
-       }
-
-       if err = pr.Issue.loadRepo(e); err != nil {
-               log.Error("pr.Issue.loadRepo[%d]: %v", pr.ID, err)
-               return nil
-       }
-
-       apiPullRequest := &api.PullRequest{
-               ID:        pr.ID,
-               URL:       pr.Issue.HTMLURL(),
-               Index:     pr.Index,
-               Poster:    apiIssue.Poster,
-               Title:     apiIssue.Title,
-               Body:      apiIssue.Body,
-               Labels:    apiIssue.Labels,
-               Milestone: apiIssue.Milestone,
-               Assignee:  apiIssue.Assignee,
-               Assignees: apiIssue.Assignees,
-               State:     apiIssue.State,
-               Comments:  apiIssue.Comments,
-               HTMLURL:   pr.Issue.HTMLURL(),
-               DiffURL:   pr.Issue.DiffURL(),
-               PatchURL:  pr.Issue.PatchURL(),
-               HasMerged: pr.HasMerged,
-               MergeBase: pr.MergeBase,
-               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{
-                       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
-       }
-
-       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
-       }
-
-       if pr.Status != PullRequestStatusChecking {
-               mergeable := !(pr.Status == PullRequestStatusConflict || pr.Status == PullRequestStatusError) && !pr.IsWorkInProgress()
-               apiPullRequest.Mergeable = mergeable
-       }
-       if pr.HasMerged {
-               apiPullRequest.Merged = pr.MergedUnix.AsTimePtr()
-               apiPullRequest.MergedCommitID = &pr.MergedCommitID
-               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) {

diff --git a/models/pull_test.go b/models/pull_test.go
index 153739f5401ca23e70683f103ff814a87d984cde..9c27b603aa534588eeba7e96b4dcfb48a8bdc6f2 100644 (file)
--- a/models/pull_test.go
+++ b/models/pull_test.go
@@ -29,16 +29,6 @@ func TestPullRequest_LoadIssue(t *testing.T) {
        assert.Equal(t, int64(2), pr.Issue.ID)
 }
 
-func TestPullRequest_APIFormat(t *testing.T) {
-       assert.NoError(t, PrepareTestDatabase())
-       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)
-}
-
 func TestPullRequest_GetBaseRepo(t *testing.T) {
        assert.NoError(t, PrepareTestDatabase())
        pr := AssertExistsAndLoadBean(t, &PullRequest{ID: 1}).(*PullRequest)

diff --git a/modules/convert/main_test.go b/modules/convert/main_test.go
new file mode 100644 (file)
index 0000000..8720ff0
--- /dev/null
+++ b/modules/convert/main_test.go
@@ -0,0 +1,16 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package convert
+
+import (
+       "path/filepath"
+       "testing"
+
+       "code.gitea.io/gitea/models"
+)
+
+func TestMain(m *testing.M) {
+       models.MainTest(m, filepath.Join("..", ".."))
+}

diff --git a/modules/convert/pull.go b/modules/convert/pull.go
new file mode 100644 (file)
index 0000000..f8534f8
--- /dev/null
+++ b/modules/convert/pull.go
@@ -0,0 +1,141 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package convert
+
+import (
+       "code.gitea.io/gitea/models"
+       "code.gitea.io/gitea/modules/git"
+       "code.gitea.io/gitea/modules/log"
+       api "code.gitea.io/gitea/modules/structs"
+)
+
+// ToAPIPullRequest assumes following fields have been assigned with valid values:
+// Required - Issue
+// Optional - Merger
+func ToAPIPullRequest(pr *models.PullRequest) *api.PullRequest {
+       var (
+               baseBranch *git.Branch
+               headBranch *git.Branch
+               baseCommit *git.Commit
+               headCommit *git.Commit
+               err        error
+       )
+       if err = pr.Issue.LoadRepo(); err != nil {
+               log.Error("loadRepo[%d]: %v", pr.ID, err)
+               return nil
+       }
+       apiIssue := pr.Issue.APIFormat()
+       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 pr.HeadRepo == nil {
+               pr.HeadRepo, err = models.GetRepositoryByID(pr.HeadRepoID)
+               if err != nil {
+                       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{
+               ID:        pr.ID,
+               URL:       pr.Issue.HTMLURL(),
+               Index:     pr.Index,
+               Poster:    apiIssue.Poster,
+               Title:     apiIssue.Title,
+               Body:      apiIssue.Body,
+               Labels:    apiIssue.Labels,
+               Milestone: apiIssue.Milestone,
+               Assignee:  apiIssue.Assignee,
+               Assignees: apiIssue.Assignees,
+               State:     apiIssue.State,
+               Comments:  apiIssue.Comments,
+               HTMLURL:   pr.Issue.HTMLURL(),
+               DiffURL:   pr.Issue.DiffURL(),
+               PatchURL:  pr.Issue.PatchURL(),
+               HasMerged: pr.HasMerged,
+               MergeBase: pr.MergeBase,
+               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{
+                       Name:       pr.BaseBranch,
+                       Ref:        pr.BaseBranch,
+                       RepoID:     pr.BaseRepoID,
+                       Repository: pr.BaseRepo.APIFormat(models.AccessModeNone),
+               }
+               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
+       }
+
+       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.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
+       }
+
+       if pr.Status != models.PullRequestStatusChecking {
+               mergeable := !(pr.Status == models.PullRequestStatusConflict || pr.Status == models.PullRequestStatusError) && !pr.IsWorkInProgress()
+               apiPullRequest.Mergeable = mergeable
+       }
+       if pr.HasMerged {
+               apiPullRequest.Merged = pr.MergedUnix.AsTimePtr()
+               apiPullRequest.MergedCommitID = &pr.MergedCommitID
+               apiPullRequest.MergedBy = pr.Merger.APIFormat()
+       }
+
+       return apiPullRequest
+}

diff --git a/modules/convert/pull_test.go b/modules/convert/pull_test.go
new file mode 100644 (file)
index 0000000..9055d55
--- /dev/null
+++ b/modules/convert/pull_test.go
@@ -0,0 +1,23 @@
+// Copyright 2020 The Gitea Authors. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package convert
+
+import (
+       "testing"
+
+       "code.gitea.io/gitea/models"
+
+       "github.com/stretchr/testify/assert"
+)
+
+func TestPullRequest_APIFormat(t *testing.T) {
+       assert.NoError(t, models.PrepareTestDatabase())
+       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)
+}

diff --git a/modules/notification/webhook/webhook.go b/modules/notification/webhook/webhook.go
index e0801445d89084431829b50dd31db9c8fa28fde0..cf4117666cd1d716899d841ab80a5c0840aadbea 100644 (file)
--- a/modules/notification/webhook/webhook.go
+++ b/modules/notification/webhook/webhook.go
@@ -6,6 +6,7 @@ package webhook
 
 import (
        "code.gitea.io/gitea/models"
+       "code.gitea.io/gitea/modules/convert"
        "code.gitea.io/gitea/modules/git"
        "code.gitea.io/gitea/modules/log"
        "code.gitea.io/gitea/modules/notification/base"
@@ -49,7 +50,7 @@ func (m *webhookNotifier) NotifyIssueClearLabels(doer *models.User, issue *model
                err = webhook_module.PrepareWebhooks(issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{
                        Action:      api.HookIssueLabelCleared,
                        Index:       issue.Index,
-                       PullRequest: issue.PullRequest.APIFormat(),
+                       PullRequest: convert.ToAPIPullRequest(issue.PullRequest),
                        Repository:  issue.Repo.APIFormat(mode),
                        Sender:      doer.APIFormat(),
                })
@@ -135,7 +136,7 @@ func (m *webhookNotifier) NotifyIssueChangeAssignee(doer *models.User, issue *mo
                issue.PullRequest.Issue = issue
                apiPullRequest := &api.PullRequestPayload{
                        Index:       issue.Index,
-                       PullRequest: issue.PullRequest.APIFormat(),
+                       PullRequest: convert.ToAPIPullRequest(issue.PullRequest),
                        Repository:  issue.Repo.APIFormat(mode),
                        Sender:      doer.APIFormat(),
                }
@@ -187,7 +188,7 @@ func (m *webhookNotifier) NotifyIssueChangeTitle(doer *models.User, issue *model
                                        From: oldTitle,
                                },
                        },
-                       PullRequest: issue.PullRequest.APIFormat(),
+                       PullRequest: convert.ToAPIPullRequest(issue.PullRequest),
                        Repository:  issue.Repo.APIFormat(mode),
                        Sender:      doer.APIFormat(),
                })
@@ -222,7 +223,7 @@ func (m *webhookNotifier) NotifyIssueChangeStatus(doer *models.User, issue *mode
                // Merge pull request calls issue.changeStatus so we need to handle separately.
                apiPullRequest := &api.PullRequestPayload{
                        Index:       issue.Index,
-                       PullRequest: issue.PullRequest.APIFormat(),
+                       PullRequest: convert.ToAPIPullRequest(issue.PullRequest),
                        Repository:  issue.Repo.APIFormat(mode),
                        Sender:      doer.APIFormat(),
                }
@@ -291,7 +292,7 @@ func (m *webhookNotifier) NotifyNewPullRequest(pull *models.PullRequest) {
        if err := webhook_module.PrepareWebhooks(pull.Issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{
                Action:      api.HookIssueOpened,
                Index:       pull.Issue.Index,
-               PullRequest: pull.APIFormat(),
+               PullRequest: convert.ToAPIPullRequest(pull),
                Repository:  pull.Issue.Repo.APIFormat(mode),
                Sender:      pull.Issue.Poster.APIFormat(),
        }); err != nil {
@@ -312,7 +313,7 @@ func (m *webhookNotifier) NotifyIssueChangeContent(doer *models.User, issue *mod
                                        From: oldContent,
                                },
                        },
-                       PullRequest: issue.PullRequest.APIFormat(),
+                       PullRequest: convert.ToAPIPullRequest(issue.PullRequest),
                        Repository:  issue.Repo.APIFormat(mode),
                        Sender:      doer.APIFormat(),
                })
@@ -439,7 +440,7 @@ func (m *webhookNotifier) NotifyIssueChangeLabels(doer *models.User, issue *mode
                err = webhook_module.PrepareWebhooks(issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{
                        Action:      api.HookIssueLabelUpdated,
                        Index:       issue.Index,
-                       PullRequest: issue.PullRequest.APIFormat(),
+                       PullRequest: convert.ToAPIPullRequest(issue.PullRequest),
                        Repository:  issue.Repo.APIFormat(models.AccessModeNone),
                        Sender:      doer.APIFormat(),
                })
@@ -481,7 +482,7 @@ func (m *webhookNotifier) NotifyIssueChangeMilestone(doer *models.User, issue *m
                err = webhook_module.PrepareWebhooks(issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{
                        Action:      hookAction,
                        Index:       issue.Index,
-                       PullRequest: issue.PullRequest.APIFormat(),
+                       PullRequest: convert.ToAPIPullRequest(issue.PullRequest),
                        Repository:  issue.Repo.APIFormat(mode),
                        Sender:      doer.APIFormat(),
                })
@@ -547,7 +548,7 @@ func (*webhookNotifier) NotifyMergePullRequest(pr *models.PullRequest, doer *mod
        // Merge pull request calls issue.changeStatus so we need to handle separately.
        apiPullRequest := &api.PullRequestPayload{
                Index:       pr.Issue.Index,
-               PullRequest: pr.APIFormat(),
+               PullRequest: convert.ToAPIPullRequest(pr),
                Repository:  pr.Issue.Repo.APIFormat(mode),
                Sender:      doer.APIFormat(),
                Action:      api.HookIssueClosed,
@@ -580,7 +581,7 @@ func (m *webhookNotifier) NotifyPullRequestChangeTargetBranch(doer *models.User,
                                From: oldBranch,
                        },
                },
-               PullRequest: issue.PullRequest.APIFormat(),
+               PullRequest: convert.ToAPIPullRequest(issue.PullRequest),
                Repository:  issue.Repo.APIFormat(mode),
                Sender:      doer.APIFormat(),
        })
@@ -619,7 +620,7 @@ func (m *webhookNotifier) NotifyPullRequestReview(pr *models.PullRequest, review
        if err := webhook_module.PrepareWebhooks(review.Issue.Repo, reviewHookType, &api.PullRequestPayload{
                Action:      api.HookIssueSynchronized,
                Index:       review.Issue.Index,
-               PullRequest: pr.APIFormat(),
+               PullRequest: convert.ToAPIPullRequest(pr),
                Repository:  review.Issue.Repo.APIFormat(mode),
                Sender:      review.Reviewer.APIFormat(),
                Review: &api.ReviewPayload{
@@ -674,7 +675,7 @@ func (m *webhookNotifier) NotifyPullRequestSynchronized(doer *models.User, pr *m
        if err := webhook_module.PrepareWebhooks(pr.Issue.Repo, models.HookEventPullRequest, &api.PullRequestPayload{
                Action:      api.HookIssueSynchronized,
                Index:       pr.Issue.Index,
-               PullRequest: pr.Issue.PullRequest.APIFormat(),
+               PullRequest: convert.ToAPIPullRequest(pr),
                Repository:  pr.Issue.Repo.APIFormat(models.AccessModeNone),
                Sender:      doer.APIFormat(),
        }); err != nil {

diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go
index 85ef41978075e89361f791df6548cafd6e99a715..8e6677116dbeb06eabd0820706a4dff6f02af891 100644 (file)
--- a/routers/api/v1/repo/pull.go
+++ b/routers/api/v1/repo/pull.go
@@ -13,6 +13,7 @@ import (
        "code.gitea.io/gitea/models"
        "code.gitea.io/gitea/modules/auth"
        "code.gitea.io/gitea/modules/context"
+       "code.gitea.io/gitea/modules/convert"
        "code.gitea.io/gitea/modules/git"
        "code.gitea.io/gitea/modules/log"
        "code.gitea.io/gitea/modules/notification"
@@ -102,7 +103,7 @@ func ListPullRequests(ctx *context.APIContext, form api.ListPullRequestsOptions)
                        ctx.Error(http.StatusInternalServerError, "GetHeadRepo", err)
                        return
                }
-               apiPrs[i] = prs[i].APIFormat()
+               apiPrs[i] = convert.ToAPIPullRequest(prs[i])
        }
 
        ctx.SetLinkHeader(int(maxResults), models.ItemsPerPage)
@@ -157,7 +158,7 @@ func GetPullRequest(ctx *context.APIContext) {
                ctx.Error(http.StatusInternalServerError, "GetHeadRepo", err)
                return
        }
-       ctx.JSON(http.StatusOK, pr.APIFormat())
+       ctx.JSON(http.StatusOK, convert.ToAPIPullRequest(pr))
 }
 
 // CreatePullRequest does what it says
@@ -321,7 +322,7 @@ func CreatePullRequest(ctx *context.APIContext, form api.CreatePullRequestOption
        notification.NotifyNewPullRequest(pr)
 
        log.Trace("Pull request created: %d/%d", repo.ID, prIssue.ID)
-       ctx.JSON(http.StatusCreated, pr.APIFormat())
+       ctx.JSON(http.StatusCreated, convert.ToAPIPullRequest(pr))
 }
 
 // EditPullRequest does what it says
@@ -479,7 +480,7 @@ func EditPullRequest(ctx *context.APIContext, form api.EditPullRequestOption) {
        }
 
        // TODO this should be 200, not 201
-       ctx.JSON(http.StatusCreated, pr.APIFormat())
+       ctx.JSON(http.StatusCreated, convert.ToAPIPullRequest(pr))
 }
 
 // IsPullRequestMerged checks if a PR exists given an index