Create new error abstraction for field validation (#239)

* NewName should be optional

* create generic error type

* update service validation

* service version 'comment' is required

* clarify in README use of pointers for mandatory fields on list operations

* remove unnecessary TLS prefix

* when updating a dict item, the value should be required not optional

* indicate both fields are required

* capture domain update 'optional field' error in a test

* start moving errors over to new field error type

* fix honeycomb to use new error field

* fix resources to use new error field validation

* refactor method name on FieldError

* tweak docstring for FieldError#Error

* revert a possibly unnecessary change

* update README (v2 and v3 links)

* update go.mod to v3

* revert to sentinel errors

* remove note

* revert errors.Is

* fix managed logging code as rebase cleared a new error sentinel

* update README to reflect merged breaking changes

* fix bad rebase

* revert change to DitionaryUpdate ItemValue (we'll move to separate PR)

* revert change to NewName being optional (we'll move to separate PR)

* Avoid hardcoded strings for validating errors

* revert change to TLS Configuration and Domain (we'll move to separate PR)

* revert change from Service to ServiceID (we'll move to separate PR)

* revert change from ID to ServiceID (we'll move to separate PR)

README.md

@@ -1,9 +1,10 @@
 # Go Fastly
 
-[![Go Documentation](http://img.shields.io/badge/go-documentation-blue.svg?style=flat-square)][godocs]
+[![Go Documentation](http://img.shields.io/badge/go-documentation-blue.svg?style=flat-square)][latest]
 
-[godocs]: https://pkg.go.dev/github.com/fastly/go-fastly/fastly?tab=doc
-[v2]: https://pkg.go.dev/github.com/fastly/go-fastly/v2
+[latest]: https://pkg.go.dev/github.com/fastly/go-fastly/v3/fastly
+[v3]: https://pkg.go.dev/github.com/fastly/go-fastly/v3/fastly
+[v2]: https://pkg.go.dev/github.com/fastly/go-fastly/v2/fastly
 [v1]: https://pkg.go.dev/github.com/fastly/go-fastly
 
 Go Fastly is a Golang API client for interacting with most facets of the
@@ -17,7 +18,7 @@ so you must be running Go 1.11 or higher.
 ## Usage
 
 ```go
-import "github.com/fastly/go-fastly/v2/fastly"
+import "github.com/fastly/go-fastly/v3/fastly"
 ```
 
 ## Migrating from v1 to v2
@@ -31,6 +32,16 @@ The move to more consistent field names in some cases will have resulted in the
 
 The change in type for [basic types](https://tour.golang.org/basics/11) that are optional on input structs related to write/update operations is designed to avoid unexpected behaviours when dealing with their zero value (see [this reference](https://willnorris.com/2014/05/go-rest-apis-and-pointers/) for more details). As part of this change we now provide [helper functions](./fastly/basictypes_helper.go) to assist with generating the new pointer types required.
 
+> Note: some read/list operations require fields to be provided but if omitted a zero value will be used when marshaling the data structure into JSON. This too can cause confusion, which is why some input structs define their mandatory fields as pointers (to ensure that the backend can distinguish between a zero value and an omitted field).
+
+## Migrating from v2 to v3
+
+There were a few breaking changes introduced in [`v3.0.0`][v3]:
+
+1. A new `FieldError` abstraction for validating API struct fields.
+2. Changing some mandatory fields to Optional (and vice-versa) to better support more _practical_ API usage.
+3. Avoid generic ID field when more explicit naming would be clearer.
+
 ## Examples
 
 Fastly's API is designed to work in the following manner:
@@ -128,7 +139,7 @@ fmt.Printf("%t\n", activeVersion.Locked)
 ```
 
 More information can be found in the
-[Fastly Godoc][godocs].
+[Fastly Godoc][latest].
 
 ## Developing
 

fastly/acl_entry.go

@@ -242,7 +242,6 @@ type BatchACLEntry struct {
 }
 
 func (c *Client) BatchModifyACLEntries(i *BatchModifyACLEntriesInput) error {
-
 	if i.ServiceID == "" {
 		return ErrMissingServiceID
 	}
@@ -252,7 +251,7 @@ func (c *Client) BatchModifyACLEntries(i *BatchModifyACLEntriesInput) error {
 	}
 
 	if len(i.Entries) > BatchModifyMaximumOperations {
-		return ErrBatchUpdateMaximumOperationsExceeded
+		return ErrMaxExceededEntries
 	}
 
 	path := fmt.Sprintf("/service/%s/acl/%s/entries", i.ServiceID, i.ACLID)

fastly/acl_entry_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_ACLEntries(t *testing.T) {
 
@@ -273,7 +275,7 @@ func TestClient_BatchModifyACLEntries_validation(t *testing.T) {
 		ACLID:     "bar",
 		Entries:   oversizedACLEntries,
 	})
-	if err != ErrBatchUpdateMaximumOperationsExceeded {
+	if err != ErrMaxExceededEntries {
 		t.Errorf("bad error: %s", err)
 	}
 

fastly/acl_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_ACLs(t *testing.T) {
 	t.Parallel()

fastly/backend_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_Backends(t *testing.T) {
 	t.Parallel()

fastly/blobstorage_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_BlobStorages(t *testing.T) {
 	t.Parallel()

fastly/cache_setting_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_CacheSettings(t *testing.T) {
 	t.Parallel()

fastly/cloudfiles_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_Cloudfiles(t *testing.T) {
 	t.Parallel()

fastly/condition_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_Conditions(t *testing.T) {
 	t.Parallel()

fastly/custom_tls_certificate_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_CustomTLSCertificate(t *testing.T) {
 	t.Parallel()

fastly/custom_tls_configuration_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_CustomTLSConfiguration(t *testing.T) {
 	t.Parallel()

fastly/datadog_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_Datadog(t *testing.T) {
 	t.Parallel()

fastly/dictionary_item.go

@@ -46,7 +46,7 @@ func (c *Client) ListDictionaryItems(i *ListDictionaryItemsInput) ([]*Dictionary
 	}
 
 	if i.DictionaryID == "" {
-		return nil, ErrMissingDictionary
+		return nil, ErrMissingDictionaryID
 	}
 
 	path := fmt.Sprintf("/service/%s/dictionary/%s/items", i.ServiceID, i.DictionaryID)
@@ -82,7 +82,7 @@ func (c *Client) CreateDictionaryItem(i *CreateDictionaryItemInput) (*Dictionary
 	}
 
 	if i.DictionaryID == "" {
-		return nil, ErrMissingDictionary
+		return nil, ErrMissingDictionaryID
 	}
 
 	path := fmt.Sprintf("/service/%s/dictionary/%s/item", i.ServiceID, i.DictionaryID)
@@ -131,7 +131,7 @@ func (c *Client) GetDictionaryItem(i *GetDictionaryItemInput) (*DictionaryItem,
 	}
 
 	if i.DictionaryID == "" {
-		return nil, ErrMissingDictionary
+		return nil, ErrMissingDictionaryID
 	}
 
 	if i.ItemKey == "" {
@@ -172,7 +172,7 @@ func (c *Client) UpdateDictionaryItem(i *UpdateDictionaryItemInput) (*Dictionary
 	}
 
 	if i.DictionaryID == "" {
-		return nil, ErrMissingDictionary
+		return nil, ErrMissingDictionaryID
 	}
 
 	if i.ItemKey == "" {
@@ -215,11 +215,11 @@ func (c *Client) BatchModifyDictionaryItems(i *BatchModifyDictionaryItemsInput)
 	}
 
 	if i.DictionaryID == "" {
-		return ErrMissingDictionary
+		return ErrMissingDictionaryID
 	}
 
 	if len(i.Items) > BatchModifyMaximumOperations {
-		return ErrBatchUpdateMaximumOperationsExceeded
+		return ErrMaxExceededItems
 	}
 
 	path := fmt.Sprintf("/service/%s/dictionary/%s/items", i.ServiceID, i.DictionaryID)
@@ -255,7 +255,7 @@ func (c *Client) DeleteDictionaryItem(i *DeleteDictionaryItemInput) error {
 	}
 
 	if i.DictionaryID == "" {
-		return ErrMissingDictionary
+		return ErrMissingDictionaryID
 	}
 
 	if i.ItemKey == "" {

fastly/dictionary_item_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_DictionaryItems(t *testing.T) {
 
@@ -125,7 +127,7 @@ func TestClient_ListDictionaryItems_validation(t *testing.T) {
 		ServiceID:    "foo",
 		DictionaryID: "",
 	})
-	if err != ErrMissingDictionary {
+	if err != ErrMissingDictionaryID {
 		t.Errorf("bad error: %s", err)
 	}
 }
@@ -143,7 +145,7 @@ func TestClient_CreateDictionaryItem_validation(t *testing.T) {
 		ServiceID:    "foo",
 		DictionaryID: "",
 	})
-	if err != ErrMissingDictionary {
+	if err != ErrMissingDictionaryID {
 		t.Errorf("bad error: %s", err)
 	}
 }
@@ -161,7 +163,7 @@ func TestClient_GetDictionaryItem_validation(t *testing.T) {
 		ServiceID:    "foo",
 		DictionaryID: "",
 	})
-	if err != ErrMissingDictionary {
+	if err != ErrMissingDictionaryID {
 		t.Errorf("bad error: %s", err)
 	}
 
@@ -188,7 +190,7 @@ func TestClient_UpdateDictionaryItem_validation(t *testing.T) {
 		ServiceID:    "foo",
 		DictionaryID: "",
 	})
-	if err != ErrMissingDictionary {
+	if err != ErrMissingDictionaryID {
 		t.Errorf("bad error: %s", err)
 	}
 
@@ -215,7 +217,7 @@ func TestClient_DeleteDictionaryItem_validation(t *testing.T) {
 		ServiceID:    "foo",
 		DictionaryID: "",
 	})
-	if err != ErrMissingDictionary {
+	if err != ErrMissingDictionaryID {
 		t.Errorf("bad error: %s", err)
 	}
 
@@ -241,7 +243,7 @@ func TestClient_BatchModifyDictionaryItem_validation(t *testing.T) {
 		ServiceID:    "foo",
 		DictionaryID: "",
 	})
-	if err != ErrMissingDictionary {
+	if err != ErrMissingDictionaryID {
 		t.Errorf("bad error: %s", err)
 	}
 
@@ -251,8 +253,7 @@ func TestClient_BatchModifyDictionaryItem_validation(t *testing.T) {
 		DictionaryID: "bar",
 		Items:        oversizedDictionaryItems,
 	})
-	if err != ErrBatchUpdateMaximumOperationsExceeded {
+	if err != ErrMaxExceededItems {
 		t.Errorf("bad error: %s", err)
 	}
-
 }

fastly/dictionary_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_Dictionaries(t *testing.T) {
 	t.Parallel()

fastly/diff_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_Diff(t *testing.T) {
 	t.Parallel()

fastly/digitalocean_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_DigitalOceans(t *testing.T) {
 	t.Parallel()

fastly/director_backend_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_DirectorBackends(t *testing.T) {
 	t.Parallel()

fastly/director_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_Directors(t *testing.T) {
 	t.Parallel()

fastly/domain_test.go

@@ -1,6 +1,8 @@
 package fastly
 
-import "testing"
+import (
+	"testing"
+)
 
 func TestClient_Domains(t *testing.T) {
 	t.Parallel()