From 3cab1d2c176c38ccf073c0155476af18cd332220 Mon Sep 17 00:00:00 2001 From: mbal Date: Mon, 4 Jan 2021 01:32:04 -0800 Subject: [PATCH 1/3] fixes issues 26 & 46. Added support for pagination --- github.go | 70 ++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 49 insertions(+), 21 deletions(-) diff --git a/github.go b/github.go index 0669639..b3c400e 100644 --- a/github.go +++ b/github.go @@ -106,31 +106,36 @@ func FetchTags(githubRepoUrl string, githubToken string, instance GitHubInstance return tagsString, wrapError(err) } - url := createGitHubRepoUrlForPath(repo, "tags") - resp, err := callGitHubApi(repo, url, map[string]string{}) - if err != nil { - return tagsString, err - } + //per_page is max to reduce network calls + for currPath := "tags?per_page=100"; currPath != ""; { + url := createGitHubRepoUrlForPath(repo, currPath) + resp, err := callGitHubApi(repo, url, map[string]string{}) + if err != nil { + return tagsString, err + } - // Convert the response body to a byte array - buf := new(bytes.Buffer) - _, goErr := buf.ReadFrom(resp.Body) - if goErr != nil { - return tagsString, wrapError(goErr) - } - jsonResp := buf.Bytes() + // Convert the response body to a byte array + buf := new(bytes.Buffer) + _, goErr := buf.ReadFrom(resp.Body) + if goErr != nil { + return tagsString, wrapError(goErr) + } + jsonResp := buf.Bytes() - // Extract the JSON into our array of gitHubTagsCommitApiResponse's - var tags []GitHubTagsApiResponse - if err := json.Unmarshal(jsonResp, &tags); err != nil { - return tagsString, wrapError(err) - } + // Extract the JSON into our array of gitHubTagsCommitApiResponse's + var tags []GitHubTagsApiResponse + if err := json.Unmarshal(jsonResp, &tags); err != nil { + return tagsString, wrapError(err) + } - for _, tag := range tags { - // Skip tags that are not semantically versioned so that they don't cause errors. (issue #75) - if _, err := version.NewVersion(tag.Name); err == nil { - tagsString = append(tagsString, tag.Name) + for _, tag := range tags { + // Skip tags that are not semantically versioned so that they don't cause errors. (issue #75) + if _, err := version.NewVersion(tag.Name); err == nil { + tagsString = append(tagsString, tag.Name) + } } + //Get paginated tags (issue #26 and #46) + currPath = getNextPath(resp.Header.Get("link")) } return tagsString, nil @@ -202,6 +207,29 @@ func createGitHubRepoUrlForPath(repo GitHubRepo, path string) string { return fmt.Sprintf("repos/%s/%s/%s", repo.Owner, repo.Name, path) } +// Get the Next paginated path from the link url api.github.com/repos/:owner/:repo/:path +// Links are formatted: "; rel=next, ; rel=last" +func getNextPath(links string) string { + if len(links) == 0 { + return "" + } + + for _, link := range strings.Split(links, ",") { + + if attrs := strings.Split(link, ";"); len(attrs) > 1 { + url, rel := attrs[0], attrs[1] + + // Get only the next link + if strings.Contains(rel, "next") { + url = strings.Trim(url, " <>") + return url[strings.LastIndex(url, "/")+1:] + } + } + } + + return "" +} + // Call the GitHub API at the given path and return the HTTP response func callGitHubApi(repo GitHubRepo, path string, customHeaders map[string]string) (*http.Response, *FetchError) { httpClient := &http.Client{} From b2377fb5875d060b5c741ddcce1c5c0382d4d46c Mon Sep 17 00:00:00 2001 From: m-bal Date: Sat, 9 Jan 2021 19:47:25 -0800 Subject: [PATCH 2/3] Clarified what getNextPath returns Co-authored-by: Yevgeniy Brikman --- github.go | 1 + 1 file changed, 1 insertion(+) diff --git a/github.go b/github.go index b3c400e..1cda3d2 100644 --- a/github.go +++ b/github.go @@ -208,6 +208,7 @@ func createGitHubRepoUrlForPath(repo GitHubRepo, path string) string { } // Get the Next paginated path from the link url api.github.com/repos/:owner/:repo/:path +// If there is no next page, return an empty string // Links are formatted: "; rel=next, ; rel=last" func getNextPath(links string) string { if len(links) == 0 { From 05948e63ab95f7e28b3db32dd524be56cfc1bff3 Mon Sep 17 00:00:00 2001 From: mbal Date: Wed, 20 Jan 2021 00:13:17 -0800 Subject: [PATCH 3/3] Added Regex and test cases for getNextPath Used Regex to parse Link headers instead of splits and index arthemtic. getNextPath now returns `string` and `*FetchError`. --- github.go | 36 +++++++++++++++++++++++++----------- github_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/github.go b/github.go index 1cda3d2..2bba850 100644 --- a/github.go +++ b/github.go @@ -134,8 +134,13 @@ func FetchTags(githubRepoUrl string, githubToken string, instance GitHubInstance tagsString = append(tagsString, tag.Name) } } + //Get paginated tags (issue #26 and #46) - currPath = getNextPath(resp.Header.Get("link")) + currPath, err = getNextPath(resp.Header.Get("link")) + if err != nil { + return tagsString, err + } + } return tagsString, nil @@ -210,25 +215,34 @@ func createGitHubRepoUrlForPath(repo GitHubRepo, path string) string { // Get the Next paginated path from the link url api.github.com/repos/:owner/:repo/:path // If there is no next page, return an empty string // Links are formatted: "; rel=next, ; rel=last" -func getNextPath(links string) string { +func getNextPath(links string) (string, *FetchError) { if len(links) == 0 { - return "" + return "", nil } - for _, link := range strings.Split(links, ",") { + nextLinkRegex, regexErr := regexp.Compile(`<(.+?)>;\s+rel="next"`) + if regexErr != nil { + return "", wrapError(regexErr) + } + pathRegex, regexErr := regexp.Compile(`([^\/]+$)`) + if regexErr != nil { + return "", wrapError(regexErr) + } - if attrs := strings.Split(link, ";"); len(attrs) > 1 { - url, rel := attrs[0], attrs[1] + for _, link := range strings.Split(links, ",") { - // Get only the next link - if strings.Contains(rel, "next") { - url = strings.Trim(url, " <>") - return url[strings.LastIndex(url, "/")+1:] + urlMatches := nextLinkRegex.FindStringSubmatch(link) + if len(urlMatches) == 2 { + // Get only the next link path + pathMatches := pathRegex.FindStringSubmatch(urlMatches[1]) + if len(pathMatches) != 2 { + return "", newError(githubRepoUrlMalformedOrNotParseable, fmt.Sprintf("Path parsed incorrectly for url: %s", urlMatches[1])) } + return pathMatches[1], nil } } - return "" + return "", nil } // Call the GitHub API at the given path and return the HTTP response diff --git a/github_test.go b/github_test.go index 5c68e33..fa73709 100644 --- a/github_test.go +++ b/github_test.go @@ -57,6 +57,33 @@ func TestGetListOfReleasesFromGitHubRepo(t *testing.T) { } } +func TestGetNextPath(t *testing.T) { + t.Parallel() + + cases := []struct { + links string + expectedNextPath string + }{ + {`; rel="next", ; rel="last"`, "code?q=addClass+user%3Amozilla&page=15"}, + {`; rel="first", ; rel="last"`, ""}, + {`; rel="next", ; rel="last"`, "tags?page=15"}, + {`; rel="next", ; rel="last"`, "tags?per_page=100&page=15"}, + } + + for _, tc := range cases { + nextPath, err := getNextPath(tc.links) + + if err != nil { + t.Fatalf("error getting next path: %s", err) + } + + if nextPath != tc.expectedNextPath { + t.Fatalf("Expected next path %s, but got %s", tc.expectedNextPath, nextPath) + } + } + +} + func TestParseUrlIntoGithubInstance(t *testing.T) { t.Parallel()