Delete partially downloaded file when giving up to retry (#6530)

6 files changed, 34 insertions, 89 deletions

diff --git a/app/src/androidTest/java/de/test/antennapod/service/download/HttpDownloaderTest.java b/app/src/androidTest/java/de/test/antennapod/service/download/HttpDownloaderTest.java
index 76cba4706..aa55a1e59 100644
--- a/app/src/androidTest/java/de/test/antennapod/service/download/HttpDownloaderTest.java
+++ b/app/src/androidTest/java/de/test/antennapod/service/download/HttpDownloaderTest.java
@@ -72,12 +72,14 @@ public class HttpDownloaderTest {
     }
 
     private Downloader download(String url, String title, boolean expectedResult) {
-        return download(url, title, expectedResult, true, null, null, true);
+        return download(url, title, expectedResult, true, null, null);
     }
 
-    private Downloader download(String url, String title, boolean expectedResult, boolean deleteExisting, String username, String password, boolean deleteOnFail) {
+    private Downloader download(String url, String title, boolean expectedResult, boolean deleteExisting,
+                                String username, String password) {
         FeedFile feedFile = setupFeedFile(url, title, deleteExisting);
-        DownloadRequest request = new DownloadRequest(feedFile.getFile_url(), url, title, 0, feedFile.getTypeAsInt(), username, password, deleteOnFail, null, false);
+        DownloadRequest request = new DownloadRequest(feedFile.getFile_url(), url, title, 0, feedFile.getTypeAsInt(),
+                username, password, null, false);
         Downloader downloader = new HttpDownloader(request);
         downloader.call();
         DownloadResult status = downloader.getResult();
@@ -112,7 +114,8 @@ public class HttpDownloaderTest {
     public void testCancel() {
         final String url = httpServer.getBaseUrl() + "/delay/3";
         FeedFileImpl feedFile = setupFeedFile(url, "delay", true);
-        final Downloader downloader = new HttpDownloader(new DownloadRequest(feedFile.getFile_url(), url, "delay", 0, feedFile.getTypeAsInt(), null, null, true, null, false));
+        final Downloader downloader = new HttpDownloader(new DownloadRequest(feedFile.getFile_url(), url, "delay", 0,
+                feedFile.getTypeAsInt(), null, null, null, false));
         Thread t = new Thread() {
             @Override
             public void run() {
@@ -128,12 +131,11 @@ public class HttpDownloaderTest {
         }
         DownloadResult result = downloader.getResult();
         assertFalse(result.isSuccessful());
-        assertFalse(new File(feedFile.getFile_url()).exists());
     }
 
     @Test
     public void testDeleteOnFailShouldDelete() {
-        Downloader downloader = download(url404, "testDeleteOnFailShouldDelete", false, true, null, null, true);
+        Downloader downloader = download(url404, "testDeleteOnFailShouldDelete", false, true, null, null);
         assertFalse(new File(downloader.getDownloadRequest().getDestination()).exists());
     }
 
@@ -143,18 +145,18 @@ public class HttpDownloaderTest {
         File dest = new File(destDir, filename);
         dest.delete();
         assertTrue(dest.createNewFile());
-        Downloader downloader = download(url404, filename, false, false, null, null, false);
+        Downloader downloader = download(url404, filename, false, false, null, null);
         assertTrue(new File(downloader.getDownloadRequest().getDestination()).exists());
     }
 
     @Test
     public void testAuthenticationShouldSucceed() throws InterruptedException {
-        download(urlAuth, "testAuthSuccess", true, true, "user", "passwd", true);
+        download(urlAuth, "testAuthSuccess", true, true, "user", "passwd");
     }
 
     @Test
     public void testAuthenticationShouldFail() {
-        Downloader downloader = download(urlAuth, "testAuthSuccess", false, true, "user", "Wrong passwd", true);
+        Downloader downloader = download(urlAuth, "testAuthSuccess", false, true, "user", "Wrong passwd");
         assertEquals(DownloadError.ERROR_UNAUTHORIZED, downloader.getResult().getReason());
     }
 
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
index 5ca904ff6..4d5f2883d 100644
--- 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
@@ -28,7 +28,6 @@ public class DownloadRequestCreator {
 
         return new DownloadRequest.Builder(dest.toString(), feed)
                 .withAuthentication(username, password)
-                .deleteOnFailure(true)
                 .lastModified(feed.getLastUpdate());
     }
 
@@ -53,7 +52,6 @@ public class DownloadRequestCreator {
                 ? media.getItem().getFeed().getPreferences().getPassword() : null;
 
         return new DownloadRequest.Builder(dest.toString(), media)
-                .deleteOnFailure(false)
                 .withAuthentication(username, password);
     }
 
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 c428bc861..62956894c 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
@@ -134,6 +134,7 @@ public class EpisodeDownloadWorker extends Worker {
             } else {
                 sendErrorNotification();
             }
+            FileUtils.deleteQuietly(new File(downloader.getDownloadRequest().getDestination()));
             return Result.failure();
         }
 
@@ -162,6 +163,9 @@ public class EpisodeDownloadWorker extends Worker {
             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()));
+            }
             return retry3times();
         }
 
@@ -177,21 +181,29 @@ public class EpisodeDownloadWorker extends Worker {
             } 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()));
+        }
         return retry3times();
     }
 
     private Result retry3times() {
-        if (getRunAttemptCount() < 2) {
-            return Result.retry();
-        } else {
+        if (isLastRunAttempt()) {
             sendErrorNotification();
             return Result.failure();
+        } else {
+            return Result.retry();
         }
     }
 
+    private boolean isLastRunAttempt() {
+        return getRunAttemptCount() >= 2;
+    }
+
     private void sendMessage(String episodeTitle, boolean retrying) {
         if (episodeTitle.length() > 20) {
             episodeTitle = episodeTitle.substring(0, 19) + "…";
diff --git a/core/src/main/java/de/danoeh/antennapod/core/service/download/HttpDownloader.java b/core/src/main/java/de/danoeh/antennapod/core/service/download/HttpDownloader.java
index 6de5e7006..94cd81337 100644
--- a/core/src/main/java/de/danoeh/antennapod/core/service/download/HttpDownloader.java
+++ b/core/src/main/java/de/danoeh/antennapod/core/service/download/HttpDownloader.java
@@ -49,12 +49,6 @@ public class HttpDownloader extends Downloader {
         File destination = new File(request.getDestination());
         final boolean fileExists = destination.exists();
 
-        if (request.isDeleteOnFailure() && fileExists) {
-            Log.w(TAG, "File already exists");
-            onSuccess();
-            return;
-        }
-
         RandomAccessFile out = null;
         InputStream connection;
         ResponseBody responseBody = null;
@@ -309,31 +303,11 @@ public class HttpDownloader extends Downloader {
     private void onFail(DownloadError reason, String reasonDetailed) {
         Log.d(TAG, "onFail() called with: " + "reason = [" + reason + "], reasonDetailed = [" + reasonDetailed + "]");
         result.setFailed(reason, reasonDetailed);
-        if (request.isDeleteOnFailure()) {
-            cleanup();
-        }
     }
 
     private void onCancelled() {
         Log.d(TAG, "Download was cancelled");
         result.setCancelled();
         cancelled = true;
-        cleanup();
-    }
-
-    /**
-     * Deletes unfinished downloads.
-     */
-    private void cleanup() {
-        if (request.getDestination() != null) {
-            File dest = new File(request.getDestination());
-            if (dest.exists()) {
-                boolean rc = dest.delete();
-                Log.d(TAG, "Deleted file " + dest.getName() + "; Result: "
-                        + rc);
-            } else {
-                Log.d(TAG, "cleanup() didn't delete file: does not exist.");
-            }
-        }
     }
 }
diff --git a/net/download/service-interface/src/main/java/de/danoeh/antennapod/net/download/serviceinterface/DownloadRequest.java b/net/download/service-interface/src/main/java/de/danoeh/antennapod/net/download/serviceinterface/DownloadRequest.java
index 9f9737edc..962ecfc84 100644
--- a/net/download/service-interface/src/main/java/de/danoeh/antennapod/net/download/serviceinterface/DownloadRequest.java
+++ b/net/download/service-interface/src/main/java/de/danoeh/antennapod/net/download/serviceinterface/DownloadRequest.java
@@ -14,7 +14,6 @@ import de.danoeh.antennapod.model.feed.FeedMedia;
 
 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;
@@ -22,7 +21,6 @@ public class DownloadRequest implements Parcelable {
     private String username;
     private String password;
     private String lastModified;
-    private final boolean deleteOnFailure;
     private final long feedfileId;
     private final int feedfileType;
     private final Bundle arguments;
@@ -35,26 +33,26 @@ public class DownloadRequest implements Parcelable {
     private boolean initiatedByUser;
 
     public DownloadRequest(@NonNull String destination, @NonNull String source, @NonNull String title, long feedfileId,
-                           int feedfileType, String username, String password, boolean deleteOnFailure,
+                           int feedfileType, String username, String password,
                            Bundle arguments, boolean initiatedByUser) {
-        this(destination, source, title, feedfileId, feedfileType, null, deleteOnFailure, username, password, false,
-             arguments, initiatedByUser);
+        this(destination, source, title, feedfileId, feedfileType, null, username, password, false,
+                arguments, initiatedByUser);
     }
 
     private DownloadRequest(Builder builder) {
         this(builder.destination, builder.source, builder.title, builder.feedfileId, builder.feedfileType,
-             builder.lastModified, builder.deleteOnFailure, builder.username, builder.password, false,
-             builder.arguments != null ? builder.arguments : new Bundle(), builder.initiatedByUser);
+                builder.lastModified, builder.username, builder.password, false,
+                builder.arguments, builder.initiatedByUser);
     }
 
     private DownloadRequest(Parcel in) {
         this(in.readString(), in.readString(), in.readString(), in.readLong(), in.readInt(), in.readString(),
-             in.readByte() > 0, nullIfEmpty(in.readString()), nullIfEmpty(in.readString()), in.readByte() > 0,
-             in.readBundle(), in.readByte() > 0);
+                nullIfEmpty(in.readString()), nullIfEmpty(in.readString()), in.readByte() > 0,
+                in.readBundle(), in.readByte() > 0);
     }
 
     private DownloadRequest(String destination, String source, String title, long feedfileId, int feedfileType,
-                            String lastModified, boolean deleteOnFailure, String username, String password,
+                            String lastModified, String username, String password,
                             boolean mediaEnqueued, Bundle arguments, boolean initiatedByUser) {
         this.destination = destination;
         this.source = source;
@@ -62,7 +60,6 @@ public class DownloadRequest implements Parcelable {
         this.feedfileId = feedfileId;
         this.feedfileType = feedfileType;
         this.lastModified = lastModified;
-        this.deleteOnFailure = deleteOnFailure;
         this.username = username;
         this.password = password;
         this.mediaEnqueued = mediaEnqueued;
@@ -83,7 +80,6 @@ public class DownloadRequest implements Parcelable {
         dest.writeLong(feedfileId);
         dest.writeInt(feedfileType);
         dest.writeString(lastModified);
-        dest.writeByte((deleteOnFailure) ? (byte) 1 : 0);
         // in case of null username/password, still write an empty string
         // (rather than skipping it). Otherwise, unmarshalling  a collection
         // of them from a Parcel (from an Intent extra to submit a request to DownloadService) will fail.
@@ -124,7 +120,6 @@ public class DownloadRequest implements Parcelable {
 
         if (lastModified != null ? !lastModified.equals(that.lastModified) : that.lastModified != null)
             return false;
-        if (deleteOnFailure != that.deleteOnFailure) return false;
         if (feedfileId != that.feedfileId) return false;
         if (feedfileType != that.feedfileType) return false;
         if (progressPercent != that.progressPercent) return false;
@@ -151,7 +146,6 @@ public class DownloadRequest implements Parcelable {
         result = 31 * result + (username != null ? username.hashCode() : 0);
         result = 31 * result + (password != null ? password.hashCode() : 0);
         result = 31 * result + (lastModified != null ? lastModified.hashCode() : 0);
-        result = 31 * result + (deleteOnFailure ? 1 : 0);
         result = 31 * result + (int) (feedfileId ^ (feedfileId >>> 32));
         result = 31 * result + feedfileType;
         result = 31 * result + arguments.hashCode();
@@ -237,26 +231,6 @@ public class DownloadRequest implements Parcelable {
         return lastModified;
     }
 
-    public boolean isDeleteOnFailure() {
-        return deleteOnFailure;
-    }
-
-    public boolean isMediaEnqueued() {
-        return mediaEnqueued;
-    }
-
-    public boolean isInitiatedByUser() {
-        return initiatedByUser;
-    }
-
-    /**
-     * Set to true if the media is enqueued because of this download.
-     * The state is helpful if the download is cancelled, and undoing the enqueue is needed.
-     */
-    public void setMediaEnqueued(boolean mediaEnqueued) {
-        this.mediaEnqueued = mediaEnqueued;
-    }
-
     public Bundle getArguments() {
         return arguments;
     }
@@ -268,7 +242,6 @@ public class DownloadRequest implements Parcelable {
         private String username;
         private String password;
         private String lastModified;
-        private boolean deleteOnFailure = false;
         private final long feedfileId;
         private final int feedfileType;
         private final Bundle arguments = new Bundle();
@@ -306,11 +279,6 @@ public class DownloadRequest implements Parcelable {
             }
         }
 
-        public Builder deleteOnFailure(boolean deleteOnFailure) {
-            this.deleteOnFailure = deleteOnFailure;
-            return this;
-        }
-
         public Builder lastModified(String lastModified) {
             this.lastModified = lastModified;
             return this;
@@ -322,12 +290,6 @@ public class DownloadRequest implements Parcelable {
             return this;
         }
 
-        public void loadAllPages(boolean loadAllPages) {
-            if (loadAllPages) {
-                arguments.putBoolean(REQUEST_ARG_LOAD_ALL_PAGES, true);
-            }
-        }
-
         public DownloadRequest build() {
             return new DownloadRequest(this);
         }
diff --git a/net/download/service-interface/src/test/java/de/danoeh/antennapod/net/download/serviceinterface/DownloadRequestTest.java b/net/download/service-interface/src/test/java/de/danoeh/antennapod/net/download/serviceinterface/DownloadRequestTest.java
index 2709744f7..a48934cfe 100644
--- a/net/download/service-interface/src/test/java/de/danoeh/antennapod/net/download/serviceinterface/DownloadRequestTest.java
+++ b/net/download/service-interface/src/test/java/de/danoeh/antennapod/net/download/serviceinterface/DownloadRequestTest.java
@@ -41,17 +41,14 @@ public class DownloadRequestTest {
         String password = "testPassword";
         FeedMedia item = createFeedItem(1);
         DownloadRequest request1 = new DownloadRequest.Builder(destStr, item)
-                .deleteOnFailure(true)
                 .withAuthentication(username, password)
                 .build();
 
         DownloadRequest request2 = new DownloadRequest.Builder(destStr, item)
-                .deleteOnFailure(true)
                 .withAuthentication(username, password)
                 .build();
 
         DownloadRequest request3 = new DownloadRequest.Builder(destStr, item)
-                .deleteOnFailure(true)
                 .withAuthentication("diffUsername", "diffPassword")
                 .build();