Add slf4j, optimize some code and fix reported lint problem

Author: Mattia Bertorello

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

@@ -31,6 +31,8 @@
 
 import org.apache.commons.compress.utils.IOUtils;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import processing.app.BaseNoGui;
 import processing.app.helpers.FileUtils;
 
@@ -44,11 +46,9 @@
 import java.util.Observable;
 import java.util.Optional;
 import java.util.logging.Level;
-import java.util.logging.Logger;
 
 public class FileDownloader extends Observable {
-  private static Logger log = Logger
-    .getLogger(FileDownloader.class.getName());
+  private static Logger log = LoggerFactory.getLogger(FileDownloader.class);
 
   public enum Status {
     CONNECTING, //
@@ -163,7 +163,7 @@ private void downloadFile(boolean noResume) throws InterruptedException {
             return;
           }
         } catch (Exception e) {
-          log.log(Level.WARNING,
+          log.warn(
             "Cannot get the file from the cache, will be downloaded a new one ", e.getCause());
 
         }
@@ -219,6 +219,7 @@ private void downloadFile(boolean noResume) throws InterruptedException {
           throw new Exception("Incomplete download");
       }
       // Set the cache whe it finish to download the file
+      IOUtils.closeQuietly(randomAccessOutputFile);
       fileDownloaderCache.fillCache(outputFile);
       setStatus(Status.COMPLETE);
     } catch (InterruptedException e) {

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

@@ -1,6 +1,8 @@
 package cc.arduino.utils.network;
 
 import com.sun.istack.internal.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import processing.app.PreferencesData;
 import processing.app.helpers.FileUtils;
 
@@ -15,24 +17,20 @@
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Optional;
-import java.util.logging.Logger;
 
 public class FileDownloaderCache {
-  private static Logger log = Logger
-    .getLogger(FileDownloaderCache.class.getName());
-  private final String cacheFolder;
+  private static Logger log = LoggerFactory.getLogger(FileDownloaderCache.class);
   private final URL remoteURL;
   private final Path cacheFilePath;
   // Will be initialized by call the checkIfTheFileIsChanged function
   private String eTag;
 
   // BaseNoGui.getSettingsFolder()
   public FileDownloaderCache(@NotNull String cacheFolder, @NotNull URL remoteURL) {
-    this.cacheFolder = cacheFolder;
     this.remoteURL = remoteURL;
     String[] splitPath = remoteURL.getPath().split("/");
     if (splitPath.length > 0) {
-      this.cacheFilePath = Paths.get(cacheFolder, splitPath[splitPath.length - 1]);
+      this.cacheFilePath = Paths.get(cacheFolder, splitPath);
     } else {
       this.cacheFilePath = null;
     }
@@ -47,13 +45,14 @@ public boolean checkIfTheFileIsChanged()
         try {
           connection.setRequestMethod("HEAD");
         } catch (ProtocolException e) {
-          throw new RuntimeException(e);
+          log.error("Invalid protocol", e);
         }
       });
     final int responseCode = headRequest.getResponseCode();
+    headRequest.disconnect();
     // Something bad is happening return a conservative true to try to download the file
     if (responseCode < 200 || responseCode >= 300) {
-      log.warning("The head request return a bad response code " + responseCode);
+      log.warn("The head request return a bad response code " + responseCode);
       return true;
     }
 
@@ -71,10 +70,8 @@ public boolean checkIfTheFileIsChanged()
   }
 
   public Optional<File> getFileFromCache() {
-    if (Optional.ofNullable(cacheFilePath).isPresent()) {
-      if (Files.exists(cacheFilePath)) {
-        return Optional.of(new File(cacheFilePath.toUri()));
-      }
+    if (Optional.ofNullable(cacheFilePath).isPresent() && Files.exists(cacheFilePath)) {
+      return Optional.of(new File(cacheFilePath.toUri()));
     }
     return Optional.empty();
 
@@ -86,9 +83,8 @@ public void fillCache(File fileToCache) throws Exception {
 
       PreferencesData.set(getPreferencesDataKey(), eTag);
       // If the cache directory does not exist create it
-      final Path cachePath = Paths.get(cacheFolder);
-      if (!Files.exists(cachePath)) {
-        Files.createDirectory(cachePath);
+      if (!Files.exists(cacheFilePath.getParent())) {
+        Files.createDirectories(cacheFilePath.getParent());
       }
       FileUtils.copyFile(fileToCache, cacheFilePath.toFile());
     }

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

@@ -1,7 +1,12 @@
 package cc.arduino.utils.network;
 
 import cc.arduino.net.CustomProxySelector;
+import com.sun.istack.internal.NotNull;
 import org.apache.commons.codec.binary.Base64;
+import org.apache.commons.httpclient.CircularRedirectException;
+import org.apache.commons.lang3.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import processing.app.BaseNoGui;
 import processing.app.PreferencesData;
 
@@ -12,12 +17,9 @@
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.function.Consumer;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 
 public class HttpConnectionManager {
-  private static Logger log = Logger
-    .getLogger(HttpConnectionManager.class.getName());
+  private static Logger log = LoggerFactory.getLogger(HttpConnectionManager.class);
   private final URL requestURL;
   private final String userAgent;
   private int connectTimeout;
@@ -41,38 +43,35 @@ public HttpConnectionManager(URL requestURL) {
         Integer.parseInt(
           PreferencesData.get("http.connection_timeout", "5000"));
     } catch (NumberFormatException e) {
-      log.log(Level.WARNING,
+      log.warn(
         "Cannot parse the http.connection_timeout configuration switch to default 5000 milliseconds", e.getCause());
       this.connectTimeout = 5000;
     }
 
   }
 
-  public HttpURLConnection makeConnection(Consumer<HttpURLConnection> beforeConnection)
-    throws URISyntaxException, NoSuchMethodException, IOException,
-    ScriptException {
+  public HttpURLConnection makeConnection(@NotNull Consumer<HttpURLConnection> beforeConnection)
+    throws IOException, NoSuchMethodException, ScriptException, URISyntaxException {
     return makeConnection(this.requestURL, 0, beforeConnection);
   }
 
-  private HttpURLConnection makeConnection(URL requestURL, int movedTimes,
-                                           Consumer<HttpURLConnection> beforeConnection)
-    throws NoSuchMethodException, ScriptException, IOException,
-    URISyntaxException {
+
+  public HttpURLConnection makeConnection()
+    throws IOException, NoSuchMethodException, ScriptException, URISyntaxException {
+    return makeConnection(this.requestURL, 0, (c) -> {});
+  }
+
+  private HttpURLConnection makeConnection(@NotNull URL requestURL, int movedTimes,
+                                           @NotNull Consumer<HttpURLConnection> beforeConnection) throws IOException, URISyntaxException, ScriptException, NoSuchMethodException {
     log.info("Prepare http request to " + requestURL);
-    if (requestURL == null) {
-      log.warning("Invalid request url is null");
-      throw new RuntimeException("Invalid request url is null");
-    }
     if (movedTimes > 3) {
-      log.warning("Too many redirect " + requestURL);
-      throw new RuntimeException("Too many redirect " + requestURL);
+      log.warn("Too many redirect " + requestURL);
+      throw new CircularRedirectException("Too many redirect " + requestURL);
     }
 
     Proxy proxy = new CustomProxySelector(PreferencesData.getMap())
       .getProxyFor(requestURL.toURI());
-    if ("true".equals(System.getProperty("DEBUG"))) {
-      System.err.println("Using proxy " + proxy);
-    }
+    log.debug("Using proxy {}", proxy);
 
     HttpURLConnection connection = (HttpURLConnection) requestURL
       .openConnection(proxy);
@@ -89,7 +88,7 @@ private HttpURLConnection makeConnection(URL requestURL, int movedTimes,
     beforeConnection.accept(connection);
 
     // Connect
-    log.info("Connect to " + requestURL);
+    log.info("Connect to {} with method {}", requestURL, connection.getRequestMethod());
 
     connection.connect();
     int resp = connection.getResponseCode();
@@ -102,7 +101,7 @@ private HttpURLConnection makeConnection(URL requestURL, int movedTimes,
 
       return this.makeConnection(newUrl, movedTimes + 1, beforeConnection);
     }
-    log.info("The response code was" + resp);
+    log.info("The response code {}, headers {}", resp, StringUtils.join(connection.getHeaderFields()));
 
     return connection;
   }

Diff for: arduino-core/src/processing/app/Platform.java

@@ -23,6 +23,11 @@
 package processing.app;
 
 import cc.arduino.packages.BoardPort;
+import cc.arduino.utils.network.HttpConnectionManager;
+import com.fasterxml.jackson.databind.DeserializationFeature;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import processing.app.debug.TargetBoard;
 import processing.app.debug.TargetPackage;
 import processing.app.debug.TargetPlatform;
@@ -31,19 +36,10 @@
 import javax.swing.*;
 import java.io.File;
 import java.io.IOException;
-import java.util.HashMap;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.ArrayList;
-import java.util.Arrays;
-
-import java.net.URL;
-import java.net.URLConnection;
-import java.net.HttpURLConnection;
-import com.fasterxml.jackson.databind.ObjectMapper;
-import com.fasterxml.jackson.databind.DeserializationFeature;
 import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.URL;
+import java.util.*;
 
 import static processing.app.I18n.tr;
 
@@ -64,7 +60,7 @@
  * know if name is proper Java package syntax.)
  */
 public class Platform {
-
+  private static Logger log = LoggerFactory.getLogger(Platform.class);
 
   /**
    * Set the default L & F. While I enjoy the bounty of the sixteen possible
@@ -207,12 +203,9 @@ public synchronized void getBoardWithMatchingVidPidFromCloud(String vid, String
     mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
     try {
       URL jsonUrl = new URL("http", "api-builder.arduino.cc", 80, "/builder/v1/boards/0x"+vid+"/0x"+pid);
-      URLConnection connection = jsonUrl.openConnection();
-      String userAgent = "ArduinoIDE/" + BaseNoGui.VERSION_NAME + " Java/"
-                  + System.getProperty("java.version");
-      connection.setRequestProperty("User-agent", userAgent);
-      connection.connect();
-      HttpURLConnection httpConnection = (HttpURLConnection) connection;
+
+      final HttpURLConnection httpConnection = new HttpConnectionManager(jsonUrl)
+        .makeConnection();
       int code = httpConnection.getResponseCode();
       if (code == 404) {
         return;
@@ -228,6 +221,7 @@ public synchronized void getBoardWithMatchingVidPidFromCloud(String vid, String
     } catch (Exception e) {
       // No connection no problem, fail silently
       //e.printStackTrace();
+
     }
   }