azkaban-aplcache

Throw detailed exceptions and better error messages in ProjectManagerResource

9/19/2016 3:24:15 PM

Details

diff --git a/azkaban-web-server/src/restli/java/azkaban/restli/ProjectManagerResource.java b/azkaban-web-server/src/restli/java/azkaban/restli/ProjectManagerResource.java
index 0c48564..4633496 100644
--- a/azkaban-web-server/src/restli/java/azkaban/restli/ProjectManagerResource.java
+++ b/azkaban-web-server/src/restli/java/azkaban/restli/ProjectManagerResource.java
@@ -19,6 +19,7 @@ import java.io.File;
 import java.io.IOException;
 import java.net.MalformedURLException;
 import java.net.URL;
+import java.util.Map;
 
 import javax.servlet.ServletException;
 
@@ -28,6 +29,7 @@ import org.apache.log4j.Logger;
 import azkaban.project.Project;
 import azkaban.project.ProjectManager;
 import azkaban.project.ProjectManagerException;
+import azkaban.project.validator.ValidationReport;
 import azkaban.user.Permission;
 import azkaban.user.User;
 import azkaban.user.UserManagerException;
@@ -35,10 +37,12 @@ import azkaban.utils.Props;
 import azkaban.utils.Utils;
 import azkaban.webapp.AzkabanWebServer;
 
+import com.linkedin.restli.common.HttpStatus;
 import com.linkedin.restli.server.annotations.Action;
 import com.linkedin.restli.server.annotations.ActionParam;
 import com.linkedin.restli.server.annotations.RestLiActions;
 import com.linkedin.restli.server.resources.ResourceContextHolder;
+import com.linkedin.restli.server.RestLiServiceException;
 
 @RestLiActions(name = "project", namespace = "azkaban.restli")
 public class ProjectManagerResource extends ResourceContextHolder {
@@ -53,8 +57,8 @@ public class ProjectManagerResource extends ResourceContextHolder {
   public String deploy(@ActionParam("sessionId") String sessionId,
       @ActionParam("projectName") String projectName,
       @ActionParam("packageUrl") String packageUrl)
-      throws ProjectManagerException, UserManagerException, ServletException,
-      IOException {
+      throws ProjectManagerException, RestLiServiceException, UserManagerException,
+      ServletException, IOException {
     logger.info("Deploy called. {sessionId: " + sessionId + ", projectName: "
         + projectName + ", packageUrl:" + packageUrl + "}");
 
@@ -65,8 +69,8 @@ public class ProjectManagerResource extends ResourceContextHolder {
     ProjectManager projectManager = getAzkaban().getProjectManager();
     Project project = projectManager.getProject(projectName);
     if (project == null) {
-      throw new ProjectManagerException("Project '" + projectName
-          + "' not found.");
+      String errorMsg = "Project '" + projectName + "' not found.";
+      throw new RestLiServiceException(HttpStatus.S_400_BAD_REQUEST, errorMsg);
     }
 
     if (!ResourceUtils.hasPermission(project, user, Permission.Type.WRITE)) {
@@ -74,7 +78,7 @@ public class ProjectManagerResource extends ResourceContextHolder {
           "User " + user.getUserId()
               + " has no permission to write to project " + project.getName();
       logger.error(errorMsg);
-      throw new ProjectManagerException(errorMsg);
+      throw new RestLiServiceException(HttpStatus.S_400_BAD_REQUEST, errorMsg);
     }
 
     logger.info("Target package URL is " + packageUrl);
@@ -84,7 +88,7 @@ public class ProjectManagerResource extends ResourceContextHolder {
     } catch (MalformedURLException e) {
       String errorMsg = "URL " + packageUrl + " is malformed.";
       logger.error(errorMsg, e);
-      throw new ProjectManagerException(errorMsg, e);
+      throw new RestLiServiceException(HttpStatus.S_400_BAD_REQUEST, errorMsg);
     }
 
     String filename = getFileName(url.getFile());
@@ -97,22 +101,51 @@ public class ProjectManagerResource extends ResourceContextHolder {
       // complete.
       logger.info("Downloading package from " + packageUrl);
       FileUtils.copyURLToFile(url, archiveFile);
-      Props props = new Props();
 
       logger.info("Downloaded to " + archiveFile.toString());
-      projectManager.uploadProject(project, archiveFile, "zip", user, props);
     } catch (IOException e) {
       String errorMsg =
           "Download of URL " + packageUrl + " to " + archiveFile.toString()
               + " failed";
       logger.error(errorMsg, e);
-      throw new ProjectManagerException(errorMsg, e);
+      if (tempDir.exists()) {
+        FileUtils.deleteDirectory(tempDir);
+      }
+      throw new RestLiServiceException(HttpStatus.S_400_BAD_REQUEST, errorMsg, e);
+    }
+
+    try {
+      // Check if project upload runs into any errors, such as the file
+      // having blacklisted jars
+      Props props = new Props();
+      Map<String, ValidationReport> reports = projectManager.uploadProject(project, archiveFile, "zip", user, props);
+      checkReports(reports);
+      return Integer.toString(project.getVersion());
+    } catch (ProjectManagerException e) {
+      String errorMsg = "Upload of project " + project + " from " + archiveFile + " failed";
+      logger.error(errorMsg, e);
+      throw e;
     } finally {
       if (tempDir.exists()) {
         FileUtils.deleteDirectory(tempDir);
       }
     }
-    return Integer.toString(project.getVersion());
+  }
+
+  void checkReports(Map<String, ValidationReport> reports) throws RestLiServiceException {
+    StringBuffer errorMsgs = new StringBuffer();
+    for (Map.Entry<String, ValidationReport> reportEntry : reports.entrySet()) {
+      ValidationReport report = reportEntry.getValue();
+      if (!report.getErrorMsgs().isEmpty()) {
+        errorMsgs.append("Validator " + reportEntry.getKey() + " reports errors: ");
+        for (String msg : report.getErrorMsgs()) {
+          errorMsgs.append(msg + System.getProperty("line.separator"));
+        }
+      }
+    }
+    if (errorMsgs.length() > 0) {
+      throw new RestLiServiceException(HttpStatus.S_400_BAD_REQUEST, errorMsgs.toString());
+    }
   }
 
   private String getFileName(String file) {
diff --git a/azkaban-web-server/src/test/java/azkaban/restli/DeployProjectTest.java b/azkaban-web-server/src/test/java/azkaban/restli/DeployProjectTest.java
new file mode 100644
index 0000000..a42abcc
--- /dev/null
+++ b/azkaban-web-server/src/test/java/azkaban/restli/DeployProjectTest.java
@@ -0,0 +1,131 @@
+/*
+ * Copyright 2016 LinkedIn Corp.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package azkaban.restli;
+
+import static org.junit.Assert.assertEquals;
+
+import azkaban.project.validator.ValidationReport;
+
+import com.linkedin.restli.common.HttpStatus;
+import com.linkedin.restli.server.RestLiServiceException;
+
+import java.util.HashSet;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Set;
+
+import org.junit.Test;
+
+/**
+ * Test response to deploy with either warn or error reports.
+ * If a report has any errors, a RestLiServiceException should be thrown.
+ * Warnings should not elicit an exception.
+ * If an exception does get raised, it should carry an HTTP 400 status
+ */
+
+public class DeployProjectTest {
+  @Test
+  public void testWarnedDeploy() {
+    ProjectManagerResource resource = new ProjectManagerResource();
+    Map<String, ValidationReport> reports = new LinkedHashMap<String, ValidationReport>();
+    for (int i = 0; i < 3; i++) {
+      addMockWarning(reports, "Test warn level info message.");
+    }
+    // We expect that no exceptions are thrown given a
+    // report with warnings. Anything thrown will result in failure
+    resource.checkReports(reports);
+  }
+
+  @Test
+  public void testErrorDeploy() {
+    ProjectManagerResource resource = new ProjectManagerResource();
+    Map<String, ValidationReport> reports = new LinkedHashMap<String, ValidationReport>();
+    for (int i = 0; i < 3; i++) {
+      addMockError(reports, "Test error level info message.");
+    }
+    // We expect that a RestLiServiceException is thrown given a
+    // report with errors. Uncaught exceptions will result in failure
+    try {
+      resource.checkReports(reports);
+    } catch (RestLiServiceException e) {
+      //Ensure we have the right status code and exit
+      assertEquals(e.getStatus(), HttpStatus.S_400_BAD_REQUEST);
+    }
+  }
+
+  @Test
+  public void testWarnErrorDeploy() {
+    ProjectManagerResource resource = new ProjectManagerResource();
+    Map<String, ValidationReport> reports = new LinkedHashMap<String, ValidationReport>();
+    for (int i = 0; i < 7; i++) {
+      // If i is even, make an error report, otherwise make a warning report
+      if (i % 2 == 0) {
+        addMockError(reports, "Test error level info message.");
+      }
+      else {
+        addMockWarning(reports, "Test warn level info message.");
+      }
+    }
+
+    // We expect that a RestLiServiceException is thrown given a
+    // report with errors. Uncaught exceptions will result in failure
+    try {
+      resource.checkReports(reports);
+    } catch (RestLiServiceException e) {
+      //Ensure we have the right status code and exit
+      assertEquals(e.getStatus(), HttpStatus.S_400_BAD_REQUEST);
+    }
+  }
+
+  /**
+   * Test that an error message is attached to the exception on an error
+   */
+  @Test
+  public void testErrorMessageDeploy() {
+    ProjectManagerResource resource = new ProjectManagerResource();
+    Map<String, ValidationReport> reports = new LinkedHashMap<String, ValidationReport>();
+
+    addMockError(reports, "This should show up.");
+
+    // We expect that a RestLiServiceException is thrown given a
+    // report with errors. Uncaught exceptions will result in failure
+    try {
+      resource.checkReports(reports);
+    } catch (RestLiServiceException e) {
+      //Ensure we have the right status code and exit
+      assertEquals(e.getStatus(), HttpStatus.S_400_BAD_REQUEST);
+      assertEquals(e.getMessage(), "Validator Error reports errors: This should show up."
+          + System.getProperty("line.separator"));
+    }
+  }
+
+  private void addMockWarning(Map<String, ValidationReport> reports, String msg) {
+    Set<String> warnMsgs = new HashSet<String>();
+    ValidationReport warnRpt = new ValidationReport();
+    warnMsgs.add(msg);
+    warnRpt.addWarningMsgs(warnMsgs);
+    reports.put("Warn", warnRpt);
+  }
+
+  private void addMockError(Map<String, ValidationReport> reports, String msg) {
+    Set<String> errorMsgs = new HashSet<String>();
+    ValidationReport errorRpt = new ValidationReport();
+    errorMsgs.add(msg);
+    errorRpt.addErrorMsgs(errorMsgs);
+    reports.put("Error", errorRpt);
+  }
+}