fix: ci failing on PR parse-community#1179

Author: rommansabbir

parse/src/main/java/com/parse/ParseFileController.java

@@ -25,7 +25,6 @@ class ParseFileController {
  private final File cachePath;
  private final List<String> currentlyDownloadedFilesNames = new ArrayList<>();
 
-
  private ParseHttpClient fileClient;
 
  public ParseFileController(ParseHttpClient restClient, File cachePath) {
@@ -172,79 +171,80 @@ public Task<File> 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<Void> 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<Void> 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());
  }
 }

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<File> file1Ref = new AtomicReference<>();
  AtomicReference<File> 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());