Fix file deletion and queueing after download (#6652)

WorkManager does not tell us whether it was cancelled by
the user (not retried) or by the system (retried later).
So we need to delete the file and remove from queue when
we know that it was actually the user. Also make sure
to always delete the file when the download fails.

Also, don't show "will retry" message on last retry attempt.

3 files changed, 40 insertions, 28 deletions

diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadServiceInterfaceImpl.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadServiceInterfaceImpl.java
index 87cbeda84..894a7153b 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadServiceInterfaceImpl.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/DownloadServiceInterfaceImpl.java
@@ -7,12 +7,18 @@ import androidx.work.ExistingWorkPolicy;
 import androidx.work.NetworkType;
 import androidx.work.OneTimeWorkRequest;
 import androidx.work.OutOfQuotaPolicy;
+import androidx.work.WorkInfo;
 import androidx.work.WorkManager;
 import de.danoeh.antennapod.core.storage.DBWriter;
 import de.danoeh.antennapod.model.feed.FeedItem;
+import de.danoeh.antennapod.model.feed.FeedMedia;
 import de.danoeh.antennapod.net.download.serviceinterface.DownloadServiceInterface;
 import de.danoeh.antennapod.storage.preferences.UserPreferences;
+import io.reactivex.Observable;
+import io.reactivex.schedulers.Schedulers;
 
+import java.util.List;
+import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 
 public class DownloadServiceInterfaceImpl extends DownloadServiceInterface {
@@ -40,13 +46,11 @@ public class DownloadServiceInterfaceImpl extends DownloadServiceInterface {
                 .setInitialDelay(0L, TimeUnit.MILLISECONDS)
                 .addTag(DownloadServiceInterface.WORK_TAG)
                 .addTag(DownloadServiceInterface.WORK_TAG_EPISODE_URL + item.getMedia().getDownload_url());
-        Data.Builder builder = new Data.Builder();
-        builder.putLong(WORK_DATA_MEDIA_ID, item.getMedia().getId());
         if (!item.isTagged(FeedItem.TAG_QUEUE) && UserPreferences.enqueueDownloadedEpisodes()) {
             DBWriter.addQueueItem(context, false, item.getId());
-            builder.putBoolean(WORK_DATA_WAS_QUEUED, true);
+            workRequest.addTag(DownloadServiceInterface.WORK_DATA_WAS_QUEUED);
         }
-        workRequest.setInputData(builder.build());
+        workRequest.setInputData(new Data.Builder().putLong(WORK_DATA_MEDIA_ID, item.getMedia().getId()).build());
         return workRequest;
     }
 
@@ -61,8 +65,26 @@ public class DownloadServiceInterfaceImpl extends DownloadServiceInterface {
     }
 
     @Override
-    public void cancel(Context context, String url) {
-        WorkManager.getInstance(context).cancelAllWorkByTag(WORK_TAG_EPISODE_URL + url);
+    public void cancel(Context context, FeedMedia media) {
+        // This needs to be done here, not in the worker. Reason: The worker might or might not be running.
+        DBWriter.deleteFeedMediaOfItem(context, media.getId()); // Remove partially downloaded file
+        String tag = WORK_TAG_EPISODE_URL + media.getDownload_url();
+        Future<List<WorkInfo>> future = WorkManager.getInstance(context).getWorkInfosByTag(tag);
+        Observable.fromFuture(future)
+                .subscribeOn(Schedulers.io())
+                .observeOn(Schedulers.io())
+                .subscribe(
+                    workInfos -> {
+                        for (WorkInfo info : workInfos) {
+                            if (info.getTags().contains(DownloadServiceInterface.WORK_DATA_WAS_QUEUED)) {
+                                DBWriter.removeQueueItem(context, false, media.getItem());
+                            }
+                        }
+                        WorkManager.getInstance(context).cancelAllWorkByTag(tag);
+                    }, exception -> {
+                        WorkManager.getInstance(context).cancelAllWorkByTag(tag);
+                        exception.printStackTrace();
+                    });
     }
 
     @Override
diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/EpisodeDownloadWorker.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/EpisodeDownloadWorker.java
index af5b5c9a4..976b3504a 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/service/download/EpisodeDownloadWorker.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/EpisodeDownloadWorker.java
@@ -93,6 +93,9 @@ public class EpisodeDownloadWorker extends Worker {
             e.printStackTrace();
             result = Result.failure();
         }
+        if (result.equals(Result.failure()) && downloader != null) {
+            FileUtils.deleteQuietly(new File(downloader.getDownloadRequest().getDestination()));
+        }
         progressUpdaterThread.interrupt();
         try {
             progressUpdaterThread.join();
@@ -156,22 +159,15 @@ public class EpisodeDownloadWorker extends Worker {
         } catch (Exception e) {
             DBWriter.addDownloadStatus(downloader.getResult());
             if (EventBus.getDefault().hasSubscriberForEvent(MessageEvent.class)) {
-                sendMessage(request.getTitle(), false);
+                sendMessage(request.getTitle(), true);
             } else {
                 sendErrorNotification();
             }
-            FileUtils.deleteQuietly(new File(downloader.getDownloadRequest().getDestination()));
             return Result.failure();
         }
 
         if (downloader.cancelled) {
-            if (getInputData().getBoolean(DownloadServiceInterface.WORK_DATA_WAS_QUEUED, false)) {
-                try {
-                    DBWriter.removeQueueItem(getApplicationContext(), false, media.getItem()).get();
-                } catch (ExecutionException | InterruptedException e) {
-                    e.printStackTrace();
-                }
-            }
+            // This also happens when the worker was preempted, not just when the user cancelled it
             return Result.success();
         }
 
@@ -188,10 +184,7 @@ public class EpisodeDownloadWorker extends Worker {
                 && Integer.parseInt(status.getReasonDetailed()) == 416) {
             Log.d(TAG, "Requested invalid range, restarting download from the beginning");
             FileUtils.deleteQuietly(new File(downloader.getDownloadRequest().getDestination()));
-            sendMessage(request.getTitle(), true);
-            if (isLastRunAttempt()) {
-                FileUtils.deleteQuietly(new File(downloader.getDownloadRequest().getDestination()));
-            }
+            sendMessage(request.getTitle(), false);
             return retry3times();
         }
 
@@ -203,17 +196,13 @@ public class EpisodeDownloadWorker extends Worker {
                 || status.getReason() == DownloadError.ERROR_IO_BLOCKED) {
             // Fail fast, these are probably unrecoverable
             if (EventBus.getDefault().hasSubscriberForEvent(MessageEvent.class)) {
-                sendMessage(request.getTitle(), false);
+                sendMessage(request.getTitle(), true);
             } else {
                 sendErrorNotification();
             }
-            FileUtils.deleteQuietly(new File(downloader.getDownloadRequest().getDestination()));
             return Result.failure();
         }
-        sendMessage(request.getTitle(), true);
-        if (isLastRunAttempt()) {
-            FileUtils.deleteQuietly(new File(downloader.getDownloadRequest().getDestination()));
-        }
+        sendMessage(request.getTitle(), false);
         return retry3times();
     }
 
@@ -230,7 +219,8 @@ public class EpisodeDownloadWorker extends Worker {
         return getRunAttemptCount() >= 2;
     }
 
-    private void sendMessage(String episodeTitle, boolean retrying) {
+    private void sendMessage(String episodeTitle, boolean isImmediateFail) {
+        boolean retrying = !isLastRunAttempt() && !isImmediateFail;
         if (episodeTitle.length() > 20) {
             episodeTitle = episodeTitle.substring(0, 19) + "…";
         }
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 11e82af27..b2cc46ee0 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
@@ -110,7 +110,7 @@ public class DBWriter {
             @NonNull Context context, @NonNull FeedMedia media) {
         Log.i(TAG, String.format(Locale.US, "Requested to delete FeedMedia [id=%d, title=%s, downloaded=%s",
                 media.getId(), media.getEpisodeTitle(), media.isDownloaded()));
-        if (media.isDownloaded()) {
+        if (media.isDownloaded() || media.getFile_url() != null) {
             // delete downloaded media file
             File mediaFile = new File(media.getFile_url());
             if (mediaFile.exists() && !mediaFile.delete()) {
@@ -206,7 +206,7 @@ public class DBWriter {
                 if (item.getMedia().isDownloaded()) {
                     deleteFeedMediaSynchronous(context, item.getMedia());
                 }
-                DownloadServiceInterface.get().cancel(context, item.getMedia().getDownload_url());
+                DownloadServiceInterface.get().cancel(context, item.getMedia());
             }
         }