From 5ff680d20d27cd91931764d63bc9b38e4a01d8bc Mon Sep 17 00:00:00 2001 From: Ben Schumacher Date: Wed, 28 Aug 2024 07:02:09 +0200 Subject: [PATCH] [MM-60262] Respect config store option when creating platform service (#28038) * Respect config store option when creating platform service * Remove ConfigStore from ServiceConfig --- server/channels/app/notification_push_test.go | 5 +-- server/channels/app/platform/config.go | 3 +- server/channels/app/platform/helper_test.go | 10 +++-- server/channels/app/platform/service.go | 15 ++++--- server/channels/app/platform/service_test.go | 42 +++++++++++-------- 5 files changed, 40 insertions(+), 35 deletions(-) diff --git a/server/channels/app/notification_push_test.go b/server/channels/app/notification_push_test.go index ff06f5c124e..aa1f8744cc0 100644 --- a/server/channels/app/notification_push_test.go +++ b/server/channels/app/notification_push_test.go @@ -1542,9 +1542,8 @@ func TestPushNotificationRace(t *testing.T) { } var err error s.platform, err = platform.New( - platform.ServiceConfig{ - ConfigStore: memoryStore, - }, + platform.ServiceConfig{}, + platform.ConfigStore(memoryStore), platform.SetFileStore(&fmocks.FileBackend{}), platform.SetExportFileStore(&fmocks.FileBackend{}), platform.StoreOverride(mockStore)) diff --git a/server/channels/app/platform/config.go b/server/channels/app/platform/config.go index 5cbf8850b7d..de1e481c0a0 100644 --- a/server/channels/app/platform/config.go +++ b/server/channels/app/platform/config.go @@ -31,8 +31,7 @@ import ( // The mandatory fields will be checked during the initialization of the service. type ServiceConfig struct { // Mandatory fields - ConfigStore *config.Store - Store store.Store + Store store.Store // Optional fields Cluster einterfaces.ClusterInterface } diff --git a/server/channels/app/platform/helper_test.go b/server/channels/app/platform/helper_test.go index 729921e14d4..4bbf0b1a9eb 100644 --- a/server/channels/app/platform/helper_test.go +++ b/server/channels/app/platform/helper_test.go @@ -151,10 +151,12 @@ func setupTestHelper(dbStore store.Store, enterprise bool, includeCacheLayer boo *memoryConfig.MetricsSettings.ListenAddress = "localhost:0" configStore.Set(memoryConfig) - ps, err := New(ServiceConfig{ - ConfigStore: configStore, - Store: dbStore, - }, options...) + options = append(options, ConfigStore(configStore)) + + ps, err := New( + ServiceConfig{ + Store: dbStore, + }, options...) if err != nil { panic(err) } diff --git a/server/channels/app/platform/service.go b/server/channels/app/platform/service.go index eca176cad7f..bff2a6f4bb6 100644 --- a/server/channels/app/platform/service.go +++ b/server/channels/app/platform/service.go @@ -117,7 +117,6 @@ func New(sc ServiceConfig, options ...Option) (*PlatformService, error) { // ConfigStore is and should be handled on a upper level. ps := &PlatformService{ Store: sc.Store, - configStore: sc.ConfigStore, clusterIFace: sc.Cluster, hashSeed: maphash.MakeSeed(), goroutineExitSignal: make(chan struct{}, 1), @@ -137,6 +136,13 @@ func New(sc ServiceConfig, options ...Option) (*PlatformService, error) { // Assume the first user account has not been created yet. A call to the DB will later check if this is really the case. ps.isFirstUserAccount.Store(true) + // Apply options, some of the options overrides the default config actually. + for _, option := range options { + if err2 := option(ps); err2 != nil { + return nil, fmt.Errorf("failed to apply option: %w", err2) + } + } + // the config store is not set, we need to create a new one if ps.configStore == nil { innerStore, err := config.NewFileStore("config.json", true) @@ -177,13 +183,6 @@ func New(sc ServiceConfig, options ...Option) (*PlatformService, error) { return nil, fmt.Errorf("unable to connect to cache provider: %w", err) } - // Apply options, some of the options overrides the default config actually. - for _, option := range options { - if err2 := option(ps); err2 != nil { - return nil, fmt.Errorf("failed to apply option: %w", err2) - } - } - // Step 2: Start logging. if err2 := ps.initLogging(); err2 != nil { return nil, fmt.Errorf("failed to initialize logging: %w", err2) diff --git a/server/channels/app/platform/service_test.go b/server/channels/app/platform/service_test.go index f4985f5fe4b..525a81bfbae 100644 --- a/server/channels/app/platform/service_test.go +++ b/server/channels/app/platform/service_test.go @@ -35,9 +35,10 @@ func TestReadReplicaDisabledBasedOnLicense(t *testing.T) { t.Run("Read Replicas with no License", func(t *testing.T) { configStore := config.NewTestMemoryStore() configStore.Set(&cfg) - ps, err := New(ServiceConfig{ - ConfigStore: configStore, - }) + ps, err := New( + ServiceConfig{}, + ConfigStore(configStore), + ) require.NoError(t, err) require.Same(t, ps.sqlStore.GetMasterX(), ps.sqlStore.GetReplicaX()) require.Len(t, ps.Config().SqlSettings.DataSourceReplicas, 1) @@ -46,12 +47,14 @@ func TestReadReplicaDisabledBasedOnLicense(t *testing.T) { t.Run("Read Replicas With License", func(t *testing.T) { configStore := config.NewTestMemoryStore() configStore.Set(&cfg) - ps, err := New(ServiceConfig{ - ConfigStore: configStore, - }, func(ps *PlatformService) error { - ps.licenseValue.Store(model.NewTestLicense()) - return nil - }) + ps, err := New( + ServiceConfig{}, + ConfigStore(configStore), + func(ps *PlatformService) error { + ps.licenseValue.Store(model.NewTestLicense()) + return nil + }, + ) require.NoError(t, err) require.NotSame(t, ps.sqlStore.GetMasterX(), ps.sqlStore.GetReplicaX()) require.Len(t, ps.Config().SqlSettings.DataSourceReplicas, 1) @@ -60,9 +63,10 @@ func TestReadReplicaDisabledBasedOnLicense(t *testing.T) { t.Run("Search Replicas with no License", func(t *testing.T) { configStore := config.NewTestMemoryStore() configStore.Set(&cfg) - ps, err := New(ServiceConfig{ - ConfigStore: configStore, - }) + ps, err := New( + ServiceConfig{}, + ConfigStore(configStore), + ) require.NoError(t, err) require.Same(t, ps.sqlStore.GetMasterX(), ps.sqlStore.GetSearchReplicaX()) require.Len(t, ps.Config().SqlSettings.DataSourceSearchReplicas, 1) @@ -71,12 +75,14 @@ func TestReadReplicaDisabledBasedOnLicense(t *testing.T) { t.Run("Search Replicas With License", func(t *testing.T) { configStore := config.NewTestMemoryStore() configStore.Set(&cfg) - ps, err := New(ServiceConfig{ - ConfigStore: configStore, - }, func(ps *PlatformService) error { - ps.licenseValue.Store(model.NewTestLicense()) - return nil - }) + ps, err := New( + ServiceConfig{}, + ConfigStore(configStore), + func(ps *PlatformService) error { + ps.licenseValue.Store(model.NewTestLicense()) + return nil + }, + ) require.NoError(t, err) require.NotSame(t, ps.sqlStore.GetMasterX(), ps.sqlStore.GetSearchReplicaX()) require.Len(t, ps.Config().SqlSettings.DataSourceSearchReplicas, 1)