Skip to content
This repository was archived by the owner on Mar 11, 2022. It is now read-only.

Commit 4bf3889

Browse files
brynhbrynh
committed
Fix 513, improve wake lock handling on Replication Policies
Also, give guidance on when to use Replication Policies and when to use JobScheduler.
1 parent f0bfc71 commit 4bf3889

File tree

7 files changed

+572
-36
lines changed

7 files changed

+572
-36
lines changed

‎cloudant-sync-datastore-android/src/main/java/com/cloudant/sync/replication/PeriodicReplicationService.java‎

Lines changed: 73 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public abstract class PeriodicReplicationService<T extends PeriodicReplicationRe
4747
* So that multiple PeriodicReplicationService instances can be used in one application without
4848
* interfering with each other, the preferences in this file are stored using keys prefixed with
4949
* the name of the concrete class implementing the PeriodicReplicationService. */
50-
private static final String PREFERENCES_FILE_NAME = "com.cloudant.preferences";
50+
static final String PREFERENCES_FILE_NAME = "com.cloudant.preferences";
5151

5252
/* We store the elapsed time since booting at which the last alarm occurred in SharedPreferences
5353
* using a key with this suffix. This is used to set the initial alarm time when periodic
@@ -69,6 +69,14 @@ public abstract class PeriodicReplicationService<T extends PeriodicReplicationRe
6969
* the service may be stopped and started by the operating system. */
7070
private static final String EXPLICITLY_STOPPED_SUFFIX = ".explicitlyStopped";
7171

72+
/* We store a flag indicating whether there are replications pending in
73+
* SharedPreferences using a key with this suffix. Replications may be pending because they
74+
* are currently in progress and have not yet completed, or becasue a previous scheduled
75+
* replication didn't take place because the conditions for replication were not met. We have
76+
* to store the flag persistently as the service may be stopped and started by the operating
77+
* system. */
78+
private static final String REPLICATIONS_PENDING_SUFFIX = ".replicationsPending";
79+
7280
private static final long MILLISECONDS_IN_SECOND = 1000L;
7381

7482
/**
@@ -156,19 +164,29 @@ public ServiceHandler(Looper looper) {
156164

157165
@Override
158166
public void handleMessage(Message msg) {
167+
Intent intent = msg.getData().getParcelable(EXTRA_INTENT);
168+
159169
switch (msg.arg2) {
160170
case COMMAND_START_PERIODIC_REPLICATION:
161171
startPeriodicReplication();
172+
releaseWakeLock(intent);
173+
stopSelf(msg.arg1);
162174
break;
163175
case COMMAND_STOP_PERIODIC_REPLICATION:
164176
stopPeriodicReplication();
165177
setExplicitlyStopped(true);
178+
releaseWakeLock(intent);
179+
stopSelf(msg.arg1);
166180
break;
167181
case COMMAND_DEVICE_REBOOTED:
168182
resetAlarmDueTimesOnReboot();
183+
releaseWakeLock(intent);
184+
stopSelf(msg.arg1);
169185
break;
170186
case COMMAND_RESET_REPLICATION_TIMERS:
171187
restartPeriodicReplications();
188+
releaseWakeLock(intent);
189+
stopSelf(msg.arg1);
172190
break;
173191
default:
174192
// Do nothing
@@ -228,6 +246,7 @@ public synchronized void onRebind(Intent intent) {
228246
protected void startReplications() {
229247
super.startReplications();
230248
setLastAlarmTime(0);
249+
setReplicationsPending(this, getClass(), true);
231250
}
232251

233252
/** Start periodic replications. */
@@ -344,6 +363,14 @@ private boolean isPeriodicReplicationEnabled() {
344363
false);
345364
}
346365

366+
public static boolean isPeriodicReplicationEnabled(Context context,
367+
Class <? extends
368+
PeriodicReplicationService> prsClass) {
369+
SharedPreferences prefs = context.getSharedPreferences(PREFERENCES_FILE_NAME, Context
370+
.MODE_PRIVATE);
371+
return prefs.getBoolean(constructKey(prsClass, PERIODIC_REPLICATION_ENABLED_SUFFIX), false);
372+
}
373+
347374
/**
348375
* Set a flag in SharedPreferences to indicate whether periodic replications were explicitly
349376
* stopped.
@@ -417,8 +444,51 @@ private int getIntervalInSeconds() {
417444
}
418445
}
419446

420-
private String constructKey(String suffix) {
421-
return getClass().getName() + suffix;
447+
String constructKey(String suffix) {
448+
return constructKey(getClass(), suffix);
449+
}
450+
451+
static String constructKey(Class<? extends PeriodicReplicationService> prsClass,
452+
String suffix) {
453+
return prsClass.getName() + suffix;
454+
}
455+
456+
@Override
457+
public void allReplicationsCompleted() {
458+
super.allReplicationsCompleted();
459+
setReplicationsPending(this, getClass(), false);
460+
}
461+
462+
/**
463+
* Sets whether there are replications pending. This may be because replications are
464+
* currently in progress and have not yet completed, or because a previous scheduled
465+
* replication didn't take place because the conditions for replication were not met.
466+
* @param context
467+
* @param prsClass
468+
* @param pending true if there is a replication pending, or false otherwise.
469+
*/
470+
public static void setReplicationsPending(Context context, Class<? extends
471+
PeriodicReplicationService> prsClass, boolean pending) {
472+
SharedPreferences prefs = context.getSharedPreferences(PREFERENCES_FILE_NAME, Context
473+
.MODE_PRIVATE);
474+
SharedPreferences.Editor editor = prefs.edit();
475+
editor.putBoolean(constructKey(prsClass, REPLICATIONS_PENDING_SUFFIX), pending);
476+
editor.apply();
477+
}
478+
479+
/**
480+
* Gets whether there are replications pending. Replications may be pending because they are
481+
* currently in progress and have not yet completed, or because a previous scheduled
482+
* replication didn't take place because the conditions for replication were not met.
483+
* @param context
484+
* @param prsClass
485+
* @return
486+
*/
487+
public static boolean replicationsPending(Context context, Class<? extends
488+
PeriodicReplicationService> prsClass) {
489+
SharedPreferences prefs = context.getSharedPreferences(PREFERENCES_FILE_NAME, Context
490+
.MODE_PRIVATE);
491+
return prefs.getBoolean(constructKey(prsClass, REPLICATIONS_PENDING_SUFFIX), true);
422492
}
423493

424494
/**

‎cloudant-sync-datastore-android/src/main/java/com/cloudant/sync/replication/ReplicationService.java‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ public abstract class ReplicationService extends Service
7676
private ReplicationPolicyManager mReplicationPolicyManager;
7777
private boolean mReplicatorsInitialised;
7878
private List<Message> mCommandQueue = new ArrayList<Message>();
79+
private Intent mStartReplicationsIntent;
80+
private int mStartReplicationsId;
7981

8082
/**
8183
* Stores the set of {@link PolicyReplicationsCompletedListener}s
@@ -124,25 +126,26 @@ public void handleMessage(Message msg) {
124126
"using an id in the range reserved for internal use. Custom commands " +
125127
"must use an id above " + INTERNALLY_RESERVED_COMMAND_MAX);
126128
}
129+
130+
Intent intent = msg.getData().getParcelable(EXTRA_INTENT);
131+
127132
// Process the commands passed in msg.arg2.
128133
switch (msg.arg2) {
129134
case COMMAND_START_REPLICATION:
135+
mStartReplicationsIntent = intent;
136+
mStartReplicationsId = msg.arg1;
130137
startReplications();
131138
break;
132139
case COMMAND_STOP_REPLICATION:
133140
stopReplications();
141+
releaseWakeLock(intent);
142+
stopSelf(msg.arg1);
134143
break;
135144
default:
136145
// Do nothing
137146
break;
138147
}
139148
} finally {
140-
// Get the Intent used to start the service and release the WakeLock if there is
141-
// one.
142-
// Calling completeWakefulIntent is safe even if there is no wakelock held.
143-
Intent intent = msg.getData().getParcelable(EXTRA_INTENT);
144-
WakefulBroadcastReceiver.completeWakefulIntent(intent);
145-
146149
notifyOperationStarted(msg.arg2);
147150
}
148151
}
@@ -159,7 +162,7 @@ public void onCreate() {
159162
WifiManager wifiManager = (WifiManager) getSystemService(Context.WIFI_SERVICE);
160163
if (wifiManager != null) {
161164
mWifiLock = wifiManager.createWifiLock(WifiManager.WIFI_MODE_FULL_HIGH_PERF,
162-
"ReplicationService");
165+
"ReplicationService");
163166
}
164167

165168
// Create a background priority thread to so we don't block the process's main thread.
@@ -289,6 +292,14 @@ private void releaseWifiLockIfHeld() {
289292
}
290293
}
291294

295+
protected void releaseWakeLock(Intent intent) {
296+
if (intent != null) {
297+
// Release the WakeLock if there is one. Calling completeWakefulIntent is safe even if
298+
// there is no wakelock held.
299+
WakefulBroadcastReceiver.completeWakefulIntent(intent);
300+
}
301+
}
302+
292303
/**
293304
* Stop replications currently in progress and terminate this Service.
294305
*/
@@ -297,7 +308,6 @@ protected void stopReplications() {
297308
mReplicationPolicyManager.stopReplications();
298309
releaseWifiLockIfHeld();
299310
}
300-
stopSelf();
301311
}
302312

303313
@Override
@@ -308,7 +318,8 @@ public void allReplicationsCompleted() {
308318
}
309319
}
310320
releaseWifiLockIfHeld();
311-
stopSelf();
321+
releaseWakeLock(mStartReplicationsIntent);
322+
stopSelf(mStartReplicationsId);
312323
}
313324

314325
@Override

‎cloudant-sync-datastore-android/src/main/java/com/cloudant/sync/replication/WifiPeriodicReplicationReceiver.java‎

Lines changed: 64 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import android.content.Context;
1818
import android.content.Intent;
19+
import android.content.SharedPreferences;
1920
import android.net.ConnectivityManager;
2021
import android.net.NetworkInfo;
2122

@@ -69,29 +70,67 @@
6970
public abstract class WifiPeriodicReplicationReceiver<T extends PeriodicReplicationService>
7071
extends PeriodicReplicationReceiver {
7172

73+
private static final String WAS_ON_WIFI_SUFFIX = ".wasOnWifi";
74+
7275
protected WifiPeriodicReplicationReceiver(Class<T> clazz) {
7376
super(clazz);
7477
}
7578

7679
@Override
7780
public void onReceive(Context context, Intent intent) {
81+
int command = ReplicationService.COMMAND_NONE;
82+
7883
if (ConnectivityManager.CONNECTIVITY_ACTION.equals(intent.getAction())) {
7984

80-
int command;
81-
if (isConnectedToWifi(context)) {
85+
boolean isConnectedToWifi = isConnectedToWifi(context);
86+
87+
if (isConnectedToWifi == wasOnWifi(context)) {
88+
// This receiver will get a CONNECTIVITY_ACTION when we disconnect from networks
89+
// as well as when we connect to networks. We only want to do anything if we were
90+
// we were on WiFi and now are not, or were not on WiFi and now are.
91+
return;
92+
} else if (isConnectedToWifi) {
8293
// State has changed to connected.
83-
command = PeriodicReplicationService.COMMAND_START_PERIODIC_REPLICATION;
84-
} else {
94+
setWasOnWifi(context, true);
95+
if (PeriodicReplicationService.replicationsPending(context, clazz)) {
96+
// There was a replication in progress when we lost WiFi, so restart
97+
// replication immediately.
98+
command = ReplicationService.COMMAND_START_REPLICATION;
99+
}
100+
} else if (!isConnectedToWifi(context)) {
85101
// State has changed to disconnected.
86-
command = PeriodicReplicationService.COMMAND_STOP_PERIODIC_REPLICATION;
102+
setWasOnWifi(context, false);
103+
104+
command = ReplicationService.COMMAND_STOP_REPLICATION;
105+
}
106+
107+
} else if (Intent.ACTION_BOOT_COMPLETED.equals(intent.getAction())) {
108+
super.onReceive(context, intent);
109+
110+
setWasOnWifi(context, false);
111+
112+
/* As well as the normal processing after a reboot, we want to restart periodic
113+
replications if they were active prior to reboot. We always want the restart of
114+
periodic replications as when a replication is due, we only pass it on from here if
115+
we're connected to WiFi. */
116+
if (PeriodicReplicationService.isPeriodicReplicationEnabled(context, clazz)) {
117+
command = PeriodicReplicationService.COMMAND_START_PERIODIC_REPLICATION;
87118
}
119+
} else if (!ALARM_ACTION.equals(intent.getAction()) || isConnectedToWifi
120+
(context)) {
121+
// Pass on the processing to the superclass if this is not an alarm, or if it's an
122+
// alarm and we're connected to WiFi.
123+
super.onReceive(context, intent);
124+
} else {
125+
PeriodicReplicationService.setReplicationsPending(context, clazz, true);
126+
}
127+
128+
if (command != ReplicationService.COMMAND_NONE) {
88129
Intent serviceIntent = new Intent(context.getApplicationContext(), clazz);
89130
serviceIntent.putExtra(PeriodicReplicationService.EXTRA_COMMAND, command);
90131
startWakefulService(context, serviceIntent);
91-
} else {
92-
// Pass on the processing to the superclass to handle alarms and reboot.
93-
super.onReceive(context, intent);
94132
}
133+
95134
}
96135

97136
public static boolean isConnectedToWifi(Context context) {
@@ -104,4 +143,21 @@ public static boolean isConnectedToWifi(Context context) {
104143
&& activeNetwork.isConnectedOrConnecting();
105144
}
106145

146+
private void setWasOnWifi(Context context, boolean onWifi) {
147+
SharedPreferences prefs = context.getSharedPreferences(PeriodicReplicationService
148+
.PREFERENCES_FILE_NAME, Context
149+
.MODE_PRIVATE);
150+
SharedPreferences.Editor editor = prefs.edit();
151+
editor.putBoolean(PeriodicReplicationService.constructKey(clazz,
152+
WAS_ON_WIFI_SUFFIX), onWifi);
153+
editor.apply();
154+
}
155+
156+
private boolean wasOnWifi(Context context) {
157+
SharedPreferences prefs = context.getSharedPreferences(PeriodicReplicationService
158+
.PREFERENCES_FILE_NAME, Context
159+
.MODE_PRIVATE);
160+
return prefs.getBoolean(PeriodicReplicationService.constructKey(clazz,
161+
WAS_ON_WIFI_SUFFIX), false);
162+
}
107163
}

‎cloudant-sync-datastore-android/src/test/java/com/cloudant/sync/replication/ReplicationServiceTest.java‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -565,8 +565,8 @@ public void operationStarted(int operationId) {
565565
/**
566566
* Check that when an intent is sent to the {@link PeriodicReplicationService} indicating that
567567
* a replication should be started, a {@link android.net.wifi.WifiManager.WifiLock} is acquired,
568-
* the {@link ReplicationPolicyManager} is started and the last alarm times in SharedPreferences
569-
* are updated to the current time.
568+
* the {@link ReplicationPolicyManager} is started, the last alarm times in SharedPreferences
569+
* are updated to the current time and the replications pending flag is set to true.
570570
*/
571571
@Test
572572
public void testOnStartCommandStartReplication() {
@@ -609,6 +609,17 @@ public void operationStarted(int operationId) {
609609
checkElapsedTime(expectedElapsedTime, prefsValues.get(0));
610610
checkClockPreferenceName(prefsKeys.get(1));
611611
checkClockTime(expectedRealTime, prefsValues.get(1));
612+
613+
ArgumentCaptor<String> captorPrefKeys2 = ArgumentCaptor.forClass(String.class);
614+
ArgumentCaptor<Boolean> captorPrefValues2 = ArgumentCaptor.forClass(Boolean.class);
615+
List<String> prefKeys = captorPrefKeys2.getAllValues();
616+
List<Boolean> prefValues = captorPrefValues2.getAllValues();
617+
verify(mMockPreferencesEditor, times(1)).putBoolean
618+
(captorPrefKeys2.capture(),
619+
captorPrefValues2.capture());
620+
621+
assertEquals(PREFERENCE_CLASS_NAME + ".replicationsPending", prefKeys.get(0));
622+
assertTrue("Replications pending flag should be true", prefValues.get(0));
612623
} catch (InterruptedException e) {
613624
e.printStackTrace();
614625
}
@@ -617,7 +628,8 @@ public void operationStarted(int operationId) {
617628
/**
618629
* Check that when an intent is sent to the {@link PeriodicReplicationService} indicating that
619630
* a replication should be stopped, the {@link android.net.wifi.WifiManager.WifiLock} is
620-
* released and the {@link ReplicationPolicyManager} is stopped.
631+
* released, the {@link ReplicationPolicyManager} is stopped and the replications pending flag
632+
* remains true.
621633
*/
622634
@Test
623635
public void testOnStartCommandStopReplication() {
@@ -652,10 +664,22 @@ public void operationStarted(int operationId) {
652664
stopIntent.putExtra(PeriodicReplicationService.EXTRA_COMMAND, PeriodicReplicationService.COMMAND_STOP_REPLICATION);
653665
service.onStartCommand(stopIntent, 0, 0);
654666
service.setReplicators(mMockReplicators);
667+
655668
try {
656669
assertTrue("The countdown should reach zero", latch.await(DEFAULT_WAIT_SECONDS, TimeUnit.SECONDS));
657670
verify(mMockWifiLock, times(1)).release();
658671
verify(mMockReplicationPolicyManager, times(1)).stopReplications();
672+
673+
ArgumentCaptor<String> captorPrefKeys = ArgumentCaptor.forClass(String.class);
674+
ArgumentCaptor<Boolean> captorPrefValues = ArgumentCaptor.forClass(Boolean.class);
675+
List<String> prefKeys = captorPrefKeys.getAllValues();
676+
List<Boolean> prefValues = captorPrefValues.getAllValues();
677+
verify(mMockPreferencesEditor, times(1)).putBoolean
678+
(captorPrefKeys.capture(),
679+
captorPrefValues.capture());
680+
681+
assertEquals(PREFERENCE_CLASS_NAME + ".replicationsPending", prefKeys.get(0));
682+
assertTrue("Replications pending flag should be true", prefValues.get(0));
659683
} catch (InterruptedException e) {
660684
e.printStackTrace();
661685
}

0 commit comments

Comments
 (0)