respect download order - dequeue upon cancelling download

This commit is contained in:
orionlee 2019-10-26 20:03:41 -07:00
parent a6e5cd144d
commit dc6221fb82
4 changed files with 122 additions and 7 deletions

View File

@ -5,7 +5,6 @@ import androidx.annotation.Nullable;
import androidx.test.InstrumentationRegistry;
import androidx.test.runner.AndroidJUnit4;
import de.danoeh.antennapod.core.service.download.DownloaderFactory;
import org.awaitility.Awaitility;
import org.awaitility.core.ConditionTimeoutException;
import org.junit.After;
@ -27,6 +26,7 @@ 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.service.download.Downloader;
import de.danoeh.antennapod.core.service.download.DownloaderFactory;
import de.danoeh.antennapod.core.service.download.StubDownloader;
import de.danoeh.antennapod.core.storage.DBReader;
import de.danoeh.antennapod.core.storage.DBWriter;
@ -59,7 +59,9 @@ public class DownloadServiceTest {
}
private Feed setUpTestFeeds() throws Exception {
Feed feed = new Feed("url", null, "Test Feed title 1");
// To avoid complication in case of test failures, leaving behind orphaned
// media files: add a timestamp so that each test run will have its own directory for media files.
Feed feed = new Feed("url", null, "Test Feed title 1 " + System.currentTimeMillis());
List<FeedItem> items = new ArrayList<>();
feed.setItems(items);
FeedItem item1 = new FeedItem(0, "Item 1-1", "Item 1-1", "url", new Date(), FeedItem.NEW, feed);
@ -123,6 +125,70 @@ public class DownloadServiceTest {
});
}
@Test
public void testCancelDownload_UndoEnqueue_Normal() throws Exception {
doTestCancelDownload_UndoEnqueue(false);
}
@Test
public void testCancelDownload_UndoEnqueue_AlreadyInQueue() throws Exception {
doTestCancelDownload_UndoEnqueue(true);
}
private void doTestCancelDownload_UndoEnqueue(boolean itemAlreadyInQueue) throws Exception {
// let download takes longer to ensure the test can cancel the download in time
DownloadService.setDownloaderFactory(new StubDownloaderFactory(150, downloadStatus -> {
downloadStatus.setSuccessful();
}));
UserPreferences.setEnqueueDownloadedEpisodes(true);
UserPreferences.setEnableAutodownload(false);
final long item1Id = testMedia11.getItem().getId();
if (itemAlreadyInQueue) {
// simulate item already in queue condition
DBWriter.addQueueItem(InstrumentationRegistry.getTargetContext(), false, item1Id).get();
assertTrue(DBReader.getQueueIDList().contains(item1Id));
} else {
assertFalse(DBReader.getQueueIDList().contains(item1Id));
}
withFeedItemEventListener(feedItemEventListener -> {
try {
DownloadRequester.getInstance().downloadMedia(false, InstrumentationRegistry.getTargetContext(),
testMedia11.getItem());
if (itemAlreadyInQueue) {
Awaitility.await("download service receives the request - "
+ "no event is expected before cancel is issued")
.atLeast(100, TimeUnit.MILLISECONDS)
.until(() -> true);
} else {
Awaitility.await("item enqueue event")
.atMost(1000, TimeUnit.MILLISECONDS)
.until(() -> feedItemEventListener.getEvents().size() >= 1);
}
DownloadRequester.getInstance().cancelDownload(InstrumentationRegistry.getTargetContext(),
testMedia11);
final int totalNumEventsExpected = itemAlreadyInQueue ? 1 : 3;
Awaitility.await("item dequeue event + download termination event")
.atMost(1000, TimeUnit.MILLISECONDS)
.until(() ->feedItemEventListener.getEvents().size() >= totalNumEventsExpected);
assertFalse("The download should have been canceled",
DBReader.getFeedMedia(testMedia11.getId()).isDownloaded());
if (itemAlreadyInQueue) {
assertTrue("The FeedItem should still be in the queue after the download is cancelled."
+ " It's there before download.",
DBReader.getQueueIDList().contains(item1Id));
} else {
assertFalse("The FeedItem should not be in the queue after the download is cancelled.",
DBReader.getQueueIDList().contains(item1Id));
}
} catch (ConditionTimeoutException cte) {
fail("The expected FeedItemEvent (for media download complete) has not been posted. "
+ cte.getMessage());
}
});
}
private static class StubDownloaderFactory implements DownloaderFactory {
private final long downloadTime;

View File

@ -544,6 +544,11 @@ public class UserPreferences {
return prefs.getBoolean(PREF_ENABLE_AUTODL, false);
}
@VisibleForTesting
public static void setEnableAutodownload(boolean enabled) {
prefs.edit().putBoolean(PREF_ENABLE_AUTODL, enabled).apply();
}
public static boolean isEnableAutodownloadOnBattery() {
return prefs.getBoolean(PREF_ENABLE_AUTODL_ON_BATTERY, true);
}

View File

@ -28,6 +28,7 @@ public class DownloadRequest implements Parcelable {
private long soFar;
private long size;
private int statusMsg;
private boolean mediaEnqueued;
public DownloadRequest(@NonNull String destination,
@NonNull String source,
@ -47,6 +48,7 @@ public class DownloadRequest implements Parcelable {
this.username = username;
this.password = password;
this.deleteOnFailure = deleteOnFailure;
this.mediaEnqueued = false;
this.arguments = (arguments != null) ? arguments : new Bundle();
}
@ -78,6 +80,7 @@ public class DownloadRequest implements Parcelable {
deleteOnFailure = (in.readByte() > 0);
username = nullIfEmpty(in.readString());
password = nullIfEmpty(in.readString());
mediaEnqueued = (in.readByte() > 0);
arguments = in.readBundle();
}
@ -102,6 +105,7 @@ public class DownloadRequest implements Parcelable {
// see: https://stackoverflow.com/a/22926342
dest.writeString(nonNullString(username));
dest.writeString(nonNullString(password));
dest.writeByte((mediaEnqueued) ? (byte) 1 : 0);
dest.writeBundle(arguments);
}
@ -148,6 +152,7 @@ public class DownloadRequest implements Parcelable {
if (title != null ? !title.equals(that.title) : that.title != null) return false;
if (username != null ? !username.equals(that.username) : that.username != null)
return false;
if (mediaEnqueued != that.mediaEnqueued) return false;
return true;
}
@ -167,6 +172,7 @@ public class DownloadRequest implements Parcelable {
result = 31 * result + (int) (soFar ^ (soFar >>> 32));
result = 31 * result + (int) (size ^ (size >>> 32));
result = 31 * result + statusMsg;
result = 31 * result + (mediaEnqueued ? 1 : 0);
return result;
}
@ -248,6 +254,18 @@ public class DownloadRequest implements Parcelable {
return deleteOnFailure;
}
public boolean isMediaEnqueued() {
return mediaEnqueued;
}
/**
* 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;
}

View File

@ -371,10 +371,20 @@ public class DownloadService extends Service {
Downloader d = getDownloader(url);
if (d != null) {
d.cancel();
DownloadRequester.getInstance().removeDownload(d.getDownloadRequest());
DownloadRequest request = d.getDownloadRequest();
DownloadRequester.getInstance().removeDownload(request);
FeedItem item = getFeedItemFromId(d.getDownloadRequest().getFeedfileId());
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 {
@ -418,10 +428,9 @@ public class DownloadService extends Service {
Log.e(TAG, "Unexpected exception during enqueue before downloads. Abort download", e);
return;
}
// TODO-2448: use itemsEnqueued to handle cancel download
for (DownloadRequest request : requests) {
onDownloadQueued(request);
onDownloadQueued(request, itemsEnqueued);
}
}
@ -443,7 +452,8 @@ public class DownloadService extends Service {
return DBTasks.enqueueFeedItemsToDownload(getApplicationContext(), feedItems);
}
private void onDownloadQueued(@NonNull DownloadRequest request) {
private void onDownloadQueued(@NonNull DownloadRequest request,
@NonNull List<? extends FeedItem> itemsEnqueued) {
writeFileUrl(request);
Downloader downloader = downloaderFactory.create(request);
@ -453,6 +463,9 @@ public class DownloadService extends Service {
if (request.getFeedfileType() == Feed.FEEDFILETYPE_FEED) {
downloads.add(0, downloader);
} else {
if (isEnqueued(request, itemsEnqueued)) {
request.setMediaEnqueued(true);
}
downloads.add(downloader);
}
downloadExecutor.submit(downloader);
@ -463,6 +476,19 @@ public class DownloadService extends Service {
queryDownloads();
}
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;
}
}
}
return false;
}
@VisibleForTesting
public static DownloaderFactory getDownloaderFactory() {
return downloaderFactory;