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.

Sign up for GitHub

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

zone: allow ListZonesContext to automatically handle pagination #534

Merged
merged 2 commits into from
Jan 17, 2021
Merged

zone: allow ListZonesContext to automatically handle pagination #534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7e47b9e
zone: allow `ListZonesContext` to automatically handle pagination
jacobbednarz Sep 27, 2020
259a650
add `total_pages` to stubbed responses
jacobbednarz Sep 30, 2020
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
9 changes: 6 additions & 3 deletions cloudflare_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,8 @@ func TestZoneIDByNameWithNonUniqueZonesWithoutOrgID(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 2000,
"total_pages": 1
}
}
`)
Expand Down Expand Up @@ -470,7 +471,8 @@ func TestZoneIDByNameWithNonUniqueZonesWithOrgId(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 2000,
"total_pages": 1
}
}
`)
Expand Down Expand Up @@ -609,7 +611,8 @@ func TestZoneIDByNameWithIDN(t *testing.T) {
"page": 1,
"per_page": 20,
"count": 1,
"total_count": 2000
"total_count": 2000,
"total_pages": 1
}
}
`)
Expand Down
46 changes: 45 additions & 1 deletion zone.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"fmt"
"net/url"
"strconv"
"sync"
"time"

Expand Down Expand Up @@ -354,6 +355,7 @@ func (api *API) ZoneActivationCheck(zoneID string) (Response, error) {
// API reference: https://api.cloudflare.com/#zone-list-zones
func (api *API) ListZones(z ...string) ([]Zone, error) {
v := url.Values{}

var res []byte
var r ZonesResponse
var zones []Zone
Expand Down Expand Up @@ -425,16 +427,21 @@ func (api *API) ListZones(z ...string) ([]Zone, error) {
return zones, nil
}

// ListZonesContext lists zones on an account. Optionally takes a list of ReqOptions.
// ListZonesContext lists all zones on an account automatically handling the
// pagination. Optionally takes a list of ReqOptions.
func (api *API) ListZonesContext(ctx context.Context, opts ...ReqOption) (r ZonesResponse, err error) {
var res []byte
var zones []Zone

opt := reqOption{
params: url.Values{},
}
for _, of := range opts {
of(&opt)
}

opt.params.Add("per_page", "50")

res, err = api.makeRequestContext(ctx, "GET", "/zones?"+opt.params.Encode(), nil)
if err != nil {
return ZonesResponse{}, errors.Wrap(err, errMakeRequestError)
Expand All @@ -444,6 +451,43 @@ func (api *API) ListZonesContext(ctx context.Context, opts ...ReqOption) (r Zone
return ZonesResponse{}, errors.Wrap(err, errUnmarshalError)
}

totalPageCount := r.TotalPages
var wg sync.WaitGroup
wg.Add(totalPageCount)
errc := make(chan error)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem we handle errors returned from channel in any way. We should return them at the end of the function, even a single one is enough. We might also want to cancel pending requests if any goroutine fails this way.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this what you mean? Or did you have something else in mind for the error being returned? I'm not explicitly cancelling the routine but can drop it in if the channel error reporting isn't enough here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Circling back on this one @patryk as we've still got an issue open in the Terraform provider depending on this one.


for i := 1; i <= totalPageCount; i++ {
go func(pageNumber int) error {
opt.params.Set("page", strconv.Itoa(pageNumber))
res, err = api.makeRequestContext(ctx, "GET", "/zones?"+opt.params.Encode(), nil)
if err != nil {
errc <- err
}

err = json.Unmarshal(res, &r)
if err != nil {
errc <- err
}

for _, zone := range r.Result {
zones = append(zones, zone)
}

select {
case err := <-errc:
return err
default:
wg.Done()
}

return nil
}(i)
}

wg.Wait()

r.Result = zones

return r, nil
}

Expand Down