Bug 1236507 - DownloadContentService: Skip download if file system is not writable. r?rnewman
--- a/mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java
+++ b/mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java
@@ -82,16 +82,21 @@ 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);
@@ -120,17 +125,17 @@ public class DownloadAction extends Base
if (callback != null) {
callback.onContentDownloaded(content);
}
if (temporaryFile != null && temporaryFile.exists()) {
temporaryFile.delete();
}
} catch (RecoverableDownloadContentException e) {
- Log.w(LOGTAG, "Downloading content failed (Recoverable): " + content , e);
+ Log.w(LOGTAG, "Downloading content failed (Recoverable): " + content, e);
if (e.shouldBeCountedAsFailure()) {
catalog.rememberFailure(content, e.getErrorType());
}
// TODO: Reschedule download (bug 1209498)
} catch (UnrecoverableDownloadContentException e) {
Log.w(LOGTAG, "Downloading content failed (Unrecoverable): " + content, e);
@@ -331,9 +336,19 @@ 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,16 +163,17 @@ 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();
@@ -197,16 +198,17 @@ 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");
@@ -296,16 +298,17 @@ 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());
@@ -328,16 +331,17 @@ 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);
@@ -438,16 +442,17 @@ 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());
@@ -494,16 +499,42 @@ 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)