Move HTTPService and ConfigService into services package (#9422)

* Move HTTPService and ConfigService into utils package

* Re-add StaticConfigService

* Move config and http services into their own packages

11 files changed, 147 insertions, 110 deletions

diff --git a/app/app.go b/app/app.go
index 1c0d56a3c..1cec749da 100644
--- a/app/app.go
+++ b/app/app.go
@@ -25,6 +25,7 @@ import (
 	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/plugin"
+	"github.com/mattermost/mattermost-server/services/httpservice"
 	"github.com/mattermost/mattermost-server/store"
 	"github.com/mattermost/mattermost-server/store/sqlstore"
 	"github.com/mattermost/mattermost-server/utils"
@@ -101,7 +102,7 @@ type App struct {
 
 	phase2PermissionsMigrationComplete bool
 
-	HTTPService HTTPService
+	HTTPService httpservice.HTTPService
 }
 
 var appCount = 0
@@ -128,7 +129,7 @@ func New(options ...Option) (outApp *App, outErr error) {
 		licenseListeners: map[string]func(){},
 	}
 
-	app.HTTPService = MakeHTTPService(app)
+	app.HTTPService = httpservice.MakeHTTPService(app)
 
 	app.CreatePushNotificationsHub()
 	app.StartPushNotificationsHubWorkers()
diff --git a/app/apptestlib.go b/app/apptestlib.go
index c0d2cfaa2..dcc1fa941 100644
--- a/app/apptestlib.go
+++ b/app/apptestlib.go
@@ -7,7 +7,6 @@ import (
 	"io"
 	"io/ioutil"
 	"net/http"
-	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"time"
@@ -21,6 +20,7 @@ import (
 	"github.com/mattermost/mattermost-server/store/sqlstore"
 	"github.com/mattermost/mattermost-server/store/storetest"
 	"github.com/mattermost/mattermost-server/utils"
+	"github.com/mattermost/mattermost-server/utils/testutils"
 )
 
 type TestHelper struct {
@@ -36,7 +36,7 @@ type TestHelper struct {
 	tempConfigPath string
 	tempWorkspace  string
 
-	MockedHTTPService *MockedHTTPService
+	MockedHTTPService *testutils.MockedHTTPService
 }
 
 type persistentTestStore struct {
@@ -168,7 +168,7 @@ func (me *TestHelper) InitSystemAdmin() *TestHelper {
 }
 
 func (me *TestHelper) MockHTTPService(handler http.Handler) *TestHelper {
-	me.MockedHTTPService = MakeMockedHTTPService(handler)
+	me.MockedHTTPService = testutils.MakeMockedHTTPService(handler)
 	me.App.HTTPService = me.MockedHTTPService
 
 	return me
@@ -514,22 +514,3 @@ 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/http_service.go b/app/http_service.go
deleted file mode 100644
index 71e72ab2f..000000000
--- a/app/http_service.go
+++ /dev/null
@@ -1,67 +0,0 @@
-// 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/jobs/server.go b/jobs/server.go
index cffc60da1..80c48a165 100644
--- a/jobs/server.go
+++ b/jobs/server.go
@@ -7,25 +7,12 @@ import (
 	ejobs "github.com/mattermost/mattermost-server/einterfaces/jobs"
 	tjobs "github.com/mattermost/mattermost-server/jobs/interfaces"
 	"github.com/mattermost/mattermost-server/model"
+	"github.com/mattermost/mattermost-server/services/configservice"
 	"github.com/mattermost/mattermost-server/store"
 )
 
-type ConfigService interface {
-	Config() *model.Config
-	AddConfigListener(func(old, current *model.Config)) string
-	RemoveConfigListener(string)
-}
-
-type StaticConfigService struct {
-	Cfg *model.Config
-}
-
-func (s StaticConfigService) Config() *model.Config                                   { return s.Cfg }
-func (StaticConfigService) AddConfigListener(func(old, current *model.Config)) string { return "" }
-func (StaticConfigService) RemoveConfigListener(string)                               {}
-
 type JobServer struct {
-	ConfigService ConfigService
+	ConfigService configservice.ConfigService
 	Store         store.Store
 	Workers       *Workers
 	Schedulers    *Schedulers
@@ -38,7 +25,7 @@ type JobServer struct {
 	Migrations              tjobs.MigrationsJobInterface
 }
 
-func NewJobServer(configService ConfigService, store store.Store) *JobServer {
+func NewJobServer(configService configservice.ConfigService, store store.Store) *JobServer {
 	return &JobServer{
 		ConfigService: configService,
 		Store:         store,
diff --git a/jobs/workers.go b/jobs/workers.go
index 9909e035c..ad457ed2a 100644
--- a/jobs/workers.go
+++ b/jobs/workers.go
@@ -8,11 +8,12 @@ import (
 
 	"github.com/mattermost/mattermost-server/mlog"
 	"github.com/mattermost/mattermost-server/model"
+	"github.com/mattermost/mattermost-server/services/configservice"
 )
 
 type Workers struct {
 	startOnce     sync.Once
-	ConfigService ConfigService
+	ConfigService configservice.ConfigService
 	Watcher       *Watcher
 
 	DataRetention            model.Worker
diff --git a/services/configservice/configservice.go b/services/configservice/configservice.go
new file mode 100644
index 000000000..bb854d21f
--- /dev/null
+++ b/services/configservice/configservice.go
@@ -0,0 +1,15 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package configservice
+
+import (
+	"github.com/mattermost/mattermost-server/model"
+)
+
+// An interface representing something that contains a Config, such as the app.App struct
+type ConfigService interface {
+	Config() *model.Config
+	AddConfigListener(func(old, current *model.Config)) string
+	RemoveConfigListener(string)
+}
diff --git a/utils/httpclient.go b/services/httpservice/client.go
index cb68462e3..268f63b24 100644
--- a/utils/httpclient.go
+++ b/services/httpservice/client.go
@@ -1,7 +1,7 @@
 // Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved.
 // See License.txt for license information.
 
-package utils
+package httpservice
 
 import (
 	"context"
diff --git a/utils/httpclient_test.go b/services/httpservice/client_test.go
index e07c54d08..ceb133140 100644
--- a/utils/httpclient_test.go
+++ b/services/httpservice/client_test.go
@@ -1,7 +1,7 @@
 // Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved.
 // See License.txt for license information.
 
-package utils
+package httpservice
 
 import (
 	"context"
diff --git a/services/httpservice/httpservice.go b/services/httpservice/httpservice.go
new file mode 100644
index 000000000..5ed42a12d
--- /dev/null
+++ b/services/httpservice/httpservice.go
@@ -0,0 +1,67 @@
+// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package httpservice
+
+import (
+	"net"
+	"net/http"
+	"strings"
+
+	"github.com/mattermost/mattermost-server/services/configservice"
+)
+
+// 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 {
+	configService configservice.ConfigService
+}
+
+func MakeHTTPService(configService configservice.ConfigService) HTTPService {
+	return &HTTPServiceImpl{configService}
+}
+
+func (h *HTTPServiceImpl) MakeClient(trustURLs bool) *http.Client {
+	insecure := h.configService.Config().ServiceSettings.EnableInsecureOutgoingConnections != nil && *h.configService.Config().ServiceSettings.EnableInsecureOutgoingConnections
+
+	if trustURLs {
+		return NewHTTPClient(insecure, nil, nil)
+	}
+
+	allowHost := func(host string) bool {
+		if h.configService.Config().ServiceSettings.AllowedUntrustedInternalConnections == nil {
+			return false
+		}
+		for _, allowed := range strings.Fields(*h.configService.Config().ServiceSettings.AllowedUntrustedInternalConnections) {
+			if host == allowed {
+				return true
+			}
+		}
+		return false
+	}
+
+	allowIP := func(ip net.IP) bool {
+		if !IsReservedIP(ip) {
+			return true
+		}
+		if h.configService.Config().ServiceSettings.AllowedUntrustedInternalConnections == nil {
+			return false
+		}
+		for _, allowed := range strings.Fields(*h.configService.Config().ServiceSettings.AllowedUntrustedInternalConnections) {
+			if _, ipRange, err := net.ParseCIDR(allowed); err == nil && ipRange.Contains(ip) {
+				return true
+			}
+		}
+		return false
+	}
+
+	return NewHTTPClient(insecure, allowHost, allowIP)
+}
+
+func (h *HTTPServiceImpl) Close() {
+	// Does nothing, but allows this to be overridden when mocking the service
+}
diff --git a/utils/testutils/mocked_http_service.go b/utils/testutils/mocked_http_service.go
new file mode 100644
index 000000000..b1e7f6963
--- /dev/null
+++ b/utils/testutils/mocked_http_service.go
@@ -0,0 +1,28 @@
+// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package testutils
+
+import (
+	"net/http"
+	"net/http/httptest"
+)
+
+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/utils/testutils/static_config_service.go b/utils/testutils/static_config_service.go
new file mode 100644
index 000000000..44705ff6e
--- /dev/null
+++ b/utils/testutils/static_config_service.go
@@ -0,0 +1,24 @@
+// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package testutils
+
+import (
+	"github.com/mattermost/mattermost-server/model"
+)
+
+type StaticConfigService struct {
+	Cfg *model.Config
+}
+
+func (s StaticConfigService) Config() *model.Config {
+	return s.Cfg
+}
+
+func (StaticConfigService) AddConfigListener(func(old, current *model.Config)) string {
+	return ""
+}
+
+func (StaticConfigService) RemoveConfigListener(string) {
+
+}