diff --git a/pkg/config/polling_manager.go b/pkg/config/polling_manager.go index 9d1806e64..12b1e3e1d 100644 --- a/pkg/config/polling_manager.go +++ b/pkg/config/polling_manager.go @@ -97,6 +97,7 @@ func WithInitialDatafile(datafile []byte) OptionFunc { // SyncConfig gets current datafile and updates projectConfig func (cm *PollingProjectConfigManager) SyncConfig(datafile []byte) { + initDatafile := datafile var e error var code int var respHeaders http.Header @@ -163,10 +164,10 @@ func (cm *PollingProjectConfigManager) SyncConfig(datafile []byte) { } closeMutex(nil) - if cm.notificationCenter != nil { + if cm.notificationCenter != nil && len(initDatafile) == 0 { projectConfigUpdateNotification := notification.ProjectConfigUpdateNotification{ Type: notification.ProjectConfigUpdate, - Revision: cm.projectConfig.GetRevision(), + Revision: projectConfig.GetRevision(), } if err = cm.notificationCenter.Send(notification.ProjectConfigUpdate, projectConfigUpdateNotification); err != nil { cmLogger.Warning("Problem with sending notification") @@ -177,6 +178,8 @@ func (cm *PollingProjectConfigManager) SyncConfig(datafile []byte) { // Start starts the polling func (cm *PollingProjectConfigManager) Start(ctx context.Context) { cmLogger.Debug("Polling Config Manager Initiated") + cm.SyncConfig([]byte{}) + t := time.NewTicker(cm.pollingInterval) for { select { @@ -205,7 +208,10 @@ func NewPollingProjectConfigManager(sdkKey string, pollingMangerOptions ...Optio } initDatafile := pollingProjectConfigManager.initDatafile - pollingProjectConfigManager.SyncConfig(initDatafile) // initial poll + if len(initDatafile) > 0 { + pollingProjectConfigManager.SyncConfig(initDatafile) // initial poll + } + return &pollingProjectConfigManager } diff --git a/pkg/config/polling_manager_test.go b/pkg/config/polling_manager_test.go index 6e593e636..5c941445d 100644 --- a/pkg/config/polling_manager_test.go +++ b/pkg/config/polling_manager_test.go @@ -19,6 +19,7 @@ package config import ( "context" "net/http" + "sync" "testing" "time" @@ -44,6 +45,14 @@ func newExecGroup() *utils.ExecGroup { return utils.NewExecGroup(context.Background()) } +// assertion method to wait for config or err for a specified period of time. +func waitForConfigOrCancelTimeout(t *testing.T, configManager ProjectConfigManager, checkError bool) { + assert.Eventually(t, func() bool { + _, err := configManager.GetConfig() + return (err != nil) == checkError + }, 500*time.Millisecond, 10*time.Millisecond) +} + func TestNewPollingProjectConfigManagerWithOptions(t *testing.T) { mockDatafile := []byte(`{"revision":"42"}`) @@ -57,6 +66,8 @@ func TestNewPollingProjectConfigManagerWithOptions(t *testing.T) { eg := newExecGroup() configManager := NewPollingProjectConfigManager(sdkKey, WithRequester(mockRequester)) eg.Go(configManager.Start) + + waitForConfigOrCancelTimeout(t, configManager, false) mockRequester.AssertExpectations(t) actual, err := configManager.GetConfig() @@ -78,10 +89,11 @@ func TestNewPollingProjectConfigManagerWithNull(t *testing.T) { eg := newExecGroup() configManager := NewPollingProjectConfigManager(sdkKey, WithRequester(mockRequester)) eg.Go(configManager.Start) + + waitForConfigOrCancelTimeout(t, configManager, true) + mockRequester.AssertExpectations(t) - _, err := configManager.GetConfig() - assert.NotNil(t, err) } func TestNewPollingProjectConfigManagerWithSimilarDatafileRevisions(t *testing.T) { @@ -96,6 +108,9 @@ func TestNewPollingProjectConfigManagerWithSimilarDatafileRevisions(t *testing.T eg := newExecGroup() configManager := NewPollingProjectConfigManager(sdkKey, WithRequester(mockRequester)) eg.Go(configManager.Start) + + waitForConfigOrCancelTimeout(t, configManager, false) + mockRequester.AssertExpectations(t) actual, err := configManager.GetConfig() @@ -125,6 +140,8 @@ func TestNewPollingProjectConfigManagerWithLastModifiedDates(t *testing.T) { configManager := NewPollingProjectConfigManager(sdkKey, WithRequester(mockRequester)) eg.Go(configManager.Start) + waitForConfigOrCancelTimeout(t, configManager, false) + // Fetch valid config actual, err := configManager.GetConfig() assert.Nil(t, err) @@ -154,6 +171,9 @@ func TestNewPollingProjectConfigManagerWithDifferentDatafileRevisions(t *testing eg := newExecGroup() configManager := NewPollingProjectConfigManager(sdkKey, WithRequester(mockRequester)) eg.Go(configManager.Start) + + waitForConfigOrCancelTimeout(t, configManager, false) + mockRequester.AssertExpectations(t) actual, err := configManager.GetConfig() @@ -170,15 +190,14 @@ func TestPollingGetOptimizelyConfig(t *testing.T) { mockDatafile1 := []byte(`{"revision":"42","botFiltering":true}`) mockDatafile2 := []byte(`{"revision":"43","botFiltering":false}`) mockRequester := new(MockRequester) - mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, http.Header{}, http.StatusOK, nil) + mockRequester.On("Get", []utils.Header(nil)).Return([]byte(mockDatafile2), http.Header{}, http.StatusOK, nil) // Test we fetch using requester sdkKey := "test_sdk_key" eg := newExecGroup() - configManager := NewPollingProjectConfigManager(sdkKey, WithRequester(mockRequester)) + configManager := NewPollingProjectConfigManager(sdkKey, WithRequester(mockRequester), WithInitialDatafile(mockDatafile1)) eg.Go(configManager.Start) - mockRequester.AssertExpectations(t) assert.Nil(t, configManager.optimizelyConfig) @@ -189,10 +208,14 @@ func TestPollingGetOptimizelyConfig(t *testing.T) { assert.Equal(t, "42", optimizelyConfig.Revision) - configManager.SyncConfig(mockDatafile2) + assert.Eventually(t, func() bool { + config, _ := configManager.GetConfig() + return config.GetRevision() == "43" + }, time.Second, 10*time.Millisecond) + optimizelyConfig = configManager.GetOptimizelyConfig() assert.Equal(t, "43", optimizelyConfig.Revision) - + mockRequester.AssertExpectations(t) } func TestNewPollingProjectConfigManagerWithErrorHandling(t *testing.T) { @@ -210,6 +233,9 @@ func TestNewPollingProjectConfigManagerWithErrorHandling(t *testing.T) { eg := newExecGroup() configManager := NewPollingProjectConfigManager(sdkKey, WithRequester(mockRequester)) eg.Go(configManager.Start) + + waitForConfigOrCancelTimeout(t, configManager, true) + mockRequester.AssertExpectations(t) actual, err := configManager.GetConfig() // polling for bad file @@ -228,7 +254,7 @@ func TestNewPollingProjectConfigManagerWithErrorHandling(t *testing.T) { assert.Equal(t, projectConfig2, actual) } -func TestNewPollingProjectConfigManagerOnDecision(t *testing.T) { +func TestNewPollingProjectConfigManagerOnConfigUpdate(t *testing.T) { mockDatafile1 := []byte(`{"revision":"42","botFiltering":true}`) mockDatafile2 := []byte(`{"revision":"43","botFiltering":false}`) @@ -240,13 +266,18 @@ func TestNewPollingProjectConfigManagerOnDecision(t *testing.T) { eg := newExecGroup() configManager := NewPollingProjectConfigManager(sdkKey, WithRequester(mockRequester)) - eg.Go(configManager.Start) - + m := sync.RWMutex{} var numberOfCalls = 0 callback := func(notification notification.ProjectConfigUpdateNotification) { + m.Lock() + defer m.Unlock() numberOfCalls++ } id, _ := configManager.OnProjectConfigUpdate(callback) + + eg.Go(configManager.Start) + waitForConfigOrCancelTimeout(t, configManager, false) + mockRequester.AssertExpectations(t) actual, err := configManager.GetConfig() @@ -259,7 +290,10 @@ func TestNewPollingProjectConfigManagerOnDecision(t *testing.T) { assert.NotNil(t, actual) assert.NotEqual(t, id, 0) + + m.Lock() assert.Equal(t, numberOfCalls, 1) + m.Unlock() err = configManager.RemoveOnProjectConfigUpdate(id) assert.Nil(t, err) @@ -268,6 +302,77 @@ func TestNewPollingProjectConfigManagerOnDecision(t *testing.T) { assert.Nil(t, err) } +func TestNewPollingProjectConfigManagerHardcodedDatafile(t *testing.T) { + mockDatafile1 := []byte(`{"revision":"42"}`) + mockDatafile2 := []byte(`{"revision":"43"}`) + sdkKey := "test_sdk_key" + + mockRequester := new(MockRequester) + mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile2, http.Header{}, http.StatusOK, nil) + + configManager := NewPollingProjectConfigManager(sdkKey, WithInitialDatafile(mockDatafile1), WithRequester(mockRequester)) + config, err := configManager.GetConfig() + + mockRequester.AssertNotCalled(t, "Get") + assert.Nil(t, err) + assert.NotNil(t, config) + assert.Equal(t, "42", config.GetRevision()) +} + +func TestNewPollingProjectConfigManagerPullImmediatelyOnStart(t *testing.T) { + m := sync.RWMutex{} + mockDatafile1 := []byte(`{"revision":"44"}`) // remote + mockDatafile2 := []byte(`{"revision":"43"}`) // hardcoded + + mockRequester := new(MockRequester) + mockRequester.On("Get", []utils.Header(nil)).Return(mockDatafile1, http.Header{}, http.StatusOK, nil) + + // Test we fetch using requester + sdkKey := "test_sdk_key" + + configManager := NewPollingProjectConfigManager(sdkKey, + WithRequester(mockRequester), + WithInitialDatafile(mockDatafile2), + // want to make sure regardless of any polling interval, syncconfig should be called immediately + WithPollingInterval(10*time.Second)) + + config, err := configManager.GetConfig() + + numberOfCalls := 0 + + // hardcoded datafile assertion + assert.Nil(t, err) + assert.NotNil(t, config) + assert.Equal(t, "43", config.GetRevision()) + mockRequester.AssertNotCalled(t, "Get") + + callback := func(notification notification.ProjectConfigUpdateNotification) { + m.Lock() + defer m.Unlock() + numberOfCalls++ + } + + configManager.OnProjectConfigUpdate(callback) + + eg := newExecGroup() + eg.Go(configManager.Start) + + assert.Eventually(t, func() bool { + m.Lock() + defer m.Unlock() + return numberOfCalls == 1 + }, 1500*time.Millisecond, 10*time.Millisecond) + + mockRequester.AssertExpectations(t) + + remoteConfig, err := configManager.GetConfig() + assert.Nil(t, err) + assert.NotNil(t, remoteConfig) + assert.Equal(t, "44", remoteConfig.GetRevision()) + + eg.TerminateAndWait() // just sending signal and improving coverage +} + func TestPollingInterval(t *testing.T) { sdkKey := "test_sdk_key" diff --git a/pkg/notification/manager.go b/pkg/notification/manager.go index b95fef810..b6799be91 100644 --- a/pkg/notification/manager.go +++ b/pkg/notification/manager.go @@ -19,6 +19,7 @@ package notification import ( "fmt" + "sync" "sync/atomic" "github.com/optimizely/go-sdk/pkg/logging" @@ -37,6 +38,7 @@ type Manager interface { type AtomicManager struct { handlers map[uint32]func(interface{}) counter uint32 + lock sync.Mutex } // NewAtomicManager creates a new instance of the atomic manager @@ -49,6 +51,8 @@ func NewAtomicManager() *AtomicManager { // Add adds the given handler func (am *AtomicManager) Add(newHandler func(interface{})) (int, error) { atomic.AddUint32(&am.counter, 1) + am.lock.Lock() + defer am.lock.Unlock() am.handlers[am.counter] = newHandler return int(am.counter), nil } @@ -56,6 +60,8 @@ func (am *AtomicManager) Add(newHandler func(interface{})) (int, error) { // Remove removes handler with the given id func (am *AtomicManager) Remove(id int) { handlerID := uint32(id) + am.lock.Lock() + defer am.lock.Unlock() if _, ok := am.handlers[handlerID]; ok { delete(am.handlers, handlerID) return @@ -66,6 +72,8 @@ func (am *AtomicManager) Remove(id int) { // Send sends the notification to the registered handlers func (am *AtomicManager) Send(notification interface{}) { + am.lock.Lock() + defer am.lock.Unlock() for _, handler := range am.handlers { handler(notification) }