Switch to Bitbucket 2.0 API for mergeable check

runatlantis · Jun 12, 2019 · 8bcb372 · 8bcb372
1 parent fdf7699
commit 8bcb372
Show file tree

Hide file tree

Showing 3 changed files with 157 additions and 22 deletions.
diff --git 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
@@ -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
@@ -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"`
 }