Support pagination

[koshatul]

Not sure how to do this nicely, probably by wrapping http.Response like how it's done in the google/go-github library.

I'd be willing to do a PR for it, but it's a fair bit of work if someone else has a better idea of how to do it.

google/go-github example: https://github.com/google/go-github/blob/master/github/github.go#L381

[shuheiktgw]

Thanks for reporting an issue! 👍

I'm not pretty sure if I understood the issue correctly, but I believe go-travis already supported pagination and every "paginatable" method should have limit and offset options. For example, JobsOption has ones.

Please let me know if there are any misunderstandings! 😄

[shuheiktgw]

We may have to have pagination related struct to metadata.go since currently there are no ways people can access next, prev, last and other pagination related information in the responses...

https://developer.travis-ci.com/pagination

[koshatul]

The options for the pages are there, but no way to check what the page details actually are.

I wrote a pretty hacky workaround and put the pagination in the method, which really isn't ideal.

Because unlike go-github the pagination is in the response body and not the headers, the options I can think of are:

• adding Pagination *Pagination json:"@pagination"`` to every response struct, then having a helper method apply that information to the http.Response wrapper (probably the best option).
• hooking the Client.Do() method to do it and copying the Body into two writers, one for the response structs and one for the pagination structs, I can kinda see where it would work, but it feels really extremely hacky.

What do you think ?

I might try modifying the repositoryResponse Repository.List method and seeing if it looks OK.

[koshatul]

I created a draft PR to see if it looks OK to you, it works with that method.

opt := &travis.RepositoriesOption{
    Limit: 1000,
}

repos := []*travis.Repository{}
for {
    reposPage, resp, err := client.Repositories.List(context.Background(), opt)
    if err != nil { panic(err) }
    repos = append(repos, reposPage...)
    if resp.IsLast || resp.NextPage == nil {
        break
    }
    opt.Limit = resp.NextPage.Limit
    opt.Offset = resp.NextPage.Offset
}

[shuheiktgw]

Thank you so much for your PR, and sorry for my tardy response... I'll check the PR, so give me a few days to review it! 👍

[koshatul]

It's only a draft at the moment, to make a PR out of it I'll need to do all the methods, if you think it works this way I'll do the rest of them.

[koshatul]

I added the pagination to all the responses that return arrays, but I'm not sure where in the travis docs it says which requests will return pagination.

The only issue I have with the implementation now is that it's not possible to tell if IsFirst/IsLast on the Response are set or just false without inspecting the Limit/Offset/Count.

Maybe changing the IsFirst/IsLast to *bool instead might make more sense, but does increase complexity slightly since the code would be.

if (resp.IsLast != nil && *resp.IsLast) || resp.NextPage == nil {
	break
}

Or maybe a method on Response like func (r *Response) IsLast() (value bool, ok bool) that returns (true, true) if the value exists and is true, (false, true) if it exists and is false or (false, false) if it doesn't exist.

if v, ok := resp.IsLast(); !ok || !v || resp.NextPage == nil {
	break
}

They all seem like crappy options, maybe just not exposing them is an option.

[koshatul]

Or maybe it's just a caveat of the API, IsFirst and IsLast should be there, and if they're not returned then the library needs to remove it from that method.

[koshatul]

After much annoying friends, I think as-is is fine and this is the example:

opt := &travis.RepositoriesOption{}

allRepos := []*travis.Repository{}
for {
    repos, resp, err := client.Repositories.List(context.Background(), opt)
    if err != nil { panic(err) }
    allRepos = append(allRepos, repos...)
    if resp.NextPage == nil {
        break
    }
    opt.Limit = resp.NextPage.Limit
    opt.Offset = resp.NextPage.Offset
}