Merge pull request #177 from learningequality/revert-175

Notification handling and work manager tweaks

.gitignore

@@ -38,3 +38,5 @@ android_root/
 .env
 .version-code
 *.keystore
+.envrc
+.idea/

python-for-android/dists/kolibri/src/main/AndroidManifest.xml

@@ -24,7 +24,7 @@
     <application android:label="@string/app_name"
                  android:icon="@mipmap/icon"
                  android:allowBackup="true"
-
+                 android:name=".App"
                  android:theme="@android:style/Theme.NoTitleBar"
                  android:hardwareAccelerated="true"
                  android:usesCleartextTraffic="true"
@@ -63,7 +63,7 @@
 
 
         <service android:name="org.learningequality.Kolibri.TaskworkerWorkerService"
-                 android:process=":worker_TaskWorker"
+                 android:process="@string/task_worker_process"
                  android:exported="false" />
 
 

python-for-android/dists/kolibri/src/main/java/org/kivy/android/PythonActivity.java

@@ -105,7 +105,6 @@ protected void onCreate(Bundle savedInstanceState) {
 
         this.mActivity = this;
         this.showLoadingScreen();
-        this.createNotificationChannel();
         new UnpackFilesTask().execute(getAppRoot());
     }
 
@@ -566,22 +565,6 @@ public void requestPermissionsWithRequestCode(String[] permissions, int requestC
     public void requestPermissions(String[] permissions) {
         requestPermissionsWithRequestCode(permissions, 1);
     }
-
-    private void createNotificationChannel() {
-        // Create the NotificationChannel, but only on API 26+ because
-        // the NotificationChannel class is not in the Support Library.
-        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
-            Context context = getApplicationContext();
-            CharSequence name = context.getString(R.string.notification_channel_title);
-            String channelId = context.getString(R.string.notification_channel_id);
-            int importance = NotificationManager.IMPORTANCE_DEFAULT;
-            NotificationChannel channel = new NotificationChannel(channelId, name, importance);
-            // Register the channel with the system. You can't change the importance
-            // or other notification behaviors after this.
-            NotificationManagerCompat notificationManager = NotificationManagerCompat.from(context);
-            notificationManager.createNotificationChannel(channel);
-        }
-    }
 }
 
 

python-for-android/dists/kolibri/src/main/java/org/kivy/android/PythonWorker.java

@@ -1,29 +1,30 @@
 package org.kivy.android;
 
+import android.annotation.SuppressLint;
 import android.content.Context;
 import android.util.Log;
 
 import androidx.annotation.NonNull;
 import androidx.concurrent.futures.CallbackToFutureAdapter;
 import androidx.work.ForegroundInfo;
 import androidx.work.WorkerParameters;
+import androidx.work.impl.utils.futures.SettableFuture;
 import androidx.work.multiprocess.RemoteListenableWorker;
 
 import com.google.common.util.concurrent.ListenableFuture;
 
-import org.learningequality.Kolibri.R;
-import org.learningequality.Notifications;
+import java.util.concurrent.RunnableFuture;
+import java.util.concurrent.ThreadPoolExecutor;
 
-import java.util.concurrent.Executors;
-import java.util.concurrent.ThreadLocalRandom;
-
-public class PythonWorker extends RemoteListenableWorker {
+abstract public class PythonWorker extends RemoteListenableWorker {
     private static final String TAG = "PythonWorker";
 
     // WorkRequest data key for python worker argument
     public static final String ARGUMENT_WORKER_ARGUMENT = "PYTHON_WORKER_ARGUMENT";
 
-    public static final String ARGUMENT_LONG_RUNNING = "LONG_RUNNING_ARGUMENT";
+    public static final String TAG_LONG_RUNNING = "worker_long_running";
+
+    public static final int MAX_WORKER_RETRIES = 3;
 
     // Python environment variables
     private String androidPrivate;
@@ -35,21 +36,13 @@ public class PythonWorker extends RemoteListenableWorker {
 
     public static PythonWorker mWorker = null;
 
-    public static ThreadLocal<Integer> threadNotificationId = new ThreadLocal<>();
-
-    private String notificationTitle;
-
     public PythonWorker(
         @NonNull Context context,
         @NonNull WorkerParameters params) {
         super(context, params);
 
         String appRoot = PythonUtil.getAppRoot(context);
 
-        notificationTitle = context.getString(R.string.app_name);
-
-        threadNotificationId.set(ThreadLocalRandom.current().nextInt(1, 65537));
-
         PythonWorker.mWorker = this;
 
         androidPrivate = appRoot;
@@ -66,67 +59,93 @@ public void setWorkerEntrypoint(String value) {
         workerEntrypoint = value;
     }
 
-    @Override
-    public ListenableFuture<Result> startRemoteWork() {
-        return CallbackToFutureAdapter.getFuture(completer -> {
-            String dataArg = getInputData().getString(ARGUMENT_WORKER_ARGUMENT);
-            final String serviceArg;
-            if (dataArg != null) {
-                Log.d(TAG, "Setting python worker argument to " + dataArg);
-                serviceArg = dataArg;
-            } else {
-                serviceArg = "";
-            }
-
-            boolean longRunning = getInputData().getBoolean(ARGUMENT_LONG_RUNNING, false);
+    public boolean isLongRunning() {
+        return getTags().contains(TAG_LONG_RUNNING);
+    }
 
-            if (longRunning) {
-                runAsForeground();
-            }
+    protected String getArgument() {
+        String dataArg = getInputData().getString(ARGUMENT_WORKER_ARGUMENT);
+        final String serviceArg;
+        if (dataArg != null) {
+            serviceArg = dataArg;
+        } else {
+            serviceArg = "";
+        }
+        return serviceArg;
+    }
 
-            int notificationId = getNotificationId();
+    protected Result doWork() {
+        String id = getId().toString();
+        String arg = getArgument();
 
-            // The python thread handling the work needs to be run in a
-            // separate thread so that future can be returned. Without
-            // it, any cancellation can't be processed.
-            final Thread pythonThread = new Thread(new Runnable() {
-                @Override
-                public void run() {
-                    Log.d(TAG, "Running with python worker argument: " + serviceArg);
+        Log.d(TAG, id + " Running with python worker argument: " + arg);
 
-                    threadNotificationId.set(notificationId);
+        int res = nativeStart(
+                androidPrivate, androidArgument,
+                workerEntrypoint, pythonName,
+                pythonHome, pythonPath,
+                arg
+        );
+        Log.d(TAG, id + " Finished remote python work: " + res);
 
-                    int res = nativeStart(
-                        androidPrivate, androidArgument,
-                        workerEntrypoint, pythonName,
-                        pythonHome, pythonPath,
-                        serviceArg
-                    );
+        if (res == 0) {
+            return Result.success();
+        }
 
-                    Log.d(TAG, "Finished remote python work: " + res);
+        return Result.failure();
+    }
 
-                    if (res == 0) {
-                        completer.set(Result.success());
+    @SuppressLint("RestrictedApi")
+    @NonNull
+    @Override
+    public ListenableFuture<Result> startRemoteWork() {
+        SettableFuture<Result> future = SettableFuture.create();
+        String id = getId().toString();
+
+        if (isLongRunning()) {
+            Log.d(TAG, id + " Enabling foreground service for long running task");
+            setForegroundAsync(getForegroundInfo());
+        }
+
+        // See executor defined in configuration
+        ThreadPoolExecutor executor = (ThreadPoolExecutor) getBackgroundExecutor();
+        // This is somewhat similar to what the plain `Worker` class does, except that we
+        // use `submit` instead of `execute` so we can propagate cancellation
+        // See https://android.googlesource.com/platform/frameworks/support/+/60ae0eec2a32396c22ad92502cde952c80d514a0/work/workmanager/src/main/java/androidx/work/Worker.java
+        RunnableFuture<?> threadFuture = (RunnableFuture<?>)executor.submit(new Runnable() {
+            @Override
+            public void run() {
+                try {
+                    Result r = doWork();
+                    future.set(r);
+                }  catch (Exception e) {
+                    if (getRunAttemptCount() > MAX_WORKER_RETRIES) {
+                        Log.e(TAG, id + " Exception in remote python work", e);
+                        future.setException(e);
                     } else {
-                        completer.set(Result.failure());
+                        Log.w(TAG, id + " Exception in remote python work, scheduling retry", e);
+                        future.set(Result.retry());
                     }
+                } finally {
+                    cleanup();
                 }
-            });
-            pythonThread.setName("python_worker_thread");
-
-            completer.addCancellationListener(new Runnable() {
-                @Override
-                public void run() {
-                    Log.i(TAG, "Interrupting remote work");
-                    pythonThread.interrupt();
-                }
-            }, Executors.newSingleThreadExecutor());
-
-            Log.i(TAG, "Starting remote python work");
-            pythonThread.start();
-
-            return TAG + " work thread";
+            }
         });
+
+        // If `RunnableFuture` was a `ListenableFuture` we could simply use `future.setFuture` to
+        // propagate the result and cancellation, but instead add listener to propagate
+        // cancellation to python thread, using the task executor which should invoke this in the
+        // main thread (where this was originally called from)
+        future.addListener(new Runnable() {
+            @Override
+            public void run() {
+                if (future.isCancelled()) {
+                    Log.i(TAG, "Interrupting python thread");
+                    threadFuture.cancel(true);
+                }
+            }
+        }, getTaskExecutor().getMainThreadExecutor());
+        return future;
     }
 
     // Native part
@@ -137,22 +156,19 @@ public static native int nativeStart(
         String pythonServiceArgument
     );
 
-    public ForegroundInfo getForegroundInfo() {
-        return new ForegroundInfo(getNotificationId(), Notifications.createNotification(notificationTitle, null, -1, -1));
+    public void onStopped() {
+        cleanup();
+        super.onStopped();
+        mWorker = null;
     }
+    protected void cleanup() {}
 
-    public void runAsForeground() {
-        setForegroundAsync(getForegroundInfo());
-    }
+    abstract public ForegroundInfo getForegroundInfo();
 
     @Override
     public ListenableFuture<ForegroundInfo> getForegroundInfoAsync() {
         return CallbackToFutureAdapter.getFuture((CallbackToFutureAdapter.Resolver<ForegroundInfo>) completer -> completer.set(getForegroundInfo()));
     }
 
     public static native int tearDownPython();
-
-    public static int getNotificationId() {
-        return threadNotificationId.get();
-    }
 }

python-for-android/dists/kolibri/src/main/java/org/learningequality/ContextUtil.java

@@ -5,18 +5,31 @@
 import org.kivy.android.PythonActivity;
 import org.kivy.android.PythonService;
 import org.kivy.android.PythonWorker;
+import org.learningequality.NotificationRef;
 
 public class ContextUtil {
     public static Context getApplicationContext() {
-        if (PythonActivity.mActivity != null) {
+        if (isActivityContext()) {
             return PythonActivity.mActivity.getApplicationContext();
         }
-        if (PythonService.mService != null) {
+        if (isServiceContext()) {
             return PythonService.mService.getApplicationContext();
         }
-        if (PythonWorker.mWorker != null) {
+        if (isWorkerContext()) {
             return PythonWorker.mWorker.getApplicationContext();
         }
         return null;
     }
+
+    public static boolean isActivityContext() {
+        return PythonActivity.mActivity != null;
+    }
+
+    public static boolean isServiceContext() {
+        return PythonService.mService != null;
+    }
+
+    public static boolean isWorkerContext() {
+        return PythonWorker.mWorker != null;
+    }
 }

python-for-android/dists/kolibri/src/main/java/org/learningequality/Kolibri/App.java

@@ -0,0 +1,67 @@
+package org.learningequality.Kolibri;
+
+import android.app.Application;
+import android.content.Context;
+import android.os.Build;
+
+import androidx.annotation.NonNull;
+import androidx.core.app.NotificationManagerCompat;
+import androidx.core.app.NotificationChannelCompat;
+import androidx.work.Configuration;
+
+import org.learningequality.NotificationRef;
+
+import java.util.concurrent.Executors;
+
+public class App extends Application implements Configuration.Provider {
+    @Override
+    public void onCreate() {
+        super.onCreate();
+        NotificationRef.initialize(this);
+        createNotificationChannels();
+    }
+
+    @NonNull
+    @Override
+    public Configuration getWorkManagerConfiguration() {
+        String processName = getApplicationContext().getPackageName();
+        processName += getApplicationContext().getString(R.string.task_worker_process);
+
+
+
+        // Using the same quantity of worker threads as Kolibri's python side:
+        // https://github.com/learningequality/kolibri/blob/release-v0.16.x/kolibri/utils/options.py#L683
+        return new Configuration.Builder()
+                .setDefaultProcessName(processName)
+            .setMinimumLoggingLevel(android.util.Log.DEBUG)
+            .setExecutor(Executors.newFixedThreadPool(6))
+            .build();
+    }
+
+    private void createNotificationChannels() {
+        // Create the NotificationChannel, but only on API 26+ because
+        // the NotificationChannel class is not in the Support Library.
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
+            Context context = getApplicationContext();
+            NotificationChannelCompat serviceChannel = new NotificationChannelCompat.Builder(
+                    context.getString(R.string.notification_service_channel_id),
+                    NotificationManagerCompat.IMPORTANCE_MIN
+            )
+                    .setName(context.getString(R.string.notification_service_channel_title))
+                    .setShowBadge(false)
+                    .build();
+            NotificationChannelCompat taskChannel = new NotificationChannelCompat.Builder(
+                    context.getString(R.string.notification_default_channel_id),
+                    NotificationManagerCompat.IMPORTANCE_DEFAULT
+            )
+                    .setName(context.getString(R.string.notification_default_channel_title))
+                    .build();
+
+            // Register the channel with the system. You can't change the importance
+            // or other notification behaviors after this.
+            NotificationManagerCompat notificationManager = NotificationManagerCompat.from(context);
+            notificationManager.createNotificationChannel(serviceChannel);
+            notificationManager.createNotificationChannel(taskChannel);
+        }
+    }
+}