From 19a252a341bfd4abc14334ff2ebb5bae434fec8d Mon Sep 17 00:00:00 2001 From: Vasiliy Zukanov Date: Sat, 20 Aug 2022 14:45:17 +0300 Subject: [PATCH 1/6] Bugfix: fixes #1155 - supporting concurrent download of the same ParseFile from multiple threads --- .../java/com/parse/ParseFileController.java | 135 ++++++++++-------- .../com/parse/ParseFileControllerTest.java | 63 ++++++++ 2 files changed, 140 insertions(+), 58 deletions(-) diff --git a/parse/src/main/java/com/parse/ParseFileController.java b/parse/src/main/java/com/parse/ParseFileController.java index df8ea2e69..849422d87 100644 --- a/parse/src/main/java/com/parse/ParseFileController.java +++ b/parse/src/main/java/com/parse/ParseFileController.java @@ -12,6 +12,8 @@ import com.parse.http.ParseHttpRequest; import java.io.File; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import java.util.concurrent.CancellationException; import org.json.JSONObject; @@ -21,6 +23,8 @@ class ParseFileController { private final Object lock = new Object(); private final ParseHttpClient restClient; private final File cachePath; + private final List currentlyDownloadedFilesNames = new ArrayList<>(); + private ParseHttpClient fileClient; @@ -168,64 +172,79 @@ public Task fetchAsync( if (cancellationToken != null && cancellationToken.isCancelled()) { return Task.cancelled(); } - final File cacheFile = getCacheFile(state); - return Task.call(cacheFile::exists, ParseExecutors.io()) - .continueWithTask( - task -> { - boolean result = task.getResult(); - if (result) { - return Task.forResult(cacheFile); - } - if (cancellationToken != null && cancellationToken.isCancelled()) { - return Task.cancelled(); - } + return Task.call(() -> { + final File cacheFile = getCacheFile(state); + + synchronized (lock) { + if (currentlyDownloadedFilesNames.contains(state.name())) { + while (currentlyDownloadedFilesNames.contains(state.name())) { + lock.wait(); + } + } + + if (cacheFile.exists()) { + return cacheFile; + } else { + currentlyDownloadedFilesNames.add(state.name()); + } + } + + try { + if (cancellationToken != null && cancellationToken.isCancelled()) { + throw new CancellationException(); + } + + // 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 ParseFileRequest request = + new ParseFileRequest( + ParseHttpRequest.Method.GET, state.url(), tempFile); + + // We do not need to delete the temp file since we always try to + // overwrite it + Task downloadTask = request.executeAsync( + fileClient(), + null, + downloadProgressCallback, + cancellationToken + ); + ParseTaskUtils.wait(downloadTask); + + // If the top-level task was cancelled, don't + // actually set the data -- just move on. + if (cancellationToken != null && cancellationToken.isCancelled()) { + throw new CancellationException(); + } + if (downloadTask.isFaulted()) { + ParseFileUtils.deleteQuietly(tempFile); + throw new RuntimeException(downloadTask.getError()); + } + + // Since we give the cacheFile pointer to + // developers, it is not safe to guarantee + // cacheFile always does not exist here, so it is + // better to delete it manually, + // otherwise moveFile may throw an exception. + ParseFileUtils.deleteQuietly(cacheFile); + ParseFileUtils.moveFile(tempFile, cacheFile); + return cacheFile; + } finally { + synchronized (lock) { + currentlyDownloadedFilesNames.remove(state.name()); + lock.notifyAll(); + } + } - // 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 ParseFileRequest request = - new ParseFileRequest( - 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( - fileClient(), - null, - downloadProgressCallback, - cancellationToken) - .continueWithTask( - task1 -> { - // If the top-level task was cancelled, don't - // actually set the data -- just move on. - if (cancellationToken != null - && cancellationToken.isCancelled()) { - throw new CancellationException(); - } - if (task1.isFaulted()) { - ParseFileUtils.deleteQuietly(tempFile); - return task1.cast(); - } - - // Since we give the cacheFile pointer to - // developers, it is not safe to guarantee - // cacheFile always does not exist here, so it is - // better to delete it manually, - // otherwise moveFile may throw an exception. - ParseFileUtils.deleteQuietly(cacheFile); - ParseFileUtils.moveFile(tempFile, cacheFile); - return Task.forResult(cacheFile); - }, - ParseExecutors.io()); - }); + }, ParseExecutors.io()); } } diff --git a/parse/src/test/java/com/parse/ParseFileControllerTest.java b/parse/src/test/java/com/parse/ParseFileControllerTest.java index 39a0d2222..f15191b7d 100644 --- a/parse/src/test/java/com/parse/ParseFileControllerTest.java +++ b/parse/src/test/java/com/parse/ParseFileControllerTest.java @@ -28,6 +28,9 @@ import java.io.IOException; import java.net.MalformedURLException; import java.net.URL; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.atomic.AtomicReference; + import org.json.JSONObject; import org.junit.After; import org.junit.Before; @@ -341,5 +344,65 @@ public void testFetchAsyncFailure() throws Exception { assertFalse(controller.getTempFile(state).exists()); } + + @Test + public void testFetchAsyncConcurrentCallsSuccess() throws Exception { + byte[] data = "hello".getBytes(); + ParseHttpResponse mockResponse = + new ParseHttpResponse.Builder() + .setStatusCode(200) + .setTotalSize((long) data.length) + .setContent(new ByteArrayInputStream(data)) + .build(); + + ParseHttpClient fileClient = mock(ParseHttpClient.class); + when(fileClient.execute(any(ParseHttpRequest.class))).thenReturn(mockResponse); + // Make sure cache dir does not exist + File root = new File(temporaryFolder.getRoot(), "cache"); + assertFalse(root.exists()); + ParseFileController controller = new ParseFileController(null, root).fileClient(fileClient); + + ParseFile.State state = new ParseFile.State.Builder().name("file_name").url("url").build(); + + CountDownLatch countDownLatch = new CountDownLatch(2); + AtomicReference file1Ref = new AtomicReference<>(); + AtomicReference file2Ref = new AtomicReference<>(); + + new Thread(() -> { + try { + file1Ref.set(ParseTaskUtils.wait(controller.fetchAsync(state, null, null, null))); + } catch (ParseException e) { + throw new RuntimeException(e); + } finally { + countDownLatch.countDown(); + } + }).start(); + + new Thread(() -> { + try { + file2Ref.set(ParseTaskUtils.wait(controller.fetchAsync(state, null, null, null))); + } catch (ParseException e) { + throw new RuntimeException(e); + } finally { + countDownLatch.countDown(); + } + }).start(); + + countDownLatch.await(); + + File result1 = file1Ref.get(); + File result2 = file2Ref.get(); + + + assertTrue(result1.exists()); + assertEquals("hello", ParseFileUtils.readFileToString(result1, "UTF-8")); + + assertTrue(result2.exists()); + assertEquals("hello", ParseFileUtils.readFileToString(result2, "UTF-8")); + + verify(fileClient, times(1)).execute(any(ParseHttpRequest.class)); + assertFalse(controller.getTempFile(state).exists()); + } + // endregion } From d2135c2e316fc75df332e3140be80ba0c21701d5 Mon Sep 17 00:00:00 2001 From: Manuel <5673677+mtrezza@users.noreply.github.com> Date: Mon, 22 Aug 2022 19:44:38 +0200 Subject: [PATCH 2/6] Update parse/src/test/java/com/parse/ParseFileControllerTest.java --- parse/src/test/java/com/parse/ParseFileControllerTest.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/parse/src/test/java/com/parse/ParseFileControllerTest.java b/parse/src/test/java/com/parse/ParseFileControllerTest.java index f15191b7d..a2488c89d 100644 --- a/parse/src/test/java/com/parse/ParseFileControllerTest.java +++ b/parse/src/test/java/com/parse/ParseFileControllerTest.java @@ -392,14 +392,11 @@ public void testFetchAsyncConcurrentCallsSuccess() throws Exception { File result1 = file1Ref.get(); File result2 = file2Ref.get(); - - + assertTrue(result1.exists()); assertEquals("hello", ParseFileUtils.readFileToString(result1, "UTF-8")); - assertTrue(result2.exists()); assertEquals("hello", ParseFileUtils.readFileToString(result2, "UTF-8")); - verify(fileClient, times(1)).execute(any(ParseHttpRequest.class)); assertFalse(controller.getTempFile(state).exists()); } From d5c50f4ca6eff76248e451cac57c36463058c526 Mon Sep 17 00:00:00 2001 From: Manuel <5673677+mtrezza@users.noreply.github.com> Date: Mon, 22 Aug 2022 19:44:43 +0200 Subject: [PATCH 3/6] Update parse/src/test/java/com/parse/ParseFileControllerTest.java --- parse/src/test/java/com/parse/ParseFileControllerTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/parse/src/test/java/com/parse/ParseFileControllerTest.java b/parse/src/test/java/com/parse/ParseFileControllerTest.java index a2488c89d..a9eb55d3c 100644 --- a/parse/src/test/java/com/parse/ParseFileControllerTest.java +++ b/parse/src/test/java/com/parse/ParseFileControllerTest.java @@ -361,9 +361,7 @@ public void testFetchAsyncConcurrentCallsSuccess() throws Exception { File root = new File(temporaryFolder.getRoot(), "cache"); assertFalse(root.exists()); ParseFileController controller = new ParseFileController(null, root).fileClient(fileClient); - ParseFile.State state = new ParseFile.State.Builder().name("file_name").url("url").build(); - CountDownLatch countDownLatch = new CountDownLatch(2); AtomicReference file1Ref = new AtomicReference<>(); AtomicReference file2Ref = new AtomicReference<>(); From 8d8a00d3d73c17f5c69f04ef60dd9339febc803a Mon Sep 17 00:00:00 2001 From: Romman Sabbir Date: Tue, 23 Aug 2022 19:15:24 +0600 Subject: [PATCH 4/6] ci failing : fixed spotless violations --- .../java/com/parse/ParseFileController.java | 144 +++++++++--------- .../com/parse/ParseFileControllerTest.java | 60 ++++---- 2 files changed, 105 insertions(+), 99 deletions(-) diff --git a/parse/src/main/java/com/parse/ParseFileController.java b/parse/src/main/java/com/parse/ParseFileController.java index 849422d87..084ac7e40 100644 --- a/parse/src/main/java/com/parse/ParseFileController.java +++ b/parse/src/main/java/com/parse/ParseFileController.java @@ -25,7 +25,6 @@ class ParseFileController { private final File cachePath; private final List currentlyDownloadedFilesNames = new ArrayList<>(); - private ParseHttpClient fileClient; public ParseFileController(ParseHttpClient restClient, File cachePath) { @@ -172,79 +171,80 @@ public Task fetchAsync( if (cancellationToken != null && cancellationToken.isCancelled()) { return Task.cancelled(); } - return Task.call(() -> { - final File cacheFile = getCacheFile(state); + return Task.call( + () -> { + final File cacheFile = getCacheFile(state); + + synchronized (lock) { + if (currentlyDownloadedFilesNames.contains(state.name())) { + while (currentlyDownloadedFilesNames.contains(state.name())) { + lock.wait(); + } + } - synchronized (lock) { - if (currentlyDownloadedFilesNames.contains(state.name())) { - while (currentlyDownloadedFilesNames.contains(state.name())) { - lock.wait(); + if (cacheFile.exists()) { + return cacheFile; + } else { + currentlyDownloadedFilesNames.add(state.name()); + } } - } - - if (cacheFile.exists()) { - return cacheFile; - } else { - currentlyDownloadedFilesNames.add(state.name()); - } - } - try { - if (cancellationToken != null && cancellationToken.isCancelled()) { - throw new CancellationException(); - } - - // 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 ParseFileRequest request = - new ParseFileRequest( - ParseHttpRequest.Method.GET, state.url(), tempFile); - - // We do not need to delete the temp file since we always try to - // overwrite it - Task downloadTask = request.executeAsync( - fileClient(), - null, - downloadProgressCallback, - cancellationToken - ); - ParseTaskUtils.wait(downloadTask); - - // If the top-level task was cancelled, don't - // actually set the data -- just move on. - if (cancellationToken != null && cancellationToken.isCancelled()) { - throw new CancellationException(); - } - if (downloadTask.isFaulted()) { - ParseFileUtils.deleteQuietly(tempFile); - throw new RuntimeException(downloadTask.getError()); - } - - // Since we give the cacheFile pointer to - // developers, it is not safe to guarantee - // cacheFile always does not exist here, so it is - // better to delete it manually, - // otherwise moveFile may throw an exception. - ParseFileUtils.deleteQuietly(cacheFile); - ParseFileUtils.moveFile(tempFile, cacheFile); - return cacheFile; - } finally { - synchronized (lock) { - currentlyDownloadedFilesNames.remove(state.name()); - lock.notifyAll(); - } - } - - }, ParseExecutors.io()); + try { + if (cancellationToken != null && cancellationToken.isCancelled()) { + throw new CancellationException(); + } + + // 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 ParseFileRequest request = + new ParseFileRequest( + ParseHttpRequest.Method.GET, state.url(), tempFile); + + // We do not need to delete the temp file since we always try to + // overwrite it + Task downloadTask = + request.executeAsync( + fileClient(), + null, + downloadProgressCallback, + cancellationToken); + ParseTaskUtils.wait(downloadTask); + + // If the top-level task was cancelled, don't + // actually set the data -- just move on. + if (cancellationToken != null && cancellationToken.isCancelled()) { + throw new CancellationException(); + } + if (downloadTask.isFaulted()) { + ParseFileUtils.deleteQuietly(tempFile); + throw new RuntimeException(downloadTask.getError()); + } + + // Since we give the cacheFile pointer to + // developers, it is not safe to guarantee + // cacheFile always does not exist here, so it is + // better to delete it manually, + // otherwise moveFile may throw an exception. + ParseFileUtils.deleteQuietly(cacheFile); + ParseFileUtils.moveFile(tempFile, cacheFile); + return cacheFile; + } finally { + synchronized (lock) { + currentlyDownloadedFilesNames.remove(state.name()); + lock.notifyAll(); + } + } + }, + ParseExecutors.io()); } } diff --git a/parse/src/test/java/com/parse/ParseFileControllerTest.java b/parse/src/test/java/com/parse/ParseFileControllerTest.java index a9eb55d3c..1dd65e151 100644 --- a/parse/src/test/java/com/parse/ParseFileControllerTest.java +++ b/parse/src/test/java/com/parse/ParseFileControllerTest.java @@ -30,7 +30,6 @@ import java.net.URL; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicReference; - import org.json.JSONObject; import org.junit.After; import org.junit.Before; @@ -344,16 +343,15 @@ public void testFetchAsyncFailure() throws Exception { assertFalse(controller.getTempFile(state).exists()); } - @Test public void testFetchAsyncConcurrentCallsSuccess() throws Exception { byte[] data = "hello".getBytes(); ParseHttpResponse mockResponse = - new ParseHttpResponse.Builder() - .setStatusCode(200) - .setTotalSize((long) data.length) - .setContent(new ByteArrayInputStream(data)) - .build(); + new ParseHttpResponse.Builder() + .setStatusCode(200) + .setTotalSize((long) data.length) + .setContent(new ByteArrayInputStream(data)) + .build(); ParseHttpClient fileClient = mock(ParseHttpClient.class); when(fileClient.execute(any(ParseHttpRequest.class))).thenReturn(mockResponse); @@ -366,31 +364,39 @@ public void testFetchAsyncConcurrentCallsSuccess() throws Exception { AtomicReference file1Ref = new AtomicReference<>(); AtomicReference file2Ref = new AtomicReference<>(); - new Thread(() -> { - try { - file1Ref.set(ParseTaskUtils.wait(controller.fetchAsync(state, null, null, null))); - } catch (ParseException e) { - throw new RuntimeException(e); - } finally { - countDownLatch.countDown(); - } - }).start(); - - new Thread(() -> { - try { - file2Ref.set(ParseTaskUtils.wait(controller.fetchAsync(state, null, null, null))); - } catch (ParseException e) { - throw new RuntimeException(e); - } finally { - countDownLatch.countDown(); - } - }).start(); + new Thread( + () -> { + try { + file1Ref.set( + ParseTaskUtils.wait( + controller.fetchAsync(state, null, null, null))); + } catch (ParseException e) { + throw new RuntimeException(e); + } finally { + countDownLatch.countDown(); + } + }) + .start(); + + new Thread( + () -> { + try { + file2Ref.set( + ParseTaskUtils.wait( + controller.fetchAsync(state, null, null, null))); + } catch (ParseException e) { + throw new RuntimeException(e); + } finally { + countDownLatch.countDown(); + } + }) + .start(); countDownLatch.await(); File result1 = file1Ref.get(); File result2 = file2Ref.get(); - + assertTrue(result1.exists()); assertEquals("hello", ParseFileUtils.readFileToString(result1, "UTF-8")); assertTrue(result2.exists()); From c19946b5068f507955a4d2e381a1bbe7de708e62 Mon Sep 17 00:00:00 2001 From: Romman Sabbir Date: Tue, 23 Aug 2022 19:34:51 +0600 Subject: [PATCH 5/6] Revert "ci failing : fixed spotless violations" This reverts commit 8d8a00d3 --- .../java/com/parse/ParseFileController.java | 144 +++++++++--------- .../com/parse/ParseFileControllerTest.java | 60 ++++---- 2 files changed, 99 insertions(+), 105 deletions(-) diff --git a/parse/src/main/java/com/parse/ParseFileController.java b/parse/src/main/java/com/parse/ParseFileController.java index 084ac7e40..849422d87 100644 --- a/parse/src/main/java/com/parse/ParseFileController.java +++ b/parse/src/main/java/com/parse/ParseFileController.java @@ -25,6 +25,7 @@ class ParseFileController { private final File cachePath; private final List currentlyDownloadedFilesNames = new ArrayList<>(); + private ParseHttpClient fileClient; public ParseFileController(ParseHttpClient restClient, File cachePath) { @@ -171,80 +172,79 @@ public Task fetchAsync( if (cancellationToken != null && cancellationToken.isCancelled()) { return Task.cancelled(); } - return Task.call( - () -> { - final File cacheFile = getCacheFile(state); - - synchronized (lock) { - if (currentlyDownloadedFilesNames.contains(state.name())) { - while (currentlyDownloadedFilesNames.contains(state.name())) { - lock.wait(); - } - } + return Task.call(() -> { + final File cacheFile = getCacheFile(state); - if (cacheFile.exists()) { - return cacheFile; - } else { - currentlyDownloadedFilesNames.add(state.name()); - } + synchronized (lock) { + if (currentlyDownloadedFilesNames.contains(state.name())) { + while (currentlyDownloadedFilesNames.contains(state.name())) { + lock.wait(); } + } - try { - if (cancellationToken != null && cancellationToken.isCancelled()) { - throw new CancellationException(); - } - - // 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 ParseFileRequest request = - new ParseFileRequest( - ParseHttpRequest.Method.GET, state.url(), tempFile); - - // We do not need to delete the temp file since we always try to - // overwrite it - Task downloadTask = - request.executeAsync( - fileClient(), - null, - downloadProgressCallback, - cancellationToken); - ParseTaskUtils.wait(downloadTask); - - // If the top-level task was cancelled, don't - // actually set the data -- just move on. - if (cancellationToken != null && cancellationToken.isCancelled()) { - throw new CancellationException(); - } - if (downloadTask.isFaulted()) { - ParseFileUtils.deleteQuietly(tempFile); - throw new RuntimeException(downloadTask.getError()); - } - - // Since we give the cacheFile pointer to - // developers, it is not safe to guarantee - // cacheFile always does not exist here, so it is - // better to delete it manually, - // otherwise moveFile may throw an exception. - ParseFileUtils.deleteQuietly(cacheFile); - ParseFileUtils.moveFile(tempFile, cacheFile); - return cacheFile; - } finally { - synchronized (lock) { - currentlyDownloadedFilesNames.remove(state.name()); - lock.notifyAll(); - } - } - }, - ParseExecutors.io()); + if (cacheFile.exists()) { + return cacheFile; + } else { + currentlyDownloadedFilesNames.add(state.name()); + } + } + + try { + if (cancellationToken != null && cancellationToken.isCancelled()) { + throw new CancellationException(); + } + + // 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 ParseFileRequest request = + new ParseFileRequest( + ParseHttpRequest.Method.GET, state.url(), tempFile); + + // We do not need to delete the temp file since we always try to + // overwrite it + Task downloadTask = request.executeAsync( + fileClient(), + null, + downloadProgressCallback, + cancellationToken + ); + ParseTaskUtils.wait(downloadTask); + + // If the top-level task was cancelled, don't + // actually set the data -- just move on. + if (cancellationToken != null && cancellationToken.isCancelled()) { + throw new CancellationException(); + } + if (downloadTask.isFaulted()) { + ParseFileUtils.deleteQuietly(tempFile); + throw new RuntimeException(downloadTask.getError()); + } + + // Since we give the cacheFile pointer to + // developers, it is not safe to guarantee + // cacheFile always does not exist here, so it is + // better to delete it manually, + // otherwise moveFile may throw an exception. + ParseFileUtils.deleteQuietly(cacheFile); + ParseFileUtils.moveFile(tempFile, cacheFile); + return cacheFile; + } finally { + synchronized (lock) { + currentlyDownloadedFilesNames.remove(state.name()); + lock.notifyAll(); + } + } + + }, ParseExecutors.io()); } } diff --git a/parse/src/test/java/com/parse/ParseFileControllerTest.java b/parse/src/test/java/com/parse/ParseFileControllerTest.java index 1dd65e151..a9eb55d3c 100644 --- a/parse/src/test/java/com/parse/ParseFileControllerTest.java +++ b/parse/src/test/java/com/parse/ParseFileControllerTest.java @@ -30,6 +30,7 @@ import java.net.URL; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicReference; + import org.json.JSONObject; import org.junit.After; import org.junit.Before; @@ -343,15 +344,16 @@ public void testFetchAsyncFailure() throws Exception { assertFalse(controller.getTempFile(state).exists()); } + @Test public void testFetchAsyncConcurrentCallsSuccess() throws Exception { byte[] data = "hello".getBytes(); ParseHttpResponse mockResponse = - new ParseHttpResponse.Builder() - .setStatusCode(200) - .setTotalSize((long) data.length) - .setContent(new ByteArrayInputStream(data)) - .build(); + new ParseHttpResponse.Builder() + .setStatusCode(200) + .setTotalSize((long) data.length) + .setContent(new ByteArrayInputStream(data)) + .build(); ParseHttpClient fileClient = mock(ParseHttpClient.class); when(fileClient.execute(any(ParseHttpRequest.class))).thenReturn(mockResponse); @@ -364,39 +366,31 @@ public void testFetchAsyncConcurrentCallsSuccess() throws Exception { AtomicReference file1Ref = new AtomicReference<>(); AtomicReference file2Ref = new AtomicReference<>(); - new Thread( - () -> { - try { - file1Ref.set( - ParseTaskUtils.wait( - controller.fetchAsync(state, null, null, null))); - } catch (ParseException e) { - throw new RuntimeException(e); - } finally { - countDownLatch.countDown(); - } - }) - .start(); - - new Thread( - () -> { - try { - file2Ref.set( - ParseTaskUtils.wait( - controller.fetchAsync(state, null, null, null))); - } catch (ParseException e) { - throw new RuntimeException(e); - } finally { - countDownLatch.countDown(); - } - }) - .start(); + new Thread(() -> { + try { + file1Ref.set(ParseTaskUtils.wait(controller.fetchAsync(state, null, null, null))); + } catch (ParseException e) { + throw new RuntimeException(e); + } finally { + countDownLatch.countDown(); + } + }).start(); + + new Thread(() -> { + try { + file2Ref.set(ParseTaskUtils.wait(controller.fetchAsync(state, null, null, null))); + } catch (ParseException e) { + throw new RuntimeException(e); + } finally { + countDownLatch.countDown(); + } + }).start(); countDownLatch.await(); File result1 = file1Ref.get(); File result2 = file2Ref.get(); - + assertTrue(result1.exists()); assertEquals("hello", ParseFileUtils.readFileToString(result1, "UTF-8")); assertTrue(result2.exists()); From ec87160a0836137b1392996141d46cc195395e3b Mon Sep 17 00:00:00 2001 From: Romman Sabbir Date: Tue, 23 Aug 2022 19:36:03 +0600 Subject: [PATCH 6/6] fix: ci failing on PR #1179 --- .../java/com/parse/ParseFileController.java | 144 +++++++++--------- .../com/parse/ParseFileControllerTest.java | 60 ++++---- 2 files changed, 105 insertions(+), 99 deletions(-) diff --git a/parse/src/main/java/com/parse/ParseFileController.java b/parse/src/main/java/com/parse/ParseFileController.java index 849422d87..084ac7e40 100644 --- a/parse/src/main/java/com/parse/ParseFileController.java +++ b/parse/src/main/java/com/parse/ParseFileController.java @@ -25,7 +25,6 @@ class ParseFileController { private final File cachePath; private final List currentlyDownloadedFilesNames = new ArrayList<>(); - private ParseHttpClient fileClient; public ParseFileController(ParseHttpClient restClient, File cachePath) { @@ -172,79 +171,80 @@ public Task fetchAsync( if (cancellationToken != null && cancellationToken.isCancelled()) { return Task.cancelled(); } - return Task.call(() -> { - final File cacheFile = getCacheFile(state); + return Task.call( + () -> { + final File cacheFile = getCacheFile(state); + + synchronized (lock) { + if (currentlyDownloadedFilesNames.contains(state.name())) { + while (currentlyDownloadedFilesNames.contains(state.name())) { + lock.wait(); + } + } - synchronized (lock) { - if (currentlyDownloadedFilesNames.contains(state.name())) { - while (currentlyDownloadedFilesNames.contains(state.name())) { - lock.wait(); + if (cacheFile.exists()) { + return cacheFile; + } else { + currentlyDownloadedFilesNames.add(state.name()); + } } - } - - if (cacheFile.exists()) { - return cacheFile; - } else { - currentlyDownloadedFilesNames.add(state.name()); - } - } - try { - if (cancellationToken != null && cancellationToken.isCancelled()) { - throw new CancellationException(); - } - - // 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 ParseFileRequest request = - new ParseFileRequest( - ParseHttpRequest.Method.GET, state.url(), tempFile); - - // We do not need to delete the temp file since we always try to - // overwrite it - Task downloadTask = request.executeAsync( - fileClient(), - null, - downloadProgressCallback, - cancellationToken - ); - ParseTaskUtils.wait(downloadTask); - - // If the top-level task was cancelled, don't - // actually set the data -- just move on. - if (cancellationToken != null && cancellationToken.isCancelled()) { - throw new CancellationException(); - } - if (downloadTask.isFaulted()) { - ParseFileUtils.deleteQuietly(tempFile); - throw new RuntimeException(downloadTask.getError()); - } - - // Since we give the cacheFile pointer to - // developers, it is not safe to guarantee - // cacheFile always does not exist here, so it is - // better to delete it manually, - // otherwise moveFile may throw an exception. - ParseFileUtils.deleteQuietly(cacheFile); - ParseFileUtils.moveFile(tempFile, cacheFile); - return cacheFile; - } finally { - synchronized (lock) { - currentlyDownloadedFilesNames.remove(state.name()); - lock.notifyAll(); - } - } - - }, ParseExecutors.io()); + try { + if (cancellationToken != null && cancellationToken.isCancelled()) { + throw new CancellationException(); + } + + // 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 ParseFileRequest request = + new ParseFileRequest( + ParseHttpRequest.Method.GET, state.url(), tempFile); + + // We do not need to delete the temp file since we always try to + // overwrite it + Task downloadTask = + request.executeAsync( + fileClient(), + null, + downloadProgressCallback, + cancellationToken); + ParseTaskUtils.wait(downloadTask); + + // If the top-level task was cancelled, don't + // actually set the data -- just move on. + if (cancellationToken != null && cancellationToken.isCancelled()) { + throw new CancellationException(); + } + if (downloadTask.isFaulted()) { + ParseFileUtils.deleteQuietly(tempFile); + throw new RuntimeException(downloadTask.getError()); + } + + // Since we give the cacheFile pointer to + // developers, it is not safe to guarantee + // cacheFile always does not exist here, so it is + // better to delete it manually, + // otherwise moveFile may throw an exception. + ParseFileUtils.deleteQuietly(cacheFile); + ParseFileUtils.moveFile(tempFile, cacheFile); + return cacheFile; + } finally { + synchronized (lock) { + currentlyDownloadedFilesNames.remove(state.name()); + lock.notifyAll(); + } + } + }, + ParseExecutors.io()); } } diff --git a/parse/src/test/java/com/parse/ParseFileControllerTest.java b/parse/src/test/java/com/parse/ParseFileControllerTest.java index a9eb55d3c..1dd65e151 100644 --- a/parse/src/test/java/com/parse/ParseFileControllerTest.java +++ b/parse/src/test/java/com/parse/ParseFileControllerTest.java @@ -30,7 +30,6 @@ import java.net.URL; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicReference; - import org.json.JSONObject; import org.junit.After; import org.junit.Before; @@ -344,16 +343,15 @@ public void testFetchAsyncFailure() throws Exception { assertFalse(controller.getTempFile(state).exists()); } - @Test public void testFetchAsyncConcurrentCallsSuccess() throws Exception { byte[] data = "hello".getBytes(); ParseHttpResponse mockResponse = - new ParseHttpResponse.Builder() - .setStatusCode(200) - .setTotalSize((long) data.length) - .setContent(new ByteArrayInputStream(data)) - .build(); + new ParseHttpResponse.Builder() + .setStatusCode(200) + .setTotalSize((long) data.length) + .setContent(new ByteArrayInputStream(data)) + .build(); ParseHttpClient fileClient = mock(ParseHttpClient.class); when(fileClient.execute(any(ParseHttpRequest.class))).thenReturn(mockResponse); @@ -366,31 +364,39 @@ public void testFetchAsyncConcurrentCallsSuccess() throws Exception { AtomicReference file1Ref = new AtomicReference<>(); AtomicReference file2Ref = new AtomicReference<>(); - new Thread(() -> { - try { - file1Ref.set(ParseTaskUtils.wait(controller.fetchAsync(state, null, null, null))); - } catch (ParseException e) { - throw new RuntimeException(e); - } finally { - countDownLatch.countDown(); - } - }).start(); - - new Thread(() -> { - try { - file2Ref.set(ParseTaskUtils.wait(controller.fetchAsync(state, null, null, null))); - } catch (ParseException e) { - throw new RuntimeException(e); - } finally { - countDownLatch.countDown(); - } - }).start(); + new Thread( + () -> { + try { + file1Ref.set( + ParseTaskUtils.wait( + controller.fetchAsync(state, null, null, null))); + } catch (ParseException e) { + throw new RuntimeException(e); + } finally { + countDownLatch.countDown(); + } + }) + .start(); + + new Thread( + () -> { + try { + file2Ref.set( + ParseTaskUtils.wait( + controller.fetchAsync(state, null, null, null))); + } catch (ParseException e) { + throw new RuntimeException(e); + } finally { + countDownLatch.countDown(); + } + }) + .start(); countDownLatch.await(); File result1 = file1Ref.get(); File result2 = file2Ref.get(); - + assertTrue(result1.exists()); assertEquals("hello", ParseFileUtils.readFileToString(result1, "UTF-8")); assertTrue(result2.exists());