diff options
5 files changed, 114 insertions, 59 deletions
diff --git a/app/src/androidTest/java/de/test/antennapod/service/download/DownloadServiceTest.java b/app/src/androidTest/java/de/test/antennapod/service/download/DownloadServiceTest.java index af87ce8f4..93723bc78 100644 --- a/app/src/androidTest/java/de/test/antennapod/service/download/DownloadServiceTest.java +++ b/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()); diff --git a/app/src/androidTest/java/de/test/antennapod/storage/DBTasksTest.java b/app/src/androidTest/java/de/test/antennapod/storage/DBTasksTest.java index cce4e5111..090cd2213 100644 --- a/app/src/androidTest/java/de/test/antennapod/storage/DBTasksTest.java +++ b/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()); diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java index 95f78366f..d9e898a88 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadService.java +++ b/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"); - DownloadRequest request = intent.getParcelableExtra(EXTRA_REQUEST); - if (request == null) { + List<DownloadRequest> requests = intent.getParcelableArrayListExtra(EXTRA_REQUESTS); + if (requests == 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); 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 4237b2175..826fe48b5 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 @@ -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()) { 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 d36592ec8..ea3724adc 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 @@ -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, - * this one will be skipped. - * @return True if the any of 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 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); - launchIntent.putExtra(DownloadService.EXTRA_REQUEST, request); - ContextCompat.startForegroundService(context, launchIntent); + 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; } @@ -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 |