Repository: cloudflare/cloudflare-go
Pull request #1327 (2023-07-05 04:31:44)

pagination: handle missing paginated responses
In the event a method tries to perform pagination without the `ResultInfo` present, just return the first page instead of sitting there indefinitely.



Patch:
diff --git a/pagination.go b/pagination.go
index 07d75ede8..696dc58ee 100644
--- a/pagination.go
+++ b/pagination.go
@@ -2,12 +2,32 @@ package cloudflare
 
 // 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 {
+		return true
+	}
+
 	return p.Page > 1 && p.Page > p.TotalPages
 }
 
 // Next advances the page of a paginated API response, but does not fetch the
 // next page of results.
 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 {
+		return p
+	}
+
+	// This shouldn't happen normally however, when it does just return the
+	// current page.
+	if p.Page > p.TotalPages {
+		return p
+	}
+
 	p.Page++
 	return p
 }
@@ -15,5 +35,9 @@ func (p ResultInfo) Next() ResultInfo {
 // HasMorePages returns whether there is another page of results after the
 // current one.
 func (p ResultInfo) HasMorePages() bool {
-	return p.Page > 1 && p.Page < p.TotalPages
+	if p.TotalPages == 0 {
+		return false
+	}
+
+	return p.Page >= 1 && p.Page < p.TotalPages
 }


Test patch:
diff --git a/pagination_test.go b/pagination_test.go
new file mode 100644
index 000000000..eb3b07ed2
--- /dev/null
+++ b/pagination_test.go
@@ -0,0 +1,106 @@
+package cloudflare
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestPagination_Done(t *testing.T) {
+	testCases := map[string]struct {
+		r        ResultInfo
+		expected bool
+	}{
+		"missing ResultInfo pagination information": {
+			r:        ResultInfo{Page: 1},
+			expected: true,
+		},
+		"total pages greater than page": {
+			r:        ResultInfo{Page: 1, TotalPages: 2},
+			expected: false,
+		},
+		"total pages greater than page (alot)": {
+			r:        ResultInfo{Page: 1, TotalPages: 200},
+			expected: false,
+		},
+		// this should never happen
+		"total pages less than page": {
+			r:        ResultInfo{Page: 3, TotalPages: 1},
+			expected: true,
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			assert.Equal(t, tc.r.Done(), tc.expected)
+		})
+	}
+}
+
+func TestPagination_Next(t *testing.T) {
+	testCases := map[string]struct {
+		r        ResultInfo
+		expected ResultInfo
+	}{
+		"missing ResultInfo pagination information": {
+			r:        ResultInfo{Page: 1},
+			expected: ResultInfo{Page: 1},
+		},
+		"total pages greater than page": {
+			r:        ResultInfo{Page: 1, TotalPages: 3},
+			expected: ResultInfo{Page: 2, TotalPages: 3},
+		},
+		"total pages greater than page (alot)": {
+			r:        ResultInfo{Page: 1, TotalPages: 3000},
+			expected: ResultInfo{Page: 2, TotalPages: 3000},
+		},
+		// bug, this should never happen
+		"total pages less than page": {
+			r:        ResultInfo{Page: 3, TotalPages: 1},
+			expected: ResultInfo{Page: 3, TotalPages: 1},
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			setup()
+			defer teardown()
+
+			assert.Equal(t, tc.r.Next(), tc.expected)
+		})
+	}
+}
+
+func TestPagination_HasMorePages(t *testing.T) {
+	testCases := map[string]struct {
+		r        ResultInfo
+		expected bool
+	}{
+		"missing ResultInfo pagination information": {
+			r:        ResultInfo{Page: 1},
+			expected: false,
+		},
+		"total pages greater than page": {
+			r:        ResultInfo{Page: 1, TotalPages: 3},
+			expected: true,
+		},
+		"total pages greater than page (alot)": {
+			r:        ResultInfo{Page: 1, TotalPages: 3000},
+			expected: true,
+		},
+		// bug, this should never happen
+		"total pages less than page": {
+			r:        ResultInfo{Page: 3, TotalPages: 1},
+			expected: false,
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			setup()
+			defer teardown()
+
+			assert.Equal(t, tc.r.HasMorePages(), tc.expected)
+		})
+	}
+}