Bug 1272354 - (DLC) Download Action: If there's no download scheduled then exit early. r?grisha draft
authorSebastian Kaspari <s.kaspari@gmail.com>
Mon, 03 Apr 2017 16:27:36 +0200
changeset 555058 067c7b1e6414304e35895c55cf71eafaff74ce0f
parent 554958 aaa0cd3bd620daf6be29c72625f6e63fd0bc1d46
child 555059 29d015e1daa9ab33c08b2e0fdd26bf76d1cdb37d
child 561410 5e306ad656fb86dc5709bad6260e80b369b2f998
push id52142
push users.kaspari@gmail.com
push dateMon, 03 Apr 2017 15:39:58 +0000
reviewersgrisha
bugs1272354
milestone55.0a1
Bug 1272354 - (DLC) Download Action: If there's no download scheduled then exit early. r?grisha There's no need to run all other checks if there's nothing scheduled to be downloaded (anymore). In later patches we want to report error states to our telemetry system and there's no need to report "no network" errors if we are not going to download anything anyways. MozReview-Commit-ID: K4bxbM3ptvZ
mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/dlc/TestDownloadAction.java
--- a/mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java
+++ b/mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java
@@ -45,16 +45,21 @@ public class DownloadAction extends Base
 
     public DownloadAction(Callback callback) {
         this.callback = callback;
     }
 
     public void perform(Context context, DownloadContentCatalog catalog) {
         Log.d(LOGTAG, "Downloading content..");
 
+        if (!catalog.hasScheduledDownloads()) {
+            Log.d(LOGTAG, "No scheduled downloads. Nothing to do.");
+            return;
+        }
+
         if (!isConnectedToNetwork(context)) {
             Log.d(LOGTAG, "No connected network available. Postponing download.");
             // TODO: Reschedule download (bug 1209498)
             return;
         }
 
         if (isActiveNetworkMetered(context)) {
             Log.d(LOGTAG, "Network is metered. Postponing download.");
--- a/mobile/android/tests/background/junit4/src/org/mozilla/gecko/dlc/TestDownloadAction.java
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/dlc/TestDownloadAction.java
@@ -44,34 +44,34 @@ public class TestDownloadAction {
      * Verify that:
      *  * No download is performed on a metered network
      */
     @Test
     public void testNothingIsDoneOnMeteredNetwork() throws Exception {
         DownloadAction action = spy(new DownloadAction(null));
         doReturn(true).when(action).isActiveNetworkMetered(RuntimeEnvironment.application);
 
-        action.perform(RuntimeEnvironment.application, null);
+        action.perform(RuntimeEnvironment.application, mockCatalogWithScheduledDownloads());
 
         verify(action, never()).buildHttpURLConnection(anyString());
         verify(action, never()).download(anyString(), any(File.class));
     }
 
     /**
      * Scenario: No (connected) network is available.
      *
      * Verify that:
      *  * No download is performed
      */
     @Test
     public void testNothingIsDoneIfNoNetworkIsAvailable() throws Exception {
         DownloadAction action = spy(new DownloadAction(null));
         doReturn(false).when(action).isConnectedToNetwork(RuntimeEnvironment.application);
 
-        action.perform(RuntimeEnvironment.application, null);
+        action.perform(RuntimeEnvironment.application, mockCatalogWithScheduledDownloads());
 
         verify(action, never()).isActiveNetworkMetered(any(Context.class));
         verify(action, never()).buildHttpURLConnection(anyString());
         verify(action, never()).download(anyString(), any(File.class));
     }
 
     /**
      * Scenario: Content is scheduled for download but already exists locally (with correct checksum).
@@ -81,16 +81,17 @@ public class TestDownloadAction {
      *  * Content is marked as downloaded in the catalog
      */
     @Test
     public void testExistingAndVerifiedFilesAreNotDownloadedAgain() throws Exception {
         DownloadContent content = new DownloadContentBuilder().build();
 
         DownloadContentCatalog catalog = mock(DownloadContentCatalog.class);
         doReturn(Collections.singletonList(content)).when(catalog).getScheduledDownloads();
+        doReturn(true).when(catalog).hasScheduledDownloads();
 
         DownloadAction action = spy(new DownloadAction(null));
         doReturn(false).when(action).isActiveNetworkMetered(RuntimeEnvironment.application);
 
         File file = mock(File.class);
         doReturn(true).when(file).exists();
         doReturn(file).when(action).createTemporaryFile(RuntimeEnvironment.application, content);
         doReturn(file).when(action).getDestinationFile(RuntimeEnvironment.application, content);
@@ -153,16 +154,17 @@ public class TestDownloadAction {
     public void testSuccessfulDownloadsAreMarkedAsDownloaded() throws Exception {
         DownloadContent content = new DownloadContentBuilder()
                 .setKind(DownloadContent.KIND_FONT)
                 .setType(DownloadContent.TYPE_ASSET_ARCHIVE)
                 .build();
 
         DownloadContentCatalog catalog = mock(DownloadContentCatalog.class);
         doReturn(Collections.singletonList(content)).when(catalog).getScheduledDownloads();
+        doReturn(true).when(catalog).hasScheduledDownloads();
 
         DownloadAction action = spy(new DownloadAction(null));
         doReturn(false).when(action).isActiveNetworkMetered(RuntimeEnvironment.application);
 
         File file = mockNotExistingFile();
         doReturn(file).when(action).createTemporaryFile(RuntimeEnvironment.application, content);
         doReturn(file).when(action).getDestinationFile(RuntimeEnvironment.application, content);
 
@@ -191,16 +193,17 @@ public class TestDownloadAction {
         DownloadContent content = new DownloadContentBuilder()
                 .setKind(DownloadContent.KIND_FONT)
                 .setType(DownloadContent.TYPE_ASSET_ARCHIVE)
                 .setSize(4223)
                 .build();
 
         DownloadContentCatalog catalog = mock(DownloadContentCatalog.class);
         doReturn(Collections.singletonList(content)).when(catalog).getScheduledDownloads();
+        doReturn(true).when(catalog).hasScheduledDownloads();
 
         DownloadAction action = spy(new DownloadAction(null));
         doReturn(false).when(action).isActiveNetworkMetered(RuntimeEnvironment.application);
 
         File temporaryFile = mockFileWithSize(1337L);
         doReturn(temporaryFile).when(action).createTemporaryFile(RuntimeEnvironment.application, content);
 
         ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
@@ -280,16 +283,17 @@ public class TestDownloadAction {
         DownloadContent content = new DownloadContentBuilder()
                 .setKind(DownloadContent.KIND_FONT)
                 .setType(DownloadContent.TYPE_ASSET_ARCHIVE)
                 .setSize(1337L)
                 .build();
 
         DownloadContentCatalog catalog = mock(DownloadContentCatalog.class);
         doReturn(Collections.singletonList(content)).when(catalog).getScheduledDownloads();
+        doReturn(true).when(catalog).hasScheduledDownloads();
 
         DownloadAction action = spy(new DownloadAction(null));
         doReturn(false).when(action).isActiveNetworkMetered(RuntimeEnvironment.application);
 
         File temporaryFile = mockFileWithSize(1337L);
         doReturn(temporaryFile).when(action).createTemporaryFile(RuntimeEnvironment.application, content);
 
         File destinationFile = mockNotExistingFile();
@@ -319,16 +323,17 @@ public class TestDownloadAction {
         DownloadContent content = new DownloadContentBuilder()
                 .setKind(DownloadContent.KIND_FONT)
                 .setType(DownloadContent.TYPE_ASSET_ARCHIVE)
                 .setSize(1337L)
                 .build();
 
         DownloadContentCatalog catalog = mock(DownloadContentCatalog.class);
         doReturn(Collections.singletonList(content)).when(catalog).getScheduledDownloads();
+        doReturn(true).when(catalog).hasScheduledDownloads();
 
         DownloadAction action = spy(new DownloadAction(null));
         doReturn(false).when(action).isActiveNetworkMetered(RuntimeEnvironment.application);
         doNothing().when(action).download(anyString(), any(File.class));
         doReturn(false).when(action).verify(any(File.class), anyString());
 
         File temporaryFile = mockNotExistingFile();
         doReturn(temporaryFile).when(action).createTemporaryFile(RuntimeEnvironment.application, content);
@@ -526,16 +531,37 @@ public class TestDownloadAction {
 
         Assert.assertEquals(AppConstants.MOZ_ANDROID_EXCLUDE_FONTS, fontContent.isKnownContent());
         Assert.assertEquals(AppConstants.MOZ_EXCLUDE_HYPHENATION_DICTIONARIES, hyphenationContent.isKnownContent());
 
         Assert.assertFalse(unknownContent.isKnownContent());
         Assert.assertFalse(contentWithUnknownType.isKnownContent());
     }
 
+    /**
+     * Scenario: Action is executed with no downloads scheduled.
+     *
+     * Verify that:
+     * * Nothing is done.
+     */
+    @Test
+    public void testNoDownloadScheduled() throws Exception {
+        final DownloadAction action = spy(new DownloadAction(null));
+
+        final DownloadContentCatalog catalog = mock(DownloadContentCatalog.class);
+        doReturn(false).when(catalog).hasScheduledDownloads();
+
+        action.perform(RuntimeEnvironment.application, catalog);
+
+        verify(catalog, never()).getScheduledDownloads();
+        verify(action, never()).isConnectedToNetwork(any(Context.class));
+        verify(action, never()).isActiveNetworkMetered(any(Context.class));
+        verify(action, never()).download(anyString(), any(File.class));
+    }
+
     private DownloadContent createUnknownContent(long size) {
         return new DownloadContentBuilder()
                 .setSize(size)
                 .build();
     }
 
     private DownloadContent createContentWithoutType(long size) {
         return new DownloadContentBuilder()
@@ -563,19 +589,26 @@ public class TestDownloadAction {
     private DownloadContent createHyphenationDictionaryWithSize(long size) {
         return new DownloadContentBuilder()
                 .setKind(DownloadContent.KIND_HYPHENATION_DICTIONARY)
                 .setType(DownloadContent.TYPE_ASSET_ARCHIVE)
                 .setSize(size)
                 .build();
     }
 
+    private DownloadContentCatalog mockCatalogWithScheduledDownloads() {
+        return mockCatalogWithScheduledDownloads(
+                createUnknownContent(1337),
+                createUnknownContent(4223));
+    }
+
     private DownloadContentCatalog mockCatalogWithScheduledDownloads(DownloadContent... content) {
         DownloadContentCatalog catalog = mock(DownloadContentCatalog.class);
         doReturn(Arrays.asList(content)).when(catalog).getScheduledDownloads();
+        doReturn(true).when(catalog).hasScheduledDownloads();
         return catalog;
     }
 
     private static File mockNotExistingFile() {
         return mockFileWithUsableSpace(false, 0, Long.MAX_VALUE);
     }
 
     private static File mockNotExistingFileWithUsableSpace(long usableSpace) {