API client, using an injectable helper to aid testing

Question

description

write a api client that fetch items from remote service

I have separated Service{} class's helper methods into another class serviceHelper{} and made it injectable to Service{} class.

With this approach I think I get these benefits

• I can mocking serviceHelper{} class
• When I mock the serviceHelper{} class then I easily testing the Service{} class.

client.go

// Media Types
const (
 ApplicationForm = "application/x-www-form-urlencoded"
)
// Client interface
type Client interface {
 Do(*http.Request, interface{}) (*http.Response, error)
}
type serviceHelperer interface {
 createSearchRequest(*search.CoralSearchRequest) (*http.Request, error)
 toAPIError(error, int) error
}
// Service service
type Service struct {
 Username string
 Password string
 client Client
 h serviceHelperer
}
type serviceHelper struct {
}
// New instances new coral service
func New(username string, password string, c Client) *Service {
 return &Service{username, password, c, &serviceHelper{}}
}
func (h *serviceHelper) toAPIError(err error, status int) error {
 if err == nil {
 return err
 }
 if status > 399 || status < 200 {
 var apiErr domain.CoralAPIError
 if err2 := json.Unmarshal([]byte(err.Error()), &apiErr); err2 != nil {
 return err
 }
 return apiErr
 }
 return err
}
const defaultCurrency = "TRY"
const defaultNationality = "TR"
// createSearchRequest creates search request
func (h *serviceHelper) createSearchRequest(r *search.CoralSearchRequest) (*http.Request, error) {
 // send hotel_codes field as post request instead of get,
 // beacause of hotel_codes there can be thousands (url too long error)
 var body io.Reader
 if len(r.HotelCodes) > 0 {
 form := url.Values{"hotel_code": r.HotelCodes}
 body = strings.NewReader(form.Encode())
 }
 query := url.Values{
 "pax": r.Pax,
 "checkin": []string{r.Checkin},
 "checkout": []string{r.Checkout},
 "client_nationality": []string{defaultNationality},
 "currency": []string{defaultCurrency},
 "destination_code": []string{r.DestinationCode},
 }
 req, err := http.NewRequest("POST", "search?"+query.Encode(), body)
 if err != nil {
 return nil, err
 }
 req.Header.Set("Content-Type", ApplicationForm)
 return req, nil
}
// Search makes search request to coral.
func (s *Service) Search(r *search.CoralSearchRequest) (*search.CoralSearchResponse, error) {
 req, err := s.h.createSearchRequest(r)
 if err != nil {
 return nil, err
 }
 req.SetBasicAuth(s.Username, s.Password)
 var sr search.CoralSearchResponse
 res, err := s.client.Do(req, &sr)
 err = s.h.toAPIError(err, res.StatusCode)
 return &sr, err
}

mock_test.go contains mock classes

// this file contains mocks shared with this package
type serviceHelperMock struct {
 mock.Mock
}
type clientMock struct {
 mock.Mock
}
func (sm *serviceHelperMock) createSearchRequest(r *search.CoralSearchRequest) (*http.Request, error) {
 args := sm.Mock.Called(r)
 return args.Get(0).(*http.Request), args.Error(1)
}
func (sm *serviceHelperMock) toAPIError(err error, status int) error {
 args := sm.Mock.Called(err, status)
 return args.Error(0)
}
func (m *clientMock) Do(r *http.Request, v interface{}) (*http.Response, error) {
 args := m.Mock.Called(r, v)
 return args.Get(0).(*http.Response), args.Error(1)
}

client_test.go contains tests

var baseURL = &url.URL{
 Scheme: "https",
 Host: "example.com",
 Path: "/",
}
func TestService_Search(t *testing.T) {
 t.Parallel()
 client := &clientMock{}
 helper := &serviceHelperMock{}
 service := Service{"", "", client, helper}
 coralSearchReq := &search.CoralSearchRequest{Pax: []string{"1"}}
 coralSearchRes := &search.CoralSearchResponse{}
 searchRes := &http.Response{StatusCode: http.StatusOK}
 helper.On("createSearchRequest", coralSearchReq).Return(&http.Request{}, nil).Once()
 client.On("Do", mock.Anything, coralSearchRes).Return(searchRes, nil).Once()
 helper.On("toAPIError", nil, searchRes.StatusCode).Return(nil).Once()
 actualCoralSearchRes, err := service.Search(coralSearchReq)
 helper.AssertExpectations(t)
 client.AssertExpectations(t)
 ensure.Nil(t, err)
 ensure.DeepEqual(t, actualCoralSearchRes, coralSearchRes)
 t.Run("should stop execution when createSearchRequest() return an error", func(t *testing.T) {
 client := &clientMock{}
 helper := &serviceHelperMock{}
 service := Service{"", "", client, helper}
 err := errors.New("unexpected")
 helper.On("createSearchRequest", coralSearchReq).Return(&http.Request{}, err).Once()
 _, actualErr := service.Search(coralSearchReq)
 helper.AssertExpectations(t)
 ensure.DeepEqual(t, actualErr, err)
 })
}
func Test_serviceHelper_createSearchRequest(t *testing.T) {
 t.Parallel()
 var pax = []string{"2,1", "3,1"}
 var checkin = "2018-06-20"
 var checkout = "2018-06-30"
 var hotelCodes = []string{"code1", "code2"}
 var destinationCode = ""
 var nationality = "TR"
 var currency = "TRY"
 query := url.Values{
 "pax": pax,
 "checkin": []string{checkin},
 "checkout": []string{checkout},
 "destination_code": []string{destinationCode},
 "client_nationality": []string{nationality},
 "currency": []string{currency},
 }
 body := ioutil.NopCloser(strings.NewReader(url.Values{"hotel_code": hotelCodes}.Encode()))
 sh := &serviceHelper{}
 r, err := sh.createSearchRequest(&search.CoralSearchRequest{
 Pax: pax,
 Checkin: checkin,
 Checkout: checkout,
 HotelCodes: hotelCodes,
 })
 ensure.Nil(t, err)
 ensure.DeepEqual(t, r.Method, "POST")
 ensure.DeepEqual(t, r.URL.String(), "search?"+query.Encode())
 ensure.DeepEqual(t, r.Header.Get("Content-Type"), ApplicationForm)
 ensure.DeepEqual(t, r.Body, body)
 // todo: test basic auth header?
}

Questions

• is this method correct?
• is there better way than that?
• do you have a better name than serviceHelper (*Helper?)

• Elias Van Ootegem: I find your use of the word class disturbing ;-P. You're trying to use patterns/approaches that don't map terribly to golang

Answer 1

I would recommend to change a little bit how you use the interfaces.

Since createSearchRequest and toAPIError don't have external dependencies, I would attach them directly to your Service (and remove serviceHelper)

Since the Client is already an interface, you are already able mock it (to prevent performing a real http request and return errors with special statuses).

However you could turn your Service into an interface:

type Searcher interface {
 Search(*search.CoralSearchRequest) (*search.CoralSearchResponse, error)
}

It would greatly ease the testing of packages depending on this Service

• alioygur: in this case how would test error case of createSearchRequest?
• oliverpool: That's a good point. Actually it should never fail: http.NewRequest returns an error if: the method is not valid ("POST" is valid) / if the url is not parsable ("search?"+query.Encode() is parsable) / if the io.Reader returns an error, which won't be the case.
• oliverpool: But if you really want to test this case, then you could provide add a NewRequest property to the Service struct and initialize it with http.NewRequest (or some other mock to trigger the error)