func populatePageValues can't handle API's returning 'since'

[brianlangdon]

API's such as the Organization ListAll can return many pages of Organizations. The pagination value is called since and holds the id of the last Org returned. A simple change to populatePageValues appears, so far, to be able to support this. Not sure if this a fault or a improvement! I'm new to golang so please forgive.

Old code in populatePageValues:

		page := q.Get("page")
		if page == "" {
			continue
		}

proposed change to populatePageValues

		page := q.Get("page")
		since := q.Get("since")
		if page == "" && since == "" {
			continue
		}

		if since != "" {
			page = since
		}

[gmlewis]

I see that orgs ListAll uses a since parameter to control getting the next group of results here:
https://docs.github.com/en/rest/reference/orgs#list-organizations
but the link then points here:
https://docs.github.com/en/rest/overview/resources-in-the-rest-api#link-header
which makes no mention of receiving since in the header.

Additionally, the only pagination documentation I could find is here:
https://docs.github.com/en/enterprise-server@3.0/rest/guides/traversing-with-pagination
which also does not include since.

Can you please show an example header response that includes the since value?
I believe you, but would like to have it documented in this issue, please.

[brianlangdon]

Here is a slightly formatted and sanitised copy of our header - we are using GHE not github.com
net/http.Header ["Content-Type": ["application/json; charset=utf-8"],
"Cache-Control": ["private, max-age=60, s-maxage=60"],
"Vary": ["Accept, Authorization, Cookie, X-GitHub-OTP"],
"X-Accepted-Oauth-Scopes": [""],
"Strict-Transport-Security": ["max-age=31536000; includeSubdomains"],
"Referrer-Policy": ["origin-when-cross-origin, strict-origin-when-cross-origin"],
"X-Oauth-Scopes": ["admin:gpg_key, admin:org, admin:org_hook, admin:pre_receive_hook, admin:public_key, admin:repo_hook, delete_repo, gist, notifications, repo, site_admin, user, write:discussion"],
"X-Github-Media-Type": ["github.v3; format=json"],
"Link": ["https://ourserver/api/v3/organizations?since=296; rel="next", https://ourserver/api/v3/organizations{?since}; rel="first""],
"Access-Control-Expose-Headers": ["ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, Deprecation, Sunset"],
"Access-Control-Allow-Origin": ["*"],
"Content-Security-Policy": ["default-src 'none'"],
"Server": ["GitHub.com"],
"X-Github-Request-Id": ["XYZ long hex number XYZ"],
"X-Xss-Protection": ["1; mode=block"],
"X-Runtime-Rack": ["0.132278"],
"Date": ["Thu, 30 Sep 2021 12:20:54 GMT"],
"Etag": ["W/"XYZ long hex number XYZ""],
"X-Github-Enterprise-Version": ["3.0.13"], "X-Frame-Options": ["deny"], "X-Content-Type-Options": ["nosniff"], ]

The func is parsing the Link entry

[gmlewis]

OK, thank you, @brianlangdon !

Would you like to create a PR or would you like me to open up this issue for one of our other amazing contributors to address this issue?

[brianlangdon]

Hi @gmlewis, I'll give it a go, although I'm not sure how I'd create tests for it.

[gmlewis]

Hi @gmlewis, I'll give it a go, although I'm not sure how I'd create tests for it.

OK, cool. It's yours, @brianlangdon .

As for a test, check out this example, and create a new test based upon it (possibly even right after it):
https://github.com/google/go-github/blob/master/github/github_test.go#L622-L649

[brianlangdon]

#2135 created

[gmlewis]

As reported here: #2135 (comment)
it turns out that both since and page can be returned in the header of the response, which causes the regular pagination to break.

I tested this myself with the following test program:

package main

import (
	"context"
	"log"
	"time"

	"github.com/google/go-github/v41/github"
)

func main() {
	client := github.NewClient(nil)
	ctx := context.Background()

	since, err := time.Parse(time.RFC3339, "2021-12-04T10:43:42Z")
	if err != nil {
		log.Fatal(err)
	}

	opts := &github.CommitsListOptions{Since: since}
	d, resp, err := client.Repositories.ListCommits(ctx, "Homebrew", "homebrew-cask", opts)
	if err != nil {
		log.Fatal(err)
	}

	log.Printf("len(d)=%v", len(d))
	log.Printf("resp.Header=%#v", resp.Header)
	log.Printf("resp.NextPage=%v", resp.NextPage)
	log.Printf("resp.PrevPage=%v", resp.PrevPage)
	log.Printf("resp.FirstPage=%v", resp.FirstPage)
	log.Printf("resp.LastPage=%v", resp.LastPage)
	log.Printf("resp.NextPageToken=%v", resp.NextPageToken)
	log.Printf("resp.Cursor=%v", resp.Cursor)
	log.Printf("resp.Before=%v", resp.Before)
	log.Printf("resp.After=%v", resp.After)
}

which returned:

$ go run main.go
2021/12/05 12:25:24 len(d)=30
2021/12/05 12:25:24 resp.Header=http.Header{...
"Link":[]string{"<https://api.github.com/repositories/3623050/commits?since=2021-12-04T10%3A43%3A42Z&page=2>; rel=\"next\", <https://api.github.com/repositories/3623050/commits?since=2021-12-04T10%3A43%3A42Z&page=2>; rel=\"last\""}, 
...}
2021/12/05 12:25:24 resp.NextPage=0
2021/12/05 12:25:24 resp.PrevPage=0
2021/12/05 12:25:24 resp.FirstPage=0
2021/12/05 12:25:24 resp.LastPage=0
2021/12/05 12:25:24 resp.NextPageToken=2021-12-04T10:43:42Z
2021/12/05 12:25:24 resp.Cursor=
2021/12/05 12:25:24 resp.Before=
2021/12/05 12:25:24 resp.After=

This should say: resp.NextPage=2 (see the header Link).