Skip to content

Commit

Permalink
Match ListDNSRecords per_page value to the API's
Browse files Browse the repository at this point in the history
As outlined in github.com//pull/1169,
when no pagination options are passed in to ListDNSRecords,
autopagination is performed. However, the current PerPage
value doesn't match that of the official API. This commit
makes both values match. A new test is also implemented that
more explicitly tests the auto pagination behaviour.

Signed-off-by: Artur Rodrigues <[email protected]>
  • Loading branch information
Artur Rodrigues committed Jan 9, 2023
1 parent 63d7aa8 commit f1a4010
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 4 deletions.
10 changes: 8 additions & 2 deletions dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ type DNSListResponse struct {
ResultInfo `json:"result_info"`
}

// listDNSRecordsDefaultPageSize represents the default per_page size of the API
var listDNSRecordsDefaultPageSize int = 100

// nontransitionalLookup implements the nontransitional processing as specified in
// Unicode Technical Standard 46 with almost all checkings off to maximize user freedom.
var nontransitionalLookup = idna.New(
Expand Down Expand Up @@ -152,7 +155,10 @@ func (api *API) CreateDNSRecord(ctx context.Context, rc *ResourceContainer, para
return recordResp, nil
}

// ListDNSRecords returns a slice of DNS records for the given zone identifier.
// ListDNSRecords returns a slice of DNS records for the given zone
// identifier. If params doesn't include any pagination (ResultInfo)
// options, auto pagination is performed with the default page size
// of 100 records per request.
//
// API reference: https://api.cloudflare.com/#dns-records-for-a-zone-list-dns-records
func (api *API) ListDNSRecords(ctx context.Context, rc *ResourceContainer, params ListDNSRecordsParams) ([]DNSRecord, *ResultInfo, error) {
Expand All @@ -170,7 +176,7 @@ func (api *API) ListDNSRecords(ctx context.Context, rc *ResourceContainer, param
}

if params.PerPage < 1 {
params.PerPage = 50
params.PerPage = listDNSRecordsDefaultPageSize
}

if params.Page < 1 {
Expand Down
37 changes: 35 additions & 2 deletions dns_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ func TestCreateDNSRecord(t *testing.T) {
assert.Equal(t, want, actual)
}

func TestDNSRecords(t *testing.T) {
func TestListDNSRecords(t *testing.T) {
setup()
defer teardown()

Expand Down Expand Up @@ -242,7 +242,7 @@ func TestDNSRecords(t *testing.T) {
assert.Equal(t, want, actual)
}

func TestDNSRecordsSearch(t *testing.T) {
func TestListDNSRecordsSearch(t *testing.T) {
setup()
defer teardown()

Expand Down Expand Up @@ -341,6 +341,39 @@ func TestDNSRecordsSearch(t *testing.T) {
assert.Equal(t, want, actual)
}

func TestListDNSRecordsPagination(t *testing.T) {
// change listDNSRecordsDefaultPageSize value to 1 to force pagination
listDNSRecordsDefaultPageSize = 1

setup()
defer teardown()

var page1Called, page2Called bool
handler := func(w http.ResponseWriter, r *http.Request) {
page := r.URL.Query().Get("page")
w.Header().Set("content-type", "application/json")

var response string
switch page {
case "1":
response = loadFixture("dns", "list_page_1")
page1Called = true
case "2":
response = loadFixture("dns", "list_page_2")
page2Called = true
}
fmt.Fprint(w, response)
}

mux.HandleFunc("/zones/"+testZoneID+"/dns_records", handler)

actual, _, err := client.ListDNSRecords(context.Background(), ZoneIdentifier(testZoneID), ListDNSRecordsParams{})
require.NoError(t, err)
assert.True(t, page1Called)
assert.True(t, page2Called)
assert.Len(t, actual, 2)
}

func TestDNSRecord(t *testing.T) {
setup()
defer teardown()
Expand Down
37 changes: 37 additions & 0 deletions testdata/fixtures/dns/list_page_1.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{
"success": true,
"errors": [],
"messages": [],
"result": [
{
"id": "372e67954025e0ba6aaa6d586b9e0b59",
"type": "A",
"name": "example.com",
"content": "198.51.100.4",
"proxiable": true,
"proxied": true,
"ttl": 120,
"locked": false,
"zone_id": "d56084adb405e0b7e32c52321bf07be6",
"zone_name": "example.com",
"created_on": "2014-01-01T05:20:00Z",
"modified_on": "2014-01-01T05:20:00Z",
"data": {},
"meta": {
"auto_added": true,
"source": "primary"
},
"tags": [
"tag1",
"tag2extended"
]
}
],
"result_info": {
"count": 1,
"page": 1,
"per_page": 1,
"total_count": 2,
"total_pages": 2
}
}
37 changes: 37 additions & 0 deletions testdata/fixtures/dns/list_page_2.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{
"success": true,
"errors": [],
"messages": [],
"result": [
{
"id": "372e67954025e0ba6aaa6d586b9e0b59",
"type": "A",
"name": "www.example.com",
"content": "198.51.100.4",
"proxiable": true,
"proxied": true,
"ttl": 120,
"locked": false,
"zone_id": "d56084adb405e0b7e32c52321bf07be6",
"zone_name": "example.com",
"created_on": "2014-01-01T05:20:00Z",
"modified_on": "2014-01-01T05:20:00Z",
"data": {},
"meta": {
"auto_added": true,
"source": "primary"
},
"tags": [
"tag1",
"tag2extended"
]
}
],
"result_info": {
"count": 1,
"page": 2,
"per_page": 1,
"total_count": 2,
"total_pages": 2
}
}

0 comments on commit f1a4010

Please sign in to comment.