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

This commit is contained in:
ByteHamster 2023-07-15 16:27:12 +02:00 committed by GitHub
parent 75c3c4cf24
commit 8d1eb62f0b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 34 additions and 89 deletions

View File

@ -72,12 +72,14 @@ public class HttpDownloaderTest {
} }
private Downloader download(String url, String title, boolean expectedResult) { 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); 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 downloader = new HttpDownloader(request);
downloader.call(); downloader.call();
DownloadResult status = downloader.getResult(); DownloadResult status = downloader.getResult();
@ -112,7 +114,8 @@ public class HttpDownloaderTest {
public void testCancel() { public void testCancel() {
final String url = httpServer.getBaseUrl() + "/delay/3"; final String url = httpServer.getBaseUrl() + "/delay/3";
FeedFileImpl feedFile = setupFeedFile(url, "delay", true); 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() { Thread t = new Thread() {
@Override @Override
public void run() { public void run() {
@ -128,12 +131,11 @@ public class HttpDownloaderTest {
} }
DownloadResult result = downloader.getResult(); DownloadResult result = downloader.getResult();
assertFalse(result.isSuccessful()); assertFalse(result.isSuccessful());
assertFalse(new File(feedFile.getFile_url()).exists());
} }
@Test @Test
public void testDeleteOnFailShouldDelete() { 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()); assertFalse(new File(downloader.getDownloadRequest().getDestination()).exists());
} }
@ -143,18 +145,18 @@ public class HttpDownloaderTest {
File dest = new File(destDir, filename); File dest = new File(destDir, filename);
dest.delete(); dest.delete();
assertTrue(dest.createNewFile()); 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()); assertTrue(new File(downloader.getDownloadRequest().getDestination()).exists());
} }
@Test @Test
public void testAuthenticationShouldSucceed() throws InterruptedException { public void testAuthenticationShouldSucceed() throws InterruptedException {
download(urlAuth, "testAuthSuccess", true, true, "user", "passwd", true); download(urlAuth, "testAuthSuccess", true, true, "user", "passwd");
} }
@Test @Test
public void testAuthenticationShouldFail() { 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()); assertEquals(DownloadError.ERROR_UNAUTHORIZED, downloader.getResult().getReason());
} }

View File

@ -28,7 +28,6 @@ public class DownloadRequestCreator {
return new DownloadRequest.Builder(dest.toString(), feed) return new DownloadRequest.Builder(dest.toString(), feed)
.withAuthentication(username, password) .withAuthentication(username, password)
.deleteOnFailure(true)
.lastModified(feed.getLastUpdate()); .lastModified(feed.getLastUpdate());
} }
@ -53,7 +52,6 @@ public class DownloadRequestCreator {
? media.getItem().getFeed().getPreferences().getPassword() : null; ? media.getItem().getFeed().getPreferences().getPassword() : null;
return new DownloadRequest.Builder(dest.toString(), media) return new DownloadRequest.Builder(dest.toString(), media)
.deleteOnFailure(false)
.withAuthentication(username, password); .withAuthentication(username, password);
} }

View File

@ -134,6 +134,7 @@ public class EpisodeDownloadWorker extends Worker {
} else { } else {
sendErrorNotification(); sendErrorNotification();
} }
FileUtils.deleteQuietly(new File(downloader.getDownloadRequest().getDestination()));
return Result.failure(); return Result.failure();
} }
@ -162,6 +163,9 @@ public class EpisodeDownloadWorker extends Worker {
Log.d(TAG, "Requested invalid range, restarting download from the beginning"); Log.d(TAG, "Requested invalid range, restarting download from the beginning");
FileUtils.deleteQuietly(new File(downloader.getDownloadRequest().getDestination())); FileUtils.deleteQuietly(new File(downloader.getDownloadRequest().getDestination()));
sendMessage(request.getTitle(), true); sendMessage(request.getTitle(), true);
if (isLastRunAttempt()) {
FileUtils.deleteQuietly(new File(downloader.getDownloadRequest().getDestination()));
}
return retry3times(); return retry3times();
} }
@ -177,21 +181,29 @@ public class EpisodeDownloadWorker extends Worker {
} else { } else {
sendErrorNotification(); sendErrorNotification();
} }
FileUtils.deleteQuietly(new File(downloader.getDownloadRequest().getDestination()));
return Result.failure(); return Result.failure();
} }
sendMessage(request.getTitle(), true); sendMessage(request.getTitle(), true);
if (isLastRunAttempt()) {
FileUtils.deleteQuietly(new File(downloader.getDownloadRequest().getDestination()));
}
return retry3times(); return retry3times();
} }
private Result retry3times() { private Result retry3times() {
if (getRunAttemptCount() < 2) { if (isLastRunAttempt()) {
return Result.retry();
} else {
sendErrorNotification(); sendErrorNotification();
return Result.failure(); return Result.failure();
} else {
return Result.retry();
} }
} }
private boolean isLastRunAttempt() {
return getRunAttemptCount() >= 2;
}
private void sendMessage(String episodeTitle, boolean retrying) { private void sendMessage(String episodeTitle, boolean retrying) {
if (episodeTitle.length() > 20) { if (episodeTitle.length() > 20) {
episodeTitle = episodeTitle.substring(0, 19) + ""; episodeTitle = episodeTitle.substring(0, 19) + "";

View File

@ -49,12 +49,6 @@ public class HttpDownloader extends Downloader {
File destination = new File(request.getDestination()); File destination = new File(request.getDestination());
final boolean fileExists = destination.exists(); final boolean fileExists = destination.exists();
if (request.isDeleteOnFailure() && fileExists) {
Log.w(TAG, "File already exists");
onSuccess();
return;
}
RandomAccessFile out = null; RandomAccessFile out = null;
InputStream connection; InputStream connection;
ResponseBody responseBody = null; ResponseBody responseBody = null;
@ -309,31 +303,11 @@ public class HttpDownloader extends Downloader {
private void onFail(DownloadError reason, String reasonDetailed) { private void onFail(DownloadError reason, String reasonDetailed) {
Log.d(TAG, "onFail() called with: " + "reason = [" + reason + "], reasonDetailed = [" + reasonDetailed + "]"); Log.d(TAG, "onFail() called with: " + "reason = [" + reason + "], reasonDetailed = [" + reasonDetailed + "]");
result.setFailed(reason, reasonDetailed); result.setFailed(reason, reasonDetailed);
if (request.isDeleteOnFailure()) {
cleanup();
}
} }
private void onCancelled() { private void onCancelled() {
Log.d(TAG, "Download was cancelled"); Log.d(TAG, "Download was cancelled");
result.setCancelled(); result.setCancelled();
cancelled = true; 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.");
}
}
} }
} }

View File

@ -14,7 +14,6 @@ import de.danoeh.antennapod.model.feed.FeedMedia;
public class DownloadRequest implements Parcelable { public class DownloadRequest implements Parcelable {
public static final String REQUEST_ARG_PAGE_NR = "page"; 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 destination;
private final String source; private final String source;
@ -22,7 +21,6 @@ public class DownloadRequest implements Parcelable {
private String username; private String username;
private String password; private String password;
private String lastModified; private String lastModified;
private final boolean deleteOnFailure;
private final long feedfileId; private final long feedfileId;
private final int feedfileType; private final int feedfileType;
private final Bundle arguments; private final Bundle arguments;
@ -35,26 +33,26 @@ public class DownloadRequest implements Parcelable {
private boolean initiatedByUser; private boolean initiatedByUser;
public DownloadRequest(@NonNull String destination, @NonNull String source, @NonNull String title, long feedfileId, 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) { Bundle arguments, boolean initiatedByUser) {
this(destination, source, title, feedfileId, feedfileType, null, deleteOnFailure, username, password, false, this(destination, source, title, feedfileId, feedfileType, null, username, password, false,
arguments, initiatedByUser); arguments, initiatedByUser);
} }
private DownloadRequest(Builder builder) { private DownloadRequest(Builder builder) {
this(builder.destination, builder.source, builder.title, builder.feedfileId, builder.feedfileType, this(builder.destination, builder.source, builder.title, builder.feedfileId, builder.feedfileType,
builder.lastModified, builder.deleteOnFailure, builder.username, builder.password, false, builder.lastModified, builder.username, builder.password, false,
builder.arguments != null ? builder.arguments : new Bundle(), builder.initiatedByUser); builder.arguments, builder.initiatedByUser);
} }
private DownloadRequest(Parcel in) { private DownloadRequest(Parcel in) {
this(in.readString(), in.readString(), in.readString(), in.readLong(), in.readInt(), in.readString(), 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, nullIfEmpty(in.readString()), nullIfEmpty(in.readString()), in.readByte() > 0,
in.readBundle(), in.readByte() > 0); in.readBundle(), in.readByte() > 0);
} }
private DownloadRequest(String destination, String source, String title, long feedfileId, int feedfileType, 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) { boolean mediaEnqueued, Bundle arguments, boolean initiatedByUser) {
this.destination = destination; this.destination = destination;
this.source = source; this.source = source;
@ -62,7 +60,6 @@ public class DownloadRequest implements Parcelable {
this.feedfileId = feedfileId; this.feedfileId = feedfileId;
this.feedfileType = feedfileType; this.feedfileType = feedfileType;
this.lastModified = lastModified; this.lastModified = lastModified;
this.deleteOnFailure = deleteOnFailure;
this.username = username; this.username = username;
this.password = password; this.password = password;
this.mediaEnqueued = mediaEnqueued; this.mediaEnqueued = mediaEnqueued;
@ -83,7 +80,6 @@ public class DownloadRequest implements Parcelable {
dest.writeLong(feedfileId); dest.writeLong(feedfileId);
dest.writeInt(feedfileType); dest.writeInt(feedfileType);
dest.writeString(lastModified); dest.writeString(lastModified);
dest.writeByte((deleteOnFailure) ? (byte) 1 : 0);
// in case of null username/password, still write an empty string // in case of null username/password, still write an empty string
// (rather than skipping it). Otherwise, unmarshalling a collection // (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. // 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) if (lastModified != null ? !lastModified.equals(that.lastModified) : that.lastModified != null)
return false; return false;
if (deleteOnFailure != that.deleteOnFailure) return false;
if (feedfileId != that.feedfileId) return false; if (feedfileId != that.feedfileId) return false;
if (feedfileType != that.feedfileType) return false; if (feedfileType != that.feedfileType) return false;
if (progressPercent != that.progressPercent) 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 + (username != null ? username.hashCode() : 0);
result = 31 * result + (password != null ? password.hashCode() : 0); result = 31 * result + (password != null ? password.hashCode() : 0);
result = 31 * result + (lastModified != null ? lastModified.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 + (int) (feedfileId ^ (feedfileId >>> 32));
result = 31 * result + feedfileType; result = 31 * result + feedfileType;
result = 31 * result + arguments.hashCode(); result = 31 * result + arguments.hashCode();
@ -237,26 +231,6 @@ public class DownloadRequest implements Parcelable {
return lastModified; 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() { public Bundle getArguments() {
return arguments; return arguments;
} }
@ -268,7 +242,6 @@ public class DownloadRequest implements Parcelable {
private String username; private String username;
private String password; private String password;
private String lastModified; private String lastModified;
private boolean deleteOnFailure = false;
private final long feedfileId; private final long feedfileId;
private final int feedfileType; private final int feedfileType;
private final Bundle arguments = new Bundle(); 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) { public Builder lastModified(String lastModified) {
this.lastModified = lastModified; this.lastModified = lastModified;
return this; return this;
@ -322,12 +290,6 @@ public class DownloadRequest implements Parcelable {
return this; return this;
} }
public void loadAllPages(boolean loadAllPages) {
if (loadAllPages) {
arguments.putBoolean(REQUEST_ARG_LOAD_ALL_PAGES, true);
}
}
public DownloadRequest build() { public DownloadRequest build() {
return new DownloadRequest(this); return new DownloadRequest(this);
} }

View File

@ -41,17 +41,14 @@ public class DownloadRequestTest {
String password = "testPassword"; String password = "testPassword";
FeedMedia item = createFeedItem(1); FeedMedia item = createFeedItem(1);
DownloadRequest request1 = new DownloadRequest.Builder(destStr, item) DownloadRequest request1 = new DownloadRequest.Builder(destStr, item)
.deleteOnFailure(true)
.withAuthentication(username, password) .withAuthentication(username, password)
.build(); .build();
DownloadRequest request2 = new DownloadRequest.Builder(destStr, item) DownloadRequest request2 = new DownloadRequest.Builder(destStr, item)
.deleteOnFailure(true)
.withAuthentication(username, password) .withAuthentication(username, password)
.build(); .build();
DownloadRequest request3 = new DownloadRequest.Builder(destStr, item) DownloadRequest request3 = new DownloadRequest.Builder(destStr, item)
.deleteOnFailure(true)
.withAuthentication("diffUsername", "diffPassword") .withAuthentication("diffUsername", "diffPassword")
.build(); .build();