Merge pull request #3357 from orionlee/autoupdate_thread_refactor_post_workmanager

Refactor automatic feed update - remove extra threads

app/build.gradle

@@ -173,8 +173,8 @@ dependencies {
 
     implementation 'com.github.mfietz:fyydlin:v0.4.2'
     implementation 'com.github.ByteHamster:SearchPreference:v1.3.0'
-    implementation "org.awaitility:awaitility:$awaitilityVersion"
 
+    androidTestImplementation "org.awaitility:awaitility:$awaitilityVersion"
     androidTestImplementation 'com.nanohttpd:nanohttpd-webserver:2.1.1'
     androidTestImplementation "com.jayway.android.robotium:robotium-solo:$robotiumSoloVersion"
     androidTestImplementation 'com.android.support.test.espresso:espresso-core:3.0.2'

app/src/main/java/de/danoeh/antennapod/fragment/EpisodesListFragment.java

@@ -39,17 +39,16 @@ import de.danoeh.antennapod.core.event.DownloadEvent;
 import de.danoeh.antennapod.core.event.DownloaderUpdate;
 import de.danoeh.antennapod.core.event.FeedItemEvent;
 import de.danoeh.antennapod.core.feed.EventDistributor;
-import de.danoeh.antennapod.core.feed.Feed;
 import de.danoeh.antennapod.core.feed.FeedItem;
 import de.danoeh.antennapod.core.feed.FeedMedia;
 import de.danoeh.antennapod.core.service.download.DownloadRequest;
 import de.danoeh.antennapod.core.service.download.DownloadService;
 import de.danoeh.antennapod.core.service.download.Downloader;
-import de.danoeh.antennapod.core.storage.DBTasks;
 import de.danoeh.antennapod.core.storage.DBWriter;
 import de.danoeh.antennapod.core.storage.DownloadRequester;
 import de.danoeh.antennapod.core.util.FeedItemUtil;
 import de.danoeh.antennapod.core.util.LongList;
+import de.danoeh.antennapod.core.util.download.AutoUpdateManager;
 import de.danoeh.antennapod.menuhandler.FeedItemMenuHandler;
 import de.danoeh.antennapod.menuhandler.MenuItemUtils;
 import de.danoeh.antennapod.view.EmptyViewHandler;
@@ -197,10 +196,7 @@ public abstract class EpisodesListFragment extends Fragment {
         if (!super.onOptionsItemSelected(item)) {
             switch (item.getItemId()) {
                 case R.id.refresh_item:
-                    List<Feed> feeds = ((MainActivity) getActivity()).getFeeds();
+                    AutoUpdateManager.runImmediate(requireContext());
-                    if (feeds != null) {
-                        DBTasks.refreshAllFeeds(getActivity(), feeds);
-                    }
                     return true;
                 case R.id.mark_all_read_item:
                     ConfirmationDialog markAllReadConfirmationDialog = new ConfirmationDialog(getActivity(),

app/src/main/java/de/danoeh/antennapod/fragment/QueueFragment.java

@@ -41,14 +41,12 @@ import de.danoeh.antennapod.core.event.DownloaderUpdate;
 import de.danoeh.antennapod.core.event.FeedItemEvent;
 import de.danoeh.antennapod.core.event.QueueEvent;
 import de.danoeh.antennapod.core.feed.EventDistributor;
-import de.danoeh.antennapod.core.feed.Feed;
 import de.danoeh.antennapod.core.feed.FeedItem;
 import de.danoeh.antennapod.core.feed.FeedMedia;
 import de.danoeh.antennapod.core.preferences.UserPreferences;
 import de.danoeh.antennapod.core.service.download.DownloadService;
 import de.danoeh.antennapod.core.service.download.Downloader;
 import de.danoeh.antennapod.core.storage.DBReader;
-import de.danoeh.antennapod.core.storage.DBTasks;
 import de.danoeh.antennapod.core.storage.DBWriter;
 import de.danoeh.antennapod.core.storage.DownloadRequester;
 import de.danoeh.antennapod.core.util.Converter;
@@ -56,6 +54,7 @@ import de.danoeh.antennapod.core.util.FeedItemUtil;
 import de.danoeh.antennapod.core.util.LongList;
 import de.danoeh.antennapod.core.util.QueueSorter;
 import de.danoeh.antennapod.core.util.SortOrder;
+import de.danoeh.antennapod.core.util.download.AutoUpdateManager;
 import de.danoeh.antennapod.dialog.EpisodesApplyActionFragment;
 import de.danoeh.antennapod.menuhandler.FeedItemMenuHandler;
 import de.danoeh.antennapod.menuhandler.MenuItemUtils;
@@ -305,10 +304,7 @@ public class QueueFragment extends Fragment {
                     toggleQueueLock();
                     return true;
                 case R.id.refresh_item:
-                    List<Feed> feeds = ((MainActivity) getActivity()).getFeeds();
+                    AutoUpdateManager.runImmediate(requireContext());
-                    if (feeds != null) {
-                        DBTasks.refreshAllFeeds(getActivity(), feeds);
-                    }
                     return true;
                 case R.id.clear_queue:
                     // make sure the user really wants to clear the queue

app/src/main/java/de/danoeh/antennapod/preferences/PreferenceUpgrader.java

@@ -3,9 +3,11 @@ package de.danoeh.antennapod.preferences;
 import android.content.Context;
 import android.content.SharedPreferences;
 import android.preference.PreferenceManager;
+
 import de.danoeh.antennapod.BuildConfig;
 import de.danoeh.antennapod.R;
 import de.danoeh.antennapod.core.preferences.UserPreferences;
+import de.danoeh.antennapod.core.util.download.AutoUpdateManager;
 import de.danoeh.antennapod.core.util.gui.NotificationUtils;
 
 public class PreferenceUpgrader {
@@ -22,7 +24,7 @@ public class PreferenceUpgrader {
 
         if (oldVersion != newVersion) {
             NotificationUtils.createChannels(context);
-            UserPreferences.restartUpdateAlarm();
+            AutoUpdateManager.restartUpdateAlarm();
 
             upgrade(oldVersion);
             upgraderPrefs.edit().putInt(PREF_CONFIGURED_VERSION, newVersion).apply();

core/build.gradle

@@ -76,7 +76,6 @@ dependencies {
     annotationProcessor "org.greenrobot:eventbus-annotation-processor:$eventbusVersion"
     implementation "io.reactivex.rxjava2:rxandroid:$rxAndroidVersion"
     implementation "io.reactivex.rxjava2:rxjava:$rxJavaVersion"
-    implementation "org.awaitility:awaitility:$awaitilityVersion"
 
     implementation "com.google.android.exoplayer:exoplayer:$exoPlayerVersion"
     implementation "com.github.AntennaPod:AntennaPod-AudioPlayer:$audioPlayerVersion"
@@ -92,6 +91,7 @@ dependencies {
         System.out.println("core: free build hack, skipping some dependencies")
     }
 
+    testImplementation "org.awaitility:awaitility:$awaitilityVersion"
     testImplementation 'junit:junit:4.12'
     testImplementation 'org.mockito:mockito-core:1.10.19'
     androidTestImplementation 'com.android.support.test.espresso:espresso-core:3.0.2'

core/src/main/java/de/danoeh/antennapod/core/preferences/UserPreferences.java

@@ -622,7 +622,7 @@ public class UserPreferences {
              .apply();
         // when updating with an interval, we assume the user wants
         // to update *now* and then every 'hours' interval thereafter.
-        restartUpdateAlarm();
+        AutoUpdateManager.restartUpdateAlarm();
     }
 
     /**
@@ -632,7 +632,7 @@ public class UserPreferences {
         prefs.edit()
              .putString(PREF_UPDATE_INTERVAL, hourOfDay + ":" + minute)
              .apply();
-        restartUpdateAlarm();
+        AutoUpdateManager.restartUpdateAlarm();
     }
 
     public static void disableAutoUpdate() {
@@ -882,19 +882,6 @@ public class UserPreferences {
         return getUpdateTimeOfDay().length == 2;
     }
 
-    public static void restartUpdateAlarm() {
-        if (isAutoUpdateDisabled()) {
-            AutoUpdateManager.disableAutoUpdate();
-        } else if (isAutoUpdateTimeOfDay()) {
-            int[] timeOfDay = getUpdateTimeOfDay();
-            Log.d(TAG, "timeOfDay: " + Arrays.toString(timeOfDay));
-            AutoUpdateManager.restartUpdateTimeOfDayAlarm(timeOfDay[0], timeOfDay[1]);
-        } else {
-            long milliseconds = getUpdateInterval();
-            AutoUpdateManager.restartUpdateIntervalAlarm(milliseconds);
-        }
-    }
-
     /**
      * Evaluates whether Cast support (Chromecast, Audio Cast, etc) is enabled on the preferences.
      */

core/src/main/java/de/danoeh/antennapod/core/receiver/FeedUpdateReceiver.java

@@ -6,8 +6,7 @@ import android.content.Intent;
 import android.util.Log;
 
 import de.danoeh.antennapod.core.ClientConfig;
-import de.danoeh.antennapod.core.preferences.UserPreferences;
+import de.danoeh.antennapod.core.util.download.AutoUpdateManager;
-import de.danoeh.antennapod.core.util.FeedUpdateUtils;
 
 /**
  * Refreshes all feeds when it receives an intent
@@ -20,7 +19,8 @@ public class FeedUpdateReceiver extends BroadcastReceiver {
     public void onReceive(Context context, Intent intent) {
         Log.d(TAG, "Received intent");
         ClientConfig.initialize(context);
-        FeedUpdateUtils.startAutoUpdate(context, null);
+
+        AutoUpdateManager.runOnce();
     }
 
 }

core/src/main/java/de/danoeh/antennapod/core/service/FeedUpdateWorker.java

@@ -2,17 +2,23 @@ package de.danoeh.antennapod.core.service;
 
 import android.content.Context;
 import android.support.annotation.NonNull;
+import android.util.Log;
+
 import androidx.work.Worker;
 import androidx.work.WorkerParameters;
+
 import de.danoeh.antennapod.core.ClientConfig;
 import de.danoeh.antennapod.core.preferences.UserPreferences;
-import de.danoeh.antennapod.core.util.FeedUpdateUtils;
+import de.danoeh.antennapod.core.storage.DBTasks;
-import org.awaitility.Awaitility;
+import de.danoeh.antennapod.core.util.NetworkUtils;
-
+import de.danoeh.antennapod.core.util.download.AutoUpdateManager;
-import java.util.concurrent.atomic.AtomicBoolean;
 
 public class FeedUpdateWorker extends Worker {
 
+    private static final String TAG = "FeedUpdateWorker";
+
+    public static final String PARAM_RUN_ONCE = "runOnce";
+
     public FeedUpdateWorker(@NonNull Context context, @NonNull WorkerParameters params) {
         super(context, params);
     }
@@ -20,16 +26,20 @@ public class FeedUpdateWorker extends Worker {
     @Override
     @NonNull
     public Result doWork() {
+        final boolean isRunOnce = getInputData().getBoolean(PARAM_RUN_ONCE, false);
+        Log.d(TAG, "doWork() : isRunOnce = " + isRunOnce);
         ClientConfig.initialize(getApplicationContext());
 
-        AtomicBoolean finished = new AtomicBoolean(false);
+        if (NetworkUtils.networkAvailable() && NetworkUtils.isFeedRefreshAllowed()) {
-        FeedUpdateUtils.startAutoUpdate(getApplicationContext(), () -> finished.set(true));
+            DBTasks.refreshAllFeeds(getApplicationContext());
-        Awaitility.await().until(finished::get);
+        } else {
+            Log.d(TAG, "Blocking automatic update: no wifi available / no mobile updates allowed");
+        }
 
-        if (UserPreferences.isAutoUpdateTimeOfDay()) {
+        if (!isRunOnce && UserPreferences.isAutoUpdateTimeOfDay()) {
             // WorkManager does not allow to set specific time for repeated tasks.
             // We repeatedly schedule a OneTimeWorkRequest instead.
-            UserPreferences.restartUpdateAlarm();
+            AutoUpdateManager.restartUpdateAlarm();
         }
 
         return Result.success();

core/src/main/java/de/danoeh/antennapod/core/storage/DBTasks.java

@@ -3,7 +3,7 @@ package de.danoeh.antennapod.core.storage;
 import android.content.Context;
 import android.content.SharedPreferences;
 import android.database.Cursor;
-import android.support.annotation.Nullable;
+import android.os.Looper;
 import android.util.Log;
 
 import java.util.ArrayList;
@@ -144,36 +144,24 @@ public final class DBTasks {
     private static final AtomicBoolean isRefreshing = new AtomicBoolean(false);
 
     /**
-     * Refreshes a given list of Feeds in a separate Thread. This method might ignore subsequent calls if it is still
+     * Refreshes all feeds.
+     * It must not be from the main thread.
+     * This method might ignore subsequent calls if it is still
      * enqueuing Feeds for download from a previous call
      *
      * @param context  Might be used for accessing the database
-     * @param feeds    List of Feeds that should be refreshed.
      */
-    public static void refreshAllFeeds(final Context context, final List<Feed> feeds) {
+    public static void refreshAllFeeds(final Context context) {
-        refreshAllFeeds(context, feeds, null);
-    }
-
-    /**
-     * Refreshes a given list of Feeds in a separate Thread. This method might ignore subsequent calls if it is still
-     * enqueuing Feeds for download from a previous call
-     *
-     * @param context  Might be used for accessing the database
-     * @param feeds    List of Feeds that should be refreshed.
-     * @param callback Called after everything was added enqueued for download. Might be null.
-     */
-    public static void refreshAllFeeds(final Context context, final List<Feed> feeds, @Nullable Runnable callback) {
         if (!isRefreshing.compareAndSet(false, true)) {
             Log.d(TAG, "Ignoring request to refresh all feeds: Refresh lock is locked");
             return;
         }
 
-        new Thread(() -> {
+        if (Looper.myLooper() == Looper.getMainLooper()) {
-            if (feeds != null) {
+            throw new IllegalStateException("DBTasks.refreshAllFeeds() must not be called from the main thread.");
-                refreshFeeds(context, feeds);
-            } else {
-                refreshFeeds(context, DBReader.getFeedList());
         }
+
+        refreshFeeds(context, DBReader.getFeedList());
         isRefreshing.set(false);
 
         SharedPreferences prefs = context.getSharedPreferences(PREF_NAME, MODE_PRIVATE);
@@ -186,11 +174,6 @@ public final class DBTasks {
         // Instead it is done after all feeds have been refreshed (asynchronously),
         // in DownloadService.onDestroy()
         // See Issue #2577 for the details of the rationale
-
-            if (callback != null) {
-                callback.run();
-            }
-        }).start();
     }
 
     /**

core/src/main/java/de/danoeh/antennapod/core/util/FeedUpdateUtils.java

@@ -1,36 +0,0 @@
-package de.danoeh.antennapod.core.util;
-
-import android.content.Context;
-import android.util.Log;
-
-import org.awaitility.core.ConditionTimeoutException;
-
-import java.util.concurrent.TimeUnit;
-
-import de.danoeh.antennapod.core.storage.DBTasks;
-
-import static org.awaitility.Awaitility.with;
-
-public class FeedUpdateUtils {
-    private static final String TAG = "FeedUpdateUtils";
-
-    private FeedUpdateUtils() {}
-
-    public static void startAutoUpdate(Context context, Runnable callback) {
-        // the network check is blocking for possibly a long time: so run the logic
-        // in a separate thread to prevent the code blocking the callers
-        final Runnable runnable = () -> {
-            try {
-                with().pollInterval(1, TimeUnit.SECONDS)
-                        .await()
-                        .atMost(10, TimeUnit.SECONDS)
-                        .until(() -> NetworkUtils.networkAvailable() && NetworkUtils.isFeedRefreshAllowed());
-                DBTasks.refreshAllFeeds(context, null, callback);
-            } catch (ConditionTimeoutException ignore) {
-                Log.d(TAG, "Blocking automatic update: no wifi available / no mobile updates allowed");
-            }
-        };
-        new Thread(runnable).start();
-    }
-
-}

core/src/main/java/de/danoeh/antennapod/core/util/download/AutoUpdateManager.java

@@ -1,31 +1,55 @@
 package de.danoeh.antennapod.core.util.download;
 
+import android.content.Context;
+import android.support.annotation.NonNull;
 import android.util.Log;
+
 import androidx.work.Constraints;
+import androidx.work.Data;
 import androidx.work.ExistingPeriodicWorkPolicy;
 import androidx.work.ExistingWorkPolicy;
 import androidx.work.NetworkType;
 import androidx.work.OneTimeWorkRequest;
 import androidx.work.PeriodicWorkRequest;
 import androidx.work.WorkManager;
-import de.danoeh.antennapod.core.preferences.UserPreferences;
-import de.danoeh.antennapod.core.service.FeedUpdateWorker;
 
+import java.util.Arrays;
 import java.util.Calendar;
 import java.util.concurrent.TimeUnit;
 
+import de.danoeh.antennapod.core.preferences.UserPreferences;
+import de.danoeh.antennapod.core.service.FeedUpdateWorker;
+import de.danoeh.antennapod.core.storage.DBTasks;
+
 public class AutoUpdateManager {
-    private static final String WORK_ID_FEED_UPDATE = FeedUpdateWorker.class.getName();
+    private static final String WORK_ID_FEED_UPDATE = "de.danoeh.antennapod.core.service.FeedUpdateWorker";
+    private static final String WORK_ID_FEED_UPDATE_ONCE = WORK_ID_FEED_UPDATE + "Once";
     private static final String TAG = "AutoUpdateManager";
 
     private AutoUpdateManager() {
 
     }
 
+    /**
+     * Start / restart periodic auto feed refresh
+     */
+    public static void restartUpdateAlarm() {
+        if (UserPreferences.isAutoUpdateDisabled()) {
+            disableAutoUpdate();
+        } else if (UserPreferences.isAutoUpdateTimeOfDay()) {
+            int[] timeOfDay = UserPreferences.getUpdateTimeOfDay();
+            Log.d(TAG, "timeOfDay: " + Arrays.toString(timeOfDay));
+            restartUpdateTimeOfDayAlarm(timeOfDay[0], timeOfDay[1]);
+        } else {
+            long milliseconds = UserPreferences.getUpdateInterval();
+            restartUpdateIntervalAlarm(milliseconds);
+        }
+    }
+
     /**
      * Sets the interval in which the feeds are refreshed automatically
      */
-    public static void restartUpdateIntervalAlarm(long intervalMillis) {
+    private static void restartUpdateIntervalAlarm(long intervalMillis) {
         Log.d(TAG, "Restarting update alarm.");
 
         PeriodicWorkRequest workRequest = new PeriodicWorkRequest.Builder(FeedUpdateWorker.class,
@@ -40,7 +64,7 @@ public class AutoUpdateManager {
     /**
      * Sets time of day the feeds are refreshed automatically
      */
-    public static void restartUpdateTimeOfDayAlarm(int hoursOfDay, int minute) {
+    private static void restartUpdateTimeOfDayAlarm(int hoursOfDay, int minute) {
         Log.d(TAG, "Restarting update alarm.");
 
         Calendar now = Calendar.getInstance();
@@ -60,6 +84,41 @@ public class AutoUpdateManager {
         WorkManager.getInstance().enqueueUniqueWork(WORK_ID_FEED_UPDATE, ExistingWorkPolicy.REPLACE, workRequest);
     }
 
+    /**
+     * Run auto feed refresh once in background, as soon as what OS scheduling allows.
+     *
+     * Callers from UI should use {@link #runImmediate(Context)}, as it will guarantee
+     * the refresh be run immediately.
+     */
+    public static void runOnce() {
+        Log.d(TAG, "Run auto update once, as soon as OS allows.");
+
+        OneTimeWorkRequest workRequest = new OneTimeWorkRequest.Builder(FeedUpdateWorker.class)
+                .setConstraints(getConstraints())
+                .setInitialDelay(0L, TimeUnit.MILLISECONDS)
+                .setInputData(new Data.Builder()
+                        .putBoolean(FeedUpdateWorker.PARAM_RUN_ONCE, true)
+                        .build()
+                )
+                .build();
+
+        WorkManager.getInstance().enqueueUniqueWork(WORK_ID_FEED_UPDATE_ONCE, ExistingWorkPolicy.REPLACE, workRequest);
+
+    }
+
+    /**
+     /**
+     * Run auto feed refresh once in background immediately, using its own thread.
+     *
+     * Callers where the additional threads is not suitable should use {@link #runOnce()}
+     */
+    public static void runImmediate(@NonNull Context context) {
+        Log.d(TAG, "Run auto update immediately in background.");
+        new Thread(() -> {
+            DBTasks.refreshAllFeeds(context.getApplicationContext());
+        }, "ManualRefreshAllFeeds").start();
+    }
+
     public static void disableAutoUpdate() {
         WorkManager.getInstance().cancelUniqueWork(WORK_ID_FEED_UPDATE);
     }
@@ -74,4 +133,5 @@ public class AutoUpdateManager {
         }
         return constraints.build();
     }
+
 }