Bug 1209513 - DownloadContentService: Add support for resuming downloads. r?rnewman
--- a/mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java
+++ b/mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java
@@ -15,16 +15,17 @@ import org.mozilla.gecko.dlc.catalog.Dow
import org.mozilla.gecko.dlc.catalog.DownloadContentCatalog;
import org.mozilla.gecko.util.HardwareUtils;
import org.mozilla.gecko.util.IOUtils;
import java.io.BufferedInputStream;
import java.io.BufferedOutputStream;
import java.io.File;
import java.io.FileInputStream;
+import java.io.FileNotFoundException;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.zip.GZIPInputStream;
import ch.boye.httpclientandroidlib.HttpEntity;
import ch.boye.httpclientandroidlib.HttpResponse;
@@ -76,66 +77,79 @@ public class DownloadAction extends Base
catalog.markAsDownloaded(content);
continue;
}
temporaryFile = createTemporaryFile(context, content);
// TODO: Check space on disk before downloading content (bug 1220145)
final String url = createDownloadURL(content);
- download(client, url, temporaryFile);
+
+ if (!temporaryFile.exists() || temporaryFile.length() < content.getSize()) {
+ download(client, url, temporaryFile);
+ }
if (!verify(temporaryFile, content.getDownloadChecksum())) {
Log.w(LOGTAG, "Wrong checksum after download, content=" + content.getId());
temporaryFile.delete();
continue;
}
if (!content.isAssetArchive()) {
Log.e(LOGTAG, "Downloaded content is not of type 'asset-archive': " + content.getType());
+ temporaryFile.delete();
continue;
}
extract(temporaryFile, destinationFile, content.getChecksum());
catalog.markAsDownloaded(content);
Log.d(LOGTAG, "Successfully downloaded: " + content);
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);
// TODO: Reschedule download (bug 1209498)
} catch (UnrecoverableDownloadContentException e) {
Log.w(LOGTAG, "Downloading content failed (Unrecoverable): " + content, e);
catalog.markAsPermanentlyFailed(content);
- } finally {
+
if (temporaryFile != null && temporaryFile.exists()) {
temporaryFile.delete();
}
}
}
Log.v(LOGTAG, "Done");
}
- /* package-private */ void download(HttpClient client, String source, File temporaryFile)
+ protected void download(HttpClient client, String source, File temporaryFile)
throws RecoverableDownloadContentException, UnrecoverableDownloadContentException {
InputStream inputStream = null;
OutputStream outputStream = null;
final HttpGet request = new HttpGet(source);
+ final long offset = temporaryFile.exists() ? temporaryFile.length() : 0;
+ if (offset > 0) {
+ request.setHeader("Range", "bytes=" + offset + "-");
+ }
+
try {
final HttpResponse response = client.execute(request);
final int status = response.getStatusLine().getStatusCode();
- if (status != HttpStatus.SC_OK) {
+ if (status != HttpStatus.SC_OK && status != HttpStatus.SC_PARTIAL_CONTENT) {
// We are trying to be smart and only retry if this is an error that might resolve in the future.
// TODO: This is guesstimating at best. We want to implement failure counters (Bug 1215106).
if (status >= 500) {
// Recoverable: Server errors 5xx
throw new RecoverableDownloadContentException("(Recoverable) Download failed. Status code: " + status);
} else if (status >= 400) {
// Unrecoverable: Client errors 4xx - Unlikely that this version of the client will ever succeed.
throw new UnrecoverableDownloadContentException("(Unrecoverable) Download failed. Status code: " + status);
@@ -149,34 +163,35 @@ public class DownloadAction extends Base
final HttpEntity entity = response.getEntity();
if (entity == null) {
// Recoverable: Should not happen for a valid asset
throw new RecoverableDownloadContentException("Null entity");
}
inputStream = new BufferedInputStream(entity.getContent());
- outputStream = new BufferedOutputStream(new FileOutputStream(temporaryFile));
+ outputStream = openFile(temporaryFile, status == HttpStatus.SC_PARTIAL_CONTENT);
IOUtils.copy(inputStream, outputStream);
inputStream.close();
outputStream.close();
} catch (IOException e) {
- // TODO: Support resuming downloads using 'Range' header (Bug 1209513)
- temporaryFile.delete();
-
// Recoverable: Just I/O discontinuation
throw new RecoverableDownloadContentException(e);
} finally {
IOUtils.safeStreamClose(inputStream);
IOUtils.safeStreamClose(outputStream);
}
}
+ protected OutputStream openFile(File file, boolean append) throws FileNotFoundException {
+ return new BufferedOutputStream(new FileOutputStream(file, append));
+ }
+
protected void extract(File sourceFile, File destinationFile, String checksum)
throws UnrecoverableDownloadContentException, RecoverableDownloadContentException {
InputStream inputStream = null;
OutputStream outputStream = null;
File temporaryFile = null;
try {
File destinationDirectory = destinationFile.getParentFile();
@@ -196,19 +211,18 @@ public class DownloadAction extends Base
if (!verify(temporaryFile, checksum)) {
Log.w(LOGTAG, "Checksum of extracted file does not match.");
return;
}
move(temporaryFile, destinationFile);
} catch (IOException e) {
- // We do not support resume yet (Bug 1209513). Therefore we have to treat this as unrecoverable: The
- // temporarily file will be deleted and we want to avoid downloading and failing repeatedly.
- throw new UnrecoverableDownloadContentException(e);
+ // We could not extract to the destination: Keep temporary file and try again next time we run.
+ throw new RecoverableDownloadContentException(e);
} finally {
IOUtils.safeStreamClose(inputStream);
IOUtils.safeStreamClose(outputStream);
if (temporaryFile != null && temporaryFile.exists()) {
temporaryFile.delete();
}
}
@@ -268,19 +282,17 @@ public class DownloadAction extends Base
inputStream = new BufferedInputStream(new FileInputStream(temporaryFile));
outputStream = new BufferedOutputStream(new FileOutputStream(destinationFile));
IOUtils.copy(inputStream, outputStream);
inputStream.close();
outputStream.close();
} catch (IOException e) {
- // Meh. This is an awkward situation: We downloaded the content but we can't move it to its destination. We
- // are treating this as "unrecoverable" error because we want to avoid downloading this again and again and
- // then always failing to copy it to the destination. This will be fixed after we implement resuming
- // downloads (Bug 1209513): We will keep the downloaded temporary file and just retry copying.
- throw new UnrecoverableDownloadContentException(e);
+ // We could not copy the temporary file to its destination: Keep the temporary file and
+ // try again the next time we run.
+ throw new RecoverableDownloadContentException(e);
} finally {
IOUtils.safeStreamClose(inputStream);
IOUtils.safeStreamClose(outputStream);
}
}
}
--- 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
@@ -1,35 +1,46 @@
/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package org.mozilla.gecko.dlc;
+import org.junit.Assert;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.ArgumentCaptor;
import org.mozilla.gecko.background.testhelpers.TestRunner;
import org.mozilla.gecko.dlc.catalog.DownloadContent;
import org.mozilla.gecko.dlc.catalog.DownloadContentCatalog;
import org.robolectric.RuntimeEnvironment;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
import java.io.File;
+import java.io.IOException;
import java.util.Collections;
+import ch.boye.httpclientandroidlib.HttpEntity;
import ch.boye.httpclientandroidlib.HttpResponse;
+import ch.boye.httpclientandroidlib.HttpStatus;
import ch.boye.httpclientandroidlib.StatusLine;
import ch.boye.httpclientandroidlib.client.HttpClient;
+import ch.boye.httpclientandroidlib.client.methods.HttpGet;
import ch.boye.httpclientandroidlib.client.methods.HttpUriRequest;
import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.anyBoolean;
+import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
/**
* DownloadAction: Download content that has been scheduled during "study" or "verify".
*/
@@ -86,50 +97,42 @@ public class TestDownloadAction {
/**
* Scenario: Server returns a server error (HTTP 500).
*
* Verify that:
* * Situation is treated as recoverable (RecoverableDownloadContentException)
*/
@Test(expected=BaseAction.RecoverableDownloadContentException.class)
public void testServerErrorsAreRecoverable() throws Exception {
- StatusLine status = mock(StatusLine.class);
- doReturn(500).when(status).getStatusCode();
+ HttpClient client = mockHttpClient(500, "");
- HttpResponse response = mock(HttpResponse.class);
- doReturn(status).when(response).getStatusLine();
-
- HttpClient client = mock(HttpClient.class);
- doReturn(response).when(client).execute(any(HttpUriRequest.class));
+ File temporaryFile = mock(File.class);
+ doReturn(false).when(temporaryFile).exists();
DownloadAction action = spy(new DownloadAction(null));
- action.download(client, TEST_URL, null);
+ action.download(client, TEST_URL, temporaryFile);
verify(client).execute(any(HttpUriRequest.class));
}
/**
* Scenario: Server returns a client error (HTTP 404).
*
* Verify that:
* * Situation is treated as unrecoverable (UnrecoverableDownloadContentException)
*/
@Test(expected=BaseAction.UnrecoverableDownloadContentException.class)
public void testClientErrorsAreUnrecoverable() throws Exception {
- StatusLine status = mock(StatusLine.class);
- doReturn(404).when(status).getStatusCode();
+ HttpClient client = mockHttpClient(404, "");
- HttpResponse response = mock(HttpResponse.class);
- doReturn(status).when(response).getStatusLine();
-
- HttpClient client = mock(HttpClient.class);
- doReturn(response).when(client).execute(any(HttpUriRequest.class));
+ File temporaryFile = mock(File.class);
+ doReturn(false).when(temporaryFile).exists();
DownloadAction action = spy(new DownloadAction(null));
- action.download(client, TEST_URL, null);
+ action.download(client, TEST_URL, temporaryFile);
verify(client).execute(any(HttpUriRequest.class));
}
/**
* Scenario: A successful download has been performed.
*
* Verify that:
@@ -161,9 +164,201 @@ public class TestDownloadAction {
action.perform(RuntimeEnvironment.application, catalog);
verify(action).buildHttpClient();
verify(action).download(any(HttpClient.class), anyString(), eq(file));
verify(action).extract(eq(file), eq(file), anyString());
verify(catalog).markAsDownloaded(content);
}
+
+ /**
+ * Scenario: Pretend a partially downloaded file already exists.
+ *
+ * Verify that:
+ * * Range header is set in request
+ * * Content will be appended to existing file
+ * * Content will be marked as downloaded in catalog
+ */
+ @Test
+ public void testResumingDownloadFromExistingFile() throws Exception {
+ DownloadContent content = new DownloadContent.Builder()
+ .setKind(DownloadContent.KIND_FONT)
+ .setType(DownloadContent.TYPE_ASSET_ARCHIVE)
+ .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);
+
+ File temporaryFile = mockTemporaryFile(true, 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");
+ doReturn(client).when(action).buildHttpClient();
+
+ File destinationFile = mock(File.class);
+ doReturn(destinationFile).when(action).getDestinationFile(RuntimeEnvironment.application, content);
+
+ doReturn(true).when(action).verify(eq(temporaryFile), anyString());
+ doNothing().when(action).extract(eq(temporaryFile), eq(destinationFile), anyString());
+
+ action.perform(RuntimeEnvironment.application, catalog);
+
+ ArgumentCaptor<HttpGet> argument = ArgumentCaptor.forClass(HttpGet.class);
+ verify(client).execute(argument.capture());
+
+ HttpGet request = argument.getValue();
+ Assert.assertTrue(request.containsHeader("Range"));
+ Assert.assertEquals("bytes=1337-", request.getFirstHeader("Range").getValue());
+ Assert.assertEquals("HelloWorld", new String(outputStream.toByteArray(), "UTF-8"));
+
+ verify(action).openFile(eq(temporaryFile), eq(true));
+ verify(catalog).markAsDownloaded(content);
+ verify(temporaryFile).delete();
+ }
+
+ /**
+ * Scenario: Download fails with IOException.
+ *
+ * Verify that:
+ * * Partially downloaded file will not be deleted
+ * * Content will not be marked as downloaded in catalog
+ */
+ @Test
+ public void testTemporaryFileIsNotDeletedAfterDownloadAborted() throws Exception {
+ DownloadContent content = new DownloadContent.Builder()
+ .setKind(DownloadContent.KIND_FONT)
+ .setType(DownloadContent.TYPE_ASSET_ARCHIVE)
+ .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);
+
+ File temporaryFile = mockTemporaryFile(true, 1337L);
+ doReturn(temporaryFile).when(action).createTemporaryFile(RuntimeEnvironment.application, content);
+
+ ByteArrayOutputStream outputStream = spy(new ByteArrayOutputStream());
+ doReturn(outputStream).when(action).openFile(eq(temporaryFile), anyBoolean());
+ doThrow(IOException.class).when(outputStream).write(any(byte[].class), anyInt(), anyInt());
+
+ HttpClient client = mockHttpClient(HttpStatus.SC_PARTIAL_CONTENT, "HelloWorld");
+ doReturn(client).when(action).buildHttpClient();
+
+ File destinationFile = mock(File.class);
+ doReturn(destinationFile).when(action).getDestinationFile(RuntimeEnvironment.application, content);
+
+ action.perform(RuntimeEnvironment.application, catalog);
+
+ verify(catalog, never()).markAsDownloaded(content);
+ verify(temporaryFile, never()).delete();
+ }
+
+ /**
+ * Scenario: Partially downloaded file is already complete.
+ *
+ * Verify that:
+ * * No download request is made
+ * * File is treated as completed and will be verified and extracted
+ * * Content is marked as downloaded in catalog
+ */
+ @Test
+ public void testNoRequestIsSentIfFileIsAlreadyComplete() throws Exception {
+ DownloadContent content = new DownloadContent.Builder()
+ .setKind(DownloadContent.KIND_FONT)
+ .setType(DownloadContent.TYPE_ASSET_ARCHIVE)
+ .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);
+
+ File temporaryFile = mockTemporaryFile(true, 1337L);
+ doReturn(temporaryFile).when(action).createTemporaryFile(RuntimeEnvironment.application, content);
+
+ File destinationFile = mock(File.class);
+ doReturn(destinationFile).when(action).getDestinationFile(RuntimeEnvironment.application, content);
+
+ 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());
+ verify(catalog).markAsDownloaded(content);
+ }
+
+ /**
+ * Scenario: Download is completed but verification (checksum) failed.
+ *
+ * Verify that:
+ * * Downloaded file is deleted
+ * * File will not be extracted
+ * * Content is not marked as downloaded in the catalog
+ */
+ @Test
+ public void testTemporaryFileWillBeDeletedIfVerificationFails() throws Exception {
+ DownloadContent content = new DownloadContent.Builder()
+ .setKind(DownloadContent.KIND_FONT)
+ .setType(DownloadContent.TYPE_ASSET_ARCHIVE)
+ .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);
+ doNothing().when(action).download(any(HttpClient.class), anyString(), any(File.class));
+ doReturn(false).when(action).verify(any(File.class), anyString());
+
+ File temporaryFile = mockTemporaryFile(true, 1337L);
+ doReturn(temporaryFile).when(action).createTemporaryFile(RuntimeEnvironment.application, content);
+
+ File destinationFile = mock(File.class);
+ doReturn(destinationFile).when(action).getDestinationFile(RuntimeEnvironment.application, content);
+
+ action.perform(RuntimeEnvironment.application, catalog);
+
+ verify(temporaryFile).delete();
+ verify(action, never()).extract(any(File.class), any(File.class), anyString());
+ verify(catalog, never()).markAsDownloaded(content);
+ }
+
+ private static File mockTemporaryFile(boolean exists, long length) {
+ File temporaryFile = mock(File.class);
+ doReturn(exists).when(temporaryFile).exists();
+ doReturn(length).when(temporaryFile).length();
+ return temporaryFile;
+ }
+
+ private static HttpClient mockHttpClient(int statusCode, String content) throws Exception {
+ StatusLine status = mock(StatusLine.class);
+ doReturn(statusCode).when(status).getStatusCode();
+
+ HttpEntity entity = mock(HttpEntity.class);
+ doReturn(new ByteArrayInputStream(content.getBytes("UTF-8"))).when(entity).getContent();
+
+ HttpResponse response = mock(HttpResponse.class);
+ doReturn(status).when(response).getStatusLine();
+ doReturn(entity).when(response).getEntity();
+
+ HttpClient client = mock(HttpClient.class);
+ doReturn(response).when(client).execute(any(HttpUriRequest.class));
+
+ return client;
+ }
}