Repository: cloudflare/cloudflare-go
Pull request #1372 (2023-08-24 18:56:32)

Update pagination to compute current page based off total_count
Also fixes pagination for a few Access Resources

## Description

In terraform, if you try to use the [Access Application Resource](https://registry.terraform.io/providers/cloudflare/cloudflare/latest/docs/resources/access_application.html) in a datasource it will fail to find the app because the List function will only check the first page. 

## Has your change been tested?

Unit tests and a local reproduction of the original issue that is now resolved.

## Types of changes

What sort of change does your code introduce/modify?

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Checklist:

- [x] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] I have added tests to cover my changes.
- [x] All new and existing tests passed.
- [x] This change is using publicly documented in [cloudflare/api-schemas](https://github.com/cloudflare/api-schemas) 
      and relies on stable APIs.

[1]: https://help.github.com/articles/closing-issues-using-keywords/



Patch:
diff --git a/.changelog/1372.txt b/.changelog/1372.txt
new file mode 100644
index 000000000..ec3f51408
--- /dev/null
+++ b/.changelog/1372.txt
@@ -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
+```
diff --git a/access_application.go b/access_application.go
index 2abc6cac5..4357fa0d1 100644
--- a/access_application.go
+++ b/access_application.go
@@ -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
 		}
 	}
diff --git a/access_ca_certificate.go b/access_ca_certificate.go
index 8becc8e72..8f167875e 100644
--- a/access_ca_certificate.go
+++ b/access_ca_certificate.go
@@ -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
 		}
 	}
diff --git a/access_group.go b/access_group.go
index a21da0df0..bdb3899ad 100644
--- a/access_group.go
+++ b/access_group.go
@@ -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
 		}
 	}
diff --git a/access_mutual_tls_certificates.go b/access_mutual_tls_certificates.go
index a7f29a6a9..393476c42 100644
--- a/access_mutual_tls_certificates.go
+++ b/access_mutual_tls_certificates.go
@@ -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
 		}
 	}
diff --git a/access_policy.go b/access_policy.go
index b5102e429..4219446f2 100644
--- a/access_policy.go
+++ b/access_policy.go
@@ -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
 		}
 	}
diff --git a/pagination.go b/pagination.go
index 696dc58ee..db06ff8a3 100644
--- a/pagination.go
+++ b/pagination.go
@@ -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
@@ -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
 	}
 
@@ -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
 }


Test patch:
diff --git a/access_application_test.go b/access_application_test.go
index 0309b6c74..ffba809fa 100644
--- a/access_application_test.go
+++ b/access_application_test.go
@@ -54,7 +54,7 @@ func TestAccessApplications(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
diff --git a/access_bookmark_test.go b/access_bookmark_test.go
index 90996859e..7b082b140 100644
--- a/access_bookmark_test.go
+++ b/access_bookmark_test.go
@@ -36,7 +36,7 @@ func TestAccessBookmarks(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
diff --git a/access_group_test.go b/access_group_test.go
index 11f538079..16bbd436e 100644
--- a/access_group_test.go
+++ b/access_group_test.go
@@ -83,7 +83,7 @@ func TestAccessGroups(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
diff --git a/account_members_test.go b/account_members_test.go
index b4d8d8348..0834c110c 100644
--- a/account_members_test.go
+++ b/account_members_test.go
@@ -182,7 +182,7 @@ func TestAccountMembers(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
@@ -459,7 +459,7 @@ func TestUpdateAccountMember(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
@@ -523,7 +523,7 @@ func TestUpdateAccountMemberWithPolicies(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}`)
 	}
diff --git a/account_roles_test.go b/account_roles_test.go
index 28d45e402..7658f3dfe 100644
--- a/account_roles_test.go
+++ b/account_roles_test.go
@@ -51,7 +51,7 @@ func TestAccountRoles(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
@@ -97,7 +97,7 @@ func TestAccountRole(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
diff --git a/accounts_test.go b/accounts_test.go
index bbd0adfb6..0f0fecc9c 100644
--- a/accounts_test.go
+++ b/accounts_test.go
@@ -48,7 +48,7 @@ func TestAccounts(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
@@ -87,7 +87,7 @@ func TestAccount(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
@@ -183,7 +183,7 @@ func TestCreateAccount(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
diff --git a/cloudflare_test.go b/cloudflare_test.go
index 315f743cb..e7ffb09f6 100644
--- a/cloudflare_test.go
+++ b/cloudflare_test.go
@@ -181,7 +181,7 @@ func TestClient_RetryCanSucceedAfterErrors(t *testing.T) {
                 "page": 1,
                 "per_page": 20,
                 "count": 1,
-                "total_count": 2000
+                "total_count": 1
             }
         }`)
 		}
diff --git a/custom_pages_test.go b/custom_pages_test.go
index 67daebb79..a8d28beed 100644
--- a/custom_pages_test.go
+++ b/custom_pages_test.go
@@ -82,7 +82,7 @@ func TestCustomPagesForZone(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
@@ -128,7 +128,7 @@ func TestCustomPagesForAccount(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
@@ -172,7 +172,7 @@ func TestCustomPageForZone(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
@@ -215,7 +215,7 @@ func TestCustomPageForAccount(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
@@ -258,7 +258,7 @@ func TestUpdateCustomPagesForAccount(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
@@ -300,7 +300,7 @@ func TestUpdateCustomPagesForZone(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
diff --git a/device_posture_rule_test.go b/device_posture_rule_test.go
index e70db3529..79ce77c51 100644
--- a/device_posture_rule_test.go
+++ b/device_posture_rule_test.go
@@ -40,7 +40,7 @@ func TestDevicePostureIntegrations(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}`)
 	}
@@ -288,7 +288,7 @@ func TestDevicePostureRules(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
diff --git a/dns_test.go b/dns_test.go
index 2f991f56f..1c575288d 100644
--- a/dns_test.go
+++ b/dns_test.go
@@ -196,7 +196,7 @@ func TestListDNSRecords(t *testing.T) {
 				"count": 1,
 				"page": 1,
 				"per_page": 20,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}`)
 	}
@@ -292,7 +292,7 @@ func TestListDNSRecordsSearch(t *testing.T) {
 				"count": 1,
 				"page": 1,
 				"per_page": 20,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}`)
 	}
@@ -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)
 }
diff --git a/load_balancing_test.go b/load_balancing_test.go
index 2bed0cab9..148746bbc 100644
--- a/load_balancing_test.go
+++ b/load_balancing_test.go
@@ -296,7 +296,7 @@ func TestListLoadBalancerPools(t *testing.T) {
                 "page": 1,
                 "per_page": 20,
                 "count": 1,
-                "total_count": 2000
+                "total_count": 1
             }
         }`)
 	}
@@ -760,7 +760,7 @@ func TestListLoadBalancerMonitors(t *testing.T) {
                 "page": 1,
                 "per_page": 20,
                 "count": 1,
-                "total_count": 2000
+                "total_count": 1
             }
         }`)
 	}
@@ -1497,7 +1497,7 @@ func TestListLoadBalancers(t *testing.T) {
                 "page": 1,
                 "per_page": 20,
                 "count": 1,
-                "total_count": 2000
+                "total_count": 1
             }
         }`)
 	}
diff --git a/lockdown_test.go b/lockdown_test.go
index 93ee813f0..470fca6bf 100644
--- a/lockdown_test.go
+++ b/lockdown_test.go
@@ -274,7 +274,7 @@ func TestListZoneLockdowns(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}`)
 	}
diff --git a/origin_ca_test.go b/origin_ca_test.go
index eca1ac28b..df79ace0a 100644
--- a/origin_ca_test.go
+++ b/origin_ca_test.go
@@ -129,7 +129,7 @@ func TestOriginCA_OriginCertificates(t *testing.T) {
     "page": 1,
     "per_page": 20,
     "count": 1,
-    "total_count": 2000
+    "total_count": 1
   }
 }`)
 	})
diff --git a/pagination_test.go b/pagination_test.go
index eb3b07ed2..2a045c637 100644
--- a/pagination_test.go
+++ b/pagination_test.go
@@ -28,6 +28,14 @@ func TestPagination_Done(t *testing.T) {
 			r:        ResultInfo{Page: 3, TotalPages: 1},
 			expected: true,
 		},
+		"total pages missing but done": {
+			r:        ResultInfo{Page: 4, Total: 70, PerPage: 25},
+			expected: true,
+		},
+		"total pages missing and not done": {
+			r:        ResultInfo{Page: 1, Total: 70, PerPage: 25},
+			expected: false,
+		},
 	}
 
 	for name, tc := range testCases {
@@ -59,6 +67,14 @@ func TestPagination_Next(t *testing.T) {
 			r:        ResultInfo{Page: 3, TotalPages: 1},
 			expected: ResultInfo{Page: 3, TotalPages: 1},
 		},
+		"use per page and greater than page": {
+			r:        ResultInfo{Page: 4, Total: 70, PerPage: 25},
+			expected: ResultInfo{Page: 4, Total: 70, PerPage: 25},
+		},
+		"use per page and less than page": {
+			r:        ResultInfo{Page: 1, Total: 70, PerPage: 25},
+			expected: ResultInfo{Page: 2, Total: 70, PerPage: 25},
+		},
 	}
 
 	for name, tc := range testCases {
@@ -93,6 +109,14 @@ func TestPagination_HasMorePages(t *testing.T) {
 			r:        ResultInfo{Page: 3, TotalPages: 1},
 			expected: false,
 		},
+		"use per page and greater than page": {
+			r:        ResultInfo{Page: 4, Total: 70, PerPage: 25},
+			expected: false,
+		},
+		"use per page and less than page": {
+			r:        ResultInfo{Page: 1, Total: 70, PerPage: 25},
+			expected: true,
+		},
 	}
 
 	for name, tc := range testCases {
diff --git a/registrar_test.go b/registrar_test.go
index 3b58be160..c1007a6cc 100644
--- a/registrar_test.go
+++ b/registrar_test.go
@@ -163,7 +163,7 @@ func TestRegistrarDomains(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
@@ -230,7 +230,7 @@ func TestTransferRegistrarDomain(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
@@ -297,7 +297,7 @@ func TestCancelRegistrarDomainTransfer(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}
 		`)
diff --git a/ssl_test.go b/ssl_test.go
index ea43e63cf..e7fb3798a 100644
--- a/ssl_test.go
+++ b/ssl_test.go
@@ -119,7 +119,7 @@ func TestListSSL(t *testing.T) {
             "page": 1,
             "per_page": 20,
             "count": 1,
-            "total_count": 2000
+            "total_count": 1
           }
         }`)
 	}
@@ -330,7 +330,7 @@ func TestReprioritizeSSL(t *testing.T) {
             "page": 1,
             "per_page": 20,
             "count": 1,
-            "total_count": 2000
+            "total_count": 1
           }
         }`)
 	}
diff --git a/teams_list_test.go b/teams_list_test.go
index f9d991a00..6dbf384f1 100644
--- a/teams_list_test.go
+++ b/teams_list_test.go
@@ -36,7 +36,7 @@ func TestTeamsLists(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000,
+				"total_count": 1,
 				"total_pages": 1
 			}
 		}
@@ -89,7 +89,7 @@ func TestTeamsList(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000,
+				"total_count": 1,
 				"total_pages": 1
 			}
 		}
@@ -143,7 +143,7 @@ func TestTeamsListItems(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000,
+				"total_count": 1,
 				"total_pages": 1
 			}
 		}
diff --git a/turnstile_test.go b/turnstile_test.go
index f2193431f..38e835514 100644
--- a/turnstile_test.go
+++ b/turnstile_test.go
@@ -122,7 +122,7 @@ func TestTurnstileWidget_List(t *testing.T) {
     "page": 1,
     "per_page": 20,
     "count": 1,
-    "total_count": 2000
+    "total_count": 1
   }
 }`)
 	})
diff --git a/waf_test.go b/waf_test.go
index 63bc2b034..31e19bef0 100644
--- a/waf_test.go
+++ b/waf_test.go
@@ -39,7 +39,7 @@ func TestListWAFPackages(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}`)
 	}
@@ -276,7 +276,7 @@ func TestListWAFGroups(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 			}`)
 	}
@@ -532,7 +532,7 @@ func TestListWAFRules(t *testing.T) {
 				"page": 1,
 				"per_page": 20,
 				"count": 1,
-				"total_count": 2000
+				"total_count": 1
 			}
 		}`)
 	}