Merge pull request #4937 from ByteHamster/downloadservice-cleanup

Clean up DownloadService

app/src/main/java/de/danoeh/antennapod/adapter/DownloadlistAdapter.java

@@ -68,8 +68,8 @@ public class DownloadlistAdapter extends BaseAdapter {
         holder.secondaryActionButton.setContentDescription(context.getString(R.string.cancel_download_label));
         holder.secondaryActionButton.setTag(downloader);
         holder.secondaryActionButton.setOnClickListener(butSecondaryListener);
-        holder.secondaryActionProgress.setPercentage(0, request);
 
+        boolean percentageWasSet = false;
         String status = "";
         if (request.getFeedfileType() == Feed.FEEDFILETYPE_FEED) {
             status += context.getString(R.string.download_type_feed);
@@ -85,8 +85,12 @@ public class DownloadlistAdapter extends BaseAdapter {
                 status += " / " + Formatter.formatShortFileSize(context, request.getSize());
                 holder.secondaryActionProgress.setPercentage(
                         0.01f * Math.max(1, request.getProgressPercent()), request);
+                percentageWasSet = true;
             }
         }
+        if (!percentageWasSet) {
+            holder.secondaryActionProgress.setPercentage(0, request);
+        }
         holder.status.setText(status);
 
         return convertView;

core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java

@@ -169,6 +169,7 @@ public class DownloadService extends Service {
             Notification notification = notificationManager.updateNotifications(
                     requester.getNumberOfDownloads(), downloads);
             startForeground(R.id.notification_downloading, notification);
+            setupNotificationUpdaterIfNecessary();
             syncExecutor.execute(() -> onDownloadQueued(intent));
         } else if (numberOfDownloads.get() == 0) {
             stopForeground(true);
@@ -192,10 +193,6 @@ public class DownloadService extends Service {
         registerReceiver(cancelDownloadReceiver, cancelDownloadReceiverFilter);
 
         downloadCompletionThread.start();
-
-        Notification notification = notificationManager.updateNotifications(
-                requester.getNumberOfDownloads(), downloads);
-        startForeground(R.id.notification_downloading, notification);
     }
 
     @Override
@@ -258,13 +255,13 @@ public class DownloadService extends Service {
                             handleSuccessfulDownload(downloader);
                             removeDownload(downloader);
                             numberOfDownloads.decrementAndGet();
-                            queryDownloadsAsync();
+                            stopServiceIfEverythingDoneAsync();
                         });
                     } else {
                         handleFailedDownload(downloader);
                         removeDownload(downloader);
                         numberOfDownloads.decrementAndGet();
-                        queryDownloadsAsync();
+                        stopServiceIfEverythingDoneAsync();
                     }
                 } catch (InterruptedException e) {
                     Log.e(TAG, "DownloadCompletionThread was interrupted");
@@ -413,7 +410,7 @@ public class DownloadService extends Service {
                 }
                 postDownloaders();
             }
-            queryDownloads();
+            stopServiceIfEverythingDone();
         }
 
     };
@@ -484,7 +481,7 @@ public class DownloadService extends Service {
                 postDownloaders();
             });
         }
-        handler.post(this::queryDownloads);
+        handler.post(this::stopServiceIfEverythingDone);
     }
 
     private static boolean isEnqueued(@NonNull DownloadRequest request,
@@ -541,30 +538,26 @@ public class DownloadService extends Service {
      * Calls query downloads on the services main thread. This method should be used instead of queryDownloads if it is
      * used from a thread other than the main thread.
      */
-    private void queryDownloadsAsync() {
+    private void stopServiceIfEverythingDoneAsync() {
-        handler.post(DownloadService.this::queryDownloads);
+        handler.post(DownloadService.this::stopServiceIfEverythingDone);
     }
 
     /**
      * Check if there's something else to download, otherwise stop.
      */
-    private void queryDownloads() {
+    private void stopServiceIfEverythingDone() {
         Log.d(TAG, numberOfDownloads.get() + " downloads left");
 
         if (numberOfDownloads.get() <= 0 && DownloadRequester.getInstance().hasNoDownloads()) {
-            Log.d(TAG, "Number of downloads is " + numberOfDownloads.get() + ", attempting shutdown");
+            Log.d(TAG, "Attempting shutdown");
             stopForeground(true);
             stopSelf();
-            if (notificationUpdater != null) {
+
+            // Trick to hide the notification more quickly when the service is stopped
+            // Without this, the second-last update of the notification stays for 3 seconds after onDestroy returns
             notificationUpdater.run();
-            } else {
+            NotificationManager nm = (NotificationManager) getSystemService(NOTIFICATION_SERVICE);
-                Log.d(TAG, "Skipping notification update");
+            nm.cancel(R.id.notification_downloading);
-            }
-        } else {
-            setupNotificationUpdater();
-            Notification notification = notificationManager.updateNotifications(
-                    requester.getNumberOfDownloads(), downloads);
-            startForeground(R.id.notification_downloading, notification);
         }
     }
 
@@ -617,7 +610,7 @@ public class DownloadService extends Service {
     /**
      * Schedules the notification updater task if it hasn't been scheduled yet.
      */
-    private void setupNotificationUpdater() {
+    private void setupNotificationUpdaterIfNecessary() {
         if (notificationUpdater == null) {
             Log.d(TAG, "Setting up notification updater");
             notificationUpdater = new NotificationUpdater();

core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadServiceNotification.java

@@ -28,7 +28,10 @@ public class DownloadServiceNotification {
 
     private void setupNotificationBuilders() {
         notificationCompatBuilder = new NotificationCompat.Builder(context, NotificationUtils.CHANNEL_ID_DOWNLOADING)
-                .setOngoing(true)
+                .setOngoing(false)
+                .setWhen(0)
+                .setOnlyAlertOnce(true)
+                .setShowWhen(false)
                 .setContentIntent(ClientConfig.downloadServiceCallbacks.getNotificationContentIntent(context))
                 .setSmallIcon(R.drawable.ic_notification_sync);
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {

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

@@ -16,7 +16,6 @@ import de.danoeh.antennapod.core.event.MessageEvent;
 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.feed.FeedPreferences;
 import de.danoeh.antennapod.core.feed.LocalFeedUpdater;
 import de.danoeh.antennapod.core.preferences.UserPreferences;
 import de.danoeh.antennapod.core.service.download.DownloadStatus;
@@ -31,6 +30,7 @@ import java.util.Collections;
 import java.util.Date;
 import java.util.Iterator;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.ExecutorService;
@@ -121,7 +121,18 @@ public final class DBTasks {
             throw new IllegalStateException("DBTasks.refreshAllFeeds() must not be called from the main thread.");
         }
 
-        refreshFeeds(context, DBReader.getFeedList(), initiatedByUser);
+        List<Feed> feeds = DBReader.getFeedList();
+        ListIterator<Feed> iterator = feeds.listIterator();
+        while (iterator.hasNext()) {
+            if (!iterator.next().getPreferences().getKeepUpdated()) {
+                iterator.remove();
+            }
+        }
+        try {
+            refreshFeeds(context, feeds, false, false, false);
+        } catch (DownloadRequestException e) {
+            e.printStackTrace();
+        }
         isRefreshing.set(false);
 
         SharedPreferences prefs = context.getSharedPreferences(PREF_NAME, MODE_PRIVATE);
@@ -134,38 +145,6 @@ public final class DBTasks {
         // See Issue #2577 for the details of the rationale
     }
 
-    /**
-     * @param context
-     * @param feedList the list of feeds to refresh
-     * @param initiatedByUser a boolean indicating if the refresh was triggered by user action.
-     */
-    private static void refreshFeeds(final Context context,
-                                     final List<Feed> feedList,
-                                     boolean initiatedByUser) {
-
-        for (Feed feed : feedList) {
-            FeedPreferences prefs = feed.getPreferences();
-            // feeds with !getKeepUpdated can only be refreshed
-            // directly from the FeedActivity
-            if (prefs.getKeepUpdated()) {
-                try {
-                    refreshFeed(context, feed);
-                } catch (DownloadRequestException e) {
-                    e.printStackTrace();
-                    DBWriter.addDownloadStatus(
-                            new DownloadStatus(feed,
-                                               feed.getHumanReadableIdentifier(),
-                                               DownloadError.ERROR_REQUEST_ERROR,
-                                               false,
-                                               e.getMessage(),
-                                               initiatedByUser)
-                    );
-                }
-            }
-        }
-
-    }
-
     /**
      * Downloads all pages of the given feed even if feed has not been modified since last refresh
      *
@@ -174,7 +153,7 @@ public final class DBTasks {
      */
     public static void forceRefreshCompleteFeed(final Context context, final Feed feed) {
         try {
-            refreshFeed(context, feed, true, true, false);
+            refreshFeeds(context, Collections.singletonList(feed), true, true, false);
         } catch (DownloadRequestException e) {
             e.printStackTrace();
             DBWriter.addDownloadStatus(
@@ -209,19 +188,6 @@ public final class DBTasks {
         }
     }
 
-    /**
-     * Refresh a specific Feed. The refresh may get canceled if the feed does not seem to be modified
-     * and the last update was only few days ago.
-     *
-     * @param context Used for requesting the download.
-     * @param feed    The Feed object.
-     */
-    private static void refreshFeed(Context context, Feed feed)
-            throws DownloadRequestException {
-        Log.d(TAG, "refreshFeed(feed.id: " + feed.getId() +")");
-        refreshFeed(context, feed, false, false, false);
-    }
-
     /**
      * Refresh a specific feed even if feed has not been modified since last refresh
      *
@@ -231,25 +197,31 @@ public final class DBTasks {
     public static void forceRefreshFeed(Context context, Feed feed, boolean initiatedByUser)
             throws DownloadRequestException {
         Log.d(TAG, "refreshFeed(feed.id: " + feed.getId() + ")");
-        refreshFeed(context, feed, false, true, initiatedByUser);
+        refreshFeeds(context, Collections.singletonList(feed), false, true, initiatedByUser);
     }
 
-    private static void refreshFeed(Context context, Feed feed, boolean loadAllPages, boolean force, boolean initiatedByUser)
+    private static void refreshFeeds(Context context, List<Feed> feeds, boolean loadAllPages,
-            throws DownloadRequestException {
+                                    boolean force, boolean initiatedByUser) throws DownloadRequestException {
-        Feed f;
+        List<Feed> localFeeds = new ArrayList<>();
-        String lastUpdate = feed.hasLastUpdateFailed() ? null : feed.getLastUpdate();
+        List<Feed> normalFeeds = new ArrayList<>();
-        if (feed.getPreferences() == null) {
-            f = new Feed(feed.getDownload_url(), lastUpdate, feed.getTitle());
-        } else {
-            f = new Feed(feed.getDownload_url(), lastUpdate, feed.getTitle(),
-                    feed.getPreferences().getUsername(), feed.getPreferences().getPassword());
-        }
-        f.setId(feed.getId());
 
-        if (f.isLocalFeed()) {
+        for (Feed feed : feeds) {
-            new Thread(() -> LocalFeedUpdater.updateFeed(f, context)).start();
+            if (feed.isLocalFeed()) {
+                localFeeds.add(feed);
             } else {
-            DownloadRequester.getInstance().downloadFeed(context, f, loadAllPages, force, initiatedByUser);
+                normalFeeds.add(feed);
+            }
+        }
+
+        if (!localFeeds.isEmpty()) {
+            new Thread(() -> {
+                for (Feed feed : localFeeds) {
+                    LocalFeedUpdater.updateFeed(feed, context);
+                }
+            }).start();
+        }
+        if (!normalFeeds.isEmpty()) {
+            DownloadRequester.getInstance().downloadFeeds(context, feeds, loadAllPages, force, initiatedByUser);
         }
     }
 

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

@@ -17,6 +17,7 @@ import org.apache.commons.io.FilenameUtils;
 
 import java.io.File;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
@@ -184,16 +185,31 @@ public class DownloadRequester implements DownloadStateProvider {
     }
 
     /**
-     * Downloads a feed
+     * Downloads a feed.
      *
      * @param context The application's environment.
-     * @param feed Feed to download
+     * @param feed Feeds to download
      * @param loadAllPages Set to true to download all pages
      */
     public synchronized void downloadFeed(Context context, Feed feed, boolean loadAllPages,
-                                          boolean force, boolean initiatedByUser)
+                                           boolean force, boolean initiatedByUser) throws DownloadRequestException {
-            throws DownloadRequestException {
+        downloadFeeds(context, Collections.singletonList(feed), loadAllPages, force, initiatedByUser);
-        if (feedFileValid(feed)) {
+    }
+
+    /**
+     * Downloads a list of feeds.
+     *
+     * @param context The application's environment.
+     * @param feeds Feeds to download
+     * @param loadAllPages Set to true to download all pages
+     */
+    public synchronized void downloadFeeds(Context context, List<Feed> feeds, boolean loadAllPages,
+                                          boolean force, boolean initiatedByUser) throws DownloadRequestException {
+        List<DownloadRequest> requests = new ArrayList<>();
+        for (Feed feed : feeds) {
+            if (!feedFileValid(feed)) {
+                continue;
+            }
             String username = (feed.getPreferences() != null) ? feed.getPreferences().getUsername() : null;
             String password = (feed.getPreferences() != null) ? feed.getPreferences().getPassword() : null;
             String lastModified = feed.isPaged() || force ? null : feed.getLastUpdate();
@@ -206,9 +222,12 @@ public class DownloadRequester implements DownloadStateProvider {
                     true, username, password, lastModified, true, args, initiatedByUser
             );
             if (request != null) {
-                download(context, request);
+                requests.add(request);
             }
         }
+        if (!requests.isEmpty()) {
+            download(context, requests.toArray(new DownloadRequest[0]));
+        }
     }
 
     public synchronized void downloadFeed(Context context, Feed feed) throws DownloadRequestException {