fixing bugs in the API server when downloading files (#2749)

* fixing bugs in the API server when downloading files * addressing comments * cleaning up * refactoring some code
apache · Feb 27, 2018 · ada6052 · ada6052
1 parent 2f26b49
commit ada6052
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 19 deletions.
diff --git a/heron/tools/apiserver/src/java/com/twitter/heron/apiserver/Runtime.java b/heron/tools/apiserver/src/java/com/twitter/heron/apiserver/Runtime.java
@@ -37,6 +37,7 @@
 import com.twitter.heron.apiserver.resources.HeronResource;
 import com.twitter.heron.apiserver.utils.ConfigUtils;
 import com.twitter.heron.apiserver.utils.Logging;
+import com.twitter.heron.apiserver.utils.Utils;
 import com.twitter.heron.spi.common.Config;
 import com.twitter.heron.spi.common.Key;
 
@@ -283,8 +284,6 @@ public static void main(String[] args) throws Exception {
             releaseFile,
             configurationOverrides);
 
-    LOG.info("heronCorePackagePath: " + heronCorePackagePath);
-
     final ResourceConfig config = new ResourceConfig(Resources.get());
     final Server server = new Server(port);
 
@@ -303,9 +302,11 @@ public static void main(String[] args) throws Exception {
     contextHandler.setAttribute(HeronResource.ATTRIBUTE_PORT,
         String.valueOf(port));
     contextHandler.setAttribute(HeronResource.ATTRIBUTE_DOWNLOAD_HOSTNAME,
-        String.valueOf(downloadHostName));
+        Utils.isNotEmpty(downloadHostName)
+            ? String.valueOf(downloadHostName) : null);
     contextHandler.setAttribute(HeronResource.ATTRIBUTE_HERON_CORE_PACKAGE_PATH,
-        String.valueOf(heronCorePackagePath));
+        Utils.isNotEmpty(heronCorePackagePath)
+            ? String.valueOf(heronCorePackagePath) : null);
 
     server.setHandler(contextHandler);
 

diff --git a/heron/tools/apiserver/src/java/com/twitter/heron/apiserver/resources/FileResource.java b/heron/tools/apiserver/src/java/com/twitter/heron/apiserver/resources/FileResource.java
@@ -29,7 +29,6 @@
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.Response;
 
-import org.eclipse.jetty.util.StringUtil;
 import org.glassfish.jersey.media.multipart.FormDataContentDisposition;
 import org.glassfish.jersey.media.multipart.FormDataParam;
 import org.slf4j.Logger;
@@ -126,16 +125,7 @@ public Response downloadFile(final @PathParam("file") String file) {
     Config config = createConfig();
     String uploadDir = config.getStringValue(FILE_SYSTEM_DIRECTORY);
     String filePath = uploadDir + "/" + file;
-    if (!new File(filePath).exists()) {
-      LOG.debug("Download request file " + file + " doesn't exist at " + uploadDir);
-      return Response.status(Response.Status.NOT_FOUND).build();
-    }
-
-    String mimeType = new MimetypesFileTypeMap().getContentType(file);
-    Response.ResponseBuilder rb = Response.ok(file, mimeType);
-    rb.header("content-disposition", "attachment; filename = "
-        + file);
-    return rb.build();
+    return getResponseByFile(filePath);
   }
 
   /**
@@ -145,7 +135,12 @@ public Response downloadFile(final @PathParam("file") String file) {
   @Path("/download/core")
   public Response downloadHeronCore() {
     String corePath = getHeronCorePackagePath();
-    File file = new File(corePath);
+    return getResponseByFile(corePath);
+  }
+
+  private Response getResponseByFile(String filePath) {
+
+    File file = new File(filePath);
     if (!file.exists()) {
       return Response.status(Response.Status.NOT_FOUND).build();
     }
@@ -154,6 +149,7 @@ public Response downloadHeronCore() {
     rb.header("content-disposition", "attachment; filename = "
         + file.getName());
     return rb.build();
+
   }
 
   private Config createConfig() {
@@ -164,11 +160,11 @@ private Config createConfig() {
 
   private String getHostNameOrIP() {
     // Override hostname if provided in flags
-    if (StringUtil.isNotBlank(getDownloadHostName())) {
+    if (Utils.isNotEmpty(getDownloadHostName())) {
       return getDownloadHostName();
-    } else if (StringUtil.isNotBlank(hostname)) {
+    } else if (Utils.isNotEmpty(hostname)) {
       return hostname;
-    } else if (ip != null && StringUtil.isNotBlank(ip.toString())) {
+    } else if (ip != null && !ip.toString().isEmpty()) {
       return ip.toString();
     }
     return "";

diff --git a/heron/tools/apiserver/src/java/com/twitter/heron/apiserver/utils/Utils.java b/heron/tools/apiserver/src/java/com/twitter/heron/apiserver/utils/Utils.java
@@ -44,4 +44,9 @@ public static String createValidationError(String message, List<String> missing)
 
     return node.toString();
   }
+
+  public static boolean isNotEmpty(String str) {
+    return str != null && !str.isEmpty();
+
+  }
 }