From b9919adccd8dd1cddec48f7d514072810c765483 Mon Sep 17 00:00:00 2001 From: Matthias Leonhardt Date: Fri, 25 Mar 2016 21:13:51 +0000 Subject: [PATCH] Added a IntentService to receive Broadcast-Intents to remotely control / configure the app. MainActivity: Moved binding-functions to onPause() and onResume() so that the SyncThingService is only bound to the activity if the activity is active. New class AppConfigReceiver: Support start and stop of the SyncThingService - restarting a running service again should not be an issue - stop service only if "always run in background"-mode is disabled. Otherwise show a notification indicating this. Instrumentation-tests: - Added tests for AppConfigReceiver - Extended MockContext to also consume stopService commands. - testGetReadableTransferRate: Apparently the return-values have changed a bit. Adjusted the asserts to the current return-values. SycthingService: Added code for thread-safety in case the service still starting when it should be stopped. Then PollWebGuiAvailableTaskImpl is active and waits for the Synthing-API to become active. So that and onDestroy have to be synchronized. Added a stopSelf() in PollWebGuiAvailableTaskImpl.onPostExecute() in case mStopScheduled was active. Commented my change in the javadoc at onDestroy. Put a reference to that comment to .onPostExecution() --- .../syncthingandroid/test/MockContext.java | 11 ++ .../test/syncthing/AppConfigReceiverTest.java | 108 ++++++++++++++++++ src/main/AndroidManifest.xml | 6 + .../activities/SyncthingActivity.java | 14 ++- .../syncthing/AppConfigReceiver.java | 75 ++++++++++++ .../syncthing/SyncthingService.java | 52 ++++++--- src/main/res/values/strings.xml | 1 + 7 files changed, 250 insertions(+), 17 deletions(-) create mode 100644 src/androidTest/java/com/nutomic/syncthingandroid/test/syncthing/AppConfigReceiverTest.java create mode 100644 src/main/java/com/nutomic/syncthingandroid/syncthing/AppConfigReceiver.java diff --git a/src/androidTest/java/com/nutomic/syncthingandroid/test/MockContext.java b/src/androidTest/java/com/nutomic/syncthingandroid/test/MockContext.java index 97c2acd5..f293b69a 100644 --- a/src/androidTest/java/com/nutomic/syncthingandroid/test/MockContext.java +++ b/src/androidTest/java/com/nutomic/syncthingandroid/test/MockContext.java @@ -16,6 +16,7 @@ import java.util.List; public class MockContext extends ContextWrapper { private ArrayList mReceivedIntents = new ArrayList<>(); + private ArrayList mStopServiceIntents = new ArrayList<>(); /** * Use the actual context for calls that aren't easily mocked. May be null if those @@ -36,10 +37,20 @@ public class MockContext extends ContextWrapper { return null; } + @Override + public boolean stopService(Intent intent) { + mStopServiceIntents.add(intent); + return true; + } + public List getReceivedIntents() { return mReceivedIntents; } + public List getStopServiceIntents() { + return mStopServiceIntents; + } + public void clearReceivedIntents() { mReceivedIntents.clear(); } diff --git a/src/androidTest/java/com/nutomic/syncthingandroid/test/syncthing/AppConfigReceiverTest.java b/src/androidTest/java/com/nutomic/syncthingandroid/test/syncthing/AppConfigReceiverTest.java new file mode 100644 index 00000000..d76da3a4 --- /dev/null +++ b/src/androidTest/java/com/nutomic/syncthingandroid/test/syncthing/AppConfigReceiverTest.java @@ -0,0 +1,108 @@ +package com.nutomic.syncthingandroid.test.syncthing; + +import android.content.Intent; +import android.preference.PreferenceManager; +import android.test.AndroidTestCase; + +import com.nutomic.syncthingandroid.syncthing.AppConfigReceiver; +import com.nutomic.syncthingandroid.syncthing.SyncthingService; +import com.nutomic.syncthingandroid.test.MockContext; + +/** + * Test the correct behaviour of the AppConfigReceiver + * + * Created by sqrt-1674 on 27.03.16. + */ +public class AppConfigReceiverTest extends AndroidTestCase { + private AppConfigReceiver mReceiver; + private MockContext mContext; + + @Override + protected void setUp() throws Exception { + super.setUp(); + mReceiver = new AppConfigReceiver(); + mContext = new MockContext(getContext()); + } + + @Override + protected void tearDown() throws Exception { + PreferenceManager.getDefaultSharedPreferences(mContext).edit().clear().commit(); + super.tearDown(); + } + + + /** + * Test starting the Syncthing-Service if "always run in background" is enabled + * In this case starting the service is allowed + */ + public void testStartSyncthingServiceBackground() { + PreferenceManager.getDefaultSharedPreferences(mContext) + .edit() + .putBoolean(SyncthingService.PREF_ALWAYS_RUN_IN_BACKGROUND, true) + .commit(); + + Intent intent = new Intent(new Intent(mContext, AppConfigReceiver.class)); + intent.setAction(AppConfigReceiver.ACTION_START); + + mReceiver.onReceive(mContext, intent); + assertEquals("Start SyncthingService Background", 1, mContext.getReceivedIntents().size()); + assertEquals("Start SyncthingService Background", SyncthingService.class.getName(), + mContext.getReceivedIntents().get(0).getComponent().getClassName()); + } + + /** + * Test stopping the service if "alway run in background" is enabled. + * Stopping the service in this mode is not allowed, so no stopService-intent may be issued. + */ + public void testStopSyncthingServiceBackground() { + PreferenceManager.getDefaultSharedPreferences(mContext) + .edit() + .putBoolean(SyncthingService.PREF_ALWAYS_RUN_IN_BACKGROUND, true) + .commit(); + + Intent intent = new Intent(new Intent(mContext, AppConfigReceiver.class)); + intent.setAction(AppConfigReceiver.ACTION_STOP); + + mReceiver.onReceive(mContext, intent); + assertEquals("Stop SyncthingService Background", 0, mContext.getStopServiceIntents().size()); + } + + /** + * Test starting the Syncthing-Service if "always run in background" is disabled + * In this case starting the service is allowed + */ + public void testStartSyncthingServiceForeground() { + PreferenceManager.getDefaultSharedPreferences(mContext) + .edit() + .putBoolean(SyncthingService.PREF_ALWAYS_RUN_IN_BACKGROUND, false) + .commit(); + + Intent intent = new Intent(new Intent(mContext, AppConfigReceiver.class)); + intent.setAction(AppConfigReceiver.ACTION_START); + + mReceiver.onReceive(mContext, intent); + assertEquals("Start SyncthingService Foreround", 1, mContext.getReceivedIntents().size()); + assertEquals("Start SyncthingService Foreround", SyncthingService.class.getName(), + mContext.getReceivedIntents().get(0).getComponent().getClassName()); + } + + /** + * Test stopping the Syncthing-Service if "always run in background" is disabled + * In this case stopping the service is allowed + */ + public void testStopSyncthingServiceForeground() { + PreferenceManager.getDefaultSharedPreferences(mContext) + .edit() + .putBoolean(SyncthingService.PREF_ALWAYS_RUN_IN_BACKGROUND, false) + .commit(); + + Intent intent = new Intent(new Intent(mContext, AppConfigReceiver.class)); + intent.setAction(AppConfigReceiver.ACTION_STOP); + + mReceiver.onReceive(mContext, intent); + assertEquals("Stop SyncthingService Foreround", 1, mContext.getStopServiceIntents().size()); + Intent receivedIntent = mContext.getStopServiceIntents().get(0); + assertEquals("Stop SyncthingService Foreround", SyncthingService.class.getName(), + receivedIntent.getComponent().getClassName()); + } +} diff --git a/src/main/AndroidManifest.xml b/src/main/AndroidManifest.xml index a710a159..0d48b0f9 100644 --- a/src/main/AndroidManifest.xml +++ b/src/main/AndroidManifest.xml @@ -91,6 +91,12 @@ + + + + + + diff --git a/src/main/java/com/nutomic/syncthingandroid/activities/SyncthingActivity.java b/src/main/java/com/nutomic/syncthingandroid/activities/SyncthingActivity.java index 2aee1937..99abdbfc 100644 --- a/src/main/java/com/nutomic/syncthingandroid/activities/SyncthingActivity.java +++ b/src/main/java/com/nutomic/syncthingandroid/activities/SyncthingActivity.java @@ -33,14 +33,20 @@ public abstract class SyncthingActivity extends ToolbarBindingActivity implement protected void onCreate(Bundle savedInstanceState) { super.onCreate(savedInstanceState); startService(new Intent(this, SyncthingService.class)); - bindService(new Intent(this, SyncthingService.class), - this, Context.BIND_AUTO_CREATE); } @Override - protected void onDestroy() { - super.onDestroy(); + protected void onPause() { unbindService(this); + + super.onPause(); + } + + @Override + protected void onResume() { + super.onResume(); + + bindService(new Intent(this, SyncthingService.class), this, Context.BIND_AUTO_CREATE); } @Override diff --git a/src/main/java/com/nutomic/syncthingandroid/syncthing/AppConfigReceiver.java b/src/main/java/com/nutomic/syncthingandroid/syncthing/AppConfigReceiver.java new file mode 100644 index 00000000..791a40be --- /dev/null +++ b/src/main/java/com/nutomic/syncthingandroid/syncthing/AppConfigReceiver.java @@ -0,0 +1,75 @@ +package com.nutomic.syncthingandroid.syncthing; + +import android.app.Notification; +import android.app.NotificationManager; +import android.app.PendingIntent; +import android.content.BroadcastReceiver; +import android.content.Context; +import android.content.Intent; +import android.os.Build; +import android.support.v4.app.NotificationCompat; + +import com.nutomic.syncthingandroid.R; +import com.nutomic.syncthingandroid.activities.MainActivity; + +/** + * Broadcast-receiver to control and configure SyncThing remotely + * + * Created by sqrt-1764 on 25.03.16. + */ +public class AppConfigReceiver extends BroadcastReceiver { + private static final int ID_NOTIFICATION_BACKGROUND_ACTIVE = 3; + + /** + * Start the Syncthing-Service + */ + public static final String ACTION_START = "com.nutomic.syncthingandroid.action.START"; + + /** + * Stop the Syncthing-Service + * If alwaysRunInBackground is enabled the service must not be stopped. Instead a + * notification is presented to the user. + */ + public static final String ACTION_STOP = "com.nutomic.syncthingandroid.action.STOP"; + + @Override + public void onReceive(Context context, Intent intent) { + switch (intent.getAction()) { + case ACTION_START: + context.startService(new Intent(context, SyncthingService.class)); + break; + + case ACTION_STOP: + if (SyncthingService.alwaysRunInBackground(context)) { + final String msg = context.getString(R.string.appconfig_receiver_background_enabled); + + Context appContext = context.getApplicationContext(); + + NotificationCompat.Builder nb = new NotificationCompat.Builder(context) + .setContentText(msg) + .setTicker(msg) + .setStyle(new NotificationCompat.BigTextStyle().bigText(msg)) + .setContentTitle(context.getText(context.getApplicationInfo().labelRes)) + .setSmallIcon(R.drawable.ic_stat_notify) + .setAutoCancel(true) + .setContentIntent(PendingIntent.getActivity(appContext, + 0, + new Intent(appContext, MainActivity.class), + PendingIntent.FLAG_UPDATE_CURRENT)); + + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { + nb.setCategory(Notification.CATEGORY_ERROR); // Only supported in API 21 or better + } + + NotificationManager nm = + (NotificationManager)context.getSystemService(Context.NOTIFICATION_SERVICE); + + nm.notify(ID_NOTIFICATION_BACKGROUND_ACTIVE, nb.build()); + + } else { + context.stopService(new Intent(context, SyncthingService.class)); + } + break; + } + } +} diff --git a/src/main/java/com/nutomic/syncthingandroid/syncthing/SyncthingService.java b/src/main/java/com/nutomic/syncthingandroid/syncthing/SyncthingService.java index 205c4d4d..efaa120f 100644 --- a/src/main/java/com/nutomic/syncthingandroid/syncthing/SyncthingService.java +++ b/src/main/java/com/nutomic/syncthingandroid/syncthing/SyncthingService.java @@ -59,7 +59,6 @@ public class SyncthingService extends Service implements public static final String ACTION_RESET = "com.nutomic.syncthingandroid.service.SyncthingService.RESET"; - /** * Interval in ms at which the GUI is updated (eg {@link com.nutomic.syncthingandroid.fragments.DrawerFragment}). */ @@ -144,6 +143,12 @@ public class SyncthingService extends Service implements private State mCurrentState = State.INIT; + /** + * Object that can be locked upon when accessing mCurrentState + * Currently used to male onDestroy() and PollWebGuiAvailableTaskImpl.onPostExcecute() tread-safe + */ + private final Object stateLock = new Object(); + /** * True if a stop was requested while syncthing is starting, in that case, perform stop in * {@link PollWebGuiAvailableTaskImpl}. @@ -227,10 +232,8 @@ public class SyncthingService extends Service implements if (mConfig != null) { mCurrentState = State.STARTING; - if (mApi != null) { + if (mApi != null) registerOnWebGuiAvailableListener(mApi); - mApi.setWebGuiUrl(mConfig.getWebGuiUrl()); - } if (mEventProcessor != null) registerOnWebGuiAvailableListener(mEventProcessor); new PollWebGuiAvailableTaskImpl(getFilesDir() + "/" + HTTPS_CERT_FILE) @@ -426,12 +429,25 @@ public class SyncthingService extends Service implements /** * Stops the native binary. + * + * The native binary crashes if stopped before it is fully active. In that case signal the + * stop request to PollWebGuiAvailableTaskImpl that is active in that situation and terminate + * the service there. */ @Override public void onDestroy() { - super.onDestroy(); - Log.i(TAG, "Shutting down service"); - shutdown(); + + synchronized (stateLock) { + if (mCurrentState == State.INIT || mCurrentState == State.STARTING) { + Log.i(TAG, "Delay shutting down service until initialisation of Syncthing finished"); + mStopScheduled = true; + + } else { + Log.i(TAG, "Shutting down service immediately"); + shutdown(); + } + } + SharedPreferences sp = PreferenceManager.getDefaultSharedPreferences(this); sp.unregisterOnSharedPreferenceChangeListener(this); } @@ -512,14 +528,24 @@ public class SyncthingService extends Service implements super(httpsCertPath); } + /** + * Wait for the web-gui of the native syncthing binary to come online. + * + * In case the binary is to be stopped, also be aware that another thread could request + * to stop the binary in the time while waiting for the GUI to become active. See the comment + * for SyncthingService.onDestroy for details. + */ @Override protected void onPostExecute(Void aVoid) { - if (mStopScheduled) { - mCurrentState = State.DISABLED; - onApiChange(); - shutdown(); - mStopScheduled = false; - return; + synchronized (stateLock) { + if (mStopScheduled) { + mCurrentState = State.DISABLED; + onApiChange(); + shutdown(); + mStopScheduled = false; + stopSelf(); + return; + } } Log.i(TAG, "Web GUI has come online at " + mConfig.getWebGuiUrl()); mCurrentState = State.STARTING; diff --git a/src/main/res/values/strings.xml b/src/main/res/values/strings.xml index d03aefba..f845f878 100644 --- a/src/main/res/values/strings.xml +++ b/src/main/res/values/strings.xml @@ -468,4 +468,5 @@ Please report any problems you encounter via Github. %1$s / %2$s + Stopping Syncthing is not supported when running in background is enabled.