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.
This commit is contained in:
ByteHamster 2023-09-24 10:03:50 +02:00 committed by GitHub
parent 8073de55af
commit 0e52f08aa5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 45 additions and 31 deletions

View File

@ -32,7 +32,7 @@ public class CancelDownloadActionButton extends ItemActionButton {
@Override
public void onClick(Context context) {
FeedMedia media = item.getMedia();
DownloadServiceInterface.get().cancel(context, media.getDownload_url());
DownloadServiceInterface.get().cancel(context, media);
if (UserPreferences.isEnableAutodownload()) {
item.disableAutoDownload();
DBWriter.setFeedItem(item);

View File

@ -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

View File

@ -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) + "";
}

View File

@ -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());
}
}

View File

@ -3,6 +3,7 @@ package de.danoeh.antennapod.net.download.serviceinterface;
import android.content.Context;
import de.danoeh.antennapod.model.download.DownloadStatus;
import de.danoeh.antennapod.model.feed.FeedItem;
import de.danoeh.antennapod.model.feed.FeedMedia;
import java.util.HashMap;
import java.util.Map;
@ -38,7 +39,7 @@ public abstract class DownloadServiceInterface {
*/
public abstract void download(Context context, FeedItem item);
public abstract void cancel(Context context, String url);
public abstract void cancel(Context context, FeedMedia media);
public abstract void cancelAll(Context context);

View File

@ -2,6 +2,7 @@ package de.danoeh.antennapod.net.download.serviceinterface;
import android.content.Context;
import de.danoeh.antennapod.model.feed.FeedItem;
import de.danoeh.antennapod.model.feed.FeedMedia;
public class DownloadServiceInterfaceStub extends DownloadServiceInterface {
@ -14,7 +15,7 @@ public class DownloadServiceInterfaceStub extends DownloadServiceInterface {
}
@Override
public void cancel(Context context, String url) {
public void cancel(Context context, FeedMedia media) {
}
@Override