Skip to content

Commit e2e5623

Browse files
dwoolgerdavid.woolger@civo.com
andauthored
Auto-paginate the no-arg List* helpers (#285)
ListIPs, ListAccounts, ListApplications, ListDatabases, ListKubernetesClusters and ListObjectStores previously sent a single GET /v2/... with no pagination parameters and silently returned whatever fit on the server-side first page (default 20 items). For accounts with more resources than that — typical in production — page 2 onward was invisible to civogo callers including civo-cli (`civo ip ls`) and terraform-provider-civo (`civo_reserved_ip`, `civo_kubernetes_cluster`, `civo_object_store`, `civo_database`). The customer ticket that surfaced this: an account with > 20 Reserved IPs in fra1 / lon1 could not see one specific IP via the CLI or Terraform. Tracked in api-go epic civo&598 (server-side off-by-one, fixed in civo/api-go!1354) and civogo epic civo&599 (this change). The fix introduces a shared `paginateAll` helper in `pagination.go` and routes the six no-arg `List*` functions through it. The merged response always reports `Page=1, Pages=1, PerPage=len(Items)`. Iteration is sequential and capped (100 pages / 10,000 items) with an explicit ErrPaginationCapExceeded rather than spinning forever. Two existing magic-number workarounds were retired in the same change: - ListAllInstances no longer uses ListInstances(1, 99999999). - FindObjectStoreCredential no longer uses ListObjectStoreCredentials(1, 10000). Both depended on api-go's paginator early-return path and would silently re-truncate the day server-side adds a per_page cap. They now share the same iterator. Includes regression tests covering: single page, multi-page in order, trailing-item-on-last-page (the customer's case), and hard-cap exhaustion. Updated affected fixtures that asserted the old buggy semantics (Pages>1 on a single-item response, PerPage=20 on the merged result). Co-authored-by: david.woolger@civo.com <david.woolger@civo.com>
1 parent 915272e commit e2e5623

17 files changed

Lines changed: 377 additions & 70 deletions

CHANGELOG.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,21 @@
11

2+
Unreleased
3+
=============
4+
5+
* **Behaviour change**: `ListIPs`, `ListAccounts`, `ListApplications`,
6+
`ListDatabases`, `ListKubernetesClusters`, and `ListObjectStores` now
7+
iterate server-side pagination internally and return every item the
8+
account owns, not just the first 20. The merged response always reports
9+
`Page=1, Pages=1, PerPage=len(Items)`. Callers iterating `Items` are
10+
unaffected; callers inspecting `Page`/`Pages`/`PerPage` to drive their
11+
own iteration should drop that logic.
12+
* `ListAllInstances` and `FindObjectStoreCredential` no longer use the
13+
brittle `per_page=99999999` / `per_page=10000` workaround; both go
14+
through the new shared iterator (`paginateAll` in `pagination.go`).
15+
* Hard cap: each `List*` call returns `ErrPaginationCapExceeded` rather
16+
than spinning if the server reports more than 100 pages / 10,000 items.
17+
* Originating customer report: api-go epic civo&598 + civogo epic civo&599.
18+
219
0.2.57
320
=============
421
2021-11-02

account.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package civogo
33
import (
44
"bytes"
55
"encoding/json"
6+
"fmt"
67
)
78

89
// PaginatedAccounts returns a paginated list of Account object
@@ -15,17 +16,21 @@ type PaginatedAccounts struct {
1516

1617
// ListAccounts lists all accounts
1718
func (c *Client) ListAccounts() (*PaginatedAccounts, error) {
18-
resp, err := c.SendGetRequest("/v2/accounts")
19+
items, err := paginateAll("accounts", func(page, perPage int) ([]Account, int, error) {
20+
resp, err := c.SendGetRequest(fmt.Sprintf("/v2/accounts?page=%d&per_page=%d", page, perPage))
21+
if err != nil {
22+
return nil, 0, decodeError(err)
23+
}
24+
pg := PaginatedAccounts{}
25+
if err := json.NewDecoder(bytes.NewReader(resp)).Decode(&pg); err != nil {
26+
return nil, 0, decodeError(err)
27+
}
28+
return pg.Items, pg.Pages, nil
29+
})
1930
if err != nil {
20-
return nil, decodeError(err)
31+
return nil, err
2132
}
22-
23-
accounts := &PaginatedAccounts{}
24-
if err := json.NewDecoder(bytes.NewReader(resp)).Decode(&accounts); err != nil {
25-
return nil, decodeError(err)
26-
}
27-
28-
return accounts, nil
33+
return &PaginatedAccounts{Page: 1, Pages: 1, PerPage: len(items), Items: items}, nil
2934
}
3035

3136
// GetAccountID returns the account ID

application.go

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -75,17 +75,21 @@ var ErrAppDomainNotFound = fmt.Errorf("domain not found")
7575

7676
// ListApplications returns all applications in that specific region
7777
func (c *Client) ListApplications() (*PaginatedApplications, error) {
78-
resp, err := c.SendGetRequest("/v2/applications")
78+
items, err := paginateAll("applications", func(page, perPage int) ([]Application, int, error) {
79+
resp, err := c.SendGetRequest(fmt.Sprintf("/v2/applications?page=%d&per_page=%d", page, perPage))
80+
if err != nil {
81+
return nil, 0, decodeError(err)
82+
}
83+
pg := PaginatedApplications{}
84+
if err := json.NewDecoder(bytes.NewReader(resp)).Decode(&pg); err != nil {
85+
return nil, 0, decodeError(err)
86+
}
87+
return pg.Items, pg.Pages, nil
88+
})
7989
if err != nil {
80-
return nil, decodeError(err)
81-
}
82-
83-
application := &PaginatedApplications{}
84-
if err := json.NewDecoder(bytes.NewReader(resp)).Decode(&application); err != nil {
85-
return nil, decodeError(err)
90+
return nil, err
8691
}
87-
88-
return application, nil
92+
return &PaginatedApplications{Page: 1, Pages: 1, PerPage: len(items), Items: items}, nil
8993
}
9094

9195
// GetApplication returns an application by ID

application_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,11 @@ func TestListApplications(t *testing.T) {
4040
return
4141
}
4242

43+
// The SDK auto-paginates: the merged response always reports
44+
// Page=1, Pages=1, PerPage=len(Items) — see paginateAll in pagination.go.
4345
expected := &PaginatedApplications{
4446
Page: 1,
45-
PerPage: 20,
47+
PerPage: 1,
4648
Pages: 1,
4749
Items: []Application{
4850
{

database.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,17 +81,21 @@ type RestoreDatabaseRequest struct {
8181

8282
// ListDatabases returns a list of all databases
8383
func (c *Client) ListDatabases() (*PaginatedDatabases, error) {
84-
resp, err := c.SendGetRequest("/v2/databases")
84+
items, err := paginateAll("databases", func(page, perPage int) ([]Database, int, error) {
85+
resp, err := c.SendGetRequest(fmt.Sprintf("/v2/databases?page=%d&per_page=%d", page, perPage))
86+
if err != nil {
87+
return nil, 0, decodeError(err)
88+
}
89+
pg := PaginatedDatabases{}
90+
if err := json.NewDecoder(bytes.NewReader(resp)).Decode(&pg); err != nil {
91+
return nil, 0, err
92+
}
93+
return pg.Items, pg.Pages, nil
94+
})
8595
if err != nil {
86-
return nil, decodeError(err)
87-
}
88-
89-
databases := &PaginatedDatabases{}
90-
if err := json.NewDecoder(bytes.NewReader(resp)).Decode(&databases); err != nil {
9196
return nil, err
9297
}
93-
94-
return databases, nil
98+
return &PaginatedDatabases{Page: 1, Pages: 1, PerPage: len(items), Items: items}, nil
9599
}
96100

97101
// GetDatabase finds a database by the database UUID

database_test.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77

88
func TestListDatabases(t *testing.T) {
99
client, server, _ := NewClientForTesting(map[string]string{
10-
"/v2/databases": `{"page": 1, "per_page": 20, "pages": 2, "items":[{"id": "12345", "name": "test-db"}]}`,
10+
"/v2/databases": `{"page": 1, "per_page": 20, "pages": 1, "items":[{"id": "12345", "name": "test-db"}]}`,
1111
})
1212
defer server.Close()
1313

@@ -17,10 +17,12 @@ func TestListDatabases(t *testing.T) {
1717
return
1818
}
1919

20+
// The SDK auto-paginates: the merged response always reports
21+
// Page=1, Pages=1, PerPage=len(Items) — see paginateAll in pagination.go.
2022
expected := &PaginatedDatabases{
2123
Page: 1,
22-
PerPage: 20,
23-
Pages: 2,
24+
PerPage: 1,
25+
Pages: 1,
2426
Items: []Database{
2527
{
2628
ID: "12345",

instance.go

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -152,14 +152,20 @@ func (c *Client) ListInstances(page int, perPage int) (*PaginatedInstanceList, e
152152
return &PaginatedInstances, err
153153
}
154154

155-
// ListAllInstances returns all (well, upto 99,999,999 instances) Instances owned by the calling API account
155+
// ListAllInstances returns every Instance owned by the calling API account
156+
// by iterating the paginated /v2/instances endpoint until exhausted.
156157
func (c *Client) ListAllInstances() ([]Instance, error) {
157-
instances, err := c.ListInstances(1, 99999999)
158+
items, err := paginateAll("instances", func(page, perPage int) ([]Instance, int, error) {
159+
pg, err := c.ListInstances(page, perPage)
160+
if err != nil {
161+
return nil, 0, err
162+
}
163+
return pg.Items, pg.Pages, nil
164+
})
158165
if err != nil {
159-
return []Instance{}, decodeError(err)
166+
return []Instance{}, err
160167
}
161-
162-
return instances.Items, nil
168+
return items, nil
163169
}
164170

165171
// FindInstance finds a instance by either part of the ID or part of the hostname

instance_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import (
66

77
func TestListInstances(t *testing.T) {
88
client, server, _ := NewClientForTesting(map[string]string{
9-
"/v2/instances": `{"page": 1, "per_page": 20, "pages": 2, "items":[{"id": "12345", "hostname": "foo.example.com"}]}`,
9+
"/v2/instances": `{"page": 1, "per_page": 20, "pages": 1, "items":[{"id": "12345", "hostname": "foo.example.com"}]}`,
1010
})
1111
defer server.Close()
1212

@@ -26,7 +26,7 @@ func TestFindInstance(t *testing.T) {
2626
{
2727
"page": 1,
2828
"per_page": 20,
29-
"pages": 2,
29+
"pages": 1,
3030
"items": [
3131
{
3232
"id": "12345",

ip.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,17 +61,21 @@ type Actions struct {
6161

6262
// ListIPs returns all reserved IPs in that specific region
6363
func (c *Client) ListIPs() (*PaginatedIPs, error) {
64-
resp, err := c.SendGetRequest("/v2/ips")
64+
items, err := paginateAll("ips", func(page, perPage int) ([]IP, int, error) {
65+
resp, err := c.SendGetRequest(fmt.Sprintf("/v2/ips?page=%d&per_page=%d", page, perPage))
66+
if err != nil {
67+
return nil, 0, decodeError(err)
68+
}
69+
pg := PaginatedIPs{}
70+
if err := json.NewDecoder(bytes.NewReader(resp)).Decode(&pg); err != nil {
71+
return nil, 0, err
72+
}
73+
return pg.Items, pg.Pages, nil
74+
})
6575
if err != nil {
66-
return nil, decodeError(err)
67-
}
68-
69-
ips := &PaginatedIPs{}
70-
if err := json.NewDecoder(bytes.NewReader(resp)).Decode(&ips); err != nil {
7176
return nil, err
7277
}
73-
74-
return ips, nil
78+
return &PaginatedIPs{Page: 1, Pages: 1, PerPage: len(items), Items: items}, nil
7579
}
7680

7781
// GetIP finds an reserved IP by the full ID

ip_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,11 @@ func TestListIPs(t *testing.T) {
2828
return
2929
}
3030

31+
// The SDK auto-paginates: the merged response always reports
32+
// Page=1, Pages=1, PerPage=len(Items) — see paginateAll in pagination.go.
3133
expected := &PaginatedIPs{
3234
Page: 1,
33-
PerPage: 20,
35+
PerPage: 1,
3436
Pages: 1,
3537
Items: []IP{
3638
{

0 commit comments

Comments
 (0)