From 32876a96ba32f17df4e29894153158a61aad7502 Mon Sep 17 00:00:00 2001 From: wangmengyan95 Date: Mon, 24 Aug 2015 19:06:00 -0700 Subject: [PATCH 1/5] Download ParseFile content to file instead of memory --- .../main/java/com/parse/ParseAWSRequest.java | 19 ++++++---- .../java/com/parse/ParseFileController.java | 38 +++++++++++++------ .../java/com/parse/ParseAWSRequestTest.java | 7 +++- .../com/parse/ParseFileControllerTest.java | 5 +++ .../test/java/com/parse/ParseRequestTest.java | 14 +++++-- 5 files changed, 60 insertions(+), 23 deletions(-) diff --git a/Parse/src/main/java/com/parse/ParseAWSRequest.java b/Parse/src/main/java/com/parse/ParseAWSRequest.java index 33d2c1a5e..593f22c3f 100644 --- a/Parse/src/main/java/com/parse/ParseAWSRequest.java +++ b/Parse/src/main/java/com/parse/ParseAWSRequest.java @@ -8,7 +8,8 @@ */ package com.parse; -import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; @@ -18,14 +19,18 @@ * Request returns a byte array of the response and provides a callback the progress of the data * read from the network. */ -/** package */ class ParseAWSRequest extends ParseRequest { +/** package */ class ParseAWSRequest extends ParseRequest { - public ParseAWSRequest(ParseHttpRequest.Method method, String url) { + // The temp file is used to save the ParseFile content when we fetch it from server + private File tempFile; + + public ParseAWSRequest(ParseHttpRequest.Method method, String url, File tempFile) { super(method, url); + this.tempFile = tempFile; } @Override - protected Task onResponseAsync(ParseHttpResponse response, + protected Task onResponseAsync(ParseHttpResponse response, final ProgressCallback downloadProgressCallback) { int statusCode = response.getStatusCode(); if (statusCode >= 200 && statusCode < 300 || statusCode == 304) { @@ -45,20 +50,20 @@ protected Task onResponseAsync(ParseHttpResponse response, InputStream responseStream = null; try { responseStream = response.getContent(); - ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + FileOutputStream tempFileStream = new FileOutputStream(tempFile); int nRead; byte[] data = new byte[32 << 10]; // 32KB while ((nRead = responseStream.read(data, 0, data.length)) != -1) { - buffer.write(data, 0, nRead); + tempFileStream.write(data, 0, nRead); downloadedSize += nRead; if (downloadProgressCallback != null && totalSize != -1) { int progressToReport = Math.round((float) downloadedSize / (float) totalSize * 100.0f); downloadProgressCallback.done(progressToReport); } } - return Task.forResult(buffer.toByteArray()); + return Task.forResult(null); } catch (IOException e) { return Task.forError(e); } finally { diff --git a/Parse/src/main/java/com/parse/ParseFileController.java b/Parse/src/main/java/com/parse/ParseFileController.java index f8a5a5f23..33d013d14 100644 --- a/Parse/src/main/java/com/parse/ParseFileController.java +++ b/Parse/src/main/java/com/parse/ParseFileController.java @@ -56,6 +56,13 @@ public File getCacheFile(ParseFile.State state) { return new File(cachePath, state.name()); } + /* package for tests */ File getTempFile(ParseFile.State state) { + if (state.url() == null) { + return null; + } + return new File(cachePath, ParseDigestUtils.md5(state.url())); + } + public boolean isDataAvailable(ParseFile.State state) { return getCacheFile(state).exists(); } @@ -172,44 +179,53 @@ public Task fetchAsync( if (cancellationToken != null && cancellationToken.isCancelled()) { return Task.cancelled(); } - final File file = getCacheFile(state); + final File cacheFile = getCacheFile(state); return Task.call(new Callable() { @Override public Boolean call() throws Exception { - return file.exists(); + return cacheFile.exists(); } }, Task.BACKGROUND_EXECUTOR).continueWithTask(new Continuation>() { @Override public Task then(Task task) throws Exception { boolean result = task.getResult(); if (result) { - return Task.forResult(file); + return Task.forResult(cacheFile); } if (cancellationToken != null && cancellationToken.isCancelled()) { return Task.cancelled(); } + // Generate the temp file path for caching ParseFile content based on ParseFile's url + // The reason we do not write to the cacheFile directly is because there is no way we can + // verify if a cacheFile is complete or not. If download is interrupted in the middle, next + // time when we download the ParseFile, since cacheFile has already existed, we will return + // this incomplete cacheFile + final File tempFile = getTempFile(state); + // network - final ParseAWSRequest request = new ParseAWSRequest(ParseHttpRequest.Method.GET, state.url()); + final ParseAWSRequest request = + new ParseAWSRequest(ParseHttpRequest.Method.GET, state.url(), tempFile); // TODO(grantland): Stream response directly to file t5042019 return request.executeAsync( awsClient(), null, downloadProgressCallback, - cancellationToken).onSuccess(new Continuation() { + cancellationToken).continueWithTask(new Continuation>() { @Override - public File then(Task task) throws Exception { + public Task then(Task task) throws Exception { // If the top-level task was cancelled, don't actually set the data -- just move on. if (cancellationToken != null && cancellationToken.isCancelled()) { throw new CancellationException(); } - - byte[] data = task.getResult(); - if (data != null) { - ParseFileUtils.writeByteArrayToFile(file, data); + if (task.isFaulted()) { + ParseFileUtils.deleteQuietly(tempFile); + return task.cast(); } - return file; + + ParseFileUtils.moveFile(tempFile, cacheFile); + return Task.forResult(cacheFile); } }); } diff --git a/Parse/src/test/java/com/parse/ParseAWSRequestTest.java b/Parse/src/test/java/com/parse/ParseAWSRequestTest.java index 748992dfe..6f20fe204 100644 --- a/Parse/src/test/java/com/parse/ParseAWSRequestTest.java +++ b/Parse/src/test/java/com/parse/ParseAWSRequestTest.java @@ -12,8 +12,10 @@ import org.junit.Rule; import org.junit.rules.ExpectedException; +import org.junit.rules.TemporaryFolder; import java.io.ByteArrayInputStream; +import java.io.File; import java.io.InputStream; import bolts.Task; @@ -44,8 +46,9 @@ public void test4XXThrowsException() throws Exception { ParseHttpClient mockHttpClient = mock(ParseHttpClient.class); when(mockHttpClient.execute(any(ParseHttpRequest.class))).thenReturn(mockResponse); - ParseAWSRequest request = new ParseAWSRequest(ParseHttpRequest.Method.GET, "http://parse.com"); - Task task = request.executeAsync(mockHttpClient); + ParseAWSRequest request = + new ParseAWSRequest(ParseHttpRequest.Method.GET, "http://parse.com", null); + Task task = request.executeAsync(mockHttpClient); task.waitForCompletion(); assertTrue(task.isFaulted()); diff --git a/Parse/src/test/java/com/parse/ParseFileControllerTest.java b/Parse/src/test/java/com/parse/ParseFileControllerTest.java index 4222fda31..76409b285 100644 --- a/Parse/src/test/java/com/parse/ParseFileControllerTest.java +++ b/Parse/src/test/java/com/parse/ParseFileControllerTest.java @@ -302,6 +302,7 @@ public void testFetchAsyncSuccess() throws Exception { ParseFile.State state = new ParseFile.State.Builder() .name("file_name") + .url("url") .build(); Task task = controller.fetchAsync(state, null, null, null); File result = ParseTaskUtils.wait(task); @@ -309,6 +310,7 @@ public void testFetchAsyncSuccess() throws Exception { verify(awsClient, times(1)).execute(any(ParseHttpRequest.class)); assertTrue(result.exists()); assertEquals("hello", ParseFileUtils.readFileToString(result, "UTF-8")); + assertFalse(controller.getTempFile(state).exists()); } @Test @@ -322,7 +324,9 @@ public void testFetchAsyncFailure() throws Exception { File root = temporaryFolder.getRoot(); ParseFileController controller = new ParseFileController(null, root).awsClient(awsClient); + // We need to set url to make getTempFile() work and check it ParseFile.State state = new ParseFile.State.Builder() + .url("test") .build(); Task task = controller.fetchAsync(state, null, null, null); task.waitForCompletion(); @@ -334,6 +338,7 @@ public void testFetchAsyncFailure() throws Exception { assertThat(error, instanceOf(ParseException.class)); assertEquals(ParseException.CONNECTION_FAILED, ((ParseException) error).getCode()); assertEquals(0, root.listFiles().length); + assertFalse(controller.getTempFile(state).exists()); } //endregion diff --git a/Parse/src/test/java/com/parse/ParseRequestTest.java b/Parse/src/test/java/com/parse/ParseRequestTest.java index 924b9be7e..41ccaf446 100644 --- a/Parse/src/test/java/com/parse/ParseRequestTest.java +++ b/Parse/src/test/java/com/parse/ParseRequestTest.java @@ -12,9 +12,12 @@ import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; import java.io.ByteArrayInputStream; +import java.io.File; import java.io.IOException; import java.util.LinkedList; import java.util.List; @@ -34,6 +37,9 @@ public class ParseRequestTest { private static byte[] data; + @Rule + public TemporaryFolder temporaryFolder = new TemporaryFolder(); + @BeforeClass public static void setUpClass() { char[] chars = new char[64 << 10]; // 64KB @@ -78,13 +84,15 @@ public void testDownloadProgress() throws Exception { ParseHttpClient mockHttpClient = mock(ParseHttpClient.class); when(mockHttpClient.execute(any(ParseHttpRequest.class))).thenReturn(mockResponse); - ParseAWSRequest request = new ParseAWSRequest(ParseHttpRequest.Method.GET, "localhost"); + File tempFile = temporaryFolder.newFile("test"); + ParseAWSRequest request = + new ParseAWSRequest(ParseHttpRequest.Method.GET, "localhost", tempFile); TestProgressCallback downloadProgressCallback = new TestProgressCallback(); - Task task = request.executeAsync(mockHttpClient, null, downloadProgressCallback); + Task task = request.executeAsync(mockHttpClient, null, downloadProgressCallback); task.waitForCompletion(); assertFalse("Download failed: " + task.getError(), task.isFaulted()); - assertEquals(data.length, task.getResult().length); + assertEquals(data.length, ParseFileUtils.readFileToByteArray(tempFile).length); assertProgressCompletedSuccessfully(downloadProgressCallback); } From d4d754f0417486dea8b9916d4b46a041c1064e2a Mon Sep 17 00:00:00 2001 From: wangmengyan95 Date: Mon, 31 Aug 2015 19:34:01 -0700 Subject: [PATCH 2/5] Update getTempFile and delete tempFile before download --- Parse/src/main/java/com/parse/ParseAWSRequest.java | 2 +- Parse/src/main/java/com/parse/ParseFileController.java | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Parse/src/main/java/com/parse/ParseAWSRequest.java b/Parse/src/main/java/com/parse/ParseAWSRequest.java index 593f22c3f..f32e70916 100644 --- a/Parse/src/main/java/com/parse/ParseAWSRequest.java +++ b/Parse/src/main/java/com/parse/ParseAWSRequest.java @@ -22,7 +22,7 @@ /** package */ class ParseAWSRequest extends ParseRequest { // The temp file is used to save the ParseFile content when we fetch it from server - private File tempFile; + private final File tempFile; public ParseAWSRequest(ParseHttpRequest.Method method, String url, File tempFile) { super(method, url); diff --git a/Parse/src/main/java/com/parse/ParseFileController.java b/Parse/src/main/java/com/parse/ParseFileController.java index 33d013d14..42a1bdcda 100644 --- a/Parse/src/main/java/com/parse/ParseFileController.java +++ b/Parse/src/main/java/com/parse/ParseFileController.java @@ -60,7 +60,7 @@ public File getCacheFile(ParseFile.State state) { if (state.url() == null) { return null; } - return new File(cachePath, ParseDigestUtils.md5(state.url())); + return new File(cachePath, state.url()+".tmp"); } public boolean isDataAvailable(ParseFile.State state) { @@ -207,7 +207,6 @@ public Task then(Task task) throws Exception { final ParseAWSRequest request = new ParseAWSRequest(ParseHttpRequest.Method.GET, state.url(), tempFile); - // TODO(grantland): Stream response directly to file t5042019 return request.executeAsync( awsClient(), null, From 228790f8605f5f22319d9031bf6f8a93073743e2 Mon Sep 17 00:00:00 2001 From: wangmengyan95 Date: Wed, 2 Sep 2015 11:54:43 -0700 Subject: [PATCH 3/5] Change to io executor, remove delete temp file and nit changes --- Parse/src/main/java/com/parse/ParseFileController.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Parse/src/main/java/com/parse/ParseFileController.java b/Parse/src/main/java/com/parse/ParseFileController.java index 42a1bdcda..4a7b05151 100644 --- a/Parse/src/main/java/com/parse/ParseFileController.java +++ b/Parse/src/main/java/com/parse/ParseFileController.java @@ -60,7 +60,7 @@ public File getCacheFile(ParseFile.State state) { if (state.url() == null) { return null; } - return new File(cachePath, state.url()+".tmp"); + return new File(cachePath, state.url() + ".tmp"); } public boolean isDataAvailable(ParseFile.State state) { @@ -185,7 +185,7 @@ public Task fetchAsync( public Boolean call() throws Exception { return cacheFile.exists(); } - }, Task.BACKGROUND_EXECUTOR).continueWithTask(new Continuation>() { + }, ParseExecutors.io()).continueWithTask(new Continuation>() { @Override public Task then(Task task) throws Exception { boolean result = task.getResult(); @@ -207,6 +207,7 @@ public Task then(Task task) throws Exception { final ParseAWSRequest request = new ParseAWSRequest(ParseHttpRequest.Method.GET, state.url(), tempFile); + // We do not need to delete the temp file since we always try to overwrite it return request.executeAsync( awsClient(), null, @@ -226,7 +227,7 @@ public Task then(Task task) throws Exception { ParseFileUtils.moveFile(tempFile, cacheFile); return Task.forResult(cacheFile); } - }); + }, ParseExecutors.io()); } }); } From 8c3ed884b876b8b47c5e23f5473a9138927efcf6 Mon Sep 17 00:00:00 2001 From: wangmengyan95 Date: Wed, 2 Sep 2015 17:26:05 -0700 Subject: [PATCH 4/5] Make ParseAWSRequest.onResponse run in io thread --- .../main/java/com/parse/ParseAWSRequest.java | 47 ++++++++++--------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/Parse/src/main/java/com/parse/ParseAWSRequest.java b/Parse/src/main/java/com/parse/ParseAWSRequest.java index f32e70916..a2fb438b5 100644 --- a/Parse/src/main/java/com/parse/ParseAWSRequest.java +++ b/Parse/src/main/java/com/parse/ParseAWSRequest.java @@ -10,8 +10,8 @@ import java.io.File; import java.io.FileOutputStream; -import java.io.IOException; import java.io.InputStream; +import java.util.concurrent.Callable; import bolts.Task; @@ -30,7 +30,7 @@ public ParseAWSRequest(ParseHttpRequest.Method method, String url, File tempFile } @Override - protected Task onResponseAsync(ParseHttpResponse response, + protected Task onResponseAsync(final ParseHttpResponse response, final ProgressCallback downloadProgressCallback) { int statusCode = response.getStatusCode(); if (statusCode >= 200 && statusCode < 300 || statusCode == 304) { @@ -45,29 +45,32 @@ protected Task onResponseAsync(ParseHttpResponse response, return null; } - long totalSize = response.getTotalSize(); - int downloadedSize = 0; - InputStream responseStream = null; - try { - responseStream = response.getContent(); - FileOutputStream tempFileStream = new FileOutputStream(tempFile); + return Task.call(new Callable() { + @Override + public Void call() throws Exception { + long totalSize = response.getTotalSize(); + int downloadedSize = 0; + InputStream responseStream = null; + try { + responseStream = response.getContent(); + FileOutputStream tempFileStream = new FileOutputStream(tempFile); - int nRead; - byte[] data = new byte[32 << 10]; // 32KB + int nRead; + byte[] data = new byte[32 << 10]; // 32KB - while ((nRead = responseStream.read(data, 0, data.length)) != -1) { - tempFileStream.write(data, 0, nRead); - downloadedSize += nRead; - if (downloadProgressCallback != null && totalSize != -1) { - int progressToReport = Math.round((float) downloadedSize / (float) totalSize * 100.0f); - downloadProgressCallback.done(progressToReport); + while ((nRead = responseStream.read(data, 0, data.length)) != -1) { + tempFileStream.write(data, 0, nRead); + downloadedSize += nRead; + if (downloadProgressCallback != null && totalSize != -1) { + int progressToReport = Math.round((float) downloadedSize / (float) totalSize * 100.0f); + downloadProgressCallback.done(progressToReport); + } + } + return null; + } finally { + ParseIOUtils.closeQuietly(responseStream); } } - return Task.forResult(null); - } catch (IOException e) { - return Task.forError(e); - } finally { - ParseIOUtils.closeQuietly(responseStream); - } + }, ParseExecutors.io()); } } From dd591a92cb175b17b0edb3264b35628cf1ed4e53 Mon Sep 17 00:00:00 2001 From: wangmengyan95 Date: Thu, 3 Sep 2015 14:49:38 -0700 Subject: [PATCH 5/5] Rebase master and nit changes --- Parse/src/main/java/com/parse/ParseAWSRequest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Parse/src/main/java/com/parse/ParseAWSRequest.java b/Parse/src/main/java/com/parse/ParseAWSRequest.java index a2fb438b5..19c3b20f5 100644 --- a/Parse/src/main/java/com/parse/ParseAWSRequest.java +++ b/Parse/src/main/java/com/parse/ParseAWSRequest.java @@ -49,7 +49,7 @@ protected Task onResponseAsync(final ParseHttpResponse response, @Override public Void call() throws Exception { long totalSize = response.getTotalSize(); - int downloadedSize = 0; + long downloadedSize = 0; InputStream responseStream = null; try { responseStream = response.getContent(); @@ -62,7 +62,8 @@ public Void call() throws Exception { tempFileStream.write(data, 0, nRead); downloadedSize += nRead; if (downloadProgressCallback != null && totalSize != -1) { - int progressToReport = Math.round((float) downloadedSize / (float) totalSize * 100.0f); + int progressToReport = + Math.round((float) downloadedSize / (float) totalSize * 100.0f); downloadProgressCallback.done(progressToReport); } }