Reduce download method complexity of FileDownloader class.

Author: Mattia Bertorello

Diff for: arduino-core/src/cc/arduino/contributions/DownloadableContributionsDownloader.java

@@ -126,7 +126,7 @@ public void download(URL url, File tmpFile, Progress progress, String statusText
   }
 
   public void download(URL url, File tmpFile, Progress progress, String statusText, ProgressListener progressListener, boolean noResume, boolean allowCache) throws Exception {
-    FileDownloader downloader = new FileDownloader(url, tmpFile);
+    FileDownloader downloader = new FileDownloader(url, tmpFile, allowCache);
     downloader.addObserver((o, arg) -> {
       FileDownloader me = (FileDownloader) o;
       String msg = "";
@@ -139,7 +139,7 @@ public void download(URL url, File tmpFile, Progress progress, String statusText
       progress.setProgress(me.getProgress());
       progressListener.onProgress(progress);
     });
-    downloader.download(noResume, allowCache);
+    downloader.download(noResume);
     if (!downloader.isCompleted()) {
       throw new Exception(format(tr("Error downloading {0}"), url), downloader.getError());
     }

Diff for: arduino-core/src/cc/arduino/utils/network/FileDownloader.java

@@ -65,14 +65,15 @@ public enum Status {
   private final URL downloadUrl;
 
   private final File outputFile;
-  private InputStream stream = null;
+  private final boolean allowCache;
   private Exception error;
 
-  public FileDownloader(URL url, File file) {
-    downloadUrl = url;
-    outputFile = file;
-    downloaded = 0;
-    initialSize = 0;
+  public FileDownloader(URL url, File file, boolean allowCache) {
+    this.downloadUrl = url;
+    this.outputFile = file;
+    this.allowCache = allowCache;
+    this.downloaded = 0;
+    this.initialSize = 0;
   }
 
   public long getInitialSize() {
@@ -118,7 +119,7 @@ public void setStatus(Status status) {
   }
 
 
-  public void download(boolean noResume, boolean allowCache) throws InterruptedException {
+  public void download(boolean noResume) throws InterruptedException {
     if ("file".equals(downloadUrl.getProtocol())) {
       saveLocalFile();
     } else {
@@ -137,33 +138,18 @@ private void saveLocalFile() {
   }
 
   private void downloadFile(boolean noResume, boolean allowCache) throws InterruptedException {
-    RandomAccessFile randomAccessOutputFile = null;
 
+    InputStream stream = null;
     try {
       setStatus(Status.CONNECTING);
 
       final Optional<FileDownloaderCache.FileCached> fileCached = FileDownloaderCache.getFileCached(downloadUrl);
-
       if (fileCached.isPresent() && fileCached.get().isNotChange()) {
-        try {
-          final Optional<File> fileFromCache =
-            fileCached.get().getFileFromCache();
-          if (fileFromCache.isPresent()) {
-            log.info("No need to download using cached file: {}", fileCached.get());
-            FileUtils.copyFile(fileFromCache.get(), outputFile);
-            setStatus(Status.COMPLETE);
-            return;
-          } else {
-            log.info(
-              "The file in the cache is not in the path or the md5 validation failed: path={}, file exist={}, md5 validation={}",
-              fileCached.get().getLocalPath(), fileCached.get().exists(), fileCached.get().md5Check());
-          }
-        } catch (Exception e) {
-          log.warn(
-            "Cannot get the file from the cache, will be downloaded a new one ", e);
+        final Optional<File> fileFromCache = getFileCached(fileCached.get());
+        if (fileFromCache.isPresent()) {
+          FileUtils.copyFile(fileFromCache.get(), outputFile);
+          setStatus(Status.COMPLETE);
         }
-      } else {
-        log.info("The file is change {}", fileCached);
       }
 
       initialSize = outputFile.length();
@@ -172,19 +158,75 @@ private void downloadFile(boolean noResume, boolean allowCache) throws Interrupt
         Files.deleteIfExists(outputFile.toPath());
         initialSize = 0;
       }
-      // Open file and seek to the end of it
-      randomAccessOutputFile = new RandomAccessFile(outputFile, "rw");
-      randomAccessOutputFile.seek(initialSize);
 
       final HttpURLConnection connection = new HttpConnectionManager(downloadUrl)
         .makeConnection((c) -> setDownloaded(0));
       final int resp = connection.getResponseCode();
 
       if (resp < 200 || resp >= 300) {
-        IOUtils.closeQuietly(randomAccessOutputFile);
         Files.deleteIfExists(outputFile.toPath());
         throw new IOException("Received invalid http status code from server: " + resp);
       }
+      copyStreamToFile(connection);
+
+      if (fileCached.isPresent() && allowCache) {
+        fileCached.get().updateCacheFile(outputFile);
+      }
+      if (!allowCache) {
+        log.info("The file {} was not cached because allow cache is false", downloadUrl);
+      }
+      setStatus(Status.COMPLETE);
+
+    } catch (InterruptedException e) {
+      setStatus(Status.CANCELLED);
+      // lets InterruptedException go up to the caller
+      throw e;
+
+    } catch (SocketTimeoutException e) {
+      setStatus(Status.CONNECTION_TIMEOUT_ERROR);
+      setError(e);
+      log.error("The request went in socket timeout", e);
+
+    } catch (Exception e) {
+      setStatus(Status.ERROR);
+      setError(e);
+      log.error("The request stop", e);
+
+    } finally {
+      IOUtils.closeQuietly(stream);
+    }
+
+
+  }
+
+  private Optional<File> getFileCached(FileDownloaderCache.FileCached fileCached) {
+
+    try {
+      final Optional<File> fileFromCache =
+        fileCached.getFileFromCache();
+      if (fileFromCache.isPresent()) {
+        log.info("No need to download using cached file: {}", fileCached);
+        return fileFromCache;
+      } else {
+        log.info(
+          "The file in the cache is not in the path or the md5 validation failed: path={}, file exist={}, md5 validation={}",
+          fileCached.getLocalPath(), fileCached.exists(), fileCached.md5Check());
+      }
+    } catch (Exception e) {
+      log.warn(
+        "Cannot get the file from the cache, will be downloaded a new one ", e);
+    }
+    log.info("The file is change {}", fileCached);
+    return Optional.empty();
+  }
+
+  private void copyStreamToFile(HttpURLConnection connection) throws Exception {
+    RandomAccessFile randomAccessOutputFile = null;
+    InputStream stream = null;
+    try {
+      // Open file and seek to the end of it
+      randomAccessOutputFile = new RandomAccessFile(outputFile, "rw");
+      randomAccessOutputFile.seek(initialSize);
 
       // Check for valid content length.
       long len = connection.getContentLength();
@@ -193,9 +235,8 @@ private void downloadFile(boolean noResume, boolean allowCache) throws Interrupt
       }
       setStatus(Status.DOWNLOADING);
 
-      synchronized (this) {
-        stream = connection.getInputStream();
-      }
+      stream = connection.getInputStream();
+
       byte[] buffer = new byte[10240];
       while (status == Status.DOWNLOADING) {
         int read = stream.read(buffer);
@@ -217,35 +258,12 @@ private void downloadFile(boolean noResume, boolean allowCache) throws Interrupt
       }
       // Set the cache whe it finish to download the file
       IOUtils.closeQuietly(randomAccessOutputFile);
-      if (fileCached.isPresent() && allowCache) {
-        fileCached.get().updateCacheFile(outputFile);
-      }
-      if (!allowCache) {
-        log.info("The file {} was not cached because allow cache is false", downloadUrl);
-      }
-      setStatus(Status.COMPLETE);
-    } catch (InterruptedException e) {
-      setStatus(Status.CANCELLED);
-      // lets InterruptedException go up to the caller
-      throw e;
-
-    } catch (SocketTimeoutException e) {
-      setStatus(Status.CONNECTION_TIMEOUT_ERROR);
-      setError(e);
-      log.error("The request went in socket timeout", e);
-
-    } catch (Exception e) {
-      setStatus(Status.ERROR);
-      setError(e);
-      log.error("The request stop", e);
 
     } finally {
       IOUtils.closeQuietly(randomAccessOutputFile);
-
-      synchronized (this) {
-        IOUtils.closeQuietly(stream);
-      }
+      IOUtils.closeQuietly(stream);
     }
+
   }
 
   private void setError(Exception e) {