From 4298f8b75eae936c06af09858c70920e956ac52d Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Fri, 6 Mar 2020 12:22:29 -0800 Subject: [PATCH 1/4] fix event flush interval parameter for consistency --- .gitignore | 1 + android-sdk/build.gradle | 1 + .../ab/android/sdk/OptimizelyManager.java | 57 ++++- .../sdk/OptimizelyManagerBuilderTest.java | 5 + .../sdk/OptimizelyManagerIntervalTest.java | 237 ++++++++++++++++++ build.gradle | 3 +- .../ab/android/test_app/MyApplication.java | 9 +- 7 files changed, 300 insertions(+), 13 deletions(-) create mode 100644 android-sdk/src/test/java/com/optimizely/ab/android/sdk/OptimizelyManagerIntervalTest.java diff --git a/.gitignore b/.gitignore index d1b433d04..d9df0e2d3 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ /build /captures values.gradle +jacoco.exec diff --git a/android-sdk/build.gradle b/android-sdk/build.gradle index b10e27b56..eb7583cd3 100644 --- a/android-sdk/build.gradle +++ b/android-sdk/build.gradle @@ -71,6 +71,7 @@ dependencies { testImplementation "junit:junit:$junit_ver" testImplementation "org.mockito:mockito-core:$mockito_ver" + testImplementation "org.powermock:powermock-mockito-release-full:$powermock_ver" testImplementation "com.noveogroup.android:android-logger:$android_logger_ver" androidTestImplementation "com.android.support.test:runner:$support_test_runner_ver" 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 cd304fd89..91a7a7e95 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 @@ -55,6 +55,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.Set; +import java.util.concurrent.TimeUnit; /** @@ -648,8 +649,10 @@ public static class Builder { // -1 will cause the background download to not be initiated. private long datafileDownloadInterval = -1L; - // -1 will cause the background download to not be initiated. - private long eventDispatchInterval = -1L; + // -1 will disable event batching. + private long eventFlushInterval = -1L; + // -l will disable periodic retries on event dispatch failures (but queued and retried on next event dispatch request) + private long eventDispatchRetryInterval = -1L; @Nullable private DatafileHandler datafileHandler = null; @Nullable private Logger logger = null; @Nullable private EventHandler eventHandler = null; @@ -722,15 +725,40 @@ public Builder withErrorHandler(ErrorHandler errorHandler) { } /** - * Sets the interval which {@link EventIntentService} will flush events. - * If you set this to -1, you disable background updates. If you don't set - * a event dispatch interval, then no background updates will be scheduled or occur. + * Sets the interval which queued events will be flushed periodically. + * If you don't set this value or set this to -1, the default interval will be used (30 seconds). * * @param interval the interval in seconds * @return this {@link Builder} instance */ + public Builder withEventFlushInterval(long interval) { + this.eventFlushInterval = interval * 1000; + return this; + } + + /** + * Sets the interval which {@link EventIntentService} will retry event dispatch periodically. + * If you don't set this value or set this to -1, periodic retries on event dispatch failures will be disabled (but still queued and retried on next event dispatch request) + * + * @param interval the interval in seconds + * @return this {@link Builder} instance + */ + public Builder withEventDispatchRetryInterval(long interval) { + this.eventDispatchRetryInterval = interval * 1000; + return this; + } + + /** + * Sets the interval which {@link EventIntentService} will retry event dispatch periodically. + * If you don't set this value or set this to -1, periodic retries on event dispatch failures will be disabled (but still queued and retried on next event dispatch request) + * + * @param interval the interval in milliseconds + * @return this {@link Builder} instance + */ + @Deprecated public Builder withEventDispatchInterval(long interval) { - this.eventDispatchInterval = interval; + this.eventFlushInterval = interval; + this.eventDispatchRetryInterval = interval; return this; } @@ -796,6 +824,19 @@ public OptimizelyManager build(Context context) { } } + if (eventFlushInterval > 1_000_000) { + logger.warn("Event flush interval {} milliseconds is too big", eventFlushInterval); + } + if (eventFlushInterval > 0 && eventFlushInterval < 1000) { + logger.warn("Event flush interval {} milliseconds is too small", eventFlushInterval); + } + if (eventDispatchRetryInterval > 1_000_000) { + logger.warn("Event dispatch retry interval {} milliseconds is too big", eventDispatchRetryInterval); + } + if (eventDispatchRetryInterval > 0 && eventDispatchRetryInterval < 1000) { + logger.warn("Event dispatch retry interval {} milliseconds is too small", eventDispatchRetryInterval); + } + if (datafileConfig == null) { datafileConfig = new DatafileConfig(projectId, sdkKey); } @@ -821,7 +862,7 @@ public OptimizelyManager build(Context context) { eventProcessor = BatchEventProcessor.builder() .withNotificationCenter(notificationCenter) .withEventHandler(eventHandler) - .withFlushInterval(eventDispatchInterval) + .withFlushInterval(eventFlushInterval) .build(); } @@ -837,7 +878,7 @@ public OptimizelyManager build(Context context) { datafileDownloadInterval, datafileHandler, errorHandler, - eventDispatchInterval, + eventDispatchRetryInterval, eventHandler, eventProcessor, userProfileService, 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 b088a7728..0affe2ed0 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 @@ -21,6 +21,7 @@ import com.optimizely.ab.android.datafile_handler.DatafileHandler; import com.optimizely.ab.android.user_profile.DefaultUserProfileService; import com.optimizely.ab.error.ErrorHandler; +import com.optimizely.ab.event.BatchEventProcessor; import com.optimizely.ab.event.EventHandler; import org.junit.Test; @@ -29,9 +30,13 @@ import org.slf4j.Logger; import static junit.framework.Assert.assertEquals; +import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; + @RunWith(MockitoJUnitRunner.class) public class OptimizelyManagerBuilderTest { diff --git a/android-sdk/src/test/java/com/optimizely/ab/android/sdk/OptimizelyManagerIntervalTest.java b/android-sdk/src/test/java/com/optimizely/ab/android/sdk/OptimizelyManagerIntervalTest.java new file mode 100644 index 000000000..4f7cf667f --- /dev/null +++ b/android-sdk/src/test/java/com/optimizely/ab/android/sdk/OptimizelyManagerIntervalTest.java @@ -0,0 +1,237 @@ +/**************************************************************************** + * Copyright 2017, 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. * + * 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.android.sdk; + +import android.content.Context; + +import com.optimizely.ab.android.datafile_handler.DatafileHandler; +import com.optimizely.ab.android.shared.DatafileConfig; +import com.optimizely.ab.android.user_profile.DefaultUserProfileService; +import com.optimizely.ab.bucketing.UserProfileService; +import com.optimizely.ab.error.ErrorHandler; +import com.optimizely.ab.event.BatchEventProcessor; +import com.optimizely.ab.event.EventHandler; +import com.optimizely.ab.event.EventProcessor; +import com.optimizely.ab.notification.NotificationCenter; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.runners.MockitoJUnitRunner; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.powermock.modules.junit4.PowerMockRunner; +import org.slf4j.Logger; + +import java.util.concurrent.BlockingQueue; +import java.util.concurrent.ExecutorService; + +import static junit.framework.Assert.assertEquals; +import static junit.framework.Assert.assertFalse; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyBoolean; +import static org.mockito.Matchers.anyLong; +import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.powermock.api.mockito.PowerMockito.verifyNew; +import static org.powermock.api.mockito.PowerMockito.whenNew; + + +@RunWith(PowerMockRunner.class) +@PrepareForTest({OptimizelyManager.class, BatchEventProcessor.class}) +public class OptimizelyManagerIntervalTest { + + private String testProjectId = "7595190003"; + private Logger logger; + + @Test + public void testBuildWithEventFlushInterval() throws Exception { + whenNew(OptimizelyManager.class).withAnyArguments().thenReturn(mock(OptimizelyManager.class)); + whenNew(BatchEventProcessor.class).withAnyArguments().thenReturn(mock(BatchEventProcessor.class)); + + Context appContext = mock(Context.class); + when(appContext.getApplicationContext()).thenReturn(appContext); + + long goodNumber = 100L; + OptimizelyManager manager = OptimizelyManager.builder("1") + .withLogger(logger) + .withEventFlushInterval(goodNumber) // seconds + .build(appContext); + + verifyNew(BatchEventProcessor.class).withArguments(any(BlockingQueue.class), + any(EventHandler.class), + anyInt(), + eq(goodNumber * 1000L), // milliseconds + anyLong(), + any(ExecutorService.class), + any(NotificationCenter.class), + any(Object.class)); + + verifyNew(OptimizelyManager.class).withArguments(anyString(), + anyString(), + any(DatafileConfig.class), + any(Logger.class), + anyLong(), + any(DatafileHandler.class), + any(ErrorHandler.class), + eq(-1L), // milliseconds + any(EventHandler.class), + any(EventProcessor.class), + any(UserProfileService.class), + any(NotificationCenter.class)); + } + + @Test + public void testBuildWithEventDispatchRetryInterval() throws Exception { + whenNew(OptimizelyManager.class).withAnyArguments().thenReturn(mock(OptimizelyManager.class)); + whenNew(BatchEventProcessor.class).withAnyArguments().thenReturn(mock(BatchEventProcessor.class)); + + Context appContext = mock(Context.class); + when(appContext.getApplicationContext()).thenReturn(appContext); + + long goodNumber = 100L; + long defaultEventFlushInterval = 30L; + + OptimizelyManager manager = OptimizelyManager.builder("1") + .withLogger(logger) + .withEventDispatchRetryInterval(goodNumber) // seconds + .build(appContext); + + verifyNew(BatchEventProcessor.class).withArguments(any(BlockingQueue.class), + any(EventHandler.class), + anyInt(), + eq(defaultEventFlushInterval * 1000L), // milliseconds + anyLong(), + any(ExecutorService.class), + any(NotificationCenter.class), + any(Object.class)); + + verifyNew(OptimizelyManager.class).withArguments(anyString(), + anyString(), + any(DatafileConfig.class), + any(Logger.class), + anyLong(), + any(DatafileHandler.class), + any(ErrorHandler.class), + eq(goodNumber * 1000L), // milliseconds + any(EventHandler.class), + any(EventProcessor.class), + any(UserProfileService.class), + any(NotificationCenter.class)); + } + + @Test + public void testBuildWithEventDispatchInterval() throws Exception { + whenNew(OptimizelyManager.class).withAnyArguments().thenReturn(mock(OptimizelyManager.class)); + whenNew(BatchEventProcessor.class).withAnyArguments().thenReturn(mock(BatchEventProcessor.class)); + + Context appContext = mock(Context.class); + when(appContext.getApplicationContext()).thenReturn(appContext); + + long goodNumber = 100L*1000L; + OptimizelyManager manager = OptimizelyManager.builder("1") + .withLogger(logger) + .withEventDispatchInterval(goodNumber) // milliseconds + .build(appContext); + + verifyNew(BatchEventProcessor.class).withArguments(any(BlockingQueue.class), + any(EventHandler.class), + anyInt(), + eq(goodNumber), // milliseconds + anyLong(), + any(ExecutorService.class), + any(NotificationCenter.class), + any(Object.class)); + + verifyNew(OptimizelyManager.class).withArguments(anyString(), + anyString(), + any(DatafileConfig.class), + any(Logger.class), + anyLong(), + any(DatafileHandler.class), + any(ErrorHandler.class), + eq(goodNumber), // milliseconds + any(EventHandler.class), + any(EventProcessor.class), + any(UserProfileService.class), + any(NotificationCenter.class)); + } + + @Test + public void testBuildWithInvalidEventDispatchInterval() { + Context appContext = mock(Context.class); + Logger logger = mock(Logger.class); + when(appContext.getApplicationContext()).thenReturn(appContext); + + long tooSmallNumber = 100L; + OptimizelyManager manager = OptimizelyManager.builder("1") + .withLogger(logger) + .withEventDispatchInterval(tooSmallNumber) + .build(appContext); + + verify(logger).warn("Event flush interval {} milliseconds is too small", tooSmallNumber); + } + + @Test + public void testBuildWithValidEventDispatchInterval() { + Context appContext = mock(Context.class); + Logger logger = mock(Logger.class); + when(appContext.getApplicationContext()).thenReturn(appContext); + + long goodNumber = 100L * 1000L; + OptimizelyManager manager = OptimizelyManager.builder("1") + .withLogger(logger) + .withEventDispatchInterval(goodNumber) + .build(appContext); + + verify(logger, never()).warn(any(String.class), any(Integer.class)); + } + + @Test + public void testBuildWithInvalidEventFlushInterval() { + Context appContext = mock(Context.class); + Logger logger = mock(Logger.class); + when(appContext.getApplicationContext()).thenReturn(appContext); + + long tooBigNumber = 100L * 1000L; + OptimizelyManager manager = OptimizelyManager.builder("1") + .withLogger(logger) + .withEventFlushInterval(tooBigNumber) + .build(appContext); + + verify(logger).warn("Event flush interval {} milliseconds is too big", tooBigNumber * 1000L); + } + + @Test + public void testBuildWithValidEventFlushInterval() { + Context appContext = mock(Context.class); + Logger logger = mock(Logger.class); + when(appContext.getApplicationContext()).thenReturn(appContext); + + long goodNumber = 100L; + OptimizelyManager manager = OptimizelyManager.builder("1") + .withLogger(logger) + .withEventFlushInterval(goodNumber) + .build(appContext); + + verify(logger, never()).warn(any(String.class), any(Integer.class)); + } + +} diff --git a/build.gradle b/build.gradle index 2e4af5732..ae25e03bb 100644 --- a/build.gradle +++ b/build.gradle @@ -58,7 +58,8 @@ ext { jacksonversion= "2.9.9.1" support_annotations_ver = "24.2.1" junit_ver = "4.12" - mockito_ver = "1.9.5" + mockito_ver = "1.10.19" + powermock_ver = "1.6.4" support_test_runner_ver = "0.5" dexmaker_ver = "1.2" espresso_ver = "2.2.2" diff --git a/test-app/src/main/java/com/optimizely/ab/android/test_app/MyApplication.java b/test-app/src/main/java/com/optimizely/ab/android/test_app/MyApplication.java index 9f6ba3086..26a06016e 100644 --- a/test-app/src/main/java/com/optimizely/ab/android/test_app/MyApplication.java +++ b/test-app/src/main/java/com/optimizely/ab/android/test_app/MyApplication.java @@ -73,9 +73,10 @@ public void onCreate() { // must match the project id of the compiled in Optimizely data file in rest/raw/data_file.json. OptimizelyManager.Builder builder = OptimizelyManager.builder(); - optimizelyManager = builder.withEventDispatchInterval(60L * 1000L) - .withDatafileDownloadInterval(0) - .withSDKKey("FCnSegiEkRry9rhVMroit4") - .build(getApplicationContext()); + optimizelyManager = builder + .withDatafileDownloadInterval(30) + .withEventFlushInterval(60) + .withSDKKey("FCnSegiEkRry9rhVMroit4") + .build(getApplicationContext()); } } From 679425da31e03b6eff9f2bc851c043bc6a73ad9d Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Fri, 6 Mar 2020 14:54:27 -0800 Subject: [PATCH 2/4] upgrade dexmaker to 1.4 for mockito upgrade --- android-sdk/build.gradle | 5 +++-- build.gradle | 2 +- datafile-handler/build.gradle | 5 +++-- event-handler/build.gradle | 5 +++-- shared/build.gradle | 5 +++-- test-app/build.gradle | 5 +++-- user-profile/build.gradle | 5 +++-- 7 files changed, 19 insertions(+), 13 deletions(-) diff --git a/android-sdk/build.gradle b/android-sdk/build.gradle index eb7583cd3..02873d856 100644 --- a/android-sdk/build.gradle +++ b/android-sdk/build.gradle @@ -80,8 +80,9 @@ dependencies { // Set this dependency to build and run Espresso tests androidTestImplementation "com.android.support.test.espresso:espresso-core:$espresso_ver" androidTestImplementation "org.mockito:mockito-core:$mockito_ver" - androidTestImplementation "com.google.dexmaker:dexmaker:$dexmaker_ver" - androidTestImplementation "com.google.dexmaker:dexmaker-mockito:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker-dx:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker-mockito:$dexmaker_ver" androidTestImplementation "com.noveogroup.android:android-logger:$android_logger_ver" androidTestImplementation "com.google.code.gson:gson:$gson_ver" androidTestImplementation "com.fasterxml.jackson.core:jackson-databind:$jacksonversion" diff --git a/build.gradle b/build.gradle index ae25e03bb..792783808 100644 --- a/build.gradle +++ b/build.gradle @@ -61,7 +61,7 @@ ext { mockito_ver = "1.10.19" powermock_ver = "1.6.4" support_test_runner_ver = "0.5" - dexmaker_ver = "1.2" + dexmaker_ver = "1.4" espresso_ver = "2.2.2" gson_ver = "2.8.5" group_id = "com.optimizely.ab" diff --git a/datafile-handler/build.gradle b/datafile-handler/build.gradle index dc1a6d857..efa42f69a 100644 --- a/datafile-handler/build.gradle +++ b/datafile-handler/build.gradle @@ -67,8 +67,9 @@ dependencies { // Set this dependency to build and run Espresso tests androidTestImplementation "com.android.support.test.espresso:espresso-core:$espresso_ver" androidTestImplementation "org.mockito:mockito-core:$mockito_ver" - androidTestImplementation "com.google.dexmaker:dexmaker:$dexmaker_ver" - androidTestImplementation "com.google.dexmaker:dexmaker-mockito:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker-dx:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker-mockito:$dexmaker_ver" androidTestImplementation "com.noveogroup.android:android-logger:$android_logger_ver" androidTestImplementation "com.fasterxml.jackson.core:jackson-databind:$jacksonversion" } diff --git a/event-handler/build.gradle b/event-handler/build.gradle index a695959cb..4dad4855e 100644 --- a/event-handler/build.gradle +++ b/event-handler/build.gradle @@ -68,8 +68,9 @@ dependencies { // Set this dependency to build and run Espresso tests androidTestImplementation "com.android.support.test.espresso:espresso-core:$espresso_ver" androidTestImplementation "org.mockito:mockito-core:$mockito_ver" - androidTestImplementation "com.google.dexmaker:dexmaker:$dexmaker_ver" - androidTestImplementation "com.google.dexmaker:dexmaker-mockito:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker-dx:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker-mockito:$dexmaker_ver" androidTestImplementation "com.noveogroup.android:android-logger:$android_logger_ver" androidTestImplementation "com.fasterxml.jackson.core:jackson-databind:$jacksonversion" } diff --git a/shared/build.gradle b/shared/build.gradle index f0c45caec..bb63fbf54 100644 --- a/shared/build.gradle +++ b/shared/build.gradle @@ -71,8 +71,9 @@ dependencies { // Set this dependency to build and run Espresso tests androidTestImplementation "com.android.support.test.espresso:espresso-core:$espresso_ver" androidTestImplementation "org.mockito:mockito-core:$mockito_ver" - androidTestImplementation "com.google.dexmaker:dexmaker:$dexmaker_ver" - androidTestImplementation "com.google.dexmaker:dexmaker-mockito:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker-dx:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker-mockito:$dexmaker_ver" androidTestImplementation "com.noveogroup.android:android-logger:$android_logger_ver" androidTestImplementation "com.fasterxml.jackson.core:jackson-databind:$jacksonversion" } diff --git a/test-app/build.gradle b/test-app/build.gradle index a51de3bda..0e2964ae8 100644 --- a/test-app/build.gradle +++ b/test-app/build.gradle @@ -63,8 +63,9 @@ dependencies { // Set this dependency to build and run Espresso tests androidTestImplementation "com.android.support.test.espresso:espresso-core:$espresso_ver" androidTestImplementation "org.mockito:mockito-core:$mockito_ver" - androidTestImplementation "com.google.dexmaker:dexmaker:$dexmaker_ver" - androidTestImplementation "com.google.dexmaker:dexmaker-mockito:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker-dx:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker-mockito:$dexmaker_ver" // androidTestImplementation 'com.optimizely.ab:android-sdk:1.0.0' androidTestImplementation project(':android-sdk') androidTestImplementation project(path: ':shared') diff --git a/user-profile/build.gradle b/user-profile/build.gradle index 7aa2f086e..1c00467d5 100644 --- a/user-profile/build.gradle +++ b/user-profile/build.gradle @@ -67,8 +67,9 @@ dependencies { // Set this dependency to build and run Espresso tests androidTestImplementation "com.android.support.test.espresso:espresso-core:$espresso_ver" androidTestImplementation "org.mockito:mockito-core:$mockito_ver" - androidTestImplementation "com.google.dexmaker:dexmaker:$dexmaker_ver" - androidTestImplementation "com.google.dexmaker:dexmaker-mockito:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker-dx:$dexmaker_ver" + androidTestImplementation "com.crittercism.dexmaker:dexmaker-mockito:$dexmaker_ver" androidTestImplementation "com.noveogroup.android:android-logger:$android_logger_ver" androidTestImplementation "com.fasterxml.jackson.core:jackson-databind:$jacksonversion" } From b6a3c318f2f9f8298a3cef185a23c0a224e55d2b Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Fri, 6 Mar 2020 16:48:56 -0800 Subject: [PATCH 3/4] clean up test errors --- .../ab/android/test_app/MainActivityEspressoTest.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test-app/src/androidTest/java/com/optimizely/ab/android/test_app/MainActivityEspressoTest.java b/test-app/src/androidTest/java/com/optimizely/ab/android/test_app/MainActivityEspressoTest.java index f5e37b8e7..dfabc3567 100644 --- a/test-app/src/androidTest/java/com/optimizely/ab/android/test_app/MainActivityEspressoTest.java +++ b/test-app/src/androidTest/java/com/optimizely/ab/android/test_app/MainActivityEspressoTest.java @@ -191,16 +191,11 @@ public void experimentActivationForWhitelistUser() throws Exception { Pair event = iterator.next(); final String url = event.first; final String payload = event.second; - if (url.equals("https://logx.optimizely.com/v1/events") && payload.contains("8126664113") && payload.contains("8146590584") + if (url.equals("https://logx.optimizely.com/v1/events") && payload.contains("11178792174") && payload.contains("11146534908") || url.equals("https://logx.optimizely.com/v1/events") && payload.contains("sample_conversion")) { iterator.remove(); } } assertTrue(events.isEmpty()); - MyApplication myApplication = (MyApplication) activityTestRule.getActivity().getApplication(); - UserProfileService userProfileService = myApplication.getOptimizelyManager().getUserProfileService(); - // Being in the white list should override user profile - Map userProfileMap = userProfileService.lookup("test_user"); - assertNull(userProfileMap); } } From 9120c9512bad05613c69c66339f9b5d9ef957e92 Mon Sep 17 00:00:00 2001 From: Jae Kim Date: Tue, 12 May 2020 09:31:37 -0700 Subject: [PATCH 4/4] change interval API to timeunit-based --- .../ab/android/sdk/OptimizelyManager.java | 81 +++++----- .../sdk/OptimizelyManagerBuilderTest.java | 5 - .../sdk/OptimizelyManagerIntervalTest.java | 149 ++++++++++-------- .../ab/android/test_app/MyApplication.java | 4 +- 4 files changed, 126 insertions(+), 113 deletions(-) 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 03e8552df..8e33141af 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 @@ -69,7 +69,7 @@ public class OptimizelyManager { @NonNull private DatafileHandler datafileHandler; private final long datafileDownloadInterval; - private final long eventDispatchInterval; + private final long eventDispatchRetryInterval; @Nullable private EventHandler eventHandler = null; @Nullable private EventProcessor eventProcessor = null; @Nullable private NotificationCenter notificationCenter = null; @@ -90,7 +90,7 @@ public class OptimizelyManager { long datafileDownloadInterval, @NonNull DatafileHandler datafileHandler, @Nullable ErrorHandler errorHandler, - long eventDispatchInterval, + long eventDispatchRetryInterval, @NonNull EventHandler eventHandler, @Nullable EventProcessor eventProcessor, @NonNull UserProfileService userProfileService, @@ -110,7 +110,7 @@ public class OptimizelyManager { this.logger = logger; this.datafileDownloadInterval = datafileDownloadInterval; this.datafileHandler = datafileHandler; - this.eventDispatchInterval = eventDispatchInterval; + this.eventDispatchRetryInterval = eventDispatchRetryInterval; this.eventHandler = eventHandler; this.eventProcessor = eventProcessor; this.errorHandler = errorHandler; @@ -592,7 +592,7 @@ public UserProfileService getUserProfileService() { protected EventHandler getEventHandler(Context context) { if (eventHandler == null) { DefaultEventHandler eventHandler = DefaultEventHandler.getInstance(context); - eventHandler.setDispatchInterval(eventDispatchInterval); + eventHandler.setDispatchInterval(eventDispatchRetryInterval); this.eventHandler = eventHandler; } @@ -722,21 +722,6 @@ public static class Builder { this.projectId = null; } - - /** - * Sets the interval which {@link DatafileService} through the {@link DatafileHandler} will attempt to update the - * cached datafile. If you set this to -1, you disable background updates. If you don't set - * a download interval (or set to less than 0), then no background updates will be scheduled or occur. - * The minimum interval is 900 secs (15 minutes) (enforced by the Android JobScheduler API. See {@link android.app.job.JobInfo}) - * - * @param interval the interval in seconds - * @return this {@link Builder} instance - */ - public Builder withDatafileDownloadInterval(long interval) { - this.datafileDownloadInterval = interval; - return this; - } - /** * Override the default {@link DatafileHandler}. * @param overrideHandler datafile handler to replace default handler @@ -772,15 +757,46 @@ public Builder withErrorHandler(ErrorHandler errorHandler) { return this; } + /** + * Sets the interval which {@link DatafileService} through the {@link DatafileHandler} will attempt to update the + * cached datafile. If you set this to -1, you disable background updates. If you don't set + * a download interval (or set to less than 0), then no background updates will be scheduled or occur. + * The minimum interval is 15 minutes (enforced by the Android JobScheduler API. See {@link android.app.job.JobInfo}) + * + * @param interval the interval + * @param timeUnit the time unit of the timeout argument + * @return this {@link Builder} instance + */ + public Builder withDatafileDownloadInterval(long interval, TimeUnit timeUnit) { + this.datafileDownloadInterval = interval > 0 ? timeUnit.toSeconds(interval) : interval; + return this; + } + + /** + * Sets the interval which {@link DatafileService} through the {@link DatafileHandler} will attempt to update the + * cached datafile. If you set this to -1, you disable background updates. If you don't set + * a download interval (or set to less than 0), then no background updates will be scheduled or occur. + * The minimum interval is 900 secs (15 minutes) (enforced by the Android JobScheduler API. See {@link android.app.job.JobInfo}) + * + * @param interval the interval in seconds + * @return this {@link Builder} instance + */ + @Deprecated + public Builder withDatafileDownloadInterval(long interval) { + this.datafileDownloadInterval = interval; + return this; + } + /** * Sets the interval which queued events will be flushed periodically. * If you don't set this value or set this to -1, the default interval will be used (30 seconds). * - * @param interval the interval in seconds + * @param interval the interval + * @param timeUnit the time unit of the timeout argument * @return this {@link Builder} instance */ - public Builder withEventFlushInterval(long interval) { - this.eventFlushInterval = interval * 1000; + public Builder withEventDispatchInterval(long interval, TimeUnit timeUnit) { + this.eventFlushInterval = interval > 0 ? timeUnit.toMillis(interval) : interval; return this; } @@ -788,11 +804,12 @@ public Builder withEventFlushInterval(long interval) { * Sets the interval which {@link EventIntentService} will retry event dispatch periodically. * If you don't set this value or set this to -1, periodic retries on event dispatch failures will be disabled (but still queued and retried on next event dispatch request) * - * @param interval the interval in seconds + * @param interval the interval + * @param timeUnit the time unit of the timeout argument * @return this {@link Builder} instance */ - public Builder withEventDispatchRetryInterval(long interval) { - this.eventDispatchRetryInterval = interval * 1000; + public Builder withEventDispatchRetryInterval(long interval, TimeUnit timeUnit) { + this.eventDispatchRetryInterval = interval > 0 ? timeUnit.toMillis(interval) : interval; return this; } @@ -810,6 +827,7 @@ public Builder withEventDispatchInterval(long interval) { return this; } + /** * Override the default {@link EventHandler}. * @@ -872,19 +890,6 @@ public OptimizelyManager build(Context context) { } } - if (eventFlushInterval > 1_000_000) { - logger.warn("Event flush interval {} milliseconds is too big", eventFlushInterval); - } - if (eventFlushInterval > 0 && eventFlushInterval < 1000) { - logger.warn("Event flush interval {} milliseconds is too small", eventFlushInterval); - } - if (eventDispatchRetryInterval > 1_000_000) { - logger.warn("Event dispatch retry interval {} milliseconds is too big", eventDispatchRetryInterval); - } - if (eventDispatchRetryInterval > 0 && eventDispatchRetryInterval < 1000) { - logger.warn("Event dispatch retry interval {} milliseconds is too small", eventDispatchRetryInterval); - } - if (datafileConfig == null) { datafileConfig = new DatafileConfig(projectId, sdkKey); } 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 8e28b9319..7dd661e76 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 @@ -21,7 +21,6 @@ import com.optimizely.ab.android.datafile_handler.DatafileHandler; import com.optimizely.ab.android.user_profile.DefaultUserProfileService; import com.optimizely.ab.error.ErrorHandler; -import com.optimizely.ab.event.BatchEventProcessor; import com.optimizely.ab.event.EventHandler; import org.junit.Test; @@ -30,13 +29,9 @@ import org.slf4j.Logger; import static junit.framework.Assert.assertEquals; -import static org.mockito.Matchers.any; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; - @RunWith(MockitoJUnitRunner.class) public class OptimizelyManagerBuilderTest { diff --git a/android-sdk/src/test/java/com/optimizely/ab/android/sdk/OptimizelyManagerIntervalTest.java b/android-sdk/src/test/java/com/optimizely/ab/android/sdk/OptimizelyManagerIntervalTest.java index 4f7cf667f..be83a87e2 100644 --- a/android-sdk/src/test/java/com/optimizely/ab/android/sdk/OptimizelyManagerIntervalTest.java +++ b/android-sdk/src/test/java/com/optimizely/ab/android/sdk/OptimizelyManagerIntervalTest.java @@ -37,6 +37,7 @@ import java.util.concurrent.BlockingQueue; import java.util.concurrent.ExecutorService; +import java.util.concurrent.TimeUnit; import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertFalse; @@ -58,11 +59,83 @@ @PrepareForTest({OptimizelyManager.class, BatchEventProcessor.class}) public class OptimizelyManagerIntervalTest { - private String testProjectId = "7595190003"; private Logger logger; + // DatafileDownloadInterval + + @Test + public void testBuildWithDatafileDownloadInterval() throws Exception { + whenNew(OptimizelyManager.class).withAnyArguments().thenReturn(mock(OptimizelyManager.class)); + + Context appContext = mock(Context.class); + when(appContext.getApplicationContext()).thenReturn(appContext); + + long goodNumber = 27; + OptimizelyManager manager = OptimizelyManager.builder("1") + .withLogger(logger) + .withDatafileDownloadInterval(goodNumber, TimeUnit.MINUTES) + .build(appContext); + + verifyNew(OptimizelyManager.class).withArguments(anyString(), + anyString(), + any(DatafileConfig.class), + any(Logger.class), + eq(goodNumber * 60L), // seconds + any(DatafileHandler.class), + any(ErrorHandler.class), + anyLong(), + any(EventHandler.class), + any(EventProcessor.class), + any(UserProfileService.class), + any(NotificationCenter.class)); + } + + @Test + public void testBuildWithDatafileDownloadIntervalDeprecated() throws Exception { + whenNew(OptimizelyManager.class).withAnyArguments().thenReturn(mock(OptimizelyManager.class)); + + Context appContext = mock(Context.class); + when(appContext.getApplicationContext()).thenReturn(appContext); + + long goodNumber = 1234L; + OptimizelyManager manager = OptimizelyManager.builder("1") + .withLogger(logger) + .withDatafileDownloadInterval(goodNumber) // deprecated + .build(appContext); + + verifyNew(OptimizelyManager.class).withArguments(anyString(), + anyString(), + any(DatafileConfig.class), + any(Logger.class), + eq(goodNumber), // seconds + any(DatafileHandler.class), + any(ErrorHandler.class), + anyLong(), + any(EventHandler.class), + any(EventProcessor.class), + any(UserProfileService.class), + any(NotificationCenter.class)); + } + + @Test + public void testBuildWithInvalidDatafileDownloadInterval() { + Context appContext = mock(Context.class); + Logger logger = mock(Logger.class); + when(appContext.getApplicationContext()).thenReturn(appContext); + + long tooSmallNumber = 60; + OptimizelyManager manager = OptimizelyManager.builder("1") + .withLogger(logger) + .withDatafileDownloadInterval(tooSmallNumber, TimeUnit.SECONDS) + .build(appContext); + + verify(logger).warn("Minimum datafile polling interval is 15 minutes. Defaulting to 15 minutes."); + } + + // EventDispatchInterval + @Test - public void testBuildWithEventFlushInterval() throws Exception { + public void testBuildWithEventDispatchInterval() throws Exception { whenNew(OptimizelyManager.class).withAnyArguments().thenReturn(mock(OptimizelyManager.class)); whenNew(BatchEventProcessor.class).withAnyArguments().thenReturn(mock(BatchEventProcessor.class)); @@ -72,7 +145,7 @@ public void testBuildWithEventFlushInterval() throws Exception { long goodNumber = 100L; OptimizelyManager manager = OptimizelyManager.builder("1") .withLogger(logger) - .withEventFlushInterval(goodNumber) // seconds + .withEventDispatchInterval(goodNumber, TimeUnit.SECONDS) .build(appContext); verifyNew(BatchEventProcessor.class).withArguments(any(BlockingQueue.class), @@ -111,7 +184,7 @@ public void testBuildWithEventDispatchRetryInterval() throws Exception { OptimizelyManager manager = OptimizelyManager.builder("1") .withLogger(logger) - .withEventDispatchRetryInterval(goodNumber) // seconds + .withEventDispatchRetryInterval(goodNumber, TimeUnit.MINUTES) .build(appContext); verifyNew(BatchEventProcessor.class).withArguments(any(BlockingQueue.class), @@ -130,7 +203,7 @@ public void testBuildWithEventDispatchRetryInterval() throws Exception { anyLong(), any(DatafileHandler.class), any(ErrorHandler.class), - eq(goodNumber * 1000L), // milliseconds + eq(goodNumber * 1000L * 60L), // milliseconds any(EventHandler.class), any(EventProcessor.class), any(UserProfileService.class), @@ -138,17 +211,17 @@ public void testBuildWithEventDispatchRetryInterval() throws Exception { } @Test - public void testBuildWithEventDispatchInterval() throws Exception { + public void testBuildWithEventDispatchIntervalDeprecated() throws Exception { whenNew(OptimizelyManager.class).withAnyArguments().thenReturn(mock(OptimizelyManager.class)); whenNew(BatchEventProcessor.class).withAnyArguments().thenReturn(mock(BatchEventProcessor.class)); Context appContext = mock(Context.class); when(appContext.getApplicationContext()).thenReturn(appContext); - long goodNumber = 100L*1000L; + long goodNumber = 1234L; OptimizelyManager manager = OptimizelyManager.builder("1") .withLogger(logger) - .withEventDispatchInterval(goodNumber) // milliseconds + .withEventDispatchInterval(goodNumber) // deprecated .build(appContext); verifyNew(BatchEventProcessor.class).withArguments(any(BlockingQueue.class), @@ -174,64 +247,4 @@ public void testBuildWithEventDispatchInterval() throws Exception { any(NotificationCenter.class)); } - @Test - public void testBuildWithInvalidEventDispatchInterval() { - Context appContext = mock(Context.class); - Logger logger = mock(Logger.class); - when(appContext.getApplicationContext()).thenReturn(appContext); - - long tooSmallNumber = 100L; - OptimizelyManager manager = OptimizelyManager.builder("1") - .withLogger(logger) - .withEventDispatchInterval(tooSmallNumber) - .build(appContext); - - verify(logger).warn("Event flush interval {} milliseconds is too small", tooSmallNumber); - } - - @Test - public void testBuildWithValidEventDispatchInterval() { - Context appContext = mock(Context.class); - Logger logger = mock(Logger.class); - when(appContext.getApplicationContext()).thenReturn(appContext); - - long goodNumber = 100L * 1000L; - OptimizelyManager manager = OptimizelyManager.builder("1") - .withLogger(logger) - .withEventDispatchInterval(goodNumber) - .build(appContext); - - verify(logger, never()).warn(any(String.class), any(Integer.class)); - } - - @Test - public void testBuildWithInvalidEventFlushInterval() { - Context appContext = mock(Context.class); - Logger logger = mock(Logger.class); - when(appContext.getApplicationContext()).thenReturn(appContext); - - long tooBigNumber = 100L * 1000L; - OptimizelyManager manager = OptimizelyManager.builder("1") - .withLogger(logger) - .withEventFlushInterval(tooBigNumber) - .build(appContext); - - verify(logger).warn("Event flush interval {} milliseconds is too big", tooBigNumber * 1000L); - } - - @Test - public void testBuildWithValidEventFlushInterval() { - Context appContext = mock(Context.class); - Logger logger = mock(Logger.class); - when(appContext.getApplicationContext()).thenReturn(appContext); - - long goodNumber = 100L; - OptimizelyManager manager = OptimizelyManager.builder("1") - .withLogger(logger) - .withEventFlushInterval(goodNumber) - .build(appContext); - - verify(logger, never()).warn(any(String.class), any(Integer.class)); - } - } diff --git a/test-app/src/main/java/com/optimizely/ab/android/test_app/MyApplication.java b/test-app/src/main/java/com/optimizely/ab/android/test_app/MyApplication.java index 3d183c6bc..899b07099 100644 --- a/test-app/src/main/java/com/optimizely/ab/android/test_app/MyApplication.java +++ b/test-app/src/main/java/com/optimizely/ab/android/test_app/MyApplication.java @@ -75,8 +75,8 @@ public void onCreate() { OptimizelyManager.Builder builder = OptimizelyManager.builder(); optimizelyManager = builder - .withDatafileDownloadInterval(TimeUnit.MINUTES.toSeconds(15)) - .withEventFlushInterval(60) + .withDatafileDownloadInterval(15, TimeUnit.MINUTES) + .withEventDispatchInterval(60, TimeUnit.SECONDS) .withSDKKey("FCnSegiEkRry9rhVMroit4") .build(getApplicationContext()); }