Fix: Added notificationRegistry to make sure that odpSettings updates (#501)

* Added notificationRegistry to make sure that odpSettings update will call upon UpdateConfig

* Updated headers and added additional checks in unittests

* Added suppress warning

* renamed NoficationRegistry function.
Added default implementation of getSDKKey in ProjectConfigManager

* reverting header change

* reverting extra changes

* reverting extra changes

* nit fix

* nit

* Resolved comments

* Added getCachedconfig Function to return ProjectConfig without wait, but the default implementation of it will return null.

* ProjectConfig getCachedConfig(); without default implementation

* Removed default implementation of getSDKkey to enforce user to make their own choice.

* Resolved comments

* setting Sdk key inside pollingConfigManager to make sure that the sdkKey for a particular config is always same.

* setting sdkKey in parent class

* fix

* making sure that if the SDKKey is set by the user then it should be picked from projectConfig

* Refactored and moved getSdkKey to pollingConfigManager to make sure that to trigger the notification always user provided sdkKey will be prioritized

Co-authored-by: mnoman09 <[email protected]>

Author: mnoman09

core-api/src/main/java/com/optimizely/ab/Optimizely.java

@@ -27,6 +27,7 @@
 import com.optimizely.ab.event.*;
 import com.optimizely.ab.event.internal.*;
 import com.optimizely.ab.event.internal.payload.EventBatch;
+import com.optimizely.ab.internal.NotificationRegistry;
 import com.optimizely.ab.notification.*;
 import com.optimizely.ab.odp.*;
 import com.optimizely.ab.optimizelyconfig.OptimizelyConfig;
@@ -124,10 +125,15 @@ private Optimizely(@Nonnull EventHandler eventHandler,
 
         if (odpManager != null) {
             odpManager.getEventManager().start();
-            if (getProjectConfig() != null) {
+            if (projectConfigManager.getCachedConfig() != null) {
                 updateODPSettings();
             }
-            addUpdateConfigNotificationHandler(configNotification -> { updateODPSettings(); });
+            if (projectConfigManager.getSDKKey() != null) {
+                NotificationRegistry.getInternalNotificationCenter(projectConfigManager.getSDKKey()).
+                    addNotificationHandler(UpdateConfigNotification.class,
+                        configNotification -> { updateODPSettings(); });
+            }
+
         }
     }
 
@@ -153,6 +159,8 @@ public void close() {
         tryClose(eventProcessor);
         tryClose(eventHandler);
         tryClose(projectConfigManager);
+        notificationCenter.clearAllNotificationListeners();
+        NotificationRegistry.clearNotificationCenterRegistry(projectConfigManager.getSDKKey());
         if (odpManager != null) {
             tryClose(odpManager);
         }
@@ -1477,8 +1485,8 @@ public void identifyUser(@Nonnull String userId) {
     }
 
     private void updateODPSettings() {
-        if (odpManager != null && getProjectConfig() != null) {
-            ProjectConfig projectConfig = getProjectConfig();
+        ProjectConfig projectConfig = projectConfigManager.getCachedConfig();
+        if (odpManager != null && projectConfig != null) {
             odpManager.updateSettings(projectConfig.getHostForODP(), projectConfig.getPublicKeyForODP(), projectConfig.getAllSegments());
         }
     }

core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java

@@ -1,6 +1,6 @@
 /**
  *
- *    Copyright 2019, Optimizely and contributors
+ *    Copyright 2019, 2023, Optimizely and contributors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -27,6 +27,21 @@ public ProjectConfig getConfig() {
         return projectConfigReference.get();
     }
 
+    /**
+     * Access to current cached project configuration.
+     *
+     * @return {@link ProjectConfig}
+     */
+    @Override
+    public ProjectConfig getCachedConfig() {
+        return projectConfigReference.get();
+    }
+
+    @Override
+    public String getSDKKey() {
+        return null;
+    }
+
     public void setConfig(ProjectConfig projectConfig) {
         projectConfigReference.set(projectConfig);
     }

core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java

@@ -1,6 +1,6 @@
 /**
  *
- *    Copyright 2019-2020, Optimizely and contributors
+ *    Copyright 2019-2020, 2023, Optimizely and contributors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
  */
 package com.optimizely.ab.config;
 
+import com.optimizely.ab.internal.NotificationRegistry;
 import com.optimizely.ab.notification.NotificationCenter;
 import com.optimizely.ab.notification.UpdateConfigNotification;
 import com.optimizely.ab.optimizelyconfig.OptimizelyConfig;
@@ -56,6 +57,7 @@ public abstract class PollingProjectConfigManager implements ProjectConfigManage
 
     private final CountDownLatch countDownLatch = new CountDownLatch(1);
 
+    private volatile String sdkKey;
     private volatile boolean started;
     private ScheduledFuture<?> scheduledFuture;
 
@@ -84,6 +86,16 @@ public PollingProjectConfigManager(long period, TimeUnit timeUnit, long blocking
 
     protected abstract ProjectConfig poll();
 
+    /**
+     * Access to current cached project configuration, This is to make sure that config returns without any wait, even if it is null.
+     *
+     * @return {@link ProjectConfig}
+     */
+    @Override
+    public ProjectConfig getCachedConfig() {
+        return currentProjectConfig.get();
+    }
+
     /**
      * Only allow the ProjectConfig to be set to a non-null value, if and only if the value has not already been set.
      * @param projectConfig
@@ -109,6 +121,13 @@ void setConfig(ProjectConfig projectConfig) {
         currentProjectConfig.set(projectConfig);
         currentOptimizelyConfig.set(new OptimizelyConfigService(projectConfig).getConfig());
         countDownLatch.countDown();
+
+        if (sdkKey == null) {
+            sdkKey = projectConfig.getSdkKey();
+        }
+        if (sdkKey != null) {
+            NotificationRegistry.getInternalNotificationCenter(sdkKey).send(SIGNAL);
+        }
         notificationCenter.send(SIGNAL);
     }
 
@@ -150,6 +169,11 @@ public OptimizelyConfig getOptimizelyConfig() {
         return currentOptimizelyConfig.get();
     }
 
+    @Override
+    public String getSDKKey() {
+        return this.sdkKey;
+    }
+
     public synchronized void start() {
         if (started) {
             logger.warn("Manager already started.");
@@ -189,6 +213,10 @@ public synchronized void close() {
         started = false;
     }
 
+    protected void setSdkKey(String sdkKey) {
+        this.sdkKey = sdkKey;
+    }
+
     public boolean isRunning() {
         return started;
     }

core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java

@@ -1,6 +1,6 @@
 /**
  *
- *    Copyright 2019, Optimizely and contributors
+ *    Copyright 2019, 2023, Optimizely and contributors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -16,12 +16,33 @@
  */
 package com.optimizely.ab.config;
 
+import javax.annotation.Nullable;
+
 public interface ProjectConfigManager {
     /**
      * Implementations of this method should block until a datafile is available.
      *
      * @return ProjectConfig
      */
     ProjectConfig getConfig();
+
+    /**
+     * Implementations of this method should not block until a datafile is available, instead return current cached project configuration.
+     * return null if ProjectConfig is not ready at the moment.
+     *
+     * NOTE: To use ODP segments, implementation of this function is required to return current project configuration.
+     * @return ProjectConfig
+     */
+    @Nullable
+    ProjectConfig getCachedConfig();
+
+    /**
+     * Implementations of this method should return SDK key. If there is no SDKKey then it should return null.
+     *
+     * NOTE: To update ODP segments configuration via polling, it is required to return sdkKey.
+     * @return String
+     */
+    @Nullable
+    String getSDKKey();
 }
 

core-api/src/main/java/com/optimizely/ab/internal/NotificationRegistry.java

@@ -0,0 +1,52 @@
+/**
+ *
+ *    Copyright 2023, Optimizely
+ *
+ *    Licensed under the Apache License, Version 2.0 (the "License");
+ *    you may not use this file except in compliance with the License.
+ *    You may obtain a copy of the License at
+ *
+ *        http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *    Unless required by applicable law or agreed to in writing, software
+ *    distributed under the License is distributed on an "AS IS" BASIS,
+ *    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *    See the License for the specific language governing permissions and
+ *    limitations under the License.
+ */
+package com.optimizely.ab.internal;
+
+import com.optimizely.ab.notification.NotificationCenter;
+
+import javax.annotation.Nonnull;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+public class NotificationRegistry {
+    private final static Map<String, NotificationCenter> _notificationCenters = new ConcurrentHashMap<>();
+
+    private NotificationRegistry()
+    {
+    }
+
+    public static NotificationCenter getInternalNotificationCenter(@Nonnull String sdkKey)
+    {
+        NotificationCenter notificationCenter = null;
+        if (sdkKey != null) {
+            if (_notificationCenters.containsKey(sdkKey)) {
+                notificationCenter = _notificationCenters.get(sdkKey);
+            } else {
+                notificationCenter = new NotificationCenter();
+                _notificationCenters.put(sdkKey, notificationCenter);
+            }
+        }
+        return notificationCenter;
+    }
+
+    public static void clearNotificationCenterRegistry(@Nonnull String sdkKey) {
+        if (sdkKey != null) {
+            _notificationCenters.remove(sdkKey);
+        }
+    }
+
+}

core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java

@@ -1,5 +1,5 @@
 /****************************************************************************
- * Copyright 2016-2022, Optimizely, Inc. and contributors                   *
+ * Copyright 2016-2023, Optimizely, Inc. and contributors                   *
  *                                                                          *
  * Licensed under the Apache License, Version 2.0 (the "License");          *
  * you may not use this file except in compliance with the License.         *
@@ -119,6 +119,23 @@ public static Collection<Object[]> data() throws IOException {
     public OptimizelyRule optimizelyBuilder = new OptimizelyRule();
     public EventHandlerRule eventHandler = new EventHandlerRule();
 
+    public ProjectConfigManager projectConfigManagerReturningNull = new ProjectConfigManager() {
+        @Override
+        public ProjectConfig getConfig() {
+            return null;
+        }
+
+        @Override
+        public ProjectConfig getCachedConfig() {
+            return null;
+        }
+
+        @Override
+        public String getSDKKey() {
+            return null;
+        }
+    };
+
     @Rule
     @SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD")
     public RuleChain ruleChain = RuleChain.outerRule(thrown)
@@ -4505,13 +4522,13 @@ public void isValidReturnsTrueWhenClientIsValid() throws Exception {
 
     @Test
     public void testGetNotificationCenter() {
-        Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build();
+        Optimizely optimizely = optimizelyBuilder.withConfigManager(projectConfigManagerReturningNull).build();
         assertEquals(optimizely.notificationCenter, optimizely.getNotificationCenter());
     }
 
     @Test
     public void testAddTrackNotificationHandler() {
-        Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build();
+        Optimizely optimizely = optimizelyBuilder.withConfigManager(projectConfigManagerReturningNull).build();
         NotificationManager<TrackNotification> manager = optimizely.getNotificationCenter()
             .getNotificationManager(TrackNotification.class);
 
@@ -4521,7 +4538,7 @@ public void testAddTrackNotificationHandler() {
 
     @Test
     public void testAddDecisionNotificationHandler() {
-        Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build();
+        Optimizely optimizely = optimizelyBuilder.withConfigManager(projectConfigManagerReturningNull).build();
         NotificationManager<DecisionNotification> manager = optimizely.getNotificationCenter()
             .getNotificationManager(DecisionNotification.class);
 
@@ -4531,7 +4548,7 @@ public void testAddDecisionNotificationHandler() {
 
     @Test
     public void testAddUpdateConfigNotificationHandler() {
-        Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build();
+        Optimizely optimizely = optimizelyBuilder.withConfigManager(projectConfigManagerReturningNull).build();
         NotificationManager<UpdateConfigNotification> manager = optimizely.getNotificationCenter()
             .getNotificationManager(UpdateConfigNotification.class);
 
@@ -4541,7 +4558,7 @@ public void testAddUpdateConfigNotificationHandler() {
 
     @Test
     public void testAddLogEventNotificationHandler() {
-        Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build();
+        Optimizely optimizely = optimizelyBuilder.withConfigManager(projectConfigManagerReturningNull).build();
         NotificationManager<LogEvent> manager = optimizely.getNotificationCenter()
             .getNotificationManager(LogEvent.class);
 
@@ -4713,24 +4730,6 @@ public void initODPManagerWithProjectConfig() throws IOException {
         verify(mockODPManager, times(1)).updateSettings(any(), any(), any());
     }
 
-    @Test
-    public void updateODPManagerWhenConfigUpdates() throws IOException {
-        ODPEventManager mockODPEventManager = mock(ODPEventManager.class);
-        ODPManager mockODPManager = mock(ODPManager.class);
-        NotificationCenter mockNotificationCenter = mock(NotificationCenter.class);
-
-        Mockito.when(mockODPManager.getEventManager()).thenReturn(mockODPEventManager);
-        Optimizely.builder()
-            .withDatafile(validConfigJsonV4())
-            .withNotificationCenter(mockNotificationCenter)
-            .withODPManager(mockODPManager)
-            .build();
-
-        verify(mockODPManager, times(1)).updateSettings(any(), any(), any());
-
-        Mockito.verify(mockNotificationCenter, times(1)).addNotificationHandler(any(), any());
-    }
-
     @Test
     public void sendODPEvent() {
         ProjectConfigManager mockProjectConfigManager = mock(ProjectConfigManager.class);