From 36cf92affa81fbb4ca3a7ded717f760870563e45 Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Tue, 21 Dec 2021 15:32:49 -0800 Subject: [PATCH] unschedule background polling worker when interval is not set --- .../ab/android/sdk/OptimizelyManagerTest.java | 4 +- .../ab/android/sdk/OptimizelyManager.java | 5 +- .../sdk/OptimizelyManagerBuilderTest.java | 142 ++++++++++++------ .../DefaultDatafileHandler.java | 3 - .../ab/android/shared/WorkerScheduler.java | 3 +- 5 files changed, 106 insertions(+), 51 deletions(-) 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 index 07062e6e5..559027430 100644 --- 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 index e5dc2f292..e46192c12 100644 --- 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 index 952259de3..97d281986 100644 --- 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,20 +61,16 @@ 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 @@ -70,57 +78,54 @@ public class OptimizelyManagerBuilderTest { */ @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,8 +135,9 @@ 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); @@ -139,4 +145,52 @@ public void testBuildWithUserProfileService() { 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 index b510e969b..875d257f7 100644 --- 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 index e9080e11b..97b2d41a5 100644 --- 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 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)