Miscellaneous app cleanup (#7594)

* app cleanup

* whoops, forgot a file

* some minor cleanup

* longer container deadline

* defensive checks

app/admin.go

@@ -15,7 +15,6 @@ import (
 
 	l4g "github.com/alecthomas/log4go"
 	"github.com/mattermost/mattermost-server/model"
-	"github.com/mattermost/mattermost-server/store"
 	"github.com/mattermost/mattermost-server/store/sqlstore"
 	"github.com/mattermost/mattermost-server/utils"
 )
@@ -187,12 +186,13 @@ func (a *App) RecycleDatabaseConnection() {
 	oldStore := a.Srv.Store
 
 	l4g.Warn(utils.T("api.admin.recycle_db_start.warn"))
-	a.Srv.Store = store.NewLayeredStore(sqlstore.NewSqlSupplier(utils.Cfg.SqlSettings, a.Metrics), a.Metrics, a.Cluster)
-
+	a.Srv.Store = a.newStore()
 	a.Jobs.Store = a.Srv.Store
 
-	time.Sleep(20 * time.Second)
-	oldStore.Close()
+	if a.Srv.Store != oldStore {
+		time.Sleep(20 * time.Second)
+		oldStore.Close()
+	}
 
 	l4g.Warn(utils.T("api.admin.recycle_db_end.warn"))
 }

app/app.go

@@ -4,17 +4,19 @@
 package app
 
 import (
-	"io/ioutil"
 	"net/http"
 	"sync/atomic"
 
 	l4g "github.com/alecthomas/log4go"
+	"github.com/gorilla/mux"
 
 	"github.com/mattermost/mattermost-server/einterfaces"
 	ejobs "github.com/mattermost/mattermost-server/einterfaces/jobs"
 	"github.com/mattermost/mattermost-server/jobs"
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/plugin/pluginenv"
+	"github.com/mattermost/mattermost-server/store"
+	"github.com/mattermost/mattermost-server/store/sqlstore"
 	"github.com/mattermost/mattermost-server/utils"
 )
 
@@ -44,44 +46,69 @@ type App struct {
 	Metrics          einterfaces.MetricsInterface
 	Mfa              einterfaces.MfaInterface
 	Saml             einterfaces.SamlInterface
+
+	newStore func() store.Store
 }
 
 var appCount = 0
 
 // New creates a new App. You must call Shutdown when you're done with it.
 // XXX: For now, only one at a time is allowed as some resources are still shared.
-func New() *App {
+func New(options ...Option) *App {
 	appCount++
 	if appCount > 1 {
 		panic("Only one App should exist at a time. Did you forget to call Shutdown()?")
 	}
 
+	l4g.Info(utils.T("api.server.new_server.init.info"))
+
 	app := &App{
 		goroutineExitSignal: make(chan struct{}, 1),
 		Jobs:                &jobs.JobServer{},
+		Srv: &Server{
+			Router: mux.NewRouter(),
+		},
 	}
 	app.initEnterprise()
+
+	for _, option := range options {
+		option(app)
+	}
+
+	if app.newStore == nil {
+		app.newStore = func() store.Store {
+			return store.NewLayeredStore(sqlstore.NewSqlSupplier(utils.Cfg.SqlSettings, app.Metrics), app.Metrics, app.Cluster)
+		}
+	}
+
+	app.Srv.Store = app.newStore()
+	app.Jobs.Store = app.Srv.Store
+
+	app.Srv.Router.NotFoundHandler = http.HandlerFunc(app.Handle404)
+
+	app.Srv.WebSocketRouter = &WebSocketRouter{
+		app:      app,
+		handlers: make(map[string]webSocketHandler),
+	}
+
 	return app
 }
 
 func (a *App) Shutdown() {
 	appCount--
 
-	if a.Srv != nil {
-		l4g.Info(utils.T("api.server.stop_server.stopping.info"))
+	l4g.Info(utils.T("api.server.stop_server.stopping.info"))
 
-		a.Srv.GracefulServer.Stop(TIME_TO_WAIT_FOR_CONNECTIONS_TO_CLOSE_ON_SERVER_SHUTDOWN)
-		<-a.Srv.GracefulServer.StopChan()
-		a.HubStop()
+	a.StopServer()
+	a.HubStop()
 
-		a.ShutDownPlugins()
-		a.WaitForGoroutines()
+	a.ShutDownPlugins()
+	a.WaitForGoroutines()
 
-		a.Srv.Store.Close()
-		a.Srv = nil
+	a.Srv.Store.Close()
+	a.Srv = nil
 
-		l4g.Info(utils.T("api.server.stop_server.stopped.info"))
-	}
+	l4g.Info(utils.T("api.server.stop_server.stopped.info"))
 }
 
 var accountMigrationInterface func(*App) einterfaces.AccountMigrationInterface
@@ -232,9 +259,11 @@ func (a *App) WaitForGoroutines() {
 	}
 }
 
-func CloseBody(r *http.Response) {
-	if r.Body != nil {
-		ioutil.ReadAll(r.Body)
-		r.Body.Close()
-	}
+func (a *App) Handle404(w http.ResponseWriter, r *http.Request) {
+	err := model.NewAppError("Handle404", "api.context.404.app_error", nil, "", http.StatusNotFound)
+	err.Translate(utils.T)
+
+	l4g.Debug("%v: code=404 ip=%v", r.URL.Path, utils.GetIpAddress(r))
+
+	utils.RenderWebError(err, w, r)
 }

app/apptestlib.go

@@ -22,26 +22,23 @@ type TestHelper struct {
 }
 
 func setupTestHelper(enterprise bool) *TestHelper {
+	utils.TranslationsPreInit()
+	utils.LoadConfig("config.json")
+	utils.InitTranslations(utils.Cfg.LocalizationSettings)
+
 	th := &TestHelper{
 		App: New(),
 	}
 
-	if th.App.Srv == nil {
-		utils.TranslationsPreInit()
-		utils.LoadConfig("config.json")
-		utils.InitTranslations(utils.Cfg.LocalizationSettings)
-		*utils.Cfg.TeamSettings.MaxUsersPerTeam = 50
-		*utils.Cfg.RateLimitSettings.Enable = false
-		utils.DisableDebugLogForTest()
-		th.App.NewServer()
-		th.App.InitStores()
-		th.App.StartServer()
-		utils.InitHTML()
-		utils.EnableDebugLogForTest()
-		th.App.Srv.Store.MarkSystemRanUnitTests()
+	*utils.Cfg.TeamSettings.MaxUsersPerTeam = 50
+	*utils.Cfg.RateLimitSettings.Enable = false
+	utils.DisableDebugLogForTest()
+	th.App.StartServer()
+	utils.InitHTML()
+	utils.EnableDebugLogForTest()
+	th.App.Srv.Store.MarkSystemRanUnitTests()
 
-		*utils.Cfg.TeamSettings.EnableOpenServer = true
-	}
+	*utils.Cfg.TeamSettings.EnableOpenServer = true
 
 	utils.SetIsLicensed(enterprise)
 	if enterprise {

app/oauth.go

@@ -7,7 +7,6 @@ import (
 	"bytes"
 	b64 "encoding/base64"
 	"io"
-	"io/ioutil"
 	"net/http"
 	"net/url"
 	"strings"
@@ -428,10 +427,7 @@ func (a *App) RevokeAccessToken(token string) *model.AppError {
 }
 
 func (a *App) CompleteOAuth(service string, body io.ReadCloser, teamId string, props map[string]string) (*model.User, *model.AppError) {
-	defer func() {
-		ioutil.ReadAll(body)
-		body.Close()
-	}()
+	defer body.Close()
 
 	action := props["action"]
 
@@ -688,11 +684,9 @@ func (a *App) AuthorizeOAuthUser(w http.ResponseWriter, r *http.Request, service
 	if resp, err := utils.HttpClient(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)
-		resp.Body = ioutil.NopCloser(bytes.NewBuffer(bodyBytes))
-
 		ar = model.AccessResponseFromJson(resp.Body)
-		defer CloseBody(resp)
+		resp.Body.Close()
+
 		if ar == nil {
 			return nil, "", stateProps, model.NewAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.bad_response.app_error", nil, "response_body="+string(bodyBytes), http.StatusInternalServerError)
 		}

app/options.go

@@ -0,0 +1,29 @@
+// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package app
+
+import (
+	"github.com/mattermost/mattermost-server/store"
+)
+
+type Option func(a *App)
+
+// By default, the app will use the store specified by the configuration. This allows you to
+// construct an app with a different store.
+//
+// The storeOrFactory parameter must be either a store.Store or func() store.Store.
+func StoreOverride(storeOrFactory interface{}) Option {
+	return func(a *App) {
+		switch s := storeOrFactory.(type) {
+		case store.Store:
+			a.newStore = func() store.Store {
+				return s
+			}
+		case func() store.Store:
+			a.newStore = s
+		default:
+			panic("invalid StoreOverride")
+		}
+	}
+}

app/post.go

@@ -679,7 +679,7 @@ func GetOpenGraphMetadata(url string) *opengraph.OpenGraph {
 		l4g.Error("GetOpenGraphMetadata request failed for url=%v with err=%v", url, err.Error())
 		return og
 	}
-	defer CloseBody(res)
+	defer res.Body.Close()
 
 	if err := og.ProcessHTML(res.Body); err != nil {
 		l4g.Error("GetOpenGraphMetadata processing failed for url=%v with err=%v", url, err.Error())

app/server.go

@@ -20,7 +20,6 @@ import (
 
 	"github.com/mattermost/mattermost-server/model"
 	"github.com/mattermost/mattermost-server/store"
-	"github.com/mattermost/mattermost-server/store/sqlstore"
 	"github.com/mattermost/mattermost-server/utils"
 )
 
@@ -78,16 +77,6 @@ func (cw *CorsWrapper) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 
 const TIME_TO_WAIT_FOR_CONNECTIONS_TO_CLOSE_ON_SERVER_SHUTDOWN = time.Second
 
-func (a *App) NewServer() {
-	l4g.Info(utils.T("api.server.new_server.init.info"))
-
-	a.Srv = &Server{}
-}
-
-func (a *App) InitStores() {
-	a.Srv.Store = store.NewLayeredStore(sqlstore.NewSqlSupplier(utils.Cfg.SqlSettings, a.Metrics), a.Metrics, a.Cluster)
-}
-
 type VaryBy struct{}
 
 func (m *VaryBy) Key(r *http.Request) string {
@@ -211,3 +200,11 @@ func (a *App) StartServer() {
 		}
 	}()
 }
+
+func (a *App) StopServer() {
+	if a.Srv.GracefulServer != nil {
+		a.Srv.GracefulServer.Stop(TIME_TO_WAIT_FOR_CONNECTIONS_TO_CLOSE_ON_SERVER_SHUTDOWN)
+		<-a.Srv.GracefulServer.StopChan()
+		a.Srv.GracefulServer = nil
+	}
+}

app/webhook.go

@@ -109,7 +109,7 @@ func (a *App) TriggerWebhook(payload *model.OutgoingWebhookPayload, hook *model.
 				if resp, err := utils.HttpClient(false).Do(req); err != nil {
 					l4g.Error(utils.T("api.post.handle_webhook_events_and_forget.event_post.error"), err.Error())
 				} else {
-					defer CloseBody(resp)
+					defer resp.Body.Close()
 					webhookResp := model.OutgoingWebhookResponseFromJson(resp.Body)
 
 					if webhookResp != nil && webhookResp.Text != nil {

app/webrtc.go

@@ -62,7 +62,7 @@ func GetWebrtcToken(sessionId string) (string, *model.AppError) {
 	if rp, err := utils.HttpClient(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 CloseBody(rp)
+		defer rp.Body.Close()
 		return "", model.AppErrorFromJson(rp.Body)
 	} else {
 		janusResponse := model.GatewayResponseFromJson(rp.Body)

app/websocket_router.go

@@ -21,13 +21,6 @@ type WebSocketRouter struct {
 	handlers map[string]webSocketHandler
 }
 
-func (a *App) NewWebSocketRouter() *WebSocketRouter {
-	return &WebSocketRouter{
-		app:      a,
-		handlers: make(map[string]webSocketHandler),
-	}
-}
-
 func (wr *WebSocketRouter) Handle(action string, handler webSocketHandler) {
 	wr.handlers[action] = handler
 }