From c27814393d60facacf88e9e98ca40474bdd3c426 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Fri, 15 Oct 2021 19:57:05 +0530 Subject: [PATCH] Improve API4 initialization (#18680) * Improve API4 initialization - Refactored openGraphDataCache to be inside app layer. - Moved the cache instance from global variable to be inside server. - Moved out the app instantiation from the global commands package to be instantiated on every call. Only the server instance is passed. - Moved InitLocal to be called from inside Init. ```release-note NONE ``` * Remove commented line ```release-note NONE ``` --- api4/api.go | 27 +++++++++++---------- api4/apitestlib.go | 5 ++-- api4/handlers.go | 36 ++++++++++++++-------------- api4/handlers_test.go | 2 +- api4/openGraph.go | 36 +++++----------------------- api4/post_test.go | 2 +- api4/system_test.go | 2 +- app/app_iface.go | 3 +-- app/opengraph.go | 31 ++++++++++++++++++++---- app/opentracing/opentracing_layer.go | 12 ++++++---- app/server.go | 16 +++++++++++++ cmd/mattermost/commands/server.go | 15 +++++------- cmd/mattermost/commands/test.go | 4 ++-- web/handlers.go | 24 ++++++++++--------- web/handlers_test.go | 26 ++++++++++---------- web/static.go | 10 ++++---- web/web.go | 8 +++---- web/web_test.go | 2 +- 18 files changed, 139 insertions(+), 122 deletions(-) diff --git a/api4/api.go b/api4/api.go index 225e2b8c42..4010879aed 100644 --- a/api4/api.go +++ b/api4/api.go @@ -135,18 +135,18 @@ type Routes struct { } type API struct { - app app.AppIface + srv *app.Server BaseRoutes *Routes } -func Init(a app.AppIface, root *mux.Router) *API { +func Init(srv *app.Server) *API { api := &API{ - app: a, + srv: srv, BaseRoutes: &Routes{}, } - api.BaseRoutes.Root = root - api.BaseRoutes.APIRoot = root.PathPrefix(model.APIURLSuffix).Subrouter() + api.BaseRoutes.Root = srv.Router + api.BaseRoutes.APIRoot = srv.Router.PathPrefix(model.APIURLSuffix).Subrouter() api.BaseRoutes.Users = api.BaseRoutes.APIRoot.PathPrefix("/users").Subrouter() api.BaseRoutes.User = api.BaseRoutes.APIRoot.PathPrefix("/users/{user_id:[A-Za-z0-9]+}").Subrouter() @@ -293,19 +293,21 @@ func Init(a app.AppIface, root *mux.Router) *API { api.InitPermissions() api.InitExport() - root.Handle("/api/v4/{anything:.*}", http.HandlerFunc(api.Handle404)) + srv.Router.Handle("/api/v4/{anything:.*}", http.HandlerFunc(api.Handle404)) + + InitLocal(srv) return api } -func InitLocal(a app.AppIface, root *mux.Router) *API { +func InitLocal(srv *app.Server) *API { api := &API{ - app: a, + srv: srv, BaseRoutes: &Routes{}, } - api.BaseRoutes.Root = root - api.BaseRoutes.APIRoot = root.PathPrefix(model.APIURLSuffix).Subrouter() + api.BaseRoutes.Root = srv.LocalRouter + api.BaseRoutes.APIRoot = srv.LocalRouter.PathPrefix(model.APIURLSuffix).Subrouter() api.BaseRoutes.Users = api.BaseRoutes.APIRoot.PathPrefix("/users").Subrouter() api.BaseRoutes.User = api.BaseRoutes.Users.PathPrefix("/{user_id:[A-Za-z0-9]+}").Subrouter() @@ -386,13 +388,14 @@ func InitLocal(a app.AppIface, root *mux.Router) *API { api.InitJobLocal() api.InitSamlLocal() - root.Handle("/api/v4/{anything:.*}", http.HandlerFunc(api.Handle404)) + srv.LocalRouter.Handle("/api/v4/{anything:.*}", http.HandlerFunc(api.Handle404)) return api } func (api *API) Handle404(w http.ResponseWriter, r *http.Request) { - web.Handle404(api.app, w, r) + app := app.New(app.ServerConnector(api.srv.Channels())) + web.Handle404(app, w, r) } var ReturnStatusOK = web.ReturnStatusOK diff --git a/api4/apitestlib.go b/api4/apitestlib.go index 32bf4c09b6..4ef51b647c 100644 --- a/api4/apitestlib.go +++ b/api4/apitestlib.go @@ -177,9 +177,8 @@ func setupTestHelper(dbStore store.Store, searchEngine *searchengine.Broker, ent panic(err) } - Init(th.App, th.App.Srv().Router) - InitLocal(th.App, th.App.Srv().LocalRouter) - web.New(th.App, th.App.Srv().Router) + Init(th.App.Srv()) + web.New(th.App.Srv()) wsapi.Init(th.App.Srv()) if enterprise { diff --git a/api4/handlers.go b/api4/handlers.go index 5feba026e4..d35a2e3434 100644 --- a/api4/handlers.go +++ b/api4/handlers.go @@ -17,7 +17,7 @@ type Context = web.Context // granted. func (api *API) APIHandler(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { handler := &web.Handler{ - App: api.app, + Srv: api.srv, HandleFunc: h, HandlerName: web.GetHandlerName(h), RequireSession: false, @@ -26,7 +26,7 @@ func (api *API) APIHandler(h func(*Context, http.ResponseWriter, *http.Request)) IsStatic: false, IsLocal: false, } - if *api.app.Config().ServiceSettings.WebserverMode == "gzip" { + if *api.srv.Config().ServiceSettings.WebserverMode == "gzip" { return gziphandler.GzipHandler(handler) } return handler @@ -36,7 +36,7 @@ func (api *API) APIHandler(h func(*Context, http.ResponseWriter, *http.Request)) // be granted. func (api *API) APISessionRequired(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { handler := &web.Handler{ - App: api.app, + Srv: api.srv, HandleFunc: h, HandlerName: web.GetHandlerName(h), RequireSession: true, @@ -45,7 +45,7 @@ func (api *API) APISessionRequired(h func(*Context, http.ResponseWriter, *http.R IsStatic: false, IsLocal: false, } - if *api.app.Config().ServiceSettings.WebserverMode == "gzip" { + if *api.srv.Config().ServiceSettings.WebserverMode == "gzip" { return gziphandler.GzipHandler(handler) } return handler @@ -55,7 +55,7 @@ func (api *API) APISessionRequired(h func(*Context, http.ResponseWriter, *http.R // CloudAPIKeyRequired provides a handler for webhook endpoints to access Cloud installations from CWS func (api *API) CloudAPIKeyRequired(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { handler := &web.Handler{ - App: api.app, + Srv: api.srv, HandleFunc: h, HandlerName: web.GetHandlerName(h), RequireSession: false, @@ -65,7 +65,7 @@ func (api *API) CloudAPIKeyRequired(h func(*Context, http.ResponseWriter, *http. IsStatic: false, IsLocal: false, } - if *api.app.Config().ServiceSettings.WebserverMode == "gzip" { + if *api.srv.Config().ServiceSettings.WebserverMode == "gzip" { return gziphandler.GzipHandler(handler) } return handler @@ -75,7 +75,7 @@ func (api *API) CloudAPIKeyRequired(h func(*Context, http.ResponseWriter, *http. // RemoteClusterTokenRequired provides a handler for remote cluster requests to /remotecluster endpoints. func (api *API) RemoteClusterTokenRequired(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { handler := &web.Handler{ - App: api.app, + Srv: api.srv, HandleFunc: h, HandlerName: web.GetHandlerName(h), RequireSession: false, @@ -86,7 +86,7 @@ func (api *API) RemoteClusterTokenRequired(h func(*Context, http.ResponseWriter, IsStatic: false, IsLocal: false, } - if *api.app.Config().ServiceSettings.WebserverMode == "gzip" { + if *api.srv.Config().ServiceSettings.WebserverMode == "gzip" { return gziphandler.GzipHandler(handler) } return handler @@ -97,7 +97,7 @@ func (api *API) RemoteClusterTokenRequired(h func(*Context, http.ResponseWriter, // authentication must be waived. func (api *API) APISessionRequiredMfa(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { handler := &web.Handler{ - App: api.app, + Srv: api.srv, HandleFunc: h, HandlerName: web.GetHandlerName(h), RequireSession: true, @@ -106,7 +106,7 @@ func (api *API) APISessionRequiredMfa(h func(*Context, http.ResponseWriter, *htt IsStatic: false, IsLocal: false, } - if *api.app.Config().ServiceSettings.WebserverMode == "gzip" { + if *api.srv.Config().ServiceSettings.WebserverMode == "gzip" { return gziphandler.GzipHandler(handler) } return handler @@ -118,7 +118,7 @@ func (api *API) APISessionRequiredMfa(h func(*Context, http.ResponseWriter, *htt // websocket. func (api *API) APIHandlerTrustRequester(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { handler := &web.Handler{ - App: api.app, + Srv: api.srv, HandleFunc: h, HandlerName: web.GetHandlerName(h), RequireSession: false, @@ -127,7 +127,7 @@ func (api *API) APIHandlerTrustRequester(h func(*Context, http.ResponseWriter, * IsStatic: false, IsLocal: false, } - if *api.app.Config().ServiceSettings.WebserverMode == "gzip" { + if *api.srv.Config().ServiceSettings.WebserverMode == "gzip" { return gziphandler.GzipHandler(handler) } return handler @@ -138,7 +138,7 @@ func (api *API) APIHandlerTrustRequester(h func(*Context, http.ResponseWriter, * // are allowed to be requested directly rather than via javascript/XMLHttpRequest, such as emoji or file uploads. func (api *API) APISessionRequiredTrustRequester(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { handler := &web.Handler{ - App: api.app, + Srv: api.srv, HandleFunc: h, HandlerName: web.GetHandlerName(h), RequireSession: true, @@ -147,7 +147,7 @@ func (api *API) APISessionRequiredTrustRequester(h func(*Context, http.ResponseW IsStatic: false, IsLocal: false, } - if *api.app.Config().ServiceSettings.WebserverMode == "gzip" { + if *api.srv.Config().ServiceSettings.WebserverMode == "gzip" { return gziphandler.GzipHandler(handler) } return handler @@ -158,7 +158,7 @@ func (api *API) APISessionRequiredTrustRequester(h func(*Context, http.ResponseW // responding with HTTP 503 (Service Unavailable). func (api *API) APISessionRequiredDisableWhenBusy(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { handler := &web.Handler{ - App: api.app, + Srv: api.srv, HandleFunc: h, HandlerName: web.GetHandlerName(h), RequireSession: true, @@ -168,7 +168,7 @@ func (api *API) APISessionRequiredDisableWhenBusy(h func(*Context, http.Response IsLocal: false, DisableWhenBusy: true, } - if *api.app.Config().ServiceSettings.WebserverMode == "gzip" { + if *api.srv.Config().ServiceSettings.WebserverMode == "gzip" { return gziphandler.GzipHandler(handler) } return handler @@ -181,7 +181,7 @@ func (api *API) APISessionRequiredDisableWhenBusy(h func(*Context, http.Response // restrictions func (api *API) APILocal(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { handler := &web.Handler{ - App: api.app, + Srv: api.srv, HandleFunc: h, HandlerName: web.GetHandlerName(h), RequireSession: false, @@ -191,7 +191,7 @@ func (api *API) APILocal(h func(*Context, http.ResponseWriter, *http.Request)) h IsLocal: true, } - if *api.app.Config().ServiceSettings.WebserverMode == "gzip" { + if *api.srv.Config().ServiceSettings.WebserverMode == "gzip" { return gziphandler.GzipHandler(handler) } return handler diff --git a/api4/handlers_test.go b/api4/handlers_test.go index 14147ccd7e..f607dab2fd 100644 --- a/api4/handlers_test.go +++ b/api4/handlers_test.go @@ -68,7 +68,7 @@ func TestAPIHandlersWithGzip(t *testing.T) { th := Setup(t) defer th.TearDown() - api := Init(th.App, th.Server.Router) + api := Init(th.Server) session, _ := th.App.GetSession(th.Client.AuthToken) t.Run("with WebserverMode == \"gzip\"", func(t *testing.T) { diff --git a/api4/openGraph.go b/api4/openGraph.go index 8dc201cf4b..22f6dd3f6f 100644 --- a/api4/openGraph.go +++ b/api4/openGraph.go @@ -5,30 +5,13 @@ package api4 import ( "net/http" - "time" "github.com/mattermost/mattermost-server/v6/model" - "github.com/mattermost/mattermost-server/v6/services/cache" + "github.com/mattermost/mattermost-server/v6/shared/mlog" ) -const OpenGraphMetadataCacheSize = 10000 - -var openGraphDataCache = cache.NewLRU(cache.LRUOptions{ - Size: OpenGraphMetadataCacheSize, -}) - func (api *API) InitOpenGraph() { api.BaseRoutes.OpenGraph.Handle("", api.APISessionRequired(getOpenGraphMetadata)).Methods("POST") - - // Dump the image cache if the proxy settings have changed. (need switch URLs to the correct proxy) - api.app.AddConfigListener(func(before, after *model.Config) { - if (before.ImageProxySettings.Enable != after.ImageProxySettings.Enable) || - (before.ImageProxySettings.ImageProxyType != after.ImageProxySettings.ImageProxyType) || - (before.ImageProxySettings.RemoteImageProxyURL != after.ImageProxySettings.RemoteImageProxyURL) || - (before.ImageProxySettings.RemoteImageProxyOptions != after.ImageProxySettings.RemoteImageProxyOptions) { - openGraphDataCache.Purge() - } - }) } func getOpenGraphMetadata(c *Context, w http.ResponseWriter, r *http.Request) { @@ -46,20 +29,13 @@ func getOpenGraphMetadata(c *Context, w http.ResponseWriter, r *http.Request) { return } - var ogJSONGeneric []byte - err := openGraphDataCache.Get(url, &ogJSONGeneric) - if err == nil { - w.Write(ogJSONGeneric) - return - } - - og := c.App.GetOpenGraphMetadata(url) - ogJSON, err := og.ToJSON() - openGraphDataCache.SetWithExpiry(url, ogJSON, 1*time.Hour) + buf, err := c.App.GetOpenGraphMetadata(url) if err != nil { + mlog.Warn("GetOpenGraphMetadata request failed", + mlog.String("requestURL", url), + mlog.Err(err)) w.Write([]byte(`{"url": ""}`)) return } - - w.Write(ogJSON) + w.Write(buf) } diff --git a/api4/post_test.go b/api4/post_test.go index f87a0ab733..ef9d7c6b7c 100644 --- a/api4/post_test.go +++ b/api4/post_test.go @@ -601,7 +601,7 @@ func TestCreatePostCheckOnlineStatus(t *testing.T) { th := Setup(t).InitBasic() defer th.TearDown() - api := Init(th.App, th.Server.Router) + api := Init(th.Server) session, _ := th.App.GetSession(th.Client.AuthToken) cli := th.CreateClient() diff --git a/api4/system_test.go b/api4/system_test.go index 1cb14165bb..0e3adfe4f0 100644 --- a/api4/system_test.go +++ b/api4/system_test.go @@ -748,7 +748,7 @@ func TestServerBusy503(t *testing.T) { func TestPushNotificationAck(t *testing.T) { th := Setup(t).InitBasic() - api := Init(th.App, th.Server.Router) + api := Init(th.Server) session, _ := th.App.GetSession(th.Client.AuthToken) defer th.TearDown() diff --git a/app/app_iface.go b/app/app_iface.go index 562a58600b..52f9ee703c 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -18,7 +18,6 @@ import ( "reflect" "time" - "github.com/dyatlov/go-opengraph/opengraph" "github.com/mattermost/mattermost-server/v6/app/request" "github.com/mattermost/mattermost-server/v6/audit" "github.com/mattermost/mattermost-server/v6/einterfaces" @@ -654,7 +653,7 @@ type AppIface interface { GetOAuthLoginEndpoint(w http.ResponseWriter, r *http.Request, service, teamID, action, redirectTo, loginHint string, isMobile bool) (string, *model.AppError) GetOAuthSignupEndpoint(w http.ResponseWriter, r *http.Request, service, teamID string) (string, *model.AppError) GetOAuthStateToken(token string) (*model.Token, *model.AppError) - GetOpenGraphMetadata(requestURL string) *opengraph.OpenGraph + GetOpenGraphMetadata(requestURL string) ([]byte, error) GetOrCreateDirectChannel(c *request.Context, userID, otherUserID string, channelOptions ...model.ChannelOption) (*model.Channel, *model.AppError) GetOutgoingWebhook(hookID string) (*model.OutgoingWebhook, *model.AppError) GetOutgoingWebhooksForChannelPageByUser(channelID string, userID string, page, perPage int) ([]*model.OutgoingWebhook, *model.AppError) diff --git a/app/opengraph.go b/app/opengraph.go index dfa5b03e1f..7ea39fe0b9 100644 --- a/app/opengraph.go +++ b/app/opengraph.go @@ -7,6 +7,7 @@ import ( "html" "io" "net/url" + "time" "github.com/dyatlov/go-opengraph/opengraph" "golang.org/x/net/html/charset" @@ -14,16 +15,36 @@ import ( "github.com/mattermost/mattermost-server/v6/shared/mlog" ) -const MaxOpenGraphResponseSize = 1024 * 1024 * 50 +const ( + MaxOpenGraphResponseSize = 1024 * 1024 * 50 + openGraphMetadataCacheSize = 10000 +) + +func (a *App) GetOpenGraphMetadata(requestURL string) ([]byte, error) { + var ogJSONGeneric []byte + err := a.Srv().openGraphDataCache.Get(requestURL, &ogJSONGeneric) + if err == nil { + return ogJSONGeneric, nil + } -func (a *App) GetOpenGraphMetadata(requestURL string) *opengraph.OpenGraph { res, err := a.HTTPService().MakeClient(false).Get(requestURL) if err != nil { - mlog.Debug("GetOpenGraphMetadata request failed", mlog.String("requestURL", requestURL), mlog.Err(err)) - return nil + return nil, err } defer res.Body.Close() - return a.parseOpenGraphMetadata(requestURL, res.Body, res.Header.Get("Content-Type")) + + graph := a.parseOpenGraphMetadata(requestURL, res.Body, res.Header.Get("Content-Type")) + + ogJSON, err := graph.ToJSON() + if err != nil { + return nil, err + } + err = a.Srv().openGraphDataCache.SetWithExpiry(requestURL, ogJSON, 1*time.Hour) + if err != nil { + return nil, err + } + + return ogJSON, nil } func (a *App) parseOpenGraphMetadata(requestURL string, body io.Reader, contentType string) *opengraph.OpenGraph { diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index 7b727f5859..d3f6521982 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -18,7 +18,6 @@ import ( "reflect" "time" - "github.com/dyatlov/go-opengraph/opengraph" "github.com/mattermost/mattermost-server/v6/app" "github.com/mattermost/mattermost-server/v6/app/request" "github.com/mattermost/mattermost-server/v6/audit" @@ -6899,7 +6898,7 @@ func (a *OpenTracingAppLayer) GetOAuthStateToken(token string) (*model.Token, *m return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) GetOpenGraphMetadata(requestURL string) *opengraph.OpenGraph { +func (a *OpenTracingAppLayer) GetOpenGraphMetadata(requestURL string) ([]byte, error) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetOpenGraphMetadata") @@ -6911,9 +6910,14 @@ func (a *OpenTracingAppLayer) GetOpenGraphMetadata(requestURL string) *opengraph }() defer span.Finish() - resultVar0 := a.app.GetOpenGraphMetadata(requestURL) + resultVar0, resultVar1 := a.app.GetOpenGraphMetadata(requestURL) - return resultVar0 + if resultVar1 != nil { + span.LogFields(spanlog.Error(resultVar1)) + ext.Error.Set(span, true) + } + + return resultVar0, resultVar1 } func (a *OpenTracingAppLayer) GetOrCreateDirectChannel(c *request.Context, userID string, otherUserID string, channelOptions ...model.ChannelOption) (*model.Channel, *model.AppError) { diff --git a/app/server.go b/app/server.go index a98668276f..6c65e0c367 100644 --- a/app/server.go +++ b/app/server.go @@ -137,6 +137,7 @@ type Server struct { htmlTemplateWatcher *templates.Container seenPendingPostIdsCache cache.Cache statusCache cache.Cache + openGraphDataCache cache.Cache configListenerId string licenseListenerId string clusterLeaderListenerId string @@ -349,6 +350,11 @@ func NewServer(options ...Option) (*Server, error) { }); err != nil { return nil, errors.Wrap(err, "Unable to create status cache") } + if s.openGraphDataCache, err = s.CacheProvider.NewCache(&cache.CacheOptions{ + Size: openGraphMetadataCacheSize, + }); err != nil { + return nil, errors.Wrap(err, "Unable to create opengraphdata cache") + } s.createPushNotificationsHub() @@ -708,6 +714,16 @@ func NewServer(options ...Option) (*Server, error) { } }) + // Dump the image cache if the proxy settings have changed. (need switch URLs to the correct proxy) + s.AddConfigListener(func(oldCfg, newCfg *model.Config) { + if (oldCfg.ImageProxySettings.Enable != newCfg.ImageProxySettings.Enable) || + (oldCfg.ImageProxySettings.ImageProxyType != newCfg.ImageProxySettings.ImageProxyType) || + (oldCfg.ImageProxySettings.RemoteImageProxyURL != newCfg.ImageProxySettings.RemoteImageProxyURL) || + (oldCfg.ImageProxySettings.RemoteImageProxyOptions != newCfg.ImageProxySettings.RemoteImageProxyOptions) { + s.openGraphDataCache.Purge() + } + }) + return s, nil } diff --git a/cmd/mattermost/commands/server.go b/cmd/mattermost/commands/server.go index 94183fda5c..b24eae8520 100644 --- a/cmd/mattermost/commands/server.go +++ b/cmd/mattermost/commands/server.go @@ -93,17 +93,14 @@ func runServer(configStore *config.Store, interruptChan chan os.Signal) error { } }() - a := app.New(app.ServerConnector(server.Channels())) - api := api4.Init(a, server.Router) - + api := api4.Init(server) wsapi.Init(server) - web.New(a, server.Router) - api4.InitLocal(a, server.LocalRouter) + web.New(server) - serverErr := server.Start() - if serverErr != nil { - mlog.Critical(serverErr.Error()) - return serverErr + err = server.Start() + if err != nil { + mlog.Critical(err.Error()) + return err } // If we allow testing then listen for manual testing URL hits diff --git a/cmd/mattermost/commands/test.go b/cmd/mattermost/commands/test.go index 5a3f87d28e..86225ee44a 100644 --- a/cmd/mattermost/commands/test.go +++ b/cmd/mattermost/commands/test.go @@ -58,7 +58,7 @@ func webClientTestsCmdF(command *cobra.Command, args []string) error { return serverErr } - api4.Init(a, a.Srv().Router) + api4.Init(a.Srv()) wsapi.Init(a.Srv()) a.UpdateConfig(setupClientTests) runWebClientTests() @@ -79,7 +79,7 @@ func serverForWebClientTestsCmdF(command *cobra.Command, args []string) error { return serverErr } - api4.Init(a, a.Srv().Router) + api4.Init(a.Srv()) wsapi.Init(a.Srv()) a.UpdateConfig(setupClientTests) diff --git a/web/handlers.go b/web/handlers.go index 3a21df8099..173dd5fe2c 100644 --- a/web/handlers.go +++ b/web/handlers.go @@ -41,7 +41,7 @@ func GetHandlerName(h func(*Context, http.ResponseWriter, *http.Request)) string func (w *Web) NewHandler(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { return &Handler{ - App: w.app, + Srv: w.srv, HandleFunc: h, HandlerName: GetHandlerName(h), RequireSession: false, @@ -55,10 +55,10 @@ func (w *Web) NewHandler(h func(*Context, http.ResponseWriter, *http.Request)) h func (w *Web) NewStaticHandler(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { // Determine the CSP SHA directive needed for subpath support, if any. This value is fixed // on server start and intentionally requires a restart to take effect. - subpath, _ := utils.GetSubpathFromConfig(w.app.Config()) + subpath, _ := utils.GetSubpathFromConfig(w.srv.Config()) return &Handler{ - App: w.app, + Srv: w.srv, HandleFunc: h, HandlerName: GetHandlerName(h), RequireSession: false, @@ -71,7 +71,7 @@ func (w *Web) NewStaticHandler(h func(*Context, http.ResponseWriter, *http.Reque } type Handler struct { - App app.AppIface + Srv *app.Server HandleFunc func(*Context, http.ResponseWriter, *http.Request) HandlerName string RequireSession bool @@ -90,6 +90,8 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { w = newWrappedWriter(w) now := time.Now() + appInstance := app.New(app.ServerConnector(h.Srv.Channels())) + requestID := model.NewId() var statusCode string defer func() { @@ -107,7 +109,7 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { c := &Context{ AppContext: &request.Context{}, - App: h.App, + App: appInstance, } t, _ := i18n.GetTranslationsAndLocaleFromRequest(r) @@ -394,7 +396,7 @@ func (h *Handler) checkCSRFToken(c *Context, r *http.Request, token string, toke // granted. func (w *Web) APIHandler(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { handler := &Handler{ - App: w.app, + Srv: w.srv, HandleFunc: h, HandlerName: GetHandlerName(h), RequireSession: false, @@ -403,7 +405,7 @@ func (w *Web) APIHandler(h func(*Context, http.ResponseWriter, *http.Request)) h IsStatic: false, IsLocal: false, } - if *w.app.Config().ServiceSettings.WebserverMode == "gzip" { + if *w.srv.Config().ServiceSettings.WebserverMode == "gzip" { return gziphandler.GzipHandler(handler) } return handler @@ -414,7 +416,7 @@ func (w *Web) APIHandler(h func(*Context, http.ResponseWriter, *http.Request)) h // websocket. func (w *Web) APIHandlerTrustRequester(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { handler := &Handler{ - App: w.app, + Srv: w.srv, HandleFunc: h, HandlerName: GetHandlerName(h), RequireSession: false, @@ -423,7 +425,7 @@ func (w *Web) APIHandlerTrustRequester(h func(*Context, http.ResponseWriter, *ht IsStatic: false, IsLocal: false, } - if *w.app.Config().ServiceSettings.WebserverMode == "gzip" { + if *w.srv.Config().ServiceSettings.WebserverMode == "gzip" { return gziphandler.GzipHandler(handler) } return handler @@ -433,7 +435,7 @@ func (w *Web) APIHandlerTrustRequester(h func(*Context, http.ResponseWriter, *ht // be granted. func (w *Web) APISessionRequired(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler { handler := &Handler{ - App: w.app, + Srv: w.srv, HandleFunc: h, HandlerName: GetHandlerName(h), RequireSession: true, @@ -442,7 +444,7 @@ func (w *Web) APISessionRequired(h func(*Context, http.ResponseWriter, *http.Req IsStatic: false, IsLocal: false, } - if *w.app.Config().ServiceSettings.WebserverMode == "gzip" { + if *w.srv.Config().ServiceSettings.WebserverMode == "gzip" { return gziphandler.GzipHandler(handler) } return handler diff --git a/web/handlers_test.go b/web/handlers_test.go index fbd3fbe137..92589d1f08 100644 --- a/web/handlers_test.go +++ b/web/handlers_test.go @@ -25,7 +25,7 @@ func TestHandlerServeHTTPErrors(t *testing.T) { th := SetupWithStoreMock(t) defer th.TearDown() - web := New(th.App, th.Server.Router) + web := New(th.Server) handler := web.NewHandler(handlerForHTTPErrors) var flagtests = []struct { @@ -84,7 +84,7 @@ func TestHandlerServeHTTPSecureTransport(t *testing.T) { *config.ServiceSettings.TLSStrictTransportMaxAge = 6000 }) - web := New(th.App, th.Server.Router) + web := New(th.Server) handler := web.NewHandler(handlerForHTTPSecureTransport) request := httptest.NewRequest("GET", "/api/v4/test", nil) @@ -136,10 +136,10 @@ func TestHandlerServeCSRFToken(t *testing.T) { t.Errorf("Expected nil, got %s", err) } - web := New(th.App, th.Server.Router) + web := New(th.Server) handler := Handler{ - App: web.app, + Srv: web.srv, HandleFunc: handlerForCSRFToken, RequireSession: true, TrustRequester: false, @@ -219,7 +219,7 @@ func TestHandlerServeCSRFToken(t *testing.T) { // Handler with RequireSession set to false handlerNoSession := Handler{ - App: th.App, + Srv: th.Server, HandleFunc: handlerForCSRFToken, RequireSession: false, TrustRequester: false, @@ -263,10 +263,10 @@ func TestHandlerServeCSPHeader(t *testing.T) { th := SetupWithStoreMock(t) defer th.TearDown() - web := New(th.App, th.Server.Router) + web := New(th.Server) handler := Handler{ - App: web.app, + Srv: web.srv, HandleFunc: handlerForCSPHeader, RequireSession: false, TrustRequester: false, @@ -285,10 +285,10 @@ func TestHandlerServeCSPHeader(t *testing.T) { th := SetupWithStoreMock(t) defer th.TearDown() - web := New(th.App, th.Server.Router) + web := New(th.Server) handler := Handler{ - App: web.app, + Srv: web.srv, HandleFunc: handlerForCSPHeader, RequireSession: false, TrustRequester: false, @@ -325,10 +325,10 @@ func TestHandlerServeCSPHeader(t *testing.T) { *cfg.ServiceSettings.SiteURL = *cfg.ServiceSettings.SiteURL + "/subpath" }) - web := New(th.App, th.Server.Router) + web := New(th.Server) handler := Handler{ - App: web.app, + Srv: web.srv, HandleFunc: handlerForCSPHeader, RequireSession: false, TrustRequester: false, @@ -380,10 +380,10 @@ func TestHandlerServeInvalidToken(t *testing.T) { *cfg.ServiceSettings.SiteURL = tc.SiteURL }) - web := New(th.App, th.Server.Router) + web := New(th.Server) handler := Handler{ - App: web.app, + Srv: web.srv, HandleFunc: handlerForCSRFToken, RequireSession: true, TrustRequester: false, diff --git a/web/static.go b/web/static.go index fc5cd3b2e3..d354962215 100644 --- a/web/static.go +++ b/web/static.go @@ -21,20 +21,20 @@ import ( var robotsTxt = []byte("User-agent: *\nDisallow: /\n") func (w *Web) InitStatic() { - if *w.app.Config().ServiceSettings.WebserverMode != "disabled" { - if err := utils.UpdateAssetsSubpathFromConfig(w.app.Config()); err != nil { + if *w.srv.Config().ServiceSettings.WebserverMode != "disabled" { + if err := utils.UpdateAssetsSubpathFromConfig(w.srv.Config()); err != nil { mlog.Error("Failed to update assets subpath from config", mlog.Err(err)) } staticDir, _ := fileutils.FindDir(model.ClientDir) mlog.Debug("Using client directory", mlog.String("clientDir", staticDir)) - subpath, _ := utils.GetSubpathFromConfig(w.app.Config()) + subpath, _ := utils.GetSubpathFromConfig(w.srv.Config()) staticHandler := staticFilesHandler(http.StripPrefix(path.Join(subpath, "static"), http.FileServer(http.Dir(staticDir)))) - pluginHandler := staticFilesHandler(http.StripPrefix(path.Join(subpath, "static", "plugins"), http.FileServer(http.Dir(*w.app.Config().PluginSettings.ClientDirectory)))) + pluginHandler := staticFilesHandler(http.StripPrefix(path.Join(subpath, "static", "plugins"), http.FileServer(http.Dir(*w.srv.Config().PluginSettings.ClientDirectory)))) - if *w.app.Config().ServiceSettings.WebserverMode == "gzip" { + if *w.srv.Config().ServiceSettings.WebserverMode == "gzip" { staticHandler = gziphandler.GzipHandler(staticHandler) pluginHandler = gziphandler.GzipHandler(pluginHandler) } diff --git a/web/web.go b/web/web.go index 3dd8aa58fa..d58bf0abe3 100644 --- a/web/web.go +++ b/web/web.go @@ -18,16 +18,16 @@ import ( ) type Web struct { - app app.AppIface + srv *app.Server MainRouter *mux.Router } -func New(a app.AppIface, root *mux.Router) *Web { +func New(srv *app.Server) *Web { mlog.Debug("Initializing web routes") web := &Web{ - app: a, - MainRouter: root, + srv: srv, + MainRouter: srv.Router, } web.InitOAuth() diff --git a/web/web_test.go b/web/web_test.go index f3ed6d09fc..c2d85183ca 100644 --- a/web/web_test.go +++ b/web/web_test.go @@ -122,7 +122,7 @@ func setupTestHelper(tb testing.TB, includeCacheLayer bool) *TestHelper { ctx := &request.Context{} a := app.New(app.ServerConnector(s.Channels())) - web := New(a, s.Router) + web := New(s) URL = fmt.Sprintf("http://localhost:%v", s.ListenAddr.Port) apiClient = model.NewAPIv4Client(URL)