Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update pagination to compute current page based off total_count #1372

Merged
merged 4 commits into from
Aug 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .changelog/1372.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
```release-note:bug
pagination: Will look at `total_count` and `per_page` to calculate `total_pages` if `total_pages` is zero
```

```release-note:bug
access_application: Use autopaginate flag as expected
```

```release-note:bug
access_ca_certificate: Use autopaginate flag as expected
```

```release-note:bug
access_group: Use autopaginate flag as expected
```

```release-note:bug
access_mutual_tls_certifcate: Use autopaginate flag as expected
```

```release-note:bug
access_policy: Use autopaginate flag as expected
```
2 changes: 1 addition & 1 deletion access_application.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ func (api *API) ListAccessApplications(ctx context.Context, rc *ResourceContaine
}
applications = append(applications, r.Result...)
params.ResultInfo = r.ResultInfo.Next()
if params.ResultInfo.Done() || autoPaginate {
if params.ResultInfo.Done() || !autoPaginate {
break
}
}
Expand Down
2 changes: 1 addition & 1 deletion access_application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestAccessApplications(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down
2 changes: 1 addition & 1 deletion access_bookmark_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestAccessBookmarks(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down
2 changes: 1 addition & 1 deletion access_ca_certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ func (api *API) ListAccessCACertificates(ctx context.Context, rc *ResourceContai
}
accessCACertificates = append(accessCACertificates, r.Result...)
params.ResultInfo = r.ResultInfo.Next()
if params.ResultInfo.Done() || autoPaginate {
if params.ResultInfo.Done() || !autoPaginate {
break
}
}
Expand Down
2 changes: 1 addition & 1 deletion access_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ func (api *API) ListAccessGroups(ctx context.Context, rc *ResourceContainer, par
}
accessGroups = append(accessGroups, r.Result...)
params.ResultInfo = r.ResultInfo.Next()
if params.ResultInfo.Done() || autoPaginate {
if params.ResultInfo.Done() || !autoPaginate {
break
}
}
Expand Down
2 changes: 1 addition & 1 deletion access_group_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func TestAccessGroups(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down
2 changes: 1 addition & 1 deletion access_mutual_tls_certificates.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func (api *API) ListAccessMutualTLSCertificates(ctx context.Context, rc *Resourc
}
accessCertificates = append(accessCertificates, r.Result...)
params.ResultInfo = r.ResultInfo.Next()
if params.ResultInfo.Done() || autoPaginate {
if params.ResultInfo.Done() || !autoPaginate {
break
}
}
Expand Down
2 changes: 1 addition & 1 deletion access_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func (api *API) ListAccessPolicies(ctx context.Context, rc *ResourceContainer, p
}
accessPolicies = append(accessPolicies, r.Result...)
params.ResultInfo = r.ResultInfo.Next()
if params.ResultInfo.Done() || autoPaginate {
if params.ResultInfo.Done() || !autoPaginate {
break
}
}
Expand Down
6 changes: 3 additions & 3 deletions account_members_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func TestAccountMembers(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down Expand Up @@ -459,7 +459,7 @@ func TestUpdateAccountMember(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down Expand Up @@ -523,7 +523,7 @@ func TestUpdateAccountMemberWithPolicies(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}`)
}
Expand Down
4 changes: 2 additions & 2 deletions account_roles_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func TestAccountRoles(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down Expand Up @@ -97,7 +97,7 @@ func TestAccountRole(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down
6 changes: 3 additions & 3 deletions accounts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func TestAccounts(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down Expand Up @@ -87,7 +87,7 @@ func TestAccount(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down Expand Up @@ -183,7 +183,7 @@ func TestCreateAccount(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down
2 changes: 1 addition & 1 deletion cloudflare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ func TestClient_RetryCanSucceedAfterErrors(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}`)
}
Expand Down
12 changes: 6 additions & 6 deletions custom_pages_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func TestCustomPagesForZone(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down Expand Up @@ -128,7 +128,7 @@ func TestCustomPagesForAccount(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down Expand Up @@ -172,7 +172,7 @@ func TestCustomPageForZone(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down Expand Up @@ -215,7 +215,7 @@ func TestCustomPageForAccount(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down Expand Up @@ -258,7 +258,7 @@ func TestUpdateCustomPagesForAccount(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down Expand Up @@ -300,7 +300,7 @@ func TestUpdateCustomPagesForZone(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down
4 changes: 2 additions & 2 deletions device_posture_rule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestDevicePostureIntegrations(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}`)
}
Expand Down Expand Up @@ -288,7 +288,7 @@ func TestDevicePostureRules(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}
`)
Expand Down
6 changes: 3 additions & 3 deletions dns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func TestListDNSRecords(t *testing.T) {
"count": 1,
"page": 1,
"per_page": 20,
"total_count": 2000
"total_count": 1
}
}`)
}
Expand Down Expand Up @@ -292,7 +292,7 @@ func TestListDNSRecordsSearch(t *testing.T) {
"count": 1,
"page": 1,
"per_page": 20,
"total_count": 2000
"total_count": 1
}
}`)
}
Expand Down Expand Up @@ -337,7 +337,7 @@ func TestListDNSRecordsSearch(t *testing.T) {
Tags: []string{"tag1", "tag2"},
})
require.NoError(t, err)
assert.Equal(t, 2000, resultInfo.Total)
assert.Equal(t, 1, resultInfo.Total)

assert.Equal(t, want, actual)
}
Expand Down
6 changes: 3 additions & 3 deletions load_balancing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ func TestListLoadBalancerPools(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}`)
}
Expand Down Expand Up @@ -760,7 +760,7 @@ func TestListLoadBalancerMonitors(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}`)
}
Expand Down Expand Up @@ -1497,7 +1497,7 @@ func TestListLoadBalancers(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}`)
}
Expand Down
2 changes: 1 addition & 1 deletion lockdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ func TestListZoneLockdowns(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}`)
}
Expand Down
2 changes: 1 addition & 1 deletion origin_ca_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func TestOriginCA_OriginCertificates(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 1
}
}`)
})
Expand Down
28 changes: 22 additions & 6 deletions pagination.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,29 @@
package cloudflare

import (
"math"
)

// Look first for total_pages, but if total_count and per_page are set then use that to get page count.
func (p ResultInfo) getTotalPages() int {
totalPages := p.TotalPages
if totalPages == 0 && p.Total > 0 && p.PerPage > 0 {
totalPages = int(math.Ceil(float64(p.Total) / float64(p.PerPage)))
}
return totalPages
}

// Done returns true for the last page and false otherwise.
func (p ResultInfo) Done() bool {
// A little hacky but if the response body is lacking a defined `ResultInfo`
// object the page will be 1 however the counts will be empty so if we have
// that response, we just assume this is the only page.
if p.Page == 1 && p.TotalPages == 0 {
totalPages := p.getTotalPages()
if p.Page == 1 && totalPages == 0 {
return true
}

return p.Page > 1 && p.Page > p.TotalPages
return p.Page > 1 && p.Page > totalPages
}

// Next advances the page of a paginated API response, but does not fetch the
Expand All @@ -18,13 +32,14 @@ func (p ResultInfo) Next() ResultInfo {
// A little hacky but if the response body is lacking a defined `ResultInfo`
// object the page will be 1 however the counts will be empty so if we have
// that response, we just assume this is the only page.
if p.Page == 1 && p.TotalPages == 0 {
totalPages := p.getTotalPages()
if p.Page == 1 && totalPages == 0 {
return p
}

// This shouldn't happen normally however, when it does just return the
// current page.
if p.Page > p.TotalPages {
if p.Page > totalPages {
return p
}

Expand All @@ -35,9 +50,10 @@ func (p ResultInfo) Next() ResultInfo {
// HasMorePages returns whether there is another page of results after the
// current one.
func (p ResultInfo) HasMorePages() bool {
if p.TotalPages == 0 {
totalPages := p.getTotalPages()
if totalPages == 0 {
return false
}

return p.Page >= 1 && p.Page < p.TotalPages
return p.Page >= 1 && p.Page < totalPages
}
Loading