Repository: google/go-github (go)

Issue:
BUG > ListIDPGroupsInOrganization paginates using int instead of string
# Description

## Current Behavior
* `ListIDPGroupsInOrganization`
  * expects to be provided with `ListOptions` with `Page` value as `int`
    * https://github.com/google/go-github/blob/master/github/github.go#L193
  * returns response with type `github.Response` and `NextPage`, `PrevPage`, etc, values as `int`
    * https://github.com/google/go-github/blob/c74e92b2275cbb12471ed0fd6204dac07e130943/github/github.go#L399
* However, unlike other GitHub APIs, `GET /orgs/:org/team-sync/groups` return a page that is not numeric
  * For example: `Link: <https://api.github.com/organizations/<redacted>/team-sync/groups?per_page=100&page=X%274<redacted>0000000000000000000000%27>; rel="next"`
    * I've redacted from 274 to 000..., but its basically an alphanumeric string
  * Resulting `github.Response` will have `NextPage` with value as `0` since its expecting `int`
  * Pagination does not work, since `NextPage` is always `0`

## Expected Behavior
* `ListIDPGroupsInOrganization` should paginate properly

## Additional Details

### Request
```
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: ---[ RESPONSE ]--------------------------------------
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: HTTP/1.1 200 OK
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: Transfer-Encoding: chunked
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: Access-Control-Allow-Origin: *
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: Access-Control-Expose-Headers: ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: Cache-Control: private, max-age=60, s-maxage=60
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: Content-Security-Policy: default-src 'none'
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: Content-Type: application/json; charset=utf-8
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: Date: Fri, 17 Jan 2020 20:43:46 GMT
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: Etag: <redacted>
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: Link: <https://api.github.com/organizations/<redacted>/team-sync/groups?Page=&PerPage=10&page=X%274<redacted>0000000000000000000000%27>; rel="next"
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: Referrer-Policy: origin-when-cross-origin, strict-origin-when-cross-origin
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: Server: GitHub.com
5:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: Status: 200 OK
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: Strict-Transport-Security: max-age=31536000; includeSubdomains; preload
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: Vary: Accept, Authorization, Cookie, X-GitHub-OTP
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: X-Accepted-Oauth-Scopes: admin:org, repo
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: X-Content-Type-Options: nosniff
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: X-Frame-Options: deny
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: X-Github-Media-Type: github.v3; param=team-sync-preview; format=json
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: X-Github-Request-Id: <redacted>
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: X-Oauth-Scopes: admin:org, delete_repo, repo, workflow
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: X-Ratelimit-Limit: 5000
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: X-Ratelimit-Remaining: 4970
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: X-Ratelimit-Reset: <redacted>
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github: X-Xss-Protection: 1; mode=block
2020-01-17T15:43:46.945-0500 [DEBUG] plugin.terraform-provider-github:
```


Pull request description:
Support cursor pagination
Fixes #1383.

Patch:
diff --git a/github/apps_installation.go b/github/apps_installation.go
index 09d4043f..c2e3b4ca 100644
--- a/github/apps_installation.go
+++ b/github/apps_installation.go
@@ -101,3 +101,17 @@ func (s *AppsService) RemoveRepository(ctx context.Context, instID, repoID int64
 
 	return s.client.Do(ctx, req, nil)
 }
+
+// RevokeInstallationToken revokes an installation token.
+//
+// GitHub docs: https://developer.github.com/v3/apps/installations/#revoke-an-installation-token
+func (s *AppsService) RevokeInstallationToken(ctx context.Context) (*Response, error) {
+	u := "installation/token"
+	req, err := s.client.NewRequest("DELETE", u, nil)
+	if err != nil {
+		return nil, err
+	}
+	req.Header.Set("Accept", mediaTypeRevokeTokenPreview)
+
+	return s.client.Do(ctx, req, nil)
+}
diff --git a/github/github.go b/github/github.go
index b752c37b..589cda53 100644
--- a/github/github.go
+++ b/github/github.go
@@ -46,6 +46,9 @@ const (
 
 	// Media Type values to access preview APIs
 
+	// https://developer.github.com/changes/2020-01-10-revoke-installation-token/
+	mediaTypeRevokeTokenPreview = "application/vnd.github.gambit-preview+json"
+
 	// https://developer.github.com/changes/2014-12-09-new-attributes-for-stars-api/
 	mediaTypeStarringPreview = "application/vnd.github.v3.star+json"
 
@@ -187,7 +190,7 @@ type service struct {
 }
 
 // ListOptions specifies the optional parameters to various List methods that
-// support pagination.
+// support offset pagination.
 type ListOptions struct {
 	// For paginated result sets, page of results to retrieve.
 	Page int `url:"page,omitempty"`
@@ -196,6 +199,16 @@ type ListOptions struct {
 	PerPage int `url:"per_page,omitempty"`
 }
 
+// ListCursorOptions specifies the optional parameters to various List methods that
+// support cursor pagination.
+type ListCursorOptions struct {
+	// For paginated result sets, page of results to retrieve.
+	Page string `url:"page,omitempty"`
+
+	// For paginated result sets, the number of results to include per page.
+	PerPage int `url:"per_page,omitempty"`
+}
+
 // UploadOptions specifies the parameters to methods that support uploads.
 type UploadOptions struct {
 	Name      string `url:"name,omitempty"`
@@ -395,12 +408,27 @@ type Response struct {
 	// results. Any or all of these may be set to the zero value for
 	// responses that are not part of a paginated set, or for which there
 	// are no additional pages.
-
+	//
+	// These fields support what is called "offset pagination" and should
+	// be used with the ListOptions struct.
 	NextPage  int
 	PrevPage  int
 	FirstPage int
 	LastPage  int
 
+	// Additionally, some APIs support "cursor pagination" instead of offset.
+	// This means that a token points directly to the next record which
+	// can lead to O(1) performance compared to O(n) performance provided
+	// by offset pagination.
+	//
+	// For APIs that support cursor pagination (such as
+	// TeamsService.ListIDPGroupsInOrganization), the following field
+	// will be populated to point to the next page.
+	//
+	// To use this token, set ListCursorOptions.Page to this value before
+	// calling the endpoint again.
+	NextPageToken string
+
 	// Explicitly specify the Rate type so Rate's String() receiver doesn't
 	// propagate to Response.
 	Rate Rate
@@ -445,7 +473,9 @@ func (r *Response) populatePageValues() {
 			for _, segment := range segments[1:] {
 				switch strings.TrimSpace(segment) {
 				case `rel="next"`:
-					r.NextPage, _ = strconv.Atoi(page)
+					if r.NextPage, err = strconv.Atoi(page); err != nil {
+						r.NextPageToken = page
+					}
 				case `rel="prev"`:
 					r.PrevPage, _ = strconv.Atoi(page)
 				case `rel="first"`:
diff --git a/github/repos_releases.go b/github/repos_releases.go
index 5c0a1cea..0a434572 100644
--- a/github/repos_releases.go
+++ b/github/repos_releases.go
@@ -266,8 +266,13 @@ func (s *RepositoriesService) GetReleaseAsset(ctx context.Context, owner, repo s
 // If a redirect is returned, the redirect URL will be returned as a string instead
 // of the io.ReadCloser. Exactly one of rc and redirectURL will be zero.
 //
+// followRedirectsClient can be passed to download the asset from a redirected
+// location. Passing http.DefaultClient is recommended unless special circumstances
+// exist, but it's possible to pass any http.Client. If nil is passed the
+// redirectURL will be returned instead.
+//
 // GitHub API docs: https://developer.github.com/v3/repos/releases/#get-a-single-release-asset
-func (s *RepositoriesService) DownloadReleaseAsset(ctx context.Context, owner, repo string, id int64) (rc io.ReadCloser, redirectURL string, err error) {
+func (s *RepositoriesService) DownloadReleaseAsset(ctx context.Context, owner, repo string, id int64, followRedirectsClient *http.Client) (rc io.ReadCloser, redirectURL string, err error) {
 	u := fmt.Sprintf("repos/%s/%s/releases/assets/%d", owner, repo, id)
 
 	req, err := s.client.NewRequest("GET", u, nil)
@@ -293,6 +298,10 @@ func (s *RepositoriesService) DownloadReleaseAsset(ctx context.Context, owner, r
 		if !strings.Contains(err.Error(), "disable redirect") {
 			return nil, "", err
 		}
+		if followRedirectsClient != nil {
+			rc, err := s.downloadReleaseAssetFromURL(ctx, followRedirectsClient, loc)
+			return rc, "", err
+		}
 		return nil, loc, nil // Intentionally return no error with valid redirect URL.
 	}
 
@@ -304,6 +313,24 @@ func (s *RepositoriesService) DownloadReleaseAsset(ctx context.Context, owner, r
 	return resp.Body, "", nil
 }
 
+func (s *RepositoriesService) downloadReleaseAssetFromURL(ctx context.Context, followRedirectsClient *http.Client, url string) (rc io.ReadCloser, err error) {
+	req, err := http.NewRequest("GET", url, nil)
+	if err != nil {
+		return nil, err
+	}
+	req = withContext(ctx, req)
+	req.Header.Set("Accept", "*/*")
+	resp, err := followRedirectsClient.Do(req)
+	if err != nil {
+		return nil, err
+	}
+	if err := CheckResponse(resp); err != nil {
+		resp.Body.Close()
+		return nil, err
+	}
+	return resp.Body, nil
+}
+
 // EditReleaseAsset edits a repository release asset.
 //
 // GitHub API docs: https://developer.github.com/v3/repos/releases/#edit-a-release-asset
diff --git a/github/teams.go b/github/teams.go
index ff2f0bc2..03284874 100644
--- a/github/teams.go
+++ b/github/teams.go
@@ -509,7 +509,7 @@ type IDPGroup struct {
 // ListIDPGroupsInOrganization lists IDP groups available in an organization.
 //
 // GitHub API docs: https://developer.github.com/v3/teams/team_sync/#list-idp-groups-in-an-organization
-func (s *TeamsService) ListIDPGroupsInOrganization(ctx context.Context, org string, opt *ListOptions) (*IDPGroupList, *Response, error) {
+func (s *TeamsService) ListIDPGroupsInOrganization(ctx context.Context, org string, opt *ListCursorOptions) (*IDPGroupList, *Response, error) {
 	u := fmt.Sprintf("orgs/%v/team-sync/groups", org)
 	u, err := addOptions(u, opt)
 	if err != nil {


Test patch:
diff --git a/github/apps_installation_test.go b/github/apps_installation_test.go
index 745e1e5d..10955bf5 100644
--- a/github/apps_installation_test.go
+++ b/github/apps_installation_test.go
@@ -101,3 +101,19 @@ func TestAppsService_RemoveRepository(t *testing.T) {
 		t.Errorf("Apps.RemoveRepository returned error: %v", err)
 	}
 }
+
+func TestAppsService_RevokeInstallationToken(t *testing.T) {
+	client, mux, _, teardown := setup()
+	defer teardown()
+
+	mux.HandleFunc("/installation/token", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "DELETE")
+		testHeader(t, r, "Accept", mediaTypeRevokeTokenPreview)
+		w.WriteHeader(http.StatusNoContent)
+	})
+
+	_, err := client.Apps.RevokeInstallationToken(context.Background())
+	if err != nil {
+		t.Errorf("Apps.RevokeInstallationToken returned error: %v", err)
+	}
+}
diff --git a/github/github_test.go b/github/github_test.go
index 0780d14d..8381280b 100644
--- a/github/github_test.go
+++ b/github/github_test.go
@@ -91,6 +91,7 @@ func openTestFile(name, content string) (file *os.File, dir string, err error) {
 }
 
 func testMethod(t *testing.T, r *http.Request, want string) {
+	t.Helper()
 	if got := r.Method; got != want {
 		t.Errorf("Request method: %v, want %v", got, want)
 	}
@@ -99,6 +100,7 @@ func testMethod(t *testing.T, r *http.Request, want string) {
 type values map[string]string
 
 func testFormValues(t *testing.T, r *http.Request, values values) {
+	t.Helper()
 	want := url.Values{}
 	for k, v := range values {
 		want.Set(k, v)
@@ -111,12 +113,14 @@ func testFormValues(t *testing.T, r *http.Request, values values) {
 }
 
 func testHeader(t *testing.T, r *http.Request, header string, want string) {
+	t.Helper()
 	if got := r.Header.Get(header); got != want {
 		t.Errorf("Header.Get(%q) returned %q, want %q", header, got, want)
 	}
 }
 
 func testURLParseError(t *testing.T, err error) {
+	t.Helper()
 	if err == nil {
 		t.Errorf("Expected error to be returned")
 	}
@@ -126,6 +130,7 @@ func testURLParseError(t *testing.T, err error) {
 }
 
 func testBody(t *testing.T, r *http.Request, want string) {
+	t.Helper()
 	b, err := ioutil.ReadAll(r.Body)
 	if err != nil {
 		t.Errorf("Error reading request body: %v", err)
@@ -138,6 +143,7 @@ func testBody(t *testing.T, r *http.Request, want string) {
 // Test whether the marshaling of v produces JSON that corresponds
 // to the want string.
 func testJSONMarshal(t *testing.T, v interface{}, want string) {
+	t.Helper()
 	// Unmarshal the wanted JSON, to verify its correctness, and marshal it back
 	// to sort the keys.
 	u := reflect.New(reflect.TypeOf(v)).Interface()
@@ -414,6 +420,35 @@ func TestResponse_populatePageValues(t *testing.T) {
 	if got, want := response.LastPage, 5; want != got {
 		t.Errorf("response.LastPage: %v, want %v", got, want)
 	}
+	if got, want := response.NextPageToken, ""; want != got {
+		t.Errorf("response.NextPageToken: %v, want %v", got, want)
+	}
+}
+
+func TestResponse_cursorPagination(t *testing.T) {
+	r := http.Response{
+		Header: http.Header{
+			"Status": {"200 OK"},
+			"Link":   {`<https://api.github.com/resource?per_page=2&page=url-encoded-next-page-token>; rel="next"`},
+		},
+	}
+
+	response := newResponse(&r)
+	if got, want := response.FirstPage, 0; got != want {
+		t.Errorf("response.FirstPage: %v, want %v", got, want)
+	}
+	if got, want := response.PrevPage, 0; want != got {
+		t.Errorf("response.PrevPage: %v, want %v", got, want)
+	}
+	if got, want := response.NextPage, 0; want != got {
+		t.Errorf("response.NextPage: %v, want %v", got, want)
+	}
+	if got, want := response.LastPage, 0; want != got {
+		t.Errorf("response.LastPage: %v, want %v", got, want)
+	}
+	if got, want := response.NextPageToken, "url-encoded-next-page-token"; want != got {
+		t.Errorf("response.NextPageToken: %v, want %v", got, want)
+	}
 }
 
 func TestResponse_populatePageValues_invalid(t *testing.T) {
diff --git a/github/repos_releases_test.go b/github/repos_releases_test.go
index 9e38d2d6..9ccf9ac4 100644
--- a/github/repos_releases_test.go
+++ b/github/repos_releases_test.go
@@ -252,7 +252,7 @@ func TestRepositoriesService_DownloadReleaseAsset_Stream(t *testing.T) {
 		fmt.Fprint(w, "Hello World")
 	})
 
-	reader, _, err := client.Repositories.DownloadReleaseAsset(context.Background(), "o", "r", 1)
+	reader, _, err := client.Repositories.DownloadReleaseAsset(context.Background(), "o", "r", 1, nil)
 	if err != nil {
 		t.Errorf("Repositories.DownloadReleaseAsset returned error: %v", err)
 	}
@@ -276,7 +276,7 @@ func TestRepositoriesService_DownloadReleaseAsset_Redirect(t *testing.T) {
 		http.Redirect(w, r, "/yo", http.StatusFound)
 	})
 
-	_, got, err := client.Repositories.DownloadReleaseAsset(context.Background(), "o", "r", 1)
+	_, got, err := client.Repositories.DownloadReleaseAsset(context.Background(), "o", "r", 1, nil)
 	if err != nil {
 		t.Errorf("Repositories.DownloadReleaseAsset returned error: %v", err)
 	}
@@ -286,6 +286,36 @@ func TestRepositoriesService_DownloadReleaseAsset_Redirect(t *testing.T) {
 	}
 }
 
+func TestRepositoriesService_DownloadReleaseAsset_FollowRedirect(t *testing.T) {
+	client, mux, _, teardown := setup()
+	defer teardown()
+
+	mux.HandleFunc("/repos/o/r/releases/assets/1", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		testHeader(t, r, "Accept", defaultMediaType)
+		// /yo, below will be served as baseURLPath/yo
+		http.Redirect(w, r, baseURLPath+"/yo", http.StatusFound)
+	})
+	mux.HandleFunc("/yo", func(w http.ResponseWriter, r *http.Request) {
+		testMethod(t, r, "GET")
+		testHeader(t, r, "Accept", "*/*")
+		w.Header().Set("Content-Type", "application/octet-stream")
+		w.Header().Set("Content-Disposition", "attachment; filename=hello-world.txt")
+		fmt.Fprint(w, "Hello World")
+	})
+
+	reader, _, err := client.Repositories.DownloadReleaseAsset(context.Background(), "o", "r", 1, http.DefaultClient)
+	content, err := ioutil.ReadAll(reader)
+	if err != nil {
+		t.Errorf("Repositories.DownloadReleaseAsset returned error: %v", err)
+	}
+	reader.Close()
+	want := []byte("Hello World")
+	if !bytes.Equal(want, content) {
+		t.Errorf("Repositories.DownloadReleaseAsset returned %+v, want %+v", content, want)
+	}
+}
+
 func TestRepositoriesService_DownloadReleaseAsset_APIError(t *testing.T) {
 	client, mux, _, teardown := setup()
 	defer teardown()
@@ -297,7 +327,7 @@ func TestRepositoriesService_DownloadReleaseAsset_APIError(t *testing.T) {
 		fmt.Fprint(w, `{"message":"Not Found","documentation_url":"https://developer.github.com/v3"}`)
 	})
 
-	resp, loc, err := client.Repositories.DownloadReleaseAsset(context.Background(), "o", "r", 1)
+	resp, loc, err := client.Repositories.DownloadReleaseAsset(context.Background(), "o", "r", 1, nil)
 	if err == nil {
 		t.Error("Repositories.DownloadReleaseAsset did not return an error")
 	}
diff --git a/github/teams_test.go b/github/teams_test.go
index 5cd160ec..ce7ece54 100644
--- a/github/teams_test.go
+++ b/github/teams_test.go
@@ -642,12 +642,12 @@ func TestTeamsService_ListIDPGroupsInOrganization(t *testing.T) {
 	mux.HandleFunc("/orgs/o/team-sync/groups", func(w http.ResponseWriter, r *http.Request) {
 		testMethod(t, r, "GET")
 		testFormValues(t, r, values{
-			"page": "2",
+			"page": "url-encoded-next-page-token",
 		})
 		fmt.Fprint(w, `{"groups": [{"group_id": "1",  "group_name": "n", "group_description": "d"}]}`)
 	})
 
-	opt := &ListOptions{Page: 2}
+	opt := &ListCursorOptions{Page: "url-encoded-next-page-token"}
 	groups, _, err := client.Teams.ListIDPGroupsInOrganization(context.Background(), "o", opt)
 	if err != nil {
 		t.Errorf("Teams.ListIDPGroupsInOrganization returned error: %v", err)
diff --git a/test/integration/repos_test.go b/test/integration/repos_test.go
index e4a09bfb..0cd2a87a 100644
--- a/test/integration/repos_test.go
+++ b/test/integration/repos_test.go
@@ -9,6 +9,8 @@ package integration
 
 import (
 	"context"
+	"io"
+	"io/ioutil"
 	"net/http"
 	"reflect"
 	"testing"
@@ -179,3 +181,19 @@ func TestRepositories_List(t *testing.T) {
 		}
 	}
 }
+
+func TestRepositories_DownloadReleaseAsset(t *testing.T) {
+	if !checkAuth("TestRepositories_DownloadReleaseAsset") {
+		return
+	}
+
+	rc, _, err := client.Repositories.DownloadReleaseAsset(context.Background(), "andersjanmyr", "goose", 484892, http.DefaultClient)
+	if err != nil {
+		t.Fatalf("Repositories.DownloadReleaseAsset(andersjanmyr, goose, 484892, true) returned error: %v", err)
+	}
+	defer func() { _ = rc.Close() }()
+	_, err = io.Copy(ioutil.Discard, rc)
+	if err != nil {
+		t.Fatalf("Repositories.DownloadReleaseAsset(andersjanmyr, goose, 484892, true) returned error: %v", err)
+	}
+}