From ab8de49f0ac46752450ba4293fee320fbbb159e1 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 3 Mar 2022 12:22:10 +0530 Subject: [PATCH] MM-40818: Refactor product initialization to happen late (#19658) This is more of a general refactor of the initialization process which should allow us to pass services more easily. The changes are minimal to keep the scope limited. For now, the objective is to pass the file service to the Channels product. For that, it was required to move some of the enterprise interfaces under Channels from Server. We also create a filestore field in the server to avoid creating filestore reference every time we make a filestore operation. This will be later passed on to the Channels product. Also removed an unnecessary test. The test was working so far because we were creating the filebackend every time for every request. But we should go via UpdateConfig call which would fail, were we to assign an invalid filestore name. So we were actually testing for a different thing. Therefore, removed the test. ```release-note NONE ``` --- api4/ldap_test.go | 2 +- api4/saml_test.go | 2 +- api4/system.go | 6 +- api4/system_test.go | 14 --- api4/user_test.go | 6 +- app/app.go | 6 +- app/app_iface.go | 2 +- app/channels.go | 21 +++++ app/config.go | 2 +- app/enterprise.go | 39 ++------- app/enterprise_test.go | 8 +- app/file.go | 70 +++------------ app/notification_push.go | 2 +- app/opentracing/opentracing_layer.go | 11 +-- app/server.go | 123 +++++++++++++-------------- app/server_test.go | 4 +- app/web_hub_test.go | 1 + 17 files changed, 119 insertions(+), 200 deletions(-) diff --git a/api4/ldap_test.go b/api4/ldap_test.go index b90e4a12b1..82e9c0b31b 100644 --- a/api4/ldap_test.go +++ b/api4/ldap_test.go @@ -153,7 +153,7 @@ func TestSyncLdap(t *testing.T) { includeRemovedMembers = args[1].(bool) ready <- true } - th.App.Srv().Ldap = ldapMock + th.App.Channels().Ldap = ldapMock th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { _, err := client.SyncLdap(false) diff --git a/api4/saml_test.go b/api4/saml_test.go index c1f21c2a70..84d32fda69 100644 --- a/api4/saml_test.go +++ b/api4/saml_test.go @@ -56,7 +56,7 @@ func TestSamlCompleteCSRFPass(t *testing.T) { func TestSamlResetId(t *testing.T) { th := SetupEnterprise(t).InitBasic() defer th.TearDown() - th.App.Srv().Saml = &mocks.SamlInterface{} + th.App.Channels().Saml = &mocks.SamlInterface{} user := th.BasicUser _, appErr := th.App.UpdateUserAuth(user.Id, &model.UserAuth{ diff --git a/api4/system.go b/api4/system.go index bc49164e7f..a1d6b10be4 100644 --- a/api4/system.go +++ b/api4/system.go @@ -95,11 +95,7 @@ func generateSupportPacket(c *Context, w http.ResponseWriter, r *http.Request) { now := time.Now() outputZipFilename := fmt.Sprintf("mattermost_support_packet_%s.zip", now.Format("2006-01-02-03-04")) - fileStorageBackend, fileBackendErr := c.App.FileBackend() - if fileBackendErr != nil { - c.Err = fileBackendErr - return - } + fileStorageBackend := c.App.FileBackend() // We do this incase we get concurrent requests, we will always have a unique directory. // This is to avoid the situation where we try to write to the same directory while we are trying to delete it (further down) diff --git a/api4/system_test.go b/api4/system_test.go index 9bf3f9e73b..68d502e4a7 100644 --- a/api4/system_test.go +++ b/api4/system_test.go @@ -57,20 +57,6 @@ func TestGetPing(t *testing.T) { require.NoError(t, err) assert.Equal(t, model.StatusOk, status) }) - - t.Run("unhealthy", func(t *testing.T) { - oldDriver := th.App.Config().FileSettings.DriverName - badDriver := "badDriverName" - th.App.Config().FileSettings.DriverName = &badDriver - defer func() { - th.App.Config().FileSettings.DriverName = oldDriver - }() - - status, resp, err := client.GetPingWithServerStatus() - require.Error(t, err) - CheckInternalErrorStatus(t, resp) - assert.Equal(t, model.StatusUnhealthy, status) - }) }, "with server status") th.TestForAllClients(t, func(t *testing.T, client *model.Client4) { diff --git a/api4/user_test.go b/api4/user_test.go index d747ee7dda..125d064c99 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -6503,7 +6503,7 @@ func TestPatchAndUpdateWithProviderAttributes(t *testing.T) { mock.Anything, // *model.User mock.Anything, // *model.Patch ).Return("") - th.App.Srv().Ldap = ldapMock + th.App.Channels().Ldap = ldapMock // CheckProviderAttributes should be called for both Patch and Update th.SystemAdminClient.PatchUser(user.Id, &model.UserPatch{}) ldapMock.AssertNumberOfCalls(t, "CheckProviderAttributes", 1) @@ -6524,7 +6524,7 @@ func TestPatchAndUpdateWithProviderAttributes(t *testing.T) { ldapMock.Mock.On( "CheckProviderAttributes", mock.Anything, mock.Anything, mock.Anything, ).Return("") - th.App.Srv().Ldap = ldapMock + th.App.Channels().Ldap = ldapMock th.SystemAdminClient.PatchUser(user.Id, &model.UserPatch{}) ldapMock.AssertNumberOfCalls(t, "CheckProviderAttributes", 1) th.SystemAdminClient.UpdateUser(user) @@ -6538,7 +6538,7 @@ func TestPatchAndUpdateWithProviderAttributes(t *testing.T) { samlMock.Mock.On( "CheckProviderAttributes", mock.Anything, mock.Anything, mock.Anything, ).Return("") - th.App.Srv().Saml = samlMock + th.App.Channels().Saml = samlMock th.SystemAdminClient.PatchUser(user.Id, &model.UserPatch{}) samlMock.AssertNumberOfCalls(t, "CheckProviderAttributes", 1) th.SystemAdminClient.UpdateUser(user) diff --git a/app/app.go b/app/app.go index 90b98a79b4..a85737e328 100644 --- a/app/app.go +++ b/app/app.go @@ -110,7 +110,7 @@ func (a *App) SearchEngine() *searchengine.Broker { return a.ch.srv.SearchEngine } func (a *App) Ldap() einterfaces.LdapInterface { - return a.ch.srv.Ldap + return a.ch.Ldap } func (a *App) MessageExport() einterfaces.MessageExportInterface { return a.ch.MessageExport @@ -119,10 +119,10 @@ func (a *App) Metrics() einterfaces.MetricsInterface { return a.ch.srv.Metrics } func (a *App) Notification() einterfaces.NotificationInterface { - return a.ch.srv.Notification + return a.ch.Notification } func (a *App) Saml() einterfaces.SamlInterface { - return a.ch.srv.Saml + return a.ch.Saml } func (a *App) Cloud() einterfaces.CloudInterface { return a.ch.srv.Cloud diff --git a/app/app_iface.go b/app/app_iface.go index ee1a01a57d..80bc7158ac 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -527,7 +527,7 @@ type AppIface interface { ExportPermissions(w io.Writer) error ExtractContentFromFileInfo(fileInfo *model.FileInfo) error FetchSamlMetadataFromIdp(url string) ([]byte, *model.AppError) - FileBackend() (filestore.FileBackend, *model.AppError) + FileBackend() filestore.FileBackend FileExists(path string) (bool, *model.AppError) FileModTime(path string) (time.Time, *model.AppError) FileSize(path string) (int64, *model.AppError) diff --git a/app/channels.go b/app/channels.go index 3604980582..fc7867e195 100644 --- a/app/channels.go +++ b/app/channels.go @@ -75,6 +75,9 @@ type Channels struct { Compliance einterfaces.ComplianceInterface DataRetention einterfaces.DataRetentionInterface MessageExport einterfaces.MessageExportInterface + Saml einterfaces.SamlInterface + Notification einterfaces.NotificationInterface + Ldap einterfaces.LdapInterface // These are used to prevent concurrent upload requests // for a given upload session which could cause inconsistencies @@ -145,6 +148,24 @@ func NewChannels(s *Server, services map[ServiceKey]interface{}) (*Channels, err if accountMigrationInterface != nil { ch.AccountMigration = accountMigrationInterface(New(ServerConnector(ch))) } + if ldapInterface != nil { + ch.Ldap = ldapInterface(New(ServerConnector(ch))) + } + if notificationInterface != nil { + ch.Notification = notificationInterface(New(ServerConnector(ch))) + } + if samlInterfaceNew != nil { + ch.Saml = samlInterfaceNew(New(ServerConnector(ch))) + if err := ch.Saml.ConfigureSP(); err != nil { + mlog.Error("An error occurred while configuring SAML Service Provider", mlog.Err(err)) + } + + ch.AddConfigListener(func(_, _ *model.Config) { + if err := ch.Saml.ConfigureSP(); err != nil { + mlog.Error("An error occurred while configuring SAML Service Provider", mlog.Err(err)) + } + }) + } var imgErr error ch.imgDecoder, imgErr = imaging.NewDecoder(imaging.DecoderOptions{ diff --git a/app/config.go b/app/config.go index 236bf2b02e..b7033d01a3 100644 --- a/app/config.go +++ b/app/config.go @@ -110,7 +110,7 @@ func (s *Server) ConfigStore() *configWrapper { } func (a *App) Config() *model.Config { - return a.Srv().Config() + return a.ch.cfgSvc.Config() } func (s *Server) EnvironmentConfig(filter func(reflect.StructField) bool) map[string]interface{} { diff --git a/app/enterprise.go b/app/enterprise.go index 7d85752ad3..46b7090861 100644 --- a/app/enterprise.go +++ b/app/enterprise.go @@ -6,9 +6,7 @@ package app import ( "github.com/mattermost/mattermost-server/v6/einterfaces" ejobs "github.com/mattermost/mattermost-server/v6/einterfaces/jobs" - "github.com/mattermost/mattermost-server/v6/model" "github.com/mattermost/mattermost-server/v6/services/searchengine" - "github.com/mattermost/mattermost-server/v6/shared/mlog" ) var accountMigrationInterface func(*App) einterfaces.AccountMigrationInterface @@ -65,9 +63,9 @@ func RegisterJobsElasticsearchIndexerInterface(f func(*Server) ejobs.IndexerJobI jobsElasticsearchIndexerInterface = f } -var jobsLdapSyncInterface func(*Server) ejobs.LdapSyncInterface +var jobsLdapSyncInterface func(*App) ejobs.LdapSyncInterface -func RegisterJobsLdapSyncInterface(f func(*Server) ejobs.LdapSyncInterface) { +func RegisterJobsLdapSyncInterface(f func(*App) ejobs.LdapSyncInterface) { jobsLdapSyncInterface = f } @@ -77,9 +75,9 @@ func RegisterJobsCloudInterface(f func(*Server) ejobs.CloudJobInterface) { jobsCloudInterface = f } -var ldapInterface func(*Server) einterfaces.LdapInterface +var ldapInterface func(*App) einterfaces.LdapInterface -func RegisterLdapInterface(f func(*Server) einterfaces.LdapInterface) { +func RegisterLdapInterface(f func(*App) einterfaces.LdapInterface) { ldapInterface = f } @@ -101,15 +99,15 @@ func RegisterMetricsInterface(f func(*Server) einterfaces.MetricsInterface) { metricsInterface = f } -var samlInterfaceNew func(*Server) einterfaces.SamlInterface +var samlInterfaceNew func(*App) einterfaces.SamlInterface -func RegisterNewSamlInterface(f func(*Server) einterfaces.SamlInterface) { +func RegisterNewSamlInterface(f func(*App) einterfaces.SamlInterface) { samlInterfaceNew = f } -var notificationInterface func(*Server) einterfaces.NotificationInterface +var notificationInterface func(*App) einterfaces.NotificationInterface -func RegisterNotificationInterface(f func(*Server) einterfaces.NotificationInterface) { +func RegisterNotificationInterface(f func(*App) einterfaces.NotificationInterface) { notificationInterface = f } @@ -135,27 +133,6 @@ func (s *Server) initEnterprise() { s.LicenseManager = licenseInterface(s) } - if ldapInterface != nil { - s.Ldap = ldapInterface(s) - } - - if notificationInterface != nil { - s.Notification = notificationInterface(s) - } - - if samlInterfaceNew != nil { - mlog.Debug("Loading SAML2 library") - s.Saml = samlInterfaceNew(s) - if err := s.Saml.ConfigureSP(); err != nil { - mlog.Error("An error occurred while configuring SAML Service Provider", mlog.Err(err)) - } - s.AddConfigListener(func(_, cfg *model.Config) { - if err := s.Saml.ConfigureSP(); err != nil { - mlog.Error("An error occurred while configuring SAML Service Provider", mlog.Err(err)) - } - }) - } - if cloudInterface != nil { s.Cloud = cloudInterface(s) } diff --git a/app/enterprise_test.go b/app/enterprise_test.go index 8e05caa05b..d01b6079dd 100644 --- a/app/enterprise_test.go +++ b/app/enterprise_test.go @@ -57,7 +57,7 @@ func TestSAMLSettings(t *testing.T) { saml2.Mock.On("ConfigureSP").Return(nil) saml2.Mock.On("GetMetadata").Return("samlTwo", nil) if tc.setNewInterface { - RegisterNewSamlInterface(func(_ *Server) einterfaces.SamlInterface { + RegisterNewSamlInterface(func(_ *App) einterfaces.SamlInterface { return saml2 }) } else { @@ -89,10 +89,10 @@ func TestSAMLSettings(t *testing.T) { } if tc.isNil { - assert.Nil(t, th.App.Srv().Saml) + assert.Nil(t, th.App.Channels().Saml) } else { - assert.NotNil(t, th.App.Srv().Saml) - metadata, err := th.App.Srv().Saml.GetMetadata() + assert.NotNil(t, th.App.Channels().Saml) + metadata, err := th.App.Channels().Saml.GetMetadata() assert.Nil(t, err) assert.Equal(t, tc.metadata, metadata) } diff --git a/app/file.go b/app/file.go index 96437b2815..7ff5f8ca53 100644 --- a/app/file.go +++ b/app/file.go @@ -47,7 +47,7 @@ const ( maxContentExtractionSize = 1024 * 1024 // 1MB ) -func (a *App) FileBackend() (filestore.FileBackend, *model.AppError) { +func (a *App) FileBackend() filestore.FileBackend { return a.Srv().FileBackend() } @@ -72,11 +72,7 @@ func connectionTestErrorToAppError(connTestErr error) *model.AppError { } func (a *App) TestFileStoreConnection() *model.AppError { - backend, err := a.FileBackend() - if err != nil { - return err - } - nErr := backend.TestConnection() + nErr := a.FileBackend().TestConnection() if nErr != nil { return connectionTestErrorToAppError(nErr) } @@ -101,11 +97,7 @@ func (a *App) ReadFile(path string) ([]byte, *model.AppError) { } func (s *Server) fileReader(path string) (filestore.ReadCloseSeeker, *model.AppError) { - backend, err := s.FileBackend() - if err != nil { - return nil, err - } - result, nErr := backend.Reader(path) + result, nErr := s.FileBackend().Reader(path) if nErr != nil { return nil, model.NewAppError("FileReader", "api.file.file_reader.app_error", nil, nErr.Error(), http.StatusInternalServerError) } @@ -122,11 +114,7 @@ func (a *App) FileExists(path string) (bool, *model.AppError) { } func (s *Server) fileExists(path string) (bool, *model.AppError) { - backend, err := s.FileBackend() - if err != nil { - return false, err - } - result, nErr := backend.FileExists(path) + result, nErr := s.FileBackend().FileExists(path) if nErr != nil { return false, model.NewAppError("FileExists", "api.file.file_exists.app_error", nil, nErr.Error(), http.StatusInternalServerError) } @@ -134,11 +122,7 @@ func (s *Server) fileExists(path string) (bool, *model.AppError) { } func (a *App) FileSize(path string) (int64, *model.AppError) { - backend, err := a.FileBackend() - if err != nil { - return 0, err - } - size, nErr := backend.FileSize(path) + size, nErr := a.FileBackend().FileSize(path) if nErr != nil { return 0, model.NewAppError("FileSize", "api.file.file_size.app_error", nil, nErr.Error(), http.StatusInternalServerError) } @@ -146,11 +130,7 @@ func (a *App) FileSize(path string) (int64, *model.AppError) { } func (a *App) FileModTime(path string) (time.Time, *model.AppError) { - backend, err := a.FileBackend() - if err != nil { - return time.Time{}, err - } - modTime, nErr := backend.FileModTime(path) + modTime, nErr := a.FileBackend().FileModTime(path) if nErr != nil { return time.Time{}, model.NewAppError("FileModTime", "api.file.file_mod_time.app_error", nil, nErr.Error(), http.StatusInternalServerError) } @@ -159,11 +139,7 @@ func (a *App) FileModTime(path string) (time.Time, *model.AppError) { } func (a *App) MoveFile(oldPath, newPath string) *model.AppError { - backend, err := a.FileBackend() - if err != nil { - return err - } - nErr := backend.MoveFile(oldPath, newPath) + nErr := a.FileBackend().MoveFile(oldPath, newPath) if nErr != nil { return model.NewAppError("MoveFile", "api.file.move_file.app_error", nil, nErr.Error(), http.StatusInternalServerError) } @@ -175,12 +151,7 @@ func (a *App) WriteFile(fr io.Reader, path string) (int64, *model.AppError) { } func (s *Server) writeFile(fr io.Reader, path string) (int64, *model.AppError) { - backend, err := s.FileBackend() - if err != nil { - return 0, err - } - - result, nErr := backend.WriteFile(fr, path) + result, nErr := s.FileBackend().WriteFile(fr, path) if nErr != nil { return result, model.NewAppError("WriteFile", "api.file.write_file.app_error", nil, nErr.Error(), http.StatusInternalServerError) } @@ -188,12 +159,7 @@ func (s *Server) writeFile(fr io.Reader, path string) (int64, *model.AppError) { } func (a *App) AppendFile(fr io.Reader, path string) (int64, *model.AppError) { - backend, err := a.FileBackend() - if err != nil { - return 0, err - } - - result, nErr := backend.AppendFile(fr, path) + result, nErr := a.FileBackend().AppendFile(fr, path) if nErr != nil { return result, model.NewAppError("AppendFile", "api.file.append_file.app_error", nil, nErr.Error(), http.StatusInternalServerError) } @@ -205,11 +171,7 @@ func (a *App) RemoveFile(path string) *model.AppError { } func (s *Server) removeFile(path string) *model.AppError { - backend, err := s.FileBackend() - if err != nil { - return err - } - nErr := backend.RemoveFile(path) + nErr := s.FileBackend().RemoveFile(path) if nErr != nil { return model.NewAppError("RemoveFile", "api.file.remove_file.app_error", nil, nErr.Error(), http.StatusInternalServerError) } @@ -221,11 +183,7 @@ func (a *App) ListDirectory(path string) ([]string, *model.AppError) { } func (s *Server) listDirectory(path string) ([]string, *model.AppError) { - backend, err := s.FileBackend() - if err != nil { - return nil, err - } - paths, nErr := backend.ListDirectory(path) + paths, nErr := s.FileBackend().ListDirectory(path) if nErr != nil { return nil, model.NewAppError("ListDirectory", "api.file.list_directory.app_error", nil, nErr.Error(), http.StatusInternalServerError) } @@ -234,11 +192,7 @@ func (s *Server) listDirectory(path string) ([]string, *model.AppError) { } func (a *App) RemoveDirectory(path string) *model.AppError { - backend, err := a.FileBackend() - if err != nil { - return err - } - nErr := backend.RemoveDirectory(path) + nErr := a.FileBackend().RemoveDirectory(path) if nErr != nil { return model.NewAppError("RemoveDirectory", "api.file.remove_directory.app_error", nil, nErr.Error(), http.StatusInternalServerError) } diff --git a/app/notification_push.go b/app/notification_push.go index d7359b920d..a69903455a 100644 --- a/app/notification_push.go +++ b/app/notification_push.go @@ -557,7 +557,7 @@ func (a *App) BuildPushNotificationMessage(contentsConfig string, post *model.Po var msg *model.PushNotification - notificationInterface := a.Srv().Notification + notificationInterface := a.ch.Notification if (notificationInterface == nil || notificationInterface.CheckLicense() != nil) && contentsConfig == model.IdLoadedNotification { contentsConfig = model.GenericNotification } diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index d8e9d79ed4..4e3d0f4c3e 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -4013,7 +4013,7 @@ func (a *OpenTracingAppLayer) FetchSamlMetadataFromIdp(url string) ([]byte, *mod return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) FileBackend() (filestore.FileBackend, *model.AppError) { +func (a *OpenTracingAppLayer) FileBackend() filestore.FileBackend { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.FileBackend") @@ -4025,14 +4025,9 @@ func (a *OpenTracingAppLayer) FileBackend() (filestore.FileBackend, *model.AppEr }() defer span.Finish() - resultVar0, resultVar1 := a.app.FileBackend() + resultVar0 := a.app.FileBackend() - if resultVar1 != nil { - span.LogFields(spanlog.Error(resultVar1)) - ext.Error.Set(span, true) - } - - return resultVar0, resultVar1 + return resultVar0 } func (a *OpenTracingAppLayer) FileExists(path string) (bool, *model.AppError) { diff --git a/app/server.go b/app/server.go index 374c995c3d..eab1724698 100644 --- a/app/server.go +++ b/app/server.go @@ -159,6 +159,7 @@ type Server struct { searchLicenseListenerId string loggerLicenseListenerId string configStore *configWrapper + filestore filestore.FileBackend telemetryService *telemetry.TelemetryService userService *users.UserService @@ -184,10 +185,7 @@ type Server struct { Cluster einterfaces.ClusterInterface Cloud einterfaces.CloudInterface Metrics einterfaces.MetricsInterface - Notification einterfaces.NotificationInterface LicenseManager einterfaces.LicenseInterface - Saml einterfaces.SamlInterface - Ldap einterfaces.LdapInterface CacheProvider cache.Provider @@ -257,27 +255,8 @@ func NewServer(options ...Option) (*Server, error) { mlog.Info("Server is initializing...", mlog.String("go_version", runtime.Version())) s.httpService = httpservice.MakeHTTPService(s) - s.licenseWrapper = &licenseWrapper{ - srv: s, - } - serviceMap := map[ServiceKey]interface{}{ - ConfigKey: s.configStore, - LicenseKey: s.licenseWrapper, - } - - // Step 3: Initialize products. - // Depends on s.httpService. - for name, initializer := range products { - prod, err2 := initializer(s, serviceMap) - if err2 != nil { - return nil, errors.Wrapf(err2, "error initializing product: %s", name) - } - - s.products[name] = prod - } - - // Step 4: Search Engine + // Step 3: Search Engine // Depends on Step 1 (config). searchEngine := searchengine.NewBroker(s.Config()) bleveEngine := bleveengine.NewBleveEngine(s.Config()) @@ -287,12 +266,11 @@ func NewServer(options ...Option) (*Server, error) { searchEngine.RegisterBleveEngine(bleveEngine) s.SearchEngine = searchEngine - // Step 5: Init Enterprise - // Depends on step 3 (s.Channels() must be non-nil) - // and step 4 (s.SearchEngine must be non-nil) + // Step 4: Init Enterprise + // Depends on step 3 (s.SearchEngine must be non-nil) s.initEnterprise() - // Step 6: Cache provider. + // Step 5: Cache provider. // At the moment we only have this implementation // in the future the cache provider will be built based on the loaded config s.CacheProvider = cache.NewProvider() @@ -300,12 +278,8 @@ func NewServer(options ...Option) (*Server, error) { return nil, errors.Wrapf(err2, "Unable to connect to cache provider") } - // It is important to initialize the hub only after the global logger is set - // to avoid race conditions while logging from inside the hub. - s.HubStart() - - // Step 7: Store. - // Depends on Step 1 (config), 5 (metrics, cluster) and 6 (cacheProvider). + // Step 6: Store. + // Depends on Step 1 (config), 4 (metrics, cluster) and 5 (cacheProvider). if s.newStore == nil { s.newStore = func() (store.Store, error) { s.sqlStore = sqlstore.New(s.Config().SqlSettings, s.Metrics) @@ -347,6 +321,42 @@ func NewServer(options ...Option) (*Server, error) { return nil, errors.Wrap(err, "cannot create store") } + if model.BuildEnterpriseReady == "true" { + s.LoadLicense() + } + + license := s.License() + // Step 7: Initialize filestore + backend, err := filestore.NewFileBackend(s.Config().FileSettings.ToFileBackendSettings(license != nil && *license.Features.Compliance)) + if err != nil { + return nil, errors.Wrap(err, "failed to initialize filebackend") + } + s.filestore = backend + + s.licenseWrapper = &licenseWrapper{ + srv: s, + } + + serviceMap := map[ServiceKey]interface{}{ + ConfigKey: s.configStore, + LicenseKey: s.licenseWrapper, + } + // Step 8: Initialize products. + // Depends on s.httpService. + for name, initializer := range products { + prod, err2 := initializer(s, serviceMap) + if err2 != nil { + return nil, errors.Wrapf(err2, "error initializing product: %s", name) + } + + s.products[name] = prod + } + + // It is important to initialize the hub only after the global logger is set + // to avoid race conditions while logging from inside the hub. + // Step 9: Hub depends on s.Channels() (step 8) + s.HubStart() + // ------------------------------------------------------------------------- // Everything below this is not order sensitive and safe to be moved around. // If you are adding a new field that is non-channels specific, please add @@ -503,10 +513,6 @@ func NewServer(options ...Option) (*Server, error) { } s.EmailService = emailService - if model.BuildEnterpriseReady == "true" { - s.LoadLicense() - } - s.setupFeatureFlags() s.initJobs() @@ -560,7 +566,6 @@ func NewServer(options ...Option) (*Server, error) { mlog.Info("Printing current working", mlog.String("directory", pwd)) mlog.Info("Loaded config", mlog.String("source", s.configStore.String())) - license := s.License() allowAdvancedLogging := license != nil && *license.Features.AdvancedLogging if s.Audit == nil { @@ -1172,18 +1177,13 @@ func (s *Server) Start() error { mlog.Error("Mail server connection test is failed", mlog.Err(err)) } - backend, appErr := s.FileBackend() - if appErr != nil { - mlog.Error("Problem with file storage settings", mlog.Err(appErr)) - } else { - err := backend.TestConnection() + err := s.FileBackend().TestConnection() + if err != nil { + if _, ok := err.(*filestore.S3FileBackendNoBucketError); ok { + err = s.FileBackend().(*filestore.S3FileBackend).MakeBucket() + } if err != nil { - if _, ok := err.(*filestore.S3FileBackendNoBucketError); ok { - err = backend.(*filestore.S3FileBackend).MakeBucket() - } - if err != nil { - mlog.Error("Problem with file storage settings", mlog.Err(err)) - } + mlog.Error("Problem with file storage settings", mlog.Err(err)) } } @@ -1227,9 +1227,9 @@ func (s *Server) Start() error { if *s.Config().RateLimitSettings.Enable { mlog.Info("RateLimiter is enabled") - rateLimiter, err := NewRateLimiter(&s.Config().RateLimitSettings, s.Config().ServiceSettings.TrustedProxyIPHeader) - if err != nil { - return err + rateLimiter, err2 := NewRateLimiter(&s.Config().RateLimitSettings, s.Config().ServiceSettings.TrustedProxyIPHeader) + if err2 != nil { + return err2 } s.RateLimiter = rateLimiter @@ -1850,13 +1850,8 @@ func (s *Server) stopSearchEngine() { } } -func (s *Server) FileBackend() (filestore.FileBackend, *model.AppError) { - license := s.License() - backend, err := filestore.NewFileBackend(s.Config().FileSettings.ToFileBackendSettings(license != nil && *license.Features.Compliance)) - if err != nil { - return nil, model.NewAppError("FileBackend", "api.file.no_driver.app_error", nil, err.Error(), http.StatusInternalServerError) - } - return backend, nil +func (s *Server) FileBackend() filestore.FileBackend { + return s.filestore } func (s *Server) TotalWebsocketConnections() int { @@ -1904,7 +1899,7 @@ func (s *Server) initJobs() { } if jobsLdapSyncInterface != nil { - builder := jobsLdapSyncInterface(s) + builder := jobsLdapSyncInterface(New(ServerConnector(s.Channels()))) s.Jobs.RegisterJobType(model.JobTypeLdapSync, builder.MakeWorker(), builder.MakeScheduler()) } @@ -2189,7 +2184,7 @@ func (a *App) generateSupportPacketYaml() (*model.FileData, string) { } // Here we are getting information regarding LDAP - ldapInterface := a.ch.srv.Ldap + ldapInterface := a.ch.Ldap var vendorName, vendorVersion string if ldapInterface != nil { vendorName, vendorVersion = ldapInterface.GetVendorNameAndVendorVersion() @@ -2270,11 +2265,7 @@ func (s *Server) GetDefaultProfileImage(user *model.User) ([]byte, *model.AppErr } func (s *Server) ReadFile(path string) ([]byte, *model.AppError) { - backend, err := s.FileBackend() - if err != nil { - return nil, err - } - result, nErr := backend.ReadFile(path) + result, nErr := s.FileBackend().ReadFile(path) if nErr != nil { return nil, model.NewAppError("ReadFile", "api.file.read_file.app_error", nil, nErr.Error(), http.StatusInternalServerError) } diff --git a/app/server_test.go b/app/server_test.go index 108ec935e3..515f76b4ee 100644 --- a/app/server_test.go +++ b/app/server_test.go @@ -191,9 +191,7 @@ func TestStartServerNoS3Bucket(t *testing.T) { defer s.Shutdown() // ensure that a new bucket was created - backend, appErr := s.FileBackend() - require.Nil(t, appErr) - err = backend.(*filestore.S3FileBackend).TestConnection() + err = s.FileBackend().(*filestore.S3FileBackend).TestConnection() require.NoError(t, err) } diff --git a/app/web_hub_test.go b/app/web_hub_test.go index 423a800053..152d9aabbb 100644 --- a/app/web_hub_test.go +++ b/app/web_hub_test.go @@ -338,6 +338,7 @@ func TestHubConnIndexInactive(t *testing.T) { } func TestReliableWebSocketSend(t *testing.T) { + t.Skip("MM-42033") testCluster := &testlib.FakeClusterInterface{} th := SetupWithClusterMock(t, testCluster)