Overhauling error handling of the different APIs as part of #24

- Overhauled the breaches API
- Added more tests

breach.go

@@ -96,81 +96,81 @@ type APIDate time.Time
 
 // Breaches returns a list of all breaches in the HIBP system
 func (b *BreachAPI) Breaches(options ...BreachOption) ([]*Breach, *http.Response, error) {
-	queryParams := b.setBreachOpts(options...)
+	qp := b.setBreachOpts(options...)
-	apiURL := fmt.Sprintf("%s/breaches", BaseURL)
+	au := fmt.Sprintf("%s/breaches", BaseURL)
 
-	hb, hr, err := b.hibp.HTTPResBody(http.MethodGet, apiURL, queryParams)
+	hb, hr, err := b.hibp.HTTPResBody(http.MethodGet, au, qp)
 	if err != nil {
-		return nil, nil, err
-	}
-
-	var breachList []*Breach
-	if err := json.Unmarshal(hb, &breachList); err != nil {
 		return nil, hr, err
 	}
 
-	return breachList, hr, nil
+	var bl []*Breach
+	if err := json.Unmarshal(hb, &bl); err != nil {
+		return nil, hr, err
+	}
+
+	return bl, hr, nil
 }
 
 // BreachByName returns a single breached site based on its name
 func (b *BreachAPI) BreachByName(n string, options ...BreachOption) (*Breach, *http.Response, error) {
-	queryParams := b.setBreachOpts(options...)
+	qp := b.setBreachOpts(options...)
 
 	if n == "" {
-		return nil, nil, fmt.Errorf("no breach name given")
+		return nil, nil, ErrNoName
 	}
 
-	apiURL := fmt.Sprintf("%s/breach/%s", BaseURL, n)
+	au := fmt.Sprintf("%s/breach/%s", BaseURL, n)
-	hb, hr, err := b.hibp.HTTPResBody(http.MethodGet, apiURL, queryParams)
+	hb, hr, err := b.hibp.HTTPResBody(http.MethodGet, au, qp)
 	if err != nil {
-		return nil, nil, err
-	}
-
-	var breachDetails *Breach
-	if err := json.Unmarshal(hb, &breachDetails); err != nil {
 		return nil, hr, err
 	}
 
-	return breachDetails, hr, nil
+	var bd *Breach
+	if err := json.Unmarshal(hb, &bd); err != nil {
+		return nil, hr, err
+	}
+
+	return bd, hr, nil
 }
 
 // DataClasses are attribute of a record compromised in a breach. This method returns a list of strings
 // with all registered data classes known to HIBP
 func (b *BreachAPI) DataClasses() ([]string, *http.Response, error) {
-	apiURL := fmt.Sprintf("%s/dataclasses", BaseURL)
+	au := fmt.Sprintf("%s/dataclasses", BaseURL)
-	hb, hr, err := b.hibp.HTTPResBody(http.MethodGet, apiURL, nil)
+	hb, hr, err := b.hibp.HTTPResBody(http.MethodGet, au, nil)
 	if err != nil {
-		return nil, nil, err
-	}
-
-	var dataClasses []string
-	if err := json.Unmarshal(hb, &dataClasses); err != nil {
 		return nil, hr, err
 	}
 
-	return dataClasses, hr, nil
+	var dc []string
+	if err := json.Unmarshal(hb, &dc); err != nil {
+		return nil, hr, err
+	}
+
+	return dc, hr, nil
 }
 
 // BreachedAccount returns a single breached site based on its name
 func (b *BreachAPI) BreachedAccount(a string, options ...BreachOption) ([]*Breach, *http.Response, error) {
-	queryParams := b.setBreachOpts(options...)
+	qp := b.setBreachOpts(options...)
 
 	if a == "" {
-		return nil, nil, fmt.Errorf("no account id given")
+		return nil, nil, ErrNoAccountID
 	}
 
-	apiURL := fmt.Sprintf("%s/breachedaccount/%s", BaseURL, a)
+	au := fmt.Sprintf("%s/breachedaccount/%s", BaseURL, a)
-	hb, hr, err := b.hibp.HTTPResBody(http.MethodGet, apiURL, queryParams)
+	hb, hr, err := b.hibp.HTTPResBody(http.MethodGet, au, qp)
 	if err != nil {
-		return nil, nil, err
-	}
-
-	var breachDetails []*Breach
-	if err := json.Unmarshal(hb, &breachDetails); err != nil {
 		return nil, hr, err
 	}
 
-	return breachDetails, hr, nil
+	var bd []*Breach
+	if err := json.Unmarshal(hb, &bd); err != nil {
+		return nil, hr, err
+	}
+
+	return bd, hr, nil
 }
 
 // WithDomain sets the domain filter for the breaches API
@@ -205,7 +205,7 @@ func (d *APIDate) UnmarshalJSON(s []byte) error {
 
 	pd, err := time.Parse("2006-01-02", ds)
 	if err != nil {
-		return fmt.Errorf("failed to convert API date string to time.Time type: %w", err)
+		return fmt.Errorf("convert API date string to time.Time type: %w", err)
 	}
 
 	*(*time.Time)(d) = pd
@@ -220,7 +220,7 @@ func (d *APIDate) Time() time.Time {
 
 // setBreachOpts returns a map of default settings and overridden values from different BreachOption
 func (b *BreachAPI) setBreachOpts(options ...BreachOption) map[string]string {
-	queryParams := map[string]string{
+	qp := map[string]string{
 		"truncateResponse":  "true",
 		"includeUnverified": "true",
 	}
@@ -233,16 +233,16 @@ func (b *BreachAPI) setBreachOpts(options ...BreachOption) map[string]string {
 	}
 
 	if b.domain != "" {
-		queryParams["domain"] = b.domain
+		qp["domain"] = b.domain
 	}
 
 	if b.disableTrunc {
-		queryParams["truncateResponse"] = "false"
+		qp["truncateResponse"] = "false"
 	}
 
 	if b.noUnverified {
-		queryParams["includeUnverified"] = "false"
+		qp["includeUnverified"] = "false"
 	}
 
-	return queryParams
+	return qp
 }

breach_test.go

@@ -2,6 +2,7 @@ package hibp
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"os"
 	"testing"
@@ -14,8 +15,8 @@ const (
 	invalidDateJSON   = `{"date": "202299-10-01"}`
 )
 
-// TestBreaches tests the Breaches() method of the breaches API
+// TestBreachAPI_Breaches tests the Breaches() method of the breaches API
-func TestBreaches(t *testing.T) {
+func TestBreachAPI_Breaches(t *testing.T) {
 	hc := New()
 	breachList, _, err := hc.BreachAPI.Breaches()
 	if err != nil {
@@ -26,20 +27,21 @@ func TestBreaches(t *testing.T) {
 	}
 }
 
-// TestBreachesWithNil tests the Breaches() method of the breaches API with a nil option
+// TestBreachAPI_Breaches_WithNIL tests the Breaches() method of the breaches API with a nil option
-func TestBreachesWithNil(t *testing.T) {
+func TestBreachAPI_Breaches_WithNIL(t *testing.T) {
 	hc := New()
 	breachList, _, err := hc.BreachAPI.Breaches(nil)
 	if err != nil {
 		t.Error(err)
+		return
 	}
 	if breachList != nil && len(breachList) <= 0 {
 		t.Error("breaches list returned 0 results")
 	}
 }
 
-// TestBreachesWithDomain tests the Breaches() method of the breaches API for a specific domain
+// TestBreachAPI_Breaches_WithDomain tests the Breaches() method of the breaches API for a specific domain
-func TestBreachesWithDomain(t *testing.T) {
+func TestBreachAPI_Breaches_WithDomain(t *testing.T) {
 	testTable := []struct {
 		testName   string
 		domain     string
@@ -55,6 +57,7 @@ func TestBreachesWithDomain(t *testing.T) {
 			breachList, _, err := hc.BreachAPI.Breaches(WithDomain(tc.domain))
 			if err != nil {
 				t.Error(err)
+				return
 			}
 
 			if breachList == nil && tc.isBreached {
@@ -75,8 +78,8 @@ func TestBreachesWithDomain(t *testing.T) {
 	}
 }
 
-// TestBreachesWithoutUnverified tests the Breaches() method of the breaches API with the unverified parameter
+// TestBreachAPI_Breaches_WithoutUnverified tests the Breaches() method of the breaches API with the unverified parameter
-func TestBreachesWithoutUnverified(t *testing.T) {
+func TestBreachAPI_Breaches_WithoutUnverified(t *testing.T) {
 	testTable := []struct {
 		testName   string
 		domain     string
@@ -94,6 +97,7 @@ func TestBreachesWithoutUnverified(t *testing.T) {
 			breachList, _, err := hc.BreachAPI.Breaches(WithDomain(tc.domain), WithoutUnverified())
 			if err != nil {
 				t.Error(err)
+				return
 			}
 
 			if breachList == nil && tc.isVerified && tc.isBreached {
@@ -104,8 +108,8 @@ func TestBreachesWithoutUnverified(t *testing.T) {
 	}
 }
 
-// TestBreachByName tests the BreachByName() method of the breaches API for a specific domain
+// TestBreachAPI_BreachByName tests the BreachByName() method of the breaches API for a specific domain
-func TestBreachByName(t *testing.T) {
+func TestBreachAPI_BreachByName(t *testing.T) {
 	testTable := []struct {
 		testName   string
 		breachName string
@@ -122,6 +126,7 @@ func TestBreachByName(t *testing.T) {
 			breachDetails, _, err := hc.BreachAPI.BreachByName(tc.breachName)
 			if err != nil && !tc.shouldFail {
 				t.Error(err)
+				return
 			}
 
 			if breachDetails == nil && tc.isBreached {
@@ -136,20 +141,42 @@ func TestBreachByName(t *testing.T) {
 	}
 }
 
-// TestDataClasses tests the DataClasses() method of the breaches API
+// TestBreachAPI_BreachByName_FailedHTTP tests the BreachByName() method with a failing HTTP request
-func TestDataClasses(t *testing.T) {
+func TestBreachAPI_BreachByName_FailedHTTP(t *testing.T) {
+	hc := New(WithRateLimitSleep())
+	_, res, err := hc.BreachAPI.BreachByName("fäiled_invalid")
+	if err == nil {
+		t.Errorf("HTTP request was supposed to fail but didn't")
+	}
+	if res == nil {
+		t.Errorf("expected HTTP response but got nil")
+	}
+}
+
+// TestBreachAPI_BreachByName_Errors tests the errors for the BreachByName() method
+func TestBreachAPI_BreachByName_Errors(t *testing.T) {
+	hc := New(WithRateLimitSleep())
+	_, _, err := hc.BreachAPI.BreachByName("")
+	if !errors.Is(err, ErrNoName) {
+		t.Errorf("expected to receive ErrNoName error but didn't")
+	}
+}
+
+// TestBreachAPI_DataClasses tests the DataClasses() method of the breaches API
+func TestBreachAPI_DataClasses(t *testing.T) {
 	hc := New()
 	classList, _, err := hc.BreachAPI.DataClasses()
 	if err != nil {
 		t.Error(err)
+		return
 	}
 	if classList != nil && len(classList) <= 0 {
 		t.Error("breaches list returned 0 results")
 	}
 }
 
-// TestBreachedAccount tests the BreachedAccount() method of the breaches API
+// TestBreachAPI_BreachedAccount tests the BreachedAccount() method of the breaches API
-func TestBreachedAccount(t *testing.T) {
+func TestBreachAPI_BreachedAccount(t *testing.T) {
 	testTable := []struct {
 		testName          string
 		accountName       string
@@ -200,9 +227,35 @@ func TestBreachedAccount(t *testing.T) {
 	}
 }
 
-// TestBreachedAccountWithoutTruncate tests the BreachedAccount() method of the breaches API with the
+// TestBreachAPI_BreachedAccount_FailedHTTP tests the BreachedAccount() method of the breaches API with a failing
+// HTTP request
+func TestBreachAPI_BreachedAccount_FailedHTTP(t *testing.T) {
+	apiKey := os.Getenv("HIBP_API_KEY")
+	if apiKey == "" {
+		t.SkipNow()
+	}
+	hc := New(WithAPIKey(apiKey), WithRateLimitSleep())
+	_, res, err := hc.BreachAPI.BreachedAccount("bröken@invalid_domain.tld")
+	if err == nil {
+		t.Error("HTTP request was supposed to fail, but didn't")
+	}
+	if res == nil {
+		t.Errorf("expected HTTP response but got nil")
+	}
+}
+
+// TestBreachAPI_BreachedAccount_Errors tests the errors for the BreachedAccount() method
+func TestBreachAPI_BreachedAccount_Errors(t *testing.T) {
+	hc := New(WithRateLimitSleep())
+	_, _, err := hc.BreachAPI.BreachedAccount("")
+	if !errors.Is(err, ErrNoAccountID) {
+		t.Errorf("expected to receive ErrNoAccountID error but didn't")
+	}
+}
+
+// TestBreachAPI_BreachedAccount_WithoutTruncate tests the BreachedAccount() method of the breaches API with the
 // truncateResponse option set to false
-func TestBreachedAccountWithoutTruncate(t *testing.T) {
+func TestBreachAPI_BreachedAccount_WithoutTruncate(t *testing.T) {
 	testTable := []struct {
 		testName     string
 		accountName  string
@@ -211,14 +264,17 @@ func TestBreachedAccountWithoutTruncate(t *testing.T) {
 		shouldFail   bool
 	}{
 		{
-			"account-exists is breached once", "account-exists@hibp-integration-tests.com", "Adobe",
+			"account-exists is breached once", "account-exists@hibp-integration-tests.com",
-			"adobe.com", false,
+			"Adobe", "adobe.com", false,
 		},
 		{
-			"multiple-breaches is breached multiple times", "multiple-breaches@hibp-integration-tests.com", "Adobe",
+			"multiple-breaches is breached multiple times", "multiple-breaches@hibp-integration-tests.com",
-			"adobe.com", false,
+			"Adobe", "adobe.com", false,
+		},
+		{
+			"opt-out is not breached", "opt-out@hibp-integration-tests.com", "",
+			"", true,
 		},
-		{"opt-out is not breached", "opt-out@hibp-integration-tests.com", "", "", true},
 		{"empty string should fail", "", "", "", true},
 	}