From 8bcb3726d49f43cb9b50d8a85ab42ecaf3f193f8 Mon Sep 17 00:00:00 2001 From: Luke Kysow <1034429+lkysow@users.noreply.github.com> Date: Wed, 12 Jun 2019 12:21:47 +0100 Subject: [PATCH] Switch to Bitbucket 2.0 API for mergeable check --- server/events/vcs/bitbucketcloud/client.go | 41 +++--- .../events/vcs/bitbucketcloud/client_test.go | 131 ++++++++++++++++++ server/events/vcs/bitbucketcloud/models.go | 7 +- 3 files changed, 157 insertions(+), 22 deletions(-) diff --git a/server/events/vcs/bitbucketcloud/client.go b/server/events/vcs/bitbucketcloud/client.go index c45a916721..8667581751 100644 --- a/server/events/vcs/bitbucketcloud/client.go +++ b/server/events/vcs/bitbucketcloud/client.go @@ -127,23 +127,32 @@ func (b *Client) PullIsApproved(repo models.Repo, pull models.PullRequest) (bool // PullIsMergeable returns true if the merge request has no conflicts and can be merged. func (b *Client) PullIsMergeable(repo models.Repo, pull models.PullRequest) (bool, error) { - // NOTE: The 1.0 API is deprecated, but the 2.0 API does not provide this endpoint. - path := fmt.Sprintf("%s/1.0/repositories/%s/pullrequests/%d/conflict-status", b.BaseURL, repo.FullName, pull.Num) - resp, err := b.makeRequest("GET", path, nil) - if err != nil { - return false, err - } - var conflictStatus ConflictStatus - if err := json.Unmarshal(resp, &conflictStatus); err != nil { - return false, errors.Wrapf(err, "Could not parse response %q", string(resp)) - } - if err := validator.New().Struct(conflictStatus); err != nil { - return false, errors.Wrapf(err, "API response %q was missing fields", string(resp)) - } - if !*conflictStatus.MergeImpossible && !*conflictStatus.IsConflicted { - return true, nil + nextPageURL := fmt.Sprintf("%s/2.0/repositories/%s/pullrequests/%d/diffstat", b.BaseURL, repo.FullName, pull.Num) + // We'll only loop 1000 times as a safety measure. + maxLoops := 1000 + for i := 0; i < maxLoops; i++ { + resp, err := b.makeRequest("GET", nextPageURL, nil) + if err != nil { + return false, err + } + var diffStat DiffStat + if err := json.Unmarshal(resp, &diffStat); err != nil { + return false, errors.Wrapf(err, "Could not parse response %q", string(resp)) + } + if err := validator.New().Struct(diffStat); err != nil { + return false, errors.Wrapf(err, "API response %q was missing fields", string(resp)) + } + for _, v := range diffStat.Values { + if *v.Status == "merge conflict" || *v.Status == "local deleted" { + return false, nil + } + } + if diffStat.Next == nil || *diffStat.Next == "" { + break + } + nextPageURL = *diffStat.Next } - return false, nil + return true, nil } // UpdateStatus updates the status of a commit. diff --git a/server/events/vcs/bitbucketcloud/client_test.go b/server/events/vcs/bitbucketcloud/client_test.go index d9fda3f5a5..7973fca674 100644 --- a/server/events/vcs/bitbucketcloud/client_test.go +++ b/server/events/vcs/bitbucketcloud/client_test.go @@ -213,3 +213,134 @@ func TestClient_PullIsApproved(t *testing.T) { }) } } + +func TestClient_PullIsMergeable(t *testing.T) { + cases := map[string]struct { + DiffStat string + ExpMergeable bool + }{ + "mergeable": { + DiffStat: `{ + "pagelen": 500, + "values": [ + { + "status": "added", + "old": null, + "lines_removed": 0, + "lines_added": 2, + "new": { + "path": "parent/child/file1.txt", + "type": "commit_file", + "links": { + "self": { + "href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example/src/1ed8205eec00dab4f1c0a8c486a4492c98c51f8e/main.tf" + } + } + }, + "type": "diffstat" + } + ], + "page": 1, + "size": 1 + }`, + ExpMergeable: true, + }, + "merge conflict": { + DiffStat: `{ + "pagelen": 500, + "values": [ + { + "status": "merge conflict", + "old": { + "path": "main.tf", + "type": "commit_file", + "links": { + "self": { + "href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example/src/6d6a8026a788621b37a9ac422a7d0ebb1500e85f/main.tf" + } + } + }, + "lines_removed": 1, + "lines_added": 0, + "new": { + "path": "main.tf", + "type": "commit_file", + "links": { + "self": { + "href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example/src/742e76108714365788f5681e99e4a64f45dce147/main.tf" + } + } + }, + "type": "diffstat" + } + ], + "page": 1, + "size": 1 + }`, + ExpMergeable: false, + }, + "merge conflict due to file deleted": { + DiffStat: `{ + "pagelen": 500, + "values": [ + { + "status": "local deleted", + "old": null, + "lines_removed": 0, + "lines_added": 3, + "new": { + "path": "main.tf", + "type": "commit_file", + "links": { + "self": { + "href": "https://api.bitbucket.org/2.0/repositories/lkysow/atlantis-example/src/3539b9f51c9f91e8f6280e89c62e2673ddc51144/main.tf" + } + } + }, + "type": "diffstat" + } + ], + "page": 1, + "size": 1 + }`, + ExpMergeable: false, + }, + } + + for name, c := range cases { + t.Run(name, func(t *testing.T) { + testServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.RequestURI { + case "/2.0/repositories/owner/repo/pullrequests/1/diffstat": + w.Write([]byte(c.DiffStat)) // nolint: errcheck + return + default: + t.Errorf("got unexpected request at %q", r.RequestURI) + http.Error(w, "not found", http.StatusNotFound) + return + } + })) + defer testServer.Close() + + client := bitbucketcloud.NewClient(http.DefaultClient, "user", "pass", "runatlantis.io") + client.BaseURL = testServer.URL + + actMergeable, err := client.PullIsMergeable(models.Repo{ + FullName: "owner/repo", + Owner: "owner", + Name: "repo", + CloneURL: "", + SanitizedCloneURL: "", + VCSHost: models.VCSHost{ + Type: models.BitbucketCloud, + Hostname: "bitbucket.org", + }, + }, models.PullRequest{ + Num: 1, + }) + Ok(t, err) + Equals(t, c.ExpMergeable, actMergeable) + }) + } + +} diff --git a/server/events/vcs/bitbucketcloud/models.go b/server/events/vcs/bitbucketcloud/models.go index 4ee784be1c..ff7931c029 100644 --- a/server/events/vcs/bitbucketcloud/models.go +++ b/server/events/vcs/bitbucketcloud/models.go @@ -28,6 +28,7 @@ type DiffStat struct { Next *string `json:"next,omitempty"` } type DiffStatValue struct { + Status *string `json:"status,omitempty" validate:"required"` // Old is the old file, this can be null. Old *DiffStatFile `json:"old,omitempty"` // New is the new file, this can be null. @@ -82,12 +83,6 @@ type Comment struct { type CommentContent struct { Raw *string `json:"raw,omitempty" validate:"required"` } - -type ConflictStatus struct { - MergeImpossible *bool `json:"mergeimpossible,omitempty" validate:"required"` - IsConflicted *bool `json:"isconflicted,omitempty" validate:"required"` -} - type Author struct { UUID *string `json:"uuid,omitempty" validate:"required"` }