Handle Go zero-value conflicts in sorting/filtering (#93)

Closes GH-88
dnsimple · May 2, 2020 · 554b1ea · 554b1ea
1 parent d209abf
commit 554b1ea
Show file tree

Hide file tree

Showing 19 changed files with 55 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,12 +3,16 @@
 
 #### master
 
+- FIXED: A zone record can be updated without the risk of overridig the name by mistake (dnsimple/dnsimple-go#88, dnsimple/dnsimple-go#93) 
+
 - FIXED: A zone record can be updated without the risk of overridig the name by mistake (dnsimple/dnsimple-go#33, dnsimple/dnsimple-go#92) 
 
 Incompatible changes:
 
 - CHANGED: CreateZoneRecord and UpdateZoneRecord now requires to use ZoneRecordAttributes instead of ZoneRecord. This is required to avoid conflicts caused by blank record names (dnsimple/dnsimple-go#92)
 
+- CHANGED: ListOptions now use pointer values (dnsimple/dnsimple-go#93)
+
 
 #### Release 0.50.0
 

diff --git a/README.md b/README.md
@@ -69,7 +69,7 @@ func main() {
     // Here's a few example:
 
     // get the list of domains filtered by name and sorted by expiration
-    client.Domains.ListDomains(context.Background(), accountID, &dnsimple.DomainListOptions{NameLike: "com", Sort: "expiration:DESC"})
+    client.Domains.ListDomains(context.Background(), accountID, &dnsimple.DomainListOptions{NameLike: dnsimple.StringP("com"), ListOptions: {Sort: dnsimple.StringP("expiration:DESC")}})
 }
 ```
 

diff --git a/dnsimple/certificates_test.go b/dnsimple/certificates_test.go
@@ -68,7 +68,7 @@ func TestCertificatesService_ListCertificates_WithOptions(t *testing.T) {
 		io.Copy(w, httpResponse.Body)
 	})
 
-	_, err := client.Certificates.ListCertificates(context.Background(), "1010", "example.com", &ListOptions{Page: 2, PerPage: 20})
+	_, err := client.Certificates.ListCertificates(context.Background(), "1010", "example.com", &ListOptions{Page: IntP(2), PerPage: IntP(20)})
 	if err != nil {
 		t.Fatalf("Certificates.ListCertificates() returned error: %v", err)
 	}

diff --git a/dnsimple/contacts_test.go b/dnsimple/contacts_test.go
@@ -69,7 +69,7 @@ func TestContactsService_List_WithOptions(t *testing.T) {
 		io.Copy(w, httpResponse.Body)
 	})
 
-	_, err := client.Contacts.ListContacts(context.Background(), "1010", &ListOptions{Page: 2, PerPage: 20})
+	_, err := client.Contacts.ListContacts(context.Background(), "1010", &ListOptions{Page: IntP(2), PerPage: IntP(20)})
 	if err != nil {
 		t.Fatalf("Contacts.ListContacts() returned error: %v", err)
 	}

diff --git a/dnsimple/dnsimple.go b/dnsimple/dnsimple.go
@@ -73,15 +73,15 @@ type Client struct {
 // in order to control parameters such as pagination and page number.
 type ListOptions struct {
 	// The page to return
-	Page int `url:"page,omitempty"`
+	Page *int `url:"page,omitempty"`
 
 	// The number of entries to return per page
-	PerPage int `url:"per_page,omitempty"`
+	PerPage *int `url:"per_page,omitempty"`
 
 	// The order criteria to sort the results.
 	// The value is a comma-separated list of field[:direction],
 	// eg. name | name:desc | name:desc,expiration:desc
-	Sort string `url:"sort,omitempty"`
+	Sort *string `url:"sort,omitempty"`
 }
 
 // NewClient returns a new DNSimple API client.
@@ -371,6 +371,10 @@ func addURLQueryOptions(path string, options interface{}) (string, error) {
 	return u.String(), nil
 }
 
+// IntP is a helper routine that allocates a new int value
+// to store v and returns a pointer to it.
+func IntP(v int) *int { return &v }
+
 // Int64P is a helper routine that allocates a new int64 value
 // to store v and returns a pointer to it.
 func Int64P(v int64) *int64 { return &v }

diff --git a/dnsimple/domains.go b/dnsimple/domains.go
@@ -53,10 +53,10 @@ type DomainsResponse struct {
 // to customize the DomainsService.ListDomains method.
 type DomainListOptions struct {
 	// Select domains where the name contains given string.
-	NameLike string `url:"name_like,omitempty"`
+	NameLike *string `url:"name_like,omitempty"`
 
 	// Select domains where the registrant matches given ID.
-	RegistrantID int `url:"registrant_id,omitempty"`
+	RegistrantID *int `url:"registrant_id,omitempty"`
 
 	ListOptions
 }

diff --git a/dnsimple/domains_collaborators_test.go b/dnsimple/domains_collaborators_test.go
@@ -74,7 +74,7 @@ func TestDomainsService_ListCollaborators_WithOptions(t *testing.T) {
 		io.Copy(w, httpResponse.Body)
 	})
 
-	_, err := client.Domains.ListCollaborators(context.Background(), "1010", "example.com", &ListOptions{Page: 2, PerPage: 20})
+	_, err := client.Domains.ListCollaborators(context.Background(), "1010", "example.com", &ListOptions{Page: IntP(2), PerPage: IntP(20)})
 	if err != nil {
 		t.Fatalf("Domains.ListCollaborators() returned error: %v", err)
 	}

diff --git a/dnsimple/domains_delegation_signer_records_test.go b/dnsimple/domains_delegation_signer_records_test.go
@@ -68,7 +68,7 @@ func TestDomainsService_ListDelegationSignerRecords_WithOptions(t *testing.T) {
 		io.Copy(w, httpResponse.Body)
 	})
 
-	_, err := client.Domains.ListDelegationSignerRecords(context.Background(), "1010", "example.com", &ListOptions{Page: 2, PerPage: 20})
+	_, err := client.Domains.ListDelegationSignerRecords(context.Background(), "1010", "example.com", &ListOptions{Page: IntP(2), PerPage: IntP(20)})
 	if err != nil {
 		t.Fatalf("Domains.ListDelegationSignerRecords() returned error: %v", err)
 	}

diff --git a/dnsimple/domains_email_forwards_test.go b/dnsimple/domains_email_forwards_test.go
@@ -71,7 +71,7 @@ func TestDomainsService_EmailForwardsList_WithOptions(t *testing.T) {
 		io.Copy(w, httpResponse.Body)
 	})
 
-	_, err := client.Domains.ListEmailForwards(context.Background(), "1010", "example.com", &ListOptions{Page: 2, PerPage: 20})
+	_, err := client.Domains.ListEmailForwards(context.Background(), "1010", "example.com", &ListOptions{Page: IntP(2), PerPage: IntP(20)})
 	if err != nil {
 		t.Fatalf("Domains.ListEmailForwards() returned error: %v", err)
 	}

diff --git a/dnsimple/domains_pushes_test.go b/dnsimple/domains_pushes_test.go
@@ -101,7 +101,7 @@ func TestDomainsService_DomainsPushesList_WithOptions(t *testing.T) {
 		io.Copy(w, httpResponse.Body)
 	})
 
-	_, err := client.Domains.ListPushes(context.Background(), "2020", &ListOptions{Page: 2, PerPage: 20})
+	_, err := client.Domains.ListPushes(context.Background(), "2020", &ListOptions{Page: IntP(2), PerPage: IntP(20)})
 	if err != nil {
 		t.Fatalf("Domains.ListPushes() returned error: %v", err)
 	}

diff --git a/dnsimple/domains_test.go b/dnsimple/domains_test.go
@@ -75,7 +75,7 @@ func TestDomainsService_ListDomains_WithOptions(t *testing.T) {
 		io.Copy(w, httpResponse.Body)
 	})
 
-	_, err := client.Domains.ListDomains(context.Background(), "1010", &DomainListOptions{"example", 10, ListOptions{Page: 2, PerPage: 20, Sort: "name,expiration:desc"}})
+	_, err := client.Domains.ListDomains(context.Background(), "1010", &DomainListOptions{NameLike: StringP("example"), RegistrantID: IntP(10), ListOptions: ListOptions{Page: IntP(2), PerPage: IntP(20), Sort: StringP("name,expiration:desc")}})
 	if err != nil {
 		t.Fatalf("Domains.ListDomains() returned error: %v", err)
 	}

diff --git a/dnsimple/services_test.go b/dnsimple/services_test.go
@@ -69,7 +69,7 @@ func TestServicesService_List_WithOptions(t *testing.T) {
 		io.Copy(w, httpResponse.Body)
 	})
 
-	_, err := client.Services.ListServices(context.Background(), &ListOptions{Page: 2, PerPage: 20})
+	_, err := client.Services.ListServices(context.Background(), &ListOptions{Page: IntP(2), PerPage: IntP(20)})
 	if err != nil {
 		t.Fatalf("Services.ListServices() returned error: %v", err)
 	}

diff --git a/dnsimple/templates_records_test.go b/dnsimple/templates_records_test.go
@@ -69,7 +69,7 @@ func TestTemplatesService_ListTemplateRecords_WithOptions(t *testing.T) {
 		io.Copy(w, httpResponse.Body)
 	})
 
-	_, err := client.Templates.ListTemplateRecords(context.Background(), "1010", "1", &ListOptions{Page: 2, PerPage: 20})
+	_, err := client.Templates.ListTemplateRecords(context.Background(), "1010", "1", &ListOptions{Page: IntP(2), PerPage: IntP(20)})
 	if err != nil {
 		t.Fatalf("Templates.ListTemplateRecords() returned error: %v", err)
 	}

diff --git a/dnsimple/templates_test.go b/dnsimple/templates_test.go
@@ -69,7 +69,7 @@ func TestTemplatesService_List_WithOptions(t *testing.T) {
 		io.Copy(w, httpResponse.Body)
 	})
 
-	_, err := client.Templates.ListTemplates(context.Background(), "1010", &ListOptions{Page: 2, PerPage: 20})
+	_, err := client.Templates.ListTemplates(context.Background(), "1010", &ListOptions{Page: IntP(2), PerPage: IntP(20)})
 	if err != nil {
 		t.Fatalf("Templates.ListTemplates() returned error: %v", err)
 	}

diff --git a/dnsimple/tlds_test.go b/dnsimple/tlds_test.go
@@ -71,7 +71,7 @@ func TestTldsService_ListTlds_WithOptions(t *testing.T) {
 		io.Copy(w, httpResponse.Body)
 	})
 
-	_, err := client.Tlds.ListTlds(context.Background(), &ListOptions{Page: 2, PerPage: 20})
+	_, err := client.Tlds.ListTlds(context.Background(), &ListOptions{Page: IntP(2), PerPage: IntP(20)})
 	if err != nil {
 		t.Fatalf("Tlds.ListTlds() returned error: %v", err)
 	}

diff --git a/dnsimple/zones.go b/dnsimple/zones.go
@@ -50,7 +50,7 @@ type ZoneFileResponse struct {
 // to customize the ZonesService.ListZones method.
 type ZoneListOptions struct {
 	// Select domains where the name contains given string.
-	NameLike string `url:"name_like,omitempty"`
+	NameLike *string `url:"name_like,omitempty"`
 
 	ListOptions
 }

diff --git a/dnsimple/zones_records.go b/dnsimple/zones_records.go
@@ -60,14 +60,14 @@ type ZoneRecordsResponse struct {
 // to customize the ZonesService.ListZoneRecords method.
 type ZoneRecordListOptions struct {
 	// Select records where the name matches given string.
-	Name string `url:"name,omitempty"`
+	Name *string `url:"name,omitempty"`
 
 	// Select records where the name contains given string.
-	NameLike string `url:"name_like,omitempty"`
+	NameLike *string `url:"name_like,omitempty"`
 
 	// Select records of given type.
 	// Eg. TXT, A, NS.
-	Type string `url:"type,omitempty"`
+	Type *string `url:"type,omitempty"`
 
 	ListOptions
 }

diff --git a/dnsimple/zones_records_test.go b/dnsimple/zones_records_test.go
@@ -78,7 +78,31 @@ func TestZonesService_ListRecords_WithOptions(t *testing.T) {
 		io.Copy(w, httpResponse.Body)
 	})
 
-	_, err := client.Zones.ListRecords(context.Background(), "1010", "example.com", &ZoneRecordListOptions{"example", "www", "A", ListOptions{Page: 2, PerPage: 20, Sort: "name,expiration:desc"}})
+	_, err := client.Zones.ListRecords(context.Background(), "1010", "example.com", &ZoneRecordListOptions{StringP("example"), StringP("www"), StringP("A"), ListOptions{Page: IntP(2), PerPage: IntP(20), Sort: StringP("name,expiration:desc")}})
+	if err != nil {
+		t.Fatalf("Zones.ListRecords() returned error: %v", err)
+	}
+}
+
+func TestZonesService_ListRecords_WithOptionsSomeBlank(t *testing.T) {
+	setupMockServer()
+	defer teardownMockServer()
+
+	mux.HandleFunc("/v2/1010/zones/example.com/records", func(w http.ResponseWriter, r *http.Request) {
+		httpResponse := httpResponseFixture(t, "/api/listZoneRecords/success.http")
+
+		testQuery(t, r, url.Values{
+			"page": []string{"2"},
+			"sort": []string{"name,expiration:desc"},
+			"name": []string{"example"},
+			"type": []string{"A"},
+		})
+
+		w.WriteHeader(httpResponse.StatusCode)
+		io.Copy(w, httpResponse.Body)
+	})
+
+	_, err := client.Zones.ListRecords(context.Background(), "1010", "example.com", &ZoneRecordListOptions{Name: StringP("example"), Type: StringP("A"), ListOptions: ListOptions{Page: IntP(2), Sort: StringP("name,expiration:desc")}})
 	if err != nil {
 		t.Fatalf("Zones.ListRecords() returned error: %v", err)
 	}

diff --git a/dnsimple/zones_test.go b/dnsimple/zones_test.go
@@ -63,7 +63,7 @@ func TestZonesService_ListZones_WithOptions(t *testing.T) {
 		io.Copy(w, httpResponse.Body)
 	})
 
-	_, err := client.Zones.ListZones(context.Background(), "1010", &ZoneListOptions{"example", ListOptions{Page: 2, PerPage: 20, Sort: "name,expiration:desc"}})
+	_, err := client.Zones.ListZones(context.Background(), "1010", &ZoneListOptions{StringP("example"), ListOptions{Page: IntP(2), PerPage: IntP(20), Sort: StringP("name,expiration:desc")}})
 	if err != nil {
 		t.Fatalf("Zones.ListZones() returned error: %v", err)
 	}