Repository: Shopify/themekit
Pull request #737 (2020-07-23 18:16:41)

Ability to add custom headers from the Theme Client
This PR adds a way to pass headers to request made by the http interface. This will allow us in the future to be able to add the content lock on the API in core using the previous checksum when that functionality is available.

### Warn Checklist
- [ ] This changes the interface and requires a Major/Minor version change.
- [ ] I have :tophat:'d these changes by using the commands I changed by hand.
- [ ] I have added a dependancy to the project.



Patch:
diff --git a/src/httpify/client.go b/src/httpify/client.go
index a8e7c5e..52c284f 100644
--- a/src/httpify/client.go
+++ b/src/httpify/client.go
@@ -65,29 +65,29 @@ func NewClient(params Params) (*HTTPClient, error) {
 }
 
 // Get will send a get request to the path provided
-func (client *HTTPClient) Get(path string) (*http.Response, error) {
-	return client.do("GET", path, nil)
+func (client *HTTPClient) Get(path string, headers map[string]string) (*http.Response, error) {
+	return client.do("GET", path, nil, headers)
 }
 
 // Post will send a Post request to the path provided and set the post body as the
 // object passed
-func (client *HTTPClient) Post(path string, body interface{}) (*http.Response, error) {
-	return client.do("POST", path, body)
+func (client *HTTPClient) Post(path string, body interface{}, headers map[string]string) (*http.Response, error) {
+	return client.do("POST", path, body, headers)
 }
 
 // Put will send a Put request to the path provided and set the post body as the
 // object passed
-func (client *HTTPClient) Put(path string, body interface{}) (*http.Response, error) {
-	return client.do("PUT", path, body)
+func (client *HTTPClient) Put(path string, body interface{}, headers map[string]string) (*http.Response, error) {
+	return client.do("PUT", path, body, headers)
 }
 
 // Delete will send a delete request to the path provided
-func (client *HTTPClient) Delete(path string) (*http.Response, error) {
-	return client.do("DELETE", path, nil)
+func (client *HTTPClient) Delete(path string, headers map[string]string) (*http.Response, error) {
+	return client.do("DELETE", path, nil, headers)
 }
 
 // do will issue an authenticated json request to shopify.
-func (client *HTTPClient) do(method, path string, body interface{}) (*http.Response, error) {
+func (client *HTTPClient) do(method, path string, body interface{}, headers map[string]string) (*http.Response, error) {
 	var jsonData io.Reader
 	if body != nil {
 		data, err := json.Marshal(body)
@@ -106,6 +106,9 @@ func (client *HTTPClient) do(method, path string, body interface{}) (*http.Respo
 	req.Header.Add("Content-Type", "application/json")
 	req.Header.Add("Accept", "application/json")
 	req.Header.Add("User-Agent", fmt.Sprintf("go/themekit (%s; %s; %s)", runtime.GOOS, runtime.GOARCH, release.ThemeKitVersion.String()))
+	for label, value := range headers {
+		req.Header.Add(label, value)
+	}
 
 	client.limit.Wait()
 	resp, err := client.client.Do(req)
diff --git a/src/shopify/_mocks/httpAdapter.go b/src/shopify/_mocks/httpAdapter.go
index c2138d5..4df4875 100644
--- a/src/shopify/_mocks/httpAdapter.go
+++ b/src/shopify/_mocks/httpAdapter.go
@@ -1,4 +1,5 @@
 // Code generated by mockery v1.0.0. DO NOT EDIT.
+
 package mocks
 
 import http "net/http"
@@ -9,13 +10,13 @@ type HttpAdapter struct {
 	mock.Mock
 }
 
-// Delete provides a mock function with given fields: _a0
-func (_m *HttpAdapter) Delete(_a0 string) (*http.Response, error) {
-	ret := _m.Called(_a0)
+// Delete provides a mock function with given fields: _a0, _a1
+func (_m *HttpAdapter) Delete(_a0 string, _a1 map[string]string) (*http.Response, error) {
+	ret := _m.Called(_a0, _a1)
 
 	var r0 *http.Response
-	if rf, ok := ret.Get(0).(func(string) *http.Response); ok {
-		r0 = rf(_a0)
+	if rf, ok := ret.Get(0).(func(string, map[string]string) *http.Response); ok {
+		r0 = rf(_a0, _a1)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).(*http.Response)
@@ -23,8 +24,8 @@ func (_m *HttpAdapter) Delete(_a0 string) (*http.Response, error) {
 	}
 
 	var r1 error
-	if rf, ok := ret.Get(1).(func(string) error); ok {
-		r1 = rf(_a0)
+	if rf, ok := ret.Get(1).(func(string, map[string]string) error); ok {
+		r1 = rf(_a0, _a1)
 	} else {
 		r1 = ret.Error(1)
 	}
@@ -32,13 +33,13 @@ func (_m *HttpAdapter) Delete(_a0 string) (*http.Response, error) {
 	return r0, r1
 }
 
-// Get provides a mock function with given fields: _a0
-func (_m *HttpAdapter) Get(_a0 string) (*http.Response, error) {
-	ret := _m.Called(_a0)
+// Get provides a mock function with given fields: _a0, _a1
+func (_m *HttpAdapter) Get(_a0 string, _a1 map[string]string) (*http.Response, error) {
+	ret := _m.Called(_a0, _a1)
 
 	var r0 *http.Response
-	if rf, ok := ret.Get(0).(func(string) *http.Response); ok {
-		r0 = rf(_a0)
+	if rf, ok := ret.Get(0).(func(string, map[string]string) *http.Response); ok {
+		r0 = rf(_a0, _a1)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).(*http.Response)
@@ -46,8 +47,8 @@ func (_m *HttpAdapter) Get(_a0 string) (*http.Response, error) {
 	}
 
 	var r1 error
-	if rf, ok := ret.Get(1).(func(string) error); ok {
-		r1 = rf(_a0)
+	if rf, ok := ret.Get(1).(func(string, map[string]string) error); ok {
+		r1 = rf(_a0, _a1)
 	} else {
 		r1 = ret.Error(1)
 	}
@@ -55,13 +56,13 @@ func (_m *HttpAdapter) Get(_a0 string) (*http.Response, error) {
 	return r0, r1
 }
 
-// Post provides a mock function with given fields: _a0, _a1
-func (_m *HttpAdapter) Post(_a0 string, _a1 interface{}) (*http.Response, error) {
-	ret := _m.Called(_a0, _a1)
+// Post provides a mock function with given fields: _a0, _a1, _a2
+func (_m *HttpAdapter) Post(_a0 string, _a1 interface{}, _a2 map[string]string) (*http.Response, error) {
+	ret := _m.Called(_a0, _a1, _a2)
 
 	var r0 *http.Response
-	if rf, ok := ret.Get(0).(func(string, interface{}) *http.Response); ok {
-		r0 = rf(_a0, _a1)
+	if rf, ok := ret.Get(0).(func(string, interface{}, map[string]string) *http.Response); ok {
+		r0 = rf(_a0, _a1, _a2)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).(*http.Response)
@@ -69,8 +70,8 @@ func (_m *HttpAdapter) Post(_a0 string, _a1 interface{}) (*http.Response, error)
 	}
 
 	var r1 error
-	if rf, ok := ret.Get(1).(func(string, interface{}) error); ok {
-		r1 = rf(_a0, _a1)
+	if rf, ok := ret.Get(1).(func(string, interface{}, map[string]string) error); ok {
+		r1 = rf(_a0, _a1, _a2)
 	} else {
 		r1 = ret.Error(1)
 	}
@@ -78,13 +79,13 @@ func (_m *HttpAdapter) Post(_a0 string, _a1 interface{}) (*http.Response, error)
 	return r0, r1
 }
 
-// Put provides a mock function with given fields: _a0, _a1
-func (_m *HttpAdapter) Put(_a0 string, _a1 interface{}) (*http.Response, error) {
-	ret := _m.Called(_a0, _a1)
+// Put provides a mock function with given fields: _a0, _a1, _a2
+func (_m *HttpAdapter) Put(_a0 string, _a1 interface{}, _a2 map[string]string) (*http.Response, error) {
+	ret := _m.Called(_a0, _a1, _a2)
 
 	var r0 *http.Response
-	if rf, ok := ret.Get(0).(func(string, interface{}) *http.Response); ok {
-		r0 = rf(_a0, _a1)
+	if rf, ok := ret.Get(0).(func(string, interface{}, map[string]string) *http.Response); ok {
+		r0 = rf(_a0, _a1, _a2)
 	} else {
 		if ret.Get(0) != nil {
 			r0 = ret.Get(0).(*http.Response)
@@ -92,8 +93,8 @@ func (_m *HttpAdapter) Put(_a0 string, _a1 interface{}) (*http.Response, error)
 	}
 
 	var r1 error
-	if rf, ok := ret.Get(1).(func(string, interface{}) error); ok {
-		r1 = rf(_a0, _a1)
+	if rf, ok := ret.Get(1).(func(string, interface{}, map[string]string) error); ok {
+		r1 = rf(_a0, _a1, _a2)
 	} else {
 		r1 = ret.Error(1)
 	}
diff --git a/src/shopify/theme_client.go b/src/shopify/theme_client.go
index ba35198..7d44b1d 100644
--- a/src/shopify/theme_client.go
+++ b/src/shopify/theme_client.go
@@ -83,10 +83,10 @@ type reqErr struct {
 }
 
 type httpAdapter interface {
-	Get(string) (*http.Response, error)
-	Post(string, interface{}) (*http.Response, error)
-	Put(string, interface{}) (*http.Response, error)
-	Delete(string) (*http.Response, error)
+	Get(string, map[string]string) (*http.Response, error)
+	Post(string, interface{}, map[string]string) (*http.Response, error)
+	Put(string, interface{}, map[string]string) (*http.Response, error)
+	Delete(string, map[string]string) (*http.Response, error)
 }
 
 // Client is the interactor with the shopify server. All actions are processed
@@ -126,7 +126,7 @@ func NewClient(e *env.Env) (Client, error) {
 
 // GetShop will return information for the shop you are working on
 func (c Client) GetShop() (Shop, error) {
-	resp, err := c.http.Get("/meta.json")
+	resp, err := c.http.Get("/meta.json", nil)
 	if err != nil {
 		return Shop{}, err
 	} else if resp.StatusCode == 404 {
@@ -143,7 +143,7 @@ func (c Client) GetShop() (Shop, error) {
 
 // Themes will return all the available themes on a domain.
 func (c Client) Themes() ([]Theme, error) {
-	resp, err := c.http.Get("/admin/themes.json")
+	resp, err := c.http.Get("/admin/themes.json", nil)
 	if err != nil {
 		return []Theme{}, err
 	}
@@ -163,7 +163,7 @@ func (c *Client) CreateNewTheme(name string) (theme Theme, err error) {
 		return Theme{}, ErrThemeNameRequired
 	}
 
-	resp, err := c.http.Post("/admin/themes.json", map[string]interface{}{"theme": Theme{Name: name}})
+	resp, err := c.http.Post("/admin/themes.json", map[string]interface{}{"theme": Theme{Name: name}}, nil)
 	if err != nil {
 		return Theme{}, err
 	}
@@ -187,7 +187,7 @@ func (c Client) GetInfo() (Theme, error) {
 		return Theme{}, ErrInfoWithoutThemeID
 	}
 
-	resp, err := c.http.Get(fmt.Sprintf("/admin/themes/%s.json", c.themeID))
+	resp, err := c.http.Get(fmt.Sprintf("/admin/themes/%s.json", c.themeID), nil)
 	if err != nil {
 		return Theme{}, err
 	} else if resp.StatusCode == 404 {
@@ -211,6 +211,7 @@ func (c Client) PublishTheme() error {
 	resp, err := c.http.Put(
 		fmt.Sprintf("/admin/themes/%s.json", c.themeID),
 		map[string]Theme{"theme": {Role: "main"}},
+		nil,
 	)
 	if err != nil {
 		return err
@@ -235,7 +236,7 @@ func (c Client) PublishTheme() error {
 // The assets returned will not have any data, only ID and filenames. This is because
 // fetching all the assets at one time is not a good idea.
 func (c Client) GetAllAssets() ([]Asset, error) {
-	resp, err := c.http.Get(c.assetPath(map[string]string{"fields": "key,checksum"}))
+	resp, err := c.http.Get(c.assetPath(map[string]string{"fields": "key,checksum"}), nil)
 	if err != nil {
 		return []Asset{}, err
 	} else if resp.StatusCode == 404 {
@@ -260,7 +261,7 @@ func (c Client) GetAllAssets() ([]Asset, error) {
 
 // GetAsset will fetch a single remote asset from the remote shopify servers.
 func (c Client) GetAsset(filename string) (Asset, error) {
-	resp, err := c.http.Get(c.assetPath(map[string]string{"asset[key]": filename}))
+	resp, err := c.http.Get(c.assetPath(map[string]string{"asset[key]": filename}), nil)
 	if err != nil {
 		return Asset{}, err
 	} else if resp.StatusCode == 404 {
@@ -286,7 +287,7 @@ func (c Client) CreateAsset(asset Asset) error {
 // If there was an error, in the request then error will be defined otherwise the
 //response will have the appropropriate data for usage.
 func (c Client) UpdateAsset(asset Asset) error {
-	resp, err := c.http.Put(c.assetPath(map[string]string{}), map[string]Asset{"asset": asset})
+	resp, err := c.http.Put(c.assetPath(map[string]string{}), map[string]Asset{"asset": asset}, nil)
 	if err != nil {
 		return err
 	} else if resp.StatusCode == 404 {
@@ -317,7 +318,7 @@ func (c Client) UpdateAsset(asset Asset) error {
 // If there was an error, in the request then error will be defined otherwise the
 //response will have the appropropriate data for usage.
 func (c Client) DeleteAsset(asset Asset) error {
-	resp, err := c.http.Delete(c.assetPath(map[string]string{"asset[key]": asset.Key}))
+	resp, err := c.http.Delete(c.assetPath(map[string]string{"asset[key]": asset.Key}), nil)
 	if err != nil {
 		return err
 	} else if resp.StatusCode == 403 {


Test patch:
diff --git a/src/httpify/client_test.go b/src/httpify/client_test.go
index d7629a6..c021f60 100644
--- a/src/httpify/client_test.go
+++ b/src/httpify/client_test.go
@@ -36,6 +36,7 @@ func TestClient_do(t *testing.T) {
 
 	var client *HTTPClient
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Equal(t, r.Header.Get("X-Custom-Header"), "Checksum")
 		assert.Equal(t, r.Header.Get("X-Shopify-Access-Token"), client.password)
 		assert.Equal(t, r.Header.Get("Content-Type"), "application/json")
 		assert.Equal(t, r.Header.Get("Accept"), "application/json")
@@ -63,17 +64,18 @@ func TestClient_do(t *testing.T) {
 	assert.NotNil(t, client)
 	assert.Nil(t, err)
 
-	resp, err := client.Post("/assets.json", body)
+	resp, err := client.Post("/assets.json", body, map[string]string{"X-Custom-Header": "Checksum"})
 	assert.Nil(t, err)
 	assert.NotNil(t, resp)
 
-	resp, err = client.Put("/assets.json", body)
+	resp, err = client.Put("/assets.json", body, map[string]string{"X-Custom-Header": "Checksum"})
 	assert.Nil(t, err)
 	assert.NotNil(t, resp)
 
 	server.Close()
 
 	server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		assert.Equal(t, r.Header.Get("X-Custom-Header"), "Foo")
 		assert.Equal(t, r.Header.Get("X-Shopify-Access-Token"), client.password)
 		assert.Equal(t, r.Header.Get("Content-Type"), "application/json")
 		assert.Equal(t, r.Header.Get("Accept"), "application/json")
@@ -87,11 +89,11 @@ func TestClient_do(t *testing.T) {
 	})
 	client.baseURL.Scheme = "http"
 
-	resp, err = client.Get("/assets.json")
+	resp, err = client.Get("/assets.json", map[string]string{"X-Custom-Header": "Foo"})
 	assert.Nil(t, err)
 	assert.NotNil(t, resp)
 
-	resp, err = client.Delete("/assets.json")
+	resp, err = client.Delete("/assets.json", map[string]string{"X-Custom-Header": "Foo"})
 	assert.Nil(t, err)
 	assert.NotNil(t, resp)
 
@@ -112,7 +114,7 @@ func TestClient_do(t *testing.T) {
 	assert.NotNil(t, client)
 	assert.Nil(t, err)
 
-	_, err = client.do("POST", "/assets.json", body)
+	_, err = client.do("POST", "/assets.json", body, nil)
 	if assert.NotNil(t, err) {
 		assert.Equal(t, err, errClientTimeout)
 	}
diff --git a/src/shopify/theme_client_test.go b/src/shopify/theme_client_test.go
index 351291b..a52de06 100644
--- a/src/shopify/theme_client_test.go
+++ b/src/shopify/theme_client_test.go
@@ -14,6 +14,8 @@ import (
 	"github.com/stretchr/testify/mock"
 )
 
+var NoHeaders = map[string]string(nil)
+
 func TestNewThemeClient(t *testing.T) {
 	testcases := []struct {
 		e   *env.Env
@@ -53,7 +55,7 @@ func TestThemeClient_GetShop(t *testing.T) {
 		client, _ := NewClient(&env.Env{ThemeID: testcase.themeID})
 		client.http = m
 
-		expectation := m.On("Get", "/meta.json")
+		expectation := m.On("Get", "/meta.json", NoHeaders)
 		if testcase.resperr != "" {
 			expectation.Return(nil, errors.New(testcase.resperr))
 		} else {
@@ -90,7 +92,7 @@ func TestThemeClient_Themes(t *testing.T) {
 		client, _ := NewClient(&env.Env{})
 		client.http = m
 
-		expectation := m.On("Get", "/admin/themes.json")
+		expectation := m.On("Get", "/admin/themes.json", NoHeaders)
 		if testcase.resperr != "" {
 			expectation.Return(nil, errors.New(testcase.resperr))
 		} else {
@@ -131,9 +133,9 @@ func TestThemeClient_CreateNewTheme(t *testing.T) {
 		query := map[string]interface{}{"theme": Theme{Name: testcase.in}}
 
 		if testcase.resp != "" {
-			m.On("Post", "/admin/themes.json", query).Return(jsonResponse(testcase.resp, 200), nil)
+			m.On("Post", "/admin/themes.json", query, NoHeaders).Return(jsonResponse(testcase.resp, 200), nil)
 		} else if testcase.resperr != "" {
-			m.On("Post", "/admin/themes.json", query).Return(nil, errors.New(testcase.resperr))
+			m.On("Post", "/admin/themes.json", query, NoHeaders).Return(nil, errors.New(testcase.resperr))
 		}
 
 		theme, err := client.CreateNewTheme(testcase.in)
@@ -169,7 +171,7 @@ func TestThemeClient_GetInfo(t *testing.T) {
 		client, _ := NewClient(&env.Env{ThemeID: testcase.themeID})
 		client.http = m
 
-		expectation := m.On("Get", fmt.Sprintf("/admin/themes/%s.json", testcase.themeID))
+		expectation := m.On("Get", fmt.Sprintf("/admin/themes/%s.json", testcase.themeID), NoHeaders)
 		if testcase.resperr != "" {
 			expectation.Return(nil, errors.New(testcase.resperr))
 		} else {
@@ -214,6 +216,7 @@ func TestThemeClient_PublishTheme(t *testing.T) {
 			"Put",
 			fmt.Sprintf("/admin/themes/%s.json", testcase.themeID),
 			map[string]Theme{"theme": {Role: "main"}},
+			NoHeaders,
 		)
 		if testcase.resperr != "" {
 			expectation.Return(nil, errors.New(testcase.resperr))
@@ -251,7 +254,7 @@ func TestThemeClient_GetAllAssets(t *testing.T) {
 		client, _ := NewClient(&env.Env{ThemeID: "123"})
 		client.http = m
 
-		expectation := m.On("Get", "/admin/themes/123/assets.json?fields=key%2Cchecksum")
+		expectation := m.On("Get", "/admin/themes/123/assets.json?fields=key%2Cchecksum", NoHeaders)
 		if testcase.resperr != "" {
 			expectation.Return(nil, errors.New(testcase.resperr))
 		} else {
@@ -294,7 +297,7 @@ func TestThemeClient_GetAllAssets(t *testing.T) {
 		m := new(mocks.HttpAdapter)
 		client, _ := NewClient(&env.Env{ThemeID: "123", IgnoredFiles: testcase.ignore})
 		client.http = m
-		m.On("Get", "/admin/themes/123/assets.json?fields=key%2Cchecksum").Return(jsonResponse(testcase.input, 200), nil)
+		m.On("Get", "/admin/themes/123/assets.json?fields=key%2Cchecksum", NoHeaders).Return(jsonResponse(testcase.input, 200), nil)
 		assets, err := client.GetAllAssets()
 		assert.Nil(t, err)
 		assert.Equal(t, testcase.expected, assets)
@@ -317,7 +320,7 @@ func TestThemeClient_GetAsset(t *testing.T) {
 		client, _ := NewClient(&env.Env{ThemeID: "123"})
 		client.http = m
 
-		expectation := m.On("Get", "/admin/themes/123/assets.json?asset%5Bkey%5D=filename.txt")
+		expectation := m.On("Get", "/admin/themes/123/assets.json?asset%5Bkey%5D=filename.txt", NoHeaders)
 		if testcase.resperr != "" {
 			expectation.Return(nil, errors.New(testcase.resperr))
 		} else if testcase.code != 0 {
@@ -353,7 +356,7 @@ func TestThemeClient_UpdateAsset(t *testing.T) {
 		client, _ := NewClient(&env.Env{ThemeID: "123"})
 		client.http = m
 
-		expectation := m.On("Put", "/admin/themes/123/assets.json", map[string]Asset{"asset": {Key: "filename.txt"}})
+		expectation := m.On("Put", "/admin/themes/123/assets.json", map[string]Asset{"asset": {Key: "filename.txt"}}, NoHeaders)
 		if testcase.resperr != "" {
 			expectation.Return(nil, errors.New(testcase.resperr))
 		} else if testcase.code != 0 {
@@ -387,6 +390,7 @@ func TestThemeClient_UpdateAsset(t *testing.T) {
 			return false
 		}),
 		map[string]Asset{"asset": asset},
+		NoHeaders,
 	).Return(&http.Response{
 		Body:       &StringReadCloser{strings.NewReader(`{"errors":{"asset":["Cannot overwrite generated asset filename.txt"]}}`)},
 		StatusCode: 422,
@@ -395,12 +399,14 @@ func TestThemeClient_UpdateAsset(t *testing.T) {
 	m.On(
 		"Delete",
 		"/admin/themes/123/assets.json?asset%5Bkey%5D=filename.txt.liquid",
+		NoHeaders,
 	).Return(jsonResponse("{}", 200), nil)
 
 	m.On(
 		"Put",
 		"/admin/themes/123/assets.json",
 		map[string]Asset{"asset": asset},
+		NoHeaders,
 	).Return(jsonResponse(`{"asset":{"key":"assets/hello.txt"}}`, 200), nil)
 
 	assert.Nil(t, client.UpdateAsset(asset))
@@ -423,7 +429,7 @@ func TestThemeClient_DeleteAsset(t *testing.T) {
 		client, _ := NewClient(&env.Env{ThemeID: "123"})
 		client.http = m
 
-		expectation := m.On("Delete", "/admin/themes/123/assets.json?asset%5Bkey%5D=filename.txt")
+		expectation := m.On("Delete", "/admin/themes/123/assets.json?asset%5Bkey%5D=filename.txt", NoHeaders)
 		if testcase.resperr != "" {
 			expectation.Return(nil, errors.New(testcase.resperr))
 		} else {