Merge pull request #3592 from orionlee/more_respect_download_order_2448_handle_cancel

More respect download order - handle cancel

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

@@ -5,7 +5,6 @@ import androidx.annotation.Nullable;
 import androidx.test.InstrumentationRegistry;
 import androidx.test.runner.AndroidJUnit4;
 
-import de.danoeh.antennapod.core.service.download.DownloaderFactory;
 import org.awaitility.Awaitility;
 import org.awaitility.core.ConditionTimeoutException;
 import org.junit.After;
@@ -22,10 +21,12 @@ 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;
 import de.danoeh.antennapod.core.service.download.Downloader;
+import de.danoeh.antennapod.core.service.download.DownloaderFactory;
 import de.danoeh.antennapod.core.service.download.StubDownloader;
 import de.danoeh.antennapod.core.storage.DBReader;
 import de.danoeh.antennapod.core.storage.DBWriter;
@@ -58,7 +59,9 @@ public class DownloadServiceTest {
     }
 
     private Feed setUpTestFeeds() throws Exception {
-        Feed feed = new Feed("url", null, "Test Feed title 1");
+        // To avoid complication in case of test failures, leaving behind orphaned
+        // media files: add a timestamp so that each test run will have its own directory for media files.
+        Feed feed = new Feed("url", null, "Test Feed title 1 " + System.currentTimeMillis());
         List<FeedItem> items = new ArrayList<>();
         feed.setItems(items);
         FeedItem item1 = new FeedItem(0, "Item 1-1", "Item 1-1", "url", new Date(), FeedItem.NEW, feed);
@@ -77,29 +80,108 @@ 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());
                 assertFalse("The media in test should not yet been downloaded",
                         DBReader.getFeedMedia(testMedia11.getId()).isDownloaded());
 
-                DownloadRequester.getInstance().downloadMedia(InstrumentationRegistry.getTargetContext(),
+                DownloadRequester.getInstance().downloadMedia(false, InstrumentationRegistry.getTargetContext(),
-                        testMedia11);
+                        testMedia11.getItem());Awaitility.await()
-                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());
+            }
+        });
+    }
+
+    @Test
+    public void testCancelDownload_UndoEnqueue_Normal() throws Exception {
+        doTestCancelDownload_UndoEnqueue(false);
+    }
+
+    @Test
+    public void testCancelDownload_UndoEnqueue_AlreadyInQueue() throws Exception {
+        doTestCancelDownload_UndoEnqueue(true);
+    }
+
+    private void doTestCancelDownload_UndoEnqueue(boolean itemAlreadyInQueue) throws Exception {
+        // let download takes longer to ensure the test can cancel the download in time
+        DownloadService.setDownloaderFactory(new StubDownloaderFactory(150, downloadStatus -> {
+            downloadStatus.setSuccessful();
+        }));
+        UserPreferences.setEnqueueDownloadedEpisodes(true);
+        UserPreferences.setEnableAutodownload(false);
+
+        final long item1Id = testMedia11.getItem().getId();
+        if (itemAlreadyInQueue) {
+            // simulate item already in queue condition
+            DBWriter.addQueueItem(InstrumentationRegistry.getTargetContext(), false, item1Id).get();
+            assertTrue(DBReader.getQueueIDList().contains(item1Id));
+        } else {
+            assertFalse(DBReader.getQueueIDList().contains(item1Id));
+        }
+
+        withFeedItemEventListener(feedItemEventListener -> {
+            try {
+                DownloadRequester.getInstance().downloadMedia(false, InstrumentationRegistry.getTargetContext(),
+                        testMedia11.getItem());
+                if (itemAlreadyInQueue) {
+                    Awaitility.await("download service receives the request - "
+                            + "no event is expected before cancel is issued")
+                            .atLeast(100, TimeUnit.MILLISECONDS)
+                            .until(() -> true);
+                } else {
+                    Awaitility.await("item enqueue event")
+                            .atMost(1000, TimeUnit.MILLISECONDS)
+                            .until(() -> feedItemEventListener.getEvents().size() >= 1);
+                }
+                DownloadRequester.getInstance().cancelDownload(InstrumentationRegistry.getTargetContext(),
+                        testMedia11);
+                final int totalNumEventsExpected = itemAlreadyInQueue ? 1 : 3;
+                Awaitility.await("item dequeue event + download termination event")
+                        .atMost(1000, TimeUnit.MILLISECONDS)
+                        .until(() ->feedItemEventListener.getEvents().size() >= totalNumEventsExpected);
+                assertFalse("The download should have been canceled",
+                        DBReader.getFeedMedia(testMedia11.getId()).isDownloaded());
+                if (itemAlreadyInQueue) {
+                    assertTrue("The FeedItem should still be in the queue after the download is cancelled."
+                                    + " It's there before download.",
+                            DBReader.getQueueIDList().contains(item1Id));
+                } else {
+                    assertFalse("The FeedItem should not be in the queue after the download is cancelled.",
+                            DBReader.getQueueIDList().contains(item1Id));
+                }
             } 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());

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

@@ -2,7 +2,6 @@ package de.danoeh.antennapod.adapter;
 
 import android.content.Context;
 import android.os.Build;
-import androidx.core.content.ContextCompat;
 import android.text.Layout;
 import android.text.format.DateUtils;
 import android.util.Log;
@@ -13,6 +12,8 @@ import android.widget.BaseAdapter;
 import android.widget.TextView;
 import android.widget.Toast;
 
+import androidx.core.content.ContextCompat;
+
 import com.joanzapata.iconify.widget.IconButton;
 import com.joanzapata.iconify.widget.IconTextView;
 
@@ -24,6 +25,7 @@ import de.danoeh.antennapod.core.service.download.DownloadStatus;
 import de.danoeh.antennapod.core.storage.DBReader;
 import de.danoeh.antennapod.core.storage.DBTasks;
 import de.danoeh.antennapod.core.storage.DownloadRequestException;
+import de.danoeh.antennapod.core.storage.DownloadRequester;
 
 /** Displays a list of DownloadStatus entries. */
 public class DownloadLogAdapter extends BaseAdapter {
@@ -132,7 +134,7 @@ public class DownloadLogAdapter extends BaseAdapter {
 				FeedMedia media = DBReader.getFeedMedia(holder.id);
 				if (media != null) {
 					try {
-						DBTasks.downloadFeedItems(context, media.getItem());
+						DownloadRequester.getInstance().downloadMedia(context, media.getItem());
 						Toast.makeText(context, R.string.status_downloading_label, Toast.LENGTH_SHORT).show();
 					} catch (DownloadRequestException e) {
 						e.printStackTrace();

app/src/main/java/de/danoeh/antennapod/adapter/actionbutton/DownloadActionButton.java

@@ -1,16 +1,16 @@
 package de.danoeh.antennapod.adapter.actionbutton;
 
 import android.content.Context;
+import android.widget.Toast;
+
 import androidx.annotation.AttrRes;
 import androidx.annotation.NonNull;
 import androidx.annotation.StringRes;
-import android.widget.Toast;
 
 import de.danoeh.antennapod.R;
 import de.danoeh.antennapod.core.dialog.DownloadRequestErrorDialogCreator;
 import de.danoeh.antennapod.core.feed.FeedItem;
 import de.danoeh.antennapod.core.feed.FeedMedia;
-import de.danoeh.antennapod.core.storage.DBTasks;
 import de.danoeh.antennapod.core.storage.DBWriter;
 import de.danoeh.antennapod.core.storage.DownloadRequestException;
 import de.danoeh.antennapod.core.storage.DownloadRequester;
@@ -64,7 +64,7 @@ class DownloadActionButton extends ItemActionButton {
 
     private void downloadEpisode(Context context) {
         try {
-            DBTasks.downloadFeedItems(context, item);
+            DownloadRequester.getInstance().downloadMedia(context, item);
             Toast.makeText(context, R.string.status_downloading_label, Toast.LENGTH_SHORT).show();
         } catch (DownloadRequestException e) {
             e.printStackTrace();

app/src/main/java/de/danoeh/antennapod/adapter/actionbutton/MobileDownloadHelper.java

@@ -8,9 +8,9 @@ import de.danoeh.antennapod.R;
 import de.danoeh.antennapod.core.dialog.DownloadRequestErrorDialogCreator;
 import de.danoeh.antennapod.core.feed.FeedItem;
 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.DownloadRequestException;
+import de.danoeh.antennapod.core.storage.DownloadRequester;
 
 class MobileDownloadHelper {
     private static long addToQueueTimestamp;
@@ -48,7 +48,7 @@ class MobileDownloadHelper {
     private static void downloadFeedItems(Context context, FeedItem item) {
         allowMobileDownloadTimestamp = System.currentTimeMillis();
         try {
-            DBTasks.downloadFeedItems(context, item);
+            DownloadRequester.getInstance().downloadMedia(context, item);
             Toast.makeText(context, R.string.status_downloading_label, Toast.LENGTH_SHORT).show();
         } catch (DownloadRequestException e) {
             e.printStackTrace();

app/src/main/java/de/danoeh/antennapod/dialog/EpisodesApplyActionFragment.java

@@ -36,9 +36,9 @@ import java.util.Map;
 import de.danoeh.antennapod.R;
 import de.danoeh.antennapod.core.dialog.DownloadRequestErrorDialogCreator;
 import de.danoeh.antennapod.core.feed.FeedItem;
-import de.danoeh.antennapod.core.storage.DBTasks;
 import de.danoeh.antennapod.core.storage.DBWriter;
 import de.danoeh.antennapod.core.storage.DownloadRequestException;
+import de.danoeh.antennapod.core.storage.DownloadRequester;
 import de.danoeh.antennapod.core.util.FeedItemPermutors;
 import de.danoeh.antennapod.core.util.LongList;
 import de.danoeh.antennapod.core.util.SortOrder;
@@ -485,7 +485,7 @@ public class EpisodesApplyActionFragment extends Fragment {
             }
         }
         try {
-            DBTasks.downloadFeedItems(getActivity(), toDownload.toArray(new FeedItem[toDownload.size()]));
+            DownloadRequester.getInstance().downloadMedia(getActivity(), toDownload.toArray(new FeedItem[toDownload.size()]));
         } catch (DownloadRequestException e) {
             e.printStackTrace();
             DownloadRequestErrorDialogCreator.newRequestErrorDialog(getActivity(), e.getMessage());

core/src/androidTest/java/de/danoeh/antennapod/core/service/download/DownloadRequestTest.java

@@ -0,0 +1,122 @@
+package de.danoeh.antennapod.core.service.download;
+
+import android.os.Bundle;
+import android.os.Parcel;
+
+import androidx.test.filters.SmallTest;
+
+import org.junit.Test;
+
+import java.util.ArrayList;
+
+import de.danoeh.antennapod.core.feed.FeedFile;
+
+import static org.junit.Assert.assertEquals;
+
+@SmallTest
+public class DownloadRequestTest {
+
+    @Test
+    public void parcelInArrayListTest_WithAuth() {
+        doTestParcelInArrayList("case has authentication",
+                "usr1", "pass1", "usr2", "pass2");
+    }
+
+    @Test
+    public void parcelInArrayListTest_NoAuth() {
+        doTestParcelInArrayList("case no authentication",
+                null, null, null, null);
+    }
+
+    @Test
+    public void parcelInArrayListTest_MixAuth() {
+        doTestParcelInArrayList("case mixed authentication",
+                null, null, "usr2", "pass2");
+    }
+
+    // Test to ensure parcel using put/getParcelableArrayList() API work
+    // based on: https://stackoverflow.com/a/13507191
+    private void doTestParcelInArrayList(String message,
+                                         String username1, String password1,
+                                         String username2, String password2) {
+        ArrayList<DownloadRequest> toParcel;
+        { // test DownloadRequests to parcel
+            String destStr = "file://location/media.mp3";
+            FeedFile item1 = createFeedItem(1);
+            Bundle arg1 = new Bundle();
+            arg1.putString("arg1", "value1");
+            DownloadRequest request1 = new DownloadRequest.Builder(destStr, item1)
+                    .withAuthentication(username1, password1)
+                    .withArguments(arg1)
+                    .build();
+
+            FeedFile item2 = createFeedItem(2);
+            DownloadRequest request2 = new DownloadRequest.Builder(destStr, item2)
+                    .withAuthentication(username2, password2)
+                    .build();
+
+            toParcel = new ArrayList<>();
+            toParcel.add(request1);
+            toParcel.add(request2);
+        }
+
+        // parcel the download requests
+        Bundle bundleIn = new Bundle();
+        bundleIn.putParcelableArrayList("r", toParcel);
+
+        Parcel parcel = Parcel.obtain();
+        bundleIn.writeToParcel(parcel, 0);
+
+        Bundle bundleOut = new Bundle();
+        bundleOut.setClassLoader(DownloadRequest.class.getClassLoader());
+        parcel.setDataPosition(0); // to read the parcel from the beginning.
+        bundleOut.readFromParcel(parcel);
+
+        ArrayList<DownloadRequest> fromParcel = bundleOut.getParcelableArrayList("r");
+
+        // spot-check contents to ensure they are the same
+        // DownloadRequest.equals() implementation doesn't quite work
+        // for DownloadRequest.argument (a Bundle)
+        assertEquals(message + " - size", toParcel.size(), fromParcel.size());
+        assertEquals(message + " - source", toParcel.get(1).getSource(), fromParcel.get(1).getSource());
+        assertEquals(message + " - password", toParcel.get(0).getPassword(), fromParcel.get(0).getPassword());
+        assertEquals(message + " - argument", toString(toParcel.get(0).getArguments()),
+                toString(fromParcel.get(0).getArguments()));
+    }
+
+    private static String toString(Bundle b) {
+        StringBuilder sb = new StringBuilder();
+        sb.append("{");
+        for (String key: b.keySet()) {
+            Object val = b.get(key);
+            sb.append("(").append(key).append(":").append(val).append(") ");
+        }
+        sb.append("}");
+        return sb.toString();
+    }
+
+    private FeedFile createFeedItem(final int id) {
+        // Use mockito would be less verbose, but it'll take extra 1 second for this tiny test
+        return new FeedFile() {
+            @Override
+            public long getId() {
+                return id;
+            }
+
+            @Override
+            public String getDownload_url() {
+                return "http://example.com/episode" + id;
+            }
+
+            @Override
+            public int getTypeAsInt() {
+                return 0;
+            }
+
+            @Override
+            public String getHumanReadableIdentifier() {
+                return "human-id-" + id;
+            }
+        };
+    }
+}

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

@@ -544,6 +544,11 @@ public class UserPreferences {
         return prefs.getBoolean(PREF_ENABLE_AUTODL, false);
     }
 
+    @VisibleForTesting
+    public static void setEnableAutodownload(boolean enabled) {
+        prefs.edit().putBoolean(PREF_ENABLE_AUTODL, enabled).apply();
+    }
+
     public static boolean isEnableAutodownloadOnBattery() {
         return prefs.getBoolean(PREF_ENABLE_AUTODL_ON_BATTERY, true);
     }

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

@@ -3,6 +3,8 @@ package de.danoeh.antennapod.core.service.download;
 import android.os.Bundle;
 import android.os.Parcel;
 import android.os.Parcelable;
+import android.text.TextUtils;
+
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
@@ -26,6 +28,7 @@ public class DownloadRequest implements Parcelable {
     private long soFar;
     private long size;
     private int statusMsg;
+    private boolean mediaEnqueued;
 
     public DownloadRequest(@NonNull String destination,
                            @NonNull String source,
@@ -45,6 +48,7 @@ public class DownloadRequest implements Parcelable {
         this.username = username;
         this.password = password;
         this.deleteOnFailure = deleteOnFailure;
+        this.mediaEnqueued = false;
         this.arguments = (arguments != null) ? arguments : new Bundle();
     }
 
@@ -74,17 +78,10 @@ public class DownloadRequest implements Parcelable {
         feedfileType = in.readInt();
         lastModified = in.readString();
         deleteOnFailure = (in.readByte() > 0);
+        username = nullIfEmpty(in.readString());
+        password = nullIfEmpty(in.readString());
+        mediaEnqueued = (in.readByte() > 0);
         arguments = in.readBundle();
-        if (in.dataAvail() > 0) {
-            username = in.readString();
-        } else {
-            username = null;
-        }
-        if (in.dataAvail() > 0) {
-            password = in.readString();
-        } else {
-            password = null;
-        }
     }
 
     @Override
@@ -101,13 +98,23 @@ public class DownloadRequest implements Parcelable {
         dest.writeInt(feedfileType);
         dest.writeString(lastModified);
         dest.writeByte((deleteOnFailure) ? (byte) 1 : 0);
+        // in case of null username/password, still write an empty string
+        // (rather than skipping it). Otherwise, unmarshalling  a collection
+        // of them from a Parcel (from an Intent extra to submit a request to DownloadService) will fail.
+        //
+        // see: https://stackoverflow.com/a/22926342
+        dest.writeString(nonNullString(username));
+        dest.writeString(nonNullString(password));
+        dest.writeByte((mediaEnqueued) ? (byte) 1 : 0);
         dest.writeBundle(arguments);
-        if (username != null) {
+    }
-            dest.writeString(username);
+
-        }
+    private static String nonNullString(String str) {
-        if (password != null) {
+        return str != null ? str : "";
-            dest.writeString(password);
+    }
-        }
+
+    private static String nullIfEmpty(String str) {
+        return TextUtils.isEmpty(str) ? null : str;
     }
 
     public static final Parcelable.Creator<DownloadRequest> CREATOR = new Parcelable.Creator<DownloadRequest>() {
@@ -145,6 +152,7 @@ public class DownloadRequest implements Parcelable {
         if (title != null ? !title.equals(that.title) : that.title != null) return false;
         if (username != null ? !username.equals(that.username) : that.username != null)
             return false;
+        if (mediaEnqueued != that.mediaEnqueued) return false;
         return true;
     }
 
@@ -164,6 +172,7 @@ public class DownloadRequest implements Parcelable {
         result = 31 * result + (int) (soFar ^ (soFar >>> 32));
         result = 31 * result + (int) (size ^ (size >>> 32));
         result = 31 * result + statusMsg;
+        result = 31 * result + (mediaEnqueued ? 1 : 0);
         return result;
     }
 
@@ -245,6 +254,18 @@ public class DownloadRequest implements Parcelable {
         return deleteOnFailure;
     }
 
+    public boolean isMediaEnqueued() {
+        return mediaEnqueued;
+    }
+
+    /**
+     * Set to true if the media is enqueued because of this download.
+     * The state is helpful if the download is cancelled, and undoing the enqueue is needed.
+     */
+    public void setMediaEnqueued(boolean mediaEnqueued) {
+        this.mediaEnqueued = mediaEnqueued;
+    }
+
     public Bundle getArguments() {
         return arguments;
     }

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();
@@ -363,10 +371,20 @@ public class DownloadService extends Service {
                 Downloader d = getDownloader(url);
                 if (d != null) {
                     d.cancel();
-                    DownloadRequester.getInstance().removeDownload(d.getDownloadRequest());
+                    DownloadRequest request = d.getDownloadRequest();
+                    DownloadRequester.getInstance().removeDownload(request);
 
-                    FeedItem item = getFeedItemFromId(d.getDownloadRequest().getFeedfileId());
+                    FeedItem item = getFeedItemFromId(request.getFeedfileId());
                     if (item != null) {
+                        // undo enqueue upon cancel
+                        if (request.isMediaEnqueued()) {
+                            Log.v(TAG, "Undoing enqueue upon cancelling download");
+                            try {
+                                DBWriter.removeQueueItem(getApplicationContext(), false, item).get();
+                            } catch (Throwable t) {
+                                Log.e(TAG, "Unexpected exception during undoing enqueue upon cancel", t);
+                            }
+                        }
                         EventBus.getDefault().post(FeedItemEvent.updated(item));
                     }
                 } else {
@@ -387,13 +405,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;
+        }
+
+        for (DownloadRequest request : requests) {
+            onDownloadQueued(request, itemsEnqueued);
+        }
+    }
+
+    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,
+                                  @NonNull List<? extends FeedItem> itemsEnqueued) {
         writeFileUrl(request);
 
         Downloader downloader = downloaderFactory.create(request);
@@ -403,6 +463,9 @@ public class DownloadService extends Service {
             if (request.getFeedfileType() == Feed.FEEDFILETYPE_FEED) {
                 downloads.add(0, downloader);
             } else {
+                if (isEnqueued(request, itemsEnqueued)) {
+                    request.setMediaEnqueued(true);
+                }
                 downloads.add(downloader);
             }
             downloadExecutor.submit(downloader);
@@ -413,6 +476,19 @@ public class DownloadService extends Service {
         queryDownloads();
     }
 
+    private static boolean isEnqueued(@NonNull DownloadRequest request,
+                                      @NonNull List<? extends FeedItem> itemsEnqueued) {
+        if (request.getFeedfileType() == FeedMedia.FEEDFILETYPE_FEEDMEDIA) {
+            final long mediaId = request.getFeedfileId();
+            for (FeedItem item : itemsEnqueued) {
+                if (item.getMedia() != null && item.getMedia().getId() == mediaId) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
     @VisibleForTesting
     public static DownloaderFactory getDownloaderFactory() {
         return downloaderFactory;

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();
                 }

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,70 +306,9 @@ public final class DBTasks {
         EventBus.getDefault().post(new FeedListUpdateEvent(media.getItem().getFeed()));
     }
 
-    /**
+    @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
-     * 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
     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

@@ -8,21 +8,28 @@ 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.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 +71,56 @@ 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
+     * @param requests The list of DownloadRequest objects. If another DownloadRequest
-     *                call will return false.
+     *                 with the same source URL is already stored, this one will be skipped.
-     * @return True if 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,
-                                         @NonNull DownloadRequest request) {
+                                         DownloadRequest... requests) {
-        if (downloads.containsKey(request.getSource())) {
+        return download(context, false, requests);
-            if (BuildConfig.DEBUG) Log.i(TAG, "DownloadRequest is already stored.");
-            return false;
-        }
-        downloads.put(request.getSource(), request);
-
-        Intent launchIntent = new Intent(context, DownloadService.class);
-        launchIntent.putExtra(DownloadService.EXTRA_REQUEST, request);
-        ContextCompat.startForegroundService(context, launchIntent);
-
-        return true;
     }
 
-    private void download(Context context, FeedFile item, FeedFile container, File dest,
+    private boolean download(@NonNull Context context, boolean cleanupMedia,
-                          boolean overwriteIfExists, String username, String password,
+                             DownloadRequest... requests) {
-                          String lastModified, boolean deleteOnFailure, Bundle arguments) {
+        boolean result = false;
+
+        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.");
+                continue;
+            }
+            downloads.put(request.getSource(), request);
+
+            requestsToSend.add(request);
+            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;
+    }
+
+    @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()
+            Log.e(TAG, "URL " + item.getDownload_url()
-                        + " is already being downloaded");
+                    + " is already being downloaded");
-            return;
+            return null;
         }
         if (!isFilenameAvailable(dest.toString()) || (!partiallyDownloadedFileExists && dest.exists())) {
             Log.d(TAG, "Filename already used.");
@@ -136,8 +159,7 @@ public class DownloadRequester implements DownloadStateProvider {
                 .lastModified(lastModified)
                 .deleteOnFailure(deleteOnFailure)
                 .withArguments(arguments);
-        DownloadRequest request = builder.build();
+        return builder.build();
-        download(context, request);
     }
 
     /**
@@ -178,8 +200,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 +210,72 @@ 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)) {
+        downloadMedia(true, context, feedItems);
-            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());
+    @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
-            } else {
+    public synchronized void downloadMedia(boolean performAutoCleanup, @NonNull Context context,
-                dest = new File(getMediafilePath(feedmedia), getMediafilename(feedmedia));
+                                    FeedItem... items)
+            throws DownloadRequestException {
+        Log.d(TAG, "downloadMedia() called with: performAutoCleanup = [" + performAutoCleanup
+                + "], #items = [" + items.length + "]");
+
+        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, performAutoCleanup, 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);
     }
 
     /**