diff options
author | Harrison Healey <harrisonmhealey@gmail.com> | 2018-09-07 09:24:18 -0400 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-09-07 09:24:18 -0400 |
commit | 0027d998555d47f9a75a896d8c6c85a8b4645ad0 (patch) | |
tree | 4a15d63c143e9891921d75f102dd008852c9abb2 /app | |
parent | 291000703320cc53c3bdecd2553fa4a3021d7234 (diff) | |
download | chat-0027d998555d47f9a75a896d8c6c85a8b4645ad0.tar.gz chat-0027d998555d47f9a75a896d8c6c85a8b4645ad0.tar.bz2 chat-0027d998555d47f9a75a896d8c6c85a8b4645ad0.zip |
MM-11855 Add App.HTTPService to allow mocking of HTTP client (#9359)
* MM-11855 Add App.HTTPService to allow mocking of HTTP client
* Initialize HTTPService earlier
Diffstat (limited to 'app')
-rw-r--r-- | app/app.go | 46 | ||||
-rw-r--r-- | app/apptestlib.go | 30 | ||||
-rw-r--r-- | app/command.go | 2 | ||||
-rw-r--r-- | app/http_service.go | 67 | ||||
-rw-r--r-- | app/http_service_test.go | 65 | ||||
-rw-r--r-- | app/notification_push.go | 2 | ||||
-rw-r--r-- | app/oauth.go | 4 | ||||
-rw-r--r-- | app/post.go | 6 | ||||
-rw-r--r-- | app/webhook.go | 2 | ||||
-rw-r--r-- | app/webrtc.go | 4 |
10 files changed, 179 insertions, 49 deletions
diff --git a/app/app.go b/app/app.go index 2e0433d23..c3fcc7aec 100644 --- a/app/app.go +++ b/app/app.go @@ -7,12 +7,10 @@ import ( "crypto/ecdsa" "fmt" "html/template" - "net" "net/http" "path" "reflect" "strconv" - "strings" "sync" "sync/atomic" @@ -100,6 +98,8 @@ type App struct { diagnosticId string phase2PermissionsMigrationComplete bool + + HTTPService HTTPService } var appCount = 0 @@ -125,6 +125,9 @@ func New(options ...Option) (outApp *App, outErr error) { clientConfig: make(map[string]string), licenseListeners: map[string]func(){}, } + + app.HTTPService = MakeHTTPService(app) + defer func() { if outErr != nil { app.Shutdown() @@ -285,6 +288,8 @@ func (a *App) Shutdown() { mlog.Info("Server stopped") a.DisableConfigWatch() + + a.HTTPService.Close() } var accountMigrationInterface func(*App) einterfaces.AccountMigrationInterface @@ -505,43 +510,6 @@ func (a *App) HTMLTemplates() *template.Template { return nil } -func (a *App) HTTPClient(trustURLs bool) *http.Client { - insecure := a.Config().ServiceSettings.EnableInsecureOutgoingConnections != nil && *a.Config().ServiceSettings.EnableInsecureOutgoingConnections - - if trustURLs { - return utils.NewHTTPClient(insecure, nil, nil) - } - - allowHost := func(host string) bool { - if a.Config().ServiceSettings.AllowedUntrustedInternalConnections == nil { - return false - } - for _, allowed := range strings.Fields(*a.Config().ServiceSettings.AllowedUntrustedInternalConnections) { - if host == allowed { - return true - } - } - return false - } - - allowIP := func(ip net.IP) bool { - if !utils.IsReservedIP(ip) { - return true - } - if a.Config().ServiceSettings.AllowedUntrustedInternalConnections == nil { - return false - } - for _, allowed := range strings.Fields(*a.Config().ServiceSettings.AllowedUntrustedInternalConnections) { - if _, ipRange, err := net.ParseCIDR(allowed); err == nil && ipRange.Contains(ip) { - return true - } - } - return false - } - - return utils.NewHTTPClient(insecure, allowHost, allowIP) -} - func (a *App) Handle404(w http.ResponseWriter, r *http.Request) { err := model.NewAppError("Handle404", "api.context.404.app_error", nil, "", http.StatusNotFound) diff --git a/app/apptestlib.go b/app/apptestlib.go index 48783f49c..c0d2cfaa2 100644 --- a/app/apptestlib.go +++ b/app/apptestlib.go @@ -6,6 +6,8 @@ package app import ( "io" "io/ioutil" + "net/http" + "net/http/httptest" "os" "path/filepath" "time" @@ -33,6 +35,8 @@ type TestHelper struct { tempConfigPath string tempWorkspace string + + MockedHTTPService *MockedHTTPService } type persistentTestStore struct { @@ -163,6 +167,13 @@ func (me *TestHelper) InitSystemAdmin() *TestHelper { return me } +func (me *TestHelper) MockHTTPService(handler http.Handler) *TestHelper { + me.MockedHTTPService = MakeMockedHTTPService(handler) + me.App.HTTPService = me.MockedHTTPService + + return me +} + func (me *TestHelper) MakeEmail() string { return "success_" + model.NewId() + "@simulator.amazonses.com" } @@ -503,3 +514,22 @@ func (me *FakeClusterInterface) sendClearRoleCacheMessage() { Event: model.CLUSTER_EVENT_INVALIDATE_CACHE_FOR_ROLES, }) } + +type MockedHTTPService struct { + Server *httptest.Server +} + +func MakeMockedHTTPService(handler http.Handler) *MockedHTTPService { + return &MockedHTTPService{ + Server: httptest.NewServer(handler), + } +} + +func (h *MockedHTTPService) MakeClient(trustURLs bool) *http.Client { + return h.Server.Client() +} + +func (h *MockedHTTPService) Close() { + h.Server.CloseClientConnections() + h.Server.Close() +} diff --git a/app/command.go b/app/command.go index 92c35865a..124668f6b 100644 --- a/app/command.go +++ b/app/command.go @@ -244,7 +244,7 @@ func (a *App) ExecuteCommand(args *model.CommandArgs) (*model.CommandResponse, * req.Header.Set("Content-Type", "application/x-www-form-urlencoded") } - if resp, err := a.HTTPClient(false).Do(req); err != nil { + if resp, err := a.HTTPService.MakeClient(false).Do(req); err != nil { return nil, model.NewAppError("command", "api.command.execute_command.failed.app_error", map[string]interface{}{"Trigger": trigger}, err.Error(), http.StatusInternalServerError) } else { if resp.StatusCode == http.StatusOK { diff --git a/app/http_service.go b/app/http_service.go new file mode 100644 index 000000000..71e72ab2f --- /dev/null +++ b/app/http_service.go @@ -0,0 +1,67 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package app + +import ( + "net" + "net/http" + "strings" + + "github.com/mattermost/mattermost-server/utils" +) + +// Wraps the functionality for creating a new http.Client to encapsulate that and allow it to be mocked when testing +type HTTPService interface { + MakeClient(trustURLs bool) *http.Client + Close() +} + +type HTTPServiceImpl struct { + app *App +} + +func MakeHTTPService(app *App) HTTPService { + return &HTTPServiceImpl{app} +} + +func (h *HTTPServiceImpl) MakeClient(trustURLs bool) *http.Client { + insecure := h.app.Config().ServiceSettings.EnableInsecureOutgoingConnections != nil && *h.app.Config().ServiceSettings.EnableInsecureOutgoingConnections + + if trustURLs { + return utils.NewHTTPClient(insecure, nil, nil) + } + + allowHost := func(host string) bool { + if h.app.Config().ServiceSettings.AllowedUntrustedInternalConnections == nil { + return false + } + for _, allowed := range strings.Fields(*h.app.Config().ServiceSettings.AllowedUntrustedInternalConnections) { + if host == allowed { + return true + } + } + return false + } + + allowIP := func(ip net.IP) bool { + if !utils.IsReservedIP(ip) { + return true + } + if h.app.Config().ServiceSettings.AllowedUntrustedInternalConnections == nil { + return false + } + for _, allowed := range strings.Fields(*h.app.Config().ServiceSettings.AllowedUntrustedInternalConnections) { + if _, ipRange, err := net.ParseCIDR(allowed); err == nil && ipRange.Contains(ip) { + return true + } + } + return false + } + + return utils.NewHTTPClient(insecure, allowHost, allowIP) +} + +func (h *HTTPServiceImpl) Close() { + // Does nothing, but allows this to be overridden when mocking the service +} diff --git a/app/http_service_test.go b/app/http_service_test.go new file mode 100644 index 000000000..396a991b1 --- /dev/null +++ b/app/http_service_test.go @@ -0,0 +1,65 @@ +package app + +import ( + "io/ioutil" + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestMockHTTPService(t *testing.T) { + getCalled := false + putCalled := false + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/get" && r.Method == http.MethodGet { + getCalled = true + + w.WriteHeader(http.StatusOK) + w.Write([]byte("OK")) + } else if r.URL.Path == "/put" && r.Method == http.MethodPut { + putCalled = true + + w.WriteHeader(http.StatusCreated) + w.Write([]byte("CREATED")) + } else { + w.WriteHeader(http.StatusNotFound) + } + }) + + th := Setup().MockHTTPService(handler) + defer th.TearDown() + + url := th.MockedHTTPService.Server.URL + + t.Run("GET", func(t *testing.T) { + client := th.App.HTTPService.MakeClient(false) + + resp, err := client.Get(url + "/get") + defer consumeAndClose(resp) + + bodyContents, _ := ioutil.ReadAll(resp.Body) + + require.Nil(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode) + assert.Equal(t, "OK", string(bodyContents)) + assert.True(t, getCalled) + }) + + t.Run("PUT", func(t *testing.T) { + client := th.App.HTTPService.MakeClient(false) + + request, _ := http.NewRequest(http.MethodPut, url+"/put", nil) + resp, err := client.Do(request) + defer consumeAndClose(resp) + + bodyContents, _ := ioutil.ReadAll(resp.Body) + + require.Nil(t, err) + assert.Equal(t, http.StatusCreated, resp.StatusCode) + assert.Equal(t, "CREATED", string(bodyContents)) + assert.True(t, putCalled) + }) +} diff --git a/app/notification_push.go b/app/notification_push.go index 12d9f5258..517988a97 100644 --- a/app/notification_push.go +++ b/app/notification_push.go @@ -184,7 +184,7 @@ func (a *App) sendToPushProxy(msg model.PushNotification, session *model.Session request, _ := http.NewRequest("POST", strings.TrimRight(*a.Config().EmailSettings.PushNotificationServer, "/")+model.API_URL_SUFFIX_V1+"/send_push", strings.NewReader(msg.ToJson())) - if resp, err := a.HTTPClient(true).Do(request); err != nil { + if resp, err := a.HTTPService.MakeClient(true).Do(request); err != nil { mlog.Error(fmt.Sprintf("Device push reported as error for UserId=%v SessionId=%v message=%v", session.UserId, session.Id, err.Error()), mlog.String("user_id", session.UserId)) } else { pushResponse := model.PushResponseFromJson(resp.Body) diff --git a/app/oauth.go b/app/oauth.go index a0123c0e9..645d502f5 100644 --- a/app/oauth.go +++ b/app/oauth.go @@ -761,7 +761,7 @@ func (a *App) AuthorizeOAuthUser(w http.ResponseWriter, r *http.Request, service var ar *model.AccessResponse var bodyBytes []byte - if resp, err := a.HTTPClient(true).Do(req); err != nil { + if resp, err := a.HTTPService.MakeClient(true).Do(req); err != nil { return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.token_failed.app_error", nil, err.Error(), http.StatusInternalServerError) } else { bodyBytes, _ = ioutil.ReadAll(resp.Body) @@ -791,7 +791,7 @@ func (a *App) AuthorizeOAuthUser(w http.ResponseWriter, r *http.Request, service req.Header.Set("Accept", "application/json") req.Header.Set("Authorization", "Bearer "+ar.AccessToken) - if resp, err := a.HTTPClient(true).Do(req); err != nil { + if resp, err := a.HTTPService.MakeClient(true).Do(req); err != nil { return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.service.app_error", map[string]interface{}{"Service": service}, err.Error(), http.StatusInternalServerError) } else { bodyBytes, _ = ioutil.ReadAll(resp.Body) diff --git a/app/post.go b/app/post.go index 30602b392..86c96926e 100644 --- a/app/post.go +++ b/app/post.go @@ -777,7 +777,7 @@ func (a *App) GetFileInfosForPost(postId string, readFromMaster bool) ([]*model. func (a *App) GetOpenGraphMetadata(requestURL string) *opengraph.OpenGraph { og := opengraph.NewOpenGraph() - res, err := a.HTTPClient(false).Get(requestURL) + res, err := a.HTTPService.MakeClient(false).Get(requestURL) if err != nil { mlog.Error(fmt.Sprintf("GetOpenGraphMetadata request failed for url=%v with err=%v", requestURL, err.Error())) return og @@ -890,9 +890,9 @@ func (a *App) DoPostAction(postId, actionId, userId, selectedOption string) *mod siteURL, _ := url.Parse(*a.Config().ServiceSettings.SiteURL) subpath, _ := utils.GetSubpathFromConfig(a.Config()) if (url.Hostname() == "localhost" || url.Hostname() == "127.0.0.1" || url.Hostname() == siteURL.Hostname()) && strings.HasPrefix(url.Path, path.Join(subpath, "plugins")) { - httpClient = a.HTTPClient(true) + httpClient = a.HTTPService.MakeClient(true) } else { - httpClient = a.HTTPClient(false) + httpClient = a.HTTPService.MakeClient(false) } resp, err := httpClient.Do(req) diff --git a/app/webhook.go b/app/webhook.go index e801b0467..f0264c0c6 100644 --- a/app/webhook.go +++ b/app/webhook.go @@ -107,7 +107,7 @@ func (a *App) TriggerWebhook(payload *model.OutgoingWebhookPayload, hook *model. req, _ := http.NewRequest("POST", url, body) req.Header.Set("Content-Type", contentType) req.Header.Set("Accept", "application/json") - if resp, err := a.HTTPClient(false).Do(req); err != nil { + if resp, err := a.HTTPService.MakeClient(false).Do(req); err != nil { mlog.Error(fmt.Sprintf("Event POST failed, err=%s", err.Error())) } else { defer consumeAndClose(resp) diff --git a/app/webrtc.go b/app/webrtc.go index b10450cab..08601a98c 100644 --- a/app/webrtc.go +++ b/app/webrtc.go @@ -59,7 +59,7 @@ func (a *App) GetWebrtcToken(sessionId string) (string, *model.AppError) { rq, _ := http.NewRequest("POST", *a.Config().WebrtcSettings.GatewayAdminUrl, strings.NewReader(model.MapToJson(data))) rq.Header.Set("Content-Type", "application/json") - if rp, err := a.HTTPClient(true).Do(rq); err != nil { + if rp, err := a.HTTPService.MakeClient(true).Do(rq); err != nil { return "", model.NewAppError("WebRTC.Token", "model.client.connecting.app_error", nil, err.Error(), http.StatusInternalServerError) } else if rp.StatusCode >= 300 { defer consumeAndClose(rp) @@ -93,5 +93,5 @@ func (a *App) RevokeWebrtcToken(sessionId string) { rq.Header.Set("Content-Type", "application/json") // we do not care about the response - a.HTTPClient(true).Do(rq) + a.HTTPService.MakeClient(true).Do(rq) } |