Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: convert jobscheduler to work manager #360

Merged
merged 14 commits into from
Feb 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/****************************************************************************
* Copyright 2016, Optimizely, Inc. and contributors *
* Copyright 2016-2021, 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. *
Expand All @@ -16,13 +16,14 @@

package com.optimizely.ab.android.sdk;

import java.util.HashMap;
import java.util.Map;
import android.content.Context;
import android.content.pm.PackageInfo;

import org.slf4j.Logger;

import java.util.HashMap;
import java.util.Map;

/**
* Class to encapsulate default attributes that will be added to attributes passed in
* by the OptimizelyClient.
Expand All @@ -45,15 +46,15 @@ public class OptimizelyDefaultAttributes {
static Map<String, String> buildDefaultAttributesMap(Context context, Logger logger) {
String androidDeviceModel = android.os.Build.MODEL;
String androidOSVersion = android.os.Build.VERSION.RELEASE;
int androidSdkVersion = 0;
String androidSdkVersion = "";
String androidSdkVersionName = "";
String androidAppVersionName = "";
int androidAppVersion = 0;

// In case there is some problem with accessing the BuildConfig file....
try {
androidSdkVersion = BuildConfig.VERSION_CODE;
androidSdkVersionName = BuildConfig.VERSION_NAME;
androidSdkVersion = BuildConfig.CLIENT_VERSION;
androidSdkVersionName = BuildConfig.LIBRARY_PACKAGE_NAME;
}
catch (Exception e) {
logger.warn("Error getting BuildConfig version code and version name");
Expand Down
3 changes: 2 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ buildscript {
google()
}
dependencies {
classpath 'com.android.tools.build:gradle:4.0.1'
classpath 'com.android.tools.build:gradle:4.1.2'
classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlin_version"

// NOTE: Do not place your application dependencies here; they belong
Expand Down Expand Up @@ -71,6 +71,7 @@ ext {
gson_ver = "2.8.6"
group_id = "com.optimizely.ab"
androidx_test = "1.1.1"
work_runtime = "2.5.0"
}

task clean(type: Delete) {
Expand Down
2 changes: 1 addition & 1 deletion datafile-handler/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ android {
dependencies {
api project(':shared')
implementation "androidx.annotation:annotation:$annotations_ver"
implementation "androidx.work:work-runtime:$work_runtime"

compileOnly "com.noveogroup.android:android-logger:$android_logger_ver"

Expand Down Expand Up @@ -131,4 +132,3 @@ android.libraryVariants.all { variant ->
project.artifacts.add("archives", tasks["${variant.name}SourcesJar"]);
}


Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
@RunWith(AndroidJUnit4.class)
public class DatafileLoaderTest {

private DatafileService datafileService;
private DatafileCache datafileCache;
private DatafileClient datafileClient;
private Client client;
Expand All @@ -71,15 +70,12 @@ public class DatafileLoaderTest {

@Before
public void setup() {
datafileService = mock(DatafileService.class);
logger = mock(Logger.class);
final Context targetContext = InstrumentationRegistry.getInstrumentation().getTargetContext();
datafileCache = new DatafileCache("1", new Cache(targetContext, logger), logger);
client = mock(Client.class);
datafileClient = new DatafileClient(client, logger);
datafileLoadedListener = mock(DatafileLoadedListener.class);
when(datafileService.getApplicationContext()).thenReturn(targetContext);
when(datafileService.isBound()).thenReturn(true);
}

@After
Expand All @@ -91,7 +87,7 @@ public void tearDown() {
public void loadFromCDNWhenNoCachedFile() throws MalformedURLException, JSONException {
final ExecutorService executor = Executors.newSingleThreadExecutor();
DatafileLoader datafileLoader =
new DatafileLoader(datafileService, datafileClient, datafileCache, executor, logger);
new DatafileLoader(context, datafileClient, datafileCache, logger);

when(client.execute(any(Client.Request.class), anyInt(), anyInt())).thenReturn("{}");

Expand All @@ -112,7 +108,7 @@ public void loadFromCDNWhenNoCachedFile() throws MalformedURLException, JSONExce
public void loadWhenCacheFileExistsAndCDNNotModified() {
final ExecutorService executor = Executors.newSingleThreadExecutor();
DatafileLoader datafileLoader =
new DatafileLoader(datafileService, datafileClient, datafileCache, executor, logger);
new DatafileLoader(context, datafileClient, datafileCache, logger);
datafileCache.save("{}");

when(client.execute(any(Client.Request.class), anyInt(), anyInt())).thenReturn("");
Expand All @@ -134,7 +130,7 @@ public void loadWhenCacheFileExistsAndCDNNotModified() {
public void noCacheAndLoadFromCDNFails() {
final ExecutorService executor = Executors.newSingleThreadExecutor();
DatafileLoader datafileLoader =
new DatafileLoader(datafileService, datafileClient, datafileCache, executor, logger);
new DatafileLoader(context, datafileClient, datafileCache, logger);

when(client.execute(any(Client.Request.class), anyInt(), anyInt())).thenReturn(null);

Expand All @@ -156,7 +152,7 @@ public void warningsAreLogged() throws IOException {
Cache cache = mock(Cache.class);
datafileCache = new DatafileCache("warningsAreLogged", cache, logger);
DatafileLoader datafileLoader =
new DatafileLoader(datafileService, datafileClient, datafileCache, executor, logger);
new DatafileLoader(context, datafileClient, datafileCache, logger);

when(client.execute(any(Client.Request.class), anyInt(), anyInt())).thenReturn("{}");
when(cache.exists(datafileCache.getFileName())).thenReturn(true);
Expand All @@ -181,17 +177,22 @@ public void debugLogged() throws IOException {
Cache cache = mock(Cache.class);
datafileCache = new DatafileCache("debugLogged", cache, logger);
DatafileLoader datafileLoader =
new DatafileLoader(datafileService, datafileClient, datafileCache, executor, logger);
new DatafileLoader(context, datafileClient, datafileCache, logger);

when(client.execute(any(Client.Request.class), anyInt(), anyInt())).thenReturn("{}");
when(cache.save(datafileCache.getFileName(), "{}")).thenReturn(true);
when(cache.exists(datafileCache.getFileName())).thenReturn(true);
when(cache.load(datafileCache.getFileName())).thenReturn("{}");

datafileLoader.getDatafile("debugLogged", datafileLoadedListener);
try {
Thread.sleep(100);
} catch (InterruptedException e) {
e.printStackTrace();
}
datafileLoader.getDatafile("debugLogged", datafileLoadedListener);
try {
executor.awaitTermination(5, TimeUnit.SECONDS);
executor.awaitTermination(1, TimeUnit.SECONDS);
} catch (InterruptedException e) {
fail();
}
Expand All @@ -207,7 +208,7 @@ public void downloadAllowedNoCache() throws IOException {
Cache cache = mock(Cache.class);
datafileCache = new DatafileCache("downloadAllowedNoCache", cache, logger);
DatafileLoader datafileLoader =
new DatafileLoader(datafileService, datafileClient, datafileCache, executor, logger);
new DatafileLoader(context, datafileClient, datafileCache, logger);

when(client.execute(any(Client.Request.class), anyInt(), anyInt())).thenReturn("{}");
when(cache.save(datafileCache.getFileName(), "{}")).thenReturn(false);
Expand All @@ -233,7 +234,7 @@ public void debugLoggedMultiThreaded() throws IOException {
Cache cache = mock(Cache.class);
datafileCache = new DatafileCache("debugLoggedMultiThreaded", cache, logger);
DatafileLoader datafileLoader =
new DatafileLoader(datafileService, datafileClient, datafileCache, executor, logger);
new DatafileLoader(context, datafileClient, datafileCache, logger);

when(client.execute(any(Client.Request.class), anyInt(), anyInt())).thenReturn("{}");
when(cache.exists(datafileCache.getFileName())).thenReturn(true);
Expand All @@ -257,8 +258,7 @@ public void debugLoggedMultiThreaded() throws IOException {
fail();
}

verify(logger, atLeast(1)).debug("Last download happened under 1 minute ago. Throttled to be at least 1 minute apart.");
verify(datafileLoadedListener, atMost(4)).onDatafileLoaded("{}");
verify(datafileLoadedListener, atMost(5)).onDatafileLoaded("{}");
verify(datafileLoadedListener, atLeast(1)).onDatafileLoaded("{}");
}

Expand Down Expand Up @@ -289,7 +289,7 @@ public void allowDoubleDownload() throws IOException {
Cache cache = mock(Cache.class);
datafileCache = new DatafileCache("allowDoubleDownload", cache, logger);
DatafileLoader datafileLoader =
new DatafileLoader(datafileService, datafileClient, datafileCache, executor, logger);
new DatafileLoader(context, datafileClient, datafileCache, logger);

// set download time to 1 second
setTestDownloadFrequency(datafileLoader, 1000L);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import android.content.Context;
import android.content.Intent;
import android.os.Build;

import androidx.test.platform.app.InstrumentationRegistry;

import com.optimizely.ab.android.shared.Cache;
Expand All @@ -34,7 +35,6 @@
import org.slf4j.Logger;

import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertNull;
import static org.mockito.Matchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
Expand Down Expand Up @@ -99,8 +99,7 @@ public void dispatchingOneWithoutEnvironment() {
backgroundWatchersCache.setIsWatching(new DatafileConfig("1", null), true);
Logger logger = mock(Logger.class);
DatafileRescheduler.Dispatcher dispatcher = new DatafileRescheduler.Dispatcher(mockContext, backgroundWatchersCache, logger);
Intent intent = new Intent(mockContext, DatafileService.class);
dispatcher.dispatch(intent);
dispatcher.dispatch();
ArgumentCaptor<Intent> captor = ArgumentCaptor.forClass(Intent.class);
verify(mockContext).startService(captor.capture());
assertEquals(new DatafileConfig("1", null).toJSONString(), captor.getValue().getStringExtra(DatafileService.EXTRA_DATAFILE_CONFIG));
Expand All @@ -116,8 +115,7 @@ public void dispatchingOneWithEnvironment() {
backgroundWatchersCache.setIsWatching(new DatafileConfig("1", "2"), true);
Logger logger = mock(Logger.class);
DatafileRescheduler.Dispatcher dispatcher = new DatafileRescheduler.Dispatcher(mockContext, backgroundWatchersCache, logger);
Intent intent = new Intent(mockContext, DatafileService.class);
dispatcher.dispatch(intent);
dispatcher.dispatch();
ArgumentCaptor<Intent> captor = ArgumentCaptor.forClass(Intent.class);
verify(mockContext).startService(captor.capture());
assertEquals(new DatafileConfig("1", "2").toJSONString(), captor.getValue().getStringExtra(DatafileService.EXTRA_DATAFILE_CONFIG));
Expand All @@ -135,8 +133,7 @@ public void dispatchingManyWithoutEnvironment() {
backgroundWatchersCache.setIsWatching(new DatafileConfig("3", null), true);
Logger logger = mock(Logger.class);
DatafileRescheduler.Dispatcher dispatcher = new DatafileRescheduler.Dispatcher(mockContext, backgroundWatchersCache, logger);
Intent intent = new Intent(mockContext, DatafileService.class);
dispatcher.dispatch(intent);
dispatcher.dispatch();
verify(mockContext, times(3)).startService(any(Intent.class));
cache.delete(BackgroundWatchersCache.BACKGROUND_WATCHERS_FILE_NAME);
}
Expand All @@ -151,8 +148,7 @@ public void dispatchingManyWithEnvironment() {
backgroundWatchersCache.setIsWatching(new DatafileConfig("3", "1"), true);
Logger logger = mock(Logger.class);
DatafileRescheduler.Dispatcher dispatcher = new DatafileRescheduler.Dispatcher(mockContext, backgroundWatchersCache, logger);
Intent intent = new Intent(mockContext, DatafileService.class);
dispatcher.dispatch(intent);
dispatcher.dispatch();
verify(mockContext, times(3)).startService(any(Intent.class));
cache.delete(BackgroundWatchersCache.BACKGROUND_WATCHERS_FILE_NAME);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
*/
// TODO These tests will pass individually but they fail when run as group
// Known bug https://code.google.com/p/android/issues/detail?id=180396
@Ignore
public class DatafileServiceTest {

private ExecutorService executor;
Expand All @@ -72,6 +73,7 @@ public void setup() {

@RequiresApi(api = Build.VERSION_CODES.HONEYCOMB)
@Test
@Ignore
public void testBinding() throws TimeoutException {
Context context = InstrumentationRegistry.getInstrumentation().getTargetContext();
Intent intent = new Intent(context, DatafileService.class);
Expand All @@ -91,8 +93,7 @@ public void testBinding() throws TimeoutException {


DatafileService datafileService = ((DatafileService.LocalBinder) binder).getService();
DatafileLoader datafileLoader = new DatafileLoader(datafileService, datafileClient, datafileCache,
Executors.newSingleThreadExecutor(), mock(Logger.class));
DatafileLoader datafileLoader = new DatafileLoader(targetContext, datafileClient, datafileCache, mock(Logger.class));
datafileService.getDatafile("1", datafileLoader, datafileLoadedListener);

assertTrue(datafileService.isBound());
Expand Down Expand Up @@ -120,6 +121,7 @@ public void testValidStart() throws TimeoutException {

@RequiresApi(api = Build.VERSION_CODES.HONEYCOMB)
@Test
@Ignore
public void testNullIntentStart() throws TimeoutException {
Context context = InstrumentationRegistry.getInstrumentation().getTargetContext();
Intent intent = new Intent(context, DatafileService.class);
Expand All @@ -139,6 +141,7 @@ public void testNullIntentStart() throws TimeoutException {

@RequiresApi(api = Build.VERSION_CODES.HONEYCOMB)
@Test
@Ignore
public void testNoProjectIdIntentStart() throws TimeoutException {
Context context = InstrumentationRegistry.getInstrumentation().getTargetContext();
Intent intent = new Intent(context, DatafileService.class);
Expand All @@ -158,6 +161,7 @@ public void testNoProjectIdIntentStart() throws TimeoutException {

@RequiresApi(api = Build.VERSION_CODES.HONEYCOMB)
@Test
@Ignore
public void testUnbind() throws TimeoutException {
Context context = InstrumentationRegistry.getInstrumentation().getTargetContext();
Intent intent = new Intent(context, DatafileService.class);
Expand Down
25 changes: 7 additions & 18 deletions datafile-handler/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,14 @@

<uses-permission android:name="android.permission.INTERNET"/>
<application>
<service
android:name=".DatafileService"
<receiver
android:name="DatafileRescheduler"
android:enabled="true"
android:exported="false">
</service>
<!--
Add these lines to your manifest if you want the services to schedule themselves again after a boot
or package replace. Without these lines in your application manifest, the DatafileRescheduler will not be used.
See {link test_app} AndroidManifest.xml as an example.

<receiver
android:name="DatafileRescheduler"
android:enabled="true"
android:exported="false">
<intent-filter>
<action android:name="android.intent.action.MY_PACKAGE_REPLACED" />
<action android:name="android.intent.action.BOOT_COMPLETED" />
</intent-filter>
</receiver>
-->
<intent-filter>
<action android:name="android.intent.action.MY_PACKAGE_REPLACED" />
<action android:name="android.intent.action.BOOT_COMPLETED" />
</intent-filter>
</receiver>
</application>
</manifest>
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
* Makes requests to the Optly CDN to get the datafile
*/
public class DatafileClient {
// easy way to set the connection timeout
public static int CONNECTION_TIMEOUT = 5 * 1000;
// the numerical base for the exponential backoff
public static final int REQUEST_BACKOFF_TIMEOUT = 2;
// power the number of retries
Expand Down Expand Up @@ -78,7 +80,7 @@ public String execute() {

client.setIfModifiedSince(urlConnection);

urlConnection.setConnectTimeout(5 * 1000);
urlConnection.setConnectTimeout(CONNECTION_TIMEOUT);
urlConnection.connect();

int status = urlConnection.getResponseCode();
Expand Down Expand Up @@ -108,6 +110,6 @@ public String execute() {
}
};

return client.execute(request, 2, 3);
return client.execute(request, REQUEST_BACKOFF_TIMEOUT, REQUEST_RETRIES_POWER);
}
}
Loading