unschedule background polling worker when interval is not set (#397)

jaeopt · web-flow · commit 708ecbfbd00a · 2022-01-05T09:08:23.000-08:00
- This fixes the bug that a polling worker scheduled before is not disabled when Optimizely is initialized again with polling disabled.
- When Optimizely is initialized, stop the previous worker and then restart it again only when polling is enabled.
diff --git a/android-sdk/src/androidTest/java/com/optimizely/ab/android/sdk/OptimizelyManagerTest.java b/android-sdk/src/androidTest/java/com/optimizely/ab/android/sdk/OptimizelyManagerTest.java
@@ -218,7 +218,7 @@ public void getDatafile() {
         assertNotNull(optimizelyManager.getDatafileHandler());
     }
 
-        @Test
+    @Test
     public void initializeAsyncWithEnvironment() {
         Logger logger = mock(Logger.class);
         DatafileHandler datafileHandler = mock(DefaultDatafileHandler.class);
@@ -377,7 +377,7 @@ public void injectOptimizely() {
     }
 
     @Test
-    public void injectOptimizelyWithDatafileLisener() {
+    public void injectOptimizelyWithDatafileListener() {
         Context context = mock(Context.class);
         UserProfileService userProfileService = mock(UserProfileService.class);
         OptimizelyStartListener startListener = mock(OptimizelyStartListener.class);
diff --git a/android-sdk/src/main/java/com/optimizely/ab/android/sdk/OptimizelyManager.java b/android-sdk/src/main/java/com/optimizely/ab/android/sdk/OptimizelyManager.java
@@ -501,6 +501,9 @@ private boolean datafileDownloadEnabled() {
     }
 
     private void startDatafileHandler(Context context) {
+        // if already running, stop it first.
+        datafileHandler.stopBackgroundUpdates(context, datafileConfig);
+
         if (!datafileDownloadEnabled()) {
             logger.debug("Invalid download interval, ignoring background updates.");
             return;
@@ -612,7 +615,7 @@ protected ErrorHandler getErrorHandler(Context context) {
         return errorHandler;
     }
 
-    private boolean isAndroidVersionSupported() {
+    protected boolean isAndroidVersionSupported() {
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.ICE_CREAM_SANDWICH) {
             return true;
         } else {
diff --git a/android-sdk/src/test/java/com/optimizely/ab/android/sdk/OptimizelyManagerBuilderTest.java b/android-sdk/src/test/java/com/optimizely/ab/android/sdk/OptimizelyManagerBuilderTest.java
@@ -19,23 +19,35 @@
 import android.content.Context;
 
 import com.optimizely.ab.android.datafile_handler.DatafileHandler;
+import com.optimizely.ab.android.datafile_handler.DefaultDatafileHandler;
+import com.optimizely.ab.android.shared.WorkerScheduler;
 import com.optimizely.ab.android.user_profile.DefaultUserProfileService;
 import com.optimizely.ab.error.ErrorHandler;
 import com.optimizely.ab.event.EventHandler;
 
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.runners.MockitoJUnitRunner;
 import org.slf4j.Logger;
 
 import static junit.framework.Assert.assertEquals;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.eq;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.sql.Time;
+import java.util.concurrent.TimeUnit;
+
 @RunWith(MockitoJUnitRunner.class)
 public class OptimizelyManagerBuilderTest {
 
     private String testProjectId = "7595190003";
+    private String testSdkKey = "1234";
     private Logger logger;
 
     private String minDatafile = "{\n" +
@@ -49,78 +61,71 @@ public class OptimizelyManagerBuilderTest {
             "events: [ ],\n" +
             "revision: \"1\"\n" +
             "}";
-    /**
-     * Verify that building the {@link OptimizelyManager} with a polling interval less than 60
-     * seconds defaults to 60 seconds.
-     */
-//    @Test
-//    public void testBuildWithInvalidPollingInterval() {
-//        Context appContext = mock(Context.class);
-//        when(appContext.getApplicationContext()).thenReturn(appContext);
-//        OptimizelyManager manager = OptimizelyManager.builder("1")
-//                .withDatafileDownloadInterval(5L)
-//                .build(appContext);
-//
-//        assertEquals(900L, manager.getDatafileDownloadInterval().longValue());
-//    }
+
+    private Context mockContext;
+    private DefaultDatafileHandler mockDatafileHandler;
+
+    @Before
+    public void setup() throws Exception {
+        mockContext = mock(Context.class);
+        when(mockContext.getApplicationContext()).thenReturn(mockContext);
+        mockDatafileHandler = mock(DefaultDatafileHandler.class);
+    }
 
     /**
      * Verify that building the {@link OptimizelyManager} with a polling interval greater than 60
      * seconds is properly registered.
      */
     @Test
     public void testBuildWithValidPollingInterval() {
-        Context appContext = mock(Context.class);
-        when(appContext.getApplicationContext()).thenReturn(appContext);
-        OptimizelyManager manager = OptimizelyManager.builder("1")
-                .withDatafileDownloadInterval(901L)
-                .build(appContext);
+        Long interval = 16L;
+        TimeUnit timeUnit = TimeUnit.MINUTES;
 
-        assertEquals(901L, manager.getDatafileDownloadInterval().longValue());
+        OptimizelyManager manager = OptimizelyManager.builder()
+                .withSDKKey(testSdkKey)
+                .withDatafileDownloadInterval(interval, timeUnit)
+                .build(mockContext);
+
+        assertEquals(interval * 60L, manager.getDatafileDownloadInterval().longValue());
     }
 
     @Test
     public void testBuildWithEventHandler() {
-        Context appContext = mock(Context.class);
-        when(appContext.getApplicationContext()).thenReturn(appContext);
         EventHandler eventHandler = mock(EventHandler.class);
-        OptimizelyManager manager = OptimizelyManager.builder(testProjectId)
-                .withDatafileDownloadInterval(901L)
+        OptimizelyManager manager = OptimizelyManager.builder()
+                .withSDKKey(testSdkKey)
+                .withDatafileDownloadInterval(901L, TimeUnit.SECONDS)
                 .withEventHandler(eventHandler)
-                .build(appContext);
+                .build(mockContext);
 
         assertEquals(901L, manager.getDatafileDownloadInterval().longValue());
-        assertEquals(manager.getEventHandler(appContext), eventHandler);
-
-
+        assertEquals(manager.getEventHandler(mockContext), eventHandler);
     }
 
     @Test
     public void testBuildWithErrorHandler() {
-        Context appContext = mock(Context.class);
-        when(appContext.getApplicationContext()).thenReturn(appContext);
         ErrorHandler errorHandler = mock(ErrorHandler.class);
-        OptimizelyManager manager = OptimizelyManager.builder(testProjectId)
-                .withDatafileDownloadInterval(61L)
+        OptimizelyManager manager = OptimizelyManager.builder()
+                .withSDKKey(testSdkKey)
+                .withDatafileDownloadInterval(61L, TimeUnit.SECONDS)
                 .withErrorHandler(errorHandler)
-                .build(appContext);
+                .build(mockContext);
 
-        manager.initialize(appContext, minDatafile);
+        manager.initialize(mockContext, minDatafile);
 
-        assertEquals(manager.getErrorHandler(appContext), errorHandler);
+        assertEquals(manager.getErrorHandler(mockContext), errorHandler);
     }
 
     @Test
     public void testBuildWithDatafileHandler() {
-        Context appContext = mock(Context.class);
-        when(appContext.getApplicationContext()).thenReturn(appContext);
-        DatafileHandler dfHandler = mock(DatafileHandler.class);
-        OptimizelyManager manager = OptimizelyManager.builder(testProjectId)
-                .withDatafileDownloadInterval(61L)
+        DefaultDatafileHandler dfHandler = mock(DefaultDatafileHandler.class);
+        OptimizelyManager manager = OptimizelyManager.builder()
+                .withSDKKey(testSdkKey)
+                .withDatafileDownloadInterval(61L, TimeUnit.SECONDS)
                 .withDatafileHandler(dfHandler)
-                .build(appContext);
+                .build(mockContext);
 
-        manager.initialize(appContext, minDatafile);
+        manager.initialize(mockContext, minDatafile);
 
         assertEquals(manager.getDatafileHandler(), dfHandler);
     }
@@ -130,13 +135,62 @@ public void testBuildWithUserProfileService() {
         Context appContext = mock(Context.class);
         when(appContext.getApplicationContext()).thenReturn(appContext);
         DefaultUserProfileService ups = mock(DefaultUserProfileService.class);
-        OptimizelyManager manager = OptimizelyManager.builder(testProjectId)
-                .withDatafileDownloadInterval(61L)
+        OptimizelyManager manager = OptimizelyManager.builder()
+                .withSDKKey(testSdkKey)
+                .withDatafileDownloadInterval(61L, TimeUnit.SECONDS)
                 .withUserProfileService(ups)
                 .build(appContext);
 
         manager.initialize(appContext, minDatafile);
 
         assertEquals(manager.getUserProfileService(), ups);
     }
+
+    // BackgroundDatafile worker tests
+
+    @Test
+    public void testBuildWithDatafileDownloadInterval_workerScheduled() throws Exception {
+        long goodNumber = 1;
+        OptimizelyManager manager = OptimizelyManager.builder()
+                .withSDKKey(testSdkKey)
+                .withDatafileHandler(mockDatafileHandler)
+                .withDatafileDownloadInterval(goodNumber, TimeUnit.MINUTES)
+                .build(mockContext);
+        OptimizelyManager spyManager = spy(manager);
+        when(spyManager.isAndroidVersionSupported()).thenReturn(true);
+        spyManager.initialize(mockContext, "");
+
+        verify(mockDatafileHandler).stopBackgroundUpdates(any(), any());
+        verify(mockDatafileHandler).startBackgroundUpdates(any(), any(), eq(goodNumber * 60L), any());
+    }
+
+    @Test
+    public void testBuildWithDatafileDownloadInterval_workerCancelledWhenIntervalIsNotPositive() throws Exception {
+        OptimizelyManager manager = OptimizelyManager.builder()
+                .withSDKKey(testSdkKey)
+                .withDatafileHandler(mockDatafileHandler)
+                .withDatafileDownloadInterval(-1, TimeUnit.MINUTES)
+                .build(mockContext);
+        OptimizelyManager spyManager = spy(manager);
+        when(spyManager.isAndroidVersionSupported()).thenReturn(true);
+        spyManager.initialize(mockContext, "");
+
+        verify(mockDatafileHandler).stopBackgroundUpdates(any(), any());
+        verify(mockDatafileHandler, never()).startBackgroundUpdates(any(), any(), any(), any());
+    }
+
+    @Test
+    public void testBuildWithDatafileDownloadInterval_workerCancelledWhenNoIntervalProvided() throws Exception {
+        OptimizelyManager manager = OptimizelyManager.builder()
+                .withSDKKey(testSdkKey)
+                .withDatafileHandler(mockDatafileHandler)
+                .build(mockContext);
+        OptimizelyManager spyManager = spy(manager);
+        when(spyManager.isAndroidVersionSupported()).thenReturn(true);
+        spyManager.initialize(mockContext, "");
+
+        verify(mockDatafileHandler).stopBackgroundUpdates(any(), any());
+        verify(mockDatafileHandler, never()).startBackgroundUpdates(any(), any(), any(), any());
+    }
+
 }
diff --git a/datafile-handler/src/main/java/com/optimizely/ab/android/datafile_handler/DefaultDatafileHandler.java b/datafile-handler/src/main/java/com/optimizely/ab/android/datafile_handler/DefaultDatafileHandler.java
@@ -118,9 +118,6 @@ public void downloadDatafileToCache(final Context context, DatafileConfig datafi
      * @param updateInterval frequency of updates in seconds
      */
     public void startBackgroundUpdates(Context context, DatafileConfig datafileConfig, Long updateInterval, DatafileLoadedListener listener) {
-        // if already running, stop it
-        stopBackgroundUpdates(context, datafileConfig);
-
         long updateIntervalInMinutes = updateInterval / 60;
 
         // save the project id background start is set.  If we get a reboot or a replace, we can restart via the
diff --git a/shared/src/main/java/com/optimizely/ab/android/shared/WorkerScheduler.java b/shared/src/main/java/com/optimizely/ab/android/shared/WorkerScheduler.java
@@ -60,7 +60,8 @@ public static void unscheduleService(Context context, String workerId) {
      * @return An (WorkRequest, Operation) that can be used for tracing work state
      */
     public static AbstractMap.SimpleEntry<WorkRequest, Operation> scheduleService(Context context, String workerId, Class clazz, Data data, long interval) {
-        WorkManager.getInstance(context).cancelAllWorkByTag(workerId);
+        unscheduleService(context, workerId);
+
         long minutes = interval < 15 ? 15 : interval;
 
         WorkRequest.Builder workRequestBuilder = new PeriodicWorkRequest.Builder(clazz, minutes, TimeUnit.MINUTES)
