Get modified Azure DevOps PR files from commit and changes API endpoi…

…nt (#1521) * get modified ado pr files from commit and changes endpoint, fixes #1397 * Add missing error handling
runatlantis · May 5, 2021 · 1777be4 · 1777be4
1 parent 79309ed
commit 1777be4
Show file tree

Hide file tree

Showing 2 changed files with 100 additions and 45 deletions.
diff --git a/server/events/vcs/azuredevops_client.go b/server/events/vcs/azuredevops_client.go
@@ -57,33 +57,52 @@ func NewAzureDevopsClient(hostname string, userName string, token string) (*Azur
 // relative to the repo root, e.g. parent/child/file.txt.
 func (g *AzureDevopsClient) GetModifiedFiles(repo models.Repo, pull models.PullRequest) ([]string, error) {
 	var files []string
+	filesSet := make(map[string]bool)
 
-	opts := azuredevops.PullRequestGetOptions{
-		IncludeWorkItemRefs: true,
-	}
 	owner, project, repoName := SplitAzureDevopsRepoFullName(repo.FullName)
-	commitIDResponse, _, _ := g.Client.PullRequests.GetWithRepo(g.ctx, owner, project, repoName, pull.Num, &opts)
 
-	commitID := commitIDResponse.GetLastMergeSourceCommit().GetCommitID()
+	commitRefs, resp, err := g.Client.PullRequests.ListCommits(g.ctx, owner, project, repoName, pull.Num)
+	if err != nil {
+		return nil, errors.Wrap(err, "getting list of pull request commits")
+	}
+	if resp.StatusCode != http.StatusOK {
+		return nil, errors.Wrapf(err, "http response code %d getting list of pull request commits", resp.StatusCode)
+	}
 
-	r, _, _ := g.Client.Git.GetChanges(g.ctx, owner, project, repoName, commitID)
+	for _, ref := range commitRefs {
+		commitID := ref.GetCommitID()
 
-	for _, change := range r.Changes {
-		item := change.GetItem()
-		// Convert the path to a relative path from the repo's root.
-		relativePath := filepath.Clean("./" + item.GetPath())
-		files = append(files, relativePath)
+		r, resp, err := g.Client.Git.GetChanges(g.ctx, owner, project, repoName, commitID)
+		if err != nil {
+			return nil, errors.Wrap(err, "getting commit changes")
+		}
+		if resp.StatusCode != http.StatusOK {
+			return nil, errors.Wrapf(err, "http response code %d getting commit changes", resp.StatusCode)
+		}
 
-		// If the file was renamed, we'll want to run plan in the directory
-		// it was moved from as well.
-		changeType := azuredevops.Rename.String()
-		if change.ChangeType == &changeType {
-			// Convert the path to a relative path from the repo's root.
-			relativePath = filepath.Clean("./" + change.GetSourceServerItem())
-			files = append(files, relativePath)
+		for _, change := range r.Changes {
+			item := change.GetItem()
+			if item.GetGitObjectType() == "blob" {
+				// Convert the path to a relative path from the repo's root.
+				relativePath := filepath.Clean("./" + item.GetPath())
+				filesSet[relativePath] = true
+
+				// If the file was renamed, we'll want to run plan in the directory
+				// it was moved from as well.
+				changeType := azuredevops.Rename.String()
+				if change.ChangeType == &changeType {
+					// Convert the path to a relative path from the repo's root.
+					relativePath = filepath.Clean("./" + change.GetSourceServerItem())
+					filesSet[relativePath] = true
+				}
+			}
 		}
 	}
 
+	for file := range filesSet {
+		files = append(files, file)
+	}
+
 	return files, nil
 }
 

diff --git a/server/events/vcs/azuredevops_client_test.go b/server/events/vcs/azuredevops_client_test.go
@@ -13,7 +13,6 @@ import (
 	"github.com/mcdafydd/go-azuredevops/azuredevops"
 	"github.com/runatlantis/atlantis/server/events/models"
 	"github.com/runatlantis/atlantis/server/events/vcs"
-	"github.com/runatlantis/atlantis/server/events/vcs/fixtures"
 	. "github.com/runatlantis/atlantis/testing"
 )
 
@@ -236,36 +235,73 @@ func TestAzureDevopsClient_UpdateStatus(t *testing.T) {
 // GetModifiedFiles should make multiple requests if more than one page
 // and concat results.
 func TestAzureDevopsClient_GetModifiedFiles(t *testing.T) {
-	itemRespTemplate := `{
-		"changes": [
-	{
-		"item": {
-			"gitObjectType": "blob",
-			"path": "%s",
-			"url": "https://dev.azure.com/fabrikam/_apis/git/repositories/278d5cd2-584d-4b63-824a-2ba458937249/items/MyWebSite/MyWebSite/%s?versionType=Commit"
-		},
-		"changeType": "add"
-	},
-	{
-		"item": {
-			"gitObjectType": "blob",
-			"path": "%s",
-			"url": "https://dev.azure.com/fabrikam/_apis/git/repositories/278d5cd2-584d-4b63-824a-2ba458937249/items/MyWebSite/MyWebSite/%s?versionType=Commit"
+	commitsResp := `{
+		"count": 2,
+		"value": [
+			{
+				"commitId": "f14e5c5227145a262c455b95696ca5647567390e",
+				"author": {
+					"name": "Jamal Hartnett",
+					"email": "fabrikamfiber4@hotmail.com",
+					"date": "2000-01-01T01:00:00Z"
+				},
+				"committer": {
+					"name": "Jamal Hartnett",
+					"email": "fabrikamfiber4@hotmail.com",
+					"date": "2000-01-01T01:00:00Z"
+				},
+				"comment": "foo bar 1",
+				"url": "https://dev.azure.com/fabrikam/1fbf87b9-64d3-4dea-94ff-352ce2647578/_apis/git/repositories/278d5cd2-584d-4b63-824a-2ba458937249/commits/f14e5c5227145a262c455b95696ca5647567390e"
+			},
+			{
+				"commitId": "39480e5a326c08131981f0bc9d6d411441ad73be",
+				"author": {
+					"name": "Jamal Hartnett",
+					"email": "fabrikamfiber4@hotmail.com",
+					"date": "2000-01-01T01:00:00Z"
+				},
+				"committer": {
+					"name": "Jamal Hartnett",
+					"email": "fabrikamfiber4@hotmail.com",
+					"date": "2000-01-01T01:00:00Z"
+				},
+				"comment": "foo bar 2",
+				"url": "https://dev.azure.com/fabrikam/1fbf87b9-64d3-4dea-94ff-352ce2647578/_apis/git/repositories/278d5cd2-584d-4b63-824a-2ba458937249/commits/39480e5a326c08131981f0bc9d6d411441ad73be"
+			}
+		]}`
+	changesRespTemplate := `{
+		"changeCounts": {
+			"Edit": 1
 		},
-		"changeType": "add"
-	}
-]}`
-	resp := fmt.Sprintf(itemRespTemplate, "/file1.txt", "/file1.txt", "/file2.txt", "/file2.txt")
+		"changes": [
+			{
+				"item": {
+					"objectId": "9347d78903e37be34de91a8c35818561baa6913f",
+					"originalObjectId": "573530927d35a9289083224c429bc15ee7f667e2",
+					"gitObjectType": "blob",
+					"commitId": "%[1]s",
+					"path": "%[2]s",
+					"url": "https://dev.azure.com/fabrikam/1fbf87b9-64d3-4dea-94ff-352ce2647578/_apis/git/repositories/278d5cd2-584d-4b63-824a-2ba458937249/commits/items/%[2]s?versionType=Commit&version=%[1]s"
+				},
+				"changeType": "edit"
+			}
+		]
+	}`
+
 	testServer := httptest.NewTLSServer(
 		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			switch r.RequestURI {
-			// The first request should hit this URL.
-			case "/owner/project/_apis/git/repositories/repo/pullrequests/1?api-version=5.1-preview.1&includeWorkItemRefs=true":
-				w.Write([]byte(fixtures.ADPullJSON)) // nolint: errcheck
-			// The second should hit this URL.
-			case "/owner/project/_apis/git/repositories/repo/commits/b60280bc6e62e2f880f1b63c1e24987664d3bda3/changes?api-version=5.1-preview.1":
-				// We write a header that means there's an additional page.
-				w.Write([]byte(resp)) // nolint: errcheck
+			// The first should hit this URL.
+			case "/owner/project/_apis/git/repositories/repo/pullrequests/1/commits?api-version=5.1-preview.1":
+				w.Write([]byte(commitsResp)) // nolint: errcheck
+				return
+			// get changes for first commit
+			case "/owner/project/_apis/git/repositories/repo/commits/f14e5c5227145a262c455b95696ca5647567390e/changes?api-version=5.1-preview.1":
+				w.Write([]byte(fmt.Sprintf(changesRespTemplate, "f14e5c5227145a262c455b95696ca5647567390e", "/file1.txt"))) // nolint: errcheck
+				return
+			// get changes for second commit
+			case "/owner/project/_apis/git/repositories/repo/commits/39480e5a326c08131981f0bc9d6d411441ad73be/changes?api-version=5.1-preview.1":
+				w.Write([]byte(fmt.Sprintf(changesRespTemplate, "39480e5a326c08131981f0bc9d6d411441ad73be", "/file2.txt"))) // nolint: errcheck
 				return
 			default:
 				t.Errorf("got unexpected request at %q", r.RequestURI)