From a006cf083ca12fb6cc66ade42f81698b8abdfa0f Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 11 Jan 2023 22:34:26 +0500 Subject: [PATCH 01/19] Added notificationRegistry to make sure that odpSettings update will call upon UpdateConfig --- .../java/com/optimizely/ab/Optimizely.java | 9 ++- .../ab/config/AtomicProjectConfigManager.java | 5 ++ .../config/PollingProjectConfigManager.java | 2 + .../ab/config/ProjectConfigManager.java | 2 + .../ab/internal/NotificationRegistry.java | 52 ++++++++++++++ .../com/optimizely/ab/OptimizelyTest.java | 42 +++++------ .../PollingProjectConfigManagerTest.java | 8 +++ .../ab/internal/NotificationRegistryTest.java | 70 +++++++++++++++++++ .../com/optimizely/ab/OptimizelyFactory.java | 13 +++- .../ab/config/HttpProjectConfigManager.java | 13 +++- .../optimizely/ab/OptimizelyFactoryTest.java | 39 ++++++++++- 11 files changed, 225 insertions(+), 30 deletions(-) create mode 100644 core-api/src/main/java/com/optimizely/ab/internal/NotificationRegistry.java create mode 100644 core-api/src/test/java/com/optimizely/ab/internal/NotificationRegistryTest.java diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 12b9f33d1..192863b57 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/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; @@ -127,7 +128,12 @@ private Optimizely(@Nonnull EventHandler eventHandler, if (getProjectConfig() != null) { updateODPSettings(); } - addUpdateConfigNotificationHandler(configNotification -> { updateODPSettings(); }); + if (projectConfigManager != null) { + NotificationRegistry.getNotificationCenter(projectConfigManager.getSDKKey()). + addNotificationHandler(UpdateConfigNotification.class, + configNotification -> { updateODPSettings(); }); + } + } } @@ -153,6 +159,7 @@ public void close() { tryClose(eventProcessor); tryClose(eventHandler); tryClose(projectConfigManager); + NotificationRegistry.clearNotificationCenterRegistry(); if (odpManager != null) { tryClose(odpManager); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java index fa1f4bd62..2674040e5 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java @@ -27,6 +27,11 @@ public ProjectConfig getConfig() { return projectConfigReference.get(); } + @Override + public String getSDKKey() { + return ""; + } + public void setConfig(ProjectConfig projectConfig) { projectConfigReference.set(projectConfig); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java index b03aeabc0..08d5adcc9 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java @@ -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; @@ -109,6 +110,7 @@ void setConfig(ProjectConfig projectConfig) { currentProjectConfig.set(projectConfig); currentOptimizelyConfig.set(new OptimizelyConfigService(projectConfig).getConfig()); countDownLatch.countDown(); + NotificationRegistry.getNotificationCenter(projectConfig.getSdkKey()).send(SIGNAL); notificationCenter.send(SIGNAL); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java index 1a1b2f4bc..299e42a11 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java @@ -23,5 +23,7 @@ public interface ProjectConfigManager { * @return ProjectConfig */ ProjectConfig getConfig(); + + String getSDKKey(); } diff --git a/core-api/src/main/java/com/optimizely/ab/internal/NotificationRegistry.java b/core-api/src/main/java/com/optimizely/ab/internal/NotificationRegistry.java new file mode 100644 index 000000000..18f34cb1c --- /dev/null +++ b/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 _notificationCenters = new ConcurrentHashMap<>(); + + private NotificationRegistry() + { + } + + public static NotificationCenter getNotificationCenter(@Nonnull String sdkKey) + { + if (sdkKey == null) { + sdkKey = ""; + } + + NotificationCenter notificationCenter; + if (_notificationCenters.containsKey(sdkKey)) { + notificationCenter = _notificationCenters.get(sdkKey); + } else { + notificationCenter = new NotificationCenter(); + _notificationCenters.put(sdkKey, notificationCenter); + } + return notificationCenter; + } + + public static void clearNotificationCenterRegistry() { + _notificationCenters.clear(); + } + +} diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index 9fd3dd675..6d6731ec8 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -4505,13 +4505,13 @@ public void isValidReturnsTrueWhenClientIsValid() throws Exception { @Test public void testGetNotificationCenter() { - Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build(); + Optimizely optimizely = optimizelyBuilder.withConfigManager(new TestProjectConfigManager()).build(); assertEquals(optimizely.notificationCenter, optimizely.getNotificationCenter()); } @Test public void testAddTrackNotificationHandler() { - Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build(); + Optimizely optimizely = optimizelyBuilder.withConfigManager(new TestProjectConfigManager()).build(); NotificationManager manager = optimizely.getNotificationCenter() .getNotificationManager(TrackNotification.class); @@ -4521,7 +4521,7 @@ public void testAddTrackNotificationHandler() { @Test public void testAddDecisionNotificationHandler() { - Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build(); + Optimizely optimizely = optimizelyBuilder.withConfigManager(new TestProjectConfigManager()).build(); NotificationManager manager = optimizely.getNotificationCenter() .getNotificationManager(DecisionNotification.class); @@ -4531,7 +4531,7 @@ public void testAddDecisionNotificationHandler() { @Test public void testAddUpdateConfigNotificationHandler() { - Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build(); + Optimizely optimizely = optimizelyBuilder.withConfigManager(new TestProjectConfigManager()).build(); NotificationManager manager = optimizely.getNotificationCenter() .getNotificationManager(UpdateConfigNotification.class); @@ -4541,7 +4541,7 @@ public void testAddUpdateConfigNotificationHandler() { @Test public void testAddLogEventNotificationHandler() { - Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build(); + Optimizely optimizely = optimizelyBuilder.withConfigManager(new TestProjectConfigManager()).build(); NotificationManager manager = optimizely.getNotificationCenter() .getNotificationManager(LogEvent.class); @@ -4713,24 +4713,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); @@ -4798,4 +4780,18 @@ public void identifyUser() { optimizely.identifyUser("the-user"); Mockito.verify(mockODPEventManager, times(1)).identifyUser("the-user"); } + + public static class TestProjectConfigManager implements ProjectConfigManager { + + @Override + public ProjectConfig getConfig() { + return null; + } + + @Override + public String getSDKKey() { + return null; + } + } } + diff --git a/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java b/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java index 390c9b874..2314f2c55 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java @@ -271,5 +271,13 @@ public int getCount() { public void release() { countDownLatch.countDown(); } + + @Override + public String getSDKKey() { + if (projectConfig == null) { + return ""; + } + return projectConfig.getSdkKey(); + } } } diff --git a/core-api/src/test/java/com/optimizely/ab/internal/NotificationRegistryTest.java b/core-api/src/test/java/com/optimizely/ab/internal/NotificationRegistryTest.java new file mode 100644 index 000000000..3fb641110 --- /dev/null +++ b/core-api/src/test/java/com/optimizely/ab/internal/NotificationRegistryTest.java @@ -0,0 +1,70 @@ +/** + * + * 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 org.junit.Assert; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + + +public class NotificationRegistryTest { + @Test + public void getSameNotificationcenterWhenSDKkeyIsNull() { + String sdkKey = null; + NotificationCenter notificationCenter1 = NotificationRegistry.getNotificationCenter(sdkKey); + NotificationCenter notificationCenter2 = NotificationRegistry.getNotificationCenter(sdkKey); + assertEquals(notificationCenter1, notificationCenter2); + } + + @Test + public void getSameNotificationcenterWhenSDKkeyIsSameButNotNull() { + String sdkKey = "testSDkKey"; + NotificationCenter notificationCenter1 = NotificationRegistry.getNotificationCenter(sdkKey); + NotificationCenter notificationCenter2 = NotificationRegistry.getNotificationCenter(sdkKey); + assertEquals(notificationCenter1, notificationCenter2); + } + + @Test + public void getSameNotificationcenterWhenSDKkeyIsNullAndAnotherIsEmpty() { + String sdkKey1 = ""; + String sdkKey2 = null; + NotificationCenter notificationCenter1 = NotificationRegistry.getNotificationCenter(sdkKey1); + NotificationCenter notificationCenter2 = NotificationRegistry.getNotificationCenter(sdkKey2); + assertEquals(notificationCenter1, notificationCenter2); + } + + @Test + public void getDifferentNotificationcenterWhenSDKkeyIsNotSame() { + String sdkKey1 = "testSDkKey1"; + String sdkKey2 = "testSDkKey2"; + NotificationCenter notificationCenter1 = NotificationRegistry.getNotificationCenter(sdkKey1); + NotificationCenter notificationCenter2 = NotificationRegistry.getNotificationCenter(sdkKey2); + Assert.assertNotEquals(notificationCenter1, notificationCenter2); + } + + @Test + public void clearRegistryNotificationcenterClearsOldNotificationCenter() { + String sdkKey1 = "testSDkKey1"; + NotificationCenter notificationCenter1 = NotificationRegistry.getNotificationCenter(sdkKey1); + NotificationRegistry.clearNotificationCenterRegistry(); + NotificationCenter notificationCenter2 = NotificationRegistry.getNotificationCenter(sdkKey1); + + Assert.assertNotEquals(notificationCenter1, notificationCenter2); + } +} diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java index 37d56da03..e9c47da49 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java @@ -17,6 +17,7 @@ package com.optimizely.ab; import com.optimizely.ab.config.HttpProjectConfigManager; +import com.optimizely.ab.config.ProjectConfig; import com.optimizely.ab.config.ProjectConfigManager; import com.optimizely.ab.event.AsyncEventHandler; import com.optimizely.ab.event.BatchEventProcessor; @@ -222,7 +223,17 @@ public static Optimizely newDefaultInstance() { public static Optimizely newDefaultInstance(String sdkKey) { if (sdkKey == null) { logger.error("Must provide an sdkKey, returning non-op Optimizely client"); - return newDefaultInstance(() -> null); + return newDefaultInstance(new ProjectConfigManager() { + @Override + public ProjectConfig getConfig() { + return null; + } + + @Override + public String getSDKKey() { + return null; + } + }); } return newDefaultInstance(sdkKey, null); diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java index cef13fdcd..2c0d30687 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java @@ -65,6 +65,7 @@ public class HttpProjectConfigManager extends PollingProjectConfigManager { private final URI uri; private final String datafileAccessToken; private String datafileLastModified; + private String sdkKey; private HttpProjectConfigManager(long period, TimeUnit timeUnit, @@ -73,11 +74,13 @@ private HttpProjectConfigManager(long period, String datafileAccessToken, long blockingTimeoutPeriod, TimeUnit blockingTimeoutUnit, - NotificationCenter notificationCenter) { + NotificationCenter notificationCenter, + String sdkKey) { super(period, timeUnit, blockingTimeoutPeriod, blockingTimeoutUnit, notificationCenter); this.httpClient = httpClient; this.uri = URI.create(url); this.datafileAccessToken = datafileAccessToken; + this.sdkKey = sdkKey; } public URI getUri() { @@ -167,6 +170,11 @@ public static Builder builder() { return new Builder(); } + @Override + public String getSDKKey() { + return this.sdkKey; + } + public static class Builder { private String datafile; private String url; @@ -357,7 +365,8 @@ public HttpProjectConfigManager build(boolean defer) { datafileAccessToken, blockingTimeoutPeriod, blockingTimeoutUnit, - notificationCenter); + notificationCenter, + sdkKey); if (datafile != null) { try { diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java index 07c2c0634..5c3aa88a1 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java @@ -19,6 +19,8 @@ import com.google.common.base.Charsets; import com.google.common.io.Resources; import com.optimizely.ab.config.HttpProjectConfigManager; +import com.optimizely.ab.config.ProjectConfig; +import com.optimizely.ab.config.ProjectConfigManager; import com.optimizely.ab.event.AsyncEventHandler; import com.optimizely.ab.event.BatchEventProcessor; import com.optimizely.ab.internal.PropertyUtils; @@ -263,14 +265,34 @@ public void newDefaultInstanceWithDatafileAccessTokenAndCustomHttpClient() throw @Test public void newDefaultInstanceWithProjectConfig() throws Exception { - optimizely = OptimizelyFactory.newDefaultInstance(() -> null); + optimizely = OptimizelyFactory.newDefaultInstance(new ProjectConfigManager() { + @Override + public ProjectConfig getConfig() { + return null; + } + + @Override + public String getSDKKey() { + return null; + } + }); assertFalse(optimizely.isValid()); } @Test public void newDefaultInstanceWithProjectConfigAndNotificationCenter() throws Exception { NotificationCenter notificationCenter = new NotificationCenter(); - optimizely = OptimizelyFactory.newDefaultInstance(() -> null, notificationCenter); + optimizely = OptimizelyFactory.newDefaultInstance(new ProjectConfigManager() { + @Override + public ProjectConfig getConfig() { + return null; + } + + @Override + public String getSDKKey() { + return null; + } + }, notificationCenter); assertFalse(optimizely.isValid()); assertEquals(notificationCenter, optimizely.getNotificationCenter()); } @@ -278,7 +300,18 @@ public void newDefaultInstanceWithProjectConfigAndNotificationCenter() throws Ex @Test public void newDefaultInstanceWithProjectConfigAndNotificationCenterAndEventHandler() { NotificationCenter notificationCenter = new NotificationCenter(); - optimizely = OptimizelyFactory.newDefaultInstance(() -> null, notificationCenter, logEvent -> {}); + optimizely = OptimizelyFactory.newDefaultInstance(new ProjectConfigManager() { + @Override + public ProjectConfig getConfig() { + return null; + } + + @Override + public String getSDKKey() { + return null; + } + }, notificationCenter, logEvent -> { + }); assertFalse(optimizely.isValid()); assertEquals(notificationCenter, optimizely.getNotificationCenter()); } From ed2bcbbb45e897331aa5768f7fa1dcd2b8cb2d54 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 11 Jan 2023 23:02:03 +0500 Subject: [PATCH 02/19] Updated headers and added additional checks in unittests --- .../ab/config/AtomicProjectConfigManager.java | 2 +- .../ab/config/PollingProjectConfigManager.java | 2 +- .../ab/config/ProjectConfigManager.java | 2 +- .../java/com/optimizely/ab/OptimizelyTest.java | 2 +- .../config/PollingProjectConfigManagerTest.java | 16 +++++++++++++--- .../ab/config/HttpProjectConfigManagerTest.java | 3 ++- 6 files changed, 19 insertions(+), 8 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java index 2674040e5..75e42b4f6 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java +++ b/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. diff --git a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java index 08d5adcc9..f1c3e7620 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java +++ b/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. diff --git a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java index 299e42a11..c4860da2a 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java +++ b/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. diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index 6d6731ec8..384e8f521 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/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. * diff --git a/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java b/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java index 2314f2c55..829a7a44d 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java @@ -1,6 +1,6 @@ /** * - * Copyright 2019-2021, Optimizely and contributors + * Copyright 2019-2021, 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 org.junit.After; @@ -95,12 +96,14 @@ public void testBlockingGetConfig() throws Exception { testProjectConfigManager.release(); TimeUnit.MILLISECONDS.sleep(PROJECT_CONFIG_DELAY); assertEquals(projectConfig, testProjectConfigManager.getConfig()); + assertEquals(projectConfig.getSdkKey(), testProjectConfigManager.getSDKKey()); } @Test public void testBlockingGetConfigWithDefault() throws Exception { testProjectConfigManager.setConfig(projectConfig); assertEquals(projectConfig, testProjectConfigManager.getConfig()); + assertEquals(projectConfig.getSdkKey(), testProjectConfigManager.getSDKKey()); } @Test @@ -124,6 +127,7 @@ public void testGetConfigNotStartedDefault() throws Exception { testProjectConfigManager.close(); assertFalse(testProjectConfigManager.isRunning()); assertEquals(projectConfig, testProjectConfigManager.getConfig()); + assertEquals(projectConfig.getSdkKey(), testProjectConfigManager.getSDKKey()); } @Test @@ -210,11 +214,17 @@ public ProjectConfig poll() { @Test public void testUpdateConfigNotificationGetsTriggered() throws InterruptedException { - CountDownLatch countDownLatch = new CountDownLatch(1); + CountDownLatch countDownLatch = new CountDownLatch(2); + NotificationCenter registryDefaultNotificationCenter = NotificationRegistry.getNotificationCenter("ValidProjectConfigV4"); + NotificationCenter userNotificationCenter = testProjectConfigManager.getNotificationCenter(); + assertNotEquals(registryDefaultNotificationCenter, userNotificationCenter); + testProjectConfigManager.getNotificationCenter() .getNotificationManager(UpdateConfigNotification.class) .addHandler(message -> {countDownLatch.countDown();}); - + NotificationRegistry.getNotificationCenter("ValidProjectConfigV4") + .getNotificationManager(UpdateConfigNotification.class) + .addHandler(message -> {countDownLatch.countDown();}); assertTrue(countDownLatch.await(500, TimeUnit.MILLISECONDS)); } diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java index c61a1f01a..317348aba 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java @@ -1,6 +1,6 @@ /** * - * Copyright 2019, Optimizely + * Copyright 2019, 2023, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -207,6 +207,7 @@ public void testBuildDefer() throws Exception { .withOptimizelyHttpClient(mockHttpClient) .withSdkKey("sdk-key") .build(true); + assertEquals("sdk-key", projectConfigManager.getSDKKey()); } @Test From 51357cf8841f6aaa064e34fd4776ecc75fb5d66a Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 12 Jan 2023 00:31:54 +0500 Subject: [PATCH 03/19] Added suppress warning --- .../com/optimizely/ab/internal/NotificationRegistryTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core-api/src/test/java/com/optimizely/ab/internal/NotificationRegistryTest.java b/core-api/src/test/java/com/optimizely/ab/internal/NotificationRegistryTest.java index 3fb641110..0e550ebdf 100644 --- a/core-api/src/test/java/com/optimizely/ab/internal/NotificationRegistryTest.java +++ b/core-api/src/test/java/com/optimizely/ab/internal/NotificationRegistryTest.java @@ -17,6 +17,7 @@ package com.optimizely.ab.internal; import com.optimizely.ab.notification.NotificationCenter; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.junit.Assert; import org.junit.Test; @@ -24,6 +25,8 @@ public class NotificationRegistryTest { + + @SuppressFBWarnings("NP_NONNULL_PARAM_VIOLATION") @Test public void getSameNotificationcenterWhenSDKkeyIsNull() { String sdkKey = null; @@ -40,6 +43,7 @@ public void getSameNotificationcenterWhenSDKkeyIsSameButNotNull() { assertEquals(notificationCenter1, notificationCenter2); } + @SuppressFBWarnings("NP_NONNULL_PARAM_VIOLATION") @Test public void getSameNotificationcenterWhenSDKkeyIsNullAndAnotherIsEmpty() { String sdkKey1 = ""; From f1c3d7aa4802f781d4fbf6ca9cb94683bd2cfe70 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 12 Jan 2023 16:16:24 +0500 Subject: [PATCH 04/19] renamed NoficationRegistry function. Added default implementation of getSDKKey in ProjectConfigManager --- .../java/com/optimizely/ab/Optimizely.java | 2 +- .../ab/config/AtomicProjectConfigManager.java | 5 ----- .../config/PollingProjectConfigManager.java | 2 +- .../ab/config/ProjectConfigManager.java | 4 +++- .../ab/internal/NotificationRegistry.java | 2 +- .../PollingProjectConfigManagerTest.java | 6 +++--- .../ab/internal/NotificationRegistryTest.java | 20 +++++++++---------- 7 files changed, 19 insertions(+), 22 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 192863b57..6569da121 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -129,7 +129,7 @@ private Optimizely(@Nonnull EventHandler eventHandler, updateODPSettings(); } if (projectConfigManager != null) { - NotificationRegistry.getNotificationCenter(projectConfigManager.getSDKKey()). + NotificationRegistry.getInternalNotificationCenter(projectConfigManager.getSDKKey()). addNotificationHandler(UpdateConfigNotification.class, configNotification -> { updateODPSettings(); }); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java index 75e42b4f6..1c75210db 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java @@ -27,11 +27,6 @@ public ProjectConfig getConfig() { return projectConfigReference.get(); } - @Override - public String getSDKKey() { - return ""; - } - public void setConfig(ProjectConfig projectConfig) { projectConfigReference.set(projectConfig); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java index f1c3e7620..b7adb1c83 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java @@ -110,7 +110,7 @@ void setConfig(ProjectConfig projectConfig) { currentProjectConfig.set(projectConfig); currentOptimizelyConfig.set(new OptimizelyConfigService(projectConfig).getConfig()); countDownLatch.countDown(); - NotificationRegistry.getNotificationCenter(projectConfig.getSdkKey()).send(SIGNAL); + NotificationRegistry.getInternalNotificationCenter(projectConfig.getSdkKey()).send(SIGNAL); notificationCenter.send(SIGNAL); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java index c4860da2a..4fae41ff5 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java @@ -24,6 +24,8 @@ public interface ProjectConfigManager { */ ProjectConfig getConfig(); - String getSDKKey(); + default String getSDKKey() { + return null; + } } diff --git a/core-api/src/main/java/com/optimizely/ab/internal/NotificationRegistry.java b/core-api/src/main/java/com/optimizely/ab/internal/NotificationRegistry.java index 18f34cb1c..428cc214f 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/NotificationRegistry.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/NotificationRegistry.java @@ -29,7 +29,7 @@ private NotificationRegistry() { } - public static NotificationCenter getNotificationCenter(@Nonnull String sdkKey) + public static NotificationCenter getInternalNotificationCenter(@Nonnull String sdkKey) { if (sdkKey == null) { sdkKey = ""; diff --git a/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java b/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java index 829a7a44d..130f8844a 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/PollingProjectConfigManagerTest.java @@ -215,14 +215,14 @@ public ProjectConfig poll() { @Test public void testUpdateConfigNotificationGetsTriggered() throws InterruptedException { CountDownLatch countDownLatch = new CountDownLatch(2); - NotificationCenter registryDefaultNotificationCenter = NotificationRegistry.getNotificationCenter("ValidProjectConfigV4"); + NotificationCenter registryDefaultNotificationCenter = NotificationRegistry.getInternalNotificationCenter("ValidProjectConfigV4"); NotificationCenter userNotificationCenter = testProjectConfigManager.getNotificationCenter(); assertNotEquals(registryDefaultNotificationCenter, userNotificationCenter); testProjectConfigManager.getNotificationCenter() .getNotificationManager(UpdateConfigNotification.class) .addHandler(message -> {countDownLatch.countDown();}); - NotificationRegistry.getNotificationCenter("ValidProjectConfigV4") + NotificationRegistry.getInternalNotificationCenter("ValidProjectConfigV4") .getNotificationManager(UpdateConfigNotification.class) .addHandler(message -> {countDownLatch.countDown();}); assertTrue(countDownLatch.await(500, TimeUnit.MILLISECONDS)); @@ -285,7 +285,7 @@ public void release() { @Override public String getSDKKey() { if (projectConfig == null) { - return ""; + return null; } return projectConfig.getSdkKey(); } diff --git a/core-api/src/test/java/com/optimizely/ab/internal/NotificationRegistryTest.java b/core-api/src/test/java/com/optimizely/ab/internal/NotificationRegistryTest.java index 0e550ebdf..10f2e99e8 100644 --- a/core-api/src/test/java/com/optimizely/ab/internal/NotificationRegistryTest.java +++ b/core-api/src/test/java/com/optimizely/ab/internal/NotificationRegistryTest.java @@ -30,16 +30,16 @@ public class NotificationRegistryTest { @Test public void getSameNotificationcenterWhenSDKkeyIsNull() { String sdkKey = null; - NotificationCenter notificationCenter1 = NotificationRegistry.getNotificationCenter(sdkKey); - NotificationCenter notificationCenter2 = NotificationRegistry.getNotificationCenter(sdkKey); + NotificationCenter notificationCenter1 = NotificationRegistry.getInternalNotificationCenter(sdkKey); + NotificationCenter notificationCenter2 = NotificationRegistry.getInternalNotificationCenter(sdkKey); assertEquals(notificationCenter1, notificationCenter2); } @Test public void getSameNotificationcenterWhenSDKkeyIsSameButNotNull() { String sdkKey = "testSDkKey"; - NotificationCenter notificationCenter1 = NotificationRegistry.getNotificationCenter(sdkKey); - NotificationCenter notificationCenter2 = NotificationRegistry.getNotificationCenter(sdkKey); + NotificationCenter notificationCenter1 = NotificationRegistry.getInternalNotificationCenter(sdkKey); + NotificationCenter notificationCenter2 = NotificationRegistry.getInternalNotificationCenter(sdkKey); assertEquals(notificationCenter1, notificationCenter2); } @@ -48,8 +48,8 @@ public void getSameNotificationcenterWhenSDKkeyIsSameButNotNull() { public void getSameNotificationcenterWhenSDKkeyIsNullAndAnotherIsEmpty() { String sdkKey1 = ""; String sdkKey2 = null; - NotificationCenter notificationCenter1 = NotificationRegistry.getNotificationCenter(sdkKey1); - NotificationCenter notificationCenter2 = NotificationRegistry.getNotificationCenter(sdkKey2); + NotificationCenter notificationCenter1 = NotificationRegistry.getInternalNotificationCenter(sdkKey1); + NotificationCenter notificationCenter2 = NotificationRegistry.getInternalNotificationCenter(sdkKey2); assertEquals(notificationCenter1, notificationCenter2); } @@ -57,17 +57,17 @@ public void getSameNotificationcenterWhenSDKkeyIsNullAndAnotherIsEmpty() { public void getDifferentNotificationcenterWhenSDKkeyIsNotSame() { String sdkKey1 = "testSDkKey1"; String sdkKey2 = "testSDkKey2"; - NotificationCenter notificationCenter1 = NotificationRegistry.getNotificationCenter(sdkKey1); - NotificationCenter notificationCenter2 = NotificationRegistry.getNotificationCenter(sdkKey2); + NotificationCenter notificationCenter1 = NotificationRegistry.getInternalNotificationCenter(sdkKey1); + NotificationCenter notificationCenter2 = NotificationRegistry.getInternalNotificationCenter(sdkKey2); Assert.assertNotEquals(notificationCenter1, notificationCenter2); } @Test public void clearRegistryNotificationcenterClearsOldNotificationCenter() { String sdkKey1 = "testSDkKey1"; - NotificationCenter notificationCenter1 = NotificationRegistry.getNotificationCenter(sdkKey1); + NotificationCenter notificationCenter1 = NotificationRegistry.getInternalNotificationCenter(sdkKey1); NotificationRegistry.clearNotificationCenterRegistry(); - NotificationCenter notificationCenter2 = NotificationRegistry.getNotificationCenter(sdkKey1); + NotificationCenter notificationCenter2 = NotificationRegistry.getInternalNotificationCenter(sdkKey1); Assert.assertNotEquals(notificationCenter1, notificationCenter2); } From 5d791f25bb9c685409925cea098ccd32e5101ad6 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 12 Jan 2023 16:17:57 +0500 Subject: [PATCH 05/19] reverting header change --- .../com/optimizely/ab/config/AtomicProjectConfigManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java index 1c75210db..fa1f4bd62 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java @@ -1,6 +1,6 @@ /** * - * Copyright 2019, 2023, Optimizely and contributors + * Copyright 2019, 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. From 01fd113c93b9cf38b491649111c646f0166579f7 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 12 Jan 2023 16:22:39 +0500 Subject: [PATCH 06/19] reverting extra changes --- .../com/optimizely/ab/OptimizelyFactory.java | 13 +------ .../optimizely/ab/OptimizelyFactoryTest.java | 38 ++----------------- 2 files changed, 4 insertions(+), 47 deletions(-) diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java index e9c47da49..37d56da03 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java @@ -17,7 +17,6 @@ package com.optimizely.ab; import com.optimizely.ab.config.HttpProjectConfigManager; -import com.optimizely.ab.config.ProjectConfig; import com.optimizely.ab.config.ProjectConfigManager; import com.optimizely.ab.event.AsyncEventHandler; import com.optimizely.ab.event.BatchEventProcessor; @@ -223,17 +222,7 @@ public static Optimizely newDefaultInstance() { public static Optimizely newDefaultInstance(String sdkKey) { if (sdkKey == null) { logger.error("Must provide an sdkKey, returning non-op Optimizely client"); - return newDefaultInstance(new ProjectConfigManager() { - @Override - public ProjectConfig getConfig() { - return null; - } - - @Override - public String getSDKKey() { - return null; - } - }); + return newDefaultInstance(() -> null); } return newDefaultInstance(sdkKey, null); diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java index 5c3aa88a1..7a871caa2 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java @@ -19,8 +19,6 @@ import com.google.common.base.Charsets; import com.google.common.io.Resources; import com.optimizely.ab.config.HttpProjectConfigManager; -import com.optimizely.ab.config.ProjectConfig; -import com.optimizely.ab.config.ProjectConfigManager; import com.optimizely.ab.event.AsyncEventHandler; import com.optimizely.ab.event.BatchEventProcessor; import com.optimizely.ab.internal.PropertyUtils; @@ -265,34 +263,14 @@ public void newDefaultInstanceWithDatafileAccessTokenAndCustomHttpClient() throw @Test public void newDefaultInstanceWithProjectConfig() throws Exception { - optimizely = OptimizelyFactory.newDefaultInstance(new ProjectConfigManager() { - @Override - public ProjectConfig getConfig() { - return null; - } - - @Override - public String getSDKKey() { - return null; - } - }); + optimizely = OptimizelyFactory.newDefaultInstance(() -> null); assertFalse(optimizely.isValid()); } @Test public void newDefaultInstanceWithProjectConfigAndNotificationCenter() throws Exception { NotificationCenter notificationCenter = new NotificationCenter(); - optimizely = OptimizelyFactory.newDefaultInstance(new ProjectConfigManager() { - @Override - public ProjectConfig getConfig() { - return null; - } - - @Override - public String getSDKKey() { - return null; - } - }, notificationCenter); + optimizely = OptimizelyFactory.newDefaultInstance(() -> null, notificationCenter); assertFalse(optimizely.isValid()); assertEquals(notificationCenter, optimizely.getNotificationCenter()); } @@ -300,17 +278,7 @@ public String getSDKKey() { @Test public void newDefaultInstanceWithProjectConfigAndNotificationCenterAndEventHandler() { NotificationCenter notificationCenter = new NotificationCenter(); - optimizely = OptimizelyFactory.newDefaultInstance(new ProjectConfigManager() { - @Override - public ProjectConfig getConfig() { - return null; - } - - @Override - public String getSDKKey() { - return null; - } - }, notificationCenter, logEvent -> { + optimizely = OptimizelyFactory.newDefaultInstance(() -> null, notificationCenter, logEvent -> { }); assertFalse(optimizely.isValid()); assertEquals(notificationCenter, optimizely.getNotificationCenter()); From 9c4f72919252be86c17eec12e91eec5b3b6ac543 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 12 Jan 2023 16:25:07 +0500 Subject: [PATCH 07/19] reverting extra changes --- .../com/optimizely/ab/OptimizelyTest.java | 24 +++++-------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index 384e8f521..f36768565 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2016-2023, Optimizely, Inc. and contributors * + * Copyright 2016-2022, 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. * @@ -4505,13 +4505,13 @@ public void isValidReturnsTrueWhenClientIsValid() throws Exception { @Test public void testGetNotificationCenter() { - Optimizely optimizely = optimizelyBuilder.withConfigManager(new TestProjectConfigManager()).build(); + Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build(); assertEquals(optimizely.notificationCenter, optimizely.getNotificationCenter()); } @Test public void testAddTrackNotificationHandler() { - Optimizely optimizely = optimizelyBuilder.withConfigManager(new TestProjectConfigManager()).build(); + Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build(); NotificationManager manager = optimizely.getNotificationCenter() .getNotificationManager(TrackNotification.class); @@ -4521,7 +4521,7 @@ public void testAddTrackNotificationHandler() { @Test public void testAddDecisionNotificationHandler() { - Optimizely optimizely = optimizelyBuilder.withConfigManager(new TestProjectConfigManager()).build(); + Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build(); NotificationManager manager = optimizely.getNotificationCenter() .getNotificationManager(DecisionNotification.class); @@ -4531,7 +4531,7 @@ public void testAddDecisionNotificationHandler() { @Test public void testAddUpdateConfigNotificationHandler() { - Optimizely optimizely = optimizelyBuilder.withConfigManager(new TestProjectConfigManager()).build(); + Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build(); NotificationManager manager = optimizely.getNotificationCenter() .getNotificationManager(UpdateConfigNotification.class); @@ -4541,7 +4541,7 @@ public void testAddUpdateConfigNotificationHandler() { @Test public void testAddLogEventNotificationHandler() { - Optimizely optimizely = optimizelyBuilder.withConfigManager(new TestProjectConfigManager()).build(); + Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build(); NotificationManager manager = optimizely.getNotificationCenter() .getNotificationManager(LogEvent.class); @@ -4781,17 +4781,5 @@ public void identifyUser() { Mockito.verify(mockODPEventManager, times(1)).identifyUser("the-user"); } - public static class TestProjectConfigManager implements ProjectConfigManager { - - @Override - public ProjectConfig getConfig() { - return null; - } - - @Override - public String getSDKKey() { - return null; - } - } } From 92004cf3788673cb338f51f76ad5695015780580 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 12 Jan 2023 16:26:17 +0500 Subject: [PATCH 08/19] nit fix --- core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index f36768565..c266f4d07 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/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. * @@ -4780,6 +4780,4 @@ public void identifyUser() { optimizely.identifyUser("the-user"); Mockito.verify(mockODPEventManager, times(1)).identifyUser("the-user"); } - } - From 01313a4004b31a058531d1e14d28245c79d7798f Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 12 Jan 2023 16:28:15 +0500 Subject: [PATCH 09/19] nit --- .../src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java index 7a871caa2..07c2c0634 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java @@ -278,8 +278,7 @@ public void newDefaultInstanceWithProjectConfigAndNotificationCenter() throws Ex @Test public void newDefaultInstanceWithProjectConfigAndNotificationCenterAndEventHandler() { NotificationCenter notificationCenter = new NotificationCenter(); - optimizely = OptimizelyFactory.newDefaultInstance(() -> null, notificationCenter, logEvent -> { - }); + optimizely = OptimizelyFactory.newDefaultInstance(() -> null, notificationCenter, logEvent -> {}); assertFalse(optimizely.isValid()); assertEquals(notificationCenter, optimizely.getNotificationCenter()); } From ecb12b5e89f0c9d20f1a4081b306aa81dc49d267 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Mon, 16 Jan 2023 14:44:41 +0500 Subject: [PATCH 10/19] Resolved comments --- .../java/com/optimizely/ab/Optimizely.java | 7 ++-- .../config/PollingProjectConfigManager.java | 4 ++- .../ab/internal/NotificationRegistry.java | 24 +++++++------- .../ab/internal/NotificationRegistryTest.java | 32 ++++++++++++------- 4 files changed, 40 insertions(+), 27 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 6569da121..824556eab 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -125,10 +125,10 @@ private Optimizely(@Nonnull EventHandler eventHandler, if (odpManager != null) { odpManager.getEventManager().start(); - if (getProjectConfig() != null) { + if (!(projectConfigManager instanceof PollingProjectConfigManager)) { updateODPSettings(); } - if (projectConfigManager != null) { + if (projectConfigManager != null && projectConfigManager.getSDKKey() != null) { NotificationRegistry.getInternalNotificationCenter(projectConfigManager.getSDKKey()). addNotificationHandler(UpdateConfigNotification.class, configNotification -> { updateODPSettings(); }); @@ -159,7 +159,8 @@ public void close() { tryClose(eventProcessor); tryClose(eventHandler); tryClose(projectConfigManager); - NotificationRegistry.clearNotificationCenterRegistry(); + notificationCenter.clearAllNotificationListeners(); + NotificationRegistry.clearNotificationCenterRegistry(projectConfigManager.getSDKKey()); if (odpManager != null) { tryClose(odpManager); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java index b7adb1c83..78a944ccb 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java @@ -110,7 +110,9 @@ void setConfig(ProjectConfig projectConfig) { currentProjectConfig.set(projectConfig); currentOptimizelyConfig.set(new OptimizelyConfigService(projectConfig).getConfig()); countDownLatch.countDown(); - NotificationRegistry.getInternalNotificationCenter(projectConfig.getSdkKey()).send(SIGNAL); + if (projectConfig.getSdkKey() != null) { + NotificationRegistry.getInternalNotificationCenter(projectConfig.getSdkKey()).send(SIGNAL); + } notificationCenter.send(SIGNAL); } diff --git a/core-api/src/main/java/com/optimizely/ab/internal/NotificationRegistry.java b/core-api/src/main/java/com/optimizely/ab/internal/NotificationRegistry.java index 428cc214f..92d0c6d38 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/NotificationRegistry.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/NotificationRegistry.java @@ -31,22 +31,22 @@ private NotificationRegistry() public static NotificationCenter getInternalNotificationCenter(@Nonnull String sdkKey) { - if (sdkKey == null) { - sdkKey = ""; - } - - NotificationCenter notificationCenter; - if (_notificationCenters.containsKey(sdkKey)) { - notificationCenter = _notificationCenters.get(sdkKey); - } else { - notificationCenter = new NotificationCenter(); - _notificationCenters.put(sdkKey, notificationCenter); + 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() { - _notificationCenters.clear(); + public static void clearNotificationCenterRegistry(@Nonnull String sdkKey) { + if (sdkKey != null) { + _notificationCenters.remove(sdkKey); + } } } diff --git a/core-api/src/test/java/com/optimizely/ab/internal/NotificationRegistryTest.java b/core-api/src/test/java/com/optimizely/ab/internal/NotificationRegistryTest.java index 10f2e99e8..4f130a848 100644 --- a/core-api/src/test/java/com/optimizely/ab/internal/NotificationRegistryTest.java +++ b/core-api/src/test/java/com/optimizely/ab/internal/NotificationRegistryTest.java @@ -22,39 +22,38 @@ import org.junit.Test; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; public class NotificationRegistryTest { @SuppressFBWarnings("NP_NONNULL_PARAM_VIOLATION") @Test - public void getSameNotificationcenterWhenSDKkeyIsNull() { + public void getNullNotificationCenterWhenSDKeyIsNull() { String sdkKey = null; - NotificationCenter notificationCenter1 = NotificationRegistry.getInternalNotificationCenter(sdkKey); - NotificationCenter notificationCenter2 = NotificationRegistry.getInternalNotificationCenter(sdkKey); - assertEquals(notificationCenter1, notificationCenter2); + NotificationCenter notificationCenter = NotificationRegistry.getInternalNotificationCenter(sdkKey); + assertNull(notificationCenter); } @Test - public void getSameNotificationcenterWhenSDKkeyIsSameButNotNull() { + public void getSameNotificationCenterWhenSDKKeyIsSameButNotNull() { String sdkKey = "testSDkKey"; NotificationCenter notificationCenter1 = NotificationRegistry.getInternalNotificationCenter(sdkKey); NotificationCenter notificationCenter2 = NotificationRegistry.getInternalNotificationCenter(sdkKey); assertEquals(notificationCenter1, notificationCenter2); } - @SuppressFBWarnings("NP_NONNULL_PARAM_VIOLATION") @Test - public void getSameNotificationcenterWhenSDKkeyIsNullAndAnotherIsEmpty() { + public void getSameNotificationCenterWhenSDKKeyIsEmpty() { String sdkKey1 = ""; - String sdkKey2 = null; + String sdkKey2 = ""; NotificationCenter notificationCenter1 = NotificationRegistry.getInternalNotificationCenter(sdkKey1); NotificationCenter notificationCenter2 = NotificationRegistry.getInternalNotificationCenter(sdkKey2); assertEquals(notificationCenter1, notificationCenter2); } @Test - public void getDifferentNotificationcenterWhenSDKkeyIsNotSame() { + public void getDifferentNotificationCenterWhenSDKKeyIsNotSame() { String sdkKey1 = "testSDkKey1"; String sdkKey2 = "testSDkKey2"; NotificationCenter notificationCenter1 = NotificationRegistry.getInternalNotificationCenter(sdkKey1); @@ -63,12 +62,23 @@ public void getDifferentNotificationcenterWhenSDKkeyIsNotSame() { } @Test - public void clearRegistryNotificationcenterClearsOldNotificationCenter() { + public void clearRegistryNotificationCenterClearsOldNotificationCenter() { String sdkKey1 = "testSDkKey1"; NotificationCenter notificationCenter1 = NotificationRegistry.getInternalNotificationCenter(sdkKey1); - NotificationRegistry.clearNotificationCenterRegistry(); + NotificationRegistry.clearNotificationCenterRegistry(sdkKey1); NotificationCenter notificationCenter2 = NotificationRegistry.getInternalNotificationCenter(sdkKey1); Assert.assertNotEquals(notificationCenter1, notificationCenter2); } + + @SuppressFBWarnings("NP_NONNULL_PARAM_VIOLATION") + @Test + public void clearRegistryNotificationCenterWillNotCauseExceptionIfPassedNullSDkKey() { + String sdkKey1 = "testSDkKey1"; + NotificationCenter notificationCenter1 = NotificationRegistry.getInternalNotificationCenter(sdkKey1); + NotificationRegistry.clearNotificationCenterRegistry(null); + NotificationCenter notificationCenter2 = NotificationRegistry.getInternalNotificationCenter(sdkKey1); + + Assert.assertEquals(notificationCenter1, notificationCenter2); + } } From 38d735a865abfae89ceac3fd5baf25756dea0858 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 18 Jan 2023 19:35:28 +0500 Subject: [PATCH 11/19] Added getCachedconfig Function to return ProjectConfig without wait, but the default implementation of it will return null. --- .../src/main/java/com/optimizely/ab/Optimizely.java | 8 ++++---- .../ab/config/AtomicProjectConfigManager.java | 12 +++++++++++- .../ab/config/PollingProjectConfigManager.java | 10 ++++++++++ .../optimizely/ab/config/ProjectConfigManager.java | 8 ++++++++ 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 824556eab..5f41bcd6e 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -125,10 +125,10 @@ private Optimizely(@Nonnull EventHandler eventHandler, if (odpManager != null) { odpManager.getEventManager().start(); - if (!(projectConfigManager instanceof PollingProjectConfigManager)) { + if (projectConfigManager.getCachedConfig() != null) { updateODPSettings(); } - if (projectConfigManager != null && projectConfigManager.getSDKKey() != null) { + if (projectConfigManager.getSDKKey() != null) { NotificationRegistry.getInternalNotificationCenter(projectConfigManager.getSDKKey()). addNotificationHandler(UpdateConfigNotification.class, configNotification -> { updateODPSettings(); }); @@ -1485,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()); } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java index fa1f4bd62..5c1074752 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java +++ b/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,16 @@ public ProjectConfig getConfig() { return projectConfigReference.get(); } + /** + * Access to current cached project configuration. + * + * @return {@link ProjectConfig} + */ + @Override + public ProjectConfig getCachedConfig() { + return projectConfigReference.get(); + } + public void setConfig(ProjectConfig projectConfig) { projectConfigReference.set(projectConfig); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java index 78a944ccb..8363f753d 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java @@ -85,6 +85,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 diff --git a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java index 4fae41ff5..0a436442f 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java @@ -24,6 +24,14 @@ public interface ProjectConfigManager { */ ProjectConfig getConfig(); + /** + * Implementations of this method should not block until a datafile is available, instead return current cached project configuration. + * + * NOTE: To use ODP segments, implementation of this function is required to return current project configuration. + * @return ProjectConfig + */ + default ProjectConfig getCachedConfig() { return null; } + default String getSDKKey() { return null; } From 0deec47c2edb60c2626ccfbea595573f42d01743 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 18 Jan 2023 19:47:25 +0500 Subject: [PATCH 12/19] ProjectConfig getCachedConfig(); without default implementation --- .../ab/config/ProjectConfigManager.java | 2 +- .../com/optimizely/ab/OptimizelyTest.java | 22 ++++++++++++++----- .../com/optimizely/ab/OptimizelyFactory.java | 15 +++++++++++-- .../optimizely/ab/OptimizelyFactoryTest.java | 21 ++++++++++++++---- 4 files changed, 48 insertions(+), 12 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java index 0a436442f..20185fc40 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java @@ -30,7 +30,7 @@ public interface ProjectConfigManager { * NOTE: To use ODP segments, implementation of this function is required to return current project configuration. * @return ProjectConfig */ - default ProjectConfig getCachedConfig() { return null; } + ProjectConfig getCachedConfig(); default String getSDKKey() { return null; diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index c266f4d07..983864d70 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -119,6 +119,18 @@ public static Collection 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; + } + }; + @Rule @SuppressFBWarnings("URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") public RuleChain ruleChain = RuleChain.outerRule(thrown) @@ -4505,13 +4517,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 manager = optimizely.getNotificationCenter() .getNotificationManager(TrackNotification.class); @@ -4521,7 +4533,7 @@ public void testAddTrackNotificationHandler() { @Test public void testAddDecisionNotificationHandler() { - Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build(); + Optimizely optimizely = optimizelyBuilder.withConfigManager(projectConfigManagerReturningNull).build(); NotificationManager manager = optimizely.getNotificationCenter() .getNotificationManager(DecisionNotification.class); @@ -4531,7 +4543,7 @@ public void testAddDecisionNotificationHandler() { @Test public void testAddUpdateConfigNotificationHandler() { - Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build(); + Optimizely optimizely = optimizelyBuilder.withConfigManager(projectConfigManagerReturningNull).build(); NotificationManager manager = optimizely.getNotificationCenter() .getNotificationManager(UpdateConfigNotification.class); @@ -4541,7 +4553,7 @@ public void testAddUpdateConfigNotificationHandler() { @Test public void testAddLogEventNotificationHandler() { - Optimizely optimizely = optimizelyBuilder.withConfigManager(() -> null).build(); + Optimizely optimizely = optimizelyBuilder.withConfigManager(projectConfigManagerReturningNull).build(); NotificationManager manager = optimizely.getNotificationCenter() .getNotificationManager(LogEvent.class); diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java index 37d56da03..2f3fc3f08 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java @@ -1,6 +1,6 @@ /** * - * Copyright 2019-2021, Optimizely + * Copyright 2019-2021, 2023, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,7 @@ package com.optimizely.ab; import com.optimizely.ab.config.HttpProjectConfigManager; +import com.optimizely.ab.config.ProjectConfig; import com.optimizely.ab.config.ProjectConfigManager; import com.optimizely.ab.event.AsyncEventHandler; import com.optimizely.ab.event.BatchEventProcessor; @@ -222,7 +223,17 @@ public static Optimizely newDefaultInstance() { public static Optimizely newDefaultInstance(String sdkKey) { if (sdkKey == null) { logger.error("Must provide an sdkKey, returning non-op Optimizely client"); - return newDefaultInstance(() -> null); + return newDefaultInstance(new ProjectConfigManager() { + @Override + public ProjectConfig getConfig() { + return null; + } + + @Override + public ProjectConfig getCachedConfig() { + return null; + } + }); } return newDefaultInstance(sdkKey, null); diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java index 07c2c0634..5b9b0492e 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java @@ -1,6 +1,6 @@ /** * - * Copyright 2019-2020, Optimizely + * Copyright 2019-2020, 2023, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,8 @@ import com.google.common.base.Charsets; import com.google.common.io.Resources; import com.optimizely.ab.config.HttpProjectConfigManager; +import com.optimizely.ab.config.ProjectConfig; +import com.optimizely.ab.config.ProjectConfigManager; import com.optimizely.ab.event.AsyncEventHandler; import com.optimizely.ab.event.BatchEventProcessor; import com.optimizely.ab.internal.PropertyUtils; @@ -260,17 +262,28 @@ public void newDefaultInstanceWithDatafileAccessTokenAndCustomHttpClient() throw optimizely = OptimizelyFactory.newDefaultInstance("sdk-key", datafileString, "auth-token", httpClient); assertTrue(optimizely.isValid()); } + public ProjectConfigManager projectConfigManagerReturningNull = new ProjectConfigManager() { + @Override + public ProjectConfig getConfig() { + return null; + } + + @Override + public ProjectConfig getCachedConfig() { + return null; + } + }; @Test public void newDefaultInstanceWithProjectConfig() throws Exception { - optimizely = OptimizelyFactory.newDefaultInstance(() -> null); + optimizely = OptimizelyFactory.newDefaultInstance(projectConfigManagerReturningNull); assertFalse(optimizely.isValid()); } @Test public void newDefaultInstanceWithProjectConfigAndNotificationCenter() throws Exception { NotificationCenter notificationCenter = new NotificationCenter(); - optimizely = OptimizelyFactory.newDefaultInstance(() -> null, notificationCenter); + optimizely = OptimizelyFactory.newDefaultInstance(projectConfigManagerReturningNull, notificationCenter); assertFalse(optimizely.isValid()); assertEquals(notificationCenter, optimizely.getNotificationCenter()); } @@ -278,7 +291,7 @@ public void newDefaultInstanceWithProjectConfigAndNotificationCenter() throws Ex @Test public void newDefaultInstanceWithProjectConfigAndNotificationCenterAndEventHandler() { NotificationCenter notificationCenter = new NotificationCenter(); - optimizely = OptimizelyFactory.newDefaultInstance(() -> null, notificationCenter, logEvent -> {}); + optimizely = OptimizelyFactory.newDefaultInstance(projectConfigManagerReturningNull, notificationCenter, logEvent -> {}); assertFalse(optimizely.isValid()); assertEquals(notificationCenter, optimizely.getNotificationCenter()); } From 43da46a75065204f69787b5e42f9cceda4e4a1fe Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 19 Jan 2023 18:54:51 +0500 Subject: [PATCH 13/19] Removed default implementation of getSDKkey to enforce user to make their own choice. --- .../ab/config/AtomicProjectConfigManager.java | 5 +++++ .../com/optimizely/ab/config/ProjectConfigManager.java | 10 +++++++--- .../test/java/com/optimizely/ab/OptimizelyTest.java | 5 +++++ .../main/java/com/optimizely/ab/OptimizelyFactory.java | 5 +++++ .../java/com/optimizely/ab/OptimizelyFactoryTest.java | 5 +++++ 5 files changed, 27 insertions(+), 3 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java index 5c1074752..336d33c0e 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/AtomicProjectConfigManager.java @@ -37,6 +37,11 @@ public ProjectConfig getCachedConfig() { return projectConfigReference.get(); } + @Override + public String getSDKKey() { + return null; + } + public void setConfig(ProjectConfig projectConfig) { projectConfigReference.set(projectConfig); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java index 20185fc40..e2e87d41f 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java @@ -32,8 +32,12 @@ public interface ProjectConfigManager { */ ProjectConfig getCachedConfig(); - default String getSDKKey() { - return null; - } + /** + * 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 + */ + String getSDKKey(); } diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index 983864d70..705ce1cb6 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -129,6 +129,11 @@ public ProjectConfig getConfig() { public ProjectConfig getCachedConfig() { return null; } + + @Override + public String getSDKKey() { + return null; + } }; @Rule diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java index 2f3fc3f08..1c6ee2820 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java @@ -233,6 +233,11 @@ public ProjectConfig getConfig() { public ProjectConfig getCachedConfig() { return null; } + + @Override + public String getSDKKey() { + return null; + } }); } diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java index 5b9b0492e..aaa3a67fa 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java @@ -272,6 +272,11 @@ public ProjectConfig getConfig() { public ProjectConfig getCachedConfig() { return null; } + + @Override + public String getSDKKey() { + return null; + } }; @Test From 0fe4fb3312869ca2358d4ebc4409e1d6a342fb12 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 25 Jan 2023 15:55:53 +0500 Subject: [PATCH 14/19] Resolved comments --- .../com/optimizely/ab/config/ProjectConfigManager.java | 5 +++++ .../com/optimizely/ab/config/HttpProjectConfigManager.java | 7 +++---- .../optimizely/ab/config/HttpProjectConfigManagerTest.java | 1 + 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java index e2e87d41f..002acae55 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfigManager.java @@ -16,6 +16,8 @@ */ package com.optimizely.ab.config; +import javax.annotation.Nullable; + public interface ProjectConfigManager { /** * Implementations of this method should block until a datafile is available. @@ -26,10 +28,12 @@ public interface ProjectConfigManager { /** * 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(); /** @@ -38,6 +42,7 @@ public interface ProjectConfigManager { * NOTE: To update ODP segments configuration via polling, it is required to return sdkKey. * @return String */ + @Nullable String getSDKKey(); } diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java index 2c0d30687..118139890 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java @@ -340,11 +340,10 @@ public HttpProjectConfigManager build(boolean defer) { .withEvictIdleConnections(evictConnectionIdleTimePeriod, evictConnectionIdleTimeUnit) .build(); } - + if (sdkKey == null) { + throw new NullPointerException("sdkKey cannot be null"); + } if (url == null) { - if (sdkKey == null) { - throw new NullPointerException("sdkKey cannot be null"); - } if (datafileAccessToken == null) { url = String.format(format, sdkKey); diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java index 317348aba..9cbc0bb01 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java @@ -129,6 +129,7 @@ public void testHttpGetByCustomUrl() throws Exception { projectConfigManager = builder() .withOptimizelyHttpClient(mockHttpClient) + .withSdkKey("custom-sdkKey") .withUrl(expected) .build(); From dd6a823688a7e31dba66ec1fa8f75df6174e38b0 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 25 Jan 2023 18:32:31 +0500 Subject: [PATCH 15/19] setting Sdk key inside pollingConfigManager to make sure that the sdkKey for a particular config is always same. --- .../ab/config/PollingProjectConfigManager.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java index 8363f753d..f150de9a4 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java @@ -57,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; @@ -120,8 +121,9 @@ void setConfig(ProjectConfig projectConfig) { currentProjectConfig.set(projectConfig); currentOptimizelyConfig.set(new OptimizelyConfigService(projectConfig).getConfig()); countDownLatch.countDown(); - if (projectConfig.getSdkKey() != null) { - NotificationRegistry.getInternalNotificationCenter(projectConfig.getSdkKey()).send(SIGNAL); + + if (sdkKey != null) { + NotificationRegistry.getInternalNotificationCenter(sdkKey).send(SIGNAL); } notificationCenter.send(SIGNAL); } @@ -203,6 +205,10 @@ public synchronized void close() { started = false; } + protected void setSdkKey(String sdkKey) { + this.sdkKey = sdkKey; + } + public boolean isRunning() { return started; } From 680a5bff81f29462321fb1e0b6ad8bf5306dfc87 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 25 Jan 2023 18:33:04 +0500 Subject: [PATCH 16/19] setting sdkKey in parent class --- .../com/optimizely/ab/config/HttpProjectConfigManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java index 118139890..638a44a99 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java @@ -179,7 +179,7 @@ public static class Builder { private String datafile; private String url; private String datafileAccessToken = null; - private String format = "https://cdn.optimizely.com/datafiles/%s.json"; + private String format = "http://localhost:3001/datafiles/%s.json"; private String authFormat = "https://config.optimizely.com/datafiles/auth/%s.json"; private OptimizelyHttpClient httpClient; private NotificationCenter notificationCenter; @@ -366,7 +366,7 @@ public HttpProjectConfigManager build(boolean defer) { blockingTimeoutUnit, notificationCenter, sdkKey); - + httpProjectManager.setSdkKey(sdkKey); if (datafile != null) { try { ProjectConfig projectConfig = HttpProjectConfigManager.parseProjectConfig(datafile); From 65c150d2f35ba2938bd16c61e1390e56261d02d0 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 25 Jan 2023 18:52:30 +0500 Subject: [PATCH 17/19] fix --- .../java/com/optimizely/ab/config/HttpProjectConfigManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java index 638a44a99..cf72d3814 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java @@ -179,7 +179,7 @@ public static class Builder { private String datafile; private String url; private String datafileAccessToken = null; - private String format = "http://localhost:3001/datafiles/%s.json"; + private String format = "https://cdn.optimizely.com/datafiles/%s.json"; private String authFormat = "https://config.optimizely.com/datafiles/auth/%s.json"; private OptimizelyHttpClient httpClient; private NotificationCenter notificationCenter; From 5bc8ffd4e98f7780e0c829862701e2f7e6c84b77 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Wed, 25 Jan 2023 19:05:46 +0500 Subject: [PATCH 18/19] making sure that if the SDKKey is set by the user then it should be picked from projectConfig --- .../com/optimizely/ab/config/PollingProjectConfigManager.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java index f150de9a4..92358375a 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java @@ -122,6 +122,9 @@ void setConfig(ProjectConfig projectConfig) { currentOptimizelyConfig.set(new OptimizelyConfigService(projectConfig).getConfig()); countDownLatch.countDown(); + if (sdkKey == null) { + sdkKey = projectConfig.getSdkKey(); + } if (sdkKey != null) { NotificationRegistry.getInternalNotificationCenter(sdkKey).send(SIGNAL); } From 26d50ae26e0433451baa1c759b57f8d63f705f07 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 26 Jan 2023 03:23:16 +0500 Subject: [PATCH 19/19] Refactored and moved getSdkKey to pollingConfigManager to make sure that to trigger the notification always user provided sdkKey will be prioritized --- .../ab/config/PollingProjectConfigManager.java | 5 +++++ .../ab/config/HttpProjectConfigManager.java | 13 ++----------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java index 92358375a..4ad34e7a8 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java +++ b/core-api/src/main/java/com/optimizely/ab/config/PollingProjectConfigManager.java @@ -169,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."); diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java index cf72d3814..4be1715d2 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java @@ -65,7 +65,6 @@ public class HttpProjectConfigManager extends PollingProjectConfigManager { private final URI uri; private final String datafileAccessToken; private String datafileLastModified; - private String sdkKey; private HttpProjectConfigManager(long period, TimeUnit timeUnit, @@ -74,13 +73,11 @@ private HttpProjectConfigManager(long period, String datafileAccessToken, long blockingTimeoutPeriod, TimeUnit blockingTimeoutUnit, - NotificationCenter notificationCenter, - String sdkKey) { + NotificationCenter notificationCenter) { super(period, timeUnit, blockingTimeoutPeriod, blockingTimeoutUnit, notificationCenter); this.httpClient = httpClient; this.uri = URI.create(url); this.datafileAccessToken = datafileAccessToken; - this.sdkKey = sdkKey; } public URI getUri() { @@ -170,11 +167,6 @@ public static Builder builder() { return new Builder(); } - @Override - public String getSDKKey() { - return this.sdkKey; - } - public static class Builder { private String datafile; private String url; @@ -364,8 +356,7 @@ public HttpProjectConfigManager build(boolean defer) { datafileAccessToken, blockingTimeoutPeriod, blockingTimeoutUnit, - notificationCenter, - sdkKey); + notificationCenter); httpProjectManager.setSdkKey(sdkKey); if (datafile != null) { try {