Bug 1237576 - DownloadAction: Remove canWrite() check. r?rnewman draft
authorSebastian Kaspari <s.kaspari@gmail.com>
Mon, 11 Jan 2016 17:51:31 +0100
changeset 320546 2f7226e0a4e745e5aaa5efebd097545f61cf98f3
parent 320545 f1515fac00a5c0637e65cac4a2bb40f65fb61fce
child 512762 ff216b37180fd9c4ab13b722317c84bae72151b2
push id9224
push users.kaspari@gmail.com
push dateMon, 11 Jan 2016 18:26:09 +0000
reviewersrnewman
bugs1237576
milestone46.0a1
Bug 1237576 - DownloadAction: Remove canWrite() check. r?rnewman We can't use File.canWrite() on files that do not exist yet. After all we are going to create the file in a folder that we just created. So it is very unlikely that writing to that folder is going to fail.
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
@@ -82,21 +82,16 @@ public class DownloadAction extends Base
                 if (destinationFile.exists() && verify(destinationFile, content.getChecksum())) {
                     Log.d(LOGTAG, "Content already exists and is up-to-date.");
                     catalog.markAsDownloaded(content);
                     continue;
                 }
 
                 temporaryFile = createTemporaryFile(context, content);
 
-                if (!canWrite(temporaryFile, destinationFile)) {
-                    throw new RecoverableDownloadContentException(RecoverableDownloadContentException.DISK_IO,
-                                                                  "Temporary or destination file not writeable");
-                }
-
                 if (!hasEnoughDiskSpace(content, destinationFile, temporaryFile)) {
                     Log.d(LOGTAG, "Not enough disk space to save content. Skipping download.");
                     continue;
                 }
 
                 // TODO: Check space on disk before downloading content (bug 1220145)
                 final String url = createDownloadURL(content);
 
@@ -336,19 +331,9 @@ public class DownloadAction extends Base
         final File destinationDirectory = destinationFile.getParentFile();
         // We need some more space to extract the file (getSize() returns the uncompressed size)
         if (destinationDirectory.getUsableSpace() < content.getSize() * 2) {
             return false;
         }
 
         return true;
     }
-
-    protected boolean canWrite(File... files) {
-        for (File file : files) {
-            if (!file.canWrite()) {
-                return false;
-            }
-        }
-
-        return true;
-    }
 }
--- 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
@@ -163,17 +163,16 @@ public class TestDownloadAction {
 
         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);
 
-        doReturn(true).when(action).canWrite(any(File.class), any(File.class));
         doReturn(false).when(action).verify(eq(file), anyString());
         doNothing().when(action).download(any(HttpClient.class), anyString(), eq(file));
         doReturn(true).when(action).verify(eq(file), anyString());
         doNothing().when(action).extract(eq(file), eq(file), anyString());
 
         action.perform(RuntimeEnvironment.application, catalog);
 
         verify(action).buildHttpClient();
@@ -198,17 +197,16 @@ public class TestDownloadAction {
                 .setSize(4223)
                 .build();
 
         DownloadContentCatalog catalog = mock(DownloadContentCatalog.class);
         doReturn(Collections.singletonList(content)).when(catalog).getScheduledDownloads();
 
         DownloadAction action = spy(new DownloadAction(null));
         doReturn(false).when(action).isActiveNetworkMetered(RuntimeEnvironment.application);
-        doReturn(true).when(action).canWrite(any(File.class), any(File.class));
 
         File temporaryFile = mockFileWithSize(1337L);
         doReturn(temporaryFile).when(action).createTemporaryFile(RuntimeEnvironment.application, content);
 
         ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
         doReturn(outputStream).when(action).openFile(eq(temporaryFile), anyBoolean());
 
         HttpClient client = mockHttpClient(HttpStatus.SC_PARTIAL_CONTENT, "HelloWorld");
@@ -298,17 +296,16 @@ public class TestDownloadAction {
         doReturn(false).when(action).isActiveNetworkMetered(RuntimeEnvironment.application);
 
         File temporaryFile = mockFileWithSize(1337L);
         doReturn(temporaryFile).when(action).createTemporaryFile(RuntimeEnvironment.application, content);
 
         File destinationFile = mockNotExistingFile();
         doReturn(destinationFile).when(action).getDestinationFile(RuntimeEnvironment.application, content);
 
-        doReturn(true).when(action).canWrite(any(File.class), any(File.class));
         doReturn(true).when(action).verify(eq(temporaryFile), anyString());
         doNothing().when(action).extract(eq(temporaryFile), eq(destinationFile), anyString());
 
         action.perform(RuntimeEnvironment.application, catalog);
 
         verify(action, never()).download(any(HttpClient.class), anyString(), eq(temporaryFile));
         verify(action).verify(eq(temporaryFile), anyString());
         verify(action).extract(eq(temporaryFile), eq(destinationFile), anyString());
@@ -331,17 +328,16 @@ public class TestDownloadAction {
                 .setSize(1337L)
                 .build();
 
         DownloadContentCatalog catalog = mock(DownloadContentCatalog.class);
         doReturn(Collections.singletonList(content)).when(catalog).getScheduledDownloads();
 
         DownloadAction action = spy(new DownloadAction(null));
         doReturn(false).when(action).isActiveNetworkMetered(RuntimeEnvironment.application);
-        doReturn(true).when(action).canWrite(any(File.class), any(File.class));
         doNothing().when(action).download(any(HttpClient.class), anyString(), any(File.class));
         doReturn(false).when(action).verify(any(File.class), anyString());
 
         File temporaryFile = mockNotExistingFile();
         doReturn(temporaryFile).when(action).createTemporaryFile(RuntimeEnvironment.application, content);
 
         File destinationFile = mockNotExistingFile();
         doReturn(destinationFile).when(action).getDestinationFile(RuntimeEnvironment.application, content);
@@ -442,17 +438,16 @@ public class TestDownloadAction {
         DownloadContentCatalog catalog = mockCatalogWithScheduledDownloads(content);
 
         DownloadAction action = spy(new DownloadAction(null));
         doReturn(true).when(action).isConnectedToNetwork(RuntimeEnvironment.application);
         doReturn(false).when(action).isActiveNetworkMetered(RuntimeEnvironment.application);
         doReturn(mockNotExistingFile()).when(action).createTemporaryFile(RuntimeEnvironment.application, content);
         doReturn(mockNotExistingFile()).when(action).getDestinationFile(RuntimeEnvironment.application, content);
         doReturn(true).when(action).hasEnoughDiskSpace(eq(content), any(File.class), any(File.class));
-        doReturn(true).when(action).canWrite(any(File.class), any(File.class));
 
         HttpClient client = mock(HttpClient.class);
         doThrow(IOException.class).when(client).execute(any(HttpUriRequest.class));
         doReturn(client).when(action).buildHttpClient();
 
         action.perform(RuntimeEnvironment.application, catalog);
 
         verify(catalog, never()).rememberFailure(eq(content), anyInt());
@@ -499,42 +494,16 @@ public class TestDownloadAction {
         }
 
         action.perform(RuntimeEnvironment.application, catalog);
 
         Assert.assertEquals(DownloadContent.STATE_FAILED, content.getState());
         verify(catalog, times(11)).rememberFailure(eq(content), anyInt());
     }
 
-    /**
-     * Scenario: Temporary or destination file is not writable.
-     *
-     * Verify that:
-     *  * No download is performed
-     *  * Error is counted as failure
-     */
-    @Test
-    public void testNoDownIsPerformedIfFilesAreNotWritable() throws Exception{
-        DownloadContent content = createFont();
-        DownloadContentCatalog catalog = mockCatalogWithScheduledDownloads(content);
-
-        DownloadAction action = spy(new DownloadAction(null));
-        doReturn(true).when(action).isConnectedToNetwork(RuntimeEnvironment.application);
-        doReturn(false).when(action).isActiveNetworkMetered(RuntimeEnvironment.application);
-        doReturn(mockNotExistingFile()).when(action).createTemporaryFile(RuntimeEnvironment.application, content);
-        doReturn(mockNotExistingFile()).when(action).getDestinationFile(RuntimeEnvironment.application, content);
-        doReturn(false).when(action).canWrite(any(File.class), any(File.class));
-
-        action.perform(RuntimeEnvironment.application, catalog);
-
-        verify(action).canWrite(any(File.class), any(File.class));
-        verify(action, never()).download(any(HttpClient.class), anyString(), any(File.class));
-        verify(catalog).rememberFailure(eq(content), anyInt());
-    }
-
     private DownloadContent createFont() {
         return createFontWithSize(102400L);
     }
 
     private DownloadContent createFontWithSize(long size) {
         return new DownloadContent.Builder()
                 .setKind(DownloadContent.KIND_FONT)
                 .setType(DownloadContent.TYPE_ASSET_ARCHIVE)