From 849bf4ad854973d39698613c3a356c09c87e687c Mon Sep 17 00:00:00 2001 From: ByteHamster Date: Thu, 6 Jan 2022 14:36:11 +0100 Subject: Rewrite download request creation (#5530) Android has a limit on the size of Intent parameters. When enqueuing a huge number of items, it just ignored the argument and did not call onNewIntent. We now load the list over in DownloadService. --- .../core/service/download/DownloadRequestTest.java | 18 ++-- .../storage/ItemEnqueuePositionCalculatorTest.java | 97 +++++++++------------- 2 files changed, 43 insertions(+), 72 deletions(-) (limited to 'core/src/test') diff --git a/core/src/test/java/de/danoeh/antennapod/core/service/download/DownloadRequestTest.java b/core/src/test/java/de/danoeh/antennapod/core/service/download/DownloadRequestTest.java index c9c941d38..a4c5b99aa 100644 --- a/core/src/test/java/de/danoeh/antennapod/core/service/download/DownloadRequestTest.java +++ b/core/src/test/java/de/danoeh/antennapod/core/service/download/DownloadRequestTest.java @@ -41,24 +41,19 @@ public class DownloadRequestTest { String username = "testUser"; String password = "testPassword"; FeedFile item = createFeedItem(1); - Bundle arg = new Bundle(); - arg.putString("arg1", "value1"); - DownloadRequest request1 = new DownloadRequest.Builder(destStr, item, true) + DownloadRequest request1 = new DownloadRequest.Builder(destStr, item) .deleteOnFailure(true) .withAuthentication(username, password) - .withArguments(arg) .build(); - DownloadRequest request2 = new DownloadRequest.Builder(destStr, item, true) + DownloadRequest request2 = new DownloadRequest.Builder(destStr, item) .deleteOnFailure(true) .withAuthentication(username, password) - .withArguments(arg) .build(); - DownloadRequest request3 = new DownloadRequest.Builder(destStr, item, true) + DownloadRequest request3 = new DownloadRequest.Builder(destStr, item) .deleteOnFailure(true) .withAuthentication("diffUsername", "diffPassword") - .withArguments(arg) .build(); assertEquals(request1, request2); @@ -74,15 +69,12 @@ public class DownloadRequestTest { { // test DownloadRequests to parcel String destStr = "file://location/media.mp3"; FeedFile item1 = createFeedItem(1); - Bundle arg1 = new Bundle(); - arg1.putString("arg1", "value1"); - DownloadRequest request1 = new DownloadRequest.Builder(destStr, item1, false) + DownloadRequest request1 = new DownloadRequest.Builder(destStr, item1) .withAuthentication(username1, password1) - .withArguments(arg1) .build(); FeedFile item2 = createFeedItem(2); - DownloadRequest request2 = new DownloadRequest.Builder(destStr, item2, true) + DownloadRequest request2 = new DownloadRequest.Builder(destStr, item2) .withAuthentication(username2, password2) .build(); diff --git a/core/src/test/java/de/danoeh/antennapod/core/storage/ItemEnqueuePositionCalculatorTest.java b/core/src/test/java/de/danoeh/antennapod/core/storage/ItemEnqueuePositionCalculatorTest.java index 91693a084..3578efdfc 100644 --- a/core/src/test/java/de/danoeh/antennapod/core/storage/ItemEnqueuePositionCalculatorTest.java +++ b/core/src/test/java/de/danoeh/antennapod/core/storage/ItemEnqueuePositionCalculatorTest.java @@ -1,5 +1,6 @@ package de.danoeh.antennapod.core.storage; +import de.danoeh.antennapod.core.service.download.DownloadService; import de.danoeh.antennapod.model.playback.RemoteMedia; import org.junit.Test; import org.junit.runner.RunWith; @@ -20,6 +21,8 @@ import de.danoeh.antennapod.model.feed.FeedMedia; import de.danoeh.antennapod.core.feed.FeedMother; import de.danoeh.antennapod.core.preferences.UserPreferences.EnqueueLocation; import de.danoeh.antennapod.model.playback.Playable; +import org.mockito.MockedStatic; +import org.mockito.Mockito; import static de.danoeh.antennapod.core.preferences.UserPreferences.EnqueueLocation.AFTER_CURRENTLY_PLAYING; import static de.danoeh.antennapod.core.preferences.UserPreferences.EnqueueLocation.BACK; @@ -29,9 +32,6 @@ import static de.danoeh.antennapod.core.util.CollectionTestUtil.list; import static de.danoeh.antennapod.core.util.FeedItemUtil.getIdList; import static java.util.Collections.emptyList; import static org.junit.Assert.assertEquals; -import static org.mockito.Mockito.any; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.when; public class ItemEnqueuePositionCalculatorTest { @@ -84,7 +84,9 @@ public class ItemEnqueuePositionCalculatorTest { idsExpected); } - Playable getCurrentlyPlaying() { return null; } + Playable getCurrentlyPlaying() { + return null; + } } @RunWith(Parameterized.class) @@ -127,12 +129,11 @@ public class ItemEnqueuePositionCalculatorTest { } @RunWith(Parameterized.class) - public static class ItemEnqueuePositionCalculatorPreserveDownloadOrderTest { - + public static class PreserveDownloadOrderTest { /** * The test covers the use case that when user initiates multiple downloads in succession, * resulting in multiple addQueueItem() calls in succession. - * the items in the queue will be in the same order as the the order user taps to download + * the items in the queue will be in the same order as the order user taps to download */ @Parameters(name = "{index}: case<{0}>") public static Iterable data() { @@ -183,61 +184,39 @@ public class ItemEnqueuePositionCalculatorTest { @Test public void testQueueOrderWhenDownloading2Items() { - - // Setup class under test - // ItemEnqueuePositionCalculator calculator = new ItemEnqueuePositionCalculator(options); - DownloadStateProvider stubDownloadStateProvider = mock(DownloadStateProvider.class); - when(stubDownloadStateProvider.isDownloadingFile(any(FeedMedia.class))).thenReturn(false); - calculator.downloadStateProvider = stubDownloadStateProvider; - - // Setup initial data - // A shallow copy, as the test code will manipulate the queue - List queue = new ArrayList<>(queueInitial); - - // Test body - Playable currentlyPlaying = getCurrentlyPlaying(idCurrentlyPlaying); - // User clicks download on feed item 101 - FeedItem tFI101 = setAsDownloading(101, stubDownloadStateProvider, true); - doAddToQueueAndAssertResult(message + " (1st download)", - calculator, tFI101, queue, currentlyPlaying, - idsExpectedAfter101); - // Then user clicks download on feed item 102 - FeedItem tFI102 = setAsDownloading(102, stubDownloadStateProvider, true); - doAddToQueueAndAssertResult(message + " (2nd download, it should preserve order of download)", - calculator, tFI102, queue, currentlyPlaying, - idsExpectedAfter102); - // simulate download failure case for 102 - setAsDownloading(tFI102, stubDownloadStateProvider, false); - // Then user clicks download on feed item 103 - FeedItem tFI103 = setAsDownloading(103, stubDownloadStateProvider, true); - doAddToQueueAndAssertResult(message - + " (3rd download, with 2nd download failed; " - + "it should be behind 1st download (unless enqueueLocation is BACK)", - calculator, tFI103, queue, currentlyPlaying, - idsExpectedAfter103); - + try (MockedStatic downloadServiceMock = Mockito.mockStatic(DownloadService.class)) { + List queue = new ArrayList<>(queueInitial); + + // Test body + Playable currentlyPlaying = getCurrentlyPlaying(idCurrentlyPlaying); + // User clicks download on feed item 101 + FeedItem feedItem101 = createFeedItem(101); + downloadServiceMock.when(() -> + DownloadService.isDownloadingFile(feedItem101.getMedia().getDownload_url())).thenReturn(true); + doAddToQueueAndAssertResult(message + " (1st download)", + calculator, feedItem101, queue, currentlyPlaying, idsExpectedAfter101); + // Then user clicks download on feed item 102 + FeedItem feedItem102 = createFeedItem(102); + downloadServiceMock.when(() -> + DownloadService.isDownloadingFile(feedItem102.getMedia().getDownload_url())).thenReturn(true); + doAddToQueueAndAssertResult(message + " (2nd download, it should preserve order of download)", + calculator, feedItem102, queue, currentlyPlaying, idsExpectedAfter102); + // simulate download failure case for 102 + downloadServiceMock.when(() -> + DownloadService.isDownloadingFile(feedItem102.getMedia().getDownload_url())).thenReturn(false); + // Then user clicks download on feed item 103 + FeedItem feedItem103 = createFeedItem(103); + downloadServiceMock.when(() -> + DownloadService.isDownloadingFile(feedItem103.getMedia().getDownload_url())).thenReturn(true); + doAddToQueueAndAssertResult(message + + " (3rd download, with 2nd download failed; " + + "it should be behind 1st download (unless enqueueLocation is BACK)", + calculator, feedItem103, queue, currentlyPlaying, idsExpectedAfter103); + } } - - - private static FeedItem setAsDownloading(int id, DownloadStateProvider stubDownloadStateProvider, - boolean isDownloading) { - FeedItem item = createFeedItem(id); - FeedMedia media = new FeedMedia(item, "http://download.url.net/" + id, 100000 + id, "audio/mp3"); - media.setId(item.getId()); - item.setMedia(media); - return setAsDownloading(item, stubDownloadStateProvider, isDownloading); - } - - private static FeedItem setAsDownloading(FeedItem item, DownloadStateProvider stubDownloadStateProvider, - boolean isDownloading) { - when(stubDownloadStateProvider.isDownloadingFile(item.getMedia())).thenReturn(isDownloading); - return item; - } - } - static void doAddToQueueAndAssertResult(String message, ItemEnqueuePositionCalculator calculator, FeedItem itemToAdd, @@ -279,7 +258,7 @@ public class ItemEnqueuePositionCalculatorTest { static FeedItem createFeedItem(long id) { FeedItem item = new FeedItem(id, "Item" + id, "ItemId" + id, "url", new Date(), FeedItem.PLAYED, FeedMother.anyFeed()); - FeedMedia media = new FeedMedia(item, "download_url", 1234567, "audio/mpeg"); + FeedMedia media = new FeedMedia(item, "http://download.url.net/" + id, 1234567, "audio/mpeg"); media.setId(item.getId()); item.setMedia(media); return item; -- cgit v1.2.3