azkaban-aplcache

Merge pull request #352 from Victsm/validator-plugin Fix

11/13/2014 8:24:08 PM

Details

diff --git a/azkaban-common/src/main/java/azkaban/project/ProjectManager.java b/azkaban-common/src/main/java/azkaban/project/ProjectManager.java
index bd5b5a6..54be62a 100644
--- a/azkaban-common/src/main/java/azkaban/project/ProjectManager.java
+++ b/azkaban-common/src/main/java/azkaban/project/ProjectManager.java
@@ -54,7 +54,6 @@ public class ProjectManager {
   private ConcurrentHashMap<String, Project> projectsByName =
       new ConcurrentHashMap<String, Project>();
   private final ProjectLoader projectLoader;
-  private final ValidatorManager validatorManager;
   private final Props props;
   private final File tempDir;
   private final int projectVersionRetention;
@@ -81,7 +80,6 @@ public class ProjectManager {
     // itself.
     Props prop = new Props(props);
     prop.put(PROJECT_ARCHIVE_FILE_PATH, "initialize");
-    validatorManager = new XmlValidatorManager(prop);
     loadAllProjects();
   }
 
@@ -377,7 +375,17 @@ public class ProjectManager {
     // values are isolated from each other.
     Props prop = new Props(props);
     prop.put(PROJECT_ARCHIVE_FILE_PATH, archive.getAbsolutePath());
-    validatorManager.loadValidators(prop, logger);
+    // Basically, we want to make sure that for different invocations to the uploadProject method,
+    // the validators are using different values for the PROJECT_ARCHIVE_FILE_PATH configuration key.
+    // In addition, we want to reload the validator objects for each upload, so that
+    // we can change the validator configuration files without having to restart Azkaban web server.
+    // If the XmlValidatorManager is an instance variable, 2 consecutive invocations to the uploadProject
+    // method might cause the second one to overwrite the PROJECT_ARCHIVE_FILE_PATH configuration parameter
+    // of the first, thus causing a wrong archive file path to be passed to the validators. Creating a
+    // separate XmlValidatorManager object for each upload will prevent this issue without having to add
+    // synchronization between uploads. Since we're already reloading the XML config file and creating
+    // validator objects for each upload, this does not add too much additional overhead.
+    ValidatorManager validatorManager = new XmlValidatorManager(prop);
     logger.info("Validating project " + archive.getName() + " using the registered validators "
         + validatorManager.getValidatorsInfo().toString());
     Map<String, ValidationReport> reports = validatorManager.validate(file);
diff --git a/azkaban-common/src/main/java/azkaban/project/validator/XmlValidatorManager.java b/azkaban-common/src/main/java/azkaban/project/validator/XmlValidatorManager.java
index c231b0a..4851f88 100644
--- a/azkaban-common/src/main/java/azkaban/project/validator/XmlValidatorManager.java
+++ b/azkaban-common/src/main/java/azkaban/project/validator/XmlValidatorManager.java
@@ -52,10 +52,11 @@ public class XmlValidatorManager implements ValidatorManager {
   public static final String ITEM_TAG = "property";
   public static final String DEFAULT_VALIDATOR_KEY = "Directory Flow";
 
+  private static Map<String, Long> resourceTimestamps = new HashMap<String, Long>();
+  private static ClassLoader validatorLoader;
+
   private Map<String, ProjectValidator> validators;
-  private Map<String, Long> resourceTimestamps;
   private String validatorDirPath;
-  private ClassLoader validatorLoader;
 
   /**
    * Load the validator plugins from the validator directory (default being validators/) into
@@ -72,26 +73,10 @@ public class XmlValidatorManager implements ValidatorManager {
           + " does not exist or is not a directory.");
     }
 
-    resourceTimestamps = new HashMap<String, Long>();
-    List<URL> resources = new ArrayList<URL>();
-    try {
-      logger.info("Adding validator resources.");
-      // Find JAR files only if the validator directory exists
-      if (validatorDir.canRead() && validatorDir.isDirectory()) {
-        for (File f : validatorDir.listFiles()) {
-          if (f.getName().endsWith(".jar")) {
-            resourceTimestamps.put(f.getName(), f.lastModified());
-            resources.add(f.toURI().toURL());
-            logger.debug("adding to classpath " + f.toURI().toURL());
-          }
-        }
-      }
-    } catch (MalformedURLException e) {
-      throw new ValidatorManagerException(e);
-    }
-    validatorLoader = new URLClassLoader(resources.toArray(new URL[resources.size()]));
+    // Check for updated validator JAR files
+    checkResources();
 
-    // Test loading the validators specified in the xml file.
+    // Load the validators specified in the xml file.
     try {
       loadValidators(props, logger);
     } catch (Exception e) {
@@ -154,9 +139,6 @@ public class XmlValidatorManager implements ValidatorManager {
       return;
     }
 
-    // Check for updated validator JAR files
-    checkResources();
-
     // Creating the document builder to parse xml.
     DocumentBuilderFactory docBuilderFactory =
         DocumentBuilderFactory.newInstance();