Add checks for arguments

Adds checks to determine if the request will actually be valid by prefiltering invalid defaults (less than 0, empty strings, nils). The goal of this change is eliminate unnecceary API calls.
bsnyder788 · Sep 25, 2015 · af5057f · af5057f
1 parent 7712039
commit af5057f
Show file tree

Hide file tree

Showing 9 changed files with 216 additions and 4 deletions.
diff --git a/action.go b/action.go
@@ -74,8 +74,12 @@ func (s *ActionsServiceOp) List(opt *ListOptions) ([]Action, *Response, error) {
 	return root.Actions, resp, err
 }
 
-// Get an action by ID
+// Get an action by ID.
 func (s *ActionsServiceOp) Get(id int) (*Action, *Response, error) {
+	if id < 1 {
+		return nil, nil, NewArgError("id", "cannot be less than 1")
+	}
+
 	path := fmt.Sprintf("%s/%d", actionsBasePath, id)
 	req, err := s.client.NewRequest("GET", path, nil)
 	if err != nil {

diff --git a/domains.go b/domains.go
@@ -87,7 +87,7 @@ func (d Domain) String() string {
 	return Stringify(d)
 }
 
-// List all domains
+// List all domains.
 func (s DomainsServiceOp) List(opt *ListOptions) ([]Domain, *Response, error) {
 	path := domainsBasePath
 	path, err := addOptions(path, opt)
@@ -112,8 +112,12 @@ func (s DomainsServiceOp) List(opt *ListOptions) ([]Domain, *Response, error) {
 	return root.Domains, resp, err
 }
 
-// Get individual domain
+// Get individual domain. It requires a non-empty domain name.
 func (s *DomainsServiceOp) Get(name string) (*Domain, *Response, error) {
+	if len(name) < 1 {
+		return nil, nil, NewArgError("name", "cannot be an empty string")
+	}
+
 	path := fmt.Sprintf("%s/%s", domainsBasePath, name)
 
 	req, err := s.client.NewRequest("GET", path, nil)
@@ -132,6 +136,10 @@ func (s *DomainsServiceOp) Get(name string) (*Domain, *Response, error) {
 
 // Create a new domain
 func (s *DomainsServiceOp) Create(createRequest *DomainCreateRequest) (*Domain, *Response, error) {
+	if createRequest == nil {
+		return nil, nil, NewArgError("createRequest", "cannot be nil")
+	}
+
 	path := domainsBasePath
 
 	req, err := s.client.NewRequest("POST", path, createRequest)
@@ -149,6 +157,10 @@ func (s *DomainsServiceOp) Create(createRequest *DomainCreateRequest) (*Domain,
 
 // Delete domain
 func (s *DomainsServiceOp) Delete(name string) (*Response, error) {
+	if len(name) < 1 {
+		return nil, NewArgError("name", "cannot be an empty string")
+	}
+
 	path := fmt.Sprintf("%s/%s", domainsBasePath, name)
 
 	req, err := s.client.NewRequest("DELETE", path, nil)
@@ -173,6 +185,10 @@ func (d DomainRecordEditRequest) String() string {
 
 // Records returns a slice of DomainRecords for a domain
 func (s *DomainsServiceOp) Records(domain string, opt *ListOptions) ([]DomainRecord, *Response, error) {
+	if len(domain) < 1 {
+		return nil, nil, NewArgError("domain", "cannot be an empty string")
+	}
+
 	path := fmt.Sprintf("%s/%s/records", domainsBasePath, domain)
 	path, err := addOptions(path, opt)
 	if err != nil {
@@ -198,6 +214,14 @@ func (s *DomainsServiceOp) Records(domain string, opt *ListOptions) ([]DomainRec
 
 // Record returns the record id from a domain
 func (s *DomainsServiceOp) Record(domain string, id int) (*DomainRecord, *Response, error) {
+	if len(domain) < 1 {
+		return nil, nil, NewArgError("domain", "cannot be an empty string")
+	}
+
+	if id < 1 {
+		return nil, nil, NewArgError("id", "cannot be less than 1")
+	}
+
 	path := fmt.Sprintf("%s/%s/records/%d", domainsBasePath, domain, id)
 
 	req, err := s.client.NewRequest("GET", path, nil)
@@ -216,6 +240,14 @@ func (s *DomainsServiceOp) Record(domain string, id int) (*DomainRecord, *Respon
 
 // DeleteRecord deletes a record from a domain identified by id
 func (s *DomainsServiceOp) DeleteRecord(domain string, id int) (*Response, error) {
+	if len(domain) < 1 {
+		return nil, NewArgError("domain", "cannot be an empty string")
+	}
+
+	if id < 1 {
+		return nil, NewArgError("id", "cannot be less than 1")
+	}
+
 	path := fmt.Sprintf("%s/%s/records/%d", domainsBasePath, domain, id)
 
 	req, err := s.client.NewRequest("DELETE", path, nil)
@@ -232,7 +264,20 @@ func (s *DomainsServiceOp) DeleteRecord(domain string, id int) (*Response, error
 func (s *DomainsServiceOp) EditRecord(
 	domain string,
 	id int,
-	editRequest *DomainRecordEditRequest) (*DomainRecord, *Response, error) {
+	editRequest *DomainRecordEditRequest,
+) (*DomainRecord, *Response, error) {
+	if len(domain) < 1 {
+		return nil, nil, NewArgError("domain", "cannot be an empty string")
+	}
+
+	if id < 1 {
+		return nil, nil, NewArgError("id", "cannot be less than 1")
+	}
+
+	if editRequest == nil {
+		return nil, nil, NewArgError("editRequest", "cannot be nil")
+	}
+
 	path := fmt.Sprintf("%s/%s/records/%d", domainsBasePath, domain, id)
 
 	req, err := s.client.NewRequest("PUT", path, editRequest)
@@ -253,6 +298,14 @@ func (s *DomainsServiceOp) EditRecord(
 func (s *DomainsServiceOp) CreateRecord(
 	domain string,
 	createRequest *DomainRecordEditRequest) (*DomainRecord, *Response, error) {
+	if len(domain) < 1 {
+		return nil, nil, NewArgError("domain", "cannot be empty string")
+	}
+
+	if createRequest == nil {
+		return nil, nil, NewArgError("createRequest", "cannot be nil")
+	}
+
 	path := fmt.Sprintf("%s/%s/records", domainsBasePath, domain)
 	req, err := s.client.NewRequest("POST", path, createRequest)
 

diff --git a/droplet_actions.go b/droplet_actions.go
@@ -161,6 +161,14 @@ func (s *DropletActionsServiceOp) Upgrade(id int) (*Action, *Response, error) {
 }
 
 func (s *DropletActionsServiceOp) doAction(id int, request *ActionRequest) (*Action, *Response, error) {
+	if id < 1 {
+		return nil, nil, NewArgError("id", "cannot be less than 1")
+	}
+
+	if request == nil {
+		return nil, nil, NewArgError("request", "request can't be nil")
+	}
+
 	path := dropletActionPath(id)
 
 	req, err := s.client.NewRequest("POST", path, request)
@@ -179,6 +187,14 @@ func (s *DropletActionsServiceOp) doAction(id int, request *ActionRequest) (*Act
 
 // Get an action for a particular droplet by id.
 func (s *DropletActionsServiceOp) Get(dropletID, actionID int) (*Action, *Response, error) {
+	if dropletID < 1 {
+		return nil, nil, NewArgError("dropletID", "cannot be less than 1")
+	}
+
+	if actionID < 1 {
+		return nil, nil, NewArgError("actionID", "cannot be less than 1")
+	}
+
 	path := fmt.Sprintf("%s/%d", dropletActionPath(dropletID), actionID)
 	return s.get(path)
 }

diff --git a/droplets.go b/droplets.go
@@ -194,6 +194,10 @@ func (s *DropletsServiceOp) List(opt *ListOptions) ([]Droplet, *Response, error)
 
 // Get individual droplet
 func (s *DropletsServiceOp) Get(dropletID int) (*Droplet, *Response, error) {
+	if dropletID < 1 {
+		return nil, nil, NewArgError("dropletID", "cannot be less than 1")
+	}
+
 	path := fmt.Sprintf("%s/%d", dropletBasePath, dropletID)
 
 	req, err := s.client.NewRequest("GET", path, nil)
@@ -212,6 +216,10 @@ func (s *DropletsServiceOp) Get(dropletID int) (*Droplet, *Response, error) {
 
 // Create droplet
 func (s *DropletsServiceOp) Create(createRequest *DropletCreateRequest) (*Droplet, *Response, error) {
+	if createRequest == nil {
+		return nil, nil, NewArgError("createRequest", "cannot be nil")
+	}
+
 	path := dropletBasePath
 
 	req, err := s.client.NewRequest("POST", path, createRequest)
@@ -233,6 +241,10 @@ func (s *DropletsServiceOp) Create(createRequest *DropletCreateRequest) (*Drople
 
 // Delete droplet
 func (s *DropletsServiceOp) Delete(dropletID int) (*Response, error) {
+	if dropletID < 1 {
+		return nil, NewArgError("dropletID", "cannot be less than 1")
+	}
+
 	path := fmt.Sprintf("%s/%d", dropletBasePath, dropletID)
 
 	req, err := s.client.NewRequest("DELETE", path, nil)
@@ -247,6 +259,10 @@ func (s *DropletsServiceOp) Delete(dropletID int) (*Response, error) {
 
 // Kernels lists kernels available for a droplet.
 func (s *DropletsServiceOp) Kernels(dropletID int, opt *ListOptions) ([]Kernel, *Response, error) {
+	if dropletID < 1 {
+		return nil, nil, NewArgError("dropletID", "cannot be less than 1")
+	}
+
 	path := fmt.Sprintf("%s/%d/kernels", dropletBasePath, dropletID)
 	path, err := addOptions(path, opt)
 	if err != nil {
@@ -269,6 +285,10 @@ func (s *DropletsServiceOp) Kernels(dropletID int, opt *ListOptions) ([]Kernel,
 
 // Actions lists the actions for a droplet.
 func (s *DropletsServiceOp) Actions(dropletID int, opt *ListOptions) ([]Action, *Response, error) {
+	if dropletID < 1 {
+		return nil, nil, NewArgError("dropletID", "cannot be less than 1")
+	}
+
 	path := fmt.Sprintf("%s/%d/actions", dropletBasePath, dropletID)
 	path, err := addOptions(path, opt)
 	if err != nil {
@@ -294,6 +314,10 @@ func (s *DropletsServiceOp) Actions(dropletID int, opt *ListOptions) ([]Action,
 
 // Backups lists the backups for a droplet.
 func (s *DropletsServiceOp) Backups(dropletID int, opt *ListOptions) ([]Image, *Response, error) {
+	if dropletID < 1 {
+		return nil, nil, NewArgError("dropletID", "cannot be less than 1")
+	}
+
 	path := fmt.Sprintf("%s/%d/backups", dropletBasePath, dropletID)
 	path, err := addOptions(path, opt)
 	if err != nil {
@@ -319,6 +343,10 @@ func (s *DropletsServiceOp) Backups(dropletID int, opt *ListOptions) ([]Image, *
 
 // Snapshots lists the snapshots available for a droplet.
 func (s *DropletsServiceOp) Snapshots(dropletID int, opt *ListOptions) ([]Image, *Response, error) {
+	if dropletID < 1 {
+		return nil, nil, NewArgError("dropletID", "cannot be less than 1")
+	}
+
 	path := fmt.Sprintf("%s/%d/snapshots", dropletBasePath, dropletID)
 	path, err := addOptions(path, opt)
 	if err != nil {
@@ -344,6 +372,10 @@ func (s *DropletsServiceOp) Snapshots(dropletID int, opt *ListOptions) ([]Image,
 
 // Neighbors lists the neighbors for a droplet.
 func (s *DropletsServiceOp) Neighbors(dropletID int) ([]Droplet, *Response, error) {
+	if dropletID < 1 {
+		return nil, nil, NewArgError("dropletID", "cannot be less than 1")
+	}
+
 	path := fmt.Sprintf("%s/%d/neighbors", dropletBasePath, dropletID)
 
 	req, err := s.client.NewRequest("GET", path, nil)

diff --git a/errors.go b/errors.go
@@ -0,0 +1,24 @@
+package godo
+
+import "fmt"
+
+// ArgError is an error that represents an error with an input to godo. It
+// identifies the argument and the cause (if possible).
+type ArgError struct {
+	arg    string
+	reason string
+}
+
+var _ error = &ArgError{}
+
+// NewArgError creates an InputError.
+func NewArgError(arg, reason string) *ArgError {
+	return &ArgError{
+		arg:    arg,
+		reason: reason,
+	}
+}
+
+func (e *ArgError) Error() string {
+	return fmt.Sprintf("%s is invalid because %s", e.arg, e.reason)
+}
diff --git a/errors_test.go b/errors_test.go
@@ -0,0 +1,11 @@
+package godo
+
+import "testing"
+
+func TestArgError(t *testing.T) {
+	expected := "foo is invalid because bar"
+	err := NewArgError("foo", "bar")
+	if got := err.Error(); got != expected {
+		t.Errorf("ArgError().Error() = %q; expected %q", got, expected)
+	}
+}
diff --git a/image_actions.go b/image_actions.go
@@ -20,6 +20,14 @@ var _ ImageActionsService = &ImageActionsServiceOp{}
 
 // Transfer an image
 func (i *ImageActionsServiceOp) Transfer(imageID int, transferRequest *ActionRequest) (*Action, *Response, error) {
+	if imageID < 1 {
+		return nil, nil, NewArgError("imageID", "cannot be less than 1")
+	}
+
+	if transferRequest == nil {
+		return nil, nil, NewArgError("transferRequest", "cannot be nil")
+	}
+
 	path := fmt.Sprintf("v2/images/%d/actions", imageID)
 
 	req, err := i.client.NewRequest("POST", path, transferRequest)
@@ -38,6 +46,14 @@ func (i *ImageActionsServiceOp) Transfer(imageID int, transferRequest *ActionReq
 
 // Get an action for a particular image by id.
 func (i *ImageActionsServiceOp) Get(imageID, actionID int) (*Action, *Response, error) {
+	if imageID < 1 {
+		return nil, nil, NewArgError("imageID", "cannot be less than 1")
+	}
+
+	if actionID < 1 {
+		return nil, nil, NewArgError("actionID", "cannot be less than 1")
+	}
+
 	path := fmt.Sprintf("v2/images/%d/actions/%d", imageID, actionID)
 
 	req, err := i.client.NewRequest("GET", path, nil)

diff --git a/images.go b/images.go
@@ -87,16 +87,32 @@ func (s *ImagesServiceOp) ListUser(opt *ListOptions) ([]Image, *Response, error)
 
 // GetByID retrieves an image by id.
 func (s *ImagesServiceOp) GetByID(imageID int) (*Image, *Response, error) {
+	if imageID < 1 {
+		return nil, nil, NewArgError("imageID", "cannot be less than 1")
+	}
+
 	return s.get(interface{}(imageID))
 }
 
 // GetBySlug retrieves an image by slug.
 func (s *ImagesServiceOp) GetBySlug(slug string) (*Image, *Response, error) {
+	if len(slug) < 1 {
+		return nil, nil, NewArgError("slug", "cannot be blank")
+	}
+
 	return s.get(interface{}(slug))
 }
 
 // Update an image name.
 func (s *ImagesServiceOp) Update(imageID int, updateRequest *ImageUpdateRequest) (*Image, *Response, error) {
+	if imageID < 1 {
+		return nil, nil, NewArgError("imageID", "cannot be less than 1")
+	}
+
+	if updateRequest == nil {
+		return nil, nil, NewArgError("updateRequest", "cannot be nil")
+	}
+
 	path := fmt.Sprintf("%s/%d", imageBasePath, imageID)
 	req, err := s.client.NewRequest("PUT", path, updateRequest)
 	if err != nil {
@@ -114,6 +130,10 @@ func (s *ImagesServiceOp) Update(imageID int, updateRequest *ImageUpdateRequest)
 
 // Delete an image.
 func (s *ImagesServiceOp) Delete(imageID int) (*Response, error) {
+	if imageID < 1 {
+		return nil, NewArgError("imageID", "cannot be less than 1")
+	}
+
 	path := fmt.Sprintf("%s/%d", imageBasePath, imageID)
 
 	req, err := s.client.NewRequest("DELETE", path, nil)