Merge pull request #349 from jabielecki/with_header

add WithHeader and convert NewClient to use options

clientopt.go

@@ -0,0 +1,55 @@
+package packngo
+
+import (
+	"net/http"
+	"net/url"
+)
+
+// ClientOpt is an option usable as an argument to NewClient constructor.
+type ClientOpt func(*Client) error
+
+// WithAuth configures Client with a specific consumerToken and apiKey for subsequent HTTP requests.
+func WithAuth(consumerToken string, apiKey string) ClientOpt {
+	return func(c *Client) error {
+		c.ConsumerToken = consumerToken
+		c.APIKey = apiKey
+		c.apiKeySet = true
+
+		return nil
+	}
+}
+
+// WithHTTPClient configures Client to use a specific httpClient for subsequent HTTP requests.
+func WithHTTPClient(httpClient *http.Client) ClientOpt {
+	return func(c *Client) error {
+		c.client = httpClient
+
+		return nil
+	}
+}
+
+// WithBaseURL configures Client to use a nonstandard API URL, e.g. for mocking the remote API.
+func WithBaseURL(apiBaseURL string) ClientOpt {
+	return func(c *Client) error {
+		u, err := url.Parse(apiBaseURL)
+		if err != nil {
+			return err
+		}
+
+		c.BaseURL = u
+
+		return nil
+	}
+}
+
+// WithHeader configures Client to use the given HTTP header set.
+// The headers X-Auth-Token, X-Consumer-Token, User-Agent will be ignored even if provided in the set.
+func WithHeader(header http.Header) ClientOpt {
+	return func(c *Client) error {
+		for k, v := range header {
+			c.header[k] = v
+		}
+
+		return nil
+	}
+}

mocks_test.go

@@ -2,11 +2,9 @@ package packngo
 
 import (
 	"bytes"
-	"encoding/json"
 	"fmt"
 	"io/ioutil"
 	"net/http"
-	"net/url"
 )
 
 // MockClient makes it simpler to test the Client
@@ -34,73 +32,6 @@ func (mc *MockClient) DoRequest(method, path string, body, v interface{}) (*Resp
 	return mc.fnDoRequest(method, path, body, v)
 }
 
-/* deadcode, for now
-func mockDoRequestWithHeader(doFn func(req *http.Request, v interface{}) (*Response, error), newRequestFn func(method, path string, body interface{}) (*http.Request, error)) func(string, map[string]string, string, interface{}, interface{}) (*Response, error) {
-	return func(method string, headers map[string]string, path string, body, v interface{}) (*Response, error) {
-		req, err := newRequestFn(method, path, body)
-		for k, v := range headers {
-			req.Header.Add(k, v)
-		}
-
-		if err != nil {
-			return nil, err
-		}
-		return doFn(req, v)
-	}
-}
-*/
-
-func mockNewRequest() func(string, string, interface{}) (*http.Request, error) {
-	baseURL := &url.URL{}
-	apiKey, consumerToken, userAgent := "", "", ""
-	return func(method, path string, body interface{}) (*http.Request, error) {
-		// relative path to append to the endpoint url, no leading slash please
-		if path[0] == '/' {
-			path = path[1:]
-		}
-		rel, err := url.Parse(path)
-		if err != nil {
-			return nil, err
-		}
-
-		u := baseURL.ResolveReference(rel)
-
-		// json encode the request body, if any
-		buf := new(bytes.Buffer)
-		if body != nil {
-			err := json.NewEncoder(buf).Encode(body)
-			if err != nil {
-				return nil, err
-			}
-		}
-
-		req, err := http.NewRequest(method, u.String(), buf)
-		if err != nil {
-			return nil, err
-		}
-
-		req.Close = true
-
-		req.Header.Add("X-Auth-Token", apiKey)
-		req.Header.Add("X-Consumer-Token", consumerToken)
-
-		req.Header.Add("Content-Type", mediaType)
-		req.Header.Add("Accept", mediaType)
-		req.Header.Add("User-Agent", userAgent)
-		return req, nil
-	}
-}
-
-func mockDoRequest(newRequestFn func(string, string, interface{}) (*http.Request, error), doFn func(*http.Request, interface{}) (*Response, error)) func(method, path string, body, v interface{}) (*Response, error) {
-	return func(method, path string, body, v interface{}) (*Response, error) {
-		req, err := newRequestFn(method, path, body)
-		if err != nil {
-			return nil, err
-		}
-		return doFn(req, v)
-	}
-}
-
 // DoRequestWithHeader uses the mock DoRequestWithHeader function
 func (mc *MockClient) DoRequestWithHeader(method string, headers map[string]string, path string, body, v interface{}) (*Response, error) {
 	return mc.fnDoRequestWithHeader(method, headers, path, body, v)

packngo.go

@@ -90,6 +90,8 @@ type Client struct {
 	UserAgent     string
 	ConsumerToken string
 	APIKey        string
+	apiKeySet     bool
+	header        http.Header
 
 	RateLimit Rate
 
@@ -176,12 +178,11 @@ func (c *Client) NewRequest(method, path string, body interface{}) (*http.Reques
 
 	req.Close = true
 
-	req.Header.Add("X-Auth-Token", c.APIKey)
-	req.Header.Add("X-Consumer-Token", c.ConsumerToken)
+	req.Header = c.header.Clone()
+	req.Header.Set("X-Auth-Token", c.APIKey)
+	req.Header.Set("X-Consumer-Token", c.ConsumerToken)
+	req.Header.Set("User-Agent", c.UserAgent)
 
-	req.Header.Add("Content-Type", mediaType)
-	req.Header.Add("Accept", mediaType)
-	req.Header.Add("User-Agent", c.UserAgent)
 	return req, nil
 }
 
@@ -333,16 +334,6 @@ func (c *Client) DoRequestWithHeader(method string, headers map[string]string, p
 	return c.Do(req, v)
 }
 
-// NewClient initializes and returns a Client
-func NewClient() (*Client, error) {
-	apiToken := os.Getenv(authTokenEnvVar)
-	if apiToken == "" {
-		return nil, fmt.Errorf("you must export %s", authTokenEnvVar)
-	}
-	c := NewClientWithAuth("packngo lib", apiToken, nil)
-	return c, nil
-}
-
 // NewClientWithAuth initializes and returns a Client, use this to get an API Client to operate on
 // N.B.: Equinix Metal's API certificate requires Go 1.5+ to successfully parse. If you are using
 // an older version of Go, pass in a custom http.Client with a custom TLS configuration
@@ -359,12 +350,37 @@ func NewClientWithBaseURL(consumerToken string, apiKey string, httpClient *http.
 		httpClient = http.DefaultClient
 	}
 
-	u, err := url.Parse(apiBaseURL)
+	return NewClient(WithAuth(consumerToken, apiKey), WithHTTPClient(httpClient), WithBaseURL(apiBaseURL))
+}
+
+// NewClient initializes and returns a Client. The opts are functions such as WithAuth,
+// WithHTTPClient, etc.
+//
+// An example:
+//
+//	c, err := NewClient()
+//
+// An alternative example, which avoids reading PACKET_AUTH_TOKEN environment variable:
+//
+//	c, err := NewClient(WithAuth("packngo lib", packetAuthToken))
+func NewClient(opts ...ClientOpt) (*Client, error) {
+	// set defaults, then let caller override them
+	c := &Client{
+		client:        http.DefaultClient,
+		UserAgent:     UserAgent,
+		ConsumerToken: "packngo lib",
+		header:        http.Header{},
+	}
+
+	c.header.Set("Content-Type", mediaType)
+	c.header.Set("Accept", mediaType)
+
+	var err error
+	c.BaseURL, err = url.Parse(baseURL)
 	if err != nil {
 		return nil, err
 	}
 
-	c := &Client{client: httpClient, BaseURL: u, UserAgent: UserAgent, ConsumerToken: consumerToken, APIKey: apiKey}
 	c.APIKeys = &APIKeyServiceOp{client: c}
 	c.BGPConfig = &BGPConfigServiceOp{client: c}
 	c.BGPSessions = &BGPSessionServiceOp{client: c}
@@ -402,6 +418,23 @@ func NewClientWithBaseURL(consumerToken string, apiKey string, httpClient *http.
 	c.VLANAssignments = &VLANAssignmentServiceOp{client: c}
 	c.debug = os.Getenv(debugEnvVar) != ""
 
+	for _, fn := range opts {
+		err := fn(c)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	if !c.apiKeySet {
+		c.APIKey = os.Getenv(authTokenEnvVar)
+
+		if c.APIKey == "" {
+			return nil, fmt.Errorf("you must export %s", authTokenEnvVar)
+		}
+
+		c.apiKeySet = true
+	}
+
 	return c, nil
 }
 

vlan_assignments_test.go

@@ -3,7 +3,6 @@ package packngo
 import (
 	"encoding/json"
 	"fmt"
-	"net/http"
 	"path"
 	"strconv"
 	"strings"
@@ -52,30 +51,26 @@ func TestVLANAssignmentServiceOp_Get(t *testing.T) {
 		{
 			name: "Simple",
 			fields: fields{
-				client: (func() *MockClient {
+				client: &MockClient{
+					fnDoRequest: func(method, pathURL string, body, v interface{}) (*Response, error) {
+						raw := mockAssignedPortBody(testAssignmentId, testPortId, testVnId)
 
-					raw := mockAssignedPortBody(
-						testAssignmentId, testPortId, testVnId)
-					mockNR := mockNewRequest()
-					mockDo := func(req *http.Request, obj interface{}) (*Response, error) {
 						// baseURL is not needed here
 						expectedPath := path.Join(portBasePath, testPortId, portVLANAssignmentsPath, testAssignmentId)
-						if expectedPath != req.URL.Path {
+						if expectedPath != pathURL {
 							return nil, fmt.Errorf("wrong url")
 						}
-						if err := json.NewDecoder(strings.NewReader(raw)).Decode(obj); err != nil {
+						if err := json.NewDecoder(strings.NewReader(raw)).Decode(v); err != nil {
 							return nil, err
 						}
 
-						return mockResponse(200, raw, req), nil
-					}
-
-					return &MockClient{
-						fnDoRequest: mockDoRequest(mockNR, mockDo),
-					}
-				})(),
+						return mockResponse(200, raw, nil), nil
+					},
+				},
 			},
 			args: args{portID: testPortId, assignmentID: testAssignmentId},
+			// TODO: This is testing the code residing inside (json.Decoder).Decode(), which is already exhaustively tested.
+			// What needs testing is the code residing inside VLANAssignmentServiceOp.Get().
 			want: &VLANAssignment{
 				ID:             testAssignmentId,
 				CreatedAt:      Timestamp{Time: func() time.Time { t, _ := time.Parse(time.RFC3339, "2021-05-28T16:02:33Z"); return t }()},