3
\$\begingroup\$

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?)
\$\endgroup\$
  • \$\begingroup\$ I find your use of the word class disturbing ;-P. You're trying to use patterns/approaches that don't map terribly to golang \$\endgroup\$ – Elias Van Ootegem Jul 23 '18 at 6:25
1
\$\begingroup\$

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

\$\endgroup\$
  • \$\begingroup\$ in this case how would test error case of createSearchRequest? \$\endgroup\$ – alioygur Jun 18 '18 at 14:59
  • \$\begingroup\$ 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. \$\endgroup\$ – oliverpool Jun 19 '18 at 7:52
  • \$\begingroup\$ 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) \$\endgroup\$ – oliverpool Jun 19 '18 at 7:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.