refactor downloadMedia() - make DownloadService accepts a batch of DownloadRequests.

- the DB logic originally in DBTasks.downloadFeedItems() are moved to DownloadService.

app/src/androidTest/java/de/test/antennapod/service/download/DownloadServiceTest.java

@@ -22,6 +22,7 @@ import java.util.concurrent.TimeUnit;
 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.DownloadRequest;
 import de.danoeh.antennapod.core.service.download.DownloadService;
 import de.danoeh.antennapod.core.service.download.DownloadStatus;
@@ -77,16 +78,29 @@ public class DownloadServiceTest {
     }
 
     @Test
-    public void testEventsGeneratedCaseMediaDownloadSuccess() throws Exception {
+    public void testEventsGeneratedCaseMediaDownloadSuccess_noEnqueue() throws Exception {
+        doTestEventsGeneratedCaseMediaDownloadSuccess(false, 1);
+    }
+
+    @Test
+    public void testEventsGeneratedCaseMediaDownloadSuccess_withEnqueue() throws Exception {
+        // enqueue itself generates additional FeedItem event
+        doTestEventsGeneratedCaseMediaDownloadSuccess(true, 2);
+    }
+
+    private void doTestEventsGeneratedCaseMediaDownloadSuccess(boolean enqueueDownloaded,
+                                                               int numEventsExpected)
+            throws Exception {
         // create a stub download that returns successful
         //
         // OPEN: Ideally, I'd like the download time long enough so that multiple in-progress DownloadEvents
         // are generated (to simulate typical download), but it'll make download time quite long (1-2 seconds)
         // to do so
         DownloadService.setDownloaderFactory(new StubDownloaderFactory(50, downloadStatus -> {
-           downloadStatus.setSuccessful();
+            downloadStatus.setSuccessful();
         }));
 
+        UserPreferences.setEnqueueDownloadedEpisodes(enqueueDownloaded);
         withFeedItemEventListener(feedItemEventListener -> {
             try {
                 assertEquals(0, feedItemEventListener.getEvents().size());
@@ -96,9 +110,12 @@ public class DownloadServiceTest {
                 DownloadRequester.getInstance().downloadMedia(false, InstrumentationRegistry.getTargetContext(),
                         testMedia11.getItem());Awaitility.await()
                         .atMost(1000, TimeUnit.MILLISECONDS)
-                        .until(() -> feedItemEventListener.getEvents().size() > 0);
+                        .until(() -> feedItemEventListener.getEvents().size() >= numEventsExpected);
                 assertTrue("After media download has completed, FeedMedia object in db should indicate so.",
                         DBReader.getFeedMedia(testMedia11.getId()).isDownloaded());
+                assertEquals("The FeedItem should have been " + (enqueueDownloaded ? "" : "not ") +  "enqueued",
+                        enqueueDownloaded,
+                        DBReader.getQueueIDList().contains(testMedia11.getItem().getId()));
             } catch (ConditionTimeoutException cte) {
                 fail("The expected FeedItemEvent (for media download complete) has not been posted. "
                         + cte.getMessage());

app/src/androidTest/java/de/test/antennapod/storage/DBTasksTest.java

@@ -206,7 +206,7 @@ public class DBTasksTest {
 
         // Run actual test and assert results
         List<? extends FeedItem> actualEnqueued =
-                DBTasks.enqueueFeedItemsToDownload(context, itemsToDownload);
+                DBTasks.enqueueFeedItemsToDownload(context, Arrays.asList(itemsToDownload));
 
         assertEqualsByIds("Only items not in the queue are enqueued", expectedEnqueued, actualEnqueued);
         assertEqualsByIds("Queue has new items appended", expectedQueue, DBReader.getQueue());
@@ -229,7 +229,7 @@ public class DBTasksTest {
 
         // Run actual test and assert results
         List<? extends FeedItem> actualEnqueued =
-                DBTasks.enqueueFeedItemsToDownload(context, itemsToDownload);
+                DBTasks.enqueueFeedItemsToDownload(context, Arrays.asList(itemsToDownload));
 
         assertEqualsByIds("No item is enqueued", expectedEnqueued, actualEnqueued);
         assertEqualsByIds("Queue is unchanged", expectedQueue, DBReader.getQueue());

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

@@ -12,8 +12,30 @@ import android.os.Handler;
 import android.os.IBinder;
 import android.text.TextUtils;
 import android.util.Log;
+
+import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
+
+import org.apache.commons.io.FileUtils;
+import org.greenrobot.eventbus.EventBus;
+
+import java.io.File;
+import java.io.IOException;
+import java.net.HttpURLConnection;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorCompletionService;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
 import de.danoeh.antennapod.core.ClientConfig;
 import de.danoeh.antennapod.core.event.DownloadEvent;
 import de.danoeh.antennapod.core.event.FeedItemEvent;
@@ -32,27 +54,10 @@ 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.DownloadError;
-import java.io.File;
-import java.io.IOException;
-import java.net.HttpURLConnection;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
-import java.util.concurrent.CompletionService;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.ExecutorCompletionService;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledFuture;
-import java.util.concurrent.ScheduledThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicInteger;
-import org.apache.commons.io.FileUtils;
-import org.greenrobot.eventbus.EventBus;
 
 /**
  * Manages the download of feedfiles in the app. Downloads can be enqueued via the startService intent.
- * The argument of the intent is an instance of DownloadRequest in the EXTRA_REQUEST field of
+ * The argument of the intent is an instance of DownloadRequest in the EXTRA_REQUESTS field of
  * the intent.
  * After the downloads have finished, the downloaded object will be passed on to a specific handler, depending on the
  * type of the feedfile.
@@ -79,7 +84,9 @@ public class DownloadService extends Service {
     /**
      * Extra for ACTION_ENQUEUE_DOWNLOAD intent.
      */
-    public static final String EXTRA_REQUEST = "request";
+    public static final String EXTRA_REQUESTS = "downloadRequests";
+
+    public static final String EXTRA_CLEANUP_MEDIA = "cleanupMedia";
 
     public static final int NOTIFICATION_ID = 2;
 
@@ -156,7 +163,8 @@ public class DownloadService extends Service {
 
     @Override
     public int onStartCommand(Intent intent, int flags, int startId) {
-        if (intent.getParcelableExtra(EXTRA_REQUEST) != null) {
+        if (intent != null &&
+                intent.getParcelableArrayListExtra(EXTRA_REQUESTS) != null) {
             onDownloadQueued(intent);
         } else if (numberOfDownloads.get() == 0) {
             stopSelf();
@@ -387,13 +395,55 @@ public class DownloadService extends Service {
     };
 
     private void onDownloadQueued(Intent intent) {
-        Log.d(TAG, "Received enqueue request");
+        List<DownloadRequest> requests = intent.getParcelableArrayListExtra(EXTRA_REQUESTS);
-        DownloadRequest request = intent.getParcelableExtra(EXTRA_REQUEST);
+        if (requests == null) {
-        if (request == null) {
             throw new IllegalArgumentException(
                     "ACTION_ENQUEUE_DOWNLOAD intent needs request extra");
         }
+        boolean cleanupMedia = intent.getBooleanExtra(EXTRA_CLEANUP_MEDIA, false);
+        Log.d(TAG, "Received enqueue request. #requests=" + requests.size()
+                + ", cleanupMedia=" + cleanupMedia);
 
+        if (cleanupMedia) {
+            ClientConfig.dbTasksCallbacks.getEpisodeCacheCleanupAlgorithm()
+                    .makeRoomForEpisodes(getApplicationContext(), requests.size());
+        }
+
+        // #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
+        List<? extends FeedItem> itemsEnqueued;
+        try {
+            itemsEnqueued = enqueueFeedItems(requests);
+        } catch (Exception e) {
+            Log.e(TAG, "Unexpected exception during enqueue before downloads. Abort download", e);
+            return;
+        }
+        // TODO-2448: use itemsEnqueued to handle cancel download
+
+        for (DownloadRequest request : requests) {
+            onDownloadQueued(request);
+        }
+    }
+
+    private List<? extends FeedItem> enqueueFeedItems(@NonNull List<? extends DownloadRequest> requests)
+        throws Exception {
+        List<FeedItem> feedItems = new ArrayList<>();
+        for (DownloadRequest request : requests) {
+            if (request.getFeedfileType() == FeedMedia.FEEDFILETYPE_FEEDMEDIA) {
+                long mediaId = request.getFeedfileId();
+                FeedMedia media = DBReader.getFeedMedia(mediaId);
+                if (media == null) {
+                    Log.w(TAG, "enqueueFeedItems() : FeedFile Id " + mediaId + " is not found. ignore it.");
+                    continue;
+                }
+                feedItems.add(media.getItem());
+            }
+        }
+
+        return DBTasks.enqueueFeedItemsToDownload(getApplicationContext(), feedItems);
+    }
+
+    private void onDownloadQueued(@NonNull DownloadRequest request) {
         writeFileUrl(request);
 
         Downloader downloader = downloaderFactory.create(request);

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

@@ -308,7 +308,7 @@ public final class DBTasks {
 
     @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
     public static List<? extends FeedItem> enqueueFeedItemsToDownload(final Context context,
-                                                                      FeedItem... items)
+                                                                      List<? extends FeedItem> items)
             throws InterruptedException, ExecutionException {
         List<FeedItem> itemsToEnqueue = new ArrayList<>();
         if (UserPreferences.enqueueDownloadedEpisodes()) {

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

@@ -21,7 +21,6 @@ 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;
@@ -77,14 +76,20 @@ public class DownloadRequester implements DownloadStateProvider {
      * ensure that the data is valid. Use downloadFeed(), downloadImage() or downloadMedia() instead.
      *
      * @param context Context object for starting the DownloadService
-     * @param requests The list of DownloadRequest objects. If another DownloadRequest with the same source URL is already stored,
+     * @param requests The list of DownloadRequest objects. If another DownloadRequest
-     *                 this one will be skipped.
+     *                 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.
+     * @return True if any of the download request was accepted, false otherwise.
      */
     public synchronized boolean download(@NonNull Context context,
                                          DownloadRequest... requests) {
+        return download(context, false, requests);
+    }
+
+    private boolean download(@NonNull Context context, boolean cleanupMedia,
+                             DownloadRequest... requests) {
         boolean result = false;
-        // TODO-2448: send the requests as a batch to the service in one intent
+
+        ArrayList<DownloadRequest> requestsToSend = new ArrayList<>(requests.length);
         for (DownloadRequest request : requests) {
             if (downloads.containsKey(request.getSource())) {
                 if (BuildConfig.DEBUG) Log.i(TAG, "DownloadRequest is already stored.");
@@ -92,11 +97,15 @@ public class DownloadRequester implements DownloadStateProvider {
             }
             downloads.put(request.getSource(), request);
 
-            Intent launchIntent = new Intent(context, DownloadService.class);
+            requestsToSend.add(request);
-            launchIntent.putExtra(DownloadService.EXTRA_REQUEST, request);
-            ContextCompat.startForegroundService(context, launchIntent);
             result = true;
         }
+        Intent launchIntent = new Intent(context, DownloadService.class);
+        launchIntent.putParcelableArrayListExtra(DownloadService.EXTRA_REQUESTS, requestsToSend);
+        if (cleanupMedia) {
+            launchIntent.putExtra(DownloadService.EXTRA_CLEANUP_MEDIA, cleanupMedia);
+        }
+        ContextCompat.startForegroundService(context, launchIntent);
 
         return result;
     }
@@ -213,27 +222,6 @@ public class DownloadRequester implements DownloadStateProvider {
             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) {
@@ -260,7 +248,7 @@ public class DownloadRequester implements DownloadStateProvider {
                 }
             }
         }
-        download(context, requests.toArray(new DownloadRequest[0]));
+        download(context, performAutoCleanup, requests.toArray(new DownloadRequest[0]));
     }
 
     @Nullable