Refactor downloadMedia() : move entry points to DownloadRequester (from DBTasks)

3 files changed, 124 insertions, 106 deletions

diff --git a/core/src/main/java/de/danoeh/antennapod/core/storage/APDownloadAlgorithm.java b/core/src/main/java/de/danoeh/antennapod/core/storage/APDownloadAlgorithm.java
index aa97b321a..218320c68 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/storage/APDownloadAlgorithm.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/storage/APDownloadAlgorithm.java
@@ -92,7 +92,7 @@ public class APDownloadAlgorithm implements AutomaticDownloadAlgorithm {
                 Log.d(TAG, "Enqueueing " + itemsToDownload.length + " items for download");
 
                 try {
-                    DBTasks.downloadFeedItems(false, context, itemsToDownload);
+                    DownloadRequester.getInstance().downloadMedia(false, context, itemsToDownload);
                 } catch (DownloadRequestException e) {
                     e.printStackTrace();
                 }
diff --git a/core/src/main/java/de/danoeh/antennapod/core/storage/DBTasks.java b/core/src/main/java/de/danoeh/antennapod/core/storage/DBTasks.java
index 7dc53f8b3..4237b2175 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/storage/DBTasks.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/storage/DBTasks.java
@@ -9,6 +9,8 @@ import android.util.Log;
 
 import androidx.annotation.VisibleForTesting;
 
+import org.greenrobot.eventbus.EventBus;
+
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Date;
@@ -38,7 +40,6 @@ import de.danoeh.antennapod.core.util.LongList;
 import de.danoeh.antennapod.core.util.comparator.FeedItemPubdateComparator;
 import de.danoeh.antennapod.core.util.exception.MediaFileNotFoundException;
 import de.danoeh.antennapod.core.util.playback.PlaybackServiceStarter;
-import org.greenrobot.eventbus.EventBus;
 
 import static android.content.Context.MODE_PRIVATE;
 
@@ -305,68 +306,7 @@ public final class DBTasks {
         EventBus.getDefault().post(new FeedListUpdateEvent(media.getItem().getFeed()));
     }
 
-    /**
-     * Requests the download of a list of FeedItem objects.
-     *
-     * @param context Used for requesting the download and accessing the DB.
-     * @param items   The FeedItem objects.
-     */
-    public static void downloadFeedItems(final Context context,
-                                         FeedItem... items) throws DownloadRequestException {
-        downloadFeedItems(true, context, items);
-    }
-
-    static void downloadFeedItems(boolean performAutoCleanup,
-                                  final Context context, final FeedItem... items)
-            throws DownloadRequestException {
-        final DownloadRequester requester = DownloadRequester.getInstance();
-
-        if (performAutoCleanup) {
-            new Thread() {
-
-                @Override
-                public void run() {
-                    ClientConfig.dbTasksCallbacks.getEpisodeCacheCleanupAlgorithm()
-                            .makeRoomForEpisodes(context, items.length);
-                }
-
-            }.start();
-        }
-        // #2448: First, add to-download items to the queue before actual download
-        // so that the resulting queue order is the same as when download is clicked
-        try {
-            enqueueFeedItemsToDownload(context, items);
-        } catch (Throwable t) {
-            throw new DownloadRequestException("Unexpected exception during enqueue before downloads", t);
-        }
-
-        // Then, download them
-        for (FeedItem item : items) {
-            if (item.getMedia() != null
-                    && !requester.isDownloadingFile(item.getMedia())
-                    && !item.getMedia().isDownloaded()) {
-                if (items.length > 1) {
-                    try {
-                        requester.downloadMedia(context, item.getMedia());
-                    } catch (DownloadRequestException e) {
-                        e.printStackTrace();
-                        DBWriter.addDownloadStatus(
-                                new DownloadStatus(item.getMedia(), item
-                                        .getMedia()
-                                        .getHumanReadableIdentifier(),
-                                        DownloadError.ERROR_REQUEST_ERROR,
-                                        false, e.getMessage()
-                                )
-                        );
-                    }
-                } else {
-                    requester.downloadMedia(context, item.getMedia());
-                }
-            }
-        }
-    }
-
-    @VisibleForTesting
+    @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
     public static List<? extends FeedItem> enqueueFeedItemsToDownload(final Context context,
                                                                       FeedItem... items)
             throws InterruptedException, ExecutionException {
diff --git a/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadRequester.java b/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadRequester.java
index c61abc168..d36592ec8 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadRequester.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadRequester.java
@@ -8,21 +8,29 @@ import android.util.Log;
 import android.webkit.URLUtil;
 
 import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
+import androidx.annotation.VisibleForTesting;
 import androidx.core.content.ContextCompat;
 
 import org.apache.commons.io.FilenameUtils;
 
 import java.io.File;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
 import de.danoeh.antennapod.core.BuildConfig;
+import de.danoeh.antennapod.core.ClientConfig;
 import de.danoeh.antennapod.core.feed.Feed;
 import de.danoeh.antennapod.core.feed.FeedFile;
+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.DownloadRequest;
 import de.danoeh.antennapod.core.service.download.DownloadService;
+import de.danoeh.antennapod.core.service.download.DownloadStatus;
+import de.danoeh.antennapod.core.util.DownloadError;
 import de.danoeh.antennapod.core.util.FileNameGenerator;
 import de.danoeh.antennapod.core.util.IntentUtils;
 import de.danoeh.antennapod.core.util.URLChecker;
@@ -64,40 +72,46 @@ public class DownloadRequester implements DownloadStateProvider {
     }
 
     /**
-     * Starts a new download with the given DownloadRequest. This method should only
+     * Starts a new download with the given a list of DownloadRequest. This method should only
      * be used from outside classes if the DownloadRequest was created by the DownloadService to
      * ensure that the data is valid. Use downloadFeed(), downloadImage() or downloadMedia() instead.
      *
      * @param context Context object for starting the DownloadService
-     * @param request The DownloadRequest. If another DownloadRequest with the same source URL is already stored, this method
-     *                call will return false.
-     * @return True if the download request was accepted, false otherwise.
+     * @param requests The list of DownloadRequest objects. If another DownloadRequest with the same source URL is already stored,
+     *                 this one will be skipped.
+     * @return True if the  any of the download request was accepted, false otherwise.
      */
     public synchronized boolean download(@NonNull Context context,
-                                         @NonNull DownloadRequest request) {
-        if (downloads.containsKey(request.getSource())) {
-            if (BuildConfig.DEBUG) Log.i(TAG, "DownloadRequest is already stored.");
-            return false;
-        }
-        downloads.put(request.getSource(), request);
+                                         DownloadRequest... requests) {
+        boolean result = false;
+        // TODO-2448: send the requests as a batch to the service in one intent
+        for (DownloadRequest request : requests) {
+            if (downloads.containsKey(request.getSource())) {
+                if (BuildConfig.DEBUG) Log.i(TAG, "DownloadRequest is already stored.");
+                continue;
+            }
+            downloads.put(request.getSource(), request);
 
-        Intent launchIntent = new Intent(context, DownloadService.class);
-        launchIntent.putExtra(DownloadService.EXTRA_REQUEST, request);
-        ContextCompat.startForegroundService(context, launchIntent);
+            Intent launchIntent = new Intent(context, DownloadService.class);
+            launchIntent.putExtra(DownloadService.EXTRA_REQUEST, request);
+            ContextCompat.startForegroundService(context, launchIntent);
+            result = true;
+        }
 
-        return true;
+        return result;
     }
 
-    private void download(Context context, FeedFile item, FeedFile container, File dest,
-                          boolean overwriteIfExists, String username, String password,
-                          String lastModified, boolean deleteOnFailure, Bundle arguments) {
+    @Nullable
+    private DownloadRequest createRequest(FeedFile item, FeedFile container, File dest,
+                                          boolean overwriteIfExists, String username, String password,
+                                          String lastModified, boolean deleteOnFailure, Bundle arguments) {
         final boolean partiallyDownloadedFileExists = item.getFile_url() != null && new File(item.getFile_url()).exists();
 
         Log.d(TAG, "partiallyDownloadedFileExists: " + partiallyDownloadedFileExists);
         if (isDownloadingFile(item)) {
-                Log.e(TAG, "URL " + item.getDownload_url()
-                        + " is already being downloaded");
-            return;
+            Log.e(TAG, "URL " + item.getDownload_url()
+                    + " is already being downloaded");
+            return null;
         }
         if (!isFilenameAvailable(dest.toString()) || (!partiallyDownloadedFileExists && dest.exists())) {
             Log.d(TAG, "Filename already used.");
@@ -136,8 +150,7 @@ public class DownloadRequester implements DownloadStateProvider {
                 .lastModified(lastModified)
                 .deleteOnFailure(deleteOnFailure)
                 .withArguments(arguments);
-        DownloadRequest request = builder.build();
-        download(context, request);
+        return builder.build();
     }
 
     /**
@@ -178,8 +191,9 @@ public class DownloadRequester implements DownloadStateProvider {
             args.putInt(REQUEST_ARG_PAGE_NR, feed.getPageNr());
             args.putBoolean(REQUEST_ARG_LOAD_ALL_PAGES, loadAllPages);
 
-            download(context, feed, null, new File(getFeedfilePath(), getFeedfileName(feed)),
+            DownloadRequest request = createRequest(feed, null, new File(getFeedfilePath(), getFeedfileName(feed)),
                     true, username, password, lastModified, true, args);
+            download(context, request);
         }
     }
 
@@ -187,29 +201,93 @@ public class DownloadRequester implements DownloadStateProvider {
         downloadFeed(context, feed, false, false);
     }
 
-    public synchronized void downloadMedia(Context context, FeedMedia feedmedia)
+    public synchronized void downloadMedia(@NonNull Context context, FeedItem... feedItems)
             throws DownloadRequestException {
-        if (feedFileValid(feedmedia)) {
-            Feed feed = feedmedia.getItem().getFeed();
-            String username;
-            String password;
-            if (feed != null && feed.getPreferences() != null) {
-                username = feed.getPreferences().getUsername();
-                password = feed.getPreferences().getPassword();
-            } else {
-                username = null;
-                password = null;
-            }
+        downloadMedia(true, context, feedItems);
 
-            File dest;
-            if (feedmedia.getFile_url() != null) {
-                dest = new File(feedmedia.getFile_url());
-            } else {
-                dest = new File(getMediafilePath(feedmedia), getMediafilename(feedmedia));
+    }
+
+    @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
+    public synchronized void downloadMedia(boolean performAutoCleanup, @NonNull Context context,
+                                    FeedItem... items)
+            throws DownloadRequestException {
+        Log.d(TAG, "downloadMedia() called with: performAutoCleanup = [" + performAutoCleanup
+                + "], #items = [" + items.length + "]");
+        // TODO-2448: OPEN: move to DownloadService as well?!
+        if (performAutoCleanup) {
+            new Thread() {
+
+                @Override
+                public void run() {
+                    ClientConfig.dbTasksCallbacks.getEpisodeCacheCleanupAlgorithm()
+                            .makeRoomForEpisodes(context, items.length);
+                }
+
+            }.start();
+        }
+
+        // TODO-2448: move to DownloadService
+        // #2448: First, add to-download items to the queue before actual download
+        // so that the resulting queue order is the same as when download is clicked
+//        try {
+//            DBTasks.enqueueFeedItemsToDownload(context, items);
+//        } catch (Throwable t) {
+//            throw new DownloadRequestException("Unexpected exception during enqueue before downloads", t);
+//        }
+
+        List<DownloadRequest> requests = new ArrayList<>(items.length);
+        for (FeedItem item : items) {
+            try {
+                DownloadRequest request = createRequest(item.getMedia());
+                if (request != null) {
+                    requests.add(request);
+                }
+            } catch (DownloadRequestException e) {
+                if (items.length < 2) {
+                    // single download, typically initiated from users
+                    throw e;
+                } else {
+                    // batch download, typically initiated by auto-download in the background
+                    e.printStackTrace();
+                    DBWriter.addDownloadStatus(
+                            new DownloadStatus(item.getMedia(), item
+                                    .getMedia()
+                                    .getHumanReadableIdentifier(),
+                                    DownloadError.ERROR_REQUEST_ERROR,
+                                    false, e.getMessage()
+                            )
+                    );
+                }
             }
-            download(context, feedmedia, feed,
-                    dest, false, username, password, null, false, null);
         }
+        download(context, requests.toArray(new DownloadRequest[0]));
+    }
+
+    @Nullable
+    private DownloadRequest createRequest(@Nullable FeedMedia feedmedia)
+            throws DownloadRequestException {
+        if (!feedFileValid(feedmedia)) {
+            return null;
+        }
+        Feed feed = feedmedia.getItem().getFeed();
+        String username;
+        String password;
+        if (feed != null && feed.getPreferences() != null) {
+            username = feed.getPreferences().getUsername();
+            password = feed.getPreferences().getPassword();
+        } else {
+            username = null;
+            password = null;
+        }
+
+        File dest;
+        if (feedmedia.getFile_url() != null) {
+            dest = new File(feedmedia.getFile_url());
+        } else {
+            dest = new File(getMediafilePath(feedmedia), getMediafilename(feedmedia));
+        }
+        return createRequest(feedmedia, feed,
+                dest, false, username, password, null, false, null);
     }
 
     /**