Skip to content
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
e729d75
first attempt at using the project config manager with android
thomaszurkan-optimizely Jun 26, 2019
61e7459
add datafile change notification
thomaszurkan-optimizely Jun 26, 2019
d7aaa17
fix unit tests
thomaszurkan-optimizely Jun 26, 2019
0f58881
update to latest build versions
thomaszurkan-optimizely Jun 27, 2019
289dab7
trying to fix travis
thomaszurkan-optimizely Jun 27, 2019
30258fd
try to get travis working
thomaszurkan-optimizely Jun 27, 2019
8034458
remove audio from emulator start
thomaszurkan-optimizely Jun 27, 2019
27d3202
incorrect check for currentConfig
thomaszurkan-optimizely Jun 27, 2019
cccaf11
fix travis yaml
thomaszurkan-optimizely Jun 27, 2019
eb6bba3
use 3.2.0 java-sdk
thomaszurkan-optimizely Jun 27, 2019
a295664
fall back to tools 28.0.3
thomaszurkan-optimizely Jun 27, 2019
c619ce6
cleanup travis yaml
thomaszurkan-optimizely Jun 27, 2019
253ba75
trying to fix travis build
thomaszurkan-optimizely Jun 27, 2019
07db97b
respond to comments, slight refactor.
thomaszurkan-optimizely Jun 27, 2019
9999cd7
use datafileloaded as datafile change. deprecate datafile handler in…
thomaszurkan-optimizely Jul 1, 2019
802c514
one more slight refactor. remove datafile config from constructor of …
thomaszurkan-optimizely Jul 2, 2019
2ae390e
slight refactor for setConfig
thomaszurkan-optimizely Jul 2, 2019
fe2d2d9
cleanup test-app a little
thomaszurkan-optimizely Jul 2, 2019
eaab9a4
cleanup from PR comments
thomaszurkan-optimizely Jul 9, 2019
435681e
removed not from null datafile or empty datafile check (#279)
mnoman09 Jul 11, 2019
0847b7b
add a few more null checks
thomaszurkan-optimizely Jul 11, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@ env:
- ADB_INSTALL_TIMEOUT=5 # minutes
- ANDROID_API=28 # api is same as gradle file
matrix:
- EMULATOR_API=22
- EMULATOR_API=21
- EMULATOR_API=22
android:
components:
- tools
- platform-tools
- tools
- platform-tools-$ANDROID_BUILD_TOOLS
- build-tools-$ANDROID_BUILD_TOOLS
- android-$ANDROID_API
- android-$EMULATOR_API
Expand All @@ -35,7 +34,7 @@ cache:
- $HOME/.gradle/caches/
- $HOME/.gradle/wrapper/
before_install:
- yes | sdkmanager "platforms;android-28"
#- yes | sdkmanager "platforms;android-$ANDROID_API"
before_script:
- echo $TRAVIS_BRANCH
- echo $TRAVIS_TAG
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,9 @@ public OptimizelyClientTest(int datafileVersion,String datafile){
eventHandler = spy(DefaultEventHandler.getInstance(InstrumentationRegistry.getTargetContext()));
optimizely = Optimizely.builder(datafile, eventHandler).build();
if(datafileVersion==3) {
when(bucketer.bucket(optimizely.getProjectConfig().getExperiments().get(0), GENERIC_USER_ID)).thenReturn(optimizely.getProjectConfig().getExperiments().get(0).getVariations().get(0));
when(bucketer.bucket(optimizely.getProjectConfig().getExperiments().get(0), GENERIC_USER_ID, optimizely.getProjectConfig())).thenReturn(optimizely.getProjectConfig().getExperiments().get(0).getVariations().get(0));
} else {
when(bucketer.bucket(optimizely.getProjectConfig().getExperimentKeyMapping().get(FEATURE_MULTI_VARIATE_EXPERIMENT_KEY), GENERIC_USER_ID)).thenReturn(optimizely.getProjectConfig().getExperimentKeyMapping().get(FEATURE_MULTI_VARIATE_EXPERIMENT_KEY).getVariations().get(1));
when(bucketer.bucket(optimizely.getProjectConfig().getExperimentKeyMapping().get(FEATURE_MULTI_VARIATE_EXPERIMENT_KEY), GENERIC_USER_ID, optimizely.getProjectConfig())).thenReturn(optimizely.getProjectConfig().getExperimentKeyMapping().get(FEATURE_MULTI_VARIATE_EXPERIMENT_KEY).getVariations().get(1));
}
spyOnConfig();
} catch (Exception configException) {
Expand Down Expand Up @@ -356,7 +356,7 @@ public void testGoodActivationBucketingId() {
Experiment experiment = optimizelyClient.getProjectConfig().getExperimentKeyMapping().get(FEATURE_ANDROID_EXPERIMENT_KEY);
attributes.put(BUCKETING_ATTRIBUTE, bucketingId);
Variation v = optimizelyClient.activate(FEATURE_ANDROID_EXPERIMENT_KEY, GENERIC_USER_ID, attributes);
verify(bucketer).bucket( experiment, bucketingId);
verify(bucketer).bucket(experiment, bucketingId, optimizely.getProjectConfig());
}

@Test
Expand Down Expand Up @@ -881,7 +881,7 @@ public void testGoodGetVariationBucketingId() {
Map<String, String> attributes = new HashMap<>();
attributes.put(BUCKETING_ATTRIBUTE, bucketingId);
Variation v = optimizelyClient.getVariation("android_experiment_key", "userId", attributes);
verify(bucketer).bucket(experiment, bucketingId);
verify(bucketer).bucket(experiment, bucketingId, optimizely.getProjectConfig());
}

@Test
Expand Down Expand Up @@ -1027,9 +1027,9 @@ public void testGoodGetProjectConfigForced() {
logger);
ProjectConfig config = optimizelyClient.getProjectConfig();
assertNotNull(config);
assertTrue(config.setForcedVariation(FEATURE_ANDROID_EXPERIMENT_KEY, GENERIC_USER_ID, "var_1"));
assertEquals(config.getForcedVariation(FEATURE_ANDROID_EXPERIMENT_KEY, GENERIC_USER_ID), config.getExperimentKeyMapping().get(FEATURE_ANDROID_EXPERIMENT_KEY).getVariations().get(0));
assertTrue(config.setForcedVariation(FEATURE_ANDROID_EXPERIMENT_KEY, GENERIC_USER_ID, null));
assertTrue(optimizelyClient.setForcedVariation(FEATURE_ANDROID_EXPERIMENT_KEY, GENERIC_USER_ID, "var_1"));
assertEquals(optimizelyClient.getForcedVariation(FEATURE_ANDROID_EXPERIMENT_KEY, GENERIC_USER_ID), config.getExperimentKeyMapping().get(FEATURE_ANDROID_EXPERIMENT_KEY).getVariations().get(0));
assertTrue(optimizelyClient.setForcedVariation(FEATURE_ANDROID_EXPERIMENT_KEY, GENERIC_USER_ID, null));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.optimizely.ab.OptimizelyRuntimeException;
import com.optimizely.ab.android.datafile_handler.DefaultDatafileHandler;
import com.optimizely.ab.android.event_handler.DefaultEventHandler;
import com.optimizely.ab.android.shared.DatafileConfig;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import seems unnecessary.

import com.optimizely.ab.android.user_profile.DefaultUserProfileService;
import com.optimizely.ab.config.Experiment;
import com.optimizely.ab.config.ProjectConfig;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
import com.optimizely.ab.android.shared.ServiceScheduler;
import com.optimizely.ab.android.user_profile.DefaultUserProfileService;
import com.optimizely.ab.bucketing.UserProfileService;
import com.optimizely.ab.config.DatafileProjectConfig;
import com.optimizely.ab.config.ProjectConfig;
import com.optimizely.ab.config.Variation;
import com.optimizely.ab.event.EventHandler;

Expand Down Expand Up @@ -74,6 +76,7 @@ public class OptimizelyManagerTest {
private ListeningExecutorService executor;
private Logger logger;
private OptimizelyManager optimizelyManager;
private DefaultDatafileHandler defaultDatafileHandler;

private String minDatafile = "{\n" +
"experiments: [ ],\n" +
Expand All @@ -88,19 +91,24 @@ public class OptimizelyManagerTest {
"}";

@Before
public void setup() {
public void setup() throws Exception {
logger = mock(Logger.class);
executor = MoreExecutors.newDirectExecutorService();
DatafileHandler datafileHandler = mock(DefaultDatafileHandler.class);
defaultDatafileHandler = mock(DefaultDatafileHandler.class);
EventHandler eventHandler = mock(DefaultEventHandler.class);
optimizelyManager = new OptimizelyManager(testProjectId, null, null, logger, 3600L, datafileHandler, null, 3600L,
optimizelyManager = new OptimizelyManager(testProjectId, null, null, logger, 3600L, defaultDatafileHandler, null, 3600L,
eventHandler, null);
String datafile = optimizelyManager.getDatafile(InstrumentationRegistry.getTargetContext(), R.raw.datafile);
ProjectConfig config = new DatafileProjectConfig.Builder().withDatafile(datafile).build();

when(defaultDatafileHandler.getConfig()).thenReturn(config);
}

@Test
public void initializeIntUseForcedVariation() {
optimizelyManager.initialize(InstrumentationRegistry.getTargetContext(), R.raw.datafile);


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems extraneous. Remove.

assertTrue(optimizelyManager.getOptimizely().setForcedVariation("android_experiment_key", "1", "var_1"));
Variation variation = optimizelyManager.getOptimizely().getForcedVariation("android_experiment_key", "1");
assertEquals(variation.getKey(), "var_1");
Expand All @@ -116,7 +124,7 @@ public void initializeInt() {

assertEquals(optimizelyManager.getDatafileUrl(), "https://cdn.optimizely.com/json/7595190003.json" );

verify(optimizelyManager.getDatafileHandler()).startBackgroundUpdates(eq(InstrumentationRegistry.getTargetContext()), eq(new DatafileConfig(testProjectId, null)), eq(3600L));
verify(optimizelyManager.getDatafileHandler()).startBackgroundUpdates(eq(InstrumentationRegistry.getTargetContext()), eq(new DatafileConfig(testProjectId, null)), eq(3600L), any(DatafileLoadedListener.class));
assertNotNull(optimizelyManager.getOptimizely());
assertNotNull(optimizelyManager.getDatafileHandler());

Expand Down Expand Up @@ -169,6 +177,7 @@ public void initializeSyncWithEmptyDatafile() {
Context appContext = mock(Context.class);
when(context.getApplicationContext()).thenReturn(appContext);
when(appContext.getPackageName()).thenReturn("com.optly");
when(defaultDatafileHandler.getConfig()).thenReturn(null);
optimizelyManager.initialize(InstrumentationRegistry.getTargetContext(), R.raw.emptydatafile);
assertFalse(optimizelyManager.getOptimizely().isValid());
}
Expand Down Expand Up @@ -227,7 +236,7 @@ public void onStart(OptimizelyClient optimizely) {
};
optimizelyManager.initialize(InstrumentationRegistry.getContext(), R.raw.datafile, listener);

verify(optimizelyManager.getDatafileHandler()).startBackgroundUpdates(any(Context.class), eq(new DatafileConfig(testProjectId, testSdkKey)), eq(3600L));
verify(optimizelyManager.getDatafileHandler()).startBackgroundUpdates(any(Context.class), eq(new DatafileConfig(testProjectId, testSdkKey)), eq(3600L), any(DatafileLoadedListener.class));


assertEquals(optimizelyManager.isDatafileCached(InstrumentationRegistry.getTargetContext()), false);
Expand Down Expand Up @@ -261,6 +270,7 @@ public void initializeWithEmptyDatafile() {
Context appContext = mock(Context.class);
when(context.getApplicationContext()).thenReturn(appContext);
when(appContext.getPackageName()).thenReturn("com.optly");
when(defaultDatafileHandler.getConfig()).thenReturn(null);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surprised this mock works since we already mocked with:

when(defaultDatafileHandler.getConfig()).thenReturn(config);

in setup(). Should we reset(defaultDatafileHandler) first?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the when override works fine. these are unit tests. however, it might also be possible to just include the default datafile handler instead of a mock and achieve the same thing.


String emptyString = "";

Expand All @@ -274,6 +284,7 @@ public void initializeWithMalformedDatafile() {
Context appContext = mock(Context.class);
when(context.getApplicationContext()).thenReturn(appContext);
when(appContext.getPackageName()).thenReturn("com.optly");
when(defaultDatafileHandler.getConfig()).thenReturn(null);

String emptyString = "malformed data";

Expand Down Expand Up @@ -347,7 +358,7 @@ public void injectOptimizely() {

verify(logger).info("Sending Optimizely instance to listener");
verify(startListener).onStart(any(OptimizelyClient.class));
verify(optimizelyManager.getDatafileHandler()).startBackgroundUpdates(eq(context), eq(new DatafileConfig(testProjectId, null)), eq(3600L));
verify(optimizelyManager.getDatafileHandler()).startBackgroundUpdates(eq(context), eq(new DatafileConfig(testProjectId, null)), eq(3600L), any(DatafileLoadedListener.class));

}

Expand All @@ -366,7 +377,7 @@ public void injectOptimizelyWithDatafileLisener() {
fail("Timed out");
}

verify(optimizelyManager.getDatafileHandler()).startBackgroundUpdates(eq(context), eq(new DatafileConfig(testProjectId, null)), eq(3600L));
verify(optimizelyManager.getDatafileHandler()).startBackgroundUpdates(eq(context), eq(new DatafileConfig(testProjectId, null)), eq(3600L), any(DatafileLoadedListener.class));
verify(logger).info("Sending Optimizely instance to listener");
verify(startListener).onStart(any(OptimizelyClient.class));
}
Expand Down Expand Up @@ -435,6 +446,7 @@ public void injectOptimizelyHandlesInvalidDatafile() {
ArgumentCaptor<DefaultUserProfileService.StartCallback> callbackArgumentCaptor =
ArgumentCaptor.forClass(DefaultUserProfileService.StartCallback.class);

when(defaultDatafileHandler.getConfig()).thenReturn(null);
optimizelyManager.setOptimizelyStartListener(null);
optimizelyManager.injectOptimizely(context, userProfileService, "{}");
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
import com.optimizely.ab.error.ErrorHandler;
import com.optimizely.ab.event.EventHandler;
import com.optimizely.ab.event.internal.payload.EventBatch;
import com.optimizely.ab.notification.NotificationCenter;
import com.optimizely.ab.notification.UpdateConfigNotification;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -184,11 +186,7 @@ protected OptimizelyClient initialize(@NonNull Context context, @Nullable String
defaultUserProfileService.start();
}
optimizelyClient = buildOptimizely(context, datafile);

if (datafileDownloadInterval > 0 && datafileHandler != null) {
datafileHandler.startBackgroundUpdates(context, datafileConfig, datafileDownloadInterval);
}

startDatafileHandler(context);
}
else {
logger.error("Invalid datafile");
Expand Down Expand Up @@ -343,9 +341,6 @@ public void onDatafileLoaded(@Nullable String datafile) {
injectOptimizely(context, userProfileService, getDatafile(context,datafileRes));
}
}

@Override
public void onStop(Context context) {}
};
}

Expand Down Expand Up @@ -434,17 +429,32 @@ public DatafileHandler getDatafileHandler() {
return datafileHandler;
}

private void startDatafileHandler(Context context) {
if (datafileDownloadInterval <= 0) {
logger.debug("Invalid download interval, ignoring background updates.");
return;
}

datafileHandler.startBackgroundUpdates(context, datafileConfig, datafileDownloadInterval, datafile1 -> {
NotificationCenter notificationCenter = getOptimizely().getNotificationCenter();
if (notificationCenter == null) {
logger.debug("NotificationCenter null, not sending notification");
return;
}
notificationCenter.send(new UpdateConfigNotification());
});
}

@RequiresApi(api = Build.VERSION_CODES.HONEYCOMB)
void injectOptimizely(@NonNull final Context context, final @NonNull UserProfileService userProfileService,
@NonNull final String datafile) {

if (datafileDownloadInterval > 0 && datafileHandler != null) {
datafileHandler.startBackgroundUpdates(context, datafileConfig, datafileDownloadInterval);
}
try {
optimizelyClient = buildOptimizely(context, datafile);
optimizelyClient.setDefaultAttributes(OptimizelyDefaultAttributes.buildDefaultAttributesMap(context, logger));

startDatafileHandler(context);

if (userProfileService instanceof DefaultUserProfileService) {
((DefaultUserProfileService) userProfileService).startInBackground(new DefaultUserProfileService.StartCallback() {
@Override
Expand Down Expand Up @@ -483,20 +493,26 @@ private OptimizelyClient buildOptimizely(@NonNull Context context, @NonNull Stri

EventBatch.ClientEngine clientEngine = OptimizelyClientEngine.getClientEngineFromContext(context);

Optimizely.Builder builder = Optimizely.builder(datafile, eventHandler)
.withClientEngine(clientEngine)
Optimizely.Builder builder = Optimizely.builder();

if (datafileHandler instanceof DefaultDatafileHandler) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this logic should be part of the OptimizelyManager.build() and not this method. This would make this much cleaner.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At build time the developer has not called an initialize/start. It was decided that there would be explicit ways to initialize sync and async. So, moving it would break backward compatibility. However, this does bring up the point on whether we should also be be deprecating the initialize methods.

DefaultDatafileHandler handler = (DefaultDatafileHandler)datafileHandler;
handler.setDatafile(datafile);
builder.withConfigManager(handler);
builder.withEventHandler(eventHandler);
}
else {
builder = Optimizely.builder(datafile, eventHandler);
}

builder.withClientEngine(clientEngine)
.withClientVersion(BuildConfig.CLIENT_VERSION);

if (errorHandler != null) {
builder.withErrorHandler(errorHandler);
}
if (userProfileService != null) {
builder.withUserProfileService(userProfileService);
}
else {
// the builder creates the default user profile service. So, this should never happen.
userProfileService = DefaultUserProfileService.newInstance(datafileConfig.getKey(), context);
builder.withUserProfileService(userProfileService);
}

builder.withUserProfileService(userProfileService);

Optimizely optimizely = builder.build();
return new OptimizelyClient(optimizely, LoggerFactory.getLogger(OptimizelyClient.class));
Expand Down Expand Up @@ -752,6 +768,10 @@ public OptimizelyManager build(Context context) {
}
}

if (datafileConfig == null) {
datafileConfig = new DatafileConfig(projectId, sdkKey);
}

if (datafileHandler == null) {
datafileHandler = new DefaultDatafileHandler();
}
Expand Down
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ ext {
build_tools_version = "28.0.3"
min_sdk_version = 14
target_sdk_version = 28
java_core_ver = "3.1.0"
java_core_ver = "3.2.0"
android_logger_ver = "1.3.6"
jacksonversion= "2.9.8"
support_annotations_ver = "24.2.1"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,6 @@ public void onDatafileLoaded(@Nullable String dataFile) {
assertNull(dataFile);
}

@Override
public void onStop(Context context) {

}
});

}
Expand All @@ -116,11 +112,6 @@ public void testAsyncDownloadWithEnvironment() throws Exception {
public void onDatafileLoaded(@Nullable String dataFile) {
assertNull(dataFile);
}

@Override
public void onStop(Context context) {

}
});

}
Expand All @@ -130,7 +121,7 @@ public void testBackgroundWithoutEnvironment() throws Exception {
// Context of the app under test.
Context appContext = InstrumentationRegistry.getTargetContext();

datafileHandler.startBackgroundUpdates(appContext, new DatafileConfig("1", null), 24 * 60 * 60L);
datafileHandler.startBackgroundUpdates(appContext, new DatafileConfig("1", null), 24 * 60 * 60L, null);

assertTrue(true);

Expand All @@ -144,7 +135,7 @@ public void testBackgroundWithEnvironment() throws Exception {
// Context of the app under test.
Context appContext = InstrumentationRegistry.getTargetContext();

datafileHandler.startBackgroundUpdates(appContext, new DatafileConfig("1", "2"), 24 * 60 * 60L);
datafileHandler.startBackgroundUpdates(appContext, new DatafileConfig("1", "2"), 24 * 60 * 60L, null);

assertTrue(true);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,21 @@

import android.content.Context;

import com.optimizely.ab.config.ProjectConfig;
import com.optimizely.ab.config.ProjectConfigManager;
import com.optimizely.ab.android.shared.DatafileConfig;

import java.util.function.Function;

/**
* DatafileHandler
* @deprecated
* This interface will be replaced by the ProjectConfigManager. If you are implementing this interface moving forward,
* you will also need to implement the {@link ProjectConfigManager} .
* class that is used to interact with the datafile_handler module. This interface can be
* overridden so that the sdk user can provide a override for the default DatafileHandler.
*/
@Deprecated
public interface DatafileHandler {
/**
* Synchronous call to download the datafile.
Expand All @@ -50,8 +58,9 @@ public interface DatafileHandler {
* @param context application context for download
* @param datafileConfig DatafileConfig for the datafile
* @param updateInterval frequency of updates in seconds
* @param listener function to call when a new datafile has been detected.
*/
void startBackgroundUpdates(Context context, DatafileConfig datafileConfig, Long updateInterval);
void startBackgroundUpdates(Context context, DatafileConfig datafileConfig, Long updateInterval, DatafileLoadedListener listener);

/**
* Stop the background updates.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,4 @@ public interface DatafileLoadedListener {
*
*/
void onDatafileLoaded(@Nullable String dataFile);

/**
* The datafile download stopped for some reason.
*
* @param context application context
*/
void onStop(Context context);
}
Loading