diff options
author | ByteHamster <ByteHamster@users.noreply.github.com> | 2022-01-06 14:36:11 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-01-06 14:36:11 +0100 |
commit | 849bf4ad854973d39698613c3a356c09c87e687c (patch) | |
tree | fbd07d17fe5f26b8c91f9ce4ff8020046070431f /core/src | |
parent | 8252eb2183c6a478fdfd9317586c8800a24054ca (diff) | |
download | AntennaPod-849bf4ad854973d39698613c3a356c09c87e687c.zip |
Rewrite download request creation (#5530)
Android has a limit on the size of Intent parameters. When enqueuing
a huge number of items, it just ignored the argument and did not call
onNewIntent. We now load the list over in DownloadService.
Diffstat (limited to 'core/src')
22 files changed, 521 insertions, 1027 deletions
diff --git a/core/src/main/java/de/danoeh/antennapod/core/backup/OpmlBackupAgent.java b/core/src/main/java/de/danoeh/antennapod/core/backup/OpmlBackupAgent.java index fca83351c..a7337f288 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/backup/OpmlBackupAgent.java +++ b/core/src/main/java/de/danoeh/antennapod/core/backup/OpmlBackupAgent.java @@ -8,6 +8,9 @@ import android.content.Context; import android.os.ParcelFileDescriptor; import android.util.Log; +import de.danoeh.antennapod.core.service.download.DownloadRequest; +import de.danoeh.antennapod.core.service.download.DownloadService; +import de.danoeh.antennapod.core.service.download.DownloadRequestCreator; import org.apache.commons.io.IOUtils; import org.xmlpull.v1.XmlPullParserException; @@ -33,8 +36,6 @@ import de.danoeh.antennapod.core.export.opml.OpmlReader; import de.danoeh.antennapod.core.export.opml.OpmlWriter; import de.danoeh.antennapod.model.feed.Feed; import de.danoeh.antennapod.core.storage.DBReader; -import de.danoeh.antennapod.core.storage.DownloadRequestException; -import de.danoeh.antennapod.core.storage.DownloadRequester; public class OpmlBackupAgent extends BackupAgentHelper { private static final String OPML_BACKUP_KEY = "opml"; @@ -146,16 +147,10 @@ public class OpmlBackupAgent extends BackupAgentHelper { try { ArrayList<OpmlElement> opmlElements = new OpmlReader().readDocument(reader); mChecksum = digester == null ? null : digester.digest(); - DownloadRequester downloader = DownloadRequester.getInstance(); - for (OpmlElement opmlElem : opmlElements) { Feed feed = new Feed(opmlElem.getXmlUrl(), null, opmlElem.getText()); - - try { - downloader.downloadFeed(mContext, feed); - } catch (DownloadRequestException e) { - Log.d(TAG, "Error while restoring/downloading feed", e); - } + DownloadRequest request = DownloadRequestCreator.create(feed).build(); + DownloadService.download(mContext, false, request); } } catch (XmlPullParserException e) { Log.e(TAG, "Error while parsing the OPML file", e); diff --git a/core/src/main/java/de/danoeh/antennapod/core/dialog/DownloadRequestErrorDialogCreator.java b/core/src/main/java/de/danoeh/antennapod/core/dialog/DownloadRequestErrorDialogCreator.java deleted file mode 100644 index e38abb990..000000000 --- a/core/src/main/java/de/danoeh/antennapod/core/dialog/DownloadRequestErrorDialogCreator.java +++ /dev/null @@ -1,24 +0,0 @@ -package de.danoeh.antennapod.core.dialog; - -import android.content.Context; -import androidx.appcompat.app.AlertDialog; - -import de.danoeh.antennapod.core.R; - -/** Creates Alert Dialogs if a DownloadRequestException has happened. */ -public class DownloadRequestErrorDialogCreator { - private DownloadRequestErrorDialogCreator() { - } - - public static void newRequestErrorDialog(Context context, - String errorMessage) { - AlertDialog.Builder builder = new AlertDialog.Builder(context); - builder.setNeutralButton(android.R.string.ok, - (dialog, which) -> dialog.dismiss()) - .setTitle(R.string.download_error_request_error) - .setMessage( - context.getString(R.string.download_request_error_dialog_message_prefix) - + errorMessage); - builder.create().show(); - } -} diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadRequest.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadRequest.java index 079abef1b..5dfbdf85a 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadRequest.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadRequest.java @@ -12,6 +12,8 @@ import de.danoeh.antennapod.model.feed.FeedFile; import de.danoeh.antennapod.core.util.URLChecker; public class DownloadRequest implements Parcelable { + public static final String REQUEST_ARG_PAGE_NR = "page"; + public static final String REQUEST_ARG_LOAD_ALL_PAGES = "loadAllPages"; private final String destination; private final String source; @@ -128,7 +130,6 @@ public class DownloadRequest implements Parcelable { if (size != that.size) return false; if (soFar != that.soFar) return false; if (statusMsg != that.statusMsg) return false; - if (!arguments.equals(that.arguments)) return false; if (!destination.equals(that.destination)) return false; if (password != null ? !password.equals(that.password) : that.password != null) return false; @@ -269,18 +270,27 @@ public class DownloadRequest implements Parcelable { private boolean deleteOnFailure = false; private final long feedfileId; private final int feedfileType; - private Bundle arguments; - private boolean initiatedByUser; + private Bundle arguments = new Bundle(); + private boolean initiatedByUser = true; - public Builder(@NonNull String destination, @NonNull FeedFile item, boolean initiatedByUser) { + public Builder(@NonNull String destination, @NonNull FeedFile item) { this.destination = destination; this.source = URLChecker.prepareURL(item.getDownload_url()); this.title = item.getHumanReadableIdentifier(); this.feedfileId = item.getId(); this.feedfileType = item.getTypeAsInt(); + } + + public void setInitiatedByUser(boolean initiatedByUser) { this.initiatedByUser = initiatedByUser; } + public void setForce(boolean force) { + if (force) { + lastModified = null; + } + } + public Builder deleteOnFailure(boolean deleteOnFailure) { this.deleteOnFailure = deleteOnFailure; return this; @@ -297,14 +307,14 @@ public class DownloadRequest implements Parcelable { return this; } - public DownloadRequest build() { - return new DownloadRequest(this); + public void loadAllPages(boolean loadAllPages) { + if (loadAllPages) { + arguments.putBoolean(REQUEST_ARG_LOAD_ALL_PAGES, true); + } } - public Builder withArguments(Bundle args) { - this.arguments = args; - return this; + public DownloadRequest build() { + return new DownloadRequest(this); } - } } diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadRequestCreator.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadRequestCreator.java new file mode 100644 index 000000000..80ce4620a --- /dev/null +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadRequestCreator.java @@ -0,0 +1,139 @@ +package de.danoeh.antennapod.core.service.download; + +import android.util.Log; +import android.webkit.URLUtil; +import de.danoeh.antennapod.core.preferences.UserPreferences; +import de.danoeh.antennapod.core.util.FileNameGenerator; +import de.danoeh.antennapod.model.feed.Feed; +import de.danoeh.antennapod.model.feed.FeedMedia; +import org.apache.commons.io.FilenameUtils; + +import java.io.File; + +/** + * Creates download requests that can be sent to the DownloadService. + */ +public class DownloadRequestCreator { + private static final String TAG = "DownloadRequester"; + private static final String FEED_DOWNLOADPATH = "cache/"; + private static final String MEDIA_DOWNLOADPATH = "media/"; + + public static DownloadRequest.Builder create(Feed feed) { + File dest = new File(getFeedfilePath(), getFeedfileName(feed)); + if (!isFilenameAvailable(dest.toString())) { + dest = findUnusedFile(dest); + } + Log.d(TAG, "Requesting download of url " + feed.getDownload_url()); + + String username = (feed.getPreferences() != null) ? feed.getPreferences().getUsername() : null; + String password = (feed.getPreferences() != null) ? feed.getPreferences().getPassword() : null; + + return new DownloadRequest.Builder(dest.toString(), feed) + .withAuthentication(username, password) + .deleteOnFailure(true) + .lastModified(feed.getLastUpdate()); + } + + public static DownloadRequest.Builder create(FeedMedia media) { + final boolean partiallyDownloadedFileExists = + media.getFile_url() != null && new File(media.getFile_url()).exists(); + File dest; + if (media.getFile_url() != null && new File(media.getFile_url()).exists()) { + dest = new File(media.getFile_url()); + } else { + dest = new File(getMediafilePath(media), getMediafilename(media)); + } + + if (!isFilenameAvailable(dest.toString()) || (!partiallyDownloadedFileExists && dest.exists())) { + dest = findUnusedFile(dest); + } + Log.d(TAG, "Requesting download of url " + media.getDownload_url()); + + String username = (media.getItem().getFeed().getPreferences() != null) + ? media.getItem().getFeed().getPreferences().getUsername() : null; + String password = (media.getItem().getFeed().getPreferences() != null) + ? media.getItem().getFeed().getPreferences().getPassword() : null; + + return new DownloadRequest.Builder(dest.toString(), media) + .deleteOnFailure(false) + .withAuthentication(username, password); + } + + private static File findUnusedFile(File dest) { + // find different name + File newDest = null; + for (int i = 1; i < Integer.MAX_VALUE; i++) { + String newName = FilenameUtils.getBaseName(dest + .getName()) + + "-" + + i + + FilenameUtils.EXTENSION_SEPARATOR + + FilenameUtils.getExtension(dest.getName()); + Log.d(TAG, "Testing filename " + newName); + newDest = new File(dest.getParent(), newName); + if (!newDest.exists() && isFilenameAvailable(newDest.toString())) { + Log.d(TAG, "File doesn't exist yet. Using " + newName); + break; + } + } + return newDest; + } + + /** + * Returns true if a filename is available and false if it has already been + * taken by another requested download. + */ + private static boolean isFilenameAvailable(String path) { + for (Downloader downloader : DownloadService.downloads) { + if (downloader.request.getDestination().equals(path)) { + return false; + } + } + return true; + } + + private static String getFeedfilePath() { + return UserPreferences.getDataFolder(FEED_DOWNLOADPATH).toString() + "/"; + } + + private static String getFeedfileName(Feed feed) { + String filename = feed.getDownload_url(); + if (feed.getTitle() != null && !feed.getTitle().isEmpty()) { + filename = feed.getTitle(); + } + return "feed-" + FileNameGenerator.generateFileName(filename); + } + + private static String getMediafilePath(FeedMedia media) { + String mediaPath = MEDIA_DOWNLOADPATH + + FileNameGenerator.generateFileName(media.getItem().getFeed().getTitle()); + return UserPreferences.getDataFolder(mediaPath).toString() + "/"; + } + + private static String getMediafilename(FeedMedia media) { + String filename; + String titleBaseFilename = ""; + + // Try to generate the filename by the item title + if (media.getItem() != null && media.getItem().getTitle() != null) { + String title = media.getItem().getTitle(); + titleBaseFilename = FileNameGenerator.generateFileName(title); + } + + String urlBaseFilename = URLUtil.guessFileName(media.getDownload_url(), null, media.getMime_type()); + + if (!titleBaseFilename.equals("")) { + // Append extension + final int filenameMaxLength = 220; + if (titleBaseFilename.length() > filenameMaxLength) { + titleBaseFilename = titleBaseFilename.substring(0, filenameMaxLength); + } + filename = titleBaseFilename + FilenameUtils.EXTENSION_SEPARATOR + + FilenameUtils.getExtension(urlBaseFilename); + } else { + // Fall back on URL file name + filename = urlBaseFilename; + } + return filename; + } +} 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 6cf6ce107..9fb624bc2 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 @@ -9,9 +9,7 @@ import android.content.Intent; import android.content.IntentFilter; import android.os.Binder; import android.os.Build; -import android.os.Handler; import android.os.IBinder; -import android.os.Looper; import android.text.TextUtils; import android.util.Log; @@ -20,6 +18,7 @@ import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import androidx.core.app.ServiceCompat; +import androidx.core.content.ContextCompat; import de.danoeh.antennapod.core.R; import org.apache.commons.io.FileUtils; import org.greenrobot.eventbus.EventBus; @@ -29,15 +28,13 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Collections; import java.util.List; -import java.util.concurrent.CompletionService; +import java.util.concurrent.CopyOnWriteArrayList; 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.event.DownloadEvent; import de.danoeh.antennapod.core.util.download.ConnectionStateMonitor; @@ -53,7 +50,6 @@ import de.danoeh.antennapod.core.service.download.handler.PostDownloaderTask; 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.DownloadRequester; import de.danoeh.antennapod.core.util.DownloadError; /** @@ -65,65 +61,34 @@ import de.danoeh.antennapod.core.util.DownloadError; */ public class DownloadService extends Service { private static final String TAG = "DownloadService"; - - /** - * Cancels one download. The intent MUST have an EXTRA_DOWNLOAD_URL extra that contains the download URL of the - * object whose download should be cancelled. - */ + private static final int SCHED_EX_POOL_SIZE = 1; public static final String ACTION_CANCEL_DOWNLOAD = "action.de.danoeh.antennapod.core.service.cancelDownload"; - - /** - * Cancels all running downloads. - */ - public static final String ACTION_CANCEL_ALL_DOWNLOADS = "action.de.danoeh.antennapod.core.service.cancelAllDownloads"; - - /** - * Extra for ACTION_CANCEL_DOWNLOAD - */ + public static final String ACTION_CANCEL_ALL_DOWNLOADS = "action.de.danoeh.antennapod.core.service.cancelAll"; public static final String EXTRA_DOWNLOAD_URL = "downloadUrl"; - - /** - * Extra for ACTION_ENQUEUE_DOWNLOAD intent. - */ public static final String EXTRA_REQUESTS = "downloadRequests"; - + public static final String EXTRA_REFRESH_ALL = "refreshAll"; + public static final String EXTRA_INITIATED_BY_USER = "initiatedByUser"; public static final String EXTRA_CLEANUP_MEDIA = "cleanupMedia"; - /** - * Contains all completed downloads that have not been included in the report yet. - */ - private final List<DownloadStatus> reportQueue; - private final ExecutorService syncExecutor; - private final CompletionService<Downloader> downloadExecutor; - private final DownloadRequester requester; - private DownloadServiceNotification notificationManager; - private final NewEpisodesNotification newEpisodesNotification; - - /** - * Currently running downloads. - */ - private final List<Downloader> downloads; - - /** - * Number of running downloads. - */ - private AtomicInteger numberOfDownloads; - - /** - * True if service is running. - */ public static boolean isRunning = false; - private Handler handler; + // Can be modified from another thread while iterating. Both possible race conditions are not critical: + // Remove while iterating: We think it is still downloading and don't start a new download with the same file. + // Add while iterating: We think it is not downloading and might start a second download with the same file. + static final List<Downloader> downloads = Collections.synchronizedList(new CopyOnWriteArrayList<>()); + private final ExecutorService downloadHandleExecutor; + private final ExecutorService downloadEnqueueExecutor; + private final List<DownloadStatus> reportQueue = new ArrayList<>(); + private DownloadServiceNotification notificationManager; + private final NewEpisodesNotification newEpisodesNotification; private NotificationUpdater notificationUpdater; private ScheduledFuture<?> notificationUpdaterFuture; private ScheduledFuture<?> downloadPostFuture; - private static final int SCHED_EX_POOL_SIZE = 1; - private final ScheduledThreadPoolExecutor schedExecutor; + private final ScheduledThreadPoolExecutor notificationUpdateExecutor; private static DownloaderFactory downloaderFactory = new DefaultDownloaderFactory(); + private final IBinder binder = new LocalBinder(); private ConnectionStateMonitor connectionMonitor; - private final IBinder mBinder = new LocalBinder(); private class LocalBinder extends Binder { public DownloadService getService() { @@ -131,34 +96,32 @@ public class DownloadService extends Service { } } + @Override + public IBinder onBind(Intent intent) { + return binder; + } + public DownloadService() { - reportQueue = Collections.synchronizedList(new ArrayList<>()); - downloads = Collections.synchronizedList(new ArrayList<>()); - numberOfDownloads = new AtomicInteger(0); - requester = DownloadRequester.getInstance(); newEpisodesNotification = new NewEpisodesNotification(); - syncExecutor = Executors.newSingleThreadExecutor(r -> { - Thread t = new Thread(r, "SyncThread"); + downloadEnqueueExecutor = Executors.newSingleThreadExecutor(r -> { + Thread t = new Thread(r, "EnqueueThread"); t.setPriority(Thread.MIN_PRIORITY); return t; }); // Must be the first runnable in syncExecutor - syncExecutor.execute(newEpisodesNotification::loadCountersBeforeRefresh); + downloadEnqueueExecutor.execute(newEpisodesNotification::loadCountersBeforeRefresh); Log.d(TAG, "parallel downloads: " + UserPreferences.getParallelDownloads()); - downloadExecutor = new ExecutorCompletionService<>( - Executors.newFixedThreadPool(UserPreferences.getParallelDownloads(), - r -> { - Thread t = new Thread(r, "DownloadThread"); - t.setPriority(Thread.MIN_PRIORITY); - return t; - } - ) - ); - schedExecutor = new ScheduledThreadPoolExecutor(SCHED_EX_POOL_SIZE, + downloadHandleExecutor = Executors.newFixedThreadPool(UserPreferences.getParallelDownloads(), + r -> { + Thread t = new Thread(r, "DownloadThread"); + t.setPriority(Thread.MIN_PRIORITY); + return t; + }); + notificationUpdateExecutor = new ScheduledThreadPoolExecutor(SCHED_EX_POOL_SIZE, r -> { - Thread t = new Thread(r, "DownloadSchedExecutorThread"); + Thread t = new Thread(r, "NotificationUpdateExecutor"); t.setPriority(Thread.MIN_PRIORITY); return t; }, (r, executor) -> Log.w(TAG, "SchedEx rejected submission of new task") @@ -166,26 +129,9 @@ public class DownloadService extends Service { } @Override - public int onStartCommand(Intent intent, int flags, int startId) { - if (intent != null && intent.getParcelableArrayListExtra(EXTRA_REQUESTS) != null) { - Notification notification = notificationManager.updateNotifications( - requester.getNumberOfDownloads(), downloads); - startForeground(R.id.notification_downloading, notification); - setupNotificationUpdaterIfNecessary(); - syncExecutor.execute(() -> onDownloadQueued(intent)); - } else if (numberOfDownloads.get() == 0) { - shutdown(); - } else { - Log.d(TAG, "onStartCommand: Unknown intent"); - } - return Service.START_NOT_STICKY; - } - - @Override public void onCreate() { Log.d(TAG, "Service started"); isRunning = true; - handler = new Handler(Looper.getMainLooper()); notificationManager = new DownloadServiceNotification(this); IntentFilter cancelDownloadReceiverFilter = new IntentFilter(); @@ -197,13 +143,106 @@ public class DownloadService extends Service { connectionMonitor = new ConnectionStateMonitor(); connectionMonitor.enable(getApplicationContext()); } + } - downloadCompletionThread.start(); + public static void download(Context context, boolean cleanupMedia, DownloadRequest... requests) { + if (requests.length > 100) { + throw new IllegalArgumentException("Android silently drops intent payloads that are too large"); + } + ArrayList<DownloadRequest> requestsToSend = new ArrayList<>(); + for (DownloadRequest request : requests) { + if (!isDownloadingFile(request.getSource())) { + requestsToSend.add(request); + } + } + if (requestsToSend.isEmpty()) { + return; + } + Intent launchIntent = new Intent(context, DownloadService.class); + launchIntent.putParcelableArrayListExtra(DownloadService.EXTRA_REQUESTS, requestsToSend); + if (cleanupMedia) { + launchIntent.putExtra(DownloadService.EXTRA_CLEANUP_MEDIA, true); + } + ContextCompat.startForegroundService(context, launchIntent); + } + + public static void refreshAllFeeds(Context context, boolean initiatedByUser) { + Intent launchIntent = new Intent(context, DownloadService.class); + launchIntent.putExtra(DownloadService.EXTRA_REFRESH_ALL, true); + launchIntent.putExtra(DownloadService.EXTRA_INITIATED_BY_USER, initiatedByUser); + ContextCompat.startForegroundService(context, launchIntent); + } + + public static void cancel(Context context, String url) { + if (!isRunning) { + return; + } + Intent cancelIntent = new Intent(DownloadService.ACTION_CANCEL_DOWNLOAD); + cancelIntent.putExtra(DownloadService.EXTRA_DOWNLOAD_URL, url); + cancelIntent.setPackage(context.getPackageName()); + context.sendBroadcast(cancelIntent); + } + + public static void cancelAll(Context context) { + if (!isRunning) { + return; + } + Intent cancelIntent = new Intent(DownloadService.ACTION_CANCEL_ALL_DOWNLOADS); + cancelIntent.setPackage(context.getPackageName()); + context.sendBroadcast(cancelIntent); + } + + public static boolean isDownloadingFeeds() { + if (!isRunning) { + return false; + } + for (Downloader downloader : downloads) { + if (downloader.request.getFeedfileType() == Feed.FEEDFILETYPE_FEED && !downloader.cancelled) { + return true; + } + } + return false; + } + + public static boolean isDownloadingFile(String downloadUrl) { + if (!isRunning) { + return false; + } + for (Downloader downloader : downloads) { + if (downloader.request.getSource().equals(downloadUrl) && !downloader.cancelled) { + return true; + } + } + return false; + } + + public static DownloadRequest findRequest(String downloadUrl) { + for (Downloader downloader : downloads) { + if (downloader.request.getSource().equals(downloadUrl)) { + return downloader.request; + } + } + return null; } @Override - public IBinder onBind(Intent intent) { - return mBinder; + public int onStartCommand(Intent intent, int flags, int startId) { + if (intent != null && intent.hasExtra(EXTRA_REQUESTS)) { + Notification notification = notificationManager.updateNotifications(downloads); + startForeground(R.id.notification_downloading, notification); + setupNotificationUpdaterIfNecessary(); + downloadEnqueueExecutor.execute(() -> onDownloadQueued(intent)); + } else if (intent != null && intent.getBooleanExtra(EXTRA_REFRESH_ALL, false)) { + Notification notification = notificationManager.updateNotifications(downloads); + startForeground(R.id.notification_downloading, notification); + setupNotificationUpdaterIfNecessary(); + downloadEnqueueExecutor.execute(() -> enqueueAll(intent)); + } else if (downloads.size() == 0) { + shutdown(); + } else { + Log.d(TAG, "onStartCommand: Unknown intent"); + } + return Service.START_NOT_STICKY; } @Override @@ -218,19 +257,14 @@ public class DownloadService extends Service { } EventBus.getDefault().postSticky(DownloadEvent.refresh(Collections.emptyList())); - - downloadCompletionThread.interrupt(); - try { - downloadCompletionThread.join(1000); - } catch (InterruptedException e) { - e.printStackTrace(); - } cancelNotificationUpdater(); - syncExecutor.shutdown(); - schedExecutor.shutdown(); + downloadHandleExecutor.shutdown(); + downloadEnqueueExecutor.shutdown(); + notificationUpdateExecutor.shutdown(); if (downloadPostFuture != null) { downloadPostFuture.cancel(true); } + downloads.clear(); unregisterReceiver(cancelDownloadReceiver); if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { connectionMonitor.disable(getApplicationContext()); @@ -240,41 +274,26 @@ public class DownloadService extends Service { DBTasks.autodownloadUndownloadedItems(getApplicationContext()); } - private final Thread downloadCompletionThread = new Thread("DownloadCompletionThread") { - private static final String TAG = "downloadCompletionThd"; - - @Override - public void run() { - Log.d(TAG, "downloadCompletionThread was started"); - while (!isInterrupted()) { - try { - Downloader downloader = downloadExecutor.take().get(); - Log.d(TAG, "Received 'Download Complete' - message."); - - if (downloader.getResult().isSuccessful()) { - syncExecutor.execute(() -> { - handleSuccessfulDownload(downloader); - removeDownload(downloader); - numberOfDownloads.decrementAndGet(); - stopServiceIfEverythingDoneAsync(); - }); - } else { - handleFailedDownload(downloader); - removeDownload(downloader); - numberOfDownloads.decrementAndGet(); - stopServiceIfEverythingDoneAsync(); - } - } catch (InterruptedException e) { - Log.e(TAG, "DownloadCompletionThread was interrupted"); - return; - } catch (ExecutionException e) { - Log.e(TAG, "ExecutionException in DownloadCompletionThread: " + e.getMessage()); - return; - } + private void performDownload(Downloader downloader) { + try { + downloader.call(); + } catch (Exception e) { + e.printStackTrace(); + } + try { + if (downloader.getResult().isSuccessful()) { + handleSuccessfulDownload(downloader); + } else { + handleFailedDownload(downloader); } - Log.d(TAG, "End of downloadCompletionThread"); + } catch (Exception e) { + e.printStackTrace(); } - }; + downloadEnqueueExecutor.submit(() -> { + downloads.remove(downloader); + stopServiceIfEverythingDone(); + }); + } private void handleSuccessfulDownload(Downloader downloader) { DownloadRequest request = downloader.getDownloadRequest(); @@ -287,12 +306,15 @@ public class DownloadService extends Service { boolean success = task.run(); if (success) { + if (request.getFeedfileId() == 0) { + return; // No download logs for new subscriptions + } // we create a 'successful' download log if the feed's last refresh failed List<DownloadStatus> log = DBReader.getFeedDownloadLog(request.getFeedfileId()); if (log.size() > 0 && !log.get(0).isSuccessful()) { saveDownloadStatus(task.getDownloadStatus()); } - if (request.getFeedfileId() != 0 && !request.isInitiatedByUser()) { + if (!request.isInitiatedByUser()) { // Was stored in the database before and not initiated manually newEpisodesNotification.showIfNeeded(DownloadService.this, task.getSavedFeed()); } @@ -320,11 +342,11 @@ public class DownloadService extends Service { Log.d(TAG, "Requested invalid range, restarting download from the beginning"); FileUtils.deleteQuietly(new File(downloader.getDownloadRequest().getDestination())); - DownloadRequester.getInstance().download(DownloadService.this, downloader.getDownloadRequest()); + download(this, false, downloader.getDownloadRequest()); } else { Log.e(TAG, "Download failed"); saveDownloadStatus(status); - syncExecutor.execute(new FailedDownloadHandler(downloader.getDownloadRequest())); + new FailedDownloadHandler(downloader.getDownloadRequest()).run(); if (type == FeedMedia.FEEDFILETYPE_FEEDMEDIA) { FeedItem item = getFeedItemFromId(status.getFeedfileId()); @@ -350,94 +372,77 @@ public class DownloadService extends Service { } } - private Downloader getDownloader(String downloadUrl) { - for (Downloader downloader : downloads) { - if (downloader.getDownloadRequest().getSource().equals(downloadUrl)) { - return downloader; - } - } - return null; - } - private final BroadcastReceiver cancelDownloadReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { + Log.d(TAG, "cancelDownloadReceiver: " + intent.getAction()); if (TextUtils.equals(intent.getAction(), ACTION_CANCEL_DOWNLOAD)) { String url = intent.getStringExtra(EXTRA_DOWNLOAD_URL); if (url == null) { throw new IllegalArgumentException("ACTION_CANCEL_DOWNLOAD intent needs download url extra"); } - - Log.d(TAG, "Cancelling download with url " + url); - Downloader d = getDownloader(url); - if (d != null) { - d.cancel(); - DownloadRequest request = d.getDownloadRequest(); - DownloadRequester.getInstance().removeDownload(request); - - 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 { - Log.e(TAG, "Could not cancel download with url " + url); - } - postDownloaders(); - + downloadEnqueueExecutor.execute(() -> { + doCancel(url); + postDownloaders(); + stopServiceIfEverythingDone(); + }); } else if (TextUtils.equals(intent.getAction(), ACTION_CANCEL_ALL_DOWNLOADS)) { - for (Downloader d : downloads) { - d.cancel(); + downloadEnqueueExecutor.execute(() -> { + for (Downloader d : downloads) { + d.cancel(); + } Log.d(TAG, "Cancelled all downloads"); - } - postDownloaders(); + postDownloaders(); + stopServiceIfEverythingDone(); + }); } - stopServiceIfEverythingDone(); } - }; + private void doCancel(String url) { + Log.d(TAG, "Cancelling download with url " + url); + for (Downloader downloader : downloads) { + if (downloader.cancelled || !downloader.getDownloadRequest().getSource().equals(url)) { + continue; + } + downloader.cancel(); + DownloadRequest request = downloader.getDownloadRequest(); + FeedItem item = getFeedItemFromId(request.getFeedfileId()); + if (item != null) { + EventBus.getDefault().post(FeedItemEvent.updated(item)); + // undo enqueue upon cancel + if (request.isMediaEnqueued()) { + Log.v(TAG, "Undoing enqueue upon cancelling download"); + DBWriter.removeQueueItem(getApplicationContext(), false, item); + } + } + } + } + private void onDownloadQueued(Intent intent) { List<DownloadRequest> requests = intent.getParcelableArrayListExtra(EXTRA_REQUESTS); if (requests == null) { - throw new IllegalArgumentException( - "ACTION_ENQUEUE_DOWNLOAD intent needs request extra"); + 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); + Log.d(TAG, "Received enqueue request. #requests=" + requests.size()); - if (cleanupMedia) { - UserPreferences.getEpisodeCleanupAlgorithm() - .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; + if (intent.getBooleanExtra(EXTRA_CLEANUP_MEDIA, false)) { + UserPreferences.getEpisodeCleanupAlgorithm().makeRoomForEpisodes(getApplicationContext(), requests.size()); } for (DownloadRequest request : requests) { - onDownloadQueued(request, itemsEnqueued); + addNewRequest(request); } + postDownloaders(); + stopServiceIfEverythingDone(); + + // Add to-download items to the queue before actual download completed + // so that the resulting queue order is the same as when download is clicked + enqueueFeedItems(requests); } - private List<? extends FeedItem> enqueueFeedItems(@NonNull List<? extends DownloadRequest> requests) - throws Exception { + private void enqueueFeedItems(@NonNull List<DownloadRequest> requests) { List<FeedItem> feedItems = new ArrayList<>(); for (DownloadRequest request : requests) { if (request.getFeedfileType() == FeedMedia.FEEDFILETYPE_FEEDMEDIA) { @@ -450,42 +455,51 @@ public class DownloadService extends Service { feedItems.add(media.getItem()); } } + List<FeedItem> actuallyEnqueued = Collections.emptyList(); + try { + actuallyEnqueued = DBTasks.enqueueFeedItemsToDownload(getApplicationContext(), feedItems); + } catch (InterruptedException | ExecutionException e) { + e.printStackTrace(); + } - return DBTasks.enqueueFeedItemsToDownload(getApplicationContext(), feedItems); + for (DownloadRequest request : requests) { + if (request.getFeedfileType() != FeedMedia.FEEDFILETYPE_FEEDMEDIA) { + continue; + } + final long mediaId = request.getFeedfileId(); + for (FeedItem item : actuallyEnqueued) { + if (item.getMedia() != null && item.getMedia().getId() == mediaId) { + request.setMediaEnqueued(true); + } + } + } } - private void onDownloadQueued(@NonNull DownloadRequest request, - @NonNull List<? extends FeedItem> itemsEnqueued) { - writeFileUrl(request); - - Downloader downloader = downloaderFactory.create(request); - if (downloader != null) { - numberOfDownloads.incrementAndGet(); - - if (request.getFeedfileType() == FeedMedia.FEEDFILETYPE_FEEDMEDIA - && isEnqueued(request, itemsEnqueued)) { - request.setMediaEnqueued(true); + private void enqueueAll(Intent intent) { + boolean initiatedByUser = intent.getBooleanExtra(EXTRA_INITIATED_BY_USER, false); + List<Feed> feeds = DBReader.getFeedList(); + for (Feed feed : feeds) { + if (feed.getPreferences().getKeepUpdated() && !feed.isLocalFeed()) { + DownloadRequest.Builder builder = DownloadRequestCreator.create(feed); + builder.setInitiatedByUser(initiatedByUser); + addNewRequest(builder.build()); } - handler.post(() -> { - downloads.add(downloader); - downloadExecutor.submit(downloader); - postDownloaders(); - }); } - handler.post(this::stopServiceIfEverythingDone); + postDownloaders(); + stopServiceIfEverythingDone(); } - 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; - } - } + private void addNewRequest(@NonNull DownloadRequest request) { + if (isDownloadingFile(request.getSource())) { + Log.d(TAG, "Skipped enqueueing request. Already running."); + return; + } + writeFileUrl(request); + Downloader downloader = downloaderFactory.create(request); + if (downloader != null) { + downloads.add(downloader); + downloadHandleExecutor.submit(() -> performDownload(downloader)); } - return false; } @VisibleForTesting @@ -501,20 +515,6 @@ public class DownloadService extends Service { } /** - * Remove download from the DownloadRequester list and from the - * DownloadService list. - */ - private void removeDownload(final Downloader d) { - handler.post(() -> { - Log.d(TAG, "Removing downloader: " + d.getDownloadRequest().getSource()); - boolean rc = downloads.remove(d); - Log.d(TAG, "Result of downloads.remove: " + rc); - DownloadRequester.getInstance().removeDownload(d.getDownloadRequest()); - postDownloaders(); - }); - } - - /** * Adds a new DownloadStatus object to the list of completed downloads and * saves it in the database * @@ -526,20 +526,11 @@ public class DownloadService extends Service { } /** - * Calls query downloads on the services main thread. This method should be used instead of queryDownloads if it is - * used from a thread other than the main thread. - */ - private void stopServiceIfEverythingDoneAsync() { - handler.post(DownloadService.this::stopServiceIfEverythingDone); - } - - /** * Check if there's something else to download, otherwise stop. */ private void stopServiceIfEverythingDone() { - Log.d(TAG, numberOfDownloads.get() + " downloads left"); - - if (numberOfDownloads.get() <= 0 && DownloadRequester.getInstance().hasNoDownloads()) { + Log.d(TAG, downloads.size() + " downloads left"); + if (downloads.size() <= 0) { Log.d(TAG, "Attempting shutdown"); shutdown(); } @@ -598,7 +589,8 @@ public class DownloadService extends Service { if (notificationUpdater == null) { Log.d(TAG, "Setting up notification updater"); notificationUpdater = new NotificationUpdater(); - notificationUpdaterFuture = schedExecutor.scheduleAtFixedRate(notificationUpdater, 1, 1, TimeUnit.SECONDS); + notificationUpdaterFuture = notificationUpdateExecutor + .scheduleAtFixedRate(notificationUpdater, 1, 1, TimeUnit.SECONDS); } } @@ -614,11 +606,10 @@ public class DownloadService extends Service { private class NotificationUpdater implements Runnable { public void run() { - Notification n = notificationManager.updateNotifications(requester.getNumberOfDownloads(), downloads); + Notification n = notificationManager.updateNotifications(downloads); if (n != null) { NotificationManager nm = (NotificationManager) getSystemService(Context.NOTIFICATION_SERVICE); nm.notify(R.id.notification_downloading, n); - Log.d(TAG, "Download progress notification was posted"); } } } @@ -627,7 +618,7 @@ public class DownloadService extends Service { new PostDownloaderTask(downloads).run(); if (downloadPostFuture == null) { - downloadPostFuture = schedExecutor.scheduleAtFixedRate( + downloadPostFuture = notificationUpdateExecutor.scheduleAtFixedRate( new PostDownloaderTask(downloads), 1, 1, TimeUnit.SECONDS); } } @@ -639,10 +630,9 @@ public class DownloadService extends Service { if (notificationUpdater != null) { notificationUpdater.run(); } - handler.post(() -> { - cancelNotificationUpdater(); - ServiceCompat.stopForeground(this, ServiceCompat.STOP_FOREGROUND_REMOVE); - stopSelf(); - }); + downloadEnqueueExecutor.shutdown(); // Do not accept new downloads + cancelNotificationUpdater(); + ServiceCompat.stopForeground(this, ServiceCompat.STOP_FOREGROUND_REMOVE); + stopSelf(); } } diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadServiceNotification.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadServiceNotification.java index 47692bf02..b80a20a25 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadServiceNotification.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadServiceNotification.java @@ -45,14 +45,14 @@ public class DownloadServiceNotification { * Updates the contents of the service's notifications. Should be called * after setupNotificationBuilders. */ - public Notification updateNotifications(int numDownloads, List<Downloader> downloads) { + public Notification updateNotifications(List<Downloader> downloads) { if (notificationCompatBuilder == null) { return null; } String contentTitle = context.getString(R.string.download_notification_title); - String downloadsLeft = (numDownloads > 0) - ? context.getResources().getQuantityString(R.plurals.downloads_left, numDownloads, numDownloads) + String downloadsLeft = (downloads.size() > 0) + ? context.getResources().getQuantityString(R.plurals.downloads_left, downloads.size(), downloads.size()) : context.getString(R.string.completing); String bigText = compileNotificationString(downloads); diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/Downloader.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/Downloader.java index 2a0989d23..862852710 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/download/Downloader.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/Downloader.java @@ -49,10 +49,6 @@ public abstract class Downloader implements Callable<Downloader> { wifiLock.release(); } - if (result == null) { - throw new IllegalStateException( - "Downloader hasn't created DownloadStatus object"); - } finished = true; return this; } diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/handler/FeedParserTask.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/handler/FeedParserTask.java index 9a0916109..21d3452d6 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/download/handler/FeedParserTask.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/handler/FeedParserTask.java @@ -7,7 +7,6 @@ import de.danoeh.antennapod.model.feed.FeedPreferences; import de.danoeh.antennapod.model.feed.VolumeAdaptionSetting; import de.danoeh.antennapod.core.service.download.DownloadRequest; import de.danoeh.antennapod.core.service.download.DownloadStatus; -import de.danoeh.antennapod.core.storage.DownloadRequester; import de.danoeh.antennapod.parser.feed.FeedHandler; import de.danoeh.antennapod.parser.feed.FeedHandlerResult; import de.danoeh.antennapod.parser.feed.UnsupportedFeedtypeException; @@ -38,7 +37,7 @@ public class FeedParserTask implements Callable<FeedHandlerResult> { feed.setDownloaded(true); feed.setPreferences(new FeedPreferences(0, true, FeedPreferences.AutoDeleteAction.GLOBAL, VolumeAdaptionSetting.OFF, request.getUsername(), request.getPassword())); - feed.setPageNr(request.getArguments().getInt(DownloadRequester.REQUEST_ARG_PAGE_NR, 0)); + feed.setPageNr(request.getArguments().getInt(DownloadRequest.REQUEST_ARG_PAGE_NR, 0)); DownloadError reason = null; String reasonDetailed = null; diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/handler/FeedSyncTask.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/handler/FeedSyncTask.java index dcc1c8fdb..9760c57b1 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/download/handler/FeedSyncTask.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/handler/FeedSyncTask.java @@ -1,14 +1,11 @@ package de.danoeh.antennapod.core.service.download.handler; import android.content.Context; -import android.util.Log; import de.danoeh.antennapod.model.feed.Feed; import de.danoeh.antennapod.core.service.download.DownloadRequest; import de.danoeh.antennapod.core.service.download.DownloadStatus; import de.danoeh.antennapod.core.storage.DBTasks; -import de.danoeh.antennapod.core.storage.DownloadRequestException; -import de.danoeh.antennapod.core.storage.DownloadRequester; import de.danoeh.antennapod.parser.feed.FeedHandlerResult; public class FeedSyncTask { @@ -34,15 +31,11 @@ public class FeedSyncTask { savedFeed = DBTasks.updateFeed(context, result.feed, false); // If loadAllPages=true, check if another page is available and queue it for download - final boolean loadAllPages = request.getArguments().getBoolean(DownloadRequester.REQUEST_ARG_LOAD_ALL_PAGES); + final boolean loadAllPages = request.getArguments().getBoolean(DownloadRequest.REQUEST_ARG_LOAD_ALL_PAGES); final Feed feed = result.feed; if (loadAllPages && feed.getNextPageLink() != null) { - try { - feed.setId(savedFeed.getId()); - DBTasks.loadNextPageOfFeed(context, feed, true); - } catch (DownloadRequestException e) { - Log.e(TAG, "Error trying to load next page", e); - } + feed.setId(savedFeed.getId()); + DBTasks.loadNextPageOfFeed(context, feed, true); } return true; } diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/handler/PostDownloaderTask.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/handler/PostDownloaderTask.java index 7c998146d..5d2c48679 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/service/download/handler/PostDownloaderTask.java +++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/handler/PostDownloaderTask.java @@ -2,7 +2,6 @@ package de.danoeh.antennapod.core.service.download.handler; import de.danoeh.antennapod.core.event.DownloadEvent; import de.danoeh.antennapod.core.service.download.Downloader; -import de.danoeh.antennapod.core.storage.DownloadRequester; import org.greenrobot.eventbus.EventBus; import java.util.ArrayList; @@ -24,7 +23,6 @@ public class PostDownloaderTask implements Runnable { runningDownloads.add(downloader); } } - DownloadRequester.getInstance().updateProgress(downloads); List<Downloader> list = Collections.unmodifiableList(runningDownloads); EventBus.getDefault().postSticky(DownloadEvent.refresh(list)); } diff --git a/core/src/main/java/de/danoeh/antennapod/core/storage/AutomaticDownloadAlgorithm.java b/core/src/main/java/de/danoeh/antennapod/core/storage/AutomaticDownloadAlgorithm.java index cf32eb838..f2afa6fd1 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/storage/AutomaticDownloadAlgorithm.java +++ b/core/src/main/java/de/danoeh/antennapod/core/storage/AutomaticDownloadAlgorithm.java @@ -7,6 +7,9 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import de.danoeh.antennapod.core.service.download.DownloadRequest; +import de.danoeh.antennapod.core.service.download.DownloadRequestCreator; +import de.danoeh.antennapod.core.service.download.DownloadService; import de.danoeh.antennapod.model.feed.FeedItem; import de.danoeh.antennapod.model.feed.FeedPreferences; import de.danoeh.antennapod.core.preferences.UserPreferences; @@ -86,17 +89,17 @@ public class AutomaticDownloadAlgorithm { episodeSpaceLeft = episodeCacheSize - (downloadedEpisodes - deletedEpisodes); } - FeedItem[] itemsToDownload = candidates.subList(0, episodeSpaceLeft) - .toArray(new FeedItem[episodeSpaceLeft]); + List<FeedItem> itemsToDownload = candidates.subList(0, episodeSpaceLeft); + if (itemsToDownload.size() > 0) { + Log.d(TAG, "Enqueueing " + itemsToDownload.size() + " items for download"); - if (itemsToDownload.length > 0) { - Log.d(TAG, "Enqueueing " + itemsToDownload.length + " items for download"); - - try { - DownloadRequester.getInstance().downloadMedia(false, context, false, itemsToDownload); - } catch (DownloadRequestException e) { - e.printStackTrace(); + List<DownloadRequest> requests = new ArrayList<>(); + for (FeedItem episode : itemsToDownload) { + DownloadRequest.Builder request = DownloadRequestCreator.create(episode.getMedia()); + request.setInitiatedByUser(false); + requests.add(request.build()); } + DownloadService.download(context, false, requests.toArray(new DownloadRequest[0])); } } }; 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 c60a2a4b6..b5bd6fca4 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 @@ -5,12 +5,14 @@ import static android.content.Context.MODE_PRIVATE; import android.content.Context; import android.content.SharedPreferences; import android.database.Cursor; -import android.os.Looper; import android.text.TextUtils; import android.util.Log; import androidx.annotation.VisibleForTesting; +import de.danoeh.antennapod.core.service.download.DownloadRequest; +import de.danoeh.antennapod.core.service.download.DownloadRequestCreator; +import de.danoeh.antennapod.core.service.download.DownloadService; import org.greenrobot.eventbus.EventBus; import java.util.ArrayList; @@ -18,14 +20,12 @@ import java.util.Collections; import java.util.Date; import java.util.Iterator; import java.util.List; -import java.util.ListIterator; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.FutureTask; -import java.util.concurrent.atomic.AtomicBoolean; import de.danoeh.antennapod.core.R; import de.danoeh.antennapod.event.FeedItemEvent; @@ -104,8 +104,6 @@ public final class DBTasks { } } - private static final AtomicBoolean isRefreshing = new AtomicBoolean(false); - /** * Refreshes all feeds. * It must not be from the main thread. @@ -116,28 +114,15 @@ public final class DBTasks { * @param initiatedByUser a boolean indicating if the refresh was triggered by user action. */ public static void refreshAllFeeds(final Context context, boolean initiatedByUser) { - if (!isRefreshing.compareAndSet(false, true)) { - Log.d(TAG, "Ignoring request to refresh all feeds: Refresh lock is locked"); - return; - } - - if (Looper.myLooper() == Looper.getMainLooper()) { - throw new IllegalStateException("DBTasks.refreshAllFeeds() must not be called from the main thread."); - } - - List<Feed> feeds = DBReader.getFeedList(); - ListIterator<Feed> iterator = feeds.listIterator(); - while (iterator.hasNext()) { - if (!iterator.next().getPreferences().getKeepUpdated()) { - iterator.remove(); + new Thread(() -> { + List<Feed> feeds = DBReader.getFeedList(); + for (Feed feed : feeds) { + if (feed.isLocalFeed() && feed.getPreferences().getKeepUpdated()) { + LocalFeedUpdater.updateFeed(feed, context); + } } - } - try { - refreshFeeds(context, feeds, false, false, false); - } catch (DownloadRequestException e) { - e.printStackTrace(); - } - isRefreshing.set(false); + }).start(); + DownloadService.refreshAllFeeds(context, initiatedByUser); SharedPreferences prefs = context.getSharedPreferences(PREF_NAME, MODE_PRIVATE); prefs.edit().putLong(PREF_LAST_REFRESH, System.currentTimeMillis()).apply(); @@ -149,27 +134,7 @@ public final class DBTasks { // See Issue #2577 for the details of the rationale } - /** - * Downloads all pages of the given feed even if feed has not been modified since last refresh - * - * @param context Used for requesting the download. - * @param feed The Feed object. - */ - public static void forceRefreshCompleteFeed(final Context context, final Feed feed) { - try { - refreshFeeds(context, Collections.singletonList(feed), true, true, false); - } catch (DownloadRequestException e) { - e.printStackTrace(); - DBWriter.addDownloadStatus( - new DownloadStatus(feed, - feed.getHumanReadableIdentifier(), - DownloadError.ERROR_REQUEST_ERROR, - false, - e.getMessage(), - false) - ); - } - } + /** * Queues the next page of this Feed for download. The given Feed has to be a paged @@ -179,53 +144,39 @@ public final class DBTasks { * @param feed The feed whose next page should be loaded. * @param loadAllPages True if any subsequent pages should also be loaded, false otherwise. */ - public static void loadNextPageOfFeed(final Context context, Feed feed, boolean loadAllPages) throws DownloadRequestException { + public static void loadNextPageOfFeed(final Context context, Feed feed, boolean loadAllPages) { if (feed.isPaged() && feed.getNextPageLink() != null) { int pageNr = feed.getPageNr() + 1; Feed nextFeed = new Feed(feed.getNextPageLink(), null, feed.getTitle() + "(" + pageNr + ")"); nextFeed.setPageNr(pageNr); nextFeed.setPaged(true); nextFeed.setId(feed.getId()); - DownloadRequester.getInstance().downloadFeed(context, nextFeed, loadAllPages, false, true); + + DownloadRequest.Builder builder = DownloadRequestCreator.create(nextFeed); + builder.loadAllPages(loadAllPages); + DownloadService.download(context, false, builder.build()); } else { Log.e(TAG, "loadNextPageOfFeed: Feed was either not paged or contained no nextPageLink"); } } - /** - * Refresh a specific feed even if feed has not been modified since last refresh - * - * @param context Used for requesting the download. - * @param feed The Feed object. - */ - public static void forceRefreshFeed(Context context, Feed feed, boolean initiatedByUser) - throws DownloadRequestException { - Log.d(TAG, "refreshFeed(feed.id: " + feed.getId() + ")"); - refreshFeeds(context, Collections.singletonList(feed), false, true, initiatedByUser); + public static void forceRefreshFeed(Context context, Feed feed, boolean initiatedByUser) { + forceRefreshFeed(context, feed, false, initiatedByUser); } - private static void refreshFeeds(Context context, List<Feed> feeds, boolean loadAllPages, - boolean force, boolean initiatedByUser) throws DownloadRequestException { - List<Feed> localFeeds = new ArrayList<>(); - List<Feed> normalFeeds = new ArrayList<>(); - - for (Feed feed : feeds) { - if (feed.isLocalFeed()) { - localFeeds.add(feed); - } else { - normalFeeds.add(feed); - } - } + public static void forceRefreshCompleteFeed(final Context context, final Feed feed) { + forceRefreshFeed(context, feed, true, true); + } - if (!localFeeds.isEmpty()) { - new Thread(() -> { - for (Feed feed : localFeeds) { - LocalFeedUpdater.updateFeed(feed, context); - } - }).start(); - } - if (!normalFeeds.isEmpty()) { - DownloadRequester.getInstance().downloadFeeds(context, normalFeeds, loadAllPages, force, initiatedByUser); + private static void forceRefreshFeed(Context context, Feed feed, boolean loadAllPages, boolean initiatedByUser) { + if (feed.isLocalFeed()) { + new Thread(() -> LocalFeedUpdater.updateFeed(feed, context)).start(); + } else { + DownloadRequest.Builder builder = DownloadRequestCreator.create(feed); + builder.setInitiatedByUser(initiatedByUser); + builder.setForce(true); + builder.loadAllPages(loadAllPages); + DownloadService.download(context, false, builder.build()); } } @@ -242,8 +193,8 @@ public final class DBTasks { EventBus.getDefault().post(new MessageEvent(context.getString(R.string.error_file_not_found))); } - public static List<? extends FeedItem> enqueueFeedItemsToDownload(final Context context, - List<? extends FeedItem> items) throws InterruptedException, ExecutionException { + public static List<FeedItem> enqueueFeedItemsToDownload(final Context context, + List<FeedItem> items) throws InterruptedException, ExecutionException { List<FeedItem> itemsToEnqueue = new ArrayList<>(); if (UserPreferences.enqueueDownloadedEpisodes()) { LongList queueIDList = DBReader.getQueueIDList(); diff --git a/core/src/main/java/de/danoeh/antennapod/core/storage/DBWriter.java b/core/src/main/java/de/danoeh/antennapod/core/storage/DBWriter.java index 280a2f118..af57cbdae 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/storage/DBWriter.java +++ b/core/src/main/java/de/danoeh/antennapod/core/storage/DBWriter.java @@ -8,6 +8,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.core.app.NotificationManagerCompat; +import de.danoeh.antennapod.core.service.download.DownloadService; import org.greenrobot.eventbus.EventBus; import java.io.File; @@ -191,7 +192,6 @@ public class DBWriter { * Deleting media also removes the download log entries. */ private static void deleteFeedItemsSynchronous(@NonNull Context context, @NonNull List<FeedItem> items) { - DownloadRequester requester = DownloadRequester.getInstance(); List<FeedItem> queue = DBReader.getQueue(); List<FeedItem> removedFromQueue = new ArrayList<>(); for (FeedItem item : items) { @@ -206,9 +206,8 @@ public class DBWriter { } if (item.getMedia().isDownloaded()) { deleteFeedMediaSynchronous(context, item.getMedia()); - } else if (requester.isDownloadingFile(item.getMedia())) { - requester.cancelDownload(context, item.getMedia()); } + DownloadService.cancel(context, item.getMedia().getDownload_url()); } } diff --git a/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadRequestException.java b/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadRequestException.java deleted file mode 100644 index f152ec531..000000000 --- a/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadRequestException.java +++ /dev/null @@ -1,25 +0,0 @@ -package de.danoeh.antennapod.core.storage; - -/** - * Thrown by the DownloadRequester if a download request contains invalid data - * or something went wrong while processing the request. - */ -public class DownloadRequestException extends Exception { - private static final long serialVersionUID = 1L; - - public DownloadRequestException() { - super(); - } - - public DownloadRequestException(String message, Throwable cause) { - super(message, cause); - } - - public DownloadRequestException(String message) { - super(message); - } - - public DownloadRequestException(Throwable cause) { - super(cause); - } -} 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 deleted file mode 100644 index 6192edf7c..000000000 --- a/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadRequester.java +++ /dev/null @@ -1,474 +0,0 @@ -package de.danoeh.antennapod.core.storage; - -import android.content.Context; -import android.content.Intent; -import android.os.Bundle; -import android.text.TextUtils; -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 de.danoeh.antennapod.core.service.download.Downloader; -import org.apache.commons.io.FilenameUtils; - -import java.io.File; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; - -import de.danoeh.antennapod.core.BuildConfig; -import de.danoeh.antennapod.model.feed.Feed; -import de.danoeh.antennapod.model.feed.FeedFile; -import de.danoeh.antennapod.model.feed.FeedItem; -import de.danoeh.antennapod.model.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; - - -/** - * Sends download requests to the DownloadService. This class should always be used for starting downloads, - * otherwise they won't work correctly. - */ -public class DownloadRequester implements DownloadStateProvider { - private static final String TAG = "DownloadRequester"; - - private static final String FEED_DOWNLOADPATH = "cache/"; - private static final String MEDIA_DOWNLOADPATH = "media/"; - - /** - * Denotes the page of the feed that is contained in the DownloadRequest sent by the DownloadRequester. - */ - public static final String REQUEST_ARG_PAGE_NR = "page"; - - /** - * True if all pages after the feed that is contained in this DownloadRequest should be downloaded. - */ - public static final String REQUEST_ARG_LOAD_ALL_PAGES = "loadAllPages"; - - private static DownloadRequester downloader; - - private final Map<String, DownloadRequest> downloads; - - private DownloadRequester() { - downloads = new ConcurrentHashMap<>(); - } - - public static synchronized DownloadRequester getInstance() { - if (downloader == null) { - downloader = new DownloadRequester(); - } - return downloader; - } - - /** - * 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 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) { - if (requests.length <= 0) { - return false; - } - 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, boolean initiatedByUser) { - 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 null; - } - if (!isFilenameAvailable(dest.toString()) || (!partiallyDownloadedFileExists && dest.exists())) { - Log.d(TAG, "Filename already used."); - if (isFilenameAvailable(dest.toString()) && overwriteIfExists) { - boolean result = dest.delete(); - Log.d(TAG, "Deleting file. Result: " + result); - } else { - // find different name - File newDest = null; - for (int i = 1; i < Integer.MAX_VALUE; i++) { - String newName = FilenameUtils.getBaseName(dest - .getName()) - + "-" - + i - + FilenameUtils.EXTENSION_SEPARATOR - + FilenameUtils.getExtension(dest.getName()); - Log.d(TAG, "Testing filename " + newName); - newDest = new File(dest.getParent(), newName); - if (!newDest.exists() - && isFilenameAvailable(newDest.toString())) { - Log.d(TAG, "File doesn't exist yet. Using " + newName); - break; - } - } - if (newDest != null) { - dest = newDest; - } - } - } - Log.d(TAG, "Requesting download of url " + item.getDownload_url()); - String baseUrl = (container != null) ? container.getDownload_url() : null; - item.setDownload_url(URLChecker.prepareURL(item.getDownload_url(), baseUrl)); - - DownloadRequest.Builder builder = new DownloadRequest.Builder(dest.toString(), item, initiatedByUser) - .withAuthentication(username, password) - .lastModified(lastModified) - .deleteOnFailure(deleteOnFailure) - .withArguments(arguments); - return builder.build(); - } - - /** - * Returns true if a filename is available and false if it has already been - * taken by another requested download. - */ - private boolean isFilenameAvailable(String path) { - for (String key : downloads.keySet()) { - DownloadRequest r = downloads.get(key); - if (TextUtils.equals(r.getDestination(), path)) { - if (BuildConfig.DEBUG) - Log.d(TAG, path - + " is already used by another requested download"); - return false; - } - } - if (BuildConfig.DEBUG) - Log.d(TAG, path + " is available as a download destination"); - return true; - } - - /** - * Downloads a feed. - * - * @param context The application's environment. - * @param feed Feeds to download - * @param loadAllPages Set to true to download all pages - */ - public synchronized void downloadFeed(Context context, Feed feed, boolean loadAllPages, - boolean force, boolean initiatedByUser) throws DownloadRequestException { - downloadFeeds(context, Collections.singletonList(feed), loadAllPages, force, initiatedByUser); - } - - /** - * Downloads a list of feeds. - * - * @param context The application's environment. - * @param feeds Feeds to download - * @param loadAllPages Set to true to download all pages - */ - public synchronized void downloadFeeds(Context context, List<Feed> feeds, boolean loadAllPages, - boolean force, boolean initiatedByUser) throws DownloadRequestException { - List<DownloadRequest> requests = new ArrayList<>(); - for (Feed feed : feeds) { - if (!feedFileValid(feed)) { - continue; - } - String username = (feed.getPreferences() != null) ? feed.getPreferences().getUsername() : null; - String password = (feed.getPreferences() != null) ? feed.getPreferences().getPassword() : null; - String lastModified = feed.isPaged() || force ? null : feed.getLastUpdate(); - - Bundle args = new Bundle(); - args.putInt(REQUEST_ARG_PAGE_NR, feed.getPageNr()); - args.putBoolean(REQUEST_ARG_LOAD_ALL_PAGES, loadAllPages); - - DownloadRequest request = createRequest(feed, null, getDownloadPathForFeed(feed), - true, username, password, lastModified, true, args, initiatedByUser - ); - if (request != null) { - requests.add(request); - } - } - if (!requests.isEmpty()) { - download(context, requests.toArray(new DownloadRequest[0])); - } - } - - public File getDownloadPathForFeed(Feed feed) throws DownloadRequestException { - return new File(getFeedfilePath(), getFeedfileName(feed)); - } - - public synchronized void downloadFeed(Context context, Feed feed) throws DownloadRequestException { - downloadFeed(context, feed, false, false, true); - } - - public synchronized void downloadMedia(@NonNull Context context, boolean initiatedByUser, FeedItem... feedItems) - throws DownloadRequestException { - downloadMedia(true, context, initiatedByUser, feedItems); - - } - - @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE) - public synchronized void downloadMedia(boolean performAutoCleanup, @NonNull Context context, - boolean initiatedByUser, 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(), initiatedByUser); - 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(), initiatedByUser - ) - ); - } - } - } - download(context, performAutoCleanup, requests.toArray(new DownloadRequest[0])); - } - - @Nullable - private DownloadRequest createRequest(@Nullable FeedMedia feedmedia, boolean initiatedByUser) - 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 && new File(feedmedia.getFile_url()).exists()) { - 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, initiatedByUser); - } - - /** - * Throws a DownloadRequestException if the feedfile or the download url of - * the feedfile is null. - * - * @throws DownloadRequestException - */ - private boolean feedFileValid(FeedFile f) throws DownloadRequestException { - if (f == null) { - throw new DownloadRequestException("Feedfile was null"); - } else if (f.getDownload_url() == null) { - throw new DownloadRequestException("File has no download URL"); - } else { - return true; - } - } - - /** - * Cancels a running download. - */ - public synchronized void cancelDownload(final Context context, final FeedFile f) { - cancelDownload(context, f.getDownload_url()); - } - - /** - * Cancels a running download. - */ - public synchronized void cancelDownload(final Context context, final String downloadUrl) { - if (BuildConfig.DEBUG) - Log.d(TAG, "Cancelling download with url " + downloadUrl); - Intent cancelIntent = new Intent(DownloadService.ACTION_CANCEL_DOWNLOAD); - cancelIntent.putExtra(DownloadService.EXTRA_DOWNLOAD_URL, downloadUrl); - cancelIntent.setPackage(context.getPackageName()); - context.sendBroadcast(cancelIntent); - } - - /** - * Cancels all running downloads - */ - public synchronized void cancelAllDownloads(Context context) { - Log.d(TAG, "Cancelling all running downloads"); - IntentUtils.sendLocalBroadcast(context, DownloadService.ACTION_CANCEL_ALL_DOWNLOADS); - } - - /** - * Returns true if there is at least one Feed in the downloads queue. - */ - public synchronized boolean isDownloadingFeeds() { - for (DownloadRequest r : downloads.values()) { - if (r.getFeedfileType() == Feed.FEEDFILETYPE_FEED) { - return true; - } - } - return false; - } - - /** - * Checks if feedfile is in the downloads list - */ - public synchronized boolean isDownloadingFile(@NonNull FeedFile item) { - return item.getDownload_url() != null && downloads.containsKey(item.getDownload_url()); - } - - /** - * Get the downloader for this item. - */ - public synchronized DownloadRequest getRequestFor(FeedFile item) { - if (isDownloadingFile(item)) { - return downloads.get(item.getDownload_url()); - } - return null; - } - - /** - * Checks if feedfile with the given download url is in the downloads list - */ - public synchronized boolean isDownloadingFile(String downloadUrl) { - return downloads.get(downloadUrl) != null; - } - - public synchronized boolean hasNoDownloads() { - return downloads.isEmpty(); - } - - /** - * Remove an object from the downloads-list of the requester. - */ - public synchronized void removeDownload(DownloadRequest r) { - if (downloads.remove(r.getSource()) == null) { - Log.e(TAG, - "Could not remove object with url " + r.getSource()); - } - } - - /** - * Get the number of uncompleted Downloads - */ - public synchronized int getNumberOfDownloads() { - return downloads.size(); - } - - private synchronized String getFeedfilePath() throws DownloadRequestException { - return getExternalFilesDirOrThrowException(FEED_DOWNLOADPATH).toString() + "/"; - } - - private synchronized String getFeedfileName(Feed feed) { - String filename = feed.getDownload_url(); - if (feed.getTitle() != null && !feed.getTitle().isEmpty()) { - filename = feed.getTitle(); - } - return "feed-" + FileNameGenerator.generateFileName(filename); - } - - private synchronized String getMediafilePath(FeedMedia media) throws DownloadRequestException { - File externalStorage = getExternalFilesDirOrThrowException( - MEDIA_DOWNLOADPATH - + FileNameGenerator.generateFileName(media.getItem() - .getFeed().getTitle()) + "/" - ); - return externalStorage.toString(); - } - - private File getExternalFilesDirOrThrowException(String type) throws DownloadRequestException { - File result = UserPreferences.getDataFolder(type); - if (result == null) { - throw new DownloadRequestException( - "Failed to access external storage"); - } - return result; - } - - private String getMediafilename(FeedMedia media) { - String filename; - String titleBaseFilename = ""; - - // Try to generate the filename by the item title - if (media.getItem() != null && media.getItem().getTitle() != null) { - String title = media.getItem().getTitle(); - titleBaseFilename = FileNameGenerator.generateFileName(title); - } - - String URLBaseFilename = URLUtil.guessFileName(media.getDownload_url(), - null, media.getMime_type()); - - if (!titleBaseFilename.equals("")) { - // Append extension - final int FILENAME_MAX_LENGTH = 220; - if (titleBaseFilename.length() > FILENAME_MAX_LENGTH) { - titleBaseFilename = titleBaseFilename.substring(0, FILENAME_MAX_LENGTH); - } - filename = titleBaseFilename + FilenameUtils.EXTENSION_SEPARATOR + - FilenameUtils.getExtension(URLBaseFilename); - } else { - // Fall back on URL file name - filename = URLBaseFilename; - } - return filename; - } - - public void updateProgress(List<Downloader> newDownloads) { - for (Downloader downloader : newDownloads) { - DownloadRequest request = downloader.getDownloadRequest(); - if (downloads.containsKey(request.getSource())) { - downloads.put(request.getSource(), request); - } - } - } -} diff --git a/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadStateProvider.java b/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadStateProvider.java deleted file mode 100644 index 051dc03db..000000000 --- a/core/src/main/java/de/danoeh/antennapod/core/storage/DownloadStateProvider.java +++ /dev/null @@ -1,16 +0,0 @@ -package de.danoeh.antennapod.core.storage; - -import androidx.annotation.NonNull; - -import de.danoeh.antennapod.model.feed.FeedFile; - -/** - * Allow callers to query the states of downloads, but not affect them. - */ -public interface DownloadStateProvider { - /** - * @return {@code true} if the named feedfile is in the downloads list - */ - boolean isDownloadingFile(@NonNull FeedFile item); - -} diff --git a/core/src/main/java/de/danoeh/antennapod/core/storage/ItemEnqueuePositionCalculator.java b/core/src/main/java/de/danoeh/antennapod/core/storage/ItemEnqueuePositionCalculator.java index ed88ac1a2..2383cf330 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/storage/ItemEnqueuePositionCalculator.java +++ b/core/src/main/java/de/danoeh/antennapod/core/storage/ItemEnqueuePositionCalculator.java @@ -4,10 +4,10 @@ import android.content.Context; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import androidx.annotation.VisibleForTesting; import java.util.List; +import de.danoeh.antennapod.core.service.download.DownloadService; import de.danoeh.antennapod.model.feed.FeedItem; import de.danoeh.antennapod.model.feed.FeedMedia; import de.danoeh.antennapod.core.preferences.UserPreferences.EnqueueLocation; @@ -22,9 +22,6 @@ class ItemEnqueuePositionCalculator { @NonNull private final EnqueueLocation enqueueLocation; - @VisibleForTesting - DownloadStateProvider downloadStateProvider = DownloadRequester.getInstance(); - public ItemEnqueuePositionCalculator(@NonNull EnqueueLocation enqueueLocation) { this.enqueueLocation = enqueueLocation; } @@ -71,14 +68,9 @@ class ItemEnqueuePositionCalculator { } catch (IndexOutOfBoundsException e) { curItem = null; } - - if (curItem != null + return curItem != null && curItem.getMedia() != null - && downloadStateProvider.isDownloadingFile(curItem.getMedia())) { - return true; - } else { - return false; - } + && DownloadService.isDownloadingFile(curItem.getMedia().getDownload_url()); } private static int getCurrentlyPlayingPosition(@NonNull List<FeedItem> curQueue, diff --git a/core/src/main/java/de/danoeh/antennapod/core/sync/SyncService.java b/core/src/main/java/de/danoeh/antennapod/core/sync/SyncService.java index 82896382d..6d6d7bb41 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/sync/SyncService.java +++ b/core/src/main/java/de/danoeh/antennapod/core/sync/SyncService.java @@ -20,6 +20,9 @@ import androidx.work.WorkManager; import androidx.work.Worker; import androidx.work.WorkerParameters; +import de.danoeh.antennapod.core.service.download.DownloadRequest; +import de.danoeh.antennapod.core.service.download.DownloadService; +import de.danoeh.antennapod.core.service.download.DownloadRequestCreator; import org.apache.commons.lang3.StringUtils; import org.greenrobot.eventbus.EventBus; @@ -35,8 +38,6 @@ import de.danoeh.antennapod.core.service.download.AntennapodHttpClient; 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; import de.danoeh.antennapod.core.sync.queue.SynchronizationQueueStorage; import de.danoeh.antennapod.core.util.FeedItemUtil; import de.danoeh.antennapod.core.util.LongList; @@ -146,11 +147,8 @@ public class SyncService extends Worker { for (String downloadUrl : subscriptionChanges.getAdded()) { if (!URLChecker.containsUrl(localSubscriptions, downloadUrl) && !queuedRemovedFeeds.contains(downloadUrl)) { Feed feed = new Feed(downloadUrl, null); - try { - DownloadRequester.getInstance().downloadFeed(getApplicationContext(), feed); - } catch (DownloadRequestException e) { - e.printStackTrace(); - } + DownloadRequest.Builder builder = DownloadRequestCreator.create(feed); + DownloadService.download(getApplicationContext(), false, builder.build()); } } @@ -188,7 +186,7 @@ public class SyncService extends Worker { private void waitForDownloadServiceCompleted() { EventBus.getDefault().postSticky(new SyncServiceEvent(R.string.sync_status_wait_for_downloads)); try { - while (DownloadRequester.getInstance().isDownloadingFeeds()) { + while (DownloadService.isRunning) { //noinspection BusyWait Thread.sleep(1000); } diff --git a/core/src/main/java/de/danoeh/antennapod/core/util/NetworkUtils.java b/core/src/main/java/de/danoeh/antennapod/core/util/NetworkUtils.java index efc7845a4..0bf301366 100644 --- a/core/src/main/java/de/danoeh/antennapod/core/util/NetworkUtils.java +++ b/core/src/main/java/de/danoeh/antennapod/core/util/NetworkUtils.java @@ -18,8 +18,8 @@ import java.util.List; import java.util.regex.Matcher; import java.util.regex.Pattern; +import de.danoeh.antennapod.core.service.download.DownloadService; import de.danoeh.antennapod.core.storage.DBTasks; -import de.danoeh.antennapod.core.storage.DownloadRequester; import de.danoeh.antennapod.model.feed.FeedMedia; import de.danoeh.antennapod.core.preferences.UserPreferences; import de.danoeh.antennapod.core.service.download.AntennapodHttpClient; @@ -238,7 +238,7 @@ public class NetworkUtils { // otherwise cancel all downloads if (NetworkUtils.isNetworkRestricted()) { Log.i(TAG, "Device is no longer connected to Wi-Fi. Cancelling ongoing downloads"); - DownloadRequester.getInstance().cancelAllDownloads(context); + DownloadService.cancelAll(context); } } } diff --git a/core/src/main/res/values/strings.xml b/core/src/main/res/values/strings.xml index dc3f390da..1196c0eee 100644 --- a/core/src/main/res/values/strings.xml +++ b/core/src/main/res/values/strings.xml @@ -280,7 +280,6 @@ <string name="download_log_title_unknown">Unknown Title</string> <string name="download_type_feed">Feed</string> <string name="download_type_media">Media file</string> - <string name="download_request_error_dialog_message_prefix">An error occurred when trying to download the file:\u0020</string> <string name="null_value_podcast_error">No podcast was provided that could be shown.</string> <string name="no_feed_url_podcast_found_by_search">The suggested podcast did not have an RSS link, AntennaPod found a podcast that could match</string> <string name="authentication_notification_title">Authentication required</string> diff --git a/core/src/test/java/de/danoeh/antennapod/core/service/download/DownloadRequestTest.java b/core/src/test/java/de/danoeh/antennapod/core/service/download/DownloadRequestTest.java index c9c941d38..a4c5b99aa 100644 --- a/core/src/test/java/de/danoeh/antennapod/core/service/download/DownloadRequestTest.java +++ b/core/src/test/java/de/danoeh/antennapod/core/service/download/DownloadRequestTest.java @@ -41,24 +41,19 @@ public class DownloadRequestTest { String username = "testUser"; String password = "testPassword"; FeedFile item = createFeedItem(1); - Bundle arg = new Bundle(); - arg.putString("arg1", "value1"); - DownloadRequest request1 = new DownloadRequest.Builder(destStr, item, true) + DownloadRequest request1 = new DownloadRequest.Builder(destStr, item) .deleteOnFailure(true) .withAuthentication(username, password) - .withArguments(arg) .build(); - DownloadRequest request2 = new DownloadRequest.Builder(destStr, item, true) + DownloadRequest request2 = new DownloadRequest.Builder(destStr, item) .deleteOnFailure(true) .withAuthentication(username, password) - .withArguments(arg) .build(); - DownloadRequest request3 = new DownloadRequest.Builder(destStr, item, true) + DownloadRequest request3 = new DownloadRequest.Builder(destStr, item) .deleteOnFailure(true) .withAuthentication("diffUsername", "diffPassword") - .withArguments(arg) .build(); assertEquals(request1, request2); @@ -74,15 +69,12 @@ public class DownloadRequestTest { { // 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, false) + DownloadRequest request1 = new DownloadRequest.Builder(destStr, item1) .withAuthentication(username1, password1) - .withArguments(arg1) .build(); FeedFile item2 = createFeedItem(2); - DownloadRequest request2 = new DownloadRequest.Builder(destStr, item2, true) + DownloadRequest request2 = new DownloadRequest.Builder(destStr, item2) .withAuthentication(username2, password2) .build(); diff --git a/core/src/test/java/de/danoeh/antennapod/core/storage/ItemEnqueuePositionCalculatorTest.java b/core/src/test/java/de/danoeh/antennapod/core/storage/ItemEnqueuePositionCalculatorTest.java index 91693a084..3578efdfc 100644 --- a/core/src/test/java/de/danoeh/antennapod/core/storage/ItemEnqueuePositionCalculatorTest.java +++ b/core/src/test/java/de/danoeh/antennapod/core/storage/ItemEnqueuePositionCalculatorTest.java @@ -1,5 +1,6 @@ package de.danoeh.antennapod.core.storage; +import de.danoeh.antennapod.core.service.download.DownloadService; import de.danoeh.antennapod.model.playback.RemoteMedia; import org.junit.Test; import org.junit.runner.RunWith; @@ -20,6 +21,8 @@ import de.danoeh.antennapod.model.feed.FeedMedia; import de.danoeh.antennapod.core.feed.FeedMother; import de.danoeh.antennapod.core.preferences.UserPreferences.EnqueueLocation; import de.danoeh.antennapod.model.playback.Playable; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import static de.danoeh.antennapod.core.preferences.UserPreferences.EnqueueLocation.AFTER_CURRENTLY_PLAYING; import static de.danoeh.antennapod.core.preferences.UserPreferences.EnqueueLocation.BACK; @@ -29,9 +32,6 @@ import static de.danoeh.antennapod.core.util.CollectionTestUtil.list; import static de.danoeh.antennapod.core.util.FeedItemUtil.getIdList; import static java.util.Collections.emptyList; import static org.junit.Assert.assertEquals; -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; public class ItemEnqueuePositionCalculatorTest { @@ -84,7 +84,9 @@ public class ItemEnqueuePositionCalculatorTest { idsExpected); } - Playable getCurrentlyPlaying() { return null; } + Playable getCurrentlyPlaying() { + return null; + } } @RunWith(Parameterized.class) @@ -127,12 +129,11 @@ public class ItemEnqueuePositionCalculatorTest { } @RunWith(Parameterized.class) - public static class ItemEnqueuePositionCalculatorPreserveDownloadOrderTest { - + public static class PreserveDownloadOrderTest { /** * The test covers the use case that when user initiates multiple downloads in succession, * resulting in multiple addQueueItem() calls in succession. - * the items in the queue will be in the same order as the the order user taps to download + * the items in the queue will be in the same order as the order user taps to download */ @Parameters(name = "{index}: case<{0}>") public static Iterable<Object[]> data() { @@ -183,61 +184,39 @@ public class ItemEnqueuePositionCalculatorTest { @Test public void testQueueOrderWhenDownloading2Items() { - - // Setup class under test - // ItemEnqueuePositionCalculator calculator = new ItemEnqueuePositionCalculator(options); - DownloadStateProvider stubDownloadStateProvider = mock(DownloadStateProvider.class); - when(stubDownloadStateProvider.isDownloadingFile(any(FeedMedia.class))).thenReturn(false); - calculator.downloadStateProvider = stubDownloadStateProvider; - - // Setup initial data - // A shallow copy, as the test code will manipulate the queue - List<FeedItem> queue = new ArrayList<>(queueInitial); - - // Test body - Playable currentlyPlaying = getCurrentlyPlaying(idCurrentlyPlaying); - // User clicks download on feed item 101 - FeedItem tFI101 = setAsDownloading(101, stubDownloadStateProvider, true); - doAddToQueueAndAssertResult(message + " (1st download)", - calculator, tFI101, queue, currentlyPlaying, - idsExpectedAfter101); - // Then user clicks download on feed item 102 - FeedItem tFI102 = setAsDownloading(102, stubDownloadStateProvider, true); - doAddToQueueAndAssertResult(message + " (2nd download, it should preserve order of download)", - calculator, tFI102, queue, currentlyPlaying, - idsExpectedAfter102); - // simulate download failure case for 102 - setAsDownloading(tFI102, stubDownloadStateProvider, false); - // Then user clicks download on feed item 103 - FeedItem tFI103 = setAsDownloading(103, stubDownloadStateProvider, true); - doAddToQueueAndAssertResult(message - + " (3rd download, with 2nd download failed; " - + "it should be behind 1st download (unless enqueueLocation is BACK)", - calculator, tFI103, queue, currentlyPlaying, - idsExpectedAfter103); - + try (MockedStatic<DownloadService> downloadServiceMock = Mockito.mockStatic(DownloadService.class)) { + List<FeedItem> queue = new ArrayList<>(queueInitial); + + // Test body + Playable currentlyPlaying = getCurrentlyPlaying(idCurrentlyPlaying); + // User clicks download on feed item 101 + FeedItem feedItem101 = createFeedItem(101); + downloadServiceMock.when(() -> + DownloadService.isDownloadingFile(feedItem101.getMedia().getDownload_url())).thenReturn(true); + doAddToQueueAndAssertResult(message + " (1st download)", + calculator, feedItem101, queue, currentlyPlaying, idsExpectedAfter101); + // Then user clicks download on feed item 102 + FeedItem feedItem102 = createFeedItem(102); + downloadServiceMock.when(() -> + DownloadService.isDownloadingFile(feedItem102.getMedia().getDownload_url())).thenReturn(true); + doAddToQueueAndAssertResult(message + " (2nd download, it should preserve order of download)", + calculator, feedItem102, queue, currentlyPlaying, idsExpectedAfter102); + // simulate download failure case for 102 + downloadServiceMock.when(() -> + DownloadService.isDownloadingFile(feedItem102.getMedia().getDownload_url())).thenReturn(false); + // Then user clicks download on feed item 103 + FeedItem feedItem103 = createFeedItem(103); + downloadServiceMock.when(() -> + DownloadService.isDownloadingFile(feedItem103.getMedia().getDownload_url())).thenReturn(true); + doAddToQueueAndAssertResult(message + + " (3rd download, with 2nd download failed; " + + "it should be behind 1st download (unless enqueueLocation is BACK)", + calculator, feedItem103, queue, currentlyPlaying, idsExpectedAfter103); + } } - - - private static FeedItem setAsDownloading(int id, DownloadStateProvider stubDownloadStateProvider, - boolean isDownloading) { - FeedItem item = createFeedItem(id); - FeedMedia media = new FeedMedia(item, "http://download.url.net/" + id, 100000 + id, "audio/mp3"); - media.setId(item.getId()); - item.setMedia(media); - return setAsDownloading(item, stubDownloadStateProvider, isDownloading); - } - - private static FeedItem setAsDownloading(FeedItem item, DownloadStateProvider stubDownloadStateProvider, - boolean isDownloading) { - when(stubDownloadStateProvider.isDownloadingFile(item.getMedia())).thenReturn(isDownloading); - return item; - } - } - static void doAddToQueueAndAssertResult(String message, ItemEnqueuePositionCalculator calculator, FeedItem itemToAdd, @@ -279,7 +258,7 @@ public class ItemEnqueuePositionCalculatorTest { static FeedItem createFeedItem(long id) { FeedItem item = new FeedItem(id, "Item" + id, "ItemId" + id, "url", new Date(), FeedItem.PLAYED, FeedMother.anyFeed()); - FeedMedia media = new FeedMedia(item, "download_url", 1234567, "audio/mpeg"); + FeedMedia media = new FeedMedia(item, "http://download.url.net/" + id, 1234567, "audio/mpeg"); media.setId(item.getId()); item.setMedia(media); return item; |